Re: [PATCH v3 4/4] x86/ucode: Add Kconfig option to remove microcode loading

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 13/01/2026 12:21 pm, Alejandro Vallejo wrote:
> Amend docs to reflect ucode= command/stanza depend on MICROCODE_LOADING
> being set.
>
> The new MICROCODE_OP() is a conditional setter for the microcode_ops
> struct. By using IS_ENABLED() there ratehr than ifdef we allow DCE to

rather

> remove all statics no longer used when microcode loading is disabled
> without tagging them with __maybe_unused.
>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> with a couple
more minor points, all of which I can fix on commit.

AFAICT, this can be reordered with the earlycpuio patch 3 ?

> diff --git a/docs/misc/efi.pandoc b/docs/misc/efi.pandoc
> index 11c1ac3346..c3fb1f3a30 100644
> --- a/docs/misc/efi.pandoc
> +++ b/docs/misc/efi.pandoc
> @@ -104,6 +104,8 @@ Specifies an XSM module to load.
>  
>  Specifies a CPU microcode blob to load. (x86 only)
>  
> +Only available when Xen is compiled with CONFIG_MICROCODE_LOADING.

enabled.

> +
>  ###`dtb=<filename>`
>  
>  Specifies a device tree file to load.  The platform firmware may provide a
> diff --git a/docs/misc/xen-command-line.pandoc 
> b/docs/misc/xen-command-line.pandoc
> index 15f7a315a4..866fa2f951 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -2748,7 +2748,7 @@ performance.
>  ### ucode
>  > `= List of [ <integer> | scan=<bool>, nmi=<bool>, digest-check=<bool> ]`
>  
> -    Applicability: x86
> +    Applicability: x86 with CONFIG_MICROCODE_LOADING active
>      Default: `scan` is selectable via Kconfig, `nmi,digest-check`

This isn't actually how we provide such information.  It's usually a
sentence in the first paragraph.

diff --git a/docs/misc/xen-command-line.pandoc
b/docs/misc/xen-command-line.pandoc
index 50d7edb2488e..5b6ff94f441a 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2751,9 +2751,10 @@ performance.
     Applicability: x86
     Default: `scan` is selectable via Kconfig, `nmi,digest-check`
 
-Controls for CPU microcode loading. For early loading, this parameter can
-specify how and where to find the microcode update blob. For late loading,
-this parameter specifies if the update happens within a NMI handler.
+Controls for CPU microcode loading, available when
`CONFIG_MICROCODE_LOADING`
+is enabled.  For early loading, this parameter can specify how and where to
+find the microcode update blob. For late loading, this parameter
specifies if
+the update happens within a NMI handler.
 
 'integer' specifies the CPU microcode update blob module index. When
positive,
 this specifies the n-th module (in the GrUB entry, zero based) to be used


which is the re-wrapped version of inserting ', available when
`CONFIG_MICROCODE_LOADING` is enabled'.

> diff --git a/xen/arch/x86/cpu/microcode/amd.c 
> b/xen/arch/x86/cpu/microcode/amd.c
> index 71b041c84b..86706a21a6 100644
> --- a/xen/arch/x86/cpu/microcode/amd.c
> +++ b/xen/arch/x86/cpu/microcode/amd.c
> @@ -561,11 +561,11 @@ static const char __initconst amd_cpio_path[] =
>      "kernel/x86/microcode/AuthenticAMD.bin";
>  
>  static const struct microcode_ops __initconst_cf_clobber amd_ucode_ops = {
> -    .cpu_request_microcode            = cpu_request_microcode,
> +    .cpu_request_microcode            = MICROCODE_OP(cpu_request_microcode),
>      .collect_cpu_info                 = collect_cpu_info,
> -    .apply_microcode                  = apply_microcode,
> -    .compare                          = amd_compare,
> -    .cpio_path                        = amd_cpio_path,
> +    .apply_microcode                  = MICROCODE_OP(apply_microcode),
> +    .compare                          = MICROCODE_OP(amd_compare),
> +    .cpio_path                        = MICROCODE_OP(amd_cpio_path),
>  };

Reordering cpu_request_microcode and collect_cpu_info like before makes
this easier to read.

> diff --git a/xen/arch/x86/cpu/microcode/private.h 
> b/xen/arch/x86/cpu/microcode/private.h
> index e6c965dc99..1167b79db1 100644
> --- a/xen/arch/x86/cpu/microcode/private.h
> +++ b/xen/arch/x86/cpu/microcode/private.h
> @@ -93,4 +93,6 @@ void ucode_probe_intel(struct microcode_ops *ops);
>  static inline void ucode_probe_intel(struct microcode_ops *ops) {}
>  #endif
>  
> +#define MICROCODE_OP(x) (IS_ENABLED(CONFIG_MICROCODE_LOADING) ? (x) : NULL)
> +

This wants /* Aids dead-code elimination of the static hooks */

and wants to be up beside struct microcode_ops.

> diff --git a/xen/arch/x86/platform_hypercall.c 
> b/xen/arch/x86/platform_hypercall.c
> index 79bb99e0b6..a55060e662 100644
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -309,22 +309,30 @@ ret_t do_platform_op(
>  
>      case XENPF_microcode_update:
>      {
> -        XEN_GUEST_HANDLE(const_void) data;
> +        ret = -EOPNOTSUPP;
> +        if ( IS_ENABLED(CONFIG_MICROCODE_LOADING) )
> +        {
> +            XEN_GUEST_HANDLE(const_void) data;
>  
> -        guest_from_compat_handle(data, op->u.microcode.data);
> +            guest_from_compat_handle(data, op->u.microcode.data);
> +            ret = ucode_update_hcall(data, op->u.microcode.length, 0);
> +        }
>  
> -        ret = ucode_update_hcall(data, op->u.microcode.length, 0);
>          break;
>      }
>  
>      case XENPF_microcode_update2:
>      {
> -        XEN_GUEST_HANDLE(const_void) data;
> +        ret = -EOPNOTSUPP;
> +        if ( IS_ENABLED(CONFIG_MICROCODE_LOADING) )
> +        {
> +            XEN_GUEST_HANDLE(const_void) data;
>  
> -        guest_from_compat_handle(data, op->u.microcode2.data);
> +            guest_from_compat_handle(data, op->u.microcode2.data);
> +            ret = ucode_update_hcall(data, op->u.microcode2.length,
> +                                     op->u.microcode2.flags);
> +        }
>  
> -        ret = ucode_update_hcall(data, op->u.microcode2.length,
> -                                 op->u.microcode2.flags);
>          break;
>      }
>  


Both these hunks will be smaller as:

    ret = -EOPNOTSUPP;
    if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) )
        break;

and that's the preferred style too.

Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> and I
can fix all of these con commit.

AFAICT, this can be reordered with the earlycpio patch 3 ?

~Andrew