[PATCH 01/13] Nested Virtualization: tools

[PATCH 01/13] Nested Virtualization: tools

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 322 bytes --]


Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_nh01_tools.diff --]
[-- Type: text/x-diff, Size: 7335 bytes --]

# HG changeset patch
# User cegger
# Date 1283345869 -7200
tools: Add nestedhvm guest config option

diff -r 80ef08613ec2 -r ecec3d163efa tools/libxc/xc_cpuid_x86.c
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -30,7 +30,7 @@
 #define set_bit(idx, dst)   ((dst) |= (1u << ((idx) & 31)))
 
 #define DEF_MAX_BASE 0x0000000du
-#define DEF_MAX_EXT  0x80000008u
+#define DEF_MAX_EXT  0x8000000au
 
 static int hypervisor_is_64bit(xc_interface *xch)
 {
@@ -78,7 +78,7 @@ static void xc_cpuid_brand_get(char *str
 static void amd_xc_cpuid_policy(
     xc_interface *xch, domid_t domid,
     const unsigned int *input, unsigned int *regs,
-    int is_pae)
+    int is_pae, int is_nestedhvm)
 {
     switch ( input[0] )
     {
@@ -97,6 +97,7 @@ static void amd_xc_cpuid_policy(
         /* Filter all other features according to a whitelist. */
         regs[2] &= ((is_64bit ? bitmaskof(X86_FEATURE_LAHF_LM) : 0) |
                     bitmaskof(X86_FEATURE_CMP_LEGACY) |
+                    (is_nestedhvm ? bitmaskof(X86_FEATURE_SVME) : 0) |
                     bitmaskof(X86_FEATURE_ALTMOVCR) |
                     bitmaskof(X86_FEATURE_ABM) |
                     bitmaskof(X86_FEATURE_SSE4A) |
@@ -121,13 +122,43 @@ static void amd_xc_cpuid_policy(
          */
         regs[2] = ((regs[2] & 0xf000u) + 1) | ((regs[2] & 0xffu) << 1) | 1u;
         break;
+
+    case 0x8000000a: {
+        uint32_t edx;
+
+        if (!is_nestedhvm) {
+            regs[0] = regs[1] = regs[2] = regs[3] = 0;
+            break;
+        }
+
+#define SVM_FEATURE_NPT            0x00000001
+#define SVM_FEATURE_LBRV           0x00000002
+#define SVM_FEATURE_SVML           0x00000004
+#define SVM_FEATURE_NRIPS          0x00000008
+#define SVM_FEATURE_PAUSEFILTER    0x00000400
+
+        /* Only passthrough SVM features which are implemented */
+        edx = 0;
+        if (regs[3] & SVM_FEATURE_NPT)
+            edx |= SVM_FEATURE_NPT;
+        if (regs[3] & SVM_FEATURE_LBRV)
+            edx |= SVM_FEATURE_LBRV;
+        if (regs[3] & SVM_FEATURE_NRIPS)
+            edx |= SVM_FEATURE_NRIPS;
+        if (regs[3] & SVM_FEATURE_PAUSEFILTER)
+            edx |= SVM_FEATURE_PAUSEFILTER;
+
+        regs[3] = edx;
+        break;
+    }
+
     }
 }
 
 static void intel_xc_cpuid_policy(
     xc_interface *xch, domid_t domid,
     const unsigned int *input, unsigned int *regs,
-    int is_pae)
+    int is_pae, int is_nestedhvm)
 {
     switch ( input[0] )
     {
@@ -161,6 +192,11 @@ static void intel_xc_cpuid_policy(
         /* Mask AMD Number of Cores information. */
         regs[2] = 0;
         break;
+
+    case 0x8000000a:
+        /* Clear AMD SVM feature bits */
+        regs[0] = regs[1] = regs[2] = regs[3] = 0;
+        break;
     }
 }
 
@@ -169,12 +205,17 @@ static void xc_cpuid_hvm_policy(
     const unsigned int *input, unsigned int *regs)
 {
     char brand[13];
+    unsigned long nestedhvm;
     unsigned long pae;
     int is_pae;
+    int is_nestedhvm;
 
     xc_get_hvm_param(xch, domid, HVM_PARAM_PAE_ENABLED, &pae);
     is_pae = !!pae;
 
+    xc_get_hvm_param(xch, domid, HVM_PARAM_NESTEDHVM, &nestedhvm);
+    is_nestedhvm = !!nestedhvm;
+
     switch ( input[0] )
     {
     case 0x00000000:
@@ -260,6 +301,7 @@ static void xc_cpuid_hvm_policy(
     case 0x80000004: /* ... continued         */
     case 0x80000005: /* AMD L1 cache/TLB info (dumped by Intel policy) */
     case 0x80000006: /* AMD L2/3 cache/TLB info ; Intel L2 cache features */
+    case 0x8000000a: /* AMD SVM feature bits */
         break;
 
     default:
@@ -269,9 +311,9 @@ static void xc_cpuid_hvm_policy(
 
     xc_cpuid_brand_get(brand);
     if ( strstr(brand, "AMD") )
-        amd_xc_cpuid_policy(xch, domid, input, regs, is_pae);
+        amd_xc_cpuid_policy(xch, domid, input, regs, is_pae, is_nestedhvm);
     else
-        intel_xc_cpuid_policy(xch, domid, input, regs, is_pae);
+        intel_xc_cpuid_policy(xch, domid, input, regs, is_pae, is_nestedhvm);
 
 }
 
diff -r 80ef08613ec2 -r ecec3d163efa tools/python/xen/xend/XendConfig.py
--- a/tools/python/xen/xend/XendConfig.py
+++ b/tools/python/xen/xend/XendConfig.py
@@ -185,6 +185,7 @@ XENAPI_PLATFORM_CFG_TYPES = {
     'vhpt': int,
     'guest_os_type': str,
     'hap': int,
+    'nestedhvm' : int,
     'xen_extended_power_mgmt': int,
     'pci_msitranslate': int,
     'pci_power_mgmt': int,
diff -r 80ef08613ec2 -r ecec3d163efa tools/python/xen/xend/XendConstants.py
--- a/tools/python/xen/xend/XendConstants.py
+++ b/tools/python/xen/xend/XendConstants.py
@@ -52,6 +52,7 @@ HVM_PARAM_TIMER_MODE   = 10
 HVM_PARAM_HPET_ENABLED = 11
 HVM_PARAM_ACPI_S_STATE = 14
 HVM_PARAM_VPT_ALIGN    = 16
+HVM_PARAM_NESTEDHVM    = 19 # x86
 
 restart_modes = [
     "restart",
diff -r 80ef08613ec2 -r ecec3d163efa tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -2585,10 +2585,15 @@ class XendDomainInfo:
             xc.hvm_set_param(self.domid, HVM_PARAM_TIMER_MODE,
                              long(timer_mode))
 
-        # Set Viridian interface configuration of domain
-        viridian = self.info["platform"].get("viridian")
-        if arch.type == "x86" and hvm and viridian is not None:
-            xc.hvm_set_param(self.domid, HVM_PARAM_VIRIDIAN, long(viridian))
+        if arch.type == "x86" and hvm:
+            # Set Viridian interface configuration of domain
+            viridian = self.info["platform"].get("viridian")
+            if viridian is not None:
+                xc.hvm_set_param(self.domid, HVM_PARAM_VIRIDIAN, long(viridian))
+            # Set nestedhvm of domain
+            nestedhvm = self.info["platform"].get("nestedhvm")
+            if nestedhvm is not None:
+                xc.hvm_set_param(self.domid, HVM_PARAM_NESTEDHVM, long(nestedhvm))
 
         # If nomigrate is set, disable migration
         nomigrate = self.info["platform"].get("nomigrate")
diff -r 80ef08613ec2 -r ecec3d163efa tools/python/xen/xm/create.py
--- a/tools/python/xen/xm/create.py
+++ b/tools/python/xen/xm/create.py
@@ -633,6 +633,11 @@ gopts.var('hap', val='HAP',
           use="""Hap status (0=hap is disabled;
           1=hap is enabled.""")
 
+gopts.var('nestedhvm', val='NESTEDHVM',
+          fn=set_int, default=0,
+          use="""Nested HVM status (0=Nested HVM is disabled;
+          1=Nested HVM is enabled.""")
+
 gopts.var('s3_integrity', val='TBOOT_MEMORY_PROTECT',
           fn=set_int, default=1,
           use="""Should domain memory integrity be verified during S3?
@@ -1083,7 +1088,7 @@ def configure_hvm(config_image, vals):
              'isa',
              'keymap',
              'localtime',
-             'nographic',
+             'nestedhvm', 'nographic',
              'opengl', 'oos',
              'pae', 'pci', 'pci_msitranslate', 'pci_power_mgmt',
              'rtc_timeoffset',
diff -r 80ef08613ec2 -r ecec3d163efa xen/include/public/hvm/params.h
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -113,6 +113,9 @@
 #define HVM_PARAM_CONSOLE_PFN    17
 #define HVM_PARAM_CONSOLE_EVTCHN 18
 
-#define HVM_NR_PARAMS          19
+/* Boolean: Enable nestedhvm */
+#define HVM_PARAM_NESTEDHVM    19
+
+#define HVM_NR_PARAMS          20
 
 #endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

RE: [PATCH 01/13] Nested Virtualization: tools

[Dong, Eddie]

Dong, Eddie wrote:
> # HG changeset patch
> # User cegger
> # Date 1283345869 -7200
> tools: Add nestedhvm guest config option
> 
> diff -r 80ef08613ec2 -r ecec3d163efa tools/libxc/xc_cpuid_x86.c
> --- a/tools/libxc/xc_cpuid_x86.c
> +++ b/tools/libxc/xc_cpuid_x86.c
> @@ -30,7 +30,7 @@
>  #define set_bit(idx, dst)   ((dst) |= (1u << ((idx) & 31)))
> 
>  #define DEF_MAX_BASE 0x0000000du
> -#define DEF_MAX_EXT  0x80000008u
> +#define DEF_MAX_EXT  0x8000000au

How can this make Intel CPU happy?
You may refer to my previous comments in V2.

> 
>  static int hypervisor_is_64bit(xc_interface *xch)
>  {
> @@ -78,7 +78,7 @@ static void xc_cpuid_brand_get(char *str
>  static void amd_xc_cpuid_policy(
>      xc_interface *xch, domid_t domid,
>      const unsigned int *input, unsigned int *regs,
> -    int is_pae)
> +    int is_pae, int is_nestedhvm)
>  {
>      switch ( input[0] )
>      {
> @@ -97,6 +97,7 @@ static void amd_xc_cpuid_policy(
>          /* Filter all other features according to a whitelist. */
>          regs[2] &= ((is_64bit ? bitmaskof(X86_FEATURE_LAHF_LM) : 0) |
>                      bitmaskof(X86_FEATURE_CMP_LEGACY) |
> +                    (is_nestedhvm ? bitmaskof(X86_FEATURE_SVME) : 0)
>                      | bitmaskof(X86_FEATURE_ALTMOVCR) |
>                      bitmaskof(X86_FEATURE_ABM) |
>                      bitmaskof(X86_FEATURE_SSE4A) |
> @@ -121,13 +122,43 @@ static void amd_xc_cpuid_policy(
>           */
>          regs[2] = ((regs[2] & 0xf000u) + 1) | ((regs[2] & 0xffu) <<
>          1) | 1u; break;
> +
> +    case 0x8000000a: {
> +        uint32_t edx;
> +
> +        if (!is_nestedhvm) {
> +            regs[0] = regs[1] = regs[2] = regs[3] = 0;
> +            break;
> +        }
> +
> +#define SVM_FEATURE_NPT            0x00000001
> +#define SVM_FEATURE_LBRV           0x00000002
> +#define SVM_FEATURE_SVML           0x00000004
> +#define SVM_FEATURE_NRIPS          0x00000008
> +#define SVM_FEATURE_PAUSEFILTER    0x00000400
> +
> +        /* Only passthrough SVM features which are implemented */
> +        edx = 0;
> +        if (regs[3] & SVM_FEATURE_NPT)
> +            edx |= SVM_FEATURE_NPT;
> +        if (regs[3] & SVM_FEATURE_LBRV)
> +            edx |= SVM_FEATURE_LBRV;
> +        if (regs[3] & SVM_FEATURE_NRIPS)
> +            edx |= SVM_FEATURE_NRIPS;
> +        if (regs[3] & SVM_FEATURE_PAUSEFILTER)
> +            edx |= SVM_FEATURE_PAUSEFILTER;
> +
> +        regs[3] = edx;
> +        break;
> +    }
> +
>      }
>  }
> 
>  static void intel_xc_cpuid_policy(
>      xc_interface *xch, domid_t domid,
>      const unsigned int *input, unsigned int *regs,
> -    int is_pae)
> +    int is_pae, int is_nestedhvm)
>  {
>      switch ( input[0] )
>      {
> @@ -161,6 +192,11 @@ static void intel_xc_cpuid_policy(
>          /* Mask AMD Number of Cores information. */
>          regs[2] = 0;
>          break;
> +
> +    case 0x8000000a:
> +        /* Clear AMD SVM feature bits */
> +        regs[0] = regs[1] = regs[2] = regs[3] = 0;
> +        break;

ditto.

>      }
>  }
> 
> @@ -169,12 +205,17 @@ static void xc_cpuid_hvm_policy(
>      const unsigned int *input, unsigned int *regs)
>  {
>      char brand[13];
> +    unsigned long nestedhvm;
>      unsigned long pae;
>      int is_pae;
> +    int is_nestedhvm;
> 
>      xc_get_hvm_param(xch, domid, HVM_PARAM_PAE_ENABLED, &pae);
>      is_pae = !!pae;
> 
> +    xc_get_hvm_param(xch, domid, HVM_PARAM_NESTEDHVM, &nestedhvm);
> +    is_nestedhvm = !!nestedhvm;
> +
>      switch ( input[0] )
>      {
>      case 0x00000000:
> @@ -260,6 +301,7 @@ static void xc_cpuid_hvm_policy(
>      case 0x80000004: /* ... continued         */
>      case 0x80000005: /* AMD L1 cache/TLB info (dumped by Intel
>      policy) */ case 0x80000006: /* AMD L2/3 cache/TLB info ; Intel
> L2 cache features */
> +    case 0x8000000a: /* AMD SVM feature bits */

Should this be in amd_xc_cpuid_policy? 

>          break;
> 
>      default:
> @@ -269,9 +311,9 @@ static void xc_cpuid_hvm_policy(
> 
>      xc_cpuid_brand_get(brand);
>      if ( strstr(brand, "AMD") )
> -        amd_xc_cpuid_policy(xch, domid, input, regs, is_pae);
> +        amd_xc_cpuid_policy(xch, domid, input, regs, is_pae,
>      is_nestedhvm); else
> -        intel_xc_cpuid_policy(xch, domid, input, regs, is_pae);
> +        intel_xc_cpuid_policy(xch, domid, input, regs, is_pae,
> is_nestedhvm); 
> 
>  }
> 


Thx, Eddie

Re: RE: [PATCH 01/13] Nested Virtualization: tools

[Keir Fraser]

On 03/09/2010 08:54, "Dong, Eddie" <eddie.dong@intel.com> wrote:

>>  #define DEF_MAX_BASE 0x0000000du
>> -#define DEF_MAX_EXT  0x80000008u
>> +#define DEF_MAX_EXT  0x8000000au
> 
> How can this make Intel CPU happy?
> You may refer to my previous comments in V2.

Perhaps this should be made dependent on the CPUID brand string, like some
other bits in this file?

 K.

RE: RE: [PATCH 01/13] Nested Virtualization: tools

[Dong, Eddie]

Keir Fraser wrote:
> On 03/09/2010 08:54, "Dong, Eddie" <eddie.dong@intel.com> wrote:
> 
>>>  #define DEF_MAX_BASE 0x0000000du
>>> -#define DEF_MAX_EXT  0x80000008u
>>> +#define DEF_MAX_EXT  0x8000000au
>> 
>> How can this make Intel CPU happy?
>> You may refer to my previous comments in V2.
> 
> Perhaps this should be made dependent on the CPUID brand string, like
> some other bits in this file?
> 
>  K.

Agree:)

Re: RE: [PATCH 01/13] Nested Virtualization: tools

[Andre Przywara]

Dong, Eddie wrote:
> Dong, Eddie wrote:
>> # HG changeset patch
>> # User cegger
>> # Date 1283345869 -7200
>> tools: Add nestedhvm guest config option
>>
>> diff -r 80ef08613ec2 -r ecec3d163efa tools/libxc/xc_cpuid_x86.c
>> --- a/tools/libxc/xc_cpuid_x86.c
>> +++ b/tools/libxc/xc_cpuid_x86.c
>> @@ -30,7 +30,7 @@
>>  #define set_bit(idx, dst)   ((dst) |= (1u << ((idx) & 31)))
>>
>>  #define DEF_MAX_BASE 0x0000000du
>> -#define DEF_MAX_EXT  0x80000008u
>> +#define DEF_MAX_EXT  0x8000000au
> 
> How can this make Intel CPU happy?
> You may refer to my previous comments in V2.
Correct me if I am wrong, but this is only a max boundary:
tools/libxc/xc_cpuid_x86.c:234
     case 0x80000000:
         if ( regs[0] > DEF_MAX_EXT )
             regs[0] = DEF_MAX_EXT;
         break;
So if an Intel CPU returns 0x80000008 here, this will be in the regs[0] 
field and thus any higher value in DEF_MAX_EXT does not affect the 
guest's CPUID response.
So as long as Intel CPUs don't return higher values which don't match 
the AMD assignment (which is extremely unlikely), extending DEF_MAX_EXT 
is fine.

Regards,
Andre.

-- 
Andre Przywara
AMD-OSRC (Dresden)
Tel: x29712

RE: RE: [PATCH 01/13] Nested Virtualization: tools

[Dong, Eddie]

Andre Przywara wrote:
> Dong, Eddie wrote:
>> Dong, Eddie wrote:
>>> # HG changeset patch
>>> # User cegger
>>> # Date 1283345869 -7200
>>> tools: Add nestedhvm guest config option
>>> 
>>> diff -r 80ef08613ec2 -r ecec3d163efa tools/libxc/xc_cpuid_x86.c
>>> --- a/tools/libxc/xc_cpuid_x86.c
>>> +++ b/tools/libxc/xc_cpuid_x86.c
>>> @@ -30,7 +30,7 @@
>>>  #define set_bit(idx, dst)   ((dst) |= (1u << ((idx) & 31)))
>>> 
>>>  #define DEF_MAX_BASE 0x0000000du
>>> -#define DEF_MAX_EXT  0x80000008u
>>> +#define DEF_MAX_EXT  0x8000000au
>> 
>> How can this make Intel CPU happy?
>> You may refer to my previous comments in V2.
> Correct me if I am wrong, but this is only a max boundary:
> tools/libxc/xc_cpuid_x86.c:234
>      case 0x80000000:
>          if ( regs[0] > DEF_MAX_EXT )
>              regs[0] = DEF_MAX_EXT;
>          break;
> So if an Intel CPU returns 0x80000008 here, this will be in the
> regs[0] field and thus any higher value in DEF_MAX_EXT does not
> affect the guest's CPUID response.
> So as long as Intel CPUs don't return higher values which don't match
> the AMD assignment (which is extremely unlikely), extending
> DEF_MAX_EXT is fine.
> 
But it is called as MAX_EXT and will cause some unreasonable setup of leaves. May you split the MACRO to _AMD & _INTEL, or a dynamic variable depending on CPU brand like Keir suggested? 

Thx, Eddie

Re: RE: [PATCH 01/13] Nested Virtualization: tools

[Andre Przywara]

Dong, Eddie wrote:
> Andre Przywara wrote:
>> Dong, Eddie wrote:
>>> Dong, Eddie wrote:
>>>> # HG changeset patch
>>>> # User cegger
>>>> # Date 1283345869 -7200
>>>> tools: Add nestedhvm guest config option
>>>>
>>>> diff -r 80ef08613ec2 -r ecec3d163efa tools/libxc/xc_cpuid_x86.c
>>>> --- a/tools/libxc/xc_cpuid_x86.c
>>>> +++ b/tools/libxc/xc_cpuid_x86.c
>>>> @@ -30,7 +30,7 @@
>>>>  #define set_bit(idx, dst)   ((dst) |= (1u << ((idx) & 31)))
>>>>
>>>>  #define DEF_MAX_BASE 0x0000000du
>>>> -#define DEF_MAX_EXT  0x80000008u
>>>> +#define DEF_MAX_EXT  0x8000000au
>>> How can this make Intel CPU happy?
>>> You may refer to my previous comments in V2.
>> Correct me if I am wrong, but this is only a max boundary:
>> tools/libxc/xc_cpuid_x86.c:234
>>      case 0x80000000:
>>          if ( regs[0] > DEF_MAX_EXT )
>>              regs[0] = DEF_MAX_EXT;
>>          break;
>> So if an Intel CPU returns 0x80000008 here, this will be in the
>> regs[0] field and thus any higher value in DEF_MAX_EXT does not
>> affect the guest's CPUID response.
>> So as long as Intel CPUs don't return higher values which don't match
>> the AMD assignment (which is extremely unlikely), extending
>> DEF_MAX_EXT is fine.
>>
> But it is called as MAX_EXT and will cause some unreasonable setup of leaves.
Where? If DEF_MAX_EXT would be 0x8FFFFFFF, CPUID would still return 
0x80000008 on Intel CPUs. I don't see any leaves setup because of a 
changed DEF_MAX_EXT value. CPUID will just return the smaller value of 
(DEF_MAX_EXT,native CPUID), any leafs beyond the value will not be 
populated in xc_cpuid_apply_policy() and thus will not appear in the 
HV's struct domain->arch.cpuids array used for delivering CPUID results 
to guests.


> May you split the MACRO to _AMD & _INTEL, or a dynamic variable depending on CPU brand like Keir suggested? 
I guess that is not needed. The leaf is properly handled in the 
{amd,intel}_xc_cpuid_policy() filters, which will only be called on the 
respective CPUs.

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12

RE: RE: [PATCH 01/13] Nested Virtualization: tools

[Dong, Eddie]

Andre Przywara wrote:
> Dong, Eddie wrote:
>> Andre Przywara wrote:
>>> Dong, Eddie wrote:
>>>> Dong, Eddie wrote:
>>>>> # HG changeset patch
>>>>> # User cegger
>>>>> # Date 1283345869 -7200
>>>>> tools: Add nestedhvm guest config option
>>>>> 
>>>>> diff -r 80ef08613ec2 -r ecec3d163efa tools/libxc/xc_cpuid_x86.c
>>>>> --- a/tools/libxc/xc_cpuid_x86.c
>>>>> +++ b/tools/libxc/xc_cpuid_x86.c
>>>>> @@ -30,7 +30,7 @@
>>>>>  #define set_bit(idx, dst)   ((dst) |= (1u << ((idx) & 31)))
>>>>> 
>>>>>  #define DEF_MAX_BASE 0x0000000du
>>>>> -#define DEF_MAX_EXT  0x80000008u
>>>>> +#define DEF_MAX_EXT  0x8000000au
>>>> How can this make Intel CPU happy?
>>>> You may refer to my previous comments in V2.
>>> Correct me if I am wrong, but this is only a max boundary:
>>> tools/libxc/xc_cpuid_x86.c:234
>>>      case 0x80000000:
>>>          if ( regs[0] > DEF_MAX_EXT )
>>>              regs[0] = DEF_MAX_EXT;
>>>          break;
>>> So if an Intel CPU returns 0x80000008 here, this will be in the
>>> regs[0] field and thus any higher value in DEF_MAX_EXT does not
>>> affect the guest's CPUID response.
>>> So as long as Intel CPUs don't return higher values which don't
>>> match the AMD assignment (which is extremely unlikely), extending
>>> DEF_MAX_EXT is fine. 
>>> 
>> But it is called as MAX_EXT and will cause some unreasonable setup
>> of leaves. 
> Where? If DEF_MAX_EXT would be 0x8FFFFFFF, CPUID would still return
> 0x80000008 on Intel CPUs. I don't see any leaves setup because of a
> changed DEF_MAX_EXT value. CPUID will just return the smaller value of
> (DEF_MAX_EXT,native CPUID), any leafs beyond the value will not be
> populated in xc_cpuid_apply_policy() and thus will not appear in the
> HV's struct domain->arch.cpuids array used for delivering CPUID
> results to guests.

Well, what does DEF_MAX_EXT mean then? Isn't it mean default maximum Extended Function CPUID Information?

If yes, you shouldn't mislead readers to think virtual Intel CPU has 0x8..A Extended Function CPUID Information now.


> 
> 
>> May you split the MACRO to _AMD & _INTEL, or a dynamic variable
>> depending on CPU brand like Keir suggested? 
> I guess that is not needed. The leaf is properly handled in the
> {amd,intel}_xc_cpuid_policy() filters, which will only be called on
> the respective CPUs.

Are you assuming that future Intel processor won't implement any leaf higher than 0x8...8/A either?

Software should follow SDM as more as possible.

Thx, Eddie