Re: [PATCH -v1 2/2] x86/microcode: Add microcode loader debugging functionality

[Nikolay Borisov]

On 20.08.25 г. 16:50 ч., Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <bp@xxxxxxxxx>
>
> Instead of adding ad-hoc debugging glue to the microcode loader each
> time I need it, add debugging functionality which is not built by
> default and when built-in, off by default so that it can only be enabled
> explicitly on the command line.
>
> Simulate all patch handling the loader does except the actual loading of
> the microcode patch into the hw.
>
> Signed-off-by: Borislav Petkov (AMD) <bp@xxxxxxxxx>
> ---
>   .../admin-guide/kernel-parameters.txt         |  6 ++
>   arch/x86/Kconfig                              | 12 +++
>   arch/x86/kernel/cpu/microcode/amd.c           | 88 ++++++++++++++-----
>   arch/x86/kernel/cpu/microcode/core.c          | 25 +++++-
>   arch/x86/kernel/cpu/microcode/internal.h      | 10 +++
>   5 files changed, 118 insertions(+), 23 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9e3bbce6583f..e7badf2aba63 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3770,6 +3770,12 @@
>   	microcode=      [X86] Control the behavior of the microcode loader.
>   	                Available options, comma separated:
>   
> +			base_rev=X - with <X> with format: <u32>
> +			Set the base microcode revision of each thread when in
> +			debug mode.
> +
> +			dbg: enable debugging mode when run in a guest

nit: s/in a guest// since nothing in the debug code is really dependent 
on whether it's run as a guest or not.
> +
>   			dis_ucode_ldr: disable the microcode loader
>   
>   			force_minrev:
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index aa250d90f927..77f72f075d89 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1360,6 +1360,18 @@ config MICROCODE_LATE_FORCE_MINREV
>   
>   	  If unsure say Y.
>   
> +config MICROCODE_DBG
> +	bool "Enable microcode loader debugging"
> +	default n
> +	depends on MICROCODE
> +	help
> +	  Enable code which allows for debugging the microcode loader in
> +	  a guest. Meaning the patch loading is simulated but everything else

dito, AFAICS it's perfectly fine to have the debug output if not run in 
a guest, no ?
> +	  related to patch parsing and handling is done as on baremetal with
> +	  the purpose of debugging solely the software side of things.
> +
> +	  You almost certainly want to say n here.
> +
>   config X86_MSR
>   	tristate "/dev/cpu/*/msr - Model-specific register support"
>   	help


<snip>