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

[Masahiro Yamada <masahiroy@kernel.org>]

On Thu, Aug 25, 2022 at 9:21 PM Yann Sionneau <ysionneau@kalray.eu> wrote:
>
> Hello Ard,
>
> On 25/08/2022 14:12, Ard Biesheuvel wrote:
> > On Thu, 25 Aug 2022 at 14:10, Yann Sionneau <ysionneau@kalray.eu> 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@kalray.eu>
> >> To:     linux-kernel@vger.kernel.org
> >> CC:     Nicolas Schier <nicolas@fjasle.eu>, Masahiro Yamada
> >> <masahiroy@kernel.org>, Jules Maselbas <jmaselbas@kalray.eu>, Julian
> >> Vetter <jvetter@kalray.eu>, Yann Sionneau <ysionneau@kalray.eu>
> >>
> >>
> >>
> > 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?
>
> >
> >> Signed-off-by: Yann Sionneau <ysionneau@kalray.eu>
> >> ---
> >> 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.

I do not think so.


I do not see such alignment in for __CRC_SYMBOL() in
include/linux/export.h

If you are talking about KCRC_ALIGN
in include/asm-generic/export.h, it is only used by *.S.


Most of EXPORT_SYMBOL's are defined in *.c files,
which include <linux/export.h>

If I am missing something, please point me to the code.








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

It seems like an interesting compiler.

Does it also enforce 8 byte alignment to u8?
(that is, 7-byte padding for u8 ?)



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


My best guess it, it was previously working for the kvm compiler
because include/linux/export.h previously used an inline assembler.

The kvm toolchain presumably gives natural alignment/padding
to '.long' assembly directive, but enforces 8-byte to u32.




-- 
Best Regards
Masahiro Yamada