| Message ID | 46d35ce06d84c55ff02a05610ca3fb6d51ee1a71.1605232743.git.isaku.yamahata@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: X86: TDX support | expand |
On Mon, Nov 16, 2020 at 10:25:48AM -0800, isaku.yamahata@intel.com wrote: > From: Zhang Chen <chen.zhang@intel.com> > > Move get_builtin_firmware() to common.c so that it can be used to get > non-ucode firmware, e.g. Intel's SEAM modules, even if MICROCODE=n. What for? This is used for microcode built in the kernel - a non-common use case. Why is your thing built into the kernel and not a normal module object?
On Wed, Nov 25, 2020, Borislav Petkov wrote: > On Mon, Nov 16, 2020 at 10:25:48AM -0800, isaku.yamahata@intel.com wrote: > > From: Zhang Chen <chen.zhang@intel.com> > > > > Move get_builtin_firmware() to common.c so that it can be used to get > > non-ucode firmware, e.g. Intel's SEAM modules, even if MICROCODE=n. > > What for? > > This is used for microcode built in the kernel - a non-common use case. > Why is your thing built into the kernel and not a normal module object? The SEAM module needs to be loaded during early boot, it can't be deferred to a module, at least not without a lot more blood, sweat, and tears. The SEAM Loader is an ACM that is invoked via GETSEC[EnterACCS], which requires all APs to be in WFS. SEAM Loader also returns control to the kernel with a null IDT and NMIs unblocked, i.e. we're toast if there's a pending NMI. And unlike the run-time SEAMCALLs, boot-time SEAMCALLs do not have a strictly bounded runtime. Invoking configuration SEAMCALLs after the kernel is fully up and running could cause instability as IRQ, NMI, and SMI are all blocked in SEAM mode, e.g. a high priority IRQ/NMI/SMI could be blocked for 50+ usecs (it might be far more than 50 usecs, I haven't seen real numbers for all SEAMCALLs).
On Thu, Nov 26, 2020 at 12:18:12AM +0000, Sean Christopherson wrote: > The SEAM module needs to be loaded during early boot, it can't be > deferred to a module, at least not without a lot more blood, sweat, > and tears. Are you also planning to support builtin seam or only thru initrd loading? In any case, this commit message needs to state intentions not me having to plow all the way up to patch 62. > The SEAM Loader is an ACM that is invoked via GETSEC[EnterACCS], which > requires all APs to be in WFS. Yah, this is the other thing that sprang at me from looking at that "small" patchset briefly - I'm being killed by abbreviations. Whoever is sending the next version, pls put a proper documentation as patch 1 of the patchset just like the CET patchset does. > SEAM Loader also returns control to the kernel with a null IDT and > NMIs unblocked, i.e. we're toast if there's a pending NMI. And unlike > the run-time SEAMCALLs, boot-time SEAMCALLs do not have a strictly > bounded runtime. Invoking configuration SEAMCALLs after the kernel is > fully up and running could cause instability as IRQ, NMI, and SMI are > all blocked in SEAM mode, e.g. a high priority IRQ/NMI/SMI could be > blocked for 50+ usecs (it might be far more than 50 usecs, I haven't > seen real numbers for all SEAMCALLs). Yah, I sure hope people have taken an ample amount of time to think about all the implications of this thing because it sounds wonky to me. amluto certainly has already gone deeper and will surely comment. :) Thx.
On Thu, Nov 26, 2020, Borislav Petkov wrote: > On Thu, Nov 26, 2020 at 12:18:12AM +0000, Sean Christopherson wrote: > > The SEAM module needs to be loaded during early boot, it can't be > > deferred to a module, at least not without a lot more blood, sweat, > > and tears. > > Are you also planning to support builtin seam or only thru initrd loading? Yep, both built-in and initrd are supported. > In any case, this commit message needs to state intentions not me having > to plow all the way up to patch 62. Yeah, most of the changelogs are in terrible shape. This first RFC was intended to get feedback on high level things like code organization and the KVM shenanigans for VMX and TDX coexistence.
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h index da78ccbd493b..0096ac7cad0a 100644 --- a/arch/x86/include/asm/cpu.h +++ b/arch/x86/include/asm/cpu.h @@ -7,6 +7,7 @@ #include <linux/topology.h> #include <linux/nodemask.h> #include <linux/percpu.h> +#include <linux/earlycpio.h> #ifdef CONFIG_SMP @@ -37,6 +38,10 @@ extern int _debug_hotplug_cpu(int cpu, int action); int mwait_usable(const struct cpuinfo_x86 *); +#if defined(CONFIG_MICROCODE) || defined(CONFIG_KVM_INTEL_TDX) +bool get_builtin_firmware(struct cpio_data *cd, const char *name); +#endif + unsigned int x86_family(unsigned int sig); unsigned int x86_model(unsigned int sig); unsigned int x86_stepping(unsigned int sig); diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h index 2b7cc5397f80..4f10089f30de 100644 --- a/arch/x86/include/asm/microcode.h +++ b/arch/x86/include/asm/microcode.h @@ -131,15 +131,12 @@ int __init microcode_init(void); extern void __init load_ucode_bsp(void); extern void load_ucode_ap(void); void reload_early_microcode(void); -extern bool get_builtin_firmware(struct cpio_data *cd, const char *name); extern bool initrd_gone; #else static inline int __init microcode_init(void) { return 0; }; static inline void __init load_ucode_bsp(void) { } static inline void load_ucode_ap(void) { } static inline void reload_early_microcode(void) { } -static inline bool -get_builtin_firmware(struct cpio_data *cd, const char *name) { return false; } #endif #endif /* _ASM_X86_MICROCODE_H */ diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 35ad8480c464..87512c5854bb 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -22,6 +22,7 @@ #include <linux/io.h> #include <linux/syscore_ops.h> #include <linux/pgtable.h> +#include <linux/firmware.h> #include <asm/cmdline.h> #include <asm/stackprotector.h> @@ -87,6 +88,25 @@ void __init setup_cpu_local_masks(void) alloc_bootmem_cpumask_var(&cpu_sibling_setup_mask); } +#if defined(CONFIG_MICROCODE) || defined(CONFIG_KVM_INTEL_TDX) +extern struct builtin_fw __start_builtin_fw[]; +extern struct builtin_fw __end_builtin_fw[]; + +bool get_builtin_firmware(struct cpio_data *cd, const char *name) +{ + struct builtin_fw *b_fw; + + for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++) { + if (!strcmp(name, b_fw->name)) { + cd->size = b_fw->size; + cd->data = b_fw->data; + return true; + } + } + return false; +} +#endif + static void default_init(struct cpuinfo_x86 *c) { #ifdef CONFIG_X86_64 diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index ec6f0415bc6d..f877a9c19f42 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -22,7 +22,6 @@ #include <linux/syscore_ops.h> #include <linux/miscdevice.h> #include <linux/capability.h> -#include <linux/firmware.h> #include <linux/kernel.h> #include <linux/delay.h> #include <linux/mutex.h> @@ -140,23 +139,6 @@ static bool __init check_loader_disabled_bsp(void) return *res; } -extern struct builtin_fw __start_builtin_fw[]; -extern struct builtin_fw __end_builtin_fw[]; - -bool get_builtin_firmware(struct cpio_data *cd, const char *name) -{ - struct builtin_fw *b_fw; - - for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++) { - if (!strcmp(name, b_fw->name)) { - cd->size = b_fw->size; - cd->data = b_fw->data; - return true; - } - } - return false; -} - void __init load_ucode_bsp(void) { unsigned int cpuid_1_eax; diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 6a99535d7f37..50e69d6cb3d9 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -36,6 +36,7 @@ #include <asm/tlbflush.h> #include <asm/setup.h> #include <asm/msr.h> +#include <asm/cpu.h> static const char ucode_path[] = "kernel/x86/microcode/GenuineIntel.bin";