Re: [PATCH v2 1/2] x86/boot: Clear XD_DISABLE from the early boot path

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.06.2023 17:49, Alejandro Vallejo wrote:
> On Thu, Jun 22, 2023 at 09:54:01AM +0200, Jan Beulich wrote:
>> On 21.06.2023 18:43, Alejandro Vallejo wrote:
>>> Sure, to everything before this
>>>
>>>>> diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
>>>>> index 168cd58f36..46b0cd8dbb 100644
>>>>> --- a/xen/arch/x86/cpu/intel.c
>>>>> +++ b/xen/arch/x86/cpu/intel.c
>>>>> @@ -305,23 +305,23 @@ static void cf_check early_init_intel(struct 
>>>>> cpuinfo_x86 *c)
>>>>>           c->x86_cache_alignment = 128;
>>>>>  
>>>>>   /* Unmask CPUID levels and NX if masked: */
>>>>> - rdmsrl(MSR_IA32_MISC_ENABLE, misc_enable);
>>>>> -
>>>>> - disable = misc_enable & (MSR_IA32_MISC_ENABLE_LIMIT_CPUID |
>>>>> -                          MSR_IA32_MISC_ENABLE_XD_DISABLE);
>>>>> - if (disable) {
>>>>> -         wrmsrl(MSR_IA32_MISC_ENABLE, misc_enable & ~disable);
>>>>> -         bootsym(trampoline_misc_enable_off) |= disable;
>>>>> -         bootsym(trampoline_efer) |= EFER_NXE;
>>>>> - }
>>>>> + if (rdmsr_safe(MSR_IA32_MISC_ENABLE, misc_enable) == 0) {
>>>>
>>>> There's no need to change rdmsrl() to rdmsr_safe(),
>>> I thought we established before some hypervisors might not implement it. In
>>> that case this function would crash. More gracefully than a triple fault,
>>> sure, but still not a very friendly thing to do.
>>
>> Yet then in early boot code you also don't (and can't) recover from getting
>> #GP(0), in case that might really happen.
>>
> That's true, strictly speaking, but that case is restricted to the
> incredibly unlikely case of "no NX and no MSR". As is, if we ever boot
> on an Intel machine without that MSR we'll hit #GP(0), regardless of NX.

Well, my comment was meant to support Andrew's earlier one. Why
check the MSR access here when we don't earlier on? Plus isn't
it the case that the MSR can be implied to exist when NX is (in
principle) available, as that "opt-out" existed specifically
when Intel first introduced NX on their CPUs? If so,
- when NX is not available, we would have crashed much earlier,
- when NX is available (or was turned on successfully) the MSR
  exists either because we wrote to it earlier or because we're
  on a newer CPU.

Jan