diff mbox series

arm64: cpufeature: Enable Qualcomm erratas

Message ID 20191029060432.1208859-1-bjorn.andersson@linaro.org (mailing list archive)
State New, archived
Headers show
Series arm64: cpufeature: Enable Qualcomm erratas | expand

Commit Message

Bjorn Andersson Oct. 29, 2019, 6:04 a.m. UTC
With the introduction of 'cce360b54ce6 ("arm64: capabilities: Filter the
entries based on a given mask")' the Qualcomm erratas are no long
applied.

The result of not applying errata 1003 is that MSM8996 runs into various
RCU stalls and fails to boot most of the times.

Give both 1003 and 1009 a "type" to ensure they are not filtered out in
update_cpu_capabilities().

Fixes: cce360b54ce6 ("arm64: capabilities: Filter the entries based on a given mask")
Cc: stable@vger.kernel.org
Reported-by: Mark Brown <broonie@kernel.org>
Suggested-by: Will Deacon <will@kernel.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 arch/arm64/kernel/cpu_errata.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Ard Biesheuvel Oct. 29, 2019, 7:46 a.m. UTC | #1
On Tue, 29 Oct 2019 at 07:04, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> With the introduction of 'cce360b54ce6 ("arm64: capabilities: Filter the
> entries based on a given mask")' the Qualcomm erratas are no long
> applied.
>
> The result of not applying errata 1003 is that MSM8996 runs into various
> RCU stalls and fails to boot most of the times.
>
> Give both 1003 and 1009 a "type" to ensure they are not filtered out in
> update_cpu_capabilities().
>
> Fixes: cce360b54ce6 ("arm64: capabilities: Filter the entries based on a given mask")
> Cc: stable@vger.kernel.org
> Reported-by: Mark Brown <broonie@kernel.org>
> Suggested-by: Will Deacon <will@kernel.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

FYI the offending commit is now queued up for 4.14 as well.

> ---
>  arch/arm64/kernel/cpu_errata.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index df9465120e2f..cdd8df033536 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -780,6 +780,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>         {
>                 .desc = "Qualcomm Technologies Falkor/Kryo erratum 1003",
>                 .capability = ARM64_WORKAROUND_QCOM_FALKOR_E1003,
> +               .type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
>                 .matches = cpucap_multi_entry_cap_matches,
>                 .match_list = qcom_erratum_1003_list,
>         },
> @@ -788,6 +789,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>         {
>                 .desc = "Qualcomm erratum 1009, ARM erratum 1286807",
>                 .capability = ARM64_WORKAROUND_REPEAT_TLBI,
> +               .type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
>                 ERRATA_MIDR_RANGE_LIST(arm64_repeat_tlbi_cpus),
>         },
>  #endif
> --
> 2.23.0
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Sasha Levin Oct. 29, 2019, 8:33 a.m. UTC | #2
Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: cce360b54ce6c arm64: capabilities: Filter the entries based on a given mask.

The bot has tested the following trees: v5.3.7, v4.19.80.

v5.3.7: Build OK!
v4.19.80: Failed to apply! Possible dependencies:
    1e013d06120cb ("arm64: cpufeature: Rework ptr auth hwcaps using multi_entry_cap_matches")
    396244692232f ("arm64: preempt: Provide our own implementation of asm/preempt.h")
    5ffdfaedfa0ab ("arm64: mm: Support Common Not Private translations")
    6984eb47d5c1a ("arm64/cpufeature: detect pointer authentication")
    7503197562567 ("arm64: add basic pointer authentication support")
    880f7cc47265e ("arm64: cpu_errata: Remove ARM64_MISMATCHED_CACHE_LINE_SIZE")
    95b861a4a6d94 ("arm64: arch_timer: Add workaround for ARM erratum 1188873")
    a3dcea2c85129 ("arm64: capabilities: Merge duplicate entries for Qualcomm erratum 1003")
    bc84a2d106bea ("Merge branch 'kvm/cortex-a76-erratum-1165522' into aarch64/for-next/core")
    bd4fb6d270bc4 ("arm64: Add support for SB barrier and patch in over DSB; ISB sequences")
    c9460dcb06ee6 ("arm64: capabilities: Merge entries for ARM64_WORKAROUND_CLEAN_CACHE")
    e03a4e5bb7430 ("arm64: Add silicon-errata.txt entry for ARM erratum 1188873")
    f58cdf7e3cab3 ("arm64: capabilities: Merge duplicate Cavium erratum entries")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?
Will Deacon Oct. 29, 2019, 11:39 a.m. UTC | #3
On Mon, Oct 28, 2019 at 11:04:32PM -0700, Bjorn Andersson wrote:
> With the introduction of 'cce360b54ce6 ("arm64: capabilities: Filter the
> entries based on a given mask")' the Qualcomm erratas are no long
> applied.
> 
> The result of not applying errata 1003 is that MSM8996 runs into various
> RCU stalls and fails to boot most of the times.
> 
> Give both 1003 and 1009 a "type" to ensure they are not filtered out in
> update_cpu_capabilities().

Oh nasty. Thanks for debugging and fixing this.

> Fixes: cce360b54ce6 ("arm64: capabilities: Filter the entries based on a given mask")
> Cc: stable@vger.kernel.org
> Reported-by: Mark Brown <broonie@kernel.org>
> Suggested-by: Will Deacon <will@kernel.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  arch/arm64/kernel/cpu_errata.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> index df9465120e2f..cdd8df033536 100644
> --- a/arch/arm64/kernel/cpu_errata.c
> +++ b/arch/arm64/kernel/cpu_errata.c
> @@ -780,6 +780,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>  	{
>  		.desc = "Qualcomm Technologies Falkor/Kryo erratum 1003",
>  		.capability = ARM64_WORKAROUND_QCOM_FALKOR_E1003,
> +		.type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
>  		.matches = cpucap_multi_entry_cap_matches,

This should probably be ARM64_CPUCAP_LOCAL_CPU_ERRATUM instead, but I'll
want Suzuki's ack before I take the change.

>  		.match_list = qcom_erratum_1003_list,
>  	},
> @@ -788,6 +789,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>  	{
>  		.desc = "Qualcomm erratum 1009, ARM erratum 1286807",
>  		.capability = ARM64_WORKAROUND_REPEAT_TLBI,
> +		.type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
>  		ERRATA_MIDR_RANGE_LIST(arm64_repeat_tlbi_cpus),

ERRATA_MIDR_RANGE_LIST sets the type already, so I think this is redundant.

Will
Suzuki K Poulose Oct. 29, 2019, 12:04 p.m. UTC | #4
On 10/29/2019 11:39 AM, Will Deacon wrote:
> On Mon, Oct 28, 2019 at 11:04:32PM -0700, Bjorn Andersson wrote:
>> With the introduction of 'cce360b54ce6 ("arm64: capabilities: Filter the
>> entries based on a given mask")' the Qualcomm erratas are no long
>> applied.
>>
>> The result of not applying errata 1003 is that MSM8996 runs into various
>> RCU stalls and fails to boot most of the times.
>>
>> Give both 1003 and 1009 a "type" to ensure they are not filtered out in
>> update_cpu_capabilities().
> 
> Oh nasty. Thanks for debugging and fixing this.
> 
>> Fixes: cce360b54ce6 ("arm64: capabilities: Filter the entries based on a given mask")
>> Cc: stable@vger.kernel.org
>> Reported-by: Mark Brown <broonie@kernel.org>
>> Suggested-by: Will Deacon <will@kernel.org>
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>> ---
>>   arch/arm64/kernel/cpu_errata.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
>> index df9465120e2f..cdd8df033536 100644
>> --- a/arch/arm64/kernel/cpu_errata.c
>> +++ b/arch/arm64/kernel/cpu_errata.c
>> @@ -780,6 +780,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>   	{
>>   		.desc = "Qualcomm Technologies Falkor/Kryo erratum 1003",
>>   		.capability = ARM64_WORKAROUND_QCOM_FALKOR_E1003,
>> +		.type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
>>   		.matches = cpucap_multi_entry_cap_matches,
> 
> This should probably be ARM64_CPUCAP_LOCAL_CPU_ERRATUM instead, but I'll
> want Suzuki's ack before I take the change.

Yes, it must be ARM64_CPUCAP_LOCAL_CPU_ERRATUM.

It may be a good idea to stick in a check to make sure that the scope is
set for all the capabilities in a separate patch. e.g,

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index d260e3bdf07b..51a79b4a44eb 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -546,6 +546,8 @@ static void __init
  init_cpu_hwcaps_indirect_list_from_array(const struct 
arm64_cpu_capabilities *caps)
  {
  	for (; caps->matches; caps++) {
+		WARN(!cpucap_default_scope(caps),
+		     "Invalid scope for capability %d\n", caps->capability);
  		if (WARN(caps->capability >= ARM64_NCAPS,
  			"Invalid capability %d\n", caps->capability))
			continue;

Otherwise looks good to me.

>>   		.match_list = qcom_erratum_1003_list,
>>   	},
>> @@ -788,6 +789,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
>>   	{
>>   		.desc = "Qualcomm erratum 1009, ARM erratum 1286807",
>>   		.capability = ARM64_WORKAROUND_REPEAT_TLBI,
>> +		.type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
>>   		ERRATA_MIDR_RANGE_LIST(arm64_repeat_tlbi_cpus),
> 
> ERRATA_MIDR_RANGE_LIST sets the type already, so I think this is redundant.
> 
> Will
> 



Cheers
Suzuki
Bjorn Andersson Oct. 29, 2019, 5:03 p.m. UTC | #5
On Tue 29 Oct 04:39 PDT 2019, Will Deacon wrote:

> On Mon, Oct 28, 2019 at 11:04:32PM -0700, Bjorn Andersson wrote:
> > With the introduction of 'cce360b54ce6 ("arm64: capabilities: Filter the
> > entries based on a given mask")' the Qualcomm erratas are no long
> > applied.
> > 
> > The result of not applying errata 1003 is that MSM8996 runs into various
> > RCU stalls and fails to boot most of the times.
> > 
> > Give both 1003 and 1009 a "type" to ensure they are not filtered out in
> > update_cpu_capabilities().
> 
> Oh nasty. Thanks for debugging and fixing this.
> 
> > Fixes: cce360b54ce6 ("arm64: capabilities: Filter the entries based on a given mask")
> > Cc: stable@vger.kernel.org
> > Reported-by: Mark Brown <broonie@kernel.org>
> > Suggested-by: Will Deacon <will@kernel.org>
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >  arch/arm64/kernel/cpu_errata.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> > index df9465120e2f..cdd8df033536 100644
> > --- a/arch/arm64/kernel/cpu_errata.c
> > +++ b/arch/arm64/kernel/cpu_errata.c
> > @@ -780,6 +780,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> >  	{
> >  		.desc = "Qualcomm Technologies Falkor/Kryo erratum 1003",
> >  		.capability = ARM64_WORKAROUND_QCOM_FALKOR_E1003,
> > +		.type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
> >  		.matches = cpucap_multi_entry_cap_matches,
> 
> This should probably be ARM64_CPUCAP_LOCAL_CPU_ERRATUM instead, but I'll
> want Suzuki's ack before I take the change.
> 

Sure thing, will fix!

> >  		.match_list = qcom_erratum_1003_list,
> >  	},
> > @@ -788,6 +789,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> >  	{
> >  		.desc = "Qualcomm erratum 1009, ARM erratum 1286807",
> >  		.capability = ARM64_WORKAROUND_REPEAT_TLBI,
> > +		.type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
> >  		ERRATA_MIDR_RANGE_LIST(arm64_repeat_tlbi_cpus),
> 
> ERRATA_MIDR_RANGE_LIST sets the type already, so I think this is redundant.
> 

You're right, I got lost in the macros. Apologies for that.

Will respin the patch.

Thanks,
Bjorn
Bjorn Andersson Oct. 29, 2019, 5:06 p.m. UTC | #6
On Tue 29 Oct 05:04 PDT 2019, Suzuki K Poulose wrote:

> On 10/29/2019 11:39 AM, Will Deacon wrote:
> > On Mon, Oct 28, 2019 at 11:04:32PM -0700, Bjorn Andersson wrote:
> > > With the introduction of 'cce360b54ce6 ("arm64: capabilities: Filter the
> > > entries based on a given mask")' the Qualcomm erratas are no long
> > > applied.
> > > 
> > > The result of not applying errata 1003 is that MSM8996 runs into various
> > > RCU stalls and fails to boot most of the times.
> > > 
> > > Give both 1003 and 1009 a "type" to ensure they are not filtered out in
> > > update_cpu_capabilities().
> > 
> > Oh nasty. Thanks for debugging and fixing this.
> > 
> > > Fixes: cce360b54ce6 ("arm64: capabilities: Filter the entries based on a given mask")
> > > Cc: stable@vger.kernel.org
> > > Reported-by: Mark Brown <broonie@kernel.org>
> > > Suggested-by: Will Deacon <will@kernel.org>
> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > > ---
> > >   arch/arm64/kernel/cpu_errata.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
> > > index df9465120e2f..cdd8df033536 100644
> > > --- a/arch/arm64/kernel/cpu_errata.c
> > > +++ b/arch/arm64/kernel/cpu_errata.c
> > > @@ -780,6 +780,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> > >   	{
> > >   		.desc = "Qualcomm Technologies Falkor/Kryo erratum 1003",
> > >   		.capability = ARM64_WORKAROUND_QCOM_FALKOR_E1003,
> > > +		.type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
> > >   		.matches = cpucap_multi_entry_cap_matches,
> > 
> > This should probably be ARM64_CPUCAP_LOCAL_CPU_ERRATUM instead, but I'll
> > want Suzuki's ack before I take the change.
> 
> Yes, it must be ARM64_CPUCAP_LOCAL_CPU_ERRATUM.
> 

Thanks, will fix.

> It may be a good idea to stick in a check to make sure that the scope is
> set for all the capabilities in a separate patch. e.g,
> 
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index d260e3bdf07b..51a79b4a44eb 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -546,6 +546,8 @@ static void __init
>  init_cpu_hwcaps_indirect_list_from_array(const struct
> arm64_cpu_capabilities *caps)
>  {
>  	for (; caps->matches; caps++) {
> +		WARN(!cpucap_default_scope(caps),
> +		     "Invalid scope for capability %d\n", caps->capability);
>  		if (WARN(caps->capability >= ARM64_NCAPS,
>  			"Invalid capability %d\n", caps->capability))
> 			continue;

In hindsight this would have saved me a considerable amount of time, so
I like this proposal!

Regards,
Bjorn

> 
> Otherwise looks good to me.
> 
> > >   		.match_list = qcom_erratum_1003_list,
> > >   	},
> > > @@ -788,6 +789,7 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
> > >   	{
> > >   		.desc = "Qualcomm erratum 1009, ARM erratum 1286807",
> > >   		.capability = ARM64_WORKAROUND_REPEAT_TLBI,
> > > +		.type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
> > >   		ERRATA_MIDR_RANGE_LIST(arm64_repeat_tlbi_cpus),
> > 
> > ERRATA_MIDR_RANGE_LIST sets the type already, so I think this is redundant.
> > 
> > Will
> > 
> 
> 
> 
> Cheers
> Suzuki
diff mbox series

Patch

diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index df9465120e2f..cdd8df033536 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -780,6 +780,7 @@  const struct arm64_cpu_capabilities arm64_errata[] = {
 	{
 		.desc = "Qualcomm Technologies Falkor/Kryo erratum 1003",
 		.capability = ARM64_WORKAROUND_QCOM_FALKOR_E1003,
+		.type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
 		.matches = cpucap_multi_entry_cap_matches,
 		.match_list = qcom_erratum_1003_list,
 	},
@@ -788,6 +789,7 @@  const struct arm64_cpu_capabilities arm64_errata[] = {
 	{
 		.desc = "Qualcomm erratum 1009, ARM erratum 1286807",
 		.capability = ARM64_WORKAROUND_REPEAT_TLBI,
+		.type = ARM64_CPUCAP_SCOPE_LOCAL_CPU,
 		ERRATA_MIDR_RANGE_LIST(arm64_repeat_tlbi_cpus),
 	},
 #endif