linux-kernel - Re: [PATCH v1 07/26] x86/cpuid: Introduce debugfs 'x86/scanned

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6e9286b1-6d76-4f30-97bd-8fcc3ca8f211@intel.com>
Date: Tue, 13 May 2025 19:56:06 -0700
From: Sohil Mehta <sohil.mehta@...el.com>
To: "Ahmed S. Darwish" <darwi@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>
CC: Thomas Gleixner <tglx@...utronix.de>, Andrew Cooper
	<andrew.cooper3@...rix.com>, "H. Peter Anvin" <hpa@...or.com>, John Ogness
	<john.ogness@...utronix.de>, <x86@...nel.org>, <x86-cpuid@...ts.linux.dev>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 07/26] x86/cpuid: Introduce debugfs
 'x86/scanned_cpuid/[0-ncpus]'

On 5/5/2025 10:04 PM, Ahmed S. Darwish wrote:
> Introduce the debugfs files 'x86/scanned_cpuid/[0-ncpus]' to dump the
> scanned CPUID table for each CPU.
> 

I tried out the patches. The debugfs hierarchy mentioned here doesn't
match the code.

The code actually builds:
  x86/scanned_cpuid/cpus/[0-ncpus].

Can we simplify it to below? The "scanned_" part seems unnecessary.
  x86/cpuid/[0-ncpus]

I would also suggest slight changes to the formatting to make it easier
to read.
1) Indent for the printed register values.
2) Move the * to the front of the value to make it more prominent.

Current
-------
Leaf 0x00000001, subleaf 0:
cached: EAX=0x000306e4  EBX=0x00200800  ECX=0x77bee3ff* EDX=0xbfebfbff
actual: EAX=0x000306e4  EBX=0x00200800  ECX=0x7fbee3ff  EDX=0xbfebfbff
Leaf 0x00000002, subleaf 0:
cached: EAX=0x76036301  EBX=0x00f0b2ff  ECX=0x00000000  EDX=0x00ca0000
actual: EAX=0x76036301  EBX=0x00f0b2ff  ECX=0x00000000  EDX=0x00ca0000
Leaf 0x00000004, subleaf 0:
cached: EAX=0x3c004121  EBX=0x01c0003f  ECX=0x0000003f  EDX=0x00000000
actual: EAX=0x3c004121  EBX=0x01c0003f  ECX=0x0000003f  EDX=0x00000000
Leaf 0x00000004, subleaf 1:
cached: EAX=0x3c004122  EBX=0x01c0003f  ECX=0x0000003f  EDX=0x00000000
actual: EAX=0x3c004122  EBX=0x01c0003f  ECX=0x0000003f  EDX=0x00000000

Suggested
---------
Leaf 0x00000001, subleaf 0:
  cached: EAX=0x000306e4  EBX=0x00200800 *ECX=0x77bee3ff  EDX=0xbfebfbff
  actual: EAX=0x000306e4  EBX=0x00200800  ECX=0x7fbee3ff  EDX=0xbfebfbff
Leaf 0x00000002, subleaf 0:
  cached: EAX=0x76036301  EBX=0x00f0b2ff  ECX=0x00000000  EDX=0x00ca0000
  actual: EAX=0x76036301  EBX=0x00f0b2ff  ECX=0x00000000  EDX=0x00ca0000
Leaf 0x00000004, subleaf 0:
  cached: EAX=0x3c004121  EBX=0x01c0003f  ECX=0x0000003f  EDX=0x00000000
  actual: EAX=0x3c004121  EBX=0x01c0003f  ECX=0x0000003f  EDX=0x00000000
Leaf 0x00000004, subleaf 1:
  cached: EAX=0x3c004122  EBX=0x01c0003f  ECX=0x0000003f  EDX=0x00000000
  actual: EAX=0x3c004122  EBX=0x01c0003f  ECX=0x0000003f  EDX=0x00000000


> While dumping the tables, for each cached CPUID leaf/subleaf entry, run
> the corresponding CPUID instruction on the target CPU.  Compare the live
> hardware output with the cached register values.  If a cached register
> differs, mark its cached value output entry with an asterisk.
> 
> This should help with tricky bug reports in the future, if/when the
> scanned CPUID tables get (unexpectedly) out of sync with actual hardware
> state.  It also simplifies the development and testing of adding new
> CPUID leaves and custom read functions to the CPUID scanner.
> 
> Note, add an extern 'cpuid_common_scan_entries[]' declaration to the
> "cpuid_scanner.h" internal header to allow the debugfs code to access the
> CPUID scan entries directly.
> 
> Signed-off-by: Ahmed S. Darwish <darwi@...utronix.de>
> ---
>  arch/x86/kernel/cpu/Makefile        |  1 +
>  arch/x86/kernel/cpu/cpuid_debugfs.c | 98 +++++++++++++++++++++++++++++
>  arch/x86/kernel/cpu/cpuid_scanner.c |  6 +-
>  arch/x86/kernel/cpu/cpuid_scanner.h |  3 +
>  4 files changed, 106 insertions(+), 2 deletions(-)
>  create mode 100644 arch/x86/kernel/cpu/cpuid_debugfs.c
> 
> diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
> index 994539fd0e17..eb9cd1dee58e 100644
> --- a/arch/x86/kernel/cpu/Makefile
> +++ b/arch/x86/kernel/cpu/Makefile
> @@ -62,6 +62,7 @@ obj-$(CONFIG_HYPERVISOR_GUEST)		+= vmware.o hypervisor.o mshyperv.o
>  obj-$(CONFIG_ACRN_GUEST)		+= acrn.o
>  
>  obj-$(CONFIG_DEBUG_FS)			+= debugfs.o
> +obj-$(CONFIG_DEBUG_FS)			+= cpuid_debugfs.o
>  

How about reusing the same line since the config option is the same?

>  obj-$(CONFIG_X86_BUS_LOCK_DETECT)	+= bus_lock.o
>