Re: [RFC PATCH 1/1] Fix __kcrctab+* sections alignment

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(cc Arnd)

On Thu, 25 Aug 2022 at 14:21, Yann Sionneau <ysionneau@xxxxxxxxx> wrote:
>
> Hello Ard,
>
> On 25/08/2022 14:12, Ard Biesheuvel wrote:
> > On Thu, 25 Aug 2022 at 14:10, Yann Sionneau <ysionneau@xxxxxxxxx> wrote:
> >> Forwarding also the actual patch to linux-kbuild and linux-arch
> >>
> >> -------- Forwarded Message --------
> >> Subject:        [RFC PATCH 1/1] Fix __kcrctab+* sections alignment
> >> Date:   Wed, 17 Aug 2022 18:14:38 +0200
> >> From:   Yann Sionneau <ysionneau@xxxxxxxxx>
> >> To:     linux-kernel@xxxxxxxxxxxxxxx
> >> CC:     Nicolas Schier <nicolas@xxxxxxxxx>, Masahiro Yamada
> >> <masahiroy@xxxxxxxxxx>, Jules Maselbas <jmaselbas@xxxxxxxxx>, Julian
> >> Vetter <jvetter@xxxxxxxxx>, Yann Sionneau <ysionneau@xxxxxxxxx>
> >>
> >>
> >>
> > What happened to the commit log?
>
> This is a forward of this thread: https://lkml.org/lkml/2022/8/17/868
>
> Either I did something wrong with my email agent or maybe the email
> containing the cover letter is taking some time to reach you?
>

Never mind, i see the other thread as well.

Exec summary: out-of-tree port for kvx architecture appears to impose
8 byte alignment for u32 types in some cases.

> >
> >> Signed-off-by: Yann Sionneau <ysionneau@xxxxxxxxx>
> >> ---
> >> include/linux/export-internal.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/linux/export-internal.h
> >> b/include/linux/export-internal.h
> >> index c2b1d4fd5987..d86bfbd7fa6d 100644
> >> --- a/include/linux/export-internal.h
> >> +++ b/include/linux/export-internal.h
> >> @@ -12,6 +12,6 @@
> >> /* __used is needed to keep __crc_* for LTO */
> >> #define SYMBOL_CRC(sym, crc, sec) \
> >> - u32 __section("___kcrctab" sec "+" #sym) __used __crc_##sym = crc
> >> + u32 __section("___kcrctab" sec "+" #sym) __used __aligned(4)

> > __aligned(4) is the default for u32 so this should not be needed.

>
> 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.
>
> 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).

So if your toolchain/architecture violates this rule, I suggest you
typedef u32 to 'unsigned int __aligned(4)' explicitly. so that things
don't break in other places.

However, even then, I am highly skeptical. This really seems like an
issue in your toolchain that could cause problems all over the place.

> But in this precise case it does break something since it will cause
> "gaps" in the end result vmlinux binary segment. For this to work I
> think we really want to enforce a 4 bytes alignment on the section.
>

You are addressing one of many potential issues that could be caused
by this, so I don't think this patch is a good idea tbh.


> >
> >
> >> __crc_##sym = crc
> >> #endif /* __LINUX_EXPORT_INTERNAL_H__ */
> >>
> >> --
> >> 2.37.2
>
> Thanks for your review :)
>
> --
>
> Yann
>
>
>
>
>



[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux