Re: [Xen-devel] [PATCH] x86emul: handle address wrapping for VMASKMOVP{S, D}

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/10/17 08:44, Jan Beulich wrote:
>>>> On 10.10.17 at 14:43, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 10/10/17 11:43, Jan Beulich wrote:
>>> There's another issue here, but I'll first have to think about possible
>>> (preferably non-intrusive) solutions: An access crossing a page
>>> boundary and having
>>> - a set mask bit corresponding to an element fully living in the first
>>>   page,
>>> - one or more clear mask bits corresponding to the initial elements on
>>>   the second page,
>>> - another higher mask bit being set
>>> would result in a wrong CR2 value to be reported in case the access to
>>> the second page would cause a fault (it would point to the start of the
>>> page instead of the element being accessed). Neither splitting the
>>> access here into multiple ones nor uniformly passing a byte or word
>>> enable mask into ->write() seem very desirable.
>> Is this just supposition, or have you confirmed what really happens on
>> hardware?
> I had done some experiments already at the time I wrote this.
> I've now done more. If the fault occurs on the high page, the CR2
> value (on Haswell) depends on whether there was an enabled
> access to the low page. If so, CR2 holds the address of the last
> byte of the overall range (even if the highest mask bit is clear). If
> not, CR2 holds the address of the lowest byte actually being
> accessed.

The answer appears to be "its complicated".

>> I expect that the mask operations turn into multi-part operations, which
>> means their behaviour on a straddled fault is implementation defined
>> (and behaves differently between Atoms and Xeons).
>>
>> One option we could do is to have a variation of the "Implement
>> hvmemul_write() using real mappings" logic where we pull mappings into
>> the vmap individually, but that would require some part of the code to
>> convert ea + mask => linear address of each unit, so the eventual
>> mapping can be constructed piece-wise.
> I certainly have no Atom to play with - on the Haswell, the write
> to the first page does not take effect when the access to the
> second page faults. Hence splitting the accesses (as suggested
> as an option above) clearly would not be valid.

Not so.  If the behaviour is implementation defined, splitting the
accesses in the emulator would be valid, even if it differs from
hardware behaviour.

The problem here is working out whether the behaviour here is
implementation defined, or whether it defined as having atomic
properties.  I am not aware of any statement in any of the vendor
manuals which confirms the atomic properties, or declares the behaviour
as implementation defined.

>
> On my Dinar (AMD Fam15), otoh, CR2 always points at the
> lowest byte of the actually accessed field(s) on the page causing
> the fault (i.e. the behavior I had implied in my original remark to
> the patch).

This is certainly more logical behaviour.

>
> Perhaps to avoid passing byte/word enables into ->read and
> ->write() (not sure why originally I had thought of the latter
> only) we could extend struct pagefault_info to include an
> "offset from start of accessed range" field, to be passed into
> x86_emul_pagefault() in addition to the current arguments?

An interesting question would be whether an access with no mask bits set
in the first page faults if the first page is not present.  This would
help identify whether a pagewalk is done for start of the access, or
whether the first page is omitted entirely when the mask permits.  (I
can see the latter behaviour being faster, but perhaps more complicated
in silicon, so I'm having a hard time judging is likely to happen).


At the end of the day, this is clearly a corner case which behaves
differently in different circumstances, which gives us some flexibility
in how we fix it.  I'd opt for atomic behaviour wherever possible,
because it is clearly how at least one piece of hardware actually
behaves, and it is the safer option to take e.g. for emulation caused by
introspection.  So long as CR2 points to the correct 4k frame, demand
paging will work inside the guest, so I don't think we should worry too
much about exactly where it points.

~Andrew

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