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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.01.2025 12:54, Frediano Ziglio wrote:
> Output file didn't have correct alignment.

Coming back to this (as it's still in my "pending" folder), what does the above
mean? In how far was which alignment not correct for what purpose?

> --- a/xen/arch/x86/boot/Makefile
> +++ b/xen/arch/x86/boot/Makefile
> @@ -40,8 +40,12 @@ 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
> +# As gap will affect the output section size it should not be huge to avoid 
> the
> +# creation of huge files.
> +# The sum of gap and diff will affect the possible alignment so should be a
> +# multiple of the possible alignment.
> +text_gap := 0x01c240
> +text_diff := 0x7e3dc0

It remains unclear what "possible alignment" here is. Maybe that's the "up to
2mb" that the description talks about, but when looking at the file later the
description of this change will not be readily available.

> @@ -80,6 +83,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') \

I've tried this out manually on $(obj)/built-in-32.o, as 
$(obj)/built-in-32.base.o
isn't there without applying this patch. I'm observing two matching sections 
there,
i.e. I wonder whether the regex shouldn't be tightened. Not being fluent with 
sed
commands, I also wonder whether passing -n to the tool in favor of the d command
might not be better.

> @@ -90,4 +94,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

As before I think this is a separate bugfix anyway.

Jan