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 <[email protected]>
> ---
>  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
>