Hi Boris,

On 7/9/25 05:49, Borislav Petkov (AMD) wrote:
> Hi,
> 
> I *think* this is how it should be done but I might be forgetting something.
> It seems to work in my testing here.
> 
> Babu, double-check me pls.
> 
> Thx.


Patch looks good. Few comments.

Is KVM aware of these bits? I didn’t notice any patches adding support for
them in the kernel.

I recommend splitting this into two separate patches:
a. One patch to introduce the new bit 0021_EAX_VERW_CLEAR.

b. Another patch to add the new feature leaf FEAT_8000_0021_ECX.
Also, don’t forget to define tsa-sq-no and tsa-l1-no in target/i386/cpu.h.


> 
> ---
> 
> Add CPUID leaf 0x8000_0021.ECX support and add the TSA CPUID flags.
> 
> Signed-off-by: Borislav Petkov (AMD) <b...@alien8.de>
> ---
>  target/i386/cpu.c | 20 +++++++++++++++++++-
>  target/i386/cpu.h |  3 +++
>  2 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 0d35e95430fe..b889de61ed9d 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1274,7 +1274,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>          .type = CPUID_FEATURE_WORD,
>          .feat_names = {
>              "no-nested-data-bp", "fs-gs-base-ns", 
> "lfence-always-serializing", NULL,
> -            NULL, NULL, "null-sel-clr-base", NULL,
> +            NULL, "verw-clear", "null-sel-clr-base", NULL,
>              "auto-ibrs", NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
>              NULL, NULL, NULL, NULL,
> @@ -1308,6 +1308,22 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>          .tcg_features = 0,
>          .unmigratable_flags = 0,
>      },
> +    [FEAT_8000_0021_ECX] = {
> +        .type = CPUID_FEATURE_WORD,
> +        .feat_names = {
> +            NULL, "tsa-sq-no", "tsa-l1-no", NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +            NULL, NULL, NULL, NULL,
> +        },
> +        .cpuid = { .eax = 0x80000021, .reg = R_ECX, },
> +        .tcg_features = 0,
> +        .unmigratable_flags = 0,
> +    },
>      [FEAT_XSAVE] = {
>          .type = CPUID_FEATURE_WORD,
>          .feat_names = {
> @@ -5835,6 +5851,7 @@ static const X86CPUDefinition builtin_x86_defs[] = {
>          .features[FEAT_8000_0021_EAX] =
>              CPUID_8000_0021_EAX_NO_NESTED_DATA_BP |
>              CPUID_8000_0021_EAX_LFENCE_ALWAYS_SERIALIZING |
> +            CPUID_8000_0021_EAX_VERW_CLEAR |
>              CPUID_8000_0021_EAX_NULL_SEL_CLR_BASE |
>              CPUID_8000_0021_EAX_AUTO_IBRS,
>          .features[FEAT_7_0_EBX] =
> @@ -7934,6 +7951,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>          *eax = *ebx = *ecx = *edx = 0;
>          *eax = env->features[FEAT_8000_0021_EAX];
>          *ebx = env->features[FEAT_8000_0021_EBX];
> +        *ecx = env->features[FEAT_8000_0021_ECX];
>          break;
>      default:
>          /* reserved values: zero */
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 51e10139dfdf..8b2703f41b73 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -641,6 +641,7 @@ typedef enum FeatureWord {
>      FEAT_8000_0008_EBX, /* CPUID[8000_0008].EBX */
>      FEAT_8000_0021_EAX, /* CPUID[8000_0021].EAX */
>      FEAT_8000_0021_EBX, /* CPUID[8000_0021].EBX */
> +    FEAT_8000_0021_ECX, /* CPUID[8000_0021].ECX */
>      FEAT_8000_0022_EAX, /* CPUID[8000_0022].EAX */
>      FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */
>      FEAT_KVM,           /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */
> @@ -1101,6 +1102,8 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU 
> *cpu, FeatureWord w);
>  #define CPUID_8000_0021_EAX_FS_GS_BASE_NS                (1U << 1)
>  /* LFENCE is always serializing */
>  #define CPUID_8000_0021_EAX_LFENCE_ALWAYS_SERIALIZING    (1U << 2)
> +/* Memory form of VERW mitigates TSA */
> +#define CPUID_8000_0021_EAX_VERW_CLEAR                   (1U << 5)
>  /* Null Selector Clears Base */
>  #define CPUID_8000_0021_EAX_NULL_SEL_CLR_BASE            (1U << 6)
>  /* Automatic IBRS */

We need to define tsa-sq-no and tsa-l1-no here.
These definitions can be used later when adding a new CPU model.

-- 
Thanks
Babu Moger

Reply via email to