linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
@ 2016-10-31 19:44 Markus Mayer
  2016-11-02 21:03 ` Will Deacon
  0 siblings, 1 reply; 12+ messages in thread
From: Markus Mayer @ 2016-10-31 19:44 UTC (permalink / raw)
  To: Will Deacon, Catalin Marinas
  Cc: ARM Kernel Mailing List, Linux Kernel Mailing List, Markus Mayer

From: Markus Mayer <mmayer@broadcom.com>

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.

[1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=7a2a1c793578a8468604e661dda025ecb8d0bd20;hp=cfbf0e3c5b637d66b2b1aeadecae9c187b825b2f

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---

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(-)

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index ab51aed..231ba69 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2016-10-31 19:44 [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53 Markus Mayer
@ 2016-11-02 21:03 ` Will Deacon
  2016-11-02 21:07   ` Markus Mayer
  0 siblings, 1 reply; 12+ messages in thread
From: Will Deacon @ 2016-11-02 21:03 UTC (permalink / raw)
  To: Markus Mayer
  Cc: Catalin Marinas, ARM Kernel Mailing List,
	Linux Kernel Mailing List, Markus Mayer

On Mon, Oct 31, 2016 at 12:44:14PM -0700, Markus Mayer wrote:
> From: Markus Mayer <mmayer@broadcom.com>
> 
> 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.
> 
> [1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=7a2a1c793578a8468604e661dda025ecb8d0bd20;hp=cfbf0e3c5b637d66b2b1aeadecae9c187b825b2f
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>

If newer versions of gold accept the correct option name, why do we care?

Will

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2016-11-02 21:03 ` Will Deacon
@ 2016-11-02 21:07   ` Markus Mayer
  2016-11-02 21:27     ` Will Deacon
  0 siblings, 1 reply; 12+ messages in thread
From: Markus Mayer @ 2016-11-02 21:07 UTC (permalink / raw)
  To: Will Deacon
  Cc: Markus Mayer, Catalin Marinas, ARM Kernel Mailing List,
	Linux Kernel Mailing List

On 2 November 2016 at 14:03, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Oct 31, 2016 at 12:44:14PM -0700, Markus Mayer wrote:
>> From: Markus Mayer <mmayer@broadcom.com>
>>
>> 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.
>>
>> [1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=7a2a1c793578a8468604e661dda025ecb8d0bd20;hp=cfbf0e3c5b637d66b2b1aeadecae9c187b825b2f
>>
>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2016-11-02 21:07   ` Markus Mayer
@ 2016-11-02 21:27     ` Will Deacon
  2016-11-02 21:41       ` Markus Mayer
  0 siblings, 1 reply; 12+ messages in thread
From: Will Deacon @ 2016-11-02 21:27 UTC (permalink / raw)
  To: Markus Mayer
  Cc: Markus Mayer, Catalin Marinas, ARM Kernel Mailing List,
	Linux Kernel Mailing List

On Wed, Nov 02, 2016 at 02:07:17PM -0700, Markus Mayer wrote:
> On 2 November 2016 at 14:03, Will Deacon <will.deacon@arm.com> wrote:
> > On Mon, Oct 31, 2016 at 12:44:14PM -0700, Markus Mayer wrote:
> >> From: Markus Mayer <mmayer@broadcom.com>
> >>
> >> 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.
> >>
> >> [1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=7a2a1c793578a8468604e661dda025ecb8d0bd20;hp=cfbf0e3c5b637d66b2b1aeadecae9c187b825b2f
> >>
> >> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> >
> > 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.

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.

> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2016-11-02 21:27     ` Will Deacon
@ 2016-11-02 21:41       ` Markus Mayer
  2016-11-02 21:57         ` Florian Fainelli
  0 siblings, 1 reply; 12+ messages in thread
From: Markus Mayer @ 2016-11-02 21:41 UTC (permalink / raw)
  To: Will Deacon
  Cc: Markus Mayer, Catalin Marinas, ARM Kernel Mailing List,
	Linux Kernel Mailing List

On 2 November 2016 at 14:27, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Nov 02, 2016 at 02:07:17PM -0700, Markus Mayer wrote:
>> On 2 November 2016 at 14:03, Will Deacon <will.deacon@arm.com> wrote:
>> > On Mon, Oct 31, 2016 at 12:44:14PM -0700, Markus Mayer wrote:
>> >> From: Markus Mayer <mmayer@broadcom.com>
>> >>
>> >> 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.
>> >>
>> >> [1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=7a2a1c793578a8468604e661dda025ecb8d0bd20;hp=cfbf0e3c5b637d66b2b1aeadecae9c187b825b2f
>> >>
>> >> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>> >
>> > 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.
>
> 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.

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

> Will

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2016-11-02 21:41       ` Markus Mayer
@ 2016-11-02 21:57         ` Florian Fainelli
  2016-11-03 14:16           ` Will Deacon
  0 siblings, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2016-11-02 21:57 UTC (permalink / raw)
  To: Markus Mayer, Will Deacon
  Cc: Markus Mayer, Catalin Marinas, Linux Kernel Mailing List,
	ARM Kernel Mailing List

On 11/02/2016 02:41 PM, Markus Mayer wrote:
> On 2 November 2016 at 14:27, Will Deacon <will.deacon@arm.com> wrote:
>> On Wed, Nov 02, 2016 at 02:07:17PM -0700, Markus Mayer wrote:
>>> On 2 November 2016 at 14:03, Will Deacon <will.deacon@arm.com> wrote:
>>>> On Mon, Oct 31, 2016 at 12:44:14PM -0700, Markus Mayer wrote:
>>>>> From: Markus Mayer <mmayer@broadcom.com>
>>>>>
>>>>> 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.
>>>>>
>>>>> [1] https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=7a2a1c793578a8468604e661dda025ecb8d0bd20;hp=cfbf0e3c5b637d66b2b1aeadecae9c187b825b2f
>>>>>
>>>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>>>>
>>>> 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.
>>
>> 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.
> 
>>> 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.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2016-11-02 21:57         ` Florian Fainelli
@ 2016-11-03 14:16           ` Will Deacon
  2016-11-03 17:20             ` Florian Fainelli
  0 siblings, 1 reply; 12+ messages in thread
From: Will Deacon @ 2016-11-03 14:16 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Markus Mayer, Markus Mayer, Catalin Marinas,
	Linux Kernel Mailing List, ARM Kernel Mailing List

On Wed, Nov 02, 2016 at 02:57:26PM -0700, Florian Fainelli wrote:
> On 11/02/2016 02:41 PM, Markus Mayer wrote:
> > On 2 November 2016 at 14:27, Will Deacon <will.deacon@arm.com> wrote:
> >> On Wed, Nov 02, 2016 at 02:07:17PM -0700, Markus Mayer wrote:
> >>> 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.
> 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2016-11-03 14:16           ` Will Deacon
@ 2016-11-03 17:20             ` Florian Fainelli
  2016-12-28 20:17               ` Florian Fainelli
  0 siblings, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2016-11-03 17:20 UTC (permalink / raw)
  To: Will Deacon
  Cc: Markus Mayer, Markus Mayer, Catalin Marinas,
	Linux Kernel Mailing List, ARM Kernel Mailing List

On 11/03/2016 07:16 AM, Will Deacon wrote:
> On Wed, Nov 02, 2016 at 02:57:26PM -0700, Florian Fainelli wrote:
>> On 11/02/2016 02:41 PM, Markus Mayer wrote:
>>> On 2 November 2016 at 14:27, Will Deacon <will.deacon@arm.com> wrote:
>>>> On Wed, Nov 02, 2016 at 02:07:17PM -0700, Markus Mayer wrote:
>>>>> 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.
>>
>> 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.

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

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2016-11-03 17:20             ` Florian Fainelli
@ 2016-12-28 20:17               ` Florian Fainelli
  2017-01-04 11:49                 ` Will Deacon
  0 siblings, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2016-12-28 20:17 UTC (permalink / raw)
  To: Will Deacon
  Cc: Markus Mayer, Markus Mayer, Catalin Marinas,
	Linux Kernel Mailing List, ARM Kernel Mailing List

On 11/03/2016 10:20 AM, Florian Fainelli wrote:
> On 11/03/2016 07:16 AM, Will Deacon wrote:
>> On Wed, Nov 02, 2016 at 02:57:26PM -0700, Florian Fainelli wrote:
>>> On 11/02/2016 02:41 PM, Markus Mayer wrote:
>>>> On 2 November 2016 at 14:27, Will Deacon <will.deacon@arm.com> wrote:
>>>>> On Wed, Nov 02, 2016 at 02:07:17PM -0700, Markus Mayer wrote:
>>>>>> 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.
>>>
>>> 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.
> 
> 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
> 
>>
>> 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.

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2016-12-28 20:17               ` Florian Fainelli
@ 2017-01-04 11:49                 ` Will Deacon
  2017-01-04 22:39                   ` Florian Fainelli
  0 siblings, 1 reply; 12+ messages in thread
From: Will Deacon @ 2017-01-04 11:49 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Markus Mayer, Markus Mayer, Catalin Marinas,
	Linux Kernel Mailing List, ARM Kernel Mailing List

Hi Florian,

On Wed, Dec 28, 2016 at 12:17:23PM -0800, Florian Fainelli wrote:
> On 11/03/2016 10:20 AM, Florian Fainelli wrote:
> > On 11/03/2016 07:16 AM, Will Deacon wrote:
> >> 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.
> 
> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2017-01-04 11:49                 ` Will Deacon
@ 2017-01-04 22:39                   ` Florian Fainelli
  2017-01-04 23:04                     ` Markus Mayer
  0 siblings, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2017-01-04 22:39 UTC (permalink / raw)
  To: Will Deacon
  Cc: Markus Mayer, Markus Mayer, Catalin Marinas,
	Linux Kernel Mailing List, ARM Kernel Mailing List

On 01/04/2017 03:49 AM, Will Deacon wrote:
> Hi Florian,
> 
> On Wed, Dec 28, 2016 at 12:17:23PM -0800, Florian Fainelli wrote:
>> On 11/03/2016 10:20 AM, Florian Fainelli wrote:
>>> On 11/03/2016 07:16 AM, Will Deacon wrote:
>>>> 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.
>>
>> 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.

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.

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

I suppose we could do that, Markus do you mind update your original
patch? Thanks!
-- 
Florian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53
  2017-01-04 22:39                   ` Florian Fainelli
@ 2017-01-04 23:04                     ` Markus Mayer
  0 siblings, 0 replies; 12+ messages in thread
From: Markus Mayer @ 2017-01-04 23:04 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Will Deacon, Markus Mayer, Catalin Marinas,
	Linux Kernel Mailing List, ARM Kernel Mailing List

On 4 January 2017 at 14:39, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 01/04/2017 03:49 AM, Will Deacon wrote:
>> Hi Florian,
>>
>> On Wed, Dec 28, 2016 at 12:17:23PM -0800, Florian Fainelli wrote:
>>> On 11/03/2016 10:20 AM, Florian Fainelli wrote:
>>>> On 11/03/2016 07:16 AM, Will Deacon wrote:
>>>>> 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.
>>>
>>> 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.
>
> 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.
>
>>
>>   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?
>
> 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

> --
> Florian

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-01-04 23:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-31 19:44 [PATCH] arm64: errata: Check for --fix-cortex-a53-843419 and --fix-cortex-a53 Markus Mayer
2016-11-02 21:03 ` Will Deacon
2016-11-02 21:07   ` Markus Mayer
2016-11-02 21:27     ` Will Deacon
2016-11-02 21:41       ` Markus Mayer
2016-11-02 21:57         ` Florian Fainelli
2016-11-03 14:16           ` Will Deacon
2016-11-03 17:20             ` Florian Fainelli
2016-12-28 20:17               ` Florian Fainelli
2017-01-04 11:49                 ` Will Deacon
2017-01-04 22:39                   ` Florian Fainelli
2017-01-04 23:04                     ` Markus Mayer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).