diff mbox series

[2/5] x86/cpu-policy: Add SVM features already used by Xen

Message ID 20240429151625.977884-3-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series x86: AMD CPUID handling improvements | expand

Commit Message

Andrew Cooper April 29, 2024, 3:16 p.m. UTC
These will replace svm_feature_flags and the SVM_FEATURE_* constants over the
next few changes.  Take the opportunity to rationalise some names.

Drop the opencoded "inherit from host" logic in calculate_hvm_max_policy() and
use 'h'/'!' annotations.  The logic needs to operate on fs, not the policy
object, given its position within the function.

Drop some trailing whitespace introduced when this block of code was last
moved.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
CC: George Dunlap <george.dunlap@citrix.com>
CC: Andrei Semenov <andrei.semenov@vates.fr>
CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
---
 tools/misc/xen-cpuid.c                      | 11 +++++++++++
 xen/arch/x86/cpu-policy.c                   | 17 +++++------------
 xen/include/public/arch-x86/cpufeatureset.h | 14 ++++++++++++++
 xen/tools/gen-cpuid.py                      |  3 +++
 4 files changed, 33 insertions(+), 12 deletions(-)

Comments

Andrew Cooper April 29, 2024, 3:24 p.m. UTC | #1
On 29/04/2024 4:16 pm, Andrew Cooper wrote:
> diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
> index bf3f9ec01e6e..f07b1f4cf905 100755
> --- a/xen/tools/gen-cpuid.py
> +++ b/xen/tools/gen-cpuid.py
> @@ -280,6 +280,9 @@ def crunch_numbers(state):
>          # standard 3DNow in the earlier K6 processors.
>          _3DNOW: [_3DNOWEXT],
>  
> +        # The SVM bit enumerates the whole SVM leave.
> +        SVM: list(range(NPT, NPT + 32)),

This should say leaf.  Fixed locally.

~Andrew
Jan Beulich April 30, 2024, 1:02 p.m. UTC | #2
On 29.04.2024 17:16, Andrew Cooper wrote:
> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -358,6 +358,20 @@ XEN_CPUFEATURE(RFDS_CLEAR,         16*32+28) /*!A Register File(s) cleared by VE
>  /* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.edx, word 17 */
>  
>  /* AMD-defined CPU features, CPUID level 0x8000000a.edx, word 18 */
> +XEN_CPUFEATURE(NPT,                18*32+ 0) /*h  Nested PageTables */
> +XEN_CPUFEATURE(V_LBR,              18*32+ 1) /*h  Virtualised LBR */
> +XEN_CPUFEATURE(SVM_LOCK,           18*32+ 2) /*   SVM locking MSR */
> +XEN_CPUFEATURE(NRIPS,              18*32+ 3) /*h  Next-RIP saved on VMExit */
> +XEN_CPUFEATURE(V_TSC_RATE,         18*32+ 4) /*   Virtualised TSC Ratio */
> +XEN_CPUFEATURE(VMCB_CLEANBITS,     18*32+ 5) /*!  VMCB Clean-bits */

Wouldn't this better be marked !h nevertheless?

As to the name - does it need to be this long? VMCB_CLEAN would be in line
with the PM. But yeah, while CLEANBITS would be clear in the context of this
leaf, there might be whatever other "cleanbits" elsewhere, so some qualifier
is wanted, I guess. As to the V_ prefixes you use for several of the
features: Isn't there a risk of this being ambiguous towards VT-x? Maybe
they should all be SVM_*, even if ...

> +XEN_CPUFEATURE(FLUSH_BY_ASID,      18*32+ 6) /*   TLB Flush by ASID */
> +XEN_CPUFEATURE(DECODE_ASSIST,      18*32+ 7) /*h  Decode assists */
> +XEN_CPUFEATURE(PAUSE_FILTER,       18*32+10) /*h  Pause filter */
> +XEN_CPUFEATURE(PAUSE_THRESH,       18*32+12) /*   Pause filter threshold */
> +XEN_CPUFEATURE(V_LOADSAVE,         18*32+15) /*   Virtualised VMLOAD/SAVE */
> +XEN_CPUFEATURE(V_GIF,              18*32+16) /*   Virtualised GIF */

... these two at least are clearly SVM terminology and hence already
unambiguous.

> +XEN_CPUFEATURE(NPT_SSS,            18*32+19) /*   NPT Supervisor Shadow Stacks */
> +XEN_CPUFEATURE(V_SPEC_CTRL,        18*32+20) /*   Virtualised MSR_SPEC_CTRL */

Whereas this, when used somewhere in isolation, would not make clear
whether AMD's or Intel's is meant.

> --- a/xen/tools/gen-cpuid.py
> +++ b/xen/tools/gen-cpuid.py
> @@ -280,6 +280,9 @@ def crunch_numbers(state):
>          # standard 3DNow in the earlier K6 processors.
>          _3DNOW: [_3DNOWEXT],
>  
> +        # The SVM bit enumerates the whole SVM leave.
> +        SVM: list(range(NPT, NPT + 32)),

Seeing this and taking it together with the somewhat confusing (to me) part
of the description of patch 1: What is it then that you try to avoid there,
when adding the dependencies here is okay, while doing so there would be
entirely impossible (short of there being identifiers)?

Jan
George Dunlap May 1, 2024, 10 a.m. UTC | #3
On Mon, Apr 29, 2024 at 4:16 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> These will replace svm_feature_flags and the SVM_FEATURE_* constants over the
> next few changes.  Take the opportunity to rationalise some names.
>
> Drop the opencoded "inherit from host" logic in calculate_hvm_max_policy() and
> use 'h'/'!' annotations.  The logic needs to operate on fs, not the policy
> object, given its position within the function.
>
> Drop some trailing whitespace introduced when this block of code was last
> moved.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
> CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> CC: George Dunlap <george.dunlap@citrix.com>
> CC: Andrei Semenov <andrei.semenov@vates.fr>
> CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
> ---
>  tools/misc/xen-cpuid.c                      | 11 +++++++++++
>  xen/arch/x86/cpu-policy.c                   | 17 +++++------------
>  xen/include/public/arch-x86/cpufeatureset.h | 14 ++++++++++++++
>  xen/tools/gen-cpuid.py                      |  3 +++
>  4 files changed, 33 insertions(+), 12 deletions(-)
>
> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
> index ab09410a05d6..0d01b0e797f1 100644
> --- a/tools/misc/xen-cpuid.c
> +++ b/tools/misc/xen-cpuid.c
> @@ -266,6 +266,17 @@ static const char *const str_m10Ah[32] =
>
>  static const char *const str_eAd[32] =
>  {
> +    [ 0] = "npt",                 [ 1] = "v-lbr",
> +    [ 2] = "svm-lock",            [ 3] = "nrips",
> +    [ 4] = "v-tsc-rate",          [ 5] = "vmcb-cleanbits",
> +    [ 6] = "flush-by-asid",       [ 7] = "decode-assist",
> +
> +    [10] = "pause-filter",
> +    [12] = "pause-thresh",
> +    /* 14 */                      [15] = "v-loadsave",
> +    [16] = "v-gif",
> +    /* 18 */                      [19] = "npt-sss",
> +    [20] = "v-spec-ctrl",
>  };
>
>  static const char *const str_e1Fa[32] =
> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
> index 4b6d96276399..da4401047e89 100644
> --- a/xen/arch/x86/cpu-policy.c
> +++ b/xen/arch/x86/cpu-policy.c
> @@ -9,7 +9,6 @@
>  #include <asm/amd.h>
>  #include <asm/cpu-policy.h>
>  #include <asm/hvm/nestedhvm.h>
> -#include <asm/hvm/svm/svm.h>
>  #include <asm/intel-family.h>
>  #include <asm/msr-index.h>
>  #include <asm/paging.h>
> @@ -748,22 +747,16 @@ static void __init calculate_hvm_max_policy(void)
>      if ( !cpu_has_vmx )
>          __clear_bit(X86_FEATURE_PKS, fs);
>
> -    /*
> +    /*
>       * Make adjustments to possible (nested) virtualization features exposed
>       * to the guest
>       */
> -    if ( p->extd.svm )
> +    if ( test_bit(X86_FEATURE_SVM, fs) )
>      {
> -        /* Clamp to implemented features which require hardware support. */
> -        p->extd.raw[0xa].d &= ((1u << SVM_FEATURE_NPT) |
> -                               (1u << SVM_FEATURE_LBRV) |
> -                               (1u << SVM_FEATURE_NRIPS) |
> -                               (1u << SVM_FEATURE_PAUSEFILTER) |
> -                               (1u << SVM_FEATURE_DECODEASSISTS));
> -        /* Enable features which are always emulated. */
> -        p->extd.raw[0xa].d |= (1u << SVM_FEATURE_VMCBCLEAN);
> +        /* Xen always emulates cleanbits. */
> +        __set_bit(X86_FEATURE_VMCB_CLEANBITS, fs);
>      }

What about this line, at the end of recalculate_cpuid_policy()?

  if ( !p->extd.svm )
        p->extd.raw[0xa] = EMPTY_LEAF;

Is there a reason to continue to operate directly on the policy here,
or would it be better to do this up in the featureset section?

 -George
Andrew Cooper May 1, 2024, 10:39 a.m. UTC | #4
On 01/05/2024 11:00 am, George Dunlap wrote:
> On Mon, Apr 29, 2024 at 4:16 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> These will replace svm_feature_flags and the SVM_FEATURE_* constants over the
>> next few changes.  Take the opportunity to rationalise some names.
>>
>> Drop the opencoded "inherit from host" logic in calculate_hvm_max_policy() and
>> use 'h'/'!' annotations.  The logic needs to operate on fs, not the policy
>> object, given its position within the function.
>>
>> Drop some trailing whitespace introduced when this block of code was last
>> moved.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Stefano Stabellini <sstabellini@kernel.org>
>> CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
>> CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
>> CC: George Dunlap <george.dunlap@citrix.com>
>> CC: Andrei Semenov <andrei.semenov@vates.fr>
>> CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
>> ---
>>  tools/misc/xen-cpuid.c                      | 11 +++++++++++
>>  xen/arch/x86/cpu-policy.c                   | 17 +++++------------
>>  xen/include/public/arch-x86/cpufeatureset.h | 14 ++++++++++++++
>>  xen/tools/gen-cpuid.py                      |  3 +++
>>  4 files changed, 33 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
>> index ab09410a05d6..0d01b0e797f1 100644
>> --- a/tools/misc/xen-cpuid.c
>> +++ b/tools/misc/xen-cpuid.c
>> @@ -266,6 +266,17 @@ static const char *const str_m10Ah[32] =
>>
>>  static const char *const str_eAd[32] =
>>  {
>> +    [ 0] = "npt",                 [ 1] = "v-lbr",
>> +    [ 2] = "svm-lock",            [ 3] = "nrips",
>> +    [ 4] = "v-tsc-rate",          [ 5] = "vmcb-cleanbits",
>> +    [ 6] = "flush-by-asid",       [ 7] = "decode-assist",
>> +
>> +    [10] = "pause-filter",
>> +    [12] = "pause-thresh",
>> +    /* 14 */                      [15] = "v-loadsave",
>> +    [16] = "v-gif",
>> +    /* 18 */                      [19] = "npt-sss",
>> +    [20] = "v-spec-ctrl",
>>  };
>>
>>  static const char *const str_e1Fa[32] =
>> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
>> index 4b6d96276399..da4401047e89 100644
>> --- a/xen/arch/x86/cpu-policy.c
>> +++ b/xen/arch/x86/cpu-policy.c
>> @@ -9,7 +9,6 @@
>>  #include <asm/amd.h>
>>  #include <asm/cpu-policy.h>
>>  #include <asm/hvm/nestedhvm.h>
>> -#include <asm/hvm/svm/svm.h>
>>  #include <asm/intel-family.h>
>>  #include <asm/msr-index.h>
>>  #include <asm/paging.h>
>> @@ -748,22 +747,16 @@ static void __init calculate_hvm_max_policy(void)
>>      if ( !cpu_has_vmx )
>>          __clear_bit(X86_FEATURE_PKS, fs);
>>
>> -    /*
>> +    /*
>>       * Make adjustments to possible (nested) virtualization features exposed
>>       * to the guest
>>       */
>> -    if ( p->extd.svm )
>> +    if ( test_bit(X86_FEATURE_SVM, fs) )
>>      {
>> -        /* Clamp to implemented features which require hardware support. */
>> -        p->extd.raw[0xa].d &= ((1u << SVM_FEATURE_NPT) |
>> -                               (1u << SVM_FEATURE_LBRV) |
>> -                               (1u << SVM_FEATURE_NRIPS) |
>> -                               (1u << SVM_FEATURE_PAUSEFILTER) |
>> -                               (1u << SVM_FEATURE_DECODEASSISTS));
>> -        /* Enable features which are always emulated. */
>> -        p->extd.raw[0xa].d |= (1u << SVM_FEATURE_VMCBCLEAN);
>> +        /* Xen always emulates cleanbits. */
>> +        __set_bit(X86_FEATURE_VMCB_CLEANBITS, fs);
>>      }
> What about this line, at the end of recalculate_cpuid_policy()?
>
>   if ( !p->extd.svm )
>         p->extd.raw[0xa] = EMPTY_LEAF;
>
> Is there a reason to continue to operate directly on the policy here,
> or would it be better to do this up in the featureset section?

That's still needed.

Otherwise in a !SVM VM you still see svm_rev and nr_asids in a leaf that
should be all zeroes.

~Andrew
George Dunlap May 1, 2024, 10:51 a.m. UTC | #5
On Wed, May 1, 2024 at 11:39 AM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 01/05/2024 11:00 am, George Dunlap wrote:
> > On Mon, Apr 29, 2024 at 4:16 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> These will replace svm_feature_flags and the SVM_FEATURE_* constants over the
> >> next few changes.  Take the opportunity to rationalise some names.
> >>
> >> Drop the opencoded "inherit from host" logic in calculate_hvm_max_policy() and
> >> use 'h'/'!' annotations.  The logic needs to operate on fs, not the policy
> >> object, given its position within the function.
> >>
> >> Drop some trailing whitespace introduced when this block of code was last
> >> moved.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> ---
> >> CC: Jan Beulich <JBeulich@suse.com>
> >> CC: Roger Pau Monné <roger.pau@citrix.com>
> >> CC: Stefano Stabellini <sstabellini@kernel.org>
> >> CC: Xenia Ragiadakou <xenia.ragiadakou@amd.com>
> >> CC: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
> >> CC: George Dunlap <george.dunlap@citrix.com>
> >> CC: Andrei Semenov <andrei.semenov@vates.fr>
> >> CC: Vaishali Thakkar <vaishali.thakkar@vates.tech>
> >> ---
> >>  tools/misc/xen-cpuid.c                      | 11 +++++++++++
> >>  xen/arch/x86/cpu-policy.c                   | 17 +++++------------
> >>  xen/include/public/arch-x86/cpufeatureset.h | 14 ++++++++++++++
> >>  xen/tools/gen-cpuid.py                      |  3 +++
> >>  4 files changed, 33 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
> >> index ab09410a05d6..0d01b0e797f1 100644
> >> --- a/tools/misc/xen-cpuid.c
> >> +++ b/tools/misc/xen-cpuid.c
> >> @@ -266,6 +266,17 @@ static const char *const str_m10Ah[32] =
> >>
> >>  static const char *const str_eAd[32] =
> >>  {
> >> +    [ 0] = "npt",                 [ 1] = "v-lbr",
> >> +    [ 2] = "svm-lock",            [ 3] = "nrips",
> >> +    [ 4] = "v-tsc-rate",          [ 5] = "vmcb-cleanbits",
> >> +    [ 6] = "flush-by-asid",       [ 7] = "decode-assist",
> >> +
> >> +    [10] = "pause-filter",
> >> +    [12] = "pause-thresh",
> >> +    /* 14 */                      [15] = "v-loadsave",
> >> +    [16] = "v-gif",
> >> +    /* 18 */                      [19] = "npt-sss",
> >> +    [20] = "v-spec-ctrl",
> >>  };
> >>
> >>  static const char *const str_e1Fa[32] =
> >> diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
> >> index 4b6d96276399..da4401047e89 100644
> >> --- a/xen/arch/x86/cpu-policy.c
> >> +++ b/xen/arch/x86/cpu-policy.c
> >> @@ -9,7 +9,6 @@
> >>  #include <asm/amd.h>
> >>  #include <asm/cpu-policy.h>
> >>  #include <asm/hvm/nestedhvm.h>
> >> -#include <asm/hvm/svm/svm.h>
> >>  #include <asm/intel-family.h>
> >>  #include <asm/msr-index.h>
> >>  #include <asm/paging.h>
> >> @@ -748,22 +747,16 @@ static void __init calculate_hvm_max_policy(void)
> >>      if ( !cpu_has_vmx )
> >>          __clear_bit(X86_FEATURE_PKS, fs);
> >>
> >> -    /*
> >> +    /*
> >>       * Make adjustments to possible (nested) virtualization features exposed
> >>       * to the guest
> >>       */
> >> -    if ( p->extd.svm )
> >> +    if ( test_bit(X86_FEATURE_SVM, fs) )
> >>      {
> >> -        /* Clamp to implemented features which require hardware support. */
> >> -        p->extd.raw[0xa].d &= ((1u << SVM_FEATURE_NPT) |
> >> -                               (1u << SVM_FEATURE_LBRV) |
> >> -                               (1u << SVM_FEATURE_NRIPS) |
> >> -                               (1u << SVM_FEATURE_PAUSEFILTER) |
> >> -                               (1u << SVM_FEATURE_DECODEASSISTS));
> >> -        /* Enable features which are always emulated. */
> >> -        p->extd.raw[0xa].d |= (1u << SVM_FEATURE_VMCBCLEAN);
> >> +        /* Xen always emulates cleanbits. */
> >> +        __set_bit(X86_FEATURE_VMCB_CLEANBITS, fs);
> >>      }
> > What about this line, at the end of recalculate_cpuid_policy()?
> >
> >   if ( !p->extd.svm )
> >         p->extd.raw[0xa] = EMPTY_LEAF;
> >
> > Is there a reason to continue to operate directly on the policy here,
> > or would it be better to do this up in the featureset section?
>
> That's still needed.
>
> Otherwise in a !SVM VM you still see svm_rev and nr_asids in a leaf that
> should be all zeroes.

...Uh, yes of course we still need to clear the non-existent CPUID
bits.  I was asking if we should change *how* we should clear them.

In the hunk I responded to, when enabling VMCBCLEAN, we switch from
setting bits in the policy, to setting bits in the featureset.  I was
asking whether it would make sense to do something like

    if !test_bit(X86_FEATURE_SVM, fs)
        fs[FEATURESET_eAd]  = 0;

before the x86_cpu_featureset_to_policy() instead.

 -George
diff mbox series

Patch

diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index ab09410a05d6..0d01b0e797f1 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -266,6 +266,17 @@  static const char *const str_m10Ah[32] =
 
 static const char *const str_eAd[32] =
 {
+    [ 0] = "npt",                 [ 1] = "v-lbr",
+    [ 2] = "svm-lock",            [ 3] = "nrips",
+    [ 4] = "v-tsc-rate",          [ 5] = "vmcb-cleanbits",
+    [ 6] = "flush-by-asid",       [ 7] = "decode-assist",
+
+    [10] = "pause-filter",
+    [12] = "pause-thresh",
+    /* 14 */                      [15] = "v-loadsave",
+    [16] = "v-gif",
+    /* 18 */                      [19] = "npt-sss",
+    [20] = "v-spec-ctrl",
 };
 
 static const char *const str_e1Fa[32] =
diff --git a/xen/arch/x86/cpu-policy.c b/xen/arch/x86/cpu-policy.c
index 4b6d96276399..da4401047e89 100644
--- a/xen/arch/x86/cpu-policy.c
+++ b/xen/arch/x86/cpu-policy.c
@@ -9,7 +9,6 @@ 
 #include <asm/amd.h>
 #include <asm/cpu-policy.h>
 #include <asm/hvm/nestedhvm.h>
-#include <asm/hvm/svm/svm.h>
 #include <asm/intel-family.h>
 #include <asm/msr-index.h>
 #include <asm/paging.h>
@@ -748,22 +747,16 @@  static void __init calculate_hvm_max_policy(void)
     if ( !cpu_has_vmx )
         __clear_bit(X86_FEATURE_PKS, fs);
 
-    /* 
+    /*
      * Make adjustments to possible (nested) virtualization features exposed
      * to the guest
      */
-    if ( p->extd.svm )
+    if ( test_bit(X86_FEATURE_SVM, fs) )
     {
-        /* Clamp to implemented features which require hardware support. */
-        p->extd.raw[0xa].d &= ((1u << SVM_FEATURE_NPT) |
-                               (1u << SVM_FEATURE_LBRV) |
-                               (1u << SVM_FEATURE_NRIPS) |
-                               (1u << SVM_FEATURE_PAUSEFILTER) |
-                               (1u << SVM_FEATURE_DECODEASSISTS));
-        /* Enable features which are always emulated. */
-        p->extd.raw[0xa].d |= (1u << SVM_FEATURE_VMCBCLEAN);
+        /* Xen always emulates cleanbits. */
+        __set_bit(X86_FEATURE_VMCB_CLEANBITS, fs);
     }
-    
+
     guest_common_max_feature_adjustments(fs);
     guest_common_feature_adjustments(fs);
 
diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h
index 0f869214811e..80d252a38c2d 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -358,6 +358,20 @@  XEN_CPUFEATURE(RFDS_CLEAR,         16*32+28) /*!A Register File(s) cleared by VE
 /* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.edx, word 17 */
 
 /* AMD-defined CPU features, CPUID level 0x8000000a.edx, word 18 */
+XEN_CPUFEATURE(NPT,                18*32+ 0) /*h  Nested PageTables */
+XEN_CPUFEATURE(V_LBR,              18*32+ 1) /*h  Virtualised LBR */
+XEN_CPUFEATURE(SVM_LOCK,           18*32+ 2) /*   SVM locking MSR */
+XEN_CPUFEATURE(NRIPS,              18*32+ 3) /*h  Next-RIP saved on VMExit */
+XEN_CPUFEATURE(V_TSC_RATE,         18*32+ 4) /*   Virtualised TSC Ratio */
+XEN_CPUFEATURE(VMCB_CLEANBITS,     18*32+ 5) /*!  VMCB Clean-bits */
+XEN_CPUFEATURE(FLUSH_BY_ASID,      18*32+ 6) /*   TLB Flush by ASID */
+XEN_CPUFEATURE(DECODE_ASSIST,      18*32+ 7) /*h  Decode assists */
+XEN_CPUFEATURE(PAUSE_FILTER,       18*32+10) /*h  Pause filter */
+XEN_CPUFEATURE(PAUSE_THRESH,       18*32+12) /*   Pause filter threshold */
+XEN_CPUFEATURE(V_LOADSAVE,         18*32+15) /*   Virtualised VMLOAD/SAVE */
+XEN_CPUFEATURE(V_GIF,              18*32+16) /*   Virtualised GIF */
+XEN_CPUFEATURE(NPT_SSS,            18*32+19) /*   NPT Supervisor Shadow Stacks */
+XEN_CPUFEATURE(V_SPEC_CTRL,        18*32+20) /*   Virtualised MSR_SPEC_CTRL */
 
 /* AMD-defined CPU features, CPUID level 0x8000001f.eax, word 19 */
 
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index bf3f9ec01e6e..f07b1f4cf905 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -280,6 +280,9 @@  def crunch_numbers(state):
         # standard 3DNow in the earlier K6 processors.
         _3DNOW: [_3DNOWEXT],
 
+        # The SVM bit enumerates the whole SVM leave.
+        SVM: list(range(NPT, NPT + 32)),
+
         # This is just the dependency between AVX512 and AVX2 of XSTATE
         # feature flags.  If want to use AVX512, AVX2 must be supported and
         # enabled.  Certain later extensions, acting on 256-bit vectors of