On 14/08/2025 3:56 pm, Jan Beulich wrote: > On 14.08.2025 16:54, Andrew Cooper wrote: >> On 14/08/2025 3:47 pm, Jan Beulich wrote: >>> On 08.08.2025 22:23, Andrew Cooper wrote: >>>> --- 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. > Oh, I see. Mind me asking to add brief comments there to this effect?
I've changed it to: /* Enable minimal CR4 features, sync cached state. */ ~Andrew
