Re: [PATCH 16/22] x86/boot: Adjust CR4 handling around ap_early_traps_init()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/08/2025 3:47 pm, Jan Beulich wrote:
> On 08.08.2025 22:23, Andrew Cooper wrote:
>> ap_early_traps_init() will shortly be setting CR4.FRED.  This requires that
>> cpu_info->cr4 is already set up, and that the enablement of CET doesn't
>> truncate FRED back out because of it's 32bit logic.
>>
>> For __high_start(), defer re-loading XEN_MINIMAL_CR4 until after %rsp is set
>> up and we can store the result in the cr4 field too.
>>
>> For s3_resume(), explicitly re-load XEN_MINIMAL_CR4.  Later when loading all
>> features, use the mmu_cr4_features variable which is how the rest of Xen
>> performs this operation.
>>
>> No functional change, yet.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

Unfortunately, ...

>
>> --- a/xen/arch/x86/acpi/wakeup_prot.S
>> +++ b/xen/arch/x86/acpi/wakeup_prot.S
>> @@ -63,6 +63,14 @@ LABEL(s3_resume)
>>          pushq   %rax
>>          lretq
>>  1:
>> +
>> +        GET_STACK_END(15)
>> +
>> +        /* Enable minimal CR4 features. */
>> +        mov     $XEN_MINIMAL_CR4, %eax
>> +        mov     %rax, STACK_CPUINFO_FIELD(cr4)(%r15)
> Strictly speaking this and ...
>
>> --- a/xen/arch/x86/boot/x86_64.S
>> +++ b/xen/arch/x86/boot/x86_64.S
>> @@ -11,16 +11,19 @@ ENTRY(__high_start)
>>          mov     %ecx,%gs
>>          mov     %ecx,%ss
>>  
>> -        /* Enable minimal CR4 features. */
>> -        mov     $XEN_MINIMAL_CR4,%rcx
>> -        mov     %rcx,%cr4
>> -
>>          mov     stack_start(%rip),%rsp
>>  
>>          /* Reset EFLAGS (subsumes CLI and CLD). */
>>          pushq   $0
>>          popf
>>  
>> +        GET_STACK_END(15)
>> +
>> +        /* Enable minimal CR4 features. */
>> +        mov     $XEN_MINIMAL_CR4, %eax
>> +        mov     %rax, STACK_CPUINFO_FIELD(cr4)(%r15)
> ... this could be 32-bit stores, even in the longer run.

... no, they can't.

The store also serves to clear out stale X86_CR4_FRED, prior to FRED
getting reconfigured again.

fatal_trap() uses info->cr4 to decide whether it's safe to look at the
extended FRED metadata.  Strictly speaking I probably ought to read the
real CR4 (in read_registers too), but using a 32bit store here would
extend a 1-instruction window into quite a larger window where exception
handling would not work quite right.

~Andrew