Re: [PATCH v3 03/22] x86/xstate: re-size save area when CPUID policy changes

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "George Dunlap" <george.dunlap@citrix.com>,
	"Ian Jackson" <iwj@xenproject.org>,
	"Julien Grall" <julien@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v3 03/22] x86/xstate: re-size save area when CPUID policy changes
Date: Tue, 11 May 2021 17:41:54 +0100	[thread overview]
Message-ID: <f16afc8a-ccd4-7e5e-e08d-d96597c6e8ab@citrix.com> (raw)
In-Reply-To: <f515fdfb-d1a6-56d8-5db3-ebddeed23806@suse.com>

On 03/05/2021 15:22, Jan Beulich wrote:
>> Another consequence is that we need to rethink our hypercall behaviour. 
>> There is no such thing as supervisor states in an uncompressed XSAVE
>> image, which means we can't continue with that being the ABI.
> I don't think the hypercall input / output blob needs to follow any
> specific hardware layout.

Currently, the blob is { xcr0, xcr0_accum, uncompressed image }.

As we haven't supported any compressed states yet, we are at liberty to
create a forward compatible change by logically s/xcr0/xstate/ and
permitting an uncompressed image.

Irritatingly, we have xcr0=0 as a permitted state and out in the field,
for "no xsave state".  This contributes a substantial quantity of
complexity in our xstate logic, and invalidates the easy fix I had for
not letting the HVM initpath explode.

The first task is to untangle the non-architectural xcr0=0 case, and to
support compressed images.  Size parsing needs to be split into two, as
for compressed images, we need to consume XSTATE_BV and XCOMP_BV to
cross-check the size.

I think we also want a rule that Xen will always send compressed if it
is using XSAVES (/XSAVEC in the interim?)  We do not want to be working
with uncompressed images at all, now that MPX is a reasonable sized hole
in the middle.

Cleaning this up will then unblock v2 of the existing xstate cleanup
series I posted.

>> In terms of actual context switching, we want to be using XSAVES/XRSTORS
>> whenever it is available, even if we're not using supervisor states. 
>> XSAVES has both the inuse and modified optimisations, without the broken
>> consequence of XSAVEOPT (which is firmly in the "don't ever use this"
>> bucket now).
> The XSAVEOPT anomaly is affecting user mode only, isn't it? Or are
> you talking of something I have forgot about?

It's not safe to use at all in L1 xen, because the tracking leaks
between non-root contexts.  I can't remember if there are further
problems for an L0 xen, but I have a nagging feeling that there is.

~Andrew