On Mon, 26 Sept 2022 at 10:48, Yann Sionneau <ysionneau@xxxxxxxxx> wrote:
>
>
> On 8/28/22 16:05, Masahiro Yamada wrote:
> > On Fri, Aug 26, 2022 at 7:17 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> >> On Thu, 25 Aug 2022 at 20:01, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
> >>> Hi Ard,
> >>>
> >>> On Thu, Aug 25, 2022 at 2:56 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> >>>> On Thu, 25 Aug 2022 at 14:21, Yann Sionneau <ysionneau@xxxxxxxxx> wrote:
> >>>>> Well, I am not completely sure about that. See my cover letter, previous
> >>>>> mechanism for symbol CRC was actually enforcing the section alignment to
> >>>>> 4 bytes boundary as well.
> >>> Yes, because else it may become 2-byte aligned on m68k.
> >>>
> >>>>> Also, I'm not sure it is forbidden for an architecture/compiler
> >>>>> implementation to actually enforce a stronger alignment on u32, which in
> >>>>> theory would not break anything.
> >>>>>
> >>>> u32 is a Linux type, and Linux expects natural alignment (and padding).
> >>> Is it? You probably mean its alignment should not be larger than
> >>> 4 bytes? Less has been working since basically forever.
> >>>
> >> You are quite right. of course. And indeed, the issue here is padding
> >> not alignment.
> >>
> > I do not know if __align(4) should be used to avoid the padding issue.
> >
> >
> >
> > Do you think it is a good idea to use an inline assembler,
> > as prior to 7b4537199a4a8480b8c3ba37a2d44765ce76cd9b ?
> >
> >
> > This patch:
> >
> > diff --git a/include/linux/export-internal.h b/include/linux/export-internal.h
> > index c2b1d4fd5987..fb90f326b1b5 100644
> > --- a/include/linux/export-internal.h
> > +++ b/include/linux/export-internal.h
> > @@ -12,6 +12,9 @@
> >
> > /* __used is needed to keep __crc_* for LTO */
> > #define SYMBOL_CRC(sym, crc, sec) \
> > - u32 __section("___kcrctab" sec "+" #sym) __used __crc_##sym = crc
> > + asm(".section \"___kcrctab" sec "+" #sym "\",\"a\"" "\n" \
> > + "__crc_" #sym ":" "\n" \
> > + ".long " #crc "\n" \
> > + ".previous" "\n")
> >
> > #endif /* __LINUX_EXPORT_INTERNAL_H__ */
>
> Ping on this topic, should we "fix our toolchain"?
>
> Or should Linux code be modified to add either __align(4) or use the
> inline assembler? (I've tried your inline asm patch and it seems to fix
> the issue I'm having).
>
> Or both?
>
There are other cases where we rely on sections containing arrays of
u32 to be concatenated without gaps. If you would ever want to enable
HAVE_ARCH_PREL32_RELOCATIONS for your architecture (in order to save
some space in the binary wasted on absolute addresses or RELA
relocations) you'd run into the same issue afaict. So I'd recommend
fixing this in your compiler or linker asap.
