Re: [Xen-devel] [PATCH] x86/PV: don't hide CPUID.OSXSAVE from user mode

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.08.16 at 18:13, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 16/08/16 17:00, Jan Beulich wrote:
>>>>> On 16.08.16 at 17:41, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 16/08/16 16:20, Jan Beulich wrote:
>>>> User mode code generally cannot be expected to invoke the PV-enabled
>>>> CPUID Xen supports, and prior to the CPUID levelling changes for 4.7
>>>> (as well as even nowadays on levelling incapable hardware) such CPUID
>>>> invocations actually saw the host CR4.OSXSAVE value. Fold in the guest
>>>> view of CR4.OSXSAVE when setting the levelling MSRs, just like we do
>>>> in other CPUID handling.
>>> How does this work?  The OSXSAVE is a fast-forwarded bit, not a regular bit.
>>>
>>> There is nothing you can do to control it on Intel, as the MSRs are
>>> strictly and AND mask, applied before OSXSAVE and APIC are fast
>>> forwarded from real hardware state.
>> Considering that the change works (and things didn't work before) I
>> assume the AND-ing happens after the fast forwarding.
> 
> That is specifically contrary to my findings.  What hardware is this
> on?  (Given the undocumented state of the rest of masking, I wouldn't be
> surprised if it differed across models).

Sandybridge.

>>> On AMD, you can force it to zero by clearing the OSXSAVE bit, but you
>>> can never cause it to appear set if Xen has it cleared in CR4.
>> We don't allow guests to use XSAVE (and hence set the bit in CR4) if
>> we don't enable it ourselves. Hence if it's off in Xen, it'll be off
>> everywhere else (and that's what we want); i.e. in the consideration
>> of how this works, please assume CR4.OSXSAVE=1 for the raw
>> hardware reg.
> 
> And I presume the usecase is to hide it from guest userspace if it is
> not enabled in the guest kernel?

It's actually both ways: Hide it when not enabled in the guest kernel
(I think that case already works, simply because without the patch
it's always hidden, and the patch sets the flag only when guest CR4
has it on) and expose it when the guest kernel has it enabled (that's
the case which didn't work so far, easily exposed by running the
x86emul test tool on a PV guest, e.g. Dom0).

> On the AMD side, this is a simple two-liner
> 
> /* Force OSXSAVE to zero if not enabled by the guest kernel. */
> if (masks != &cpuidmask_defaults && !(current->arch.pv_vcpu.ctrlreg[4] &
> X86_CR4_OSXSAVE))
>     masks->_1cd &= ~cpufeat_mask(X86_FEATURE_OSXSAVE);
> 
> to counteract the default set up in update_domain_cpuid_info().

Oh, indeed, I didn't notice update_domain_cpuid_info() inverting
the sense already for AMD. Exactly what you suggest above
won't work, but I'll have to modify the AMD case of the patch to
AND out the bit instead of OR-ing it in. I guess I'll wait with v2
until I can actually test it on my AMD box.

Jan


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