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

[Shuai Ruan <shuai.ruan@linux.intel.com>]

On Tue, Aug 11, 2015 at 10:37:56AM +0100, Andrew Cooper wrote:
> 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.
> 
You mean xstate_offsets xstate_sizes I introduced in patch 5.
> >> 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.
> 
Ok, I will fix this in next version.
> ~Andrew
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>