Xen devel - Apr 2006 - RE: [PATCH] add support for XCHG instruction accessingAPIC

I think the reason we don''t take any lock here is; we are sure this is
just used for local APIC range. However, if in future someone adds another MMIO
range or if IOAPIC is accessed with XCHG (will this happen?), this may have
potential issue.

So I''d suggest adding comments that we didn''t take any lock
here, or add a check to make sure the range is on local APIC range.

Thanks
Yunhong Jiang
>-----Original Message-----
>From: xen-devel-bounces@lists.xensource.com
>[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Boris Ostrovsky
>Sent: 2006年4月5日 7:26
>To: Keir Fraser
>Cc: xen-devel@lists.xensource.com
>Subject: Re: [Xen-devel] [PATCH] add support for XCHG instruction
accessingAPIC
>
>Keir Fraser wrote:
>
>>>
>>> My only argument in favor of using the lock would be for
completeness
>>> of the emulation. You are
>>> absolutely right in that for Linux there seems to be no need to
hold
>>> the lock. My concern is that
>>> other OSs  may treat this differently. And if we don''t
have sources,
>>> it may be somewhat difficult
>>> to  figure out that the atomicity (or lack of it) was the cause of
a
>>> problem.
>>>
>>> If, however, there is a strong feeling that we don''t need
the lock, I
>>> am happy to drop it.
>>> I guess you are mostly unhappy about adding a new field to
>>> hvm_domain, not about performance
>>> impact?
>>
>>
>> Yes, also my second argument was that there is *no way* for two VCPUs
>> to conflict on a local APIC access, since LAPIC accesses are always to
>> the VCPU''s own LAPIC. So there is no potential concurrency
that needs
>> to be serialised, regardless of the guest OS.
>
>
>OK, that''s fair. Here is updated patch with lock removed.
>
>I don''t think I then understand why Linux is using atomic accesses
to
>local APICs. It''s interesting though that 64-bit code
doesn''t do it ---
>they use vanilla apic_write().
>
>-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 5 Apr 2006, at 17:50, Jiang, Yunhong wrote:
> I think the reason we don''t take any lock here is; we are sure
this is
> just used for local APIC range. However, if in future someone adds 
> another MMIO range or if IOAPIC is accessed with XCHG (will this 
> happen?), this may have potential issue.
>
> So I''d suggest adding comments that we didn''t take any
lock here, or
> add a check to make sure the range is on local APIC range.
There is a comment in the patch I applied, and that section of code is 
specific to LAPIC handling.

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel