| Message ID | 20190617104237.2082388-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | arm64/sve: fix genksyms generation | expand |
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
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
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
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
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
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.
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
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
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
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
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
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
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) {
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(+) -- 2.20.0