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

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "Juergen Gross" <jgross@suse.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH 2/2] x86emul: adjust MOVSXD source operand handling
Date: Fri, 4 Oct 2019 13:32:08 +0100	[thread overview]
Message-ID: <39f3558a-b04a-eb9e-ccf7-872b87aa2d9a@citrix.com> (raw)
In-Reply-To: <5bd70e24-4a0f-20ef-c847-3bc82aa35325@suse.com>

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel