Re: [PATCH 3/3] x86: Support booting under Secure Startup via SKINIT

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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