Hi, One more day, one more revision. Same as v2 with feedback addressed. v1: https://lore.kernel.org/xen-devel/20251112162219.226075-1-alejandro.garciavallejo@amd.com/ v2: https://lore.kernel.org/xen-devel/20260112150259.74535-1-alejandro.garciavallejo@amd.com/ pipeline (green): https://gitlab.com/xen-project/people/agvallejo/xen/-/pipelines/2260019646 Cheers, Alejandro Alejandro Vallejo (4): x86/ucode: Fix typo s/mitigiated/mitigated/ x86/ucode: Rename UCODE_SCAN_DEFAULT to MICROCODE_SCAN_DEFAULT earlycpio: lib-ify earlycpio.c x86/ucode: Add Kconfig option to remove microcode loading automation/gitlab-ci/build.yaml | 2 +- docs/admin-guide/microcode-loading.rst | 2 ++ docs/misc/efi.pandoc | 2 ++ docs/misc/xen-command-line.pandoc | 4 ++-- docs/misra/exclude-list.json | 8 ++++---- xen/arch/x86/Kconfig | 16 +++++++++++++++- xen/arch/x86/cpu/microcode/amd.c | 22 ++++++++++++---------- xen/arch/x86/cpu/microcode/core.c | 17 +++++++++++++---- xen/arch/x86/cpu/microcode/intel.c | 11 +++++++---- xen/arch/x86/cpu/microcode/private.h | 2 ++ xen/arch/x86/efi/efi-boot.h | 3 ++- xen/arch/x86/platform_hypercall.c | 22 +++++++++++++++------- xen/common/Makefile | 2 +- xen/lib/Makefile | 1 + xen/{common => lib}/earlycpio.c | 0 15 files changed, 79 insertions(+), 35 deletions(-) rename xen/{common => lib}/earlycpio.c (100%) base-commit: a2a34d76643e49ccc949296c9a45888034e50b55 -- 2.43.0 Not a functional change. Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> --- v3: * No changes --- xen/arch/x86/cpu/microcode/amd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/cpu/microcode/amd.c +++ b/xen/arch/x86/cpu/microcode/amd.c @@ -XXX,XX +XXX,XX @@ static const struct patch_digest { } patch_digests[] = { #include "amd-patch-digests.c" }; -static bool __ro_after_init entrysign_mitigiated_in_firmware; +static bool __ro_after_init entrysign_mitigated_in_firmware; static int cf_check cmp_patch_id(const void *key, const void *elem) { @@ -XXX,XX +XXX,XX @@ static bool check_digest(const struct container_microcode *mc) * the digest of the patch against a list of known provenance. */ if ( boot_cpu_data.family < 0x17 || boot_cpu_data.family > 0x1a || - entrysign_mitigiated_in_firmware || !opt_digest_check ) + entrysign_mitigated_in_firmware || !opt_digest_check ) return true; pd = bsearch(&patch->patch_id, patch_digests, ARRAY_SIZE(patch_digests), @@ -XXX,XX +XXX,XX @@ void __init amd_check_entrysign(void) */ if ( (uint8_t)curr_rev >= fixed_rev ) { - entrysign_mitigiated_in_firmware = true; + entrysign_mitigated_in_firmware = true; return; } -- 2.43.0 We'd rather have the full spelling in Kconfig. Adjusts every other reference to the name too. Not a functional change. Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- v3: * New patch. Previously integrated in larger patch. --- automation/gitlab-ci/build.yaml | 2 +- docs/misc/xen-command-line.pandoc | 2 +- xen/arch/x86/Kconfig | 2 +- xen/arch/x86/cpu/microcode/core.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml index XXXXXXX..XXXXXXX 100644 --- a/automation/gitlab-ci/build.yaml +++ b/automation/gitlab-ci/build.yaml @@ -XXX,XX +XXX,XX @@ alpine-3.18-gcc-debug: CONFIG_ARGO=y CONFIG_UBSAN=y CONFIG_UBSAN_FATAL=y - CONFIG_UCODE_SCAN_DEFAULT=y + CONFIG_MICROCODE_SCAN_DEFAULT=y CONFIG_XHCI=y debian-13-x86_64-gcc-debug: diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index XXXXXXX..XXXXXXX 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -XXX,XX +XXX,XX @@ microcode in the cpio name space must be: - on AMD : kernel/x86/microcode/AuthenticAMD.bin When using xen.efi, the `ucode=<filename>` config file setting takes precedence over `scan`. The default value for `scan` is set with -`CONFIG_UCODE_SCAN_DEFAULT`. +`CONFIG_MICROCODE_SCAN_DEFAULT`. 'nmi' determines late loading is performed in NMI handler or just in stop_machine context. In NMI handler, even NMIs are blocked, which is diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -XXX,XX +XXX,XX @@ config REQUIRE_NX was unavailable. However, if enabled, Xen will no longer boot on any CPU which is lacking NX support. -config UCODE_SCAN_DEFAULT +config MICROCODE_SCAN_DEFAULT bool "Scan for microcode by default" help During boot, Xen can scan the multiboot images for a CPIO archive diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/cpu/microcode/core.c +++ b/xen/arch/x86/cpu/microcode/core.c @@ -XXX,XX +XXX,XX @@ static struct microcode_patch *microcode_cache; * location we require that they are not both active together. */ static int __initdata opt_mod_idx; -static bool __initdata opt_scan = IS_ENABLED(CONFIG_UCODE_SCAN_DEFAULT); +static bool __initdata opt_scan = IS_ENABLED(CONFIG_MICROCODE_SCAN_DEFAULT); bool __ro_after_init opt_digest_check = true; /* -- 2.43.0 It's only used for microcode loading on x86. By lib-ifying it we can make it go away automatically when microcode loading becomes an optional feature in follow-up patches. Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- v3: * New patch. Subsumes earlier conditionalisation of earlycpio.c on CONFIG_MICROCODE_LOADING. --- docs/misra/exclude-list.json | 8 ++++---- xen/common/Makefile | 2 +- xen/lib/Makefile | 1 + xen/{common => lib}/earlycpio.c | 0 4 files changed, 6 insertions(+), 5 deletions(-) rename xen/{common => lib}/earlycpio.c (100%) diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json index XXXXXXX..XXXXXXX 100644 --- a/docs/misra/exclude-list.json +++ b/docs/misra/exclude-list.json @@ -XXX,XX +XXX,XX @@ "rel_path": "common/bunzip2.c", "comment": "Imported from Linux, ignore for now" }, - { - "rel_path": "common/earlycpio.c", - "comment": "Imported from Linux, ignore for now" - }, { "rel_path": "common/gzip/*", "comment": "Imported from Linux, ignore for now" @@ -XXX,XX +XXX,XX @@ "rel_path": "include/xen/decompress.h", "comment": "Imported from Linux, ignore for now" }, + { + "rel_path": "lib/earlycpio.c", + "comment": "Imported from Linux, ignore for now" + }, { "rel_path": "lib/find-next-bit.c", "comment": "Imported from Linux, ignore for now" diff --git a/xen/common/Makefile b/xen/common/Makefile index XXXXXXX..XXXXXXX 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -XXX,XX +XXX,XX @@ obj-y += wait.o obj-bin-y += warning.init.o obj-y += xmalloc_tlsf.o -obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma lzo unlzo unlz4 unzstd earlycpio,$(n).init.o) +obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma lzo unlzo unlz4 unzstd,$(n).init.o) obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o memory.o multicall.o xlat.o) diff --git a/xen/lib/Makefile b/xen/lib/Makefile index XXXXXXX..XXXXXXX 100644 --- a/xen/lib/Makefile +++ b/xen/lib/Makefile @@ -XXX,XX +XXX,XX @@ obj-$(CONFIG_X86) += x86/ lib-y += bsearch.o lib-y += ctors.o lib-y += ctype.o +lib-y += earlycpio.o lib-y += find-next-bit.o lib-y += generic-ffsl.o lib-y += generic-flsl.o diff --git a/xen/common/earlycpio.c b/xen/lib/earlycpio.c similarity index 100% rename from xen/common/earlycpio.c rename to xen/lib/earlycpio.c -- 2.43.0 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 remove all statics no longer used when microcode loading is disabled without tagging them with __maybe_unused. Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- v3: * Moved MICROCODE_SCAN_DEFAULT to a separate patch * Added doc change to admin-guide * Added Andrew's suggestion for Kconfig help text * Inlined can_apply_microcode() macro in core.c * Kept the platform-ops, gating them on CONFIG_MICROCODE_LOADING through the new MICROCODE_OP() macro. --- docs/admin-guide/microcode-loading.rst | 2 ++ docs/misc/efi.pandoc | 2 ++ docs/misc/xen-command-line.pandoc | 2 +- xen/arch/x86/Kconfig | 14 ++++++++++++++ xen/arch/x86/cpu/microcode/amd.c | 16 +++++++++------- xen/arch/x86/cpu/microcode/core.c | 15 ++++++++++++--- xen/arch/x86/cpu/microcode/intel.c | 11 +++++++---- xen/arch/x86/cpu/microcode/private.h | 2 ++ xen/arch/x86/efi/efi-boot.h | 3 ++- xen/arch/x86/platform_hypercall.c | 22 +++++++++++++++------- 10 files changed, 66 insertions(+), 23 deletions(-) diff --git a/docs/admin-guide/microcode-loading.rst b/docs/admin-guide/microcode-loading.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/admin-guide/microcode-loading.rst +++ b/docs/admin-guide/microcode-loading.rst @@ -XXX,XX +XXX,XX @@ TSX errata which necessitated disabling the feature entirely), or the addition of brand new features (e.g. the Spectre v2 controls to work around speculative execution vulnerabilities). +Microcode loading support in Xen is controlled by the +``CONFIG_MICROCODE_LOADING`` Kconfig option. Boot time microcode loading --------------------------- diff --git a/docs/misc/efi.pandoc b/docs/misc/efi.pandoc index XXXXXXX..XXXXXXX 100644 --- a/docs/misc/efi.pandoc +++ b/docs/misc/efi.pandoc @@ -XXX,XX +XXX,XX @@ 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. + ###`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 XXXXXXX..XXXXXXX 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -XXX,XX +XXX,XX @@ 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` Controls for CPU microcode loading. For early loading, this parameter can diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -XXX,XX +XXX,XX @@ config REQUIRE_NX was unavailable. However, if enabled, Xen will no longer boot on any CPU which is lacking NX support. +config MICROCODE_LOADING + bool "Microcode loading" + default y + help + Microcode updates for CPUs fix errata and provide new functionality for + software to work around bugs, such as the speculative execution + vulnerabilities. It is common for OSes to carry updated microcode as + software tends to get updated more frequently than firmware. + + For embedded environments where a full firmware/software stack is being + provided, it might not be necessary for Xen to need to load microcode + itself. + config MICROCODE_SCAN_DEFAULT bool "Scan for microcode by default" + depends on MICROCODE_LOADING 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/amd.c b/xen/arch/x86/cpu/microcode/amd.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/cpu/microcode/amd.c +++ b/xen/arch/x86/cpu/microcode/amd.c @@ -XXX,XX +XXX,XX @@ 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), }; void __init ucode_probe_amd(struct microcode_ops *ops) @@ -XXX,XX +XXX,XX @@ 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 && + if ( IS_ENABLED(CONFIG_MICROCODE_LOADING) && + boot_cpu_data.family >= 0x17 && boot_cpu_data.family <= 0x1a && !opt_digest_check ) { printk(XENLOG_WARNING @@ -XXX,XX +XXX,XX @@ void __init amd_check_entrysign(void) unsigned int curr_rev; uint8_t fixed_rev; - if ( boot_cpu_data.vendor != X86_VENDOR_AMD || - boot_cpu_data.family < 0x17 || + if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) || + boot_cpu_data.vendor != X86_VENDOR_AMD || + boot_cpu_data.family < 0x17 || boot_cpu_data.family > 0x1a ) return; diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/cpu/microcode/core.c +++ b/xen/arch/x86/cpu/microcode/core.c @@ -XXX,XX +XXX,XX @@ */ #define MICROCODE_UPDATE_TIMEOUT_US 1000000 -static bool __initdata ucode_mod_forced; +static bool __initdata __maybe_unused ucode_mod_forced; static unsigned int nr_cores; /* @@ -XXX,XX +XXX,XX @@ static int __initdata opt_mod_idx; static bool __initdata opt_scan = IS_ENABLED(CONFIG_MICROCODE_SCAN_DEFAULT); bool __ro_after_init opt_digest_check = true; +#ifdef CONFIG_MICROCODE_LOADING /* * Used by the EFI path only, when xen.cfg identifies an explicit microcode * file. Overrides ucode=<int>|scan on the regular command line. @@ -XXX,XX +XXX,XX @@ static int __init cf_check parse_ucode(const char *s) return rc; } custom_param("ucode", parse_ucode); +#endif /* CONFIG_MICROCODE_LOADING */ static struct microcode_ops __ro_after_init ucode_ops; @@ -XXX,XX +XXX,XX @@ struct ucode_buf { char buffer[]; }; -static long cf_check ucode_update_hcall_cont(void *data) +static long cf_check __maybe_unused ucode_update_hcall_cont(void *data) { struct microcode_patch *patch = NULL; int ret, result; @@ -XXX,XX +XXX,XX @@ static long cf_check ucode_update_hcall_cont(void *data) return ret; } +#ifdef CONFIG_MICROCODE_LOADING int ucode_update_hcall(XEN_GUEST_HANDLE(const_void) buf, unsigned long len, unsigned int flags) { @@ -XXX,XX +XXX,XX @@ int ucode_update_hcall(XEN_GUEST_HANDLE(const_void) buf, */ return continue_hypercall_on_cpu(0, ucode_update_hcall_cont, buffer); } +#endif /* CONFIG_MICROCODE_LOADING */ /* Load a cached update to current cpu */ int microcode_update_one(void) @@ -XXX,XX +XXX,XX @@ int microcode_update_one(void) if ( ucode_ops.collect_cpu_info ) alternative_vcall(ucode_ops.collect_cpu_info); - if ( !ucode_ops.apply_microcode ) + if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) || !ucode_ops.apply_microcode ) return -EOPNOTSUPP; spin_lock(&microcode_mutex); @@ -XXX,XX +XXX,XX @@ int microcode_update_one(void) */ static int __initdata early_mod_idx = -1; +#ifdef CONFIG_MICROCODE_LOADING static int __init cf_check microcode_init_cache(void) { struct boot_info *bi = &xen_boot_info; @@ -XXX,XX +XXX,XX @@ static int __init cf_check microcode_init_cache(void) return rc; } presmp_initcall(microcode_init_cache); +#endif /* CONFIG_MICROCODE_LOADING */ /* * There are several tasks: @@ -XXX,XX +XXX,XX @@ int __init early_microcode_init(struct boot_info *bi) printk(XENLOG_INFO "BSP microcode revision: 0x%08x\n", this_cpu(cpu_sig).rev); + if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) ) + return -ENODEV; + /* * Some hypervisors deliberately report a microcode revision of -1 to * mean that they will not accept microcode updates. diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/cpu/microcode/intel.c +++ b/xen/arch/x86/cpu/microcode/intel.c @@ -XXX,XX +XXX,XX @@ static 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, + .cpu_request_microcode = MICROCODE_OP(cpu_request_microcode), .collect_cpu_info = collect_cpu_info, - .apply_microcode = apply_microcode, - .compare = intel_compare, - .cpio_path = intel_cpio_path, + .apply_microcode = MICROCODE_OP(apply_microcode), + .compare = MICROCODE_OP(intel_compare), + .cpio_path = MICROCODE_OP(intel_cpio_path), }; void __init ucode_probe_intel(struct microcode_ops *ops) { *ops = intel_ucode_ops; + if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) ) + return; + if ( !can_load_microcode() ) ops->apply_microcode = NULL; } diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/cpu/microcode/private.h +++ b/xen/arch/x86/cpu/microcode/private.h @@ -XXX,XX +XXX,XX @@ 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) + #endif /* ASM_X86_MICROCODE_PRIVATE_H */ diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -XXX,XX +XXX,XX @@ 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_MICROCODE_LOADING) || + 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/platform_hypercall.c +++ b/xen/arch/x86/platform_hypercall.c @@ -XXX,XX +XXX,XX @@ 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; } -- 2.43.0 Hi, v1: https://lore.kernel.org/xen-devel/20251112162219.226075-1-alejandro.garciavallejo@amd.com/ v2: https://lore.kernel.org/xen-devel/20260112150259.74535-1-alejandro.garciavallejo@amd.com/ v3: https://lore.kernel.org/xen-devel/20260113122109.62399-1-alejandro.garciavallejo@amd.com/ pipeline (red, but see below): https://gitlab.com/xen-project/people/agvallejo/xen/-/pipelines/2272127547 Pipeline failures are on ARM builds, so clearly unrelated to this. Also has an eclair run on allcode that shows no new failures in earlycpio.c. The only dependency here is patch 2 going in before patch 3. Everything else can be freely rearranged. Cheers, Alejandro Alejandro Vallejo (5): x86/ucode: Add Kconfig option to remove microcode loading xen: Allow lib-y targets to also be .init.o earlycpio: lib-ify earlycpio.c docs/misra: Remove earlycpio.c from the Eclair exclusion list. automation: Disable ucode loading on AMD's analysis run automation/gitlab-ci/analyze.yaml | 1 + docs/admin-guide/microcode-loading.rst | 2 ++ docs/misc/efi.pandoc | 2 ++ docs/misc/xen-command-line.pandoc | 7 ++++--- docs/misra/exclude-list.json | 4 ---- xen/Rules.mk | 10 +++++----- xen/arch/x86/Kconfig | 14 ++++++++++++++ xen/arch/x86/cpu/microcode/amd.c | 16 +++++++++------- xen/arch/x86/cpu/microcode/core.c | 15 ++++++++++++--- xen/arch/x86/cpu/microcode/intel.c | 11 +++++++---- xen/arch/x86/cpu/microcode/private.h | 3 +++ xen/arch/x86/efi/efi-boot.h | 3 ++- xen/arch/x86/platform_hypercall.c | 10 ++++++++-- xen/common/Makefile | 2 +- xen/lib/Makefile | 1 + xen/{common => lib}/earlycpio.c | 0 16 files changed, 71 insertions(+), 30 deletions(-) rename xen/{common => lib}/earlycpio.c (100%) base-commit: 7b3e1b4e848d34c9a5b6634009959a7b9dd42104 -- 2.43.0 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 rather than ifdef we allow DCE to remove all statics no longer used when microcode loading is disabled without tagging them with __maybe_unused. Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> --- v4: * Minor spell fix in commit message ("rather") * efi.pandoc: terminate sentence with "enabled". * xen-command-line: State applicability as first line. * reorder cpu hooks. * MICROCODE_OP() comment * MICROCODE_OP() moved next to microcode_ops * Re-arranged platform_ops to turn them into smaller hunks. --- docs/admin-guide/microcode-loading.rst | 2 ++ docs/misc/efi.pandoc | 2 ++ docs/misc/xen-command-line.pandoc | 7 ++++--- xen/arch/x86/Kconfig | 14 ++++++++++++++ xen/arch/x86/cpu/microcode/amd.c | 16 +++++++++------- xen/arch/x86/cpu/microcode/core.c | 15 ++++++++++++--- xen/arch/x86/cpu/microcode/intel.c | 11 +++++++---- xen/arch/x86/cpu/microcode/private.h | 3 +++ xen/arch/x86/efi/efi-boot.h | 3 ++- xen/arch/x86/platform_hypercall.c | 10 ++++++++-- 10 files changed, 63 insertions(+), 20 deletions(-) diff --git a/docs/admin-guide/microcode-loading.rst b/docs/admin-guide/microcode-loading.rst index XXXXXXX..XXXXXXX 100644 --- a/docs/admin-guide/microcode-loading.rst +++ b/docs/admin-guide/microcode-loading.rst @@ -XXX,XX +XXX,XX @@ TSX errata which necessitated disabling the feature entirely), or the addition of brand new features (e.g. the Spectre v2 controls to work around speculative execution vulnerabilities). +Microcode loading support in Xen is controlled by the +``CONFIG_MICROCODE_LOADING`` Kconfig option. Boot time microcode loading --------------------------- diff --git a/docs/misc/efi.pandoc b/docs/misc/efi.pandoc index XXXXXXX..XXXXXXX 100644 --- a/docs/misc/efi.pandoc +++ b/docs/misc/efi.pandoc @@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -XXX,XX +XXX,XX @@ 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 diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/Kconfig +++ b/xen/arch/x86/Kconfig @@ -XXX,XX +XXX,XX @@ config REQUIRE_NX was unavailable. However, if enabled, Xen will no longer boot on any CPU which is lacking NX support. +config MICROCODE_LOADING + bool "Microcode loading" + default y + help + Microcode updates for CPUs fix errata and provide new functionality for + software to work around bugs, such as the speculative execution + vulnerabilities. It is common for OSes to carry updated microcode as + software tends to get updated more frequently than firmware. + + For embedded environments where a full firmware/software stack is being + provided, it might not be necessary for Xen to need to load microcode + itself. + config MICROCODE_SCAN_DEFAULT bool "Scan for microcode by default" + depends on MICROCODE_LOADING 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/amd.c b/xen/arch/x86/cpu/microcode/amd.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/cpu/microcode/amd.c +++ b/xen/arch/x86/cpu/microcode/amd.c @@ -XXX,XX +XXX,XX @@ 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, .collect_cpu_info = collect_cpu_info, - .apply_microcode = apply_microcode, - .compare = amd_compare, - .cpio_path = amd_cpio_path, + .cpu_request_microcode = MICROCODE_OP(cpu_request_microcode), + .apply_microcode = MICROCODE_OP(apply_microcode), + .compare = MICROCODE_OP(amd_compare), + .cpio_path = MICROCODE_OP(amd_cpio_path), }; void __init ucode_probe_amd(struct microcode_ops *ops) @@ -XXX,XX +XXX,XX @@ 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 && + if ( IS_ENABLED(CONFIG_MICROCODE_LOADING) && + boot_cpu_data.family >= 0x17 && boot_cpu_data.family <= 0x1a && !opt_digest_check ) { printk(XENLOG_WARNING @@ -XXX,XX +XXX,XX @@ void __init amd_check_entrysign(void) unsigned int curr_rev; uint8_t fixed_rev; - if ( boot_cpu_data.vendor != X86_VENDOR_AMD || - boot_cpu_data.family < 0x17 || + if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) || + boot_cpu_data.vendor != X86_VENDOR_AMD || + boot_cpu_data.family < 0x17 || boot_cpu_data.family > 0x1a ) return; diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/cpu/microcode/core.c +++ b/xen/arch/x86/cpu/microcode/core.c @@ -XXX,XX +XXX,XX @@ */ #define MICROCODE_UPDATE_TIMEOUT_US 1000000 -static bool __initdata ucode_mod_forced; +static bool __initdata __maybe_unused ucode_mod_forced; static unsigned int nr_cores; /* @@ -XXX,XX +XXX,XX @@ static int __initdata opt_mod_idx; static bool __initdata opt_scan = IS_ENABLED(CONFIG_MICROCODE_SCAN_DEFAULT); bool __ro_after_init opt_digest_check = true; +#ifdef CONFIG_MICROCODE_LOADING /* * Used by the EFI path only, when xen.cfg identifies an explicit microcode * file. Overrides ucode=<int>|scan on the regular command line. @@ -XXX,XX +XXX,XX @@ static int __init cf_check parse_ucode(const char *s) return rc; } custom_param("ucode", parse_ucode); +#endif /* CONFIG_MICROCODE_LOADING */ static struct microcode_ops __ro_after_init ucode_ops; @@ -XXX,XX +XXX,XX @@ struct ucode_buf { char buffer[]; }; -static long cf_check ucode_update_hcall_cont(void *data) +static long cf_check __maybe_unused ucode_update_hcall_cont(void *data) { struct microcode_patch *patch = NULL; int ret, result; @@ -XXX,XX +XXX,XX @@ static long cf_check ucode_update_hcall_cont(void *data) return ret; } +#ifdef CONFIG_MICROCODE_LOADING int ucode_update_hcall(XEN_GUEST_HANDLE(const_void) buf, unsigned long len, unsigned int flags) { @@ -XXX,XX +XXX,XX @@ int ucode_update_hcall(XEN_GUEST_HANDLE(const_void) buf, */ return continue_hypercall_on_cpu(0, ucode_update_hcall_cont, buffer); } +#endif /* CONFIG_MICROCODE_LOADING */ /* Load a cached update to current cpu */ int microcode_update_one(void) @@ -XXX,XX +XXX,XX @@ int microcode_update_one(void) if ( ucode_ops.collect_cpu_info ) alternative_vcall(ucode_ops.collect_cpu_info); - if ( !ucode_ops.apply_microcode ) + if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) || !ucode_ops.apply_microcode ) return -EOPNOTSUPP; spin_lock(&microcode_mutex); @@ -XXX,XX +XXX,XX @@ int microcode_update_one(void) */ static int __initdata early_mod_idx = -1; +#ifdef CONFIG_MICROCODE_LOADING static int __init cf_check microcode_init_cache(void) { struct boot_info *bi = &xen_boot_info; @@ -XXX,XX +XXX,XX @@ static int __init cf_check microcode_init_cache(void) return rc; } presmp_initcall(microcode_init_cache); +#endif /* CONFIG_MICROCODE_LOADING */ /* * There are several tasks: @@ -XXX,XX +XXX,XX @@ int __init early_microcode_init(struct boot_info *bi) printk(XENLOG_INFO "BSP microcode revision: 0x%08x\n", this_cpu(cpu_sig).rev); + if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) ) + return -ENODEV; + /* * Some hypervisors deliberately report a microcode revision of -1 to * mean that they will not accept microcode updates. diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/cpu/microcode/intel.c +++ b/xen/arch/x86/cpu/microcode/intel.c @@ -XXX,XX +XXX,XX @@ static 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, + .cpu_request_microcode = MICROCODE_OP(cpu_request_microcode), + .apply_microcode = MICROCODE_OP(apply_microcode), + .compare = MICROCODE_OP(intel_compare), + .cpio_path = MICROCODE_OP(intel_cpio_path), }; void __init ucode_probe_intel(struct microcode_ops *ops) { *ops = intel_ucode_ops; + if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) ) + return; + if ( !can_load_microcode() ) ops->apply_microcode = NULL; } diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/cpu/microcode/private.h +++ b/xen/arch/x86/cpu/microcode/private.h @@ -XXX,XX +XXX,XX @@ /* Opaque. Internals are vendor-specific. */ struct microcode_patch; +/* Aids dead-code elimination of the static hooks */ +#define MICROCODE_OP(x) (IS_ENABLED(CONFIG_MICROCODE_LOADING) ? (x) : NULL) + struct microcode_ops { /* * Parse a microcode container. Format is vendor-specific. diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -XXX,XX +XXX,XX @@ 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_MICROCODE_LOADING) || + 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 XXXXXXX..XXXXXXX 100644 --- a/xen/arch/x86/platform_hypercall.c +++ b/xen/arch/x86/platform_hypercall.c @@ -XXX,XX +XXX,XX @@ ret_t do_platform_op( { XEN_GUEST_HANDLE(const_void) data; - guest_from_compat_handle(data, op->u.microcode.data); + ret = -EOPNOTSUPP; + if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) ) + break; + guest_from_compat_handle(data, op->u.microcode.data); ret = ucode_update_hcall(data, op->u.microcode.length, 0); break; } @@ -XXX,XX +XXX,XX @@ ret_t do_platform_op( { XEN_GUEST_HANDLE(const_void) data; - guest_from_compat_handle(data, op->u.microcode2.data); + ret = -EOPNOTSUPP; + if ( !IS_ENABLED(CONFIG_MICROCODE_LOADING) ) + break; + guest_from_compat_handle(data, op->u.microcode2.data); ret = ucode_update_hcall(data, op->u.microcode2.length, op->u.microcode2.flags); break; -- 2.43.0 There's some assumptions as to which targets may be init-only. But there's little reason to preclude libraries from being init-only. Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- xen/Rules.mk | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/xen/Rules.mk b/xen/Rules.mk index XXXXXXX..XXXXXXX 100644 --- a/xen/Rules.mk +++ b/xen/Rules.mk @@ -XXX,XX +XXX,XX @@ endif targets += $(targets-for-builtin) -$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): CFLAGS-y += -DINIT_SECTIONS_ONLY +$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y) $(lib-y)): CFLAGS-y += -DINIT_SECTIONS_ONLY -non-init-objects = $(filter-out %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)) +non-init-objects = $(filter-out %.init.o, $(obj-y) $(obj-bin-y) $(extra-y) $(lib-y)) ifeq ($(CONFIG_CC_IS_CLANG),y) cov-cflags-$(CONFIG_COVERAGE) := -fprofile-instr-generate -fcoverage-mapping @@ -XXX,XX +XXX,XX @@ endif $(call cc-option-add,cov-cflags-$(CONFIG_COVERAGE),CC,-fprofile-update=atomic) # Reset cov-cflags-y in cases where an objects has another one as prerequisite -$(nocov-y) $(filter %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)): \ +$(nocov-y) $(filter %.init.o, $(obj-y) $(obj-bin-y) $(extra-y) $(lib-y)): \ cov-cflags-y := $(non-init-objects): _c_flags += $(cov-cflags-y) @@ -XXX,XX +XXX,XX @@ ifeq ($(CONFIG_UBSAN),y) UBSAN_FLAGS := $(filter-out -fno-%,$(CFLAGS_UBSAN)) $(filter -fno-%,$(CFLAGS_UBSAN)) # Reset UBSAN_FLAGS in cases where an objects has another one as prerequisite -$(noubsan-y) $(filter %.init.o, $(obj-y) $(obj-bin-y) $(extra-y)): \ +$(noubsan-y) $(filter %.init.o, $(obj-y) $(obj-bin-y) $(extra-y) $(lib-y)): \ UBSAN_FLAGS := $(non-init-objects): _c_flags += $(UBSAN_FLAGS) @@ -XXX,XX +XXX,XX @@ define cmd_obj_init_o $(OBJCOPY) $(foreach s,$(SPECIAL_DATA_SECTIONS),--rename-section .$(s)=.init.$(s)) $< $@ endef -$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y)): $(obj)/%.init.o: $(obj)/%.o FORCE +$(filter %.init.o,$(obj-y) $(obj-bin-y) $(extra-y) $(lib-y)): $(obj)/%.init.o: $(obj)/%.o FORCE $(call if_changed,obj_init_o) quiet_cmd_cpp_i_c = CPP $@ -- 2.43.0 It's only used for microcode loading on x86. By lib-ifying it we can make it go away automatically when microcode loading becomes an optional feature in follow-up patches. Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- v4: * Makes the lib-y target init-only --- xen/common/Makefile | 2 +- xen/lib/Makefile | 1 + xen/{common => lib}/earlycpio.c | 0 3 files changed, 2 insertions(+), 1 deletion(-) rename xen/{common => lib}/earlycpio.c (100%) diff --git a/xen/common/Makefile b/xen/common/Makefile index XXXXXXX..XXXXXXX 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -XXX,XX +XXX,XX @@ obj-y += wait.o obj-bin-y += warning.init.o obj-y += xmalloc_tlsf.o -obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma lzo unlzo unlz4 unzstd earlycpio,$(n).init.o) +obj-bin-$(CONFIG_X86) += $(foreach n,decompress bunzip2 unxz unlzma lzo unlzo unlz4 unzstd,$(n).init.o) obj-$(CONFIG_COMPAT) += $(addprefix compat/,domain.o memory.o multicall.o xlat.o) diff --git a/xen/lib/Makefile b/xen/lib/Makefile index XXXXXXX..XXXXXXX 100644 --- a/xen/lib/Makefile +++ b/xen/lib/Makefile @@ -XXX,XX +XXX,XX @@ obj-$(CONFIG_X86) += x86/ lib-y += bsearch.o lib-y += ctors.o lib-y += ctype.o +lib-y += earlycpio.init.o lib-y += find-next-bit.o lib-y += generic-ffsl.o lib-y += generic-flsl.o diff --git a/xen/common/earlycpio.c b/xen/lib/earlycpio.c similarity index 100% rename from xen/common/earlycpio.c rename to xen/lib/earlycpio.c -- 2.43.0 It's clean. Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- docs/misra/exclude-list.json | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docs/misra/exclude-list.json b/docs/misra/exclude-list.json index XXXXXXX..XXXXXXX 100644 --- a/docs/misra/exclude-list.json +++ b/docs/misra/exclude-list.json @@ -XXX,XX +XXX,XX @@ "rel_path": "common/bunzip2.c", "comment": "Imported from Linux, ignore for now" }, - { - "rel_path": "common/earlycpio.c", - "comment": "Imported from Linux, ignore for now" - }, { "rel_path": "common/gzip/*", "comment": "Imported from Linux, ignore for now" -- 2.43.0 Explicitly set CONFIG_MICROCODE_LOADING=n Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com> --- automation/gitlab-ci/analyze.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/automation/gitlab-ci/analyze.yaml b/automation/gitlab-ci/analyze.yaml index XXXXXXX..XXXXXXX 100644 --- a/automation/gitlab-ci/analyze.yaml +++ b/automation/gitlab-ci/analyze.yaml @@ -XXX,XX +XXX,XX @@ eclair-x86_64-amd: CONFIG_DEBUG_LOCKS=n CONFIG_SCRUB_DEBUG=n CONFIG_XMEM_POOL_POISON=n + CONFIG_MICROCODE_LOADING=n rules: - if: $ECLAIR_SAFETY when: always -- 2.43.0