Re: [Xen-devel] [PATCH V3 3/6] x86/xsaves: enable xsaves/xrstors for hvm guest

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/08/15 08:59, Shuai Ruan wrote:
> On Fri, Aug 07, 2015 at 02:04:51PM +0100, Andrew Cooper wrote:
>> On 07/08/15 09:22, Shuai Ruan wrote:
>>>>>  void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>>>>>                                     unsigned int *ecx, unsigned int *edx)
>>>>>  {
>>>>> @@ -4456,6 +4460,34 @@ void hvm_cpuid(unsigned int input, unsigned int 
>>>>> *eax, unsigned int *ebx,
>>>>>                      *ebx = _eax + _ebx;
>>>>>              }
>>>>>          }
>>>>> +        if ( count == 1 )
>>>>> +        {
>>>>> +            if ( cpu_has_xsaves )
>>>>> +            {
>>>>> +                *ebx = XSTATE_AREA_MIN_SIZE;
>>>>> +                if ( v->arch.xcr0 | v->arch.msr_ia32_xss )
>>>>> +                    for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ )
>>>>> +                    {
>>>>> +                        if ( !((v->arch.xcr0 | v->arch.msr_ia32_xss)
>>>>> +                    & (1ULL << sub_leaf)) )
>>>>> +                            continue;
>>>>> +                        domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, 
>>>>> &_ecx,
>>>>> +                                     &_edx);
>>>>> +                        *ebx =  *ebx + _eax;
>>>>> +                    }
>>>>> +            }
>>>>> +            else
>>>>> +            {
>>>>> +                *eax &= ~XSAVES;
>>>>> +                *ebx = *ecx = *edx = 0;
>>>>> +            }
>>>>> +            if ( !cpu_has_xgetbv1 )
>>>>> +                *eax &= ~XGETBV1;
>>>>> +            if ( !cpu_has_xsavec )
>>>>> +                *eax &= ~XSAVEC;
>>>>> +            if ( !cpu_has_xsaveopt )
>>>>> +                *eax &= ~XSAVEOPT;
>>>>> +        }
>>>> Urgh - I really need to get domain cpuid fixed in Xen.  This is
>>>> currently making a very bad situation a little worse.
>>>>
>>> In patch 4, I expose the xsaves/xsavec/xsaveopt and need to check
>>> whether the hardware supoort it. What's your suggestion about this?
>> Calling into domain_cpuid() in the loop is not useful as nothing will
>> set the subleaves up.  As a first pass, reading from
>> xstate_{offsets,sizes} will be better than nothing, as it will at least
> What do you mean by xstate_{offsets,sizes}?

Shorthand for xstate_offsets xstate_sizes, per the standard shell expansion.

>> match reality until the domain is migrated.
>>
> For CPUID(eax=0dh) with subleaf 1, the value of ebx will change
> according to the v->arch.xcr0 | v->arch.msr_ia32_xss. So add
> code in hvm_cpuid function is the best way I can think of. Your
> suggestions :)?

Which is liable to change on different hardware.  Once a vm has
migrated, Xen may not legitimately execute another cpuid instruction as
part of emulating guest cpuid, as it is not necessarily accurate.

Xen currently does not currently have proper cpuid encapsulation, which
causes host-specific details to leak into guests across migrate.  I have
a longterm plan to fix it, but it is not simple or quick to do.

In this case, reading from xstate_{offsets,sizes} is better than
nothing, but will need fixing in the longterm.

~Andrew

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