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

[Frediano Ziglio <frediano.ziglio@xxxxxxxxx>]

On Thu, Jan 9, 2025 at 4:03 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>
> 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.
>

What about something like
https://github.com/freddy77/xen/commit/299a1fd70a84e9b52b84d59daff6878a3c42a595
?

Frediano