The arm64 defconfig build failed with gcc-8 and passed with gcc-13.
Reported-by: Linux Kernel Functional Testing <[email protected]>
Build log:
---
/tmp/ccSUNNZy.s: Assembler messages:
/tmp/ccSUNNZy.s:3159: Error: unknown architectural extension `pauth'
make[5]: *** [scripts/Makefile.build:244: arch/arm64/kvm/pauth.o] Error 1
Steps to reproduce:
---
# tuxmake --runtime podman --target-arch arm64 --toolchain gcc-8
--kconfig defconfig
Links:
- https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20240422/testrun/23551634/suite/build/test/gcc-8-defconfig/details/
- https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20240422/testrun/23551634/suite/build/test/gcc-8-defconfig/log
- https://storage.tuxsuite.com/public/linaro/lkft/builds/2fRe0ZWWmise7cetIz0aXdnq4jJ/
--
Linaro LKFT
https://lkft.linaro.org
On Mon, Apr 22, 2024 at 02:04:43PM +0530, Naresh Kamboju wrote:
> The arm64 defconfig build failed with gcc-8 and passed with gcc-13.
>
> Reported-by: Linux Kernel Functional Testing <[email protected]>
>
> Build log:
> ---
> /tmp/ccSUNNZy.s: Assembler messages:
> /tmp/ccSUNNZy.s:3159: Error: unknown architectural extension `pauth'
> make[5]: *** [scripts/Makefile.build:244: arch/arm64/kvm/pauth.o] Error 1
>
> Steps to reproduce:
> ---
> # tuxmake --runtime podman --target-arch arm64 --toolchain gcc-8
> --kconfig defconfig
I think the key thing here is GCC 8; the associated assembler won't necessarily
have ARMv8.3-A support, since all the relevant bits got added around GCC 9.
Looking at the commits, I think this is broken since its introduction in commit:
6ccc971ee2c61a1f ("KVM: arm64: nv: Add emulation for ERETAx instructions")
.. where the pauth.c file only depends on ARM64_PTR_AUTH (which doesn't imply
AS_HAS_ARMV8_3), but in the file we do:
asm volatile(ARM64_ASM_PREAMBLE ".arch_extension pauth\n"
"pacga %0, %1, %2" : "=r" (pac) : "r" (ptr), "r" (mod));
Given the minimum supported toolchain comes with an assembler that doesn't
necessarily support ARMv8.3, I reckon we'll either have to make NV pauth
support depend upon AS_HAS_ARMV8_3, or manually assemble the PACGA instruction.
I suspect the latter is the better option.
Mark.
>
> Links:
> - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20240422/testrun/23551634/suite/build/test/gcc-8-defconfig/details/
> - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20240422/testrun/23551634/suite/build/test/gcc-8-defconfig/log
> - https://storage.tuxsuite.com/public/linaro/lkft/builds/2fRe0ZWWmise7cetIz0aXdnq4jJ/
>
> --
> Linaro LKFT
> https://lkft.linaro.org
>
On Mon, Apr 22, 2024, at 11:13, Mark Rutland wrote:
> On Mon, Apr 22, 2024 at 02:04:43PM +0530, Naresh Kamboju wrote:
> Given the minimum supported toolchain comes with an assembler that doesn't
> necessarily support ARMv8.3, I reckon we'll either have to make NV pauth
> support depend upon AS_HAS_ARMV8_3, or manually assemble the PACGA instruction.
>
> I suspect the latter is the better option.
The .config linked from the report shows
CONFIG_AS_VERSION=23101
CONFIG_ARM64_PTR_AUTH_KERNEL=y
CONFIG_AS_HAS_ARMV8_3=y
So it gets detected as supporting ARMv8.3. Is this the wrong
conditional to check, or does it get misdetected for an unsupported
assembler?
Arnd
On Mon, Apr 22, 2024 at 11:25:25AM +0200, Arnd Bergmann wrote:
> On Mon, Apr 22, 2024, at 11:13, Mark Rutland wrote:
> > On Mon, Apr 22, 2024 at 02:04:43PM +0530, Naresh Kamboju wrote:
> > Given the minimum supported toolchain comes with an assembler that doesn't
> > necessarily support ARMv8.3, I reckon we'll either have to make NV pauth
> > support depend upon AS_HAS_ARMV8_3, or manually assemble the PACGA instruction.
> >
> > I suspect the latter is the better option.
>
> The .config linked from the report shows
>
> CONFIG_AS_VERSION=23101
> CONFIG_ARM64_PTR_AUTH_KERNEL=y
> CONFIG_AS_HAS_ARMV8_3=y
>
> So it gets detected as supporting ARMv8.3. Is this the wrong
> conditional to check, or does it get misdetected for an unsupported
> assembler?
I suspect that means the 'pauth' arch extension was added after armv8.3
support, and the assembler supports `-march=armv8.3-a` but does not support
`.arch_extension pauth`. So for this code, it'd be wrong to check for
AS_HAS_ARMV8_3, unless we used `.march armv8.3-a`, but even then that'd still
mean configurations where we couldn't support this code.
I reckon manually assembing the PACGA is the best thing to do; that sidesteps
the need for either `.arch_extension pauth` or `.march armv8.3-a`, and aligns
with what we do for CONFIG_ARM64_PTR_AUTH=y generally.
Elsewhere in the kernel where we check for CONFIG_AS_HAS_ARMV8_3, we rely on
ARM64_ASM_PREAMBLE containing `.arch armv8.3-a` or a later version that implies
the presence of ARMv8.3-A instructions, and so pauth usage elsewhere is fine.
Mark.
On Mon, Apr 22, 2024, at 11:40, Mark Rutland wrote:
> On Mon, Apr 22, 2024 at 11:25:25AM +0200, Arnd Bergmann wrote:
>> On Mon, Apr 22, 2024, at 11:13, Mark Rutland wrote:
>> > On Mon, Apr 22, 2024 at 02:04:43PM +0530, Naresh Kamboju wrote:
>> > Given the minimum supported toolchain comes with an assembler that doesn't
>> > necessarily support ARMv8.3, I reckon we'll either have to make NV pauth
>> > support depend upon AS_HAS_ARMV8_3, or manually assemble the PACGA instruction.
>> >
>> > I suspect the latter is the better option.
>>
>> The .config linked from the report shows
>>
>> CONFIG_AS_VERSION=23101
>> CONFIG_ARM64_PTR_AUTH_KERNEL=y
>> CONFIG_AS_HAS_ARMV8_3=y
>>
>> So it gets detected as supporting ARMv8.3. Is this the wrong
>> conditional to check, or does it get misdetected for an unsupported
>> assembler?
>
> I suspect that means the 'pauth' arch extension was added after armv8.3
> support, and the assembler supports `-march=armv8.3-a` but does not support
> `.arch_extension pauth`. So for this code, it'd be wrong to check for
> AS_HAS_ARMV8_3, unless we used `.march armv8.3-a`, but even then that'd still
> mean configurations where we couldn't support this code.
>
> I reckon manually assembing the PACGA is the best thing to do; that sidesteps
> the need for either `.arch_extension pauth` or `.march armv8.3-a`, and aligns
> with what we do for CONFIG_ARM64_PTR_AUTH=y generally.
>
> Elsewhere in the kernel where we check for CONFIG_AS_HAS_ARMV8_3, we rely on
> ARM64_ASM_PREAMBLE containing `.arch armv8.3-a` or a later version that implies
> the presence of ARMv8.3-A instructions, and so pauth usage elsewhere is fine.
I tested with the old binutils versions I have here and found
that anything that supports v8.3 also understands pacga, but
'.arch_extension pauth' only works in binutils-2.35 and higher,
presumably because it started out as a v8.3+ feature but was
later turned into an optional extension for all versions.
Since there is a Kconfig check for armv8.3-a support already, I think
it's safe to just drop the .arch_extension pauth.
Arnd
On Mon, Apr 22, 2024 at 02:11:05PM +0200, Arnd Bergmann wrote:
> On Mon, Apr 22, 2024, at 11:40, Mark Rutland wrote:
> > On Mon, Apr 22, 2024 at 11:25:25AM +0200, Arnd Bergmann wrote:
> >> On Mon, Apr 22, 2024, at 11:13, Mark Rutland wrote:
> >> > On Mon, Apr 22, 2024 at 02:04:43PM +0530, Naresh Kamboju wrote:
> >> > Given the minimum supported toolchain comes with an assembler that doesn't
> >> > necessarily support ARMv8.3, I reckon we'll either have to make NV pauth
> >> > support depend upon AS_HAS_ARMV8_3, or manually assemble the PACGA instruction.
> >> >
> >> > I suspect the latter is the better option.
> >>
> >> The .config linked from the report shows
> >>
> >> CONFIG_AS_VERSION=23101
> >> CONFIG_ARM64_PTR_AUTH_KERNEL=y
> >> CONFIG_AS_HAS_ARMV8_3=y
> >>
> >> So it gets detected as supporting ARMv8.3. Is this the wrong
> >> conditional to check, or does it get misdetected for an unsupported
> >> assembler?
> >
> > I suspect that means the 'pauth' arch extension was added after armv8.3
> > support, and the assembler supports `-march=armv8.3-a` but does not support
> > `.arch_extension pauth`. So for this code, it'd be wrong to check for
> > AS_HAS_ARMV8_3, unless we used `.march armv8.3-a`, but even then that'd still
> > mean configurations where we couldn't support this code.
> >
> > I reckon manually assembing the PACGA is the best thing to do; that sidesteps
> > the need for either `.arch_extension pauth` or `.march armv8.3-a`, and aligns
> > with what we do for CONFIG_ARM64_PTR_AUTH=y generally.
> >
> > Elsewhere in the kernel where we check for CONFIG_AS_HAS_ARMV8_3, we rely on
> > ARM64_ASM_PREAMBLE containing `.arch armv8.3-a` or a later version that implies
> > the presence of ARMv8.3-A instructions, and so pauth usage elsewhere is fine.
>
> I tested with the old binutils versions I have here and found
> that anything that supports v8.3 also understands pacga, but
> '.arch_extension pauth' only works in binutils-2.35 and higher,
> presumably because it started out as a v8.3+ feature but was
> later turned into an optional extension for all versions.
>
> Since there is a Kconfig check for armv8.3-a support already, I think
> it's safe to just drop the .arch_extension pauth.
That'll be safe, but it does mean that we'd need to *not* support pointer auth
for nested virt when we have a toolchain for which CONFIG_AS_HAS_ARMV8_3=n,
unless our minimum supported AS supports ARMv8.3.
If our minimum supported AS *doesn't* support ARMv8.3, then we'd either need a
new Kconfig symbol for NV_PAUTH support, or make CONFIG_ARM64_PTR_AUTH depend
upon CONFIG_AS_HAS_ARMV8_3.
AFAICT our options are:
(a) Manually assembly PACGA
(b) Change CONFIG_ARM64_PTR_AUTH to depend upon CONFIG_AS_HAS_ARMV8_3=y
(c) Add and use new Kconfig symbol for NV PAUTH, dependent upon
CONFIG_AS_HAS_ARMV8_3=y
(d) Bump the minimum supported version of AS so that we can depend upon ARMv8.3
support, and just open-code the ".arch armv8.3-a" in the NV pauth code.
.. and maybe some variations on that.
Mark.
On Mon, Apr 22, 2024, at 19:20, Mark Rutland wrote:
> On Mon, Apr 22, 2024 at 02:11:05PM +0200, Arnd Bergmann wrote:
>> On Mon, Apr 22, 2024, at 11:40, Mark Rutland wrote:
>> > On Mon, Apr 22, 2024 at 11:25:25AM +0200, Arnd Bergmann wrote:
>> >> On Mon, Apr 22, 2024, at 11:13, Mark Rutland wrote:
>> > the presence of ARMv8.3-A instructions, and so pauth usage elsewhere is fine.
>>
>> I tested with the old binutils versions I have here and found
>> that anything that supports v8.3 also understands pacga, but
>> '.arch_extension pauth' only works in binutils-2.35 and higher,
>> presumably because it started out as a v8.3+ feature but was
>> later turned into an optional extension for all versions.
>>
>> Since there is a Kconfig check for armv8.3-a support already, I think
>> it's safe to just drop the .arch_extension pauth.
>
> That'll be safe, but it does mean that we'd need to *not* support pointer auth
> for nested virt when we have a toolchain for which CONFIG_AS_HAS_ARMV8_3=n,
> unless our minimum supported AS supports ARMv8.3.
>
> If our minimum supported AS *doesn't* support ARMv8.3, then we'd either need a
> new Kconfig symbol for NV_PAUTH support, or make CONFIG_ARM64_PTR_AUTH depend
> upon CONFIG_AS_HAS_ARMV8_3.
>
> AFAICT our options are:
>
> (a) Manually assembly PACGA
>
> (b) Change CONFIG_ARM64_PTR_AUTH to depend upon CONFIG_AS_HAS_ARMV8_3=y
>
> (c) Add and use new Kconfig symbol for NV PAUTH, dependent upon
> CONFIG_AS_HAS_ARMV8_3=y
>
> (d) Bump the minimum supported version of AS so that we can depend upon ARMv8.3
> support, and just open-code the ".arch armv8.3-a" in the NV pauth code.
>
> ... and maybe some variations on that.
Right, I misread the current Kconfig logic and assumed that
we already do b), but it's conly CONFIG_ARM64_PTR_AUTH_KERNEL
that depends on CONFIG_AS_HAS_ARMV8_3, not the
more global CONFIG_ARM64_PTR_AUTH.
I think any of the above would be fine. I see that binutils-2.28
was the first version to include armv8.3-a support back in 2016.
The only distros that are old enough to ship something older
than that (rhel 7, sles 12, debian 8) also ship with a gcc version
that no longer builds the kernel, so doing b) would be the minimum
effort fix and still have very little risk of anyone having to
turn it off.
Arnd
On Mon, 22 Apr 2024 10:13:59 +0100,
Mark Rutland <[email protected]> wrote:
>
> On Mon, Apr 22, 2024 at 02:04:43PM +0530, Naresh Kamboju wrote:
> > The arm64 defconfig build failed with gcc-8 and passed with gcc-13.
> >
> > Reported-by: Linux Kernel Functional Testing <[email protected]>
> >
> > Build log:
> > ---
> > /tmp/ccSUNNZy.s: Assembler messages:
> > /tmp/ccSUNNZy.s:3159: Error: unknown architectural extension `pauth'
> > make[5]: *** [scripts/Makefile.build:244: arch/arm64/kvm/pauth.o] Error 1
> >
> > Steps to reproduce:
> > ---
> > # tuxmake --runtime podman --target-arch arm64 --toolchain gcc-8
> > --kconfig defconfig
>
> I think the key thing here is GCC 8; the associated assembler won't necessarily
> have ARMv8.3-A support, since all the relevant bits got added around GCC 9.
>
> Looking at the commits, I think this is broken since its introduction in commit:
>
> 6ccc971ee2c61a1f ("KVM: arm64: nv: Add emulation for ERETAx instructions")
>
> ... where the pauth.c file only depends on ARM64_PTR_AUTH (which doesn't imply
> AS_HAS_ARMV8_3), but in the file we do:
>
> asm volatile(ARM64_ASM_PREAMBLE ".arch_extension pauth\n"
> "pacga %0, %1, %2" : "=r" (pac) : "r" (ptr), "r" (mod));
>
> Given the minimum supported toolchain comes with an assembler that doesn't
> necessarily support ARMv8.3, I reckon we'll either have to make NV pauth
> support depend upon AS_HAS_ARMV8_3, or manually assemble the PACGA instruction.
>
> I suspect the latter is the better option.
This is what I've done [1]. It certainly isn't pretty, but this counts
as an incentive to drop some of the old stuff in the near-ish future.
I've pushed this out to -next, and hopefully the breakage will be
solved tomorrow.
M.
[1] https://lore.kernel.org/r/[email protected]
--
Without deviation from the norm, progress is not possible.