Re: [PATCH] x86/boot: adjust text gap/diff to work with 64-bytes alignment too

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/01/2025 3:30 pm, Frediano Ziglio wrote:
> On Thu, Jan 9, 2025 at 1:44 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
> wrote:
>> On 09/01/2025 1:23 pm, Jan Beulich wrote:
>>> On 09.01.2025 14:15, Marek Marczykowski-Górecki wrote:
>>>> Xen compiled with -mtune=generic has .text alignment set to 64-bytes.
>>>> Having text_diff non-64-bytes-aligned breaks stuff:
>>>>
>>>>     Traceback (most recent call last):
>>>>       File 
>>>> "/builddir/build/BUILD/xen-4.20.0-build/xen-4.20.0-rc0/xen/./tools/combine_two_binaries.py",
>>>>  line 96, in <module>
>>>>         raise Exception('File sizes do not match')
>>>>     Exception: File sizes do not match: 70160 != 4080 + 66048
>>>>
>>>> Adjust the numbers as suggested by Frediano to work with 64-bytes and
>>>> even 128-bytes alignment.
>>> And then breaking at 256? As indicated on Matrix, imo we should be able to
>>> cope with anything up to at least PAGE_SIZE. Or we should reduce .text
>>> alignment before linking.
>> Do you have a concrete proposal on how to do this?
>>
>> Because if not, we've had 2 downstreams hit by this build failure, and
>> we probably ought to take this patch as a stopgap fix, and see about
>> doing the better fix for 4.20.
>>
> I agree with this, merge this and then leave the improvements for follow 
> up(s).
>
> Yesterday I checked the output object file (built-in-32.o) to find
> that the output alignment is 1 byte, which is obviously wrong!

No so.  It's perfectly easy to end up with 1 byte alignment on .text,
commonly with -Os.

e.g. In my build, reloc-trampoline.32.o has:

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
  0 .group        00000008  00000000  00000000  00000034  2**2
                  CONTENTS, READONLY, GROUP, LINK_ONCE_DISCARD
  1 .text         00000076  00000000  00000000  0000003c  2**0
                  CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE
  2 .data         00000000  00000000  00000000  000000b2  2**0
                  CONTENTS, ALLOC, LOAD, DATA
  3 .bss          00000000  00000000  00000000  000000b2  2**0
                  ALLOC
  4 .text.__x86.get_pc_thunk.bx 00000004  00000000  00000000  000000b2  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  5 .comment      00000020  00000000  00000000  000000b6  2**0
                  CONTENTS, READONLY
  6 .note.GNU-stack 00000000  00000000  00000000  000000d6  2**0
                  CONTENTS, READONLY

and this is entirely reasonable IMO.

That said, the fact that built-in-32.S does not have at least one .align
directive probably is an issue.

x86 will cope, and there's nothing interesting in this code that's going
to choke on being misaligned.

~Andrew