Re: [PATCH 3/3] tools/x86/kcpuid: Dump the CPUID function in detailed view

[Feng Tang <feng.tang@intel.com>]

On Tue, Feb 07, 2023 at 05:17:33PM +0100, Borislav Petkov wrote:
> On Tue, Feb 07, 2023 at 11:42:09AM +0800, Feng Tang wrote:
> > Maybe we can check the sum of subleaf->info.nr[EAX/EBX/ECX/EDX],
> > and only print it out when it is not zero.
> 
> Considering how this is only a --detail output and I kinda find the
> default output a bit too terse, yeah, I think we should dump all the
> values with -d. Something like the below diff ontop.
> 
> I've added the output at the end and I think it makes perfect sense to
> dump those raw values with -d. In time, we will start decoding them too
> so that we can have a full, human readable dump of the CPUID leafs and
> all the data needed.
> 
> ---
> 
> diff --git a/tools/arch/x86/kcpuid/cpuid.csv b/tools/arch/x86/kcpuid/cpuid.csv
> index 59ee3b53309a..baadf2cdd1c6 100644
> --- a/tools/arch/x86/kcpuid/cpuid.csv
> +++ b/tools/arch/x86/kcpuid/cpuid.csv
> @@ -375,7 +375,7 @@
>  0x80000001,    0,  ECX,     27, perftsc, Performance time-stamp counter supported
>  0x80000001,    0,  ECX,     28, perfctrextllc, Indicates support for L3 performance counter extensions
>  0x80000001,    0,  ECX,     29, mwaitextended, MWAITX and MONITORX capability is supported
> -0x80000001,    0,  ECX,     30. admskextn, Indicates support for address mask extension (to 32 bits and to all 4 DRs) for instruction breakpoints
> +0x80000001,    0,  ECX,     30, admskextn, Indicates support for address mask extension (to 32 bits and to all 4 DRs) for instruction breakpoints
>  
>  0x80000001,    0,  EDX,      0, fpu, x87 floating point unit on-chip
>  0x80000001,    0,  EDX,      1, vme, Virtual-mode enhancements
> diff --git a/tools/arch/x86/kcpuid/kcpuid.c b/tools/arch/x86/kcpuid/kcpuid.c
> index 26fa5255c42b..1b1fa13a9921 100644
> --- a/tools/arch/x86/kcpuid/kcpuid.c
> +++ b/tools/arch/x86/kcpuid/kcpuid.c
> @@ -33,7 +33,7 @@ struct reg_desc {
>  	struct bits_desc descs[32];
>  };
>  
> -enum {
> +enum cpuid_reg {
>  	R_EAX = 0,
>  	R_EBX,
>  	R_ECX,
> @@ -41,6 +41,10 @@ enum {
>  	NR_REGS
>  };
>  
> +static const char * const reg_names[] = {
> +	"EAX", "EBX", "ECX", "EDX",
> +};
> +
>  struct subleaf {
>  	u32 index;
>  	u32 sub;
> @@ -445,12 +449,18 @@ static void parse_text(void)
>  
>  
>  /* Decode every eax/ebx/ecx/edx */
> -static void decode_bits(u32 value, struct reg_desc *rdesc)
> +static void decode_bits(u32 value, struct reg_desc *rdesc, enum cpuid_reg reg)
>  {
>  	struct bits_desc *bdesc;
>  	int start, end, i;
>  	u32 mask;
>  
> +	if (!rdesc->nr) {
> +		if (show_details)
> +			printf("\t %s: 0x%08x\n", reg_names[reg], value);
> +		return;
> +	}
> +
>  	for (i = 0; i < rdesc->nr; i++) {
>  		bdesc = &rdesc->descs[i];
>  
> @@ -493,10 +503,10 @@ static void show_leaf(struct subleaf *leaf)
>  				leaf->index, leaf->sub);
>  	}
>  
> -	decode_bits(leaf->eax, &leaf->info[R_EAX]);
> -	decode_bits(leaf->ebx, &leaf->info[R_EBX]);
> -	decode_bits(leaf->ecx, &leaf->info[R_ECX]);
> -	decode_bits(leaf->edx, &leaf->info[R_EDX]);
> +	decode_bits(leaf->eax, &leaf->info[R_EAX], R_EAX);
> +	decode_bits(leaf->ebx, &leaf->info[R_EBX], R_EBX);
> +	decode_bits(leaf->ecx, &leaf->info[R_ECX], R_ECX);
> +	decode_bits(leaf->edx, &leaf->info[R_EDX], R_EDX);
>  
>  	if (!show_raw && show_details)
>  		printf("\n");

I also gave it a run, and all looks great to me, thanks!

Reviewed-by: Feng Tang <feng.tang@intel.com>

Thanks,
Feng