On 13.11.2024 12:19, Andrew Cooper wrote: > On 13/11/2024 10:20 am, Jan Beulich wrote: >> On 13.11.2024 10:30, Andrew Cooper wrote: >>> --- a/xen/arch/x86/include/asm/trampoline.h >>> +++ b/xen/arch/x86/include/asm/trampoline.h >>> @@ -37,12 +37,63 @@ >>> * manually as part of placement. >>> */ >>> >>> +/* >>> + * Layout of the trampoline. Logical areas, in ascending order: >>> + * >>> + * 1) AP boot: >>> + * >>> + * The INIT-SIPI-SIPI entrypoint. This logic is stack-less so the >>> identity >>> + * mapping (which must be executable) can at least be Read Only. >>> + * >>> + * 2) S3 resume: >>> + * >>> + * The S3 wakeup logic may need to interact with the BIOS, so needs a >>> + * stack. The stack pointer is set to trampoline_phys + 4k and >>> clobbers an >>> + * undefined part of the the boot trampoline. The stack is only used >>> with >>> + * paging disabled. >>> + * >>> + * 3) Boot trampoline: >>> + * >>> + * This region houses various data used by the AP/S3 paths too. >> This is confusing to have here - isn't the boot part (that isn't in the >> same page as the tail of the AP/S3 region) being boot-time only, and hence >> unavailable for S3 and post-boot AP bringup? Both here and with the numbers >> in the description - what position did you use as separator between 2) and >> 3)? >> >> Then again it may be just me who is confused: Didn't we, at some point, limit >> the resident trampoline to just one page? Was that only a plan, or a patch >> that never was committed? > > The positioning of various things is rather complicated. > > Only a single 4k page is mapped into idle_pg_table[]. > > But, the AP/S3 path use: > trampoline_cpu_started > idt_48 > gdt_48 > trampoline_xen_phys_start > trampoline_misc_enable_off > trampoline_efer > > Which is beyond the content of wakeup.S. The GDT in particular needs to > stay valid with paging enabled, to load __HYPERVISOR_CS. > > We have /* From here on early boot only. */ in trampoline.S but that > seems to be the extent of checking. Everything needed for AP/S3 is in > the first 0x229.
I was fearing it might be as weak. We probably want to gain a symbol there, for use in another linker script assertion. Jan
