Re: [Xen-devel] [PATCH] Fix PV CPUID virtualization of XSave

[Haitao Shan <maillists.shan@xxxxxxxxx>]

Jan, Keir,

Updated patch is attached.

The patch will fix XSave CPUID virtualization for PV guests. The XSave
area size returned by CPUID leaf D is changed dynamically depending on
the XCR0. Tools/libxc only assigns a static value. The fix will adjust
xsave area size during runtime.

Note: This fix is already in HVM cpuid virtualization. And Dom0 is not
affected, either.

Signed-off-by:  Shan Haitao <haitao.shan@xxxxxxxxx>

diff -r e90438f6e6d1 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c    Wed Sep 14 11:38:13 2011 +0100
+++ b/xen/arch/x86/hvm/hvm.c    Mon Sep 19 14:34:15 2011 +0800
@@ -2426,7 +2426,7 @@ void hvm_cpuid(unsigned int input, unsig
         {
             /* reset EBX to default value first */
             *ebx = XSTATE_AREA_MIN_SIZE;
-            for ( sub_leaf = 2; sub_leaf < 64; sub_leaf++ )
+            for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ )
             {
                 if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) )
                     continue;
diff -r e90438f6e6d1 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c      Wed Sep 14 11:38:13 2011 +0100
+++ b/xen/arch/x86/traps.c      Mon Sep 19 14:34:15 2011 +0800
@@ -770,6 +770,30 @@ static void pv_cpuid(struct cpu_user_reg
     {
         if ( !cpuid_hypervisor_leaves(a, c, &a, &b, &c, &d) )
             domain_cpuid(current->domain, a, c, &a, &b, &c, &d);
+
+        switch ( a )
+        {
+        case 0xd:
+        {
+            unsigned int sub_leaf, _eax, _ebx, _ecx, _edx;
+            /* EBX value of main leaf 0 depends on enabled xsave features */
+            if ( c == 0 && current->arch.xcr0 )
+            {
+                /* reset EBX to default value first */
+                b = XSTATE_AREA_MIN_SIZE;
+                for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ )
+                {
+                    if ( !(current->arch.xcr0 & (1ULL << sub_leaf)) )
+                        continue;
+                    domain_cpuid(current->domain, a, c, &_eax, &_ebx, &_ecx,
+                                 &_edx);
+                    if ( (_eax + _ebx) > b )
+                        b = _eax + _ebx;
+                }
+            }
+        break;
+        }
+        }
         goto out;
     }



2011/9/18 Haitao Shan <maillists.shan@xxxxxxxxx>:
> 2011/9/16 Jan Beulich <JBeulich@xxxxxxxx>:
>>>>> On 16.09.11 at 02:46, Haitao Shan <maillists.shan@xxxxxxxxx> wrote:
>>> Hi, Keir,
>>>
>>> The patch will fix XSave CPUID virtualization for PV guests. The XSave
>>> area size returned by CPUID leaf D is changed dynamically depending on
>>> the XCR0. Tools/libxc only assigns a static value. The fix will adjust
>>> xsave area size during runtime.
>>>
>>> Note: This fix is already in HVM cpuid virtualization. And Dom0 is not
>>> affected, either.
>>>
>>> Signed-off-by:  Shan Haitao <haitao.shan@xxxxxxxxx>
>>>
>>> Shan Haitao
>>>
>>> diff -r 5fe770c8a8a3 xen/arch/x86/traps.c
>>> --- a/xen/arch/x86/traps.c    Tue Sep 06 15:49:40 2011 +0100
>>> +++ b/xen/arch/x86/traps.c    Wed Sep 07 02:09:12 2011 +0800
>>> @@ -770,6 +770,30 @@ static void pv_cpuid(struct cpu_user_reg
>>>      {
>>>          if ( !cpuid_hypervisor_leaves(a, c, &a, &b, &c, &d) )
>>>              domain_cpuid(current->domain, a, c, &a, &b, &c, &d);
>>> +
>>> +        switch ( a )
>>> +        {
>>> +        case 0xd:
>>> +        {
>>> +            unsigned int sub_leaf, _eax, _ebx, _ecx, _edx;
>>> +            /* EBX value of main leaf 0 depends on enabled xsave features
>>> */
>>> +            if ( c == 0 && current->arch.xcr0 )
>>> +            {
>>> +                /* reset EBX to default value first */
>>> +                b = XSTATE_AREA_MIN_SIZE;
>>> +                for ( sub_leaf = 2; sub_leaf < 64; sub_leaf++ )
>>
>> Shouldn't the upper bound be 63 here (as bit 63 serves a different
>> purpose, and if that bit was set code changes would be required in
>> various other places)?
>>
>> Jan
> Nice catch! I will update the patch. The same piece of code is
> borrowed from hvm_cpuid(), where I can change the value from 64 to 63,
> too.
>
> Shan Haitao
>
>>
>>> +                {
>>> +                    if ( !(current->arch.xcr0 & (1ULL << sub_leaf)) )
>>> +                        continue;
>>> +                    domain_cpuid(current->domain, a, c, &_eax, &_ebx, 
>>> &_ecx,
>>> +                                 &_edx);
>>> +                    if ( (_eax + _ebx) > b )
>>> +                        b = _eax + _ebx;
>>> +                }
>>> +            }
>>> +        break;
>>> +        }
>>> +        }
>>>          goto out;
>>>      }
>>
>>
>>
>>
>

Attachment: pv_xsave_cpuid_fix.patch
Description: Binary data

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel