[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 28.01.2021 21:26, Andrew Cooper wrote:
> 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,

In which case there wouldn't have been any fault to catch and
ignore.

> 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.

I'm not sure which variant is strictly better, but if you stick
to wrmsr_safe(), may I ask that you say this is out of paranoia
in the comment, so future readers will not wonder like I did?

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.