Re: [PATCH v3 5/5] x86/boot: Clarify comment

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On Fri, Oct 11, 2024 at 09:52:44AM +0100, Frediano Ziglio wrote:
> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> ---
>  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.

My .02 anyway.

>       */
>      uint32_t ptr;
>  } memctx;
> -- 
> 2.34.1
> 
> 

Cheers,
Alejandro