Re: [Xen-devel] Future x86 emulator direction

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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?

> As previously stated, you've raised performance concerns which seemed to
> require a different direction, namely the one Andrew is now suggesting,
> which indeed, aside from being somewhat faster is also safer for all
> cases (including the one you've mentioned, where one path can write
> normally and the other does so via emulation).
>
> The old patch itself is still alive in the XenServer patch queue, albeit
> quite unlikely to be trivial to apply to the current Xen 4.9-unstable
> code in its current form:
>
> https://github.com/xenserver/xen-4.7.pg/blob/master/master/xen-x86-emulate-syncrhonise-LOCKed-instruction-emulation.patch
>
> Again, if you decide that this patch is preferable, I can try to rework
> it for the current version of Xen.

FWIW, I always have a version of the XenServer patch queue rebased on to
staging, which is how I submit proposed upstream development work for
testing in the XenServer automation system.  (This particular patch has
been a constant source of textural merge conflicts with the recent
emulator development work)

>> 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.

~Andrew

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