Re: [PATCH v2] x86/cpufeatures: cpuid features for Sierra Forest

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/05/2025 9:13 am, Kevin Lampis wrote:
> Add new cpuid features for Sierra Forest.
>
> Signed-off-by: Kevin Lampis <kevin.lampis@xxxxxxxxx>
> ---
> Changes in v2:
> - change INVD_DISABLE to NO_INVD
> - change UC_LOCK_DISABLE to UC_LOCK_DIS
> - better comment for UIRET_UIF
> - use MCU_ENUM because it's shorter and add better comment
> ---
>  xen/include/public/arch-x86/cpufeatureset.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/xen/include/public/arch-x86/cpufeatureset.h 
> b/xen/include/public/arch-x86/cpufeatureset.h
> index cc6e984a88..089a133519 100644
> --- a/xen/include/public/arch-x86/cpufeatureset.h
> +++ b/xen/include/public/arch-x86/cpufeatureset.h
> @@ -304,13 +304,18 @@ XEN_CPUFEATURE(SM3,          10*32+ 1) /*A  SM3 
> Instructions */
>  XEN_CPUFEATURE(SM4,          10*32+ 2) /*A  SM4 Instructions */
>  XEN_CPUFEATURE(AVX_VNNI,     10*32+ 4) /*A  AVX-VNNI Instructions */
>  XEN_CPUFEATURE(AVX512_BF16,  10*32+ 5) /*A  AVX512 BFloat16 Instructions */
> +XEN_CPUFEATURE(LASS,         10*32+ 6) /*   Linear Address Space Separation 
> */
>  XEN_CPUFEATURE(CMPCCXADD,    10*32+ 7) /*a  CMPccXADD Instructions */
> +XEN_CPUFEATURE(ARCH_PERF_MON, 10*32+ 8) /*  ArchPerfMonExt */
>  XEN_CPUFEATURE(FZRM,         10*32+10) /*A  Fast Zero-length REP MOVSB */
>  XEN_CPUFEATURE(FSRS,         10*32+11) /*A  Fast Short REP STOSB */
>  XEN_CPUFEATURE(FSRCS,        10*32+12) /*A  Fast Short REP CMPSB/SCASB */
>  XEN_CPUFEATURE(WRMSRNS,      10*32+19) /*S  WRMSR Non-Serialising */
>  XEN_CPUFEATURE(AMX_FP16,     10*32+21) /*   AMX FP16 instruction */
>  XEN_CPUFEATURE(AVX_IFMA,     10*32+23) /*A  AVX-IFMA Instructions */
> +XEN_CPUFEATURE(LAM,          10*32+26) /*   Linear Address Masking */
> +XEN_CPUFEATURE(MSRLIST,      10*32+27) /*   RDMSRLIST/WRMSRLIST/WRMSRNS */

"{RD,WR}MSRLIST instructions"

WRMSRNS is separately enumerated (bit 19).

> +XEN_CPUFEATURE(NO_INVD,      10*32+30) /*   INVD_DISABLE_POST_BIOS_DONE */
>  
>  /* AMD-defined CPU features, CPUID level 0x80000021.eax, word 11 */
>  XEN_CPUFEATURE(NO_NEST_BP,         11*32+ 0) /*A  No Nested Data Breakpoints 
> */
> @@ -340,6 +345,7 @@ XEN_CPUFEATURE(RRSBA_CTRL,         13*32+ 2) /*S  
> MSR_SPEC_CTRL.RRSBA_DIS_* */
>  XEN_CPUFEATURE(DDP_CTRL,           13*32+ 3) /*   MSR_SPEC_CTRL.DDP_DIS_U */
>  XEN_CPUFEATURE(BHI_CTRL,           13*32+ 4) /*S  MSR_SPEC_CTRL.BHI_DIS_S */
>  XEN_CPUFEATURE(MCDT_NO,            13*32+ 5) /*A  MCDT_NO */
> +XEN_CPUFEATURE(UC_LOCK_DIS,        13*32+ 6) /*   UC-lock disable */
>  
>  /* Intel-defined CPU features, CPUID level 0x00000007:1.ecx, word 14 */
>  
> @@ -349,7 +355,9 @@ XEN_CPUFEATURE(AVX_NE_CONVERT,     15*32+ 5) /*A  
> AVX-NE-CONVERT Instructions */
>  XEN_CPUFEATURE(AMX_COMPLEX,        15*32+ 8) /*   AMX Complex Instructions */
>  XEN_CPUFEATURE(AVX_VNNI_INT16,     15*32+10) /*A  AVX-VNNI-INT16 
> Instructions */
>  XEN_CPUFEATURE(PREFETCHI,          15*32+14) /*A  PREFETCHIT{0,1} 
> Instructions */
> +XEN_CPUFEATURE(UIRET_UIF,          15*32+17) /*   UIRET updates UIF */
>  XEN_CPUFEATURE(CET_SSS,            15*32+18) /*   CET Supervisor Shadow 
> Stacks safe to use */
> +XEN_CPUFEATURE(SLSM,               15*32+24) /*   Static Lockstep Mode */
>  
>  /* Intel-defined CPU features, MSR_ARCH_CAPS 0x10a.eax, word 16 */
>  XEN_CPUFEATURE(RDCL_NO,            16*32+ 0) /*A  No Rogue Data Cache Load 
> (Meltdown) */
> @@ -368,6 +376,7 @@ XEN_CPUFEATURE(DOITM,              16*32+12) /*   Data 
> Operand Invariant Timing
>  XEN_CPUFEATURE(SBDR_SSDP_NO,       16*32+13) /*A  No Shared Buffer Data Read 
> or Sideband Stale Data Propagation */
>  XEN_CPUFEATURE(FBSDP_NO,           16*32+14) /*A  No Fill Buffer Stale Data 
> Propagation */
>  XEN_CPUFEATURE(PSDP_NO,            16*32+15) /*A  No Primary Stale Data 
> Propagation */
> +XEN_CPUFEATURE(MCU_ENUM,           16*32+16) /*   Runtime Microcode Update 
> features */

Thinking about this, I'm tempted to call it MCU_EXT because that's a bit
better than MCU_ENUM.

That, and "MCU_STATUS/ENUM MSRs" as a comment gets the point across, I
think.

>  XEN_CPUFEATURE(FB_CLEAR,           16*32+17) /*!A| Fill Buffers cleared by 
> VERW */
>  XEN_CPUFEATURE(FB_CLEAR_CTRL,      16*32+18) /*   
> MSR_OPT_CPU_CTRL.FB_CLEAR_DIS */
>  XEN_CPUFEATURE(RRSBA,              16*32+19) /*!  Restricted RSB Alternative 
> */
> @@ -379,6 +388,7 @@ XEN_CPUFEATURE(GDS_CTRL,           16*32+25) /*   
> MCU_OPT_CTRL.GDS_MIT_{DIS,LOCK
>  XEN_CPUFEATURE(GDS_NO,             16*32+26) /*A  No Gather Data Sampling */
>  XEN_CPUFEATURE(RFDS_NO,            16*32+27) /*A  No Register File Data 
> Sampling */
>  XEN_CPUFEATURE(RFDS_CLEAR,         16*32+28) /*!A| Register File(s) cleared 
> by VERW */
> +XEN_CPUFEATURE(IGN_UMONITOR,       16*32+29) /*   IGN_UMONITOR_SUPPORT */

"MCU_OPT_CTRL.IGN_UMONITOR"

While not strictly SRF, We should include bit 30 too, because its
related and retrofitted onto older CPUs in microcode.  See
https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/technical-documentation/monitor-umonitor-performance-guidance.html
for full details.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

I can fix all on commit.

~Andrew