arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
PATCH 1/1 arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
From: Markus Mayer <[email protected]>
The new errata check leads to a warning with some older versions of the linker that do know how to work around the errata, but still use the original name of the command line option: --fix-cortex-a53. The commit in question that changed the name of the option can be found at [1]. It looks like only "gold" is affected by this rename. Traditional "ld" isn't. (There, the argument was always called --fix-cortex-a53-843419.)
To allow older versions of gold to properly handle the erratum if they can, check whether ld supports the old name of this option in addition to checking the new one.
---
Using the change proposed here, things work for me as expected:
$ aarch64-linux-ld -v GNU ld (GNU Binutils) Linaro 2014.11-2 2.24.0.20141017
$ grep fix-cortex aarch64.log /bin/bash scripts/link-vmlinux.sh aarch64-linux-ld -EL -p --no-undefined -X --fix-cortex-a53 --build-id ; true + aarch64-linux-ld -EL -p --no-undefined -X --fix-cortex-a53 --build-id -o .tmp_vmlinux1 -T ./arch/arm64/kernel/vmlinux.lds arch/arm64/kernel/head.o init/built-in.o --start-group usr/built-in.o arch/arm64/kernel/built-in.o arch/arm64/mm/built-in.o arch/arm64/net/built-in.o arch/arm64/kvm/built-in.o arch/arm64/xen/built-in.o arch/arm64/crypto/built-in.o ./drivers/firmware/efi/libstub/lib.a kernel/built-in.o certs/built-in.o mm/built-in.o fs/built-in.o ipc/built-in.o security/built-in.o crypto/built-in.o block/built-in.o arch/arm64/lib/lib.a lib/lib.a arch/arm64/lib/built-in.o lib/built-in.o drivers/built-in.o sound/built-in.o firmware/built-in.o net/built-in.o virt/built-in.o --end-group [...]
arch/arm64/Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
arch/arm64/Makefile +7 -1
@@ -20,7 +20,13 @@ endif ifeq ($(CONFIG_ARM64_ERRATUM_843419),y) ifeq ($(call ld-option, --fix-cortex-a53-843419),) -$(warning ld does not support --fix-cortex-a53-843419; kernel may be susceptible to erratum) + ifeq ($(call ld-option, --fix-cortex-a53),) +$(warning ld does not support --fix-cortex-a53-843419 or --fix-cortex-a53; kernel may be susceptible to erratum) + else +# When this option was first introduced, it was called --fix-cortex-a53 in gold. +# So, let's use that as fall-back if the linker understands it. +LDFLAGS_vmlinux += --fix-cortex-a53 + endif else LDFLAGS_vmlinux += --fix-cortex-a53-843419 endif
Signature · 1 line
2.7.4
Quoted 9 lines · Markus Mayer
From: Markus Mayer <[email protected]>
The new errata check leads to a warning with some older versions of the linker that do know how to work around the errata, but still use the original name of the command line option: --fix-cortex-a53. The commit in question that changed the name of the option can be found at [1]. It looks like only "gold" is affected by this rename. Traditional "ld" isn't. (There, the argument was always called --fix-cortex-a53-843419.)
Quoted 7 lines
To allow older versions of gold to properly handle the erratum if they can, check whether ld supports the old name of this option in addition to checking the new one.
Signed-off-by Markus Mayer <[email protected]>
If newer versions of gold accept the correct option name, why do we care?
Will
Quoted 14 lines · Will Deacon
Quoted 13 lines · Markus Mayer
From: Markus Mayer <[email protected]>
The new errata check leads to a warning with some older versions of the linker that do know how to work around the errata, but still use the original name of the command line option: --fix-cortex-a53. The commit in question that changed the name of the option can be found at [1]. It looks like only "gold" is affected by this rename. Traditional "ld" isn't. (There, the argument was always called --fix-cortex-a53-843419.)
To allow older versions of gold to properly handle the erratum if they can, check whether ld supports the old name of this option in addition to checking the new one.
Quoted 5 lines
Quoted 3 lines
Signed-off-by Markus Mayer <[email protected]>If newer versions of gold accept the correct option name, why do we care?
Because Documentation/Changes states that the minimum requirement for binutils is 2.12. Right now, that is not really true. And not everybody can always use the newest toolchain, for various reasons.
The question I am asking is: What do we have to lose by supporting both options?
Thanks,
-Markus
Quoted 17 lines · Markus Mayer
Quoted 16 lines · Will Deacon
Quoted 15 lines · Markus Mayer
From: Markus Mayer <[email protected]>
The new errata check leads to a warning with some older versions of the linker that do know how to work around the errata, but still use the original name of the command line option: --fix-cortex-a53. The commit in question that changed the name of the option can be found at [1]. It looks like only "gold" is affected by this rename. Traditional "ld" isn't. (There, the argument was always called --fix-cortex-a53-843419.)
To allow older versions of gold to properly handle the erratum if they can, check whether ld supports the old name of this option in addition to checking the new one.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
Signed-off-by Markus Mayer <[email protected]>If newer versions of gold accept the correct option name, why do we care?
Because
Documentation/Changesstates that the minimum requirement for binutils is 2.12. Right now, that is not really true. And not everybody can always use the newest toolchain, for various reasons.
Well the kernel still builds, right? Can binutils 2.12 even work around 843419? For people who can't use a recent toolchain, then they don't get erratum workaround and we warn them about it.
Quoted 1 line
The question I am asking is: What do we have to lose by supporting both options?
We end up passing "--fix-cortex-a53" to the linker, without knowing what it might do in the future.
Will
Quoted 22 lines · Will Deacon
Quoted 21 lines · Markus Mayer
Quoted 19 lines · Will Deacon
Quoted 9 lines · Markus Mayer
From: Markus Mayer <[email protected]>
The new errata check leads to a warning with some older versions of the linker that do know how to work around the errata, but still use the original name of the command line option: --fix-cortex-a53. The commit in question that changed the name of the option can be found at [1]. It looks like only "gold" is affected by this rename. Traditional "ld" isn't. (There, the argument was always called --fix-cortex-a53-843419.)
Quoted 7 lines
To allow older versions of gold to properly handle the erratum if they can, check whether ld supports the old name of this option in addition to checking the new one.
Signed-off-by Markus Mayer <[email protected]>If newer versions of gold accept the correct option name, why do we care?
Quoted 7 lines
Quoted 3 lines
Because
Documentation/Changesstates that the minimum requirement for binutils is 2.12. Right now, that is not really true. And not everybody can always use the newest toolchain, for various reasons.Well the kernel still builds, right? Can binutils 2.12 even work around 843419? For people who can't use a recent toolchain, then they don't get erratum workaround and we warn them about it.
Correct. Linkers as old as 2.12 are not able to work around the erratum, and the warning is accurate. So, there's no problem there.
But there are newer versions that do know how to work around the erratum, but use the original name of the option. Right now, you could be using a linker that knows how to fix the erratum, but instead, the you get a warning, and the erratum is not fixed.
This one, for instance:
$ arm-linux-ld -v GNU ld (GNU Binutils) Linaro 2014.11-2 2.24.0.20141017
Without the proposed patch, this linker will produce a kernel without the workaround, but not because the linker can't do it, but because it isn't given the command line argument it understands.
Quoted 4 lines
Quoted 1 line
The question I am asking is: What do we have to lose by supporting both options?
We end up passing "--fix-cortex-a53" to the linker, without knowing what it might do in the future.
It seems highly unlikely that such a generic option would be added in the future, both, because the precedent has been set for topic specific options, and because they know it has been used in the past, so they wouldn't add a previously used option to do something completely different. (And if they really did, then that would be a huge binutils bug.)
So, we have a trade-off between a real world problem that does currently exist and avoiding a theoretical issue that may never materialize.
Regards,
-Markus
Quoted 1 line
Will
Quoted 60 lines · Markus Mayer
Quoted 22 lines · Will Deacon
Quoted 21 lines · Markus Mayer
Quoted 19 lines · Will Deacon
Quoted 9 lines · Markus Mayer
From: Markus Mayer <[email protected]>
The new errata check leads to a warning with some older versions of the linker that do know how to work around the errata, but still use the original name of the command line option: --fix-cortex-a53. The commit in question that changed the name of the option can be found at [1]. It looks like only "gold" is affected by this rename. Traditional "ld" isn't. (There, the argument was always called --fix-cortex-a53-843419.)
Quoted 7 lines
To allow older versions of gold to properly handle the erratum if they can, check whether ld supports the old name of this option in addition to checking the new one.
Signed-off-by Markus Mayer <[email protected]>If newer versions of gold accept the correct option name, why do we care?
Quoted 7 lines
Quoted 3 lines
Because
Documentation/Changesstates that the minimum requirement for binutils is 2.12. Right now, that is not really true. And not everybody can always use the newest toolchain, for various reasons.Well the kernel still builds, right? Can binutils 2.12 even work around 843419? For people who can't use a recent toolchain, then they don't get erratum workaround and we warn them about it.
Correct. Linkers as old as 2.12 are not able to work around the erratum, and the warning is accurate. So, there's no problem there.
But there are newer versions that do know how to work around the erratum, but use the original name of the option. Right now, you could be using a linker that knows how to fix the erratum, but instead, the you get a warning, and the erratum is not fixed.
This one, for instance:
$ arm-linux-ld -v GNU ld (GNU Binutils) Linaro 2014.11-2 2.24.0.20141017Without the proposed patch, this linker will produce a kernel without the workaround, but not because the linker can't do it, but because it isn't given the command line argument it understands.
Quoted 4 lines
Quoted 1 line
The question I am asking is: What do we have to lose by supporting both options?
We end up passing "--fix-cortex-a53" to the linker, without knowing what it might do in the future.
It seems highly unlikely that such a generic option would be added in the future, both, because the precedent has been set for topic specific options, and because they know it has been used in the past, so they wouldn't add a previously used option to do something completely different. (And if they really did, then that would be a huge binutils bug.)
Quoted 3 lines
So, we have a trade-off between a real world problem that does currently exist and avoiding a theoretical issue that may never materialize.
Agreed, also the way Markus' patch is designed makes it such that we first try the full and current option name, and if not supported, try the second (and earlier, now obsolete) option name, so I really don't see a lot of room for things to go wrong here...
Signature · 1 line
Florian
Quoted 15 lines · Florian Fainelli
Quoted 14 lines · Markus Mayer
Quoted 5 lines · Will Deacon
Quoted 1 line · Markus Mayer
The question I am asking is: What do we have to lose by supporting both options?
We end up passing "--fix-cortex-a53" to the linker, without knowing what it might do in the future.
It seems highly unlikely that such a generic option would be added in the future, both, because the precedent has been set for topic specific options, and because they know it has been used in the past, so they wouldn't add a previously used option to do something completely different. (And if they really did, then that would be a huge binutils bug.)
Quoted 8 lines
Quoted 3 lines
So, we have a trade-off between a real world problem that does currently exist and avoiding a theoretical issue that may never materialize.
Agreed, also the way Markus' patch is designed makes it such that we first try the full and current option name, and if not supported, try the second (and earlier, now obsolete) option name, so I really don't see a lot of room for things to go wrong here...
It's not beyond the realms of possibility that ld will grow a "fix-cortex-a53" option in the future, that enables all of the a53 workarounds. Since ld is the linker supported by the kernel and gold isn't, I don't want to pass this option down.
If you can't change toolchain and you want this worked around, why can't you either build gold with it enabled by default, or pass the extra flag on the command line to the kernel build system?
Will
Quoted 25 lines · Will Deacon
Quoted 15 lines · Florian Fainelli
Quoted 14 lines · Markus Mayer
Quoted 5 lines · Will Deacon
Quoted 1 line · Markus Mayer
The question I am asking is: What do we have to lose by supporting both options?
We end up passing "--fix-cortex-a53" to the linker, without knowing what it might do in the future.
It seems highly unlikely that such a generic option would be added in the future, both, because the precedent has been set for topic specific options, and because they know it has been used in the past, so they wouldn't add a previously used option to do something completely different. (And if they really did, then that would be a huge binutils bug.)
Quoted 8 lines
Quoted 3 lines
So, we have a trade-off between a real world problem that does currently exist and avoiding a theoretical issue that may never materialize.
Agreed, also the way Markus' patch is designed makes it such that we first try the full and current option name, and if not supported, try the second (and earlier, now obsolete) option name, so I really don't see a lot of room for things to go wrong here...
Quoted 4 lines
It's not beyond the realms of possibility that ld will grow a "fix-cortex-a53" option in the future, that enables all of the a53 workarounds. Since ld is the linker supported by the kernel and gold isn't, I don't want to pass this option down.
No it's entirely reasonable to think this may happen, although:
- this has not happened yet, so once this happens, you will need to cook a patch for this anyway, and you will be able to gate this catch all linker option by an appropriate version check presumably
- you would supposedly want a fine grained set of linker options that are specific to workarounds you have to have enabled, instead of a catch all "enable all Cortex A53" workarounds
Quoted 4 lines
If you can't change toolchain and you want this worked around, why can't you either build gold with it enabled by default, or pass the extra flag on the command line to the kernel build system?
Because that creates a distribution problem and now we have to document this for people who want to build a kernel on their own, without necessarily understanding if this is something they might need, or why this is needed, and why the kernel is not taking care of that on its own? So yes, this comes down to who is responsible for what, in that case the kernel's Makefile is the best place where to put such knowledge as to which workaround needs to be enabled by the linker and it simplifies things a lot for people.
Signature · 1 line
Florian
Quoted 46 lines · Florian Fainelli
Quoted 25 lines · Will Deacon
Quoted 15 lines · Florian Fainelli
Quoted 14 lines · Markus Mayer
Quoted 5 lines · Will Deacon
Quoted 1 line · Markus Mayer
The question I am asking is: What do we have to lose by supporting both options?
We end up passing "--fix-cortex-a53" to the linker, without knowing what it might do in the future.
It seems highly unlikely that such a generic option would be added in the future, both, because the precedent has been set for topic specific options, and because they know it has been used in the past, so they wouldn't add a previously used option to do something completely different. (And if they really did, then that would be a huge binutils bug.)
Quoted 8 lines
Quoted 3 lines
So, we have a trade-off between a real world problem that does currently exist and avoiding a theoretical issue that may never materialize.
Agreed, also the way Markus' patch is designed makes it such that we first try the full and current option name, and if not supported, try the second (and earlier, now obsolete) option name, so I really don't see a lot of room for things to go wrong here...
Quoted 4 lines
It's not beyond the realms of possibility that ld will grow a "fix-cortex-a53" option in the future, that enables all of the a53 workarounds. Since ld is the linker supported by the kernel and gold isn't, I don't want to pass this option down.
No it's entirely reasonable to think this may happen, although:
- this has not happened yet, so once this happens, you will need to cook a patch for this anyway, and you will be able to gate this catch all linker option by an appropriate version check presumably
- you would supposedly want a fine grained set of linker options that are specific to workarounds you have to have enabled, instead of a catch all "enable all Cortex A53" workarounds
Quoted 4 lines
If you can't change toolchain and you want this worked around, why can't you either build gold with it enabled by default, or pass the extra flag on the command line to the kernel build system?
Quoted 8 lines
Because that creates a distribution problem and now we have to document this for people who want to build a kernel on their own, without necessarily understanding if this is something they might need, or why this is needed, and why the kernel is not taking care of that on its own? So yes, this comes down to who is responsible for what, in that case the kernel's
Makefileis the best place where to put such knowledge as to which workaround needs to be enabled by the linker and it simplifies things a lot for people.
Was this convincing enough for Catalin to pick Markus' patch or does that mean this patch needs to remain out of tree for us because of using a slightly older toolchain?
Thanks
Signature · 1 line
Florian
Hi Florian,
Quoted 15 lines · Florian Fainelli
Quoted 5 lines · Florian Fainelli
Quoted 3 lines · Will Deacon
If you can't change toolchain and you want this worked around, why can't you either build gold with it enabled by default, or pass the extra flag on the command line to the kernel build system?
Quoted 8 lines
Because that creates a distribution problem and now we have to document this for people who want to build a kernel on their own, without necessarily understanding if this is something they might need, or why this is needed, and why the kernel is not taking care of that on its own? So yes, this comes down to who is responsible for what, in that case the kernel's
Makefileis the best place where to put such knowledge as to which workaround needs to be enabled by the linker and it simplifies things a lot for people.
Quoted 3 lines
Was this convincing enough for Catalin to pick Markus' patch or does that mean this patch needs to remain out of tree for us because of using a slightly older toolchain?
I thought more about this last night, and there are two questions that might sway me:
1. How prevalent is the binary toolchain with this issue? Is it, for example, shipping as part of a publicly available LTS distribution? I know you quoted some Linaro build, but I can't actually find those binaries on their website.
2. Could we extend the Makefile magic to detect that, not only is --fix-cortex-a53-843419 unsupported, but also that the linker is in fact gold?
Will
Quoted 22 lines · Will Deacon
Hi Florian,
Quoted 15 lines · Florian Fainelli
Quoted 5 lines · Florian Fainelli
Quoted 3 lines · Will Deacon
If you can't change toolchain and you want this worked around, why can't you either build gold with it enabled by default, or pass the extra flag on the command line to the kernel build system?
Quoted 8 lines
Because that creates a distribution problem and now we have to document this for people who want to build a kernel on their own, without necessarily understanding if this is something they might need, or why this is needed, and why the kernel is not taking care of that on its own? So yes, this comes down to who is responsible for what, in that case the kernel's
Makefileis the best place where to put such knowledge as to which workaround needs to be enabled by the linker and it simplifies things a lot for people.Quoted 3 lines
Was this convincing enough for Catalin to pick Markus' patch or does that mean this patch needs to remain out of tree for us because of using a slightly older toolchain?
Quoted 7 lines
I thought more about this last night, and there are two questions that might sway me:
1. How prevalent is the binary toolchain with this issue? Is it, for example, shipping as part of a publicly available LTS distribution? I know you quoted some Linaro build, but I can't actually find those binaries on their website.
The toolchain is available for download from Broadcom's website at
https://www.broadcom.com/support/download-search/?pg=Broadband:+CPE-Gateway,+Infrastructure,+and+Set-top+Box&pf=Set-top+Box+Solutions
(working on the download agreement as we speak) and is used by the large majority of our customers today, it's hard to give you numbers, but several hundreds if not more people definitively use it AFAICT.
Quoted 4 lines
2. Could we extend the
Makefilemagic to detect that, not only is --fix-cortex-a53-843419 unsupported, but also that the linker is in fact gold?
I suppose we could do that, Markus do you mind update your original patch? Thanks!
Signature · 1 line
Florian
Quoted 38 lines · Florian Fainelli
Quoted 22 lines · Will Deacon
Hi Florian,
Quoted 15 lines · Florian Fainelli
Quoted 5 lines · Florian Fainelli
Quoted 3 lines · Will Deacon
If you can't change toolchain and you want this worked around, why can't you either build gold with it enabled by default, or pass the extra flag on the command line to the kernel build system?
Quoted 8 lines
Because that creates a distribution problem and now we have to document this for people who want to build a kernel on their own, without necessarily understanding if this is something they might need, or why this is needed, and why the kernel is not taking care of that on its own? So yes, this comes down to who is responsible for what, in that case the kernel's
Makefileis the best place where to put such knowledge as to which workaround needs to be enabled by the linker and it simplifies things a lot for people.Quoted 3 lines
Was this convincing enough for Catalin to pick Markus' patch or does that mean this patch needs to remain out of tree for us because of using a slightly older toolchain?
Quoted 7 lines
I thought more about this last night, and there are two questions that might sway me:
1. How prevalent is the binary toolchain with this issue? Is it, for example, shipping as part of a publicly available LTS distribution? I know you quoted some Linaro build, but I can't actually find those binaries on their website.
The toolchain is available for download from Broadcom's website at
https://www.broadcom.com/support/download-search/?pg=Broadband:+CPE-Gateway,+Infrastructure,+and+Set-top+Box&pf=Set-top+Box+Solutions
(working on the download agreement as we speak) and is used by the large majority of our customers today, it's hard to give you numbers, but several hundreds if not more people definitively use it AFAICT.Quoted 1 line
Quoted 6 lines
Quoted 3 lines
2. Could we extend the
Makefilemagic to detect that, not only is --fix-cortex-a53-843419 unsupported, but also that the linker is in fact gold?I suppose we could do that, Markus do you mind update your original patch? Thanks!
Hm. Unfortunately, the linker doesn't identify as "gold." I could try to use "Linaro 2014" or similar from the version string as an additional test, but I don't know if that would catch all cases. It would certainly catch ours, so it may be good enough.
$ aarch64-linux-gnu-ld -v GNU ld (GNU Binutils) Linaro 2014.11-2 2.24.0.20141017
$ aarch64-linux-gnu-ld --fix-cortex-a53-843419 aarch64-linux-gnu-ld: unrecognized option '--fix-cortex-a53-843419' aarch64-linux-gnu-ld: use the --help option for usage information
$ aarch64-linux-gnu-ld --fix-cortex-a53 aarch64-linux-gnu-ld: no input files
Regards,
-Markus
Quoted 2 lines
--
Florian