Re: [Xen-devel] Future x86 emulator direction

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.12.16 at 13:32, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 14/12/16 07:37, Razvan Cojocaru wrote:
>> On 12/14/2016 09:14 AM, Jan Beulich wrote:
>>>>>> On 13.12.16 at 23:02, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> On 13/12/2016 21:55, Razvan Cojocaru wrote:
>>>>> On a somewhat related note, it's important to also figure out how best
>>>>> to avoid emulation races such as the LOCK CMPXCHG issue we've discussed
>>>>> in the past. Maybe that's also worth taking into consideration at this
>>>>> early stage.
>>>> Funny you should ask that.
>>>>
>>>> The only possible way to do this safely is to have the emulator map the
>>>> target frame(s) and execute a locked stub instruction with a memory
>>>> operand pointing at the mapping.  We have no other way of interacting
>>>> with the cache coherency fabric.
>>> Well, that approach is necessary only if one path (vCPU) can write
>>> to a page, while another one needs emulation. If pages are globally
>>> write-protected, an approach following the model from Razvan's
>>> earlier patch (which I have no idea what has become of) would
>>> seem to suffice.
> 
> How do you suggest evaluating whether the page is read-only, without
> doing 99% of the work necessary to map it anyway?

That wasn't my point. I merely wanted to express that for a limited
set of use cases not using the stub approach would suffice.
Meanwhile I've in fact realized that there's another relevant aspect
requiring us to use an actual CMPXCHG here: Other than a CMP-and-
MOV sequence, CMPXCHG always writes its memory operand, which
for MMIO may make a difference.

>>> I think the shadow cmpxchg() hook is nearly there, although the
>>> implementation does need to up into the body of the emulator along with
>>> a map()/unmap() pair of primitives.
>>
>> To be honest, I don't think we should have map/unmap primitives.
>> Instead the cmpxchg() hook needs to be told enough to properly
>> do its job.
> 
> The current implementation of ->cmpxchg() is certainly sub-par.
> 
> However, you can't sensibly implement something like `lock btc ...` in
> terms of cmpxchg(), so fixing the behaviour of cmpxchg() isn't
> sufficient to fix the atomicity problems.

Well, I've realized that too. The set of LOCK-able insn luckily is
rather small, so I think we instead may want a ->locked_rmw()
op, which all guest uses of LOCK should go to (with an enum
specifying the actual operation).

Jan


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