Re: [PATCH v5 5/5] x86/cpufeatures: Add the CPU feature bit for MSR immediate form instructions

[Xin Li <xin@zytor.com>]

On 2/22/2025 8:30 AM, Borislav Petkov wrote:
> On Sun, Jan 05, 2025 at 11:07:27PM -0800, Xin Li (Intel) wrote:
>> The immediate form of MSR access instructions are primarily motivated by
>> performance, not code size: by having the MSR number in an immediate, it
>> is available *much* earlier in the pipeline, which allows the hardware
>> much more leeway about how a particular MSR is handled.
>>
>> Add a new CPU feature word for CPUID.7.1.ECX and then the CPU feature bit
>> for MSR immediate form.
> 
> Nope, scattered.c.

Oh, neat.

>> @@ -476,6 +476,9 @@
>>   #define X86_FEATURE_AMD_HETEROGENEOUS_CORES (21*32 + 6) /* Heterogeneous Core Topology */
>>   #define X86_FEATURE_AMD_WORKLOAD_CLASS	(21*32 + 7) /* Workload Classification */
>>   
>> +/* Intel-defined CPU features, CPUID level 0x00000007:1 (ECX), word 22 */
>> +#define X86_FEATURE_MSR_IMM		(22*32+ 5) /* "msr_imm" MSR immediate form instructions */
> 
> Also no "msr_imm": Documentation/arch/x86/cpuinfo.rst

My bad.

> 
> In any case, this patch doesn't belong in this set.
> 

Right.

I tried to show that we don't need to make the following changes due to
NCAPINTS increased to 23 if this patch is based on this patch set.

But if this feature is added through scattered.c, NCAPINTS is not even
changed...

Thanks!
     Xin

---
diff --git a/arch/x86/include/asm/cpufeature.h 
b/arch/x86/include/asm/cpufeature.h
index de1ad09fe8d7..051d006cc0c6 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -95,7 +96,7 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
            CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 20, feature_bit) ||    \
            CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 21, feature_bit) ||    \
            REQUIRED_MASK_CHECK                                    ||    \
-          BUILD_BUG_ON_ZERO(NCAPINTS != 22))
+          BUILD_BUG_ON_ZERO(NCAPINTS != 23))

  #define DISABLED_MASK_BIT_SET(feature_bit)                             \
          ( CHECK_BIT_IN_MASK_WORD(DISABLED_MASK,  0, feature_bit) ||    \
@@ -121,7 +122,7 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
            CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 20, feature_bit) ||    \
            CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 21, feature_bit) ||    \
            DISABLED_MASK_CHECK                                    ||    \
-          BUILD_BUG_ON_ZERO(NCAPINTS != 22))
+          BUILD_BUG_ON_ZERO(NCAPINTS != 23))

  #define cpu_has(c, bit) 
         \
         (__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 :  \
diff --git a/arch/x86/include/asm/disabled-features.h 
b/arch/x86/include/asm/disabled-features.h
index c492bdc97b05..c6a1f962185c 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -156,6 +156,6 @@
  #define DISABLED_MASK19        (DISABLE_SEV_SNP)
  #define DISABLED_MASK20        0
  #define DISABLED_MASK21        0
-#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 22)
+#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 23)

  #endif /* _ASM_X86_DISABLED_FEATURES_H */
diff --git a/arch/x86/include/asm/required-features.h 
b/arch/x86/include/asm/required-features.h
index e9187ddd3d1f..a346db1d078c 100644
--- a/arch/x86/include/asm/required-features.h
+++ b/arch/x86/include/asm/required-features.h
@@ -100,6 +100,6 @@
  #define REQUIRED_MASK19        0
  #define REQUIRED_MASK20        0
  #define REQUIRED_MASK21        0
-#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 22)
+#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 23)

  #endif /* _ASM_X86_REQUIRED_FEATURES_H */