[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <82cdc358-3551-4c8b-a387-0e9da4668b44@intel.com>
Date: Mon, 11 Aug 2025 20:04:58 -0700
From: Sohil Mehta <sohil.mehta@...el.com>
To: Borislav Petkov <bp@...nel.org>, X86 ML <x86@...nel.org>
CC: LKML <linux-kernel@...r.kernel.org>, "Borislav Petkov (AMD)"
<bp@...en8.de>
Subject: Re: [PATCH 2/2] x86/microcode: Add microcode loader debugging
functionality
On 8/9/2025 2:42 AM, Borislav Petkov wrote:
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 7c095177da85..fc002b1a9f57 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3770,6 +3770,13 @@
> microcode= [X86] Control the behavior of the microcode loader.
> Available options, comma separated:
>
> + dbg - Format: <bool>
Since this is all new code, would it be better to use the expanded form
of dbg everywhere?
s/dbg/debug/
s/MICROCODE_DBG/MICROCODE_DEBUG
s/ucode_dbg/ucode_debug
Also, I didn't understand the "Format: <bool>".
Isn't the usage similar to force_minrev? So microcode=debug should be
enough, right?
> + enable debugging mode when run in a guest
> +
> + base_rev=X - with <X> with format: <u32>
> + Set the base microcode revision of each thread when in
> + debug mode.
> +
> force_minrev
> Enable or disable the microcode minimal revision
> enforcement for the runtime microcode loader.
Slight preference for a tabbed description:
debug
enable debugging mode when run in a guest
base_rev=X - with <X> with format: <u32>
Set the base microcode revision of each thread when in
debug mode.
force_minrev
Enable or disable the microcode minimal revision
enforcement for the runtime microcode loader.
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 58d890fe2100..462bf03aeda5 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
Isn't default n redundant? I am fine with keeping it to make it obvious.
> + depends on MICROCODE
Should we also include a dependency on DEBUG_KERNEL?
> + help
> + Enable code which allows for debugging the microcode loader in
> + a guest. Meaning the patch loading is simulated but everything else
> + 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.
> +
Powered by blists - more mailing lists