Re: [PATCH RFC] x86/emulate: implement hvmemul_cmpxchg() with an actual CMPXCHG

["Jan Beulich" <JBeulich@suse.com>]

>>> On 28.03.17 at 12:25, <andrew.cooper3@citrix.com> wrote:
> On 28/03/17 11:03, Jan Beulich wrote:
>>>>> On 28.03.17 at 11:14, <rcojocaru@bitdefender.com> wrote:
>>> I'm not sure that the RETRY model is what the guest OS expects. AFAIK, a
>>> failed CMPXCHG should happen just once, with the proper registers and ZF
>>> set. The guest surely expects neither that the instruction resume until
>>> it succeeds, nor that some hidden loop goes on for an undeterminate
>>> ammount of time until a CMPXCHG succeeds.
>> The guest doesn't observe the CMPXCHG failing - RETRY leads to
>> the instruction being restarted instead of completed.
> 
> And this probably is the root of the problem.  CMPXCHG on a contended
> location should be observed to fail from the guests point of view.

Oops - my reply was imprecise: A _guest_ CMPXCHG would of
course be observed to have failed by the guest. Us using
CMPXCHG to carry out some other atomic op wouldn't.

>>> The picture is further complicated by the two-part handling of
>>> instructions in x86_emulate(). For example, for CMPXCHG we have:
>>> [...]
>>> I now see that this is why using a spinlock only around the writeback
>>> part did not solve the issue: both the compare part and the writeback
>>> part need to be part of the same atomic operation - lock needs to be
>>> aquired before the cmp / ZF part.
>> No exactly: RMW insns require the lock to be acquired ahead of
>> the memory read, and dropped after the memory write.
>>
>>> Opinions and suggestions are appreciated. If I'm not mistaken, it looks
>>> like the smp_lock design is the better solution to the problem.
>> Andrew has told me that the re-work of how we do memory accesses
>> in the emulator is pretty high on his priority list now. Whether we'd
>> want to introduce an interim solution therefore depends on the time
>> scale to possibly reach the only ultimately correct solution (no longer
>> acting on intermediate copies of the data coming from guest memory,
>> which is only correct for plain reads and plain writes).
> 
> This re-enforces my opinion that mapping the destination and pointing a
> stub at the mapping is the only viable option.

Well, you re-state what I was saying.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel