On 11/10/2024 2:28 pm, Alejandro Vallejo wrote: > On Fri, Oct 11, 2024 at 02:08:37PM +0100, Frediano Ziglio wrote: >> On Fri, Oct 11, 2024 at 1:56 PM Alejandro Vallejo >> <[email protected]> wrote: >>> On Fri, Oct 11, 2024 at 09:52:44AM +0100, Frediano Ziglio wrote: >>>> Signed-off-by: Frediano Ziglio <[email protected]> >>>> --- >>>> xen/arch/x86/boot/reloc.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c >>>> index e50e161b27..e725cfb6eb 100644 >>>> --- a/xen/arch/x86/boot/reloc.c >>>> +++ b/xen/arch/x86/boot/reloc.c >>>> @@ -65,7 +65,7 @@ typedef struct memctx { >>>> /* >>>> * Simple bump allocator. >>>> * >>>> - * It starts from the base of the trampoline and allocates downwards. >>>> + * It starts on top of space reserved for the trampoline and >>>> allocates downwards. >>> nit: Not sure this is much clearer. The trampoline is not a stack (and even >>> if >>> it was, I personally find "top" and "bottom" quite ambiguous when it grows >>> backwards), so calling top to its lowest address seems more confusing than >>> not. >>> >>> If anything clarification ought to go in the which direction it takes. >>> Leaving >>> "base" instead of "top" and replacing "downwards" by "backwards" to make it >>> crystal clear that it's a pointer that starts where the trampoline starts, >>> but >>> moves in the opposite direction. >>> >> Base looks confusing to me, but surely that comment could be confusing. >> For the trampoline 64 KB are reserved. Last 4 KB are used as a normal >> stack (push/pop/call/whatever), first part gets a copy of the >> trampoline code/data (about 6 Kb) the rest (so 64 - 4 - ~6 = ~54 kb) >> is used for the copy of MBI information. That "rest" is what we are >> talking about here. > Last? From what I looked at it seems to be the first 12K. > > #define TRAMPOLINE_STACK_SPACE PAGE_SIZE > #define TRAMPOLINE_SPACE (KB(64) - TRAMPOLINE_STACK_SPACE) > > To put it another way, with left=lo-addr and right=hi-addr. The code seems to > do this... > > |<--------------64K-------------->| > |<-----12K--->| | > +-------------+-----+-------------+ > | stack-space | mbi | trampoline | > +-------------+-----+-------------+ > ^ ^ > | | > | +-- copied Multiboot info + modules > +----- initial memctx.ptr > > ... with the stack growing backwards to avoid overflowing onto mbi. > > Or am I missing something?
So I was hoping for some kind of diagram like this, to live in arch/x86/include/asm/trampoline.h with the other notes about the trampoline. But, is that diagram accurate? Looking at
