[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 3/3] x86: Support booting under Secure Startup via SKINIT
On Tue, Jan 19, 2021 at 05:23:25PM +0000, Andrew Cooper wrote: > On 19/01/2021 15:48, Roger Pau Monné wrote: > > On Fri, Jan 15, 2021 at 11:10:46PM +0000, Andrew Cooper wrote: > >> From: Norbert Kamiński <norbert.kaminski@xxxxxxxxx> > >> > >> For now, this is simply enough logic to let Xen come up after the > >> bootloader > >> has executed an SKINIT instruction to begin a Secure Startup. > > Since I know very little about this, I might as well ask. Reading the > > PM, SKINIT requires a payload, which is an image to boot. That image > > must be <= 64KB and needs a special header. > > > > In case of booting Xen using SKINIT, what would be that payload? > > (called SLB in the PM). > > The SK/SLB is implemented by https://github.com/TrenchBoot/landing-zone/ > which does all the low level platform stuff. It is the logical > equivalent of the Intel-provided TXT ACM which is a blob as far as the > world is concerned. > > From a "system boot" perspective, the plan is something like this: > > * Grub puts xen/kernel/initrd in memory > * A final stanza line of "secure_launch" changes the exit of grub to be > a DRTM DLE (Dynamic Launch Event). > ** For Intel TXT, this is the SENTER instruction, provided that the > firmware loaded the TXT ACM properly > ** For AMD/Hygon, this is additionally loading landing-zone into memory, > and using the SKINIT instruction. > * TXT blob, or Landing Zone, do low level things. > * They enter xen/Linux/other via a common protocol. > > With this patch series in place, Xen's Multiboot2 entrypoint works just > fine as far as "invoke the next thing" goes. Linux has a > work-in-progress extension to their zero-page protocol. > > >> During a Secure Startup, the BSP operates with the GIF clear (blocks all > >> external interrupts, even SMI/NMI), and INIT_REDIRECTION active (converts > >> INIT > >> IPIs to #SX exceptions, if e.g. the platform needs to scrub secrets before > >> resetting). To afford APs the same Secure Startup protections as the BSP, > >> the > >> INIT IPI must be skipped, and SIPI must be the first interrupt seen. > >> > >> Full details are available in AMD APM Vol2 15.27 "Secure Startup with > >> SKINIT" > >> > >> Introduce skinit_enable_intr() and call it from cpu_init(), next to the > >> enable_nmis() which performs a related function for tboot startups. > >> > >> Also introduce ap_boot_method to control the sequence of actions for AP > >> boot. > >> > >> Signed-off-by: Marek Kasiewicz <marek.kasiewicz@xxxxxxxxx> > >> Signed-off-by: Norbert Kamiński <norbert.kaminski@xxxxxxxxx> > >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > >> --- > >> CC: Jan Beulich <JBeulich@xxxxxxxx> > >> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx> > >> CC: Wei Liu <wl@xxxxxxx> > >> CC: Marek Kasiewicz <marek.kasiewicz@xxxxxxxxx> > >> CC: Norbert Kamiński <norbert.kaminski@xxxxxxxxx> > >> CC: Michal Zygowski <michal.zygowski@xxxxxxxxx> > >> CC: Piotr Krol <piotr.krol@xxxxxxxx> > >> CC: Krystian Hebel <krystian.hebel@xxxxxxxxx> > >> CC: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx> > >> CC: Rich Persaud <persaur@xxxxxxxxx> > >> CC: Christopher Clark <christopher.w.clark@xxxxxxxxx> > >> --- > >> xen/arch/x86/cpu/common.c | 32 ++++++++++++++++++++++++++++++++ > >> xen/arch/x86/smpboot.c | 12 +++++++++++- > >> xen/include/asm-x86/cpufeature.h | 1 + > >> xen/include/asm-x86/msr-index.h | 1 + > >> xen/include/asm-x86/processor.h | 6 ++++++ > >> 5 files changed, 51 insertions(+), 1 deletion(-) > >> > >> diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c > >> index a684519a20..d9a103e721 100644 > >> --- a/xen/arch/x86/cpu/common.c > >> +++ b/xen/arch/x86/cpu/common.c > >> @@ -834,6 +834,29 @@ void load_system_tables(void) > >> BUG_ON(system_state != SYS_STATE_early_boot && (stack_bottom & 0xf)); > >> } > >> > >> +static void skinit_enable_intr(void) > > Since this is AFAICT AMD specific code, shouldn't it better be in > > cpu/amd.c instead of cpu/common.c? > > Hygon will get sad... > > It's deliberately not in AMD-specific code. Hygon already calls into AMD specific functions in amd.c (early_init_amd, amd_log_freq), so it wouldn't seem that weird to place it in amd.c and share with other AMD derivatives. Likely not worth arguing about. > >> +{ > >> + uint64_t val; > >> + > >> + /* > >> + * If the platform is performing a Secure Launch via SKINIT > >> + * INIT_REDIRECTION flag will be active. > >> + */ > >> + if ( !cpu_has_skinit || rdmsr_safe(MSR_K8_VM_CR, val) || > >> + !(val & VM_CR_INIT_REDIRECTION) ) > > I would add some kind of check here to assert that APs are started in > > the correct state, ie: > > > > BUG_ON(ap_boot_method == AP_BOOT_SKINIT); > > At the moment, it really doesn't matter. This change is to simply let > Xen boot. > > Later changes which do the TPM and attestation work is going to need to > know details like this, but it will be elsewhere on boot, and one > recovery option is going to be "please just boot and let be get back > into the system", in which case a crosscheck is not wanted. I still think printing a message might be helpful to know the AP has been started in an unexpected state. > >> + > >> + /* > >> + * We don't yet handle #SX. Disable INIT_REDIRECTION first, before > >> + * enabling GIF, so a pending INIT resets us, rather than causing a > >> + * panic due to an unknown exception. > >> + */ > >> + wrmsr_safe(MSR_K8_VM_CR, val & ~VM_CR_INIT_REDIRECTION); > >> + asm volatile ( ".byte 0x0f,0x01,0xdc" /* STGI */ ::: "memory" ); > > We already have one of those in smv/entry.S, so I would rather not > > open-code the opcodes here again, but I'm not unsure whether there's a > > suitable place for those. > > I deliberately didn't. SGTI is not something we should have a helper > for - it's not safe for general use. Oh, I wasn't thinking of a helper, but rather a define for the open-coded .byte directive (ie: like the one in svm/entry.S) that's shared between both users. No big deal anyway. Thanks, Roger.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |