On 20/01/2021 09:19, Jan Beulich wrote: > On 16.01.2021 00:10, Andrew Cooper wrote: >> --- 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) >> +{ >> + 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) ) >> + return; >> + >> + ap_boot_method = AP_BOOT_SKINIT; >> + >> + /* >> + * 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); > Why wrmsr_safe() without checking its return value? If the write > faults, we're hosed anyway, aren't we, so we may as well crash on > the offending WRMSR rather than some time later?
Paranoia. Xen's old MSR behaviour would have leaked INIT_REDIRECTION into guest context but discarded writes, and there are usecases to keep INIT_REDIRECTION enabled (if you're willing to sacrifice PV guests to avoid #SX-over-the-syscall-gap or back-to-back-INIT-on-IST shaped security holes). I can make it unconditional if you'd prefer. At the moment, all this is is a best-effort attempt to get back into the old state, so development can continue more easily. ~Andrew