[PATCH] arm64/sve: fix genksyms generation

[PATCH] arm64/sve: fix genksyms generation

[Arnd Bergmann]

genksyms does not understand __uint128_t, so we get a build failure
in the fpsimd module when it cannot export a symbol right:

WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version generation failed, symbol will not be versioned.
/home/arnd/cross/x86_64/gcc-8.1.0-nolibc/aarch64-linux/bin/aarch64-linux-ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against `__crc_kernel_neon_begin' can not be used when making a shared object
arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation: unsupported relocation
arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x0): dangerous relocation: unsupported relocation
arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x8): dangerous relocation: unsupported relocation

We could teach genksyms about the type, but it's easier to just
work around it by defining that type locally in a way that genksyms
understands.

Fixes: 41040cf7c5f0 ("arm64/sve: Fix missing SVE/FPSIMD endianness conversions")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm64/kernel/fpsimd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 07f238ef47ae..2aba07cccf50 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -400,6 +400,9 @@ static int __init sve_sysctl_init(void) { return 0; }
 #define ZREG(sve_state, vq, n) ((char *)(sve_state) +		\
 	(SVE_SIG_ZREG_OFFSET(vq, n) - SVE_SIG_REGS_OFFSET))
 
+#ifdef __GENKSYMS__
+typedef __u64 __uint128_t[2];
+#endif
 #ifdef CONFIG_CPU_BIG_ENDIAN
 static __uint128_t arm64_cpu_to_le128(__uint128_t x)
 {
-- 
2.20.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Will Deacon]

Hi Arnd,

On Mon, Jun 17, 2019 at 12:42:11PM +0200, Arnd Bergmann wrote:
> genksyms does not understand __uint128_t, so we get a build failure
> in the fpsimd module when it cannot export a symbol right:

The fpsimd code is builtin, so which module is actually failing? My
allmodconfig build succeeds, so I must be missing something.

> WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version generation failed, symbol will not be versioned.
> /home/arnd/cross/x86_64/gcc-8.1.0-nolibc/aarch64-linux/bin/aarch64-linux-ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against `__crc_kernel_neon_begin' can not be used when making a shared object
> arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation: unsupported relocation
> arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x0): dangerous relocation: unsupported relocation
> arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x8): dangerous relocation: unsupported relocation
> 
> We could teach genksyms about the type, but it's easier to just
> work around it by defining that type locally in a way that genksyms
> understands.
> 
> Fixes: 41040cf7c5f0 ("arm64/sve: Fix missing SVE/FPSIMD endianness conversions")

I can't see which part of that patch causes the problem, so I'm a bit wary
of the fix. We've been using __uint128_t for a while now, and I see there's
one in the x86 kvm code as well, so it would be nice to understand what's
happening here so that we can avoid running into it in future as well.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm64/kernel/fpsimd.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index 07f238ef47ae..2aba07cccf50 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -400,6 +400,9 @@ static int __init sve_sysctl_init(void) { return 0; }
>  #define ZREG(sve_state, vq, n) ((char *)(sve_state) +		\
>  	(SVE_SIG_ZREG_OFFSET(vq, n) - SVE_SIG_REGS_OFFSET))
>  
> +#ifdef __GENKSYMS__
> +typedef __u64 __uint128_t[2];
> +#endif

I suspect I need to figure out what genksyms is doing, but I'm nervous
about exposing this as an array type without understanding whether or
not that has consequences for its operation.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Arnd Bergmann]

On Mon, Jun 17, 2019 at 1:26 PM Will Deacon <will.deacon@arm.com> wrote:
>
> Hi Arnd,
>
> On Mon, Jun 17, 2019 at 12:42:11PM +0200, Arnd Bergmann wrote:
> > genksyms does not understand __uint128_t, so we get a build failure
> > in the fpsimd module when it cannot export a symbol right:
>
> The fpsimd code is builtin, so which module is actually failing? My
> allmodconfig build succeeds, so I must be missing something.

It happened for me on randconfig builds, you can find one such configuration
at https://pastebin.com/cU8iQ4ta now. I was building this with clang
rather than gcc, which may affect the issue, but I assumed not.

> > WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version generation failed, symbol will not be versioned.
> > /home/arnd/cross/x86_64/gcc-8.1.0-nolibc/aarch64-linux/bin/aarch64-linux-ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against `__crc_kernel_neon_begin' can not be used when making a shared object
> > arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation: unsupported relocation
> > arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x0): dangerous relocation: unsupported relocation
> > arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x8): dangerous relocation: unsupported relocation
> >
> > We could teach genksyms about the type, but it's easier to just
> > work around it by defining that type locally in a way that genksyms
> > understands.
> >
> > Fixes: 41040cf7c5f0 ("arm64/sve: Fix missing SVE/FPSIMD endianness conversions")
>
> I can't see which part of that patch causes the problem, so I'm a bit wary
> of the fix. We've been using __uint128_t for a while now, and I see there's
> one in the x86 kvm code as well, so it would be nice to understand what's
> happening here so that we can avoid running into it in future as well.

The problem is only in files that export a symbol. This is also the
case in arch/x86/kernel/kvm.c, but it may be lucky because the
type only appears /after/ the last export in that file.

> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  arch/arm64/kernel/fpsimd.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > index 07f238ef47ae..2aba07cccf50 100644
> > --- a/arch/arm64/kernel/fpsimd.c
> > +++ b/arch/arm64/kernel/fpsimd.c
> > @@ -400,6 +400,9 @@ static int __init sve_sysctl_init(void) { return 0; }
> >  #define ZREG(sve_state, vq, n) ((char *)(sve_state) +                \
> >       (SVE_SIG_ZREG_OFFSET(vq, n) - SVE_SIG_REGS_OFFSET))
> >
> > +#ifdef __GENKSYMS__
> > +typedef __u64 __uint128_t[2];
> > +#endif
>
> I suspect I need to figure out what genksyms is doing, but I'm nervous
> about exposing this as an array type without understanding whether or
> not that has consequences for its operation.

The entire point is genksyms is to ensure that types of exported symbols
are compatible. To do this, it has a limited parser for C source code that
understands the basic types (char, int, long, _Bool, etc) and how to
aggregate them into structs and function arguments. This process has
always been fragile, and it clearly breaks when it fails to understand a
particular type.

          Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Alex Bennée]

Arnd Bergmann <arnd@arndb.de> writes:

> On Mon, Jun 17, 2019 at 1:26 PM Will Deacon <will.deacon@arm.com> wrote:
>>
>> Hi Arnd,
>>
>> On Mon, Jun 17, 2019 at 12:42:11PM +0200, Arnd Bergmann wrote:
>> > genksyms does not understand __uint128_t, so we get a build failure
>> > in the fpsimd module when it cannot export a symbol right:
>>
>> The fpsimd code is builtin, so which module is actually failing? My
>> allmodconfig build succeeds, so I must be missing something.
>
> It happened for me on randconfig builds, you can find one such configuration
> at https://pastebin.com/cU8iQ4ta now. I was building this with clang
> rather than gcc, which may affect the issue, but I assumed not.
>
>> > WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version generation failed, symbol will not be versioned.
>> > /home/arnd/cross/x86_64/gcc-8.1.0-nolibc/aarch64-linux/bin/aarch64-linux-ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against `__crc_kernel_neon_begin' can not be used when making a shared object
>> > arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation: unsupported relocation
>> > arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x0): dangerous relocation: unsupported relocation
>> > arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x8): dangerous relocation: unsupported relocation
>> >
>> > We could teach genksyms about the type, but it's easier to just
>> > work around it by defining that type locally in a way that genksyms
>> > understands.
>> >
>> > Fixes: 41040cf7c5f0 ("arm64/sve: Fix missing SVE/FPSIMD endianness conversions")
>>
>> I can't see which part of that patch causes the problem, so I'm a bit wary
>> of the fix. We've been using __uint128_t for a while now, and I see there's
>> one in the x86 kvm code as well, so it would be nice to understand what's
>> happening here so that we can avoid running into it in future as well.
>
> The problem is only in files that export a symbol. This is also the
> case in arch/x86/kernel/kvm.c, but it may be lucky because the
> type only appears /after/ the last export in that file.
>
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> > ---
>> >  arch/arm64/kernel/fpsimd.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>> >
>> > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
>> > index 07f238ef47ae..2aba07cccf50 100644
>> > --- a/arch/arm64/kernel/fpsimd.c
>> > +++ b/arch/arm64/kernel/fpsimd.c
>> > @@ -400,6 +400,9 @@ static int __init sve_sysctl_init(void) { return 0; }
>> >  #define ZREG(sve_state, vq, n) ((char *)(sve_state) +                \
>> >       (SVE_SIG_ZREG_OFFSET(vq, n) - SVE_SIG_REGS_OFFSET))
>> >
>> > +#ifdef __GENKSYMS__
>> > +typedef __u64 __uint128_t[2];
>> > +#endif
>>
>> I suspect I need to figure out what genksyms is doing, but I'm nervous
>> about exposing this as an array type without understanding whether or
>> not that has consequences for its operation.
>
> The entire point is genksyms is to ensure that types of exported symbols
> are compatible. To do this, it has a limited parser for C source code that
> understands the basic types (char, int, long, _Bool, etc) and how to
> aggregate them into structs and function arguments. This process has
> always been fragile, and it clearly breaks when it fails to understand a
> particular type.

Shouldn't the solution for this be to fix genksyms to be less fragile
and more understanding? The code base doesn't seem to be full of these
sorts of ifdef workarounds.

>
>           Arnd


--
Alex Bennée

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Arnd Bergmann]

On Mon, Jun 17, 2019 at 4:59 PM Alex Bennée <alex.bennee@linaro.org> wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
> > On Mon, Jun 17, 2019 at 1:26 PM Will Deacon <will.deacon@arm.com> wrote:
> >> On Mon, Jun 17, 2019 at 12:42:11PM +0200, Arnd Bergmann wrote:
> >> I suspect I need to figure out what genksyms is doing, but I'm nervous
> >> about exposing this as an array type without understanding whether or
> >> not that has consequences for its operation.
> >
> > The entire point is genksyms is to ensure that types of exported symbols
> > are compatible. To do this, it has a limited parser for C source code that
> > understands the basic types (char, int, long, _Bool, etc) and how to
> > aggregate them into structs and function arguments. This process has
> > always been fragile, and it clearly breaks when it fails to understand a
> > particular type.
>
> Shouldn't the solution for this be to fix genksyms to be less fragile
> and more understanding? The code base doesn't seem to be full of these
> sorts of ifdef workarounds.

It is one of the things I tried before I got to the version I send.
Unfortunately
the genksyms codebase is a big complex and I quickly got lost in it.

You're welcome to volunteer fixing it though. My main problem was that
I couldn't even find out which types exactly are supported, as __uint128_t
is not even in the gcc documentation. "unsigned __int128" is a documented
type, but is also not in genksyms.

        Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Will Deacon]

On Mon, Jun 17, 2019 at 02:21:46PM +0200, Arnd Bergmann wrote:
> On Mon, Jun 17, 2019 at 1:26 PM Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Jun 17, 2019 at 12:42:11PM +0200, Arnd Bergmann wrote:
> > > genksyms does not understand __uint128_t, so we get a build failure
> > > in the fpsimd module when it cannot export a symbol right:
> >
> > The fpsimd code is builtin, so which module is actually failing? My
> > allmodconfig build succeeds, so I must be missing something.
> 
> It happened for me on randconfig builds, you can find one such configuration
> at https://pastebin.com/cU8iQ4ta now. I was building this with clang
> rather than gcc, which may affect the issue, but I assumed not.

Hmm, I've failed to reproduce the issue with that config and either GCC
(7.1.1 and 8.3.0) or Clang (a flavour of 9.0.0 from a few months ago).

> > > WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version generation failed, symbol will not be versioned.
> > > /home/arnd/cross/x86_64/gcc-8.1.0-nolibc/aarch64-linux/bin/aarch64-linux-ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against `__crc_kernel_neon_begin' can not be used when making a shared object
> > > arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation: unsupported relocation
> > > arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x0): dangerous relocation: unsupported relocation
> > > arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x8): dangerous relocation: unsupported relocation
> > >
> > > We could teach genksyms about the type, but it's easier to just
> > > work around it by defining that type locally in a way that genksyms
> > > understands.
> > >
> > > Fixes: 41040cf7c5f0 ("arm64/sve: Fix missing SVE/FPSIMD endianness conversions")
> >
> > I can't see which part of that patch causes the problem, so I'm a bit wary
> > of the fix. We've been using __uint128_t for a while now, and I see there's
> > one in the x86 kvm code as well, so it would be nice to understand what's
> > happening here so that we can avoid running into it in future as well.
> 
> The problem is only in files that export a symbol. This is also the
> case in arch/x86/kernel/kvm.c, but it may be lucky because the
> type only appears /after/ the last export in that file.
> 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > >  arch/arm64/kernel/fpsimd.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > > index 07f238ef47ae..2aba07cccf50 100644
> > > --- a/arch/arm64/kernel/fpsimd.c
> > > +++ b/arch/arm64/kernel/fpsimd.c
> > > @@ -400,6 +400,9 @@ static int __init sve_sysctl_init(void) { return 0; }
> > >  #define ZREG(sve_state, vq, n) ((char *)(sve_state) +                \
> > >       (SVE_SIG_ZREG_OFFSET(vq, n) - SVE_SIG_REGS_OFFSET))
> > >
> > > +#ifdef __GENKSYMS__
> > > +typedef __u64 __uint128_t[2];
> > > +#endif
> >
> > I suspect I need to figure out what genksyms is doing, but I'm nervous
> > about exposing this as an array type without understanding whether or
> > not that has consequences for its operation.
> 
> The entire point is genksyms is to ensure that types of exported symbols
> are compatible. To do this, it has a limited parser for C source code that
> understands the basic types (char, int, long, _Bool, etc) and how to
> aggregate them into structs and function arguments. This process has
> always been fragile, and it clearly breaks when it fails to understand a
> particular type.

Ok, but the patch that appears to cause this problem doesn't change the
type of anything we're exporting. The symbol in your log is
"kernel_neon_begin" which is:

	void kernel_neon_begin(void);

so I'm still fairly confused about the problem. In fact, even if I create
a silly:

	void will_kernel_neon_begin(__uint128_t);

function, then somehow I see it being processed:

	__crc_will_kernel_neon_begin = 0x5401d250;

Is there some way that your passing '-w' to genksyms?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Ard Biesheuvel]

On Mon, 17 Jun 2019 at 18:13, Will Deacon <will.deacon@arm.com> wrote:
>
> On Mon, Jun 17, 2019 at 02:21:46PM +0200, Arnd Bergmann wrote:
> > On Mon, Jun 17, 2019 at 1:26 PM Will Deacon <will.deacon@arm.com> wrote:
> > > On Mon, Jun 17, 2019 at 12:42:11PM +0200, Arnd Bergmann wrote:
> > > > genksyms does not understand __uint128_t, so we get a build failure
> > > > in the fpsimd module when it cannot export a symbol right:
> > >
> > > The fpsimd code is builtin, so which module is actually failing? My
> > > allmodconfig build succeeds, so I must be missing something.
> >
> > It happened for me on randconfig builds, you can find one such configuration
> > at https://pastebin.com/cU8iQ4ta now. I was building this with clang
> > rather than gcc, which may affect the issue, but I assumed not.
>
> Hmm, I've failed to reproduce the issue with that config and either GCC
> (7.1.1 and 8.3.0) or Clang (a flavour of 9.0.0 from a few months ago).
>
> > > > WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version generation failed, symbol will not be versioned.
> > > > /home/arnd/cross/x86_64/gcc-8.1.0-nolibc/aarch64-linux/bin/aarch64-linux-ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against `__crc_kernel_neon_begin' can not be used when making a shared object
> > > > arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation: unsupported relocation
> > > > arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x0): dangerous relocation: unsupported relocation
> > > > arch/arm64/kernel/fpsimd.o:(".discard.addressable"+0x8): dangerous relocation: unsupported relocation
> > > >
> > > > We could teach genksyms about the type, but it's easier to just
> > > > work around it by defining that type locally in a way that genksyms
> > > > understands.
> > > >
> > > > Fixes: 41040cf7c5f0 ("arm64/sve: Fix missing SVE/FPSIMD endianness conversions")
> > >
> > > I can't see which part of that patch causes the problem, so I'm a bit wary
> > > of the fix. We've been using __uint128_t for a while now, and I see there's
> > > one in the x86 kvm code as well, so it would be nice to understand what's
> > > happening here so that we can avoid running into it in future as well.
> >
> > The problem is only in files that export a symbol. This is also the
> > case in arch/x86/kernel/kvm.c, but it may be lucky because the
> > type only appears /after/ the last export in that file.
> >
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > ---
> > > >  arch/arm64/kernel/fpsimd.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > > > index 07f238ef47ae..2aba07cccf50 100644
> > > > --- a/arch/arm64/kernel/fpsimd.c
> > > > +++ b/arch/arm64/kernel/fpsimd.c
> > > > @@ -400,6 +400,9 @@ static int __init sve_sysctl_init(void) { return 0; }
> > > >  #define ZREG(sve_state, vq, n) ((char *)(sve_state) +                \
> > > >       (SVE_SIG_ZREG_OFFSET(vq, n) - SVE_SIG_REGS_OFFSET))
> > > >
> > > > +#ifdef __GENKSYMS__
> > > > +typedef __u64 __uint128_t[2];
> > > > +#endif
> > >
> > > I suspect I need to figure out what genksyms is doing, but I'm nervous
> > > about exposing this as an array type without understanding whether or
> > > not that has consequences for its operation.
> >
> > The entire point is genksyms is to ensure that types of exported symbols
> > are compatible. To do this, it has a limited parser for C source code that
> > understands the basic types (char, int, long, _Bool, etc) and how to
> > aggregate them into structs and function arguments. This process has
> > always been fragile, and it clearly breaks when it fails to understand a
> > particular type.
>
> Ok, but the patch that appears to cause this problem doesn't change the
> type of anything we're exporting. The symbol in your log is
> "kernel_neon_begin" which is:
>
>         void kernel_neon_begin(void);
>
> so I'm still fairly confused about the problem. In fact, even if I create
> a silly:
>
>         void will_kernel_neon_begin(__uint128_t);
>
> function, then somehow I see it being processed:
>
>         __crc_will_kernel_neon_begin = 0x5401d250;
>
> Is there some way that your passing '-w' to genksyms?
>

The problem is not about the types we're *exporting*. Genksyms just
gives up halfway through the file, as soon as it encounters something
it doesn't like, and any symbol that hasn't been encountered yet at
that point will not have a crc generated for it.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Will Deacon]

On Mon, Jun 17, 2019 at 06:32:16PM +0200, Ard Biesheuvel wrote:
> On Mon, 17 Jun 2019 at 18:13, Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Jun 17, 2019 at 02:21:46PM +0200, Arnd Bergmann wrote:
> > > On Mon, Jun 17, 2019 at 1:26 PM Will Deacon <will.deacon@arm.com> wrote:
> > > > On Mon, Jun 17, 2019 at 12:42:11PM +0200, Arnd Bergmann wrote:
> > > > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> > > > > index 07f238ef47ae..2aba07cccf50 100644
> > > > > --- a/arch/arm64/kernel/fpsimd.c
> > > > > +++ b/arch/arm64/kernel/fpsimd.c
> > > > > @@ -400,6 +400,9 @@ static int __init sve_sysctl_init(void) { return 0; }
> > > > >  #define ZREG(sve_state, vq, n) ((char *)(sve_state) +                \
> > > > >       (SVE_SIG_ZREG_OFFSET(vq, n) - SVE_SIG_REGS_OFFSET))
> > > > >
> > > > > +#ifdef __GENKSYMS__
> > > > > +typedef __u64 __uint128_t[2];
> > > > > +#endif
> > > >
> > > > I suspect I need to figure out what genksyms is doing, but I'm nervous
> > > > about exposing this as an array type without understanding whether or
> > > > not that has consequences for its operation.
> > >
> > > The entire point is genksyms is to ensure that types of exported symbols
> > > are compatible. To do this, it has a limited parser for C source code that
> > > understands the basic types (char, int, long, _Bool, etc) and how to
> > > aggregate them into structs and function arguments. This process has
> > > always been fragile, and it clearly breaks when it fails to understand a
> > > particular type.
> >
> > Ok, but the patch that appears to cause this problem doesn't change the
> > type of anything we're exporting. The symbol in your log is
> > "kernel_neon_begin" which is:
> >
> >         void kernel_neon_begin(void);
> >
> > so I'm still fairly confused about the problem. In fact, even if I create
> > a silly:
> >
> >         void will_kernel_neon_begin(__uint128_t);
> >
> > function, then somehow I see it being processed:
> >
> >         __crc_will_kernel_neon_begin = 0x5401d250;
> >
> > Is there some way that your passing '-w' to genksyms?
> >
> 
> The problem is not about the types we're *exporting*. Genksyms just
> gives up halfway through the file, as soon as it encounters something
> it doesn't like, and any symbol that hasn't been encountered yet at
> that point will not have a crc generated for it.

Hmm, but it appears to be either working or failing silently for me, which
doesn't match what Arnd is seeing. I'd prefer to fix genksyms but I'm not
happy touching it if I can't show it's broken to begin with. If I pass '-w'
I see it barfing on all sorts of random stuff, for example the static_assert
in include/linux/fs.h:

	static_assert(offsetof(struct filename, iname) % sizeof(long) == 0);

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Will Deacon]

Hi Arnd, Ard,

On Mon, Jun 17, 2019 at 05:45:56PM +0100, Will Deacon wrote:
> On Mon, Jun 17, 2019 at 06:32:16PM +0200, Ard Biesheuvel wrote:
> > The problem is not about the types we're *exporting*. Genksyms just
> > gives up halfway through the file, as soon as it encounters something
> > it doesn't like, and any symbol that hasn't been encountered yet at
> > that point will not have a crc generated for it.
> 
> Hmm, but it appears to be either working or failing silently for me, which
> doesn't match what Arnd is seeing. I'd prefer to fix genksyms but I'm not
> happy touching it if I can't show it's broken to begin with. If I pass '-w'
> I see it barfing on all sorts of random stuff, for example the static_assert
> in include/linux/fs.h:
> 
> 	static_assert(offsetof(struct filename, iname) % sizeof(long) == 0);

Ok, I had some more fun with this today. First of all, we needed a new
.config, but also, the issue only appears with linux-next. With that
configuration, I can hit the issue.

What seems to occur is that when parsing:

	static __uint128_t arm64_cpu_to_le128(__uint128_t x)
	{
		u64 a = swab64(x);
		u64 b = swab64(x >> 64);

		return ((__uint128_t)a << 64) | b;
	}

in fpsimd.c, then genksyms doesn't treate __uint128_t as a type and
therefore fails to figure out that this is a function. Consequently, it
keeps trying (and failing) to parse until it sees the end of the current
expression. This happens when it hits:

	EXPORT_SYMBOL(kernel_neon_begin);

thanks to the semi-colon.

So one nasty bodge to fix this is:


diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index bb42cd04baec..693a978f41f9 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -368,6 +368,8 @@ static __uint128_t arm64_cpu_to_le128(__uint128_t x)
 }
 #endif
 
+short of_fixing_genksyms_we_must_use_this_hack;
+
 #define arm64_le128_to_cpu(x) arm64_cpu_to_le128(x)
 
 /*


but actually, I think I've managed to hack genksyms itself. Patch below.

Will

--->8

From 6e004b8824d4eb6a4e61cd794fbc3a761b50135b Mon Sep 17 00:00:00 2001
From: Will Deacon <will.deacon@arm.com>
Date: Tue, 18 Jun 2019 12:56:49 +0100
Subject: [PATCH] genksyms: Teach parse about __uint128_t built-in type

__uint128_t crops up in a few files that export symbols to modules, so
teach genksyms about it so that we don't end up skipping the CRC
generation for some symbols due to the parser failing to spot them:

  | WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version
  |          generation failed, symbol will not be versioned.
  | ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against
  |     `__crc_kernel_neon_begin' can not be used when making a shared
  |     object
  | ld: arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation:
  |     unsupported relocation

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 scripts/genksyms/keywords.c | 1 +
 scripts/genksyms/parse.y    | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/scripts/genksyms/keywords.c b/scripts/genksyms/keywords.c
index 9f40bcd17d07..6ec585febfa4 100644
--- a/scripts/genksyms/keywords.c
+++ b/scripts/genksyms/keywords.c
@@ -53,6 +53,7 @@ static struct resword {
 	{ "struct", STRUCT_KEYW },
 	{ "typedef", TYPEDEF_KEYW },
 	{ "typeof", TYPEOF_KEYW },
+	{ "__uint128_t", BUILTIN_INT_KEYW },
 	{ "union", UNION_KEYW },
 	{ "unsigned", UNSIGNED_KEYW },
 	{ "void", VOID_KEYW },
diff --git a/scripts/genksyms/parse.y b/scripts/genksyms/parse.y
index 00a6d7e54971..1ebcf52cd0f9 100644
--- a/scripts/genksyms/parse.y
+++ b/scripts/genksyms/parse.y
@@ -76,6 +76,7 @@ static void record_compound(struct string_list **keyw,
 %token ATTRIBUTE_KEYW
 %token AUTO_KEYW
 %token BOOL_KEYW
+%token BUILTIN_INT_KEYW
 %token CHAR_KEYW
 %token CONST_KEYW
 %token DOUBLE_KEYW
@@ -263,6 +264,7 @@ simple_type_specifier:
 	| VOID_KEYW
 	| BOOL_KEYW
 	| VA_LIST_KEYW
+	| BUILTIN_INT_KEYW
 	| TYPE			{ (*$1)->tag = SYM_TYPEDEF; $$ = $1; }
 	;
 
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Arnd Bergmann]

On Tue, Jun 18, 2019 at 2:03 PM Will Deacon <will.deacon@arm.com> wrote:
>
> From 6e004b8824d4eb6a4e61cd794fbc3a761b50135b Mon Sep 17 00:00:00 2001
> From: Will Deacon <will.deacon@arm.com>
> Date: Tue, 18 Jun 2019 12:56:49 +0100
> Subject: [PATCH] genksyms: Teach parse about __uint128_t built-in type
>
> __uint128_t crops up in a few files that export symbols to modules, so
> teach genksyms about it so that we don't end up skipping the CRC
> generation for some symbols due to the parser failing to spot them:
>
>   | WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version
>   |          generation failed, symbol will not be versioned.
>   | ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against
>   |     `__crc_kernel_neon_begin' can not be used when making a shared
>   |     object
>   | ld: arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation:
>   |     unsupported relocation
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Looks good to me,

Acked-by: Arnd Bergmann <arnd@arndb.de>

I've added this to my randconfig build setup, replacing my earlier
patch, and will let you know if any problems with it remain.

         Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Laura Abbott]

On 6/18/19 10:15 AM, Arnd Bergmann wrote:
> On Tue, Jun 18, 2019 at 2:03 PM Will Deacon <will.deacon@arm.com> wrote:
>>
>>  From 6e004b8824d4eb6a4e61cd794fbc3a761b50135b Mon Sep 17 00:00:00 2001
>> From: Will Deacon <will.deacon@arm.com>
>> Date: Tue, 18 Jun 2019 12:56:49 +0100
>> Subject: [PATCH] genksyms: Teach parse about __uint128_t built-in type
>>
>> __uint128_t crops up in a few files that export symbols to modules, so
>> teach genksyms about it so that we don't end up skipping the CRC
>> generation for some symbols due to the parser failing to spot them:
>>
>>    | WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version
>>    |          generation failed, symbol will not be versioned.
>>    | ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against
>>    |     `__crc_kernel_neon_begin' can not be used when making a shared
>>    |     object
>>    | ld: arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation:
>>    |     unsupported relocation
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> Looks good to me,
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> I've added this to my randconfig build setup, replacing my earlier
> patch, and will let you know if any problems with it remain.
> 
>           Arnd
> 

I just hit this on 5ad18b2e60b75c7297a998dea702451d33a052ed on Linus'
branch. The fix works for me (feel free to add Tested-by). Is this
already queued up for Linus?

Thanks,
Laura

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Will Deacon]

Hi Laura, [+Masahiro]

On Tue, Jul 09, 2019 at 03:06:49PM -0400, Laura Abbott wrote:
> On 6/18/19 10:15 AM, Arnd Bergmann wrote:
> > On Tue, Jun 18, 2019 at 2:03 PM Will Deacon <will.deacon@arm.com> wrote:
> > >  From 6e004b8824d4eb6a4e61cd794fbc3a761b50135b Mon Sep 17 00:00:00 2001
> > > From: Will Deacon <will.deacon@arm.com>
> > > Date: Tue, 18 Jun 2019 12:56:49 +0100
> > > Subject: [PATCH] genksyms: Teach parse about __uint128_t built-in type
> > > 
> > > __uint128_t crops up in a few files that export symbols to modules, so
> > > teach genksyms about it so that we don't end up skipping the CRC
> > > generation for some symbols due to the parser failing to spot them:
> > > 
> > >    | WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version
> > >    |          generation failed, symbol will not be versioned.
> > >    | ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against
> > >    |     `__crc_kernel_neon_begin' can not be used when making a shared
> > >    |     object
> > >    | ld: arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation:
> > >    |     unsupported relocation
> > > 
> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Reported-by: Arnd Bergmann <arnd@arndb.de>
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > 
> > Looks good to me,
> > 
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > 
> > I've added this to my randconfig build setup, replacing my earlier
> > patch, and will let you know if any problems with it remain.
> > 
> 
> I just hit this on 5ad18b2e60b75c7297a998dea702451d33a052ed on Linus'
> branch. The fix works for me (feel free to add Tested-by). Is this
> already queued up for Linus?

I think there's a fix queued via the kbuild tree:

https://lkml.kernel.org/r/CAK7LNAQRMnovWQA0F8A6mEqDjPxXOGM-1AHoyh1gQa367f+FqQ@mail.gmail.com

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64/sve: fix genksyms generation

[Masahiro Yamada]

On Wed, Jul 10, 2019 at 5:15 PM Will Deacon <will@kernel.org> wrote:
>
> Hi Laura, [+Masahiro]
>
> On Tue, Jul 09, 2019 at 03:06:49PM -0400, Laura Abbott wrote:
> > On 6/18/19 10:15 AM, Arnd Bergmann wrote:
> > > On Tue, Jun 18, 2019 at 2:03 PM Will Deacon <will.deacon@arm.com> wrote:
> > > >  From 6e004b8824d4eb6a4e61cd794fbc3a761b50135b Mon Sep 17 00:00:00 2001
> > > > From: Will Deacon <will.deacon@arm.com>
> > > > Date: Tue, 18 Jun 2019 12:56:49 +0100
> > > > Subject: [PATCH] genksyms: Teach parse about __uint128_t built-in type
> > > >
> > > > __uint128_t crops up in a few files that export symbols to modules, so
> > > > teach genksyms about it so that we don't end up skipping the CRC
> > > > generation for some symbols due to the parser failing to spot them:
> > > >
> > > >    | WARNING: EXPORT symbol "kernel_neon_begin" [vmlinux] version
> > > >    |          generation failed, symbol will not be versioned.
> > > >    | ld: arch/arm64/kernel/fpsimd.o: relocation R_AARCH64_ABS32 against
> > > >    |     `__crc_kernel_neon_begin' can not be used when making a shared
> > > >    |     object
> > > >    | ld: arch/arm64/kernel/fpsimd.o:(.data+0x0): dangerous relocation:
> > > >    |     unsupported relocation
> > > >
> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > Reported-by: Arnd Bergmann <arnd@arndb.de>
> > > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > >
> > > Looks good to me,
> > >
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > >
> > > I've added this to my randconfig build setup, replacing my earlier
> > > patch, and will let you know if any problems with it remain.
> > >
> >
> > I just hit this on 5ad18b2e60b75c7297a998dea702451d33a052ed on Linus'
> > branch. The fix works for me (feel free to add Tested-by). Is this
> > already queued up for Linus?
>
> I think there's a fix queued via the kbuild tree:
>
> https://lkml.kernel.org/r/CAK7LNAQRMnovWQA0F8A6mEqDjPxXOGM-1AHoyh1gQa367f+FqQ@mail.gmail.com
>
> Will


Yes, I will send a pull request shortly.



-- 
Best Regards
Masahiro Yamada

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel