Re: [PATCH v2 1/3] x86/boot: Fix intermediate file names to generate 32 bit code

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 06/11/2024 11:41 am, Frediano Ziglio wrote:
> The "base" and "offset" definition were inverted, "base" file
> should be the files without offsets applied while "offset" should
> have the offsets applied.
> Also update an old usage of "final" to "apply offset" to make
> more clear and consistent (in former commit messages the "final"
> term was used instead of "offset").
>
> Fixes: aa9045e77130 ('x86/boot: Rework how 32bit C is linked/included for 
> early boot')
>

We don't usually have blank lines between Fixes and other tags.  Can fix
on commit.

> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> --

Note, this wants to be 3 dashes not 2, like git automatically inserts ...


> Anthony was right, it was the opposite
> ---

... here.  (Various of the automated tools don't do the right thing
otherwise).

>  xen/arch/x86/boot/Makefile      | 7 ++++---
>  xen/arch/x86/boot/build32.lds.S | 2 +-
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
> index e102bd8c70..777b4befeb 100644
> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -44,7 +44,8 @@ text_gap := 0x010200
>  text_diff := 0x408020
>  
>  $(obj)/build32.base.lds: AFLAGS-y += -DGAP=$(text_gap) 
> -DTEXT_DIFF=$(text_diff)
> -$(obj)/build32.offset.lds: AFLAGS-y += -DGAP=$(text_gap) 
> -DTEXT_DIFF=$(text_diff) -DFINAL
> +$(obj)/build32.offset.lds: AFLAGS-y += -DGAP=$(text_gap) 
> -DTEXT_DIFF=$(text_diff) \
> +                                       -DAPPLY_OFFSET

I'd prefer to have this on a single line than re-flowed like this.  We
have various exceptions to the default width, and Makefiles are no
exception either.

Also happy to fix on commit.

Overall, Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

~Andrew