RE: [Xen-ia64-devel][PATCH][VTD] small patches for VTD

["Xu, Anthony" <anthony.xu@xxxxxxxxx>]

Isaku Yamahata wrote:
> On Thu, Oct 23, 2008 at 03:10:55PM +0800, Xu, Anthony wrote:
>> The new one,
> 
> It looks almost okay. The last one.
> 
> 
>> diff -r 02c8733e2d91 xen/arch/ia64/vmx/viosapic.c
>> --- a/xen/arch/ia64/vmx/viosapic.c   Wed Oct 22 17:20:15 2008 +0900
>> +++ b/xen/arch/ia64/vmx/viosapic.c   Thu Oct 23 14:48:09 2008 +0800 @@
>>                       -121,6 +121,13 @@ redir_num, vector);
>>          return;
>>      }
>> +    if ( iommu_enabled )
>> +    {
>> +        spin_unlock(&viosapic->lock);
>> +        hvm_dpci_eoi(current->domain, redir_num,
>> &viosapic->redirtbl[redir_num]); +        spin_lock(&viosapic->lock);
>> +    }
>> +
>>      service_iosapic(viosapic);
>>      spin_unlock(&viosapic->lock);
>>  }
> 
> Is this unlock/lock sequence okay?
> I'm asking simply because I'm not sure.
> 
> viosapic->irr and isr are protected by the lock.
> And viosapic_update_EOI() updates them atomically.
> The above unlock/lock seems to break its atomicity.

I think it is Okay,
One atomical operation in viosapic_update_EOI is divided into two atomical 
operations.
If you get spin_lock again, when returning from hvm_dpci_eoi.
There are many code segments in linux kernel.
And viosapic->irr and isr is still protected by lock.

Anthony

> 
> I'm not sure it's okay or not. To make sure, it is required
> to take closer look at viosapic.c.
> 
> thanks,
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel