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')

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
--
Anthony was right, it was the opposite
---
 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
 $(obj)/build32.base.lds $(obj)/build32.offset.lds: $(src)/build32.lds.S FORCE
 	$(call if_changed_dep,cpp_lds_S)
 
@@ -75,10 +76,10 @@ cmd_combine = \
     $(PYTHON) $(srctree)/tools/combine_two_binaries.py \
               --gap       $(text_gap) \
               --text-diff $(text_diff) \
-              --script    $(obj)/build32.offset.lds \
+              --script    $(obj)/build32.base.lds \
               --bin1      $(obj)/built-in-32.base.bin \
               --bin2      $(obj)/built-in-32.offset.bin \
-              --map       $(obj)/built-in-32.offset.map \
+              --map       $(obj)/built-in-32.base.map \
               --exports   cmdline_parse_early,reloc,reloc_trampoline32 \
               --output    $@
 
diff --git a/xen/arch/x86/boot/build32.lds.S b/xen/arch/x86/boot/build32.lds.S
index f20fc18977..9b29f0184f 100644
--- a/xen/arch/x86/boot/build32.lds.S
+++ b/xen/arch/x86/boot/build32.lds.S
@@ -15,7 +15,7 @@
  * with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifdef FINAL
+#ifndef APPLY_OFFSET
 # undef GAP
 # define GAP 0
 # define MULT 0
-- 
2.34.1

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@cloud.com>
> --

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@citrix.com>

~Andrew