Re: [Xen-devel] [PATCH 2/2] x86emul: adjust MOVSXD source operand handling

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/10/2019 10:17, Jan Beulich wrote:
> On 02.10.2019 10:51, Andrew Cooper wrote:
>> On 02/10/2019 08:07, Jan Beulich wrote:
>>> On 01.10.2019 21:44, Andrew Cooper wrote:
>>>> In this example, hardware can the emulator can disagree by using a
>>>> different access width.
>>>>
>>>> I've been experimenting with my Rome system, and an emulator hardcoded
>>>> to use 2-byte accesses.  After some investigation, the livelock only
>>>> occurs for access-rights faults.  Translation faults get identified as
>>>> not a shadow fault, and bounced back to the guest.
>>>>
>>>> Shadow guests can use PKRU, so can generate an access fault by marking
>>>> the page at 0x2000 as no-access, so I think that in principle, this
>>>> change will result in a new latent livelock case, but I can't actually
>>>> confirm it.
>>> I think I see what you mean, but then I don't see how this is an
>>> argument against the patch in its current shape: It actually
>>> reduces the cases of disagreement between hardware and emulator.
>> At the moment, the emulator is strictly 4 bytes, and hardware may be 4
>> or 2.  Therefore, there is no chance of the pipeline yielding #PF while
>> the emulator yielding OK.
> At the expense of possibly yielding #PF when the pipeline wouldn't.

Right, but given the differing behaviour, no code can reasonably expect
not to get the second #PF.

>> With the change in place, older Intel parts which do use a 4 byte access
>> now come with a risk of livelock.  Whichever parts these are, they
>> predate PKRU so in this specific case, the problem is only theoretical
>> AFAICT.
> Plus at this point we don't even know whether there are any such
> parts.

I'll see if I can find out.  Given this is a 64-bit only instruction, it
is possible that Intel has always had the described behaviour, and it
was just the documentation which was incorrect.

>> Also, in this specific case, Intel's warning of "Don't use this
>> instruction without a REX prefix" means that we shouldn't see it in
>> anything but test scenarios.
> It's extremely unlikely at least.
>
>>> One possibility to make a further step in that direction would
>>> be to make behavior dependent upon the underlying hardware's
>>> vendor, rather than the one the guest sees.
>> I considered this.  It would work on native (at the expense of
>> complicating the emulator), but won't work properly if Xen is
>> virtualisied and migrated.  I can't see a way around this.
> Are you concerned about Xen getting cross-vendor migrated? If
> you'd accept things to not be 100% right in such a case, I could
> simply probe hardware while booting.

To be honest, when Xen isn't L0, all of this is liable to break under
our feet.  I don't think probing at boot is going to provide a
meaningful improvement on that.

For this specific movxsd change, lets call it Acked-by me.  Whatever the
behaviour on ancient processors, the livelock case is safe because they
don't support PKRU.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel