[PATCH] x86/boot: Handle better alignment for 32 bit code

[PATCH] x86/boot: Handle better alignment for 32 bit code

[Frediano Ziglio]

Output file didn't have correct alignment.
Allows alignment into data or code up to 2mb.

Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
 xen/arch/x86/boot/Makefile        | 8 ++++----
 xen/tools/combine_two_binaries.py | 7 ++++++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
index 13d4583173..9a8ecba7aa 100644
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -40,8 +40,8 @@ LD32 := $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT))
 # are affected by both text_diff and text_gap.  Ensure the sum of gap and diff
 # is greater than 2^16 so that any 16bit relocations if present in the object
 # file turns into a build-time error.
-text_gap := 0x010200
-text_diff := 0x408020
+text_gap := 0x01c240
+text_diff := 0x7e3dc0
 
 $(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) -DAPPLY_OFFSET
@@ -69,7 +69,6 @@ $(obj)/built-in-32.%.bin: $(obj)/build32.%.lds $(obj)/built-in-32.tmp.o
 	$(LD32) $(orphan-handling-y) -N -T $< -o $(@:bin=o) $(filter %.o,$^)
 	$(NM) -p --format=bsd $(@:bin=o) > $(@:bin=map)
 	$(OBJCOPY) -j .text -O binary $(@:bin=o) $@
-	rm -f $(@:bin=o)
 
 quiet_cmd_combine = GEN     $@
 cmd_combine = \
@@ -80,6 +79,7 @@ cmd_combine = \
               --bin1      $(obj)/built-in-32.base.bin \
               --bin2      $(obj)/built-in-32.offset.bin \
               --map       $(obj)/built-in-32.base.map \
+              --align     $(shell $(OBJDUMP) -h $(obj)/built-in-32.base.o|sed '/text.*2\*\*/ {s/.*2\*\*//;p;}; d') \
               --exports   cmdline_parse_early,reloc,reloc_trampoline32 \
               --output    $@
 
@@ -90,4 +90,4 @@ $(obj)/built-in-32.S: $(obj)/built-in-32.base.bin $(obj)/built-in-32.offset.bin
                       $(srctree)/tools/combine_two_binaries.py FORCE
 	$(call if_changed,combine)
 
-clean-files := built-in-32.*.bin built-in-32.*.map build32.*.lds
+clean-files := built-in-32.*.bin built-in-32.*.map built-in-32.*.o build32.*.lds
diff --git a/xen/tools/combine_two_binaries.py b/xen/tools/combine_two_binaries.py
index 581e57cbc0..8e587c24fb 100755
--- a/xen/tools/combine_two_binaries.py
+++ b/xen/tools/combine_two_binaries.py
@@ -26,6 +26,10 @@ parser.add_argument('--text-diff', dest='text_diff',
                     required=True,
                     type=auto_int,
                     help='Difference between code section start')
+parser.add_argument('--align', dest='align',
+                    default=2,
+                    type=auto_int,
+                    help='Alignment in power of 2')
 parser.add_argument('--output', dest='output',
                     help='Output file')
 parser.add_argument('--map', dest='mapfile',
@@ -93,7 +97,7 @@ if size1 > size2:
     file1, file2 = file2, file1
     size1, size2 = size2, size1
 if size2 != size1 + gap:
-    raise Exception('File sizes do not match')
+    raise Exception('File sizes do not match %d != %d + %d' % (size2, size1, gap))
 del size2
 
 file1.seek(0, 0)
@@ -219,6 +223,7 @@ print('''/*
  * File autogenerated by combine_two_binaries.py DO NOT EDIT
  */''', file=out)
 print('\t' + args.section_header, file=out)
+print('\t.p2align\t' + str(args.align), file=out)
 print('obj32_start:', file=out)
 output(out)
 print('\n\t.section .note.GNU-stack,"",@progbits', file=out)
-- 
2.34.1

Re: [PATCH] x86/boot: Handle better alignment for 32 bit code

[Jan Beulich]

On 14.01.2025 10:59, Frediano Ziglio wrote:
> Output file didn't have correct alignment.
> Allows alignment into data or code up to 2mb.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>

Afaic this is way too little of a description. For example, ...

> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -40,8 +40,8 @@ LD32 := $(LD) $(subst x86_64,i386,$(LDFLAGS_DIRECT))
>  # are affected by both text_diff and text_gap.  Ensure the sum of gap and diff
>  # is greater than 2^16 so that any 16bit relocations if present in the object
>  # file turns into a build-time error.
> -text_gap := 0x010200
> -text_diff := 0x408020
> +text_gap := 0x01c240
> +text_diff := 0x7e3dc0

... how is anyone to derive how we ended up with these magic numbers?
What parts of them are arbitrary, and what parts are required to be the
way they are?

> @@ -69,7 +69,6 @@ $(obj)/built-in-32.%.bin: $(obj)/build32.%.lds $(obj)/built-in-32.tmp.o
>  	$(LD32) $(orphan-handling-y) -N -T $< -o $(@:bin=o) $(filter %.o,$^)
>  	$(NM) -p --format=bsd $(@:bin=o) > $(@:bin=map)
>  	$(OBJCOPY) -j .text -O binary $(@:bin=o) $@
> -	rm -f $(@:bin=o)

This looks like an unrelated change. It may be okay to have here, but
then it needs mentioning in the description.

> @@ -80,6 +79,7 @@ cmd_combine = \
>                --bin1      $(obj)/built-in-32.base.bin \
>                --bin2      $(obj)/built-in-32.offset.bin \
>                --map       $(obj)/built-in-32.base.map \
> +              --align     $(shell $(OBJDUMP) -h $(obj)/built-in-32.base.o|sed '/text.*2\*\*/ {s/.*2\*\*//;p;}; d') \
>                --exports   cmdline_parse_early,reloc,reloc_trampoline32 \
>                --output    $@
>  
> @@ -90,4 +90,4 @@ $(obj)/built-in-32.S: $(obj)/built-in-32.base.bin $(obj)/built-in-32.offset.bin
>                        $(srctree)/tools/combine_two_binaries.py FORCE
>  	$(call if_changed,combine)
>  
> -clean-files := built-in-32.*.bin built-in-32.*.map build32.*.lds
> +clean-files := built-in-32.*.bin built-in-32.*.map built-in-32.*.o build32.*.lds

This looks like it would have been needed already before, if the build
process was interrupted before the "rm" that you remove above.

Jan