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

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

>>> On 30.03.17 at 16:08, <rcojocaru@bitdefender.com> wrote:
> On 03/30/2017 03:56 PM, Razvan Cojocaru wrote:
>> On 03/30/2017 03:05 PM, Jan Beulich wrote:
>>> What I do note though is that you don't copy back the value
>>> __cmpxchg() returns, yet that's what is needed. *map may
>>> have changed again already.
>> 
>> Changing the code to:
>> 
>> 1162     ret = __cmpxchg(map, old, new, bytes);
>> 1163
>> 1164     if ( ret != old )
>> 1165     {
>> 1166         memcpy(p_old, &ret, bytes);
>> 1167         rc = X86EMUL_CMPXCHG_FAILED;
>> 1168     }
>> 
>> where ret is an unsigned long still triggers BSODs when I add my patch
>> to yours. I'll need to dig deeper.
> 
> Nevermind, I've found the culprit: hvm_emulate_one_vm_event()'s code
> needs to be wrapped in a loop that checks for X86EMUL_RETRY again, since
> hvmemul_cmpxchg() may return RETRY even for some mapping problems, in
> which case we again end up with the guest trying to re-execute an
> emulable CMPXCHG.

This seems wrong to me - note how my patch changes behavior
regarding the return value from paging_cmpxchg_guest_entry()
in ptwr_emulated_update().

> However, this gets me back to my original problem when I "solved" it in
> the same manner (looping until emulation succeeds) back when
> hvmemul_cmpxchg() failures were reported with RETRY: eventually the
> guest BSODs with code 101. RETRY failures are still possible coming from
> the hvmemul_vaddr_to_mfn() code in my patch.
> 
> I wonder if I should just return X86EMUL_CMPXCHG_FAILED for all those as
> well and just never end up returning RETRY from hvmemul_cmpxchg().

That would seem similarly wrong to me - it ought to be
UNHANDLEABLE, I would think. In any event, never returning
RETRY would also be wrong in case you hit emulated MMIO, but
that's really the only case where RETRY should be passed back.

Jan


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