* [PATCH 1/4] x86: Split out AMD-specific code to be executed without ucode loading
2025-11-12 16:22 [PATCH 0/4] Add Kconfig option to remove microcode loading support Alejandro Vallejo
@ 2025-11-12 16:22 ` Alejandro Vallejo
2025-11-12 16:22 ` [PATCH 2/4] x86: Split out Intel-specific " Alejandro Vallejo
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Alejandro Vallejo @ 2025-11-12 16:22 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné
Some code must be executed even with microcode loading disabled to find
out the current microcode revision. This is important to determine active
erratas and such.
With the intent of stripping microcode loading via Kconfig, move such
essential AMD-specific code to an amd-base.c file.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/cpu/microcode/Makefile | 1 +
xen/arch/x86/cpu/microcode/amd-base.c | 50 ++++++++++++++++++++++++
xen/arch/x86/cpu/microcode/amd.c | 55 +++------------------------
xen/arch/x86/cpu/microcode/amd.h | 15 ++++++++
4 files changed, 72 insertions(+), 49 deletions(-)
create mode 100644 xen/arch/x86/cpu/microcode/amd-base.c
create mode 100644 xen/arch/x86/cpu/microcode/amd.h
diff --git a/xen/arch/x86/cpu/microcode/Makefile b/xen/arch/x86/cpu/microcode/Makefile
index 30d600544f..00aa0f24e4 100644
--- a/xen/arch/x86/cpu/microcode/Makefile
+++ b/xen/arch/x86/cpu/microcode/Makefile
@@ -1,3 +1,4 @@
obj-$(CONFIG_AMD) += amd.o
+obj-$(CONFIG_AMD) += amd-base.o
obj-y += core.o
obj-$(CONFIG_INTEL) += intel.o
diff --git a/xen/arch/x86/cpu/microcode/amd-base.c b/xen/arch/x86/cpu/microcode/amd-base.c
new file mode 100644
index 0000000000..f8f5fac1e1
--- /dev/null
+++ b/xen/arch/x86/cpu/microcode/amd-base.c
@@ -0,0 +1,50 @@
+#include <xen/init.h>
+
+#include <asm/msr.h>
+#include <asm/processor.h>
+#include <asm/x86-vendors.h>
+
+#include "amd.h"
+
+#define pr_debug(x...) ((void)0)
+
+static void cf_check collect_cpu_info(void)
+{
+ struct cpu_signature *csig = &this_cpu(cpu_sig);
+
+ memset(csig, 0, sizeof(*csig));
+
+ csig->sig = cpuid_eax(1);
+ rdmsrl(MSR_AMD_PATCHLEVEL, csig->rev);
+
+ pr_debug("microcode: CPU%d collect_cpu_info: patch_id=%#x\n",
+ smp_processor_id(), csig->rev);
+}
+
+static const struct microcode_ops __initconst_cf_clobber amd_ucode_ops = {
+ .cpu_request_microcode = amd_cpu_request_microcode,
+ .collect_cpu_info = collect_cpu_info,
+ .apply_microcode = amd_apply_microcode,
+ .compare = amd_compare,
+ .cpio_path = amd_cpio_path,
+};
+
+void __init ucode_probe_amd(struct microcode_ops *ops)
+{
+ /*
+ * The Entrysign vulnerability (SB-7033, CVE-2024-36347) affects Zen1-5
+ * CPUs. Taint Xen if digest checking is turned off.
+ */
+ if ( boot_cpu_data.family >= 0x17 && boot_cpu_data.family <= 0x1a &&
+ !opt_digest_check )
+ {
+ printk(XENLOG_WARNING
+ "Microcode patch additional digest checks disabled\n");
+ add_taint(TAINT_CPU_OUT_OF_SPEC);
+ }
+
+ if ( boot_cpu_data.family < 0x10 )
+ return;
+
+ *ops = amd_ucode_ops;
+}
diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 550b8c1e57..c6d61fd38c 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -22,9 +22,7 @@
#include <asm/msr.h>
-#include "private.h"
-
-#define pr_debug(x...) ((void)0)
+#include "amd.h"
struct equiv_cpu_entry {
uint32_t installed_cpu;
@@ -153,19 +151,6 @@ static bool check_digest(const struct container_microcode *mc)
return true;
}
-static void cf_check collect_cpu_info(void)
-{
- struct cpu_signature *csig = &this_cpu(cpu_sig);
-
- memset(csig, 0, sizeof(*csig));
-
- csig->sig = cpuid_eax(1);
- rdmsrl(MSR_AMD_PATCHLEVEL, csig->rev);
-
- pr_debug("microcode: CPU%d collect_cpu_info: patch_id=%#x\n",
- smp_processor_id(), csig->rev);
-}
-
static bool verify_patch_size(uint32_t patch_size)
{
uint32_t max_size;
@@ -264,7 +249,7 @@ static bool microcode_fits_cpu(const struct microcode_patch *patch)
return equiv.id == patch->processor_rev_id;
}
-static int cf_check amd_compare(
+int cf_check amd_compare(
const struct microcode_patch *old, const struct microcode_patch *new)
{
/* Both patches to compare are supposed to be applicable to local CPU. */
@@ -310,8 +295,8 @@ static bool check_min_rev(const struct microcode_patch *patch)
return this_cpu(cpu_sig).rev >= patch->min_rev;
}
-static int cf_check apply_microcode(const struct microcode_patch *patch,
- unsigned int flags)
+int cf_check amd_apply_microcode(const struct microcode_patch *patch,
+ unsigned int flags)
{
int hw_err, result;
unsigned int cpu = smp_processor_id();
@@ -424,7 +409,7 @@ static int scan_equiv_cpu_table(const struct container_equiv_table *et)
return -ESRCH;
}
-static struct microcode_patch *cf_check cpu_request_microcode(
+struct microcode_patch *cf_check amd_cpu_request_microcode(
const void *buf, size_t size, bool make_copy)
{
const struct microcode_patch *saved = NULL;
@@ -559,37 +544,9 @@ static struct microcode_patch *cf_check cpu_request_microcode(
return patch;
}
-static const char __initconst amd_cpio_path[] =
+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,
- .collect_cpu_info = collect_cpu_info,
- .apply_microcode = apply_microcode,
- .compare = amd_compare,
- .cpio_path = amd_cpio_path,
-};
-
-void __init ucode_probe_amd(struct microcode_ops *ops)
-{
- /*
- * The Entrysign vulnerability (SB-7033, CVE-2024-36347) affects Zen1-5
- * CPUs. Taint Xen if digest checking is turned off.
- */
- if ( boot_cpu_data.family >= 0x17 && boot_cpu_data.family <= 0x1a &&
- !opt_digest_check )
- {
- printk(XENLOG_WARNING
- "Microcode patch additional digest checks disabled\n");
- add_taint(TAINT_CPU_OUT_OF_SPEC);
- }
-
- if ( boot_cpu_data.family < 0x10 )
- return;
-
- *ops = amd_ucode_ops;
-}
-
#if 0 /* Manual CONFIG_SELF_TESTS */
static void __init __constructor test_digests_sorted(void)
{
diff --git a/xen/arch/x86/cpu/microcode/amd.h b/xen/arch/x86/cpu/microcode/amd.h
new file mode 100644
index 0000000000..1df1b61adb
--- /dev/null
+++ b/xen/arch/x86/cpu/microcode/amd.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef ASM_X86_MICROCODE_AMD_H
+#define ASM_X86_MICROCODE_AMD_H
+
+#include "private.h"
+
+int cf_check amd_compare(const struct microcode_patch *old,
+ const struct microcode_patch *new);
+int cf_check amd_apply_microcode(const struct microcode_patch *patch,
+ unsigned int flags);
+struct microcode_patch *cf_check amd_cpu_request_microcode(
+ const void *buf, size_t size, bool make_copy);
+extern const char amd_cpio_path[];
+
+#endif /* ASM_X86_MICROCODE_AMD_H */
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/4] x86: Split out Intel-specific code to be executed without ucode loading
2025-11-12 16:22 [PATCH 0/4] Add Kconfig option to remove microcode loading support Alejandro Vallejo
2025-11-12 16:22 ` [PATCH 1/4] x86: Split out AMD-specific code to be executed without ucode loading Alejandro Vallejo
@ 2025-11-12 16:22 ` Alejandro Vallejo
2025-11-12 16:22 ` [PATCH 3/4] x86: Split out early_microcode_load() and microcode_load_one() Alejandro Vallejo
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Alejandro Vallejo @ 2025-11-12 16:22 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné
Some code must be executed even with microcode loading disabled to find
out the current microcode revision. This is important to determine active
erratas and such.
With the intent of stripping microcode loading via Kconfig, move such
essential Intel-specific code to an intel-base.c file.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/cpu/microcode/Makefile | 1 +
xen/arch/x86/cpu/microcode/intel-base.c | 48 +++++++++++++++++++++
xen/arch/x86/cpu/microcode/intel.c | 56 ++++---------------------
xen/arch/x86/cpu/microcode/intel.h | 16 +++++++
4 files changed, 72 insertions(+), 49 deletions(-)
create mode 100644 xen/arch/x86/cpu/microcode/intel-base.c
create mode 100644 xen/arch/x86/cpu/microcode/intel.h
diff --git a/xen/arch/x86/cpu/microcode/Makefile b/xen/arch/x86/cpu/microcode/Makefile
index 00aa0f24e4..74289981e3 100644
--- a/xen/arch/x86/cpu/microcode/Makefile
+++ b/xen/arch/x86/cpu/microcode/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_AMD) += amd.o
obj-$(CONFIG_AMD) += amd-base.o
obj-y += core.o
obj-$(CONFIG_INTEL) += intel.o
+obj-$(CONFIG_INTEL) += intel-base.o
diff --git a/xen/arch/x86/cpu/microcode/intel-base.c b/xen/arch/x86/cpu/microcode/intel-base.c
new file mode 100644
index 0000000000..4fcacaa192
--- /dev/null
+++ b/xen/arch/x86/cpu/microcode/intel-base.c
@@ -0,0 +1,48 @@
+#include <xen/init.h>
+
+#include <asm/msr.h>
+#include <asm/processor.h>
+
+#include "intel.h"
+
+#define pr_debug(x...) ((void)0)
+
+static void cf_check collect_cpu_info(void)
+{
+ struct cpu_signature *csig = &this_cpu(cpu_sig);
+ uint64_t msr_content;
+
+ memset(csig, 0, sizeof(*csig));
+
+ rdmsrl(MSR_IA32_PLATFORM_ID, msr_content);
+ csig->pf = 1 << ((msr_content >> 50) & 7);
+
+ /*
+ * Obtaining the microcode version involves writing 0 to the "read only"
+ * UCODE_REV MSR, executing any CPUID instruction, after which a nonzero
+ * revision should appear.
+ */
+ wrmsrl(MSR_IA32_UCODE_REV, 0);
+ csig->sig = cpuid_eax(1);
+ rdmsrl(MSR_IA32_UCODE_REV, msr_content);
+ csig->rev = msr_content >> 32;
+
+ pr_debug("microcode: collect_cpu_info : sig=%#x, pf=%#x, rev=%#x\n",
+ csig->sig, csig->pf, csig->rev);
+}
+
+static const struct microcode_ops __initconst_cf_clobber intel_ucode_ops = {
+ .cpu_request_microcode = intel_cpu_request_microcode,
+ .apply_microcode = intel_apply_microcode,
+ .collect_cpu_info = collect_cpu_info,
+ .compare = intel_compare,
+ .cpio_path = intel_cpio_path,
+};
+
+void __init ucode_probe_intel(struct microcode_ops *ops)
+{
+ *ops = intel_ucode_ops;
+
+ if ( !intel_can_load_microcode() )
+ ops->apply_microcode = NULL;
+}
diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c
index 281993e725..c5e0012a03 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -31,9 +31,7 @@
#include <asm/processor.h>
#include <asm/system.h>
-#include "private.h"
-
-#define pr_debug(x...) ((void)0)
+#include "intel.h"
struct microcode_patch {
uint32_t hdrver;
@@ -120,30 +118,6 @@ static bool signature_matches(const struct cpu_signature *cpu_sig,
return cpu_sig->pf & ucode_pf;
}
-static void cf_check collect_cpu_info(void)
-{
- struct cpu_signature *csig = &this_cpu(cpu_sig);
- uint64_t msr_content;
-
- memset(csig, 0, sizeof(*csig));
-
- rdmsrl(MSR_IA32_PLATFORM_ID, msr_content);
- csig->pf = 1 << ((msr_content >> 50) & 7);
-
- /*
- * Obtaining the microcode version involves writing 0 to the "read only"
- * UCODE_REV MSR, executing any CPUID instruction, after which a nonzero
- * revision should appear.
- */
- wrmsrl(MSR_IA32_UCODE_REV, 0);
- csig->sig = cpuid_eax(1);
- rdmsrl(MSR_IA32_UCODE_REV, msr_content);
- csig->rev = msr_content >> 32;
-
- pr_debug("microcode: collect_cpu_info : sig=%#x, pf=%#x, rev=%#x\n",
- csig->sig, csig->pf, csig->rev);
-}
-
/*
* Sanity check a blob which is expected to be a microcode patch. The 48 byte
* header is of a known format, and together with totalsize are within the
@@ -273,7 +247,7 @@ static bool microcode_fits_cpu(const struct microcode_patch *mc)
return false;
}
-static int cf_check intel_compare(
+int cf_check intel_compare(
const struct microcode_patch *old, const struct microcode_patch *new)
{
/*
@@ -286,8 +260,8 @@ static int cf_check intel_compare(
return compare_revisions(old->rev, new->rev);
}
-static int cf_check apply_microcode(const struct microcode_patch *patch,
- unsigned int flags)
+int cf_check intel_apply_microcode(const struct microcode_patch *patch,
+ unsigned int flags)
{
uint64_t msr_content;
unsigned int cpu = smp_processor_id();
@@ -333,7 +307,7 @@ static int cf_check apply_microcode(const struct microcode_patch *patch,
return 0;
}
-static struct microcode_patch *cf_check cpu_request_microcode(
+struct microcode_patch *cf_check intel_cpu_request_microcode(
const void *buf, size_t size, bool make_copy)
{
int error = 0;
@@ -391,7 +365,7 @@ static struct microcode_patch *cf_check cpu_request_microcode(
return patch;
}
-static bool __init can_load_microcode(void)
+bool __init intel_can_load_microcode(void)
{
uint64_t mcu_ctrl;
@@ -404,21 +378,5 @@ static bool __init can_load_microcode(void)
return !(mcu_ctrl & MCU_CONTROL_DIS_MCU_LOAD);
}
-static const char __initconst intel_cpio_path[] =
+const char __initconst intel_cpio_path[] =
"kernel/x86/microcode/GenuineIntel.bin";
-
-static const struct microcode_ops __initconst_cf_clobber intel_ucode_ops = {
- .cpu_request_microcode = cpu_request_microcode,
- .collect_cpu_info = collect_cpu_info,
- .apply_microcode = apply_microcode,
- .compare = intel_compare,
- .cpio_path = intel_cpio_path,
-};
-
-void __init ucode_probe_intel(struct microcode_ops *ops)
-{
- *ops = intel_ucode_ops;
-
- if ( !can_load_microcode() )
- ops->apply_microcode = NULL;
-}
diff --git a/xen/arch/x86/cpu/microcode/intel.h b/xen/arch/x86/cpu/microcode/intel.h
new file mode 100644
index 0000000000..3c1419dc77
--- /dev/null
+++ b/xen/arch/x86/cpu/microcode/intel.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef ASM_X86_MICROCODE_INTEL_H
+#define ASM_X86_MICROCODE_INTEL_H
+
+#include "private.h"
+
+bool intel_can_load_microcode(void);
+int cf_check intel_compare(const struct microcode_patch *old,
+ const struct microcode_patch *new);
+int cf_check intel_apply_microcode(const struct microcode_patch *patch,
+ unsigned int flags);
+struct microcode_patch *cf_check intel_cpu_request_microcode(
+ const void *buf, size_t size, bool make_copy);
+extern const char intel_cpio_path[];
+
+#endif /* ASM_X86_MICROCODE_INTEL_H */
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/4] x86: Split out early_microcode_load() and microcode_load_one()
2025-11-12 16:22 [PATCH 0/4] Add Kconfig option to remove microcode loading support Alejandro Vallejo
2025-11-12 16:22 ` [PATCH 1/4] x86: Split out AMD-specific code to be executed without ucode loading Alejandro Vallejo
2025-11-12 16:22 ` [PATCH 2/4] x86: Split out Intel-specific " Alejandro Vallejo
@ 2025-11-12 16:22 ` Alejandro Vallejo
2025-11-12 16:22 ` [PATCH 4/4] x86: Add Kconfig option to disable microcode loading Alejandro Vallejo
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Alejandro Vallejo @ 2025-11-12 16:22 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné
A later patch compiles out most of the microcode loading code by removing
core.c from the Makefile based on Kconfig. These functions are important
to set up the ucode_op to read the microcode revision and report it on
every CPU.
Not a functional change.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/cpu/microcode/Makefile | 1 +
xen/arch/x86/cpu/microcode/base.c | 72 ++++++++++++++++++++++++++++
xen/arch/x86/cpu/microcode/core.c | 57 +---------------------
xen/arch/x86/cpu/microcode/private.h | 14 ++++++
4 files changed, 89 insertions(+), 55 deletions(-)
create mode 100644 xen/arch/x86/cpu/microcode/base.c
diff --git a/xen/arch/x86/cpu/microcode/Makefile b/xen/arch/x86/cpu/microcode/Makefile
index 74289981e3..765195ada3 100644
--- a/xen/arch/x86/cpu/microcode/Makefile
+++ b/xen/arch/x86/cpu/microcode/Makefile
@@ -1,5 +1,6 @@
obj-$(CONFIG_AMD) += amd.o
obj-$(CONFIG_AMD) += amd-base.o
+obj-y += base.o
obj-y += core.o
obj-$(CONFIG_INTEL) += intel.o
obj-$(CONFIG_INTEL) += intel-base.o
diff --git a/xen/arch/x86/cpu/microcode/base.c b/xen/arch/x86/cpu/microcode/base.c
new file mode 100644
index 0000000000..895ee78d2e
--- /dev/null
+++ b/xen/arch/x86/cpu/microcode/base.c
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <xen/alternative-call.h>
+
+#include <xen/errno.h>
+#include <xen/init.h>
+#include <xen/lib.h>
+
+#include <asm/asm_defns.h>
+#include <asm/cpufeature.h>
+#include <asm/x86-vendors.h>
+#include <asm/microcode.h>
+
+#include "private.h"
+
+struct microcode_ops __ro_after_init ucode_ops;
+
+int microcode_update_one(void)
+{
+ /*
+ * This path is used for APs and S3 resume. Read the microcode revision
+ * if possible, even if we can't load microcode.
+ */
+ if ( ucode_ops.collect_cpu_info )
+ alternative_vcall(ucode_ops.collect_cpu_info);
+
+ return _microcode_update_one();
+}
+
+int __init early_microcode_init(struct boot_info *bi)
+{
+ const struct cpuinfo_x86 *c = &boot_cpu_data;
+
+ switch ( c->vendor )
+ {
+ case X86_VENDOR_AMD:
+ ucode_probe_amd(&ucode_ops);
+ break;
+
+ case X86_VENDOR_INTEL:
+ ucode_probe_intel(&ucode_ops);
+ break;
+ }
+
+ if ( !ucode_ops.collect_cpu_info )
+ {
+ printk(XENLOG_INFO "Microcode loading not available\n");
+ return -ENODEV;
+ }
+
+ ucode_ops.collect_cpu_info();
+
+ printk(XENLOG_INFO "BSP microcode revision: 0x%08x\n", this_cpu(cpu_sig).rev);
+
+ /*
+ * Some hypervisors deliberately report a microcode revision of -1 to
+ * mean that they will not accept microcode updates.
+ *
+ * It's also possible the hardware might have built-in support to disable
+ * updates and someone (e.g: a baremetal cloud provider) disabled them.
+ *
+ * Take the hint in either case and ignore the microcode interface.
+ */
+ if ( !ucode_ops.apply_microcode || this_cpu(cpu_sig).rev == ~0 )
+ {
+ printk(XENLOG_INFO "Microcode loading disabled due to: %s\n",
+ ucode_ops.apply_microcode ? "rev = ~0" : "HW toggle");
+ ucode_ops.apply_microcode = NULL;
+ return -ENODEV;
+ }
+
+ return early_microcode_load(bi);
+}
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 1d1a5aa4b0..553a0ced15 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -162,8 +162,6 @@ static int __init cf_check parse_ucode(const char *s)
}
custom_param("ucode", parse_ucode);
-static struct microcode_ops __ro_after_init ucode_ops;
-
static DEFINE_SPINLOCK(microcode_mutex);
DEFINE_PER_CPU(struct cpu_signature, cpu_sig);
@@ -648,7 +646,7 @@ int ucode_update_hcall(XEN_GUEST_HANDLE(const_void) buf,
}
/* Load a cached update to current cpu */
-int microcode_update_one(void)
+int _microcode_update_one(void)
{
int rc;
@@ -736,13 +734,7 @@ static int __init cf_check microcode_init_cache(void)
}
presmp_initcall(microcode_init_cache);
-/*
- * There are several tasks:
- * - Locate the ucode blob in the boot modules.
- * - Parse and attempt in-place load.
- * - Inform microcode_init_cache() of how to find the blob again.
- */
-static int __init early_microcode_load(struct boot_info *bi)
+int __init early_microcode_load(struct boot_info *bi)
{
void *data = NULL;
size_t size;
@@ -873,48 +865,3 @@ static int __init early_microcode_load(struct boot_info *bi)
return rc;
}
-
-int __init early_microcode_init(struct boot_info *bi)
-{
- const struct cpuinfo_x86 *c = &boot_cpu_data;
-
- switch ( c->vendor )
- {
- case X86_VENDOR_AMD:
- ucode_probe_amd(&ucode_ops);
- break;
-
- case X86_VENDOR_INTEL:
- ucode_probe_intel(&ucode_ops);
- break;
- }
-
- if ( !ucode_ops.collect_cpu_info )
- {
- printk(XENLOG_INFO "Microcode loading not available\n");
- return -ENODEV;
- }
-
- ucode_ops.collect_cpu_info();
-
- printk(XENLOG_INFO "BSP microcode revision: 0x%08x\n", this_cpu(cpu_sig).rev);
-
- /*
- * Some hypervisors deliberately report a microcode revision of -1 to
- * mean that they will not accept microcode updates.
- *
- * It's also possible the hardware might have built-in support to disable
- * updates and someone (e.g: a baremetal cloud provider) disabled them.
- *
- * Take the hint in either case and ignore the microcode interface.
- */
- if ( !ucode_ops.apply_microcode || this_cpu(cpu_sig).rev == ~0 )
- {
- printk(XENLOG_INFO "Microcode loading disabled due to: %s\n",
- ucode_ops.apply_microcode ? "rev = ~0" : "HW toggle");
- ucode_ops.apply_microcode = NULL;
- return -ENODEV;
- }
-
- return early_microcode_load(bi);
-}
diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h
index e6c965dc99..881ea7d8d9 100644
--- a/xen/arch/x86/cpu/microcode/private.h
+++ b/xen/arch/x86/cpu/microcode/private.h
@@ -5,6 +5,8 @@
#include <asm/microcode.h>
+struct boot_info;
+
/* Opaque. Internals are vendor-specific. */
struct microcode_patch;
@@ -68,6 +70,7 @@ struct microcode_ops {
};
extern bool opt_digest_check;
+extern struct microcode_ops ucode_ops;
/*
* Microcode loading falls into one of 3 states.
@@ -93,4 +96,15 @@ void ucode_probe_intel(struct microcode_ops *ops);
static inline void ucode_probe_intel(struct microcode_ops *ops) {}
#endif
+/*
+ * There are several tasks:
+ * - Locate the ucode blob in the boot modules.
+ * - Parse and attempt in-place load.
+ * - Inform microcode_init_cache() of how to find the blob again.
+ */
+int early_microcode_load(struct boot_info *bi);
+
+/* Attempt performaing a microcode load */
+int _microcode_update_one(void);
+
#endif /* ASM_X86_MICROCODE_PRIVATE_H */
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/4] x86: Add Kconfig option to disable microcode loading
2025-11-12 16:22 [PATCH 0/4] Add Kconfig option to remove microcode loading support Alejandro Vallejo
` (2 preceding siblings ...)
2025-11-12 16:22 ` [PATCH 3/4] x86: Split out early_microcode_load() and microcode_load_one() Alejandro Vallejo
@ 2025-11-12 16:22 ` Alejandro Vallejo
2025-11-13 7:36 ` [PATCH 0/4] Add Kconfig option to remove microcode loading support Jan Beulich
2025-11-13 8:50 ` Andrew Cooper
5 siblings, 0 replies; 12+ messages in thread
From: Alejandro Vallejo @ 2025-11-12 16:22 UTC (permalink / raw)
To: xen-devel
Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Daniel P. Smith,
Marek Marczykowski-Górecki
Keeps around the microcode revision reading logic, as that's security
sensitive to detect out-of-date patforms and report them.
Move cpu_sig to base.c, because that's externally visible symbol outside
the microcode subsystem and we need it always accesible.
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
xen/arch/x86/Kconfig | 12 ++++++++++++
xen/arch/x86/cpu/microcode/Makefile | 6 +++---
xen/arch/x86/cpu/microcode/amd-base.c | 9 +++++++--
xen/arch/x86/cpu/microcode/base.c | 21 +++++++++++----------
xen/arch/x86/cpu/microcode/core.c | 1 -
xen/arch/x86/cpu/microcode/intel-base.c | 6 ++++--
xen/arch/x86/efi/efi-boot.h | 2 +-
xen/arch/x86/platform_hypercall.c | 2 ++
8 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 3f0f3a0f3a..948dd00dbc 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -330,8 +330,20 @@ config REQUIRE_NX
was unavailable. However, if enabled, Xen will no longer boot on
any CPU which is lacking NX support.
+config UCODE
+ bool "Microcode loading"
+ default y
+ help
+ Support updating the microcode revision of available CPUs with a newer
+ vendor-provided microcode blob. Microcode updates address some classes of
+ silicon defects. It's a very common delivery mechanism for fixes or
+ workarounds for speculative execution vulnerabilities.
+
+ If unsure, say Y
+
config UCODE_SCAN_DEFAULT
bool "Scan for microcode by default"
+ depends on UCODE
help
During boot, Xen can scan the multiboot images for a CPIO archive
containing CPU microcode to be loaded, which is Linux's mechanism for
diff --git a/xen/arch/x86/cpu/microcode/Makefile b/xen/arch/x86/cpu/microcode/Makefile
index 765195ada3..4ec38b56a2 100644
--- a/xen/arch/x86/cpu/microcode/Makefile
+++ b/xen/arch/x86/cpu/microcode/Makefile
@@ -1,6 +1,6 @@
-obj-$(CONFIG_AMD) += amd.o
+obj-$(filter $(CONFIG_AMD),$(CONFIG_UCODE)) += amd.o
obj-$(CONFIG_AMD) += amd-base.o
obj-y += base.o
-obj-y += core.o
-obj-$(CONFIG_INTEL) += intel.o
+obj-$(CONFIG_UCODE) += core.o
+obj-$(filter $(CONFIG_INTEL),$(CONFIG_UCODE)) += intel.o
obj-$(CONFIG_INTEL) += intel-base.o
diff --git a/xen/arch/x86/cpu/microcode/amd-base.c b/xen/arch/x86/cpu/microcode/amd-base.c
index f8f5fac1e1..4e705fe602 100644
--- a/xen/arch/x86/cpu/microcode/amd-base.c
+++ b/xen/arch/x86/cpu/microcode/amd-base.c
@@ -22,19 +22,23 @@ static void cf_check collect_cpu_info(void)
}
static const struct microcode_ops __initconst_cf_clobber amd_ucode_ops = {
- .cpu_request_microcode = amd_cpu_request_microcode,
.collect_cpu_info = collect_cpu_info,
+#ifdef CONFIG_UCODE
+ .cpu_request_microcode = amd_cpu_request_microcode,
.apply_microcode = amd_apply_microcode,
.compare = amd_compare,
.cpio_path = amd_cpio_path,
+#endif /* CONFIG_UCODE */
};
void __init ucode_probe_amd(struct microcode_ops *ops)
{
/*
* The Entrysign vulnerability (SB-7033, CVE-2024-36347) affects Zen1-5
- * CPUs. Taint Xen if digest checking is turned off.
+ * CPUs. Taint Xen if digest checking is turned off and microcode loading is
+ * compiled in.
*/
+#ifdef CONFIG_UCODE
if ( boot_cpu_data.family >= 0x17 && boot_cpu_data.family <= 0x1a &&
!opt_digest_check )
{
@@ -42,6 +46,7 @@ void __init ucode_probe_amd(struct microcode_ops *ops)
"Microcode patch additional digest checks disabled\n");
add_taint(TAINT_CPU_OUT_OF_SPEC);
}
+#endif /* CONFIG_UCODE */
if ( boot_cpu_data.family < 0x10 )
return;
diff --git a/xen/arch/x86/cpu/microcode/base.c b/xen/arch/x86/cpu/microcode/base.c
index 895ee78d2e..3e0b5a7447 100644
--- a/xen/arch/x86/cpu/microcode/base.c
+++ b/xen/arch/x86/cpu/microcode/base.c
@@ -13,6 +13,7 @@
#include "private.h"
struct microcode_ops __ro_after_init ucode_ops;
+DEFINE_PER_CPU(struct cpu_signature, cpu_sig);
int microcode_update_one(void)
{
@@ -23,6 +24,9 @@ int microcode_update_one(void)
if ( ucode_ops.collect_cpu_info )
alternative_vcall(ucode_ops.collect_cpu_info);
+ if ( !IS_ENABLED(CONFIG_UCODE) )
+ return 0;
+
return _microcode_update_one();
}
@@ -30,16 +34,10 @@ int __init early_microcode_init(struct boot_info *bi)
{
const struct cpuinfo_x86 *c = &boot_cpu_data;
- switch ( c->vendor )
- {
- case X86_VENDOR_AMD:
+ if ( IS_ENABLED(CONFIG_AMD) && c->vendor == X86_VENDOR_AMD )
ucode_probe_amd(&ucode_ops);
- break;
-
- case X86_VENDOR_INTEL:
+ else if ( IS_ENABLED(CONFIG_INTEL) && c->vendor == X86_VENDOR_INTEL )
ucode_probe_intel(&ucode_ops);
- break;
- }
if ( !ucode_ops.collect_cpu_info )
{
@@ -60,10 +58,13 @@ int __init early_microcode_init(struct boot_info *bi)
*
* Take the hint in either case and ignore the microcode interface.
*/
- if ( !ucode_ops.apply_microcode || this_cpu(cpu_sig).rev == ~0 )
+ if ( !IS_ENABLED(CONFIG_UCODE) || !ucode_ops.apply_microcode ||
+ this_cpu(cpu_sig).rev == ~0 )
{
printk(XENLOG_INFO "Microcode loading disabled due to: %s\n",
- ucode_ops.apply_microcode ? "rev = ~0" : "HW toggle");
+ !IS_ENABLED(CONFIG_UCODE) ? "not compiled-in" :
+ ucode_ops.apply_microcode ? "rev = ~0" :
+ "HW toggle");
ucode_ops.apply_microcode = NULL;
return -ENODEV;
}
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 553a0ced15..d6ba250dca 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -164,7 +164,6 @@ custom_param("ucode", parse_ucode);
static DEFINE_SPINLOCK(microcode_mutex);
-DEFINE_PER_CPU(struct cpu_signature, cpu_sig);
/* Store error code of the work done in NMI handler */
static DEFINE_PER_CPU(int, loading_err);
diff --git a/xen/arch/x86/cpu/microcode/intel-base.c b/xen/arch/x86/cpu/microcode/intel-base.c
index 4fcacaa192..18fdb4e7fc 100644
--- a/xen/arch/x86/cpu/microcode/intel-base.c
+++ b/xen/arch/x86/cpu/microcode/intel-base.c
@@ -32,17 +32,19 @@ static void cf_check collect_cpu_info(void)
}
static const struct microcode_ops __initconst_cf_clobber intel_ucode_ops = {
+ .collect_cpu_info = collect_cpu_info,
+#ifdef CONFIG_UCODE
.cpu_request_microcode = intel_cpu_request_microcode,
.apply_microcode = intel_apply_microcode,
- .collect_cpu_info = collect_cpu_info,
.compare = intel_compare,
.cpio_path = intel_cpio_path,
+#endif /* CONFIG_UCODE */
};
void __init ucode_probe_intel(struct microcode_ops *ops)
{
*ops = intel_ucode_ops;
- if ( !intel_can_load_microcode() )
+ if ( IS_ENABLED(CONFIG_UCODE) && !intel_can_load_microcode() )
ops->apply_microcode = NULL;
}
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 0194720003..9ec9291681 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -295,7 +295,7 @@ static void __init efi_arch_cfg_file_late(const EFI_LOADED_IMAGE *image,
{
union string name;
- if ( read_section(image, L"ucode", &ucode, NULL) )
+ if ( !IS_ENABLED(CONFIG_UCODE) || read_section(image, L"ucode", &ucode, NULL) )
return;
name.s = get_value(&cfg, section, "ucode");
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 79bb99e0b6..b2527bca93 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -307,6 +307,7 @@ ret_t do_platform_op(
break;
}
+#ifdef CONFIG_UCODE
case XENPF_microcode_update:
{
XEN_GUEST_HANDLE(const_void) data;
@@ -327,6 +328,7 @@ ret_t do_platform_op(
op->u.microcode2.flags);
break;
}
+#endif /* CONFIG_UCODE */
case XENPF_platform_quirk:
{
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 0/4] Add Kconfig option to remove microcode loading support
2025-11-12 16:22 [PATCH 0/4] Add Kconfig option to remove microcode loading support Alejandro Vallejo
` (3 preceding siblings ...)
2025-11-12 16:22 ` [PATCH 4/4] x86: Add Kconfig option to disable microcode loading Alejandro Vallejo
@ 2025-11-13 7:36 ` Jan Beulich
2025-11-13 12:12 ` Alejandro Vallejo
2025-11-13 8:50 ` Andrew Cooper
5 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2025-11-13 7:36 UTC (permalink / raw)
To: Alejandro Vallejo
Cc: Andrew Cooper, Roger Pau Monné, Daniel P. Smith,
Marek Marczykowski-Górecki, xen-devel
On 12.11.2025 17:22, Alejandro Vallejo wrote:
> Hi,
>
> The series is mostly a refactor between everything needed to load microcode and
> the bare minimum to probe the current microcode revision.
>
> The Kconfig option keeps the reading of microcode rev data around, as it's very
> relevant for security and debuggability in order to deduce which erratas apply
> to the current platform.
>
> The idea is to move everything that must still be compiled with !CONFIG_UCODE
> onto {,amd-,intel-}base.c, then remove everything else conditionally at the
> Makefile level.
>
> Renaming files (e.g: s/base/core/ and s/core/common/) would better reflect
> post-series reality, but it'd be annoying for later backports in this general
> area.
>
> Cheers,
> Alejandro
>
> Alejandro Vallejo (4):
> x86: Split out AMD-specific code to be executed without ucode loading
> x86: Split out Intel-specific code to be executed without ucode
> loading
> x86: Split out early_microcode_load() and microcode_load_one()
> x86: Add Kconfig option to disable microcode loading
>
> xen/arch/x86/Kconfig | 12 ++++
> xen/arch/x86/cpu/microcode/Makefile | 9 ++-
> xen/arch/x86/cpu/microcode/amd-base.c | 55 +++++++++++++++++++
> xen/arch/x86/cpu/microcode/amd.c | 55 ++-----------------
> xen/arch/x86/cpu/microcode/amd.h | 15 +++++
> xen/arch/x86/cpu/microcode/base.c | 73 +++++++++++++++++++++++++
> xen/arch/x86/cpu/microcode/core.c | 58 +-------------------
> xen/arch/x86/cpu/microcode/intel-base.c | 50 +++++++++++++++++
> xen/arch/x86/cpu/microcode/intel.c | 56 +++----------------
> xen/arch/x86/cpu/microcode/intel.h | 16 ++++++
> xen/arch/x86/cpu/microcode/private.h | 14 +++++
> xen/arch/x86/efi/efi-boot.h | 2 +-
> xen/arch/x86/platform_hypercall.c | 2 +
> 13 files changed, 259 insertions(+), 158 deletions(-)
> create mode 100644 xen/arch/x86/cpu/microcode/amd-base.c
> create mode 100644 xen/arch/x86/cpu/microcode/amd.h
> create mode 100644 xen/arch/x86/cpu/microcode/base.c
> create mode 100644 xen/arch/x86/cpu/microcode/intel-base.c
> create mode 100644 xen/arch/x86/cpu/microcode/intel.h
Purely based on this diffstat: A doc update likely is necessary as well, as
the ucode= command line option now becomes only conditionally applicable (aiui,
i.e. without having looked at the patches them selves).
Jan
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/4] Add Kconfig option to remove microcode loading support
2025-11-13 7:36 ` [PATCH 0/4] Add Kconfig option to remove microcode loading support Jan Beulich
@ 2025-11-13 12:12 ` Alejandro Vallejo
0 siblings, 0 replies; 12+ messages in thread
From: Alejandro Vallejo @ 2025-11-13 12:12 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Roger Pau Monné, Daniel P. Smith,
Marek Marczykowski-Górecki, xen-devel
On Thu Nov 13, 2025 at 8:36 AM CET, Jan Beulich wrote:
> On 12.11.2025 17:22, Alejandro Vallejo wrote:
>> Hi,
>>
>> The series is mostly a refactor between everything needed to load microcode and
>> the bare minimum to probe the current microcode revision.
>>
>> The Kconfig option keeps the reading of microcode rev data around, as it's very
>> relevant for security and debuggability in order to deduce which erratas apply
>> to the current platform.
>>
>> The idea is to move everything that must still be compiled with !CONFIG_UCODE
>> onto {,amd-,intel-}base.c, then remove everything else conditionally at the
>> Makefile level.
>>
>> Renaming files (e.g: s/base/core/ and s/core/common/) would better reflect
>> post-series reality, but it'd be annoying for later backports in this general
>> area.
>>
>> Cheers,
>> Alejandro
>>
>> Alejandro Vallejo (4):
>> x86: Split out AMD-specific code to be executed without ucode loading
>> x86: Split out Intel-specific code to be executed without ucode
>> loading
>> x86: Split out early_microcode_load() and microcode_load_one()
>> x86: Add Kconfig option to disable microcode loading
>>
>> xen/arch/x86/Kconfig | 12 ++++
>> xen/arch/x86/cpu/microcode/Makefile | 9 ++-
>> xen/arch/x86/cpu/microcode/amd-base.c | 55 +++++++++++++++++++
>> xen/arch/x86/cpu/microcode/amd.c | 55 ++-----------------
>> xen/arch/x86/cpu/microcode/amd.h | 15 +++++
>> xen/arch/x86/cpu/microcode/base.c | 73 +++++++++++++++++++++++++
>> xen/arch/x86/cpu/microcode/core.c | 58 +-------------------
>> xen/arch/x86/cpu/microcode/intel-base.c | 50 +++++++++++++++++
>> xen/arch/x86/cpu/microcode/intel.c | 56 +++----------------
>> xen/arch/x86/cpu/microcode/intel.h | 16 ++++++
>> xen/arch/x86/cpu/microcode/private.h | 14 +++++
>> xen/arch/x86/efi/efi-boot.h | 2 +-
>> xen/arch/x86/platform_hypercall.c | 2 +
>> 13 files changed, 259 insertions(+), 158 deletions(-)
>> create mode 100644 xen/arch/x86/cpu/microcode/amd-base.c
>> create mode 100644 xen/arch/x86/cpu/microcode/amd.h
>> create mode 100644 xen/arch/x86/cpu/microcode/base.c
>> create mode 100644 xen/arch/x86/cpu/microcode/intel-base.c
>> create mode 100644 xen/arch/x86/cpu/microcode/intel.h
>
> Purely based on this diffstat: A doc update likely is necessary as well, as
> the ucode= command line option now becomes only conditionally applicable (aiui,
> i.e. without having looked at the patches them selves).
>
> Jan
Yes, that sounds sensible.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] Add Kconfig option to remove microcode loading support
2025-11-12 16:22 [PATCH 0/4] Add Kconfig option to remove microcode loading support Alejandro Vallejo
` (4 preceding siblings ...)
2025-11-13 7:36 ` [PATCH 0/4] Add Kconfig option to remove microcode loading support Jan Beulich
@ 2025-11-13 8:50 ` Andrew Cooper
2025-11-13 12:11 ` Alejandro Vallejo
2025-11-17 16:55 ` Jan Beulich
5 siblings, 2 replies; 12+ messages in thread
From: Andrew Cooper @ 2025-11-13 8:50 UTC (permalink / raw)
To: Alejandro Vallejo, xen-devel
Cc: Jan Beulich, Roger Pau Monné, Daniel P. Smith,
Marek Marczykowski-Górecki
On 12/11/2025 4:22 pm, Alejandro Vallejo wrote:
> xen/arch/x86/Kconfig | 12 ++++
> xen/arch/x86/cpu/microcode/Makefile | 9 ++-
> xen/arch/x86/cpu/microcode/amd-base.c | 55 +++++++++++++++++++
> xen/arch/x86/cpu/microcode/amd.c | 55 ++-----------------
> xen/arch/x86/cpu/microcode/amd.h | 15 +++++
> xen/arch/x86/cpu/microcode/base.c | 73 +++++++++++++++++++++++++
> xen/arch/x86/cpu/microcode/core.c | 58 +-------------------
> xen/arch/x86/cpu/microcode/intel-base.c | 50 +++++++++++++++++
> xen/arch/x86/cpu/microcode/intel.c | 56 +++----------------
> xen/arch/x86/cpu/microcode/intel.h | 16 ++++++
> xen/arch/x86/cpu/microcode/private.h | 14 +++++
> xen/arch/x86/efi/efi-boot.h | 2 +-
> xen/arch/x86/platform_hypercall.c | 2 +
> 13 files changed, 259 insertions(+), 158 deletions(-)
> create mode 100644 xen/arch/x86/cpu/microcode/amd-base.c
> create mode 100644 xen/arch/x86/cpu/microcode/amd.h
> create mode 100644 xen/arch/x86/cpu/microcode/base.c
> create mode 100644 xen/arch/x86/cpu/microcode/intel-base.c
> create mode 100644 xen/arch/x86/cpu/microcode/intel.h
This is awfully invasive for something that ultimately drops only a
handful of lines of code.
First, it should be CONFIG_MICROCODE_LOADING. We're not getting rid of
all microcode capabilities. Also, of all the places where UCODE needs
expanding properly, it's Kconfig.
Next, annotate the functions that you conditionally don't reference in
{amd,intel}_ucode_ops with __maybe_unused, and dead code elimination
should do the rest.
~Andrew
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/4] Add Kconfig option to remove microcode loading support
2025-11-13 8:50 ` Andrew Cooper
@ 2025-11-13 12:11 ` Alejandro Vallejo
2025-11-17 16:55 ` Jan Beulich
1 sibling, 0 replies; 12+ messages in thread
From: Alejandro Vallejo @ 2025-11-13 12:11 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: Jan Beulich, Roger Pau Monné, Daniel P. Smith,
Marek Marczykowski-Górecki
On Thu Nov 13, 2025 at 9:50 AM CET, Andrew Cooper wrote:
> On 12/11/2025 4:22 pm, Alejandro Vallejo wrote:
>> xen/arch/x86/Kconfig | 12 ++++
>> xen/arch/x86/cpu/microcode/Makefile | 9 ++-
>> xen/arch/x86/cpu/microcode/amd-base.c | 55 +++++++++++++++++++
>> xen/arch/x86/cpu/microcode/amd.c | 55 ++-----------------
>> xen/arch/x86/cpu/microcode/amd.h | 15 +++++
>> xen/arch/x86/cpu/microcode/base.c | 73 +++++++++++++++++++++++++
>> xen/arch/x86/cpu/microcode/core.c | 58 +-------------------
>> xen/arch/x86/cpu/microcode/intel-base.c | 50 +++++++++++++++++
>> xen/arch/x86/cpu/microcode/intel.c | 56 +++----------------
>> xen/arch/x86/cpu/microcode/intel.h | 16 ++++++
>> xen/arch/x86/cpu/microcode/private.h | 14 +++++
>> xen/arch/x86/efi/efi-boot.h | 2 +-
>> xen/arch/x86/platform_hypercall.c | 2 +
>> 13 files changed, 259 insertions(+), 158 deletions(-)
>> create mode 100644 xen/arch/x86/cpu/microcode/amd-base.c
>> create mode 100644 xen/arch/x86/cpu/microcode/amd.h
>> create mode 100644 xen/arch/x86/cpu/microcode/base.c
>> create mode 100644 xen/arch/x86/cpu/microcode/intel-base.c
>> create mode 100644 xen/arch/x86/cpu/microcode/intel.h
>
> This is awfully invasive for something that ultimately drops only a
> handful of lines of code.
Two handfuls when considering both AMD and Intel. But yes, I'd rather not modify
as much as this. However, for our purposes it's important to physically move out
the code we want to Kconfig out, as then coverage reports don't need exceptions.
>
> First, it should be CONFIG_MICROCODE_LOADING. [...]
Sure, I don't mind it being $FOO or $BAR.
> Next, annotate the functions that you conditionally don't reference in
> {amd,intel}_ucode_ops with __maybe_unused, and dead code elimination
> should do the rest.
I considered that, but local DCE poses problems for coverage tracking, which
we care about deeply. I'll check if there's some magic we can do to enable that
pattern but as of now, there isn't.
If I can't make it work I'll just take the sledgehammer and drop all ops and
dependent functions.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/4] Add Kconfig option to remove microcode loading support
2025-11-13 8:50 ` Andrew Cooper
2025-11-13 12:11 ` Alejandro Vallejo
@ 2025-11-17 16:55 ` Jan Beulich
2025-11-18 10:19 ` Alejandro Vallejo
1 sibling, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2025-11-17 16:55 UTC (permalink / raw)
To: Andrew Cooper
Cc: Roger Pau Monné, Daniel P. Smith,
Marek Marczykowski-Górecki, Alejandro Vallejo, xen-devel
On 13.11.2025 09:50, Andrew Cooper wrote:
> On 12/11/2025 4:22 pm, Alejandro Vallejo wrote:
>> xen/arch/x86/Kconfig | 12 ++++
>> xen/arch/x86/cpu/microcode/Makefile | 9 ++-
>> xen/arch/x86/cpu/microcode/amd-base.c | 55 +++++++++++++++++++
>> xen/arch/x86/cpu/microcode/amd.c | 55 ++-----------------
>> xen/arch/x86/cpu/microcode/amd.h | 15 +++++
>> xen/arch/x86/cpu/microcode/base.c | 73 +++++++++++++++++++++++++
>> xen/arch/x86/cpu/microcode/core.c | 58 +-------------------
>> xen/arch/x86/cpu/microcode/intel-base.c | 50 +++++++++++++++++
>> xen/arch/x86/cpu/microcode/intel.c | 56 +++----------------
>> xen/arch/x86/cpu/microcode/intel.h | 16 ++++++
>> xen/arch/x86/cpu/microcode/private.h | 14 +++++
>> xen/arch/x86/efi/efi-boot.h | 2 +-
>> xen/arch/x86/platform_hypercall.c | 2 +
>> 13 files changed, 259 insertions(+), 158 deletions(-)
>> create mode 100644 xen/arch/x86/cpu/microcode/amd-base.c
>> create mode 100644 xen/arch/x86/cpu/microcode/amd.h
>> create mode 100644 xen/arch/x86/cpu/microcode/base.c
>> create mode 100644 xen/arch/x86/cpu/microcode/intel-base.c
>> create mode 100644 xen/arch/x86/cpu/microcode/intel.h
>
> This is awfully invasive for something that ultimately drops only a
> handful of lines of code.
>
> First, it should be CONFIG_MICROCODE_LOADING. We're not getting rid of
> all microcode capabilities. Also, of all the places where UCODE needs
> expanding properly, it's Kconfig.
>
> Next, annotate the functions that you conditionally don't reference in
> {amd,intel}_ucode_ops with __maybe_unused, and dead code elimination
> should do the rest.
Are you, btw, sure this would be Misra-compliant? Right now we solely
deviate __maybe_unused when used on labels which may really be unused,
afaics.
Jan
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/4] Add Kconfig option to remove microcode loading support
2025-11-17 16:55 ` Jan Beulich
@ 2025-11-18 10:19 ` Alejandro Vallejo
2025-11-18 11:22 ` Andrew Cooper
0 siblings, 1 reply; 12+ messages in thread
From: Alejandro Vallejo @ 2025-11-18 10:19 UTC (permalink / raw)
To: Jan Beulich, Andrew Cooper
Cc: Roger Pau Monné, Daniel P. Smith,
Marek Marczykowski-Górecki, xen-devel
On Mon Nov 17, 2025 at 5:55 PM CET, Jan Beulich wrote:
> On 13.11.2025 09:50, Andrew Cooper wrote:
>> On 12/11/2025 4:22 pm, Alejandro Vallejo wrote:
>>> xen/arch/x86/Kconfig | 12 ++++
>>> xen/arch/x86/cpu/microcode/Makefile | 9 ++-
>>> xen/arch/x86/cpu/microcode/amd-base.c | 55 +++++++++++++++++++
>>> xen/arch/x86/cpu/microcode/amd.c | 55 ++-----------------
>>> xen/arch/x86/cpu/microcode/amd.h | 15 +++++
>>> xen/arch/x86/cpu/microcode/base.c | 73 +++++++++++++++++++++++++
>>> xen/arch/x86/cpu/microcode/core.c | 58 +-------------------
>>> xen/arch/x86/cpu/microcode/intel-base.c | 50 +++++++++++++++++
>>> xen/arch/x86/cpu/microcode/intel.c | 56 +++----------------
>>> xen/arch/x86/cpu/microcode/intel.h | 16 ++++++
>>> xen/arch/x86/cpu/microcode/private.h | 14 +++++
>>> xen/arch/x86/efi/efi-boot.h | 2 +-
>>> xen/arch/x86/platform_hypercall.c | 2 +
>>> 13 files changed, 259 insertions(+), 158 deletions(-)
>>> create mode 100644 xen/arch/x86/cpu/microcode/amd-base.c
>>> create mode 100644 xen/arch/x86/cpu/microcode/amd.h
>>> create mode 100644 xen/arch/x86/cpu/microcode/base.c
>>> create mode 100644 xen/arch/x86/cpu/microcode/intel-base.c
>>> create mode 100644 xen/arch/x86/cpu/microcode/intel.h
>>
>> This is awfully invasive for something that ultimately drops only a
>> handful of lines of code.
>>
>> First, it should be CONFIG_MICROCODE_LOADING. We're not getting rid of
>> all microcode capabilities. Also, of all the places where UCODE needs
>> expanding properly, it's Kconfig.
>>
>> Next, annotate the functions that you conditionally don't reference in
>> {amd,intel}_ucode_ops with __maybe_unused, and dead code elimination
>> should do the rest.
I've done a few tests to see how something along those lines would pan out for
our needs. Our coverage tool correctly ignores ellided functions, so I'll be
sending a v2 shortly.
>
> Are you, btw, sure this would be Misra-compliant? Right now we solely
> deviate __maybe_unused when used on labels which may really be unused,
> afaics.
>
> Jan
Rather than appending an unconditional __maybe_unused (that's, imo, a bad idea),
I'll be creating a local __ucode_loading attribute in private.h that maps to
__maybe_unused when CONFIG_MICROCODE_LOADING is not set and to nothing when it
is set.
However, I'm tentatively keeping the movement from core.c to base.c, as there's
just way too many functions with external linkage to ifdef. It'd be an utterly
confusing file otherwise.
Plus, I'll be conditionally getting rid of earlycpio.c too, which is something I
neglected to do in v1 even if it's only used for microcode loading.
Cheers,
Alejandro
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 0/4] Add Kconfig option to remove microcode loading support
2025-11-18 10:19 ` Alejandro Vallejo
@ 2025-11-18 11:22 ` Andrew Cooper
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2025-11-18 11:22 UTC (permalink / raw)
To: Alejandro Vallejo, Jan Beulich
Cc: Roger Pau Monné, Daniel P. Smith,
Marek Marczykowski-Górecki, xen-devel
On 18/11/2025 10:19 am, Alejandro Vallejo wrote:
> On Mon Nov 17, 2025 at 5:55 PM CET, Jan Beulich wrote:
>> On 13.11.2025 09:50, Andrew Cooper wrote:
>>> On 12/11/2025 4:22 pm, Alejandro Vallejo wrote:
>>>> xen/arch/x86/Kconfig | 12 ++++
>>>> xen/arch/x86/cpu/microcode/Makefile | 9 ++-
>>>> xen/arch/x86/cpu/microcode/amd-base.c | 55 +++++++++++++++++++
>>>> xen/arch/x86/cpu/microcode/amd.c | 55 ++-----------------
>>>> xen/arch/x86/cpu/microcode/amd.h | 15 +++++
>>>> xen/arch/x86/cpu/microcode/base.c | 73 +++++++++++++++++++++++++
>>>> xen/arch/x86/cpu/microcode/core.c | 58 +-------------------
>>>> xen/arch/x86/cpu/microcode/intel-base.c | 50 +++++++++++++++++
>>>> xen/arch/x86/cpu/microcode/intel.c | 56 +++----------------
>>>> xen/arch/x86/cpu/microcode/intel.h | 16 ++++++
>>>> xen/arch/x86/cpu/microcode/private.h | 14 +++++
>>>> xen/arch/x86/efi/efi-boot.h | 2 +-
>>>> xen/arch/x86/platform_hypercall.c | 2 +
>>>> 13 files changed, 259 insertions(+), 158 deletions(-)
>>>> create mode 100644 xen/arch/x86/cpu/microcode/amd-base.c
>>>> create mode 100644 xen/arch/x86/cpu/microcode/amd.h
>>>> create mode 100644 xen/arch/x86/cpu/microcode/base.c
>>>> create mode 100644 xen/arch/x86/cpu/microcode/intel-base.c
>>>> create mode 100644 xen/arch/x86/cpu/microcode/intel.h
>>> This is awfully invasive for something that ultimately drops only a
>>> handful of lines of code.
>>>
>>> First, it should be CONFIG_MICROCODE_LOADING. We're not getting rid of
>>> all microcode capabilities. Also, of all the places where UCODE needs
>>> expanding properly, it's Kconfig.
>>>
>>> Next, annotate the functions that you conditionally don't reference in
>>> {amd,intel}_ucode_ops with __maybe_unused, and dead code elimination
>>> should do the rest.
> I've done a few tests to see how something along those lines would pan out for
> our needs. Our coverage tool correctly ignores ellided functions, so I'll be
> sending a v2 shortly.
>
>> Are you, btw, sure this would be Misra-compliant? Right now we solely
>> deviate __maybe_unused when used on labels which may really be unused,
>> afaics.
>>
>> Jan
> Rather than appending an unconditional __maybe_unused (that's, imo, a bad idea),
> I'll be creating a local __ucode_loading attribute in private.h that maps to
> __maybe_unused when CONFIG_MICROCODE_LOADING is not set and to nothing when it
> is set.
__maybe_unused literally exists for this purpose. See it's comment.
Wrapping in another condition just adds complexity for no gain. This
case is unlike livepatch_or_$FOO because it's not choice between two
different things.
>
> However, I'm tentatively keeping the movement from core.c to base.c, as there's
> just way too many functions with external linkage to ifdef. It'd be an utterly
> confusing file otherwise.
There are 4 functions with external linkage, only one of which you can
fully elide.
(I think) you can do everything you need here with 4 IS_ENABLED()'s (two
in do_platform_op(), one in early_microcode_init() and
microcode_update_one()), one ifdef around ucode_update_hcall(), and one
__maybe_unused for ucode_update_hcall().
I'm going to save you some time, and insist that core.c is not split;
I'm not willing to take that kind of disruption into logic this
complicated. The result is far nicer not split than split.
>
> Plus, I'll be conditionally getting rid of earlycpio.c too, which is something I
> neglected to do in v1 even if it's only used for microcode loading.
This is fine, but cpio should be lib-ified like sha2 so it's simply
dropped if not referenced.
In fact, the one ifdef mentioned above could be dropped if someone were
to get --gc-sections working, but I suppose that is a task for a
different day.
~Andrew
^ permalink raw reply [flat|nested] 12+ messages in thread