| Message ID | 20241114090810.1961175-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/trampoline: Layout description improvements. | expand |
On 14.11.2024 10:08, Andrew Cooper wrote: > By checking that the permanent trampoline fits within 1k (at the time of > writing, it's 0x229 bytes), we can simplify the wakeup_stack handling. > > Move the setup into wakeup.S, because it's rather out of place in > trampoline.S, and change it to a local symbol. > > Drop wakeup_stack_start and WAKEUP_STACK_MIN entirely. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> with one nit: > --- a/xen/arch/x86/boot/trampoline.S > +++ b/xen/arch/x86/boot/trampoline.S > @@ -156,11 +156,6 @@ GLOBAL(trampoline_xen_phys_start) > GLOBAL(trampoline_cpu_started) > .byte 0 > > -/* The first page of trampoline is permanent, the rest boot-time only. */ > -/* Reuse the boot trampoline on the 1st trampoline page as stack for wakeup. */ > - .equ wakeup_stack, trampoline_start + PAGE_SIZE > - .global wakeup_stack > - > LABEL(trampoline_perm_end, 0) > > /* From here on early boot only. */ > diff --git a/xen/arch/x86/boot/wakeup.S b/xen/arch/x86/boot/wakeup.S > index df5ea2445739..d53f92b02463 100644 > --- a/xen/arch/x86/boot/wakeup.S > +++ b/xen/arch/x86/boot/wakeup.S > @@ -1,5 +1,10 @@ > .code16 > > +/* The first page of trampoline is permanent, the rest boot-time only. */ > +/* Reuse the boot logic on the first trampoline page as stack for wakeup. */ > + .equ wakeup_stack, trampoline_start + PAGE_SIZE > + .local wakeup_stack > + > #define wakesym(sym) (sym - wakeup_start) As you move it, it would be nice for the commentary to become a single multi-line comment rather than two single-line ones. Jan
diff --git a/xen/arch/x86/boot/trampoline.S b/xen/arch/x86/boot/trampoline.S index 55e4a3e402f7..924bda37c1b7 100644 --- a/xen/arch/x86/boot/trampoline.S +++ b/xen/arch/x86/boot/trampoline.S @@ -156,11 +156,6 @@ GLOBAL(trampoline_xen_phys_start) GLOBAL(trampoline_cpu_started) .byte 0 -/* The first page of trampoline is permanent, the rest boot-time only. */ -/* Reuse the boot trampoline on the 1st trampoline page as stack for wakeup. */ - .equ wakeup_stack, trampoline_start + PAGE_SIZE - .global wakeup_stack - LABEL(trampoline_perm_end, 0) /* From here on early boot only. */ diff --git a/xen/arch/x86/boot/wakeup.S b/xen/arch/x86/boot/wakeup.S index df5ea2445739..d53f92b02463 100644 --- a/xen/arch/x86/boot/wakeup.S +++ b/xen/arch/x86/boot/wakeup.S @@ -1,5 +1,10 @@ .code16 +/* The first page of trampoline is permanent, the rest boot-time only. */ +/* Reuse the boot logic on the first trampoline page as stack for wakeup. */ + .equ wakeup_stack, trampoline_start + PAGE_SIZE + .local wakeup_stack + #define wakesym(sym) (sym - wakeup_start) /* @@ -166,6 +171,3 @@ wakeup_64: /* Jump to high mappings and the higher-level wakeup code. */ movabs $s3_resume, %rbx jmp *%rbx - -/* Stack for wakeup: rest of first trampoline page. */ -ENTRY(wakeup_stack_start) diff --git a/xen/arch/x86/include/asm/config.h b/xen/arch/x86/include/asm/config.h index f8a5a4913b07..84696e0a7db5 100644 --- a/xen/arch/x86/include/asm/config.h +++ b/xen/arch/x86/include/asm/config.h @@ -53,8 +53,6 @@ #define TRAMPOLINE_STACK_SPACE PAGE_SIZE #define TRAMPOLINE_SPACE (KB(64) - TRAMPOLINE_STACK_SPACE) -#define WAKEUP_STACK_MIN 3072 - #define MBI_SPACE_MIN (2 * PAGE_SIZE) /* Primary stack is restricted to 8kB by guard pages. */ diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S index 221fc2ef3f35..224b46771d0c 100644 --- a/xen/arch/x86/xen.lds.S +++ b/xen/arch/x86/xen.lds.S @@ -413,11 +413,12 @@ ASSERT(!SIZEOF(.rela), "leftover relocations") /* * The permanent trampoline resides in a single 4k page. Placement logic * takes care to ensure that trampoline_phys is page aligned. + * + * The wakeup stack wants to reside in the same page and wants to be at least + * 3k in size, so make sure the text/data fits in 1k. */ -ASSERT((trampoline_perm_end - trampoline_start) <= PAGE_SIZE, +ASSERT((trampoline_perm_end - trampoline_start) <= 1024, "Permentant trampoline too large") ASSERT((trampoline_end - trampoline_start) < TRAMPOLINE_SPACE - MBI_SPACE_MIN, "not enough room for trampoline and mbi data") -ASSERT((wakeup_stack - wakeup_stack_start) >= WAKEUP_STACK_MIN, - "wakeup stack too small")
By checking that the permanent trampoline fits within 1k (at the time of writing, it's 0x229 bytes), we can simplify the wakeup_stack handling. Move the setup into wakeup.S, because it's rather out of place in trampoline.S, and change it to a local symbol. Drop wakeup_stack_start and WAKEUP_STACK_MIN entirely. No functional change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Daniel P. Smith <dpsmith@apertussolutions.com> CC: Frediano Ziglio <frediano.ziglio@cloud.com> CC: Alejandro Vallejo <alejandro.vallejo@cloud.com> v2: * New --- xen/arch/x86/boot/trampoline.S | 5 ----- xen/arch/x86/boot/wakeup.S | 8 +++++--- xen/arch/x86/include/asm/config.h | 2 -- xen/arch/x86/xen.lds.S | 7 ++++--- 4 files changed, 9 insertions(+), 13 deletions(-)