Date:   Wed, 8 Feb 2023 14:13:51 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Borislav Petkov <bp@alien8.de>
CC:     Terry Bowman <terry.bowman@amd.com>,
        <linux-kernel@vger.kernel.org>, <x86@kernel.org>
Subject: Re: [PATCH 3/3] tools/x86/kcpuid: Dump the CPUID function in
 detailed view
Message-ID: <Y+M9n8MUd60Wc6S7@feng-clx>
References: <20230206141832.4162264-1-terry.bowman@amd.com>
 <20230206141832.4162264-4-terry.bowman@amd.com>
 <Y+HIkTzNh4SXgVEc@feng-clx>
 <Y+J5jDfO4PMujuXD@zn.tnic>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Y+J5jDfO4PMujuXD@zn.tnic>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?twcMh5zlXM38pDlvKFJ7CkgazKVxBRhBTYsxFCrttx0ySCrG8UZdH4W2uFta?=
 =?us-ascii?Q?xwZDTVv6I6y3/c2odYm0yvcrMkcgZjllLin6zAI97qrShqjJkGK+1TAb01uC?=
 =?us-ascii?Q?hqYvn/lOLcKjRdoBVNGf7jqqB7K1TSbWYm06/QmroqoN4ABBlzG2py+1LUXJ?=
 =?us-ascii?Q?XDVq15jZUL8ov176pctTLihbxlqTuqrabekw/+OktWfjTr2BtCaNz3gL/kAq?=
 =?us-ascii?Q?6u+Z/4hXUDm0gkf/d1wgtXZhZ9pMEUDK4jEEL3MK76zQ97F+GStSJgNwOpLj?=
 =?us-ascii?Q?JdPES43BCQYgz6K6IFsKdK6QFe9yBt+aTGwErArPnzYOuvimnWe6mTNXbqdN?=
 =?us-ascii?Q?kLR8fc4IM2LsPMwGtuwpTL+MY4vAfe5m6lSWQrd1+ZI8fgMo0hugvh9XDK7y?=
 =?us-ascii?Q?55c7iMmxdlKiecnO+z8IoUzc/GPbsPsXifizi9UP79U4UdX55rtgKZ6yPEZe?=
 =?us-ascii?Q?Fjz8ytNjj27iY+0dl+PTRWO+kp2WKG/CmZm0BmfKxfBWdhXK34T0Tkt6oay/?=
 =?us-ascii?Q?asXXW8Cylj+GWj/ogKhvFPV9nvCxCH4sqQwTsFxZzTv5ZIKDyIAKDYED57ub?=
 =?us-ascii?Q?QROQC0h+f5+WqPEIxmlkiu7Fs6LJQ35Lq+46NsMt5lVGorhSmPJTMYjjg7FJ?=
 =?us-ascii?Q?YgDYBHdPLtiEKiyYNh5tQiHrPBy2NberGs4mvVLA46zjNAv7Syv5YKb59WLA?=
 =?us-ascii?Q?gzh8w9A450cQOqFOBPMKfueaxFeyHP4m6EISX+1EmZs0mIXIayrjhDnkRnFZ?=
 =?us-ascii?Q?851qjIqV2WERRgxX+rKwhgDhwr/thdM/2GXg20BSaT32psoW4prHb6QDGwOA?=
 =?us-ascii?Q?IFiVSIrF33GI5o1hSQwTiG8YV2BKHL3ChWUlwbGcDAc6DjnHNQpSyMLzO41X?=
 =?us-ascii?Q?hA2V3zF4c6k8+ZymM5wxqZYcttPAcIVBtHB2TkI/bYcsLt5PTc4ZYfvTSGJ6?=
 =?us-ascii?Q?gjdiDsyQF9mRnpnwr8R8k33m5L45272dYaJTB3kodk3pxy7Q0YA02SwXtZUD?=
 =?us-ascii?Q?JcjwamYp+25v4rxMX7bmgxLDd/UaBr4fRlojdpfKx2N778/+TYgOrwWIXaaa?=
 =?us-ascii?Q?w24s4FkjrEJqEnL21YDvSTVtWwpcFUA+sxQjFQ4DBe4gdxK2RRyT33deVs/2?=
 =?us-ascii?Q?XmOHtT08IJo1fOwgmGdfBh+owwOUx1fNuESCL3d6t4rAUstSler6QGOHovg+?=
 =?us-ascii?Q?HOXtQz1e6LTKWDDjayIFggTUNMlMHYMv9c8rqapsBTTGRfxwub86jImh+Wnq?=
 =?us-ascii?Q?S9rB0xBl5rx0XPGSICcF0V5UjbLYu3vWxE2I0AXuvkgKDA5FnNihobR7gL/R?=
 =?us-ascii?Q?+86TpBPWVwobWT6jiiD+dahy/iyILr8c9OHRzuLm3HdTLQNXGpLMr3A7yRFQ?=
 =?us-ascii?Q?NWDclVkOB9Tds89BoGlSU/QvoNNCkOlo/qpkVSsDYQdhoUFZ2hK+ni82/cqH?=
 =?us-ascii?Q?FAHyH2nT9sj7gUPfFmF78yNwevap92xdMyvb0eyLW0ZtablqQvDwySrWnj5J?=
 =?us-ascii?Q?hZJJXU1WDPWjykWd/bQOAywMqL8mqX8FtonD8fW9F5z5cWbvkwyx5F9kBgj1?=
 =?us-ascii?Q?Xd6vW/EDrKfKnKmiZU9936kh1XiHspj6Lc51z+j8?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a4803802-ded3-47ac-92ca-08db099c4ce6
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Feb 2023 06:18:31.4502
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: RWBpwQZztgwQE62CCDJbwNOirAFM4KcH5uoSf1vOO9eSujMK0OsBlcT2j6fcIpL/KCUhAaY5UhLIq5Iz6WOGdA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6884
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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