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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/05/2025 4:17 pm, Kevin Lampis wrote:
> Add new cpuid features for Sierra Forest.
>
> Signed-off-by: Kevin Lampis <kevin.lampis@xxxxxxxxx>

One minor thing, you should have CC'd the x86 maintainers on this patch,
which I've done.

> ---
>  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..c0587be972 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 */

This is a corner case, but I typically take out the space before the 10
to keep the latter part aligned.

(Although this is going to suck for ARCH_PERF_MON2 which is coming soon
too.)

>  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 */
> +XEN_CPUFEATURE(INVD_DISABLE, 10*32+30) /*   INVD_DISABLE_POST_BIOS_DONE */

I see this is the name Intel gave it, but "NO_INVD" is shorter and the
semantic is only relevant to very early firmware.

Also, 36 years late, but at least the fix has gotten out eventually... 
AMD fixed this decades ago by treating INVD as WBINVD.

>  
>  /* 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_DISABLE,    13*32+ 6) /*   UC-lock disable */

We tend to abbreviate to DIS.  (Intel is inconsistent on whether they do
or not.)

>  
>  /* 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_UIF */

For the comment, "UIRET updates UIF" which is a bit better than simply
restating the name.  (Although Intel's "flexible update of ..." when
they mean "oops we didn't design it right to start with" can stay in the
Intel manual).

>  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_ENUMERATION,    16*32+16) /*   MCU_ENUMERATION */

I don't have a better suggestion, but I'm not thrilled by the name
MCU_ENUMERATION.

It's a whole bunch of different microcode loading changes.

~Andrew