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/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.
Oh, I see. Mind me asking to add brief comments there to this effect? Jan
