Re: [Qemu-devel] [PATCH 14/14] target-i386: Enable XCR0 features for user-mode

[Richard Henderson <rth@twiddle.net>]

On 07/09/2015 02:15 PM, Paolo Bonzini wrote:
> On 09/07/2015 10:17, Richard Henderson wrote:
>> +
>> +    /* ??? This variable is somewhat silly.  Methinks KVM should be
>> +       using XCR0 to store into the XSTATE_BV field.  Either that or
>> +       there's more missing information, e.g. the AVX bits.  */
>> +    env->xstate_bv = XSTATE_FP;
>> +    if (env->features[FEAT_1_EDX] & CPUID_SSE) {
>> +        env->xstate_bv |= XSTATE_SSE;
>> +    }
>> +    if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_MPX) {
>> +        env->xstate_bv |= XSTATE_BNDREGS | XSTATE_BNDCSR;
>> +    }
>
> xstate_bv != xcr0 if the kernel is using XSAVEOPT and some of the values
> were in the initial state.  Legacy state is never optimized, hence the
> value of env->xstate_bv after reset.  So I think this hunk is wrong,
> while the other is correct.

Yes, it's a copy of the field of the same name from the xsave format.

Have we stopped using tcg entirely when kvm is enabled?  I guess so, since I 
seem to recall an effort to build qemu without tcg support.  So any fears about 
tcg corrupting kvm state would be unfounded, right?

If so, I can see how this variable aids initial xsave construction as well as 
copying the xsave block during migration.

It does beg the question of why xstate_bv isn't zero at reset.  Surely all of 
the xmm and fpu registers are in INIT state at this time, and that's what the 
XRSTOR that will consume this block is going to care about.


r~