Re: [Xen-devel] [PATCH] x86/cpuid: fix dom0 crash on skylake machine

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 01/06/16 10:17, Jan Beulich wrote:
>>>> On 01.06.16 at 11:03, <andrew.cooper3@xxxxxxxxxx> wrote:
>> While this does work, it undoes some of the work I started with my cpuid
>> improvements in 4.7
>>
>> Does the attached patch also resolve your issue?
> While that's much better than the original, I don't think it's quite
> enough. The rest of the domain policy should be taken into account
> (and I think I had suggested to do so during review of your CPUID
> rework series), i.e. this can't be calculated once for every domain.

Like the current use of {hvm,pv}_featureset, as an upper bound, this is
just a stopgap fix.

Fixing this in a properly per-domain way is part of my further plans for
cpuid improvements.  The reason it isn't done like this yet is because
there is a substantial quantity of work required to make this function.

> And then, as said in reply to the original patch, handle_xsetbv()'s
> checking should be generalized from the special casing of PKRU (or
> if we don't want that, then that special case would better get
> removed for consistency reasons).

Oh - I hadn't even noticed that.  How about this incremental change?

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index a0cfcc2..67c0e4b 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -658,8 +658,8 @@ int handle_xsetbv(u32 index, u64 new_bv)
     if ( (new_bv & ~xfeature_mask) || !valid_xcr0(new_bv) )
         return -EINVAL;
 
-    /* XCR0.PKRU is disabled on PV mode. */
-    if ( is_pv_vcpu(curr) && (new_bv & XSTATE_PKRU) )
+    /* Sanity check against domain maximum. */
+    if ( new_bv & ~(is_pv_vcpu(curr) ? pv_xfeature_mask :
hvm_xfeature_mask) )
         return -EOPNOTSUPP;
 
     if ( !set_xcr0(new_bv) )


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