Message ID | 20180117100556.29270-1-suzuki.poulose@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Wed, Jan 17, 2018 at 10:05:56AM +0000, Suzuki K Poulose wrote: > When a CPU is brought up after we have finalised the system > wide capabilities (i.e, features and errata), we make sure the > new CPU doesn't need a new errata work around which has not been > detected already. However we don't run enable() method on the new > CPU for the errata work arounds already detected. This could > cause the new CPU running without potential work arounds. > It is upto the "enable()" method to decide if this CPU should > do something about the errata. > > Fixes: commit 6a6efbb45b7d95c84 ("arm64: Verify CPU errata work arounds on hotplugged CPU") > Cc: Will Deacon <will.deacon@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Andre Przywara <andre.przywara@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Dave Martin <dave.martin@arm.com> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > --- > arch/arm64/kernel/cpu_errata.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c > index 90a9e465339c..54e41dfe41f6 100644 > --- a/arch/arm64/kernel/cpu_errata.c > +++ b/arch/arm64/kernel/cpu_errata.c > @@ -373,15 +373,18 @@ void verify_local_cpu_errata_workarounds(void) > { > const struct arm64_cpu_capabilities *caps = arm64_errata; > > - for (; caps->matches; caps++) > - if (!cpus_have_cap(caps->capability) && > - caps->matches(caps, SCOPE_LOCAL_CPU)) { > + for (; caps->matches; caps++) { > + if (cpus_have_cap(caps->capability)) { > + if (caps->enable) > + caps->enable((void *)caps); Do we really need this cast? Can enable() fail, or do we already guarantee that it succeeds (by having detected the cap in the first place)? > + } else if (caps->matches(caps, SCOPE_LOCAL_CPU)) { [...] Cheers ---Dave
On 17/01/18 12:25, Dave Martin wrote: > On Wed, Jan 17, 2018 at 10:05:56AM +0000, Suzuki K Poulose wrote: >> When a CPU is brought up after we have finalised the system >> wide capabilities (i.e, features and errata), we make sure the >> new CPU doesn't need a new errata work around which has not been >> detected already. However we don't run enable() method on the new >> CPU for the errata work arounds already detected. This could >> cause the new CPU running without potential work arounds. >> It is upto the "enable()" method to decide if this CPU should >> do something about the errata. >> >> Fixes: commit 6a6efbb45b7d95c84 ("arm64: Verify CPU errata work arounds on hotplugged CPU") >> Cc: Will Deacon <will.deacon@arm.com> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Cc: Andre Przywara <andre.przywara@arm.com> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Dave Martin <dave.martin@arm.com> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> arch/arm64/kernel/cpu_errata.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c >> index 90a9e465339c..54e41dfe41f6 100644 >> --- a/arch/arm64/kernel/cpu_errata.c >> +++ b/arch/arm64/kernel/cpu_errata.c >> @@ -373,15 +373,18 @@ void verify_local_cpu_errata_workarounds(void) >> { >> const struct arm64_cpu_capabilities *caps = arm64_errata; >> >> - for (; caps->matches; caps++) >> - if (!cpus_have_cap(caps->capability) && >> - caps->matches(caps, SCOPE_LOCAL_CPU)) { >> + for (; caps->matches; caps++) { >> + if (cpus_have_cap(caps->capability)) { >> + if (caps->enable) >> + caps->enable((void *)caps); > > Do we really need this cast? Seems to me like the prototype for .enable needs updating. If any existing callback was actually using the (non-const) void* for some purpose (thankfully nothing seems to be), then passing the capability pointer into that would be unlikely to end well anyway. We probably want to replace or append that with a proper const struct arm64_cpu_capabilities* argument, to make it more of a method call. Robin. > Can enable() fail, or do we already guarantee that it succeeds (by > having detected the cap in the first place)? > >> + } else if (caps->matches(caps, SCOPE_LOCAL_CPU)) { > > [...] > > Cheers > ---Dave > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On 17/01/18 12:25, Dave Martin wrote: > On Wed, Jan 17, 2018 at 10:05:56AM +0000, Suzuki K Poulose wrote: >> When a CPU is brought up after we have finalised the system >> wide capabilities (i.e, features and errata), we make sure the >> new CPU doesn't need a new errata work around which has not been >> detected already. However we don't run enable() method on the new >> CPU for the errata work arounds already detected. This could >> cause the new CPU running without potential work arounds. >> It is upto the "enable()" method to decide if this CPU should >> do something about the errata. >> >> Fixes: commit 6a6efbb45b7d95c84 ("arm64: Verify CPU errata work arounds on hotplugged CPU") >> Cc: Will Deacon <will.deacon@arm.com> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Cc: Andre Przywara <andre.przywara@arm.com> >> Cc: Catalin Marinas <catalin.marinas@arm.com> >> Cc: Dave Martin <dave.martin@arm.com> >> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >> --- >> arch/arm64/kernel/cpu_errata.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c >> index 90a9e465339c..54e41dfe41f6 100644 >> --- a/arch/arm64/kernel/cpu_errata.c >> +++ b/arch/arm64/kernel/cpu_errata.c >> @@ -373,15 +373,18 @@ void verify_local_cpu_errata_workarounds(void) >> { >> const struct arm64_cpu_capabilities *caps = arm64_errata; >> >> - for (; caps->matches; caps++) >> - if (!cpus_have_cap(caps->capability) && >> - caps->matches(caps, SCOPE_LOCAL_CPU)) { >> + for (; caps->matches; caps++) { >> + if (cpus_have_cap(caps->capability)) { >> + if (caps->enable) >> + caps->enable((void *)caps); > > Do we really need this cast? Yes, otherwise we would be passing a "const *" where a "void *" is expected, and the compiler warns. Or we could simply change the prototype of the enable() method to accept a const capability ptr. > > Can enable() fail, or do we already guarantee that it succeeds (by > having detected the cap in the first place)? enable() can't fail, since as you said the cap is already detected by the scope of the capability. It is just the matter of enable() deciding to do some action on the calling CPU depending on the type of the work around (e.g, enable SCTLR bit or enable trapping etc). It is left to the capability to decide whether the calling CPU needs any action or not (e.g, bp hardening). Cheers Suzuki > >> + } else if (caps->matches(caps, SCOPE_LOCAL_CPU)) { > > [...] > > Cheers > ---Dave >
On 17/01/18 13:20, Robin Murphy wrote: > On 17/01/18 12:25, Dave Martin wrote: >> On Wed, Jan 17, 2018 at 10:05:56AM +0000, Suzuki K Poulose wrote: >>> When a CPU is brought up after we have finalised the system >>> wide capabilities (i.e, features and errata), we make sure the >>> new CPU doesn't need a new errata work around which has not been >>> detected already. However we don't run enable() method on the new >>> CPU for the errata work arounds already detected. This could >>> cause the new CPU running without potential work arounds. >>> It is upto the "enable()" method to decide if this CPU should >>> do something about the errata. >>> >>> Fixes: commit 6a6efbb45b7d95c84 ("arm64: Verify CPU errata work arounds on hotplugged CPU") >>> Cc: Will Deacon <will.deacon@arm.com> >>> Cc: Mark Rutland <mark.rutland@arm.com> >>> Cc: Andre Przywara <andre.przywara@arm.com> >>> Cc: Catalin Marinas <catalin.marinas@arm.com> >>> Cc: Dave Martin <dave.martin@arm.com> >>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >>> --- >>> arch/arm64/kernel/cpu_errata.c | 9 ++++++--- >>> 1 file changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c >>> index 90a9e465339c..54e41dfe41f6 100644 >>> --- a/arch/arm64/kernel/cpu_errata.c >>> +++ b/arch/arm64/kernel/cpu_errata.c >>> @@ -373,15 +373,18 @@ void verify_local_cpu_errata_workarounds(void) >>> { >>> const struct arm64_cpu_capabilities *caps = arm64_errata; >>> - for (; caps->matches; caps++) >>> - if (!cpus_have_cap(caps->capability) && >>> - caps->matches(caps, SCOPE_LOCAL_CPU)) { >>> + for (; caps->matches; caps++) { >>> + if (cpus_have_cap(caps->capability)) { >>> + if (caps->enable) >>> + caps->enable((void *)caps); >> >> Do we really need this cast? > > Seems to me like the prototype for .enable needs updating. If any existing callback was actually using the (non-const) void* for some purpose (thankfully nothing seems to be), then passing the capability pointer into that would be unlikely to end well anyway. I agree. This was initially written such that we could call it via on_each_cpu(). But then we later switched to stop_machine(). And we weren't using the argument until very recently with the introduction of multiple entries for the same capability. I will try to clean this up in a separate series, which would involve cleaning up all the enable(), quite invasive. I would like this to go in for 4.16, as it is needed for things like KPTI and some of the existing caps. Cheers Suzuki
On 17/01/18 13:31, Suzuki K Poulose wrote: > On 17/01/18 13:20, Robin Murphy wrote: >> On 17/01/18 12:25, Dave Martin wrote: >>> On Wed, Jan 17, 2018 at 10:05:56AM +0000, Suzuki K Poulose wrote: >>>> When a CPU is brought up after we have finalised the system >>>> wide capabilities (i.e, features and errata), we make sure the >>>> new CPU doesn't need a new errata work around which has not been >>>> detected already. However we don't run enable() method on the new >>>> CPU for the errata work arounds already detected. This could >>>> cause the new CPU running without potential work arounds. >>>> It is upto the "enable()" method to decide if this CPU should >>>> do something about the errata. >>>> >>>> Fixes: commit 6a6efbb45b7d95c84 ("arm64: Verify CPU errata work >>>> arounds on hotplugged CPU") >>>> Cc: Will Deacon <will.deacon@arm.com> >>>> Cc: Mark Rutland <mark.rutland@arm.com> >>>> Cc: Andre Przywara <andre.przywara@arm.com> >>>> Cc: Catalin Marinas <catalin.marinas@arm.com> >>>> Cc: Dave Martin <dave.martin@arm.com> >>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >>>> --- >>>> arch/arm64/kernel/cpu_errata.c | 9 ++++++--- >>>> 1 file changed, 6 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/arch/arm64/kernel/cpu_errata.c >>>> b/arch/arm64/kernel/cpu_errata.c >>>> index 90a9e465339c..54e41dfe41f6 100644 >>>> --- a/arch/arm64/kernel/cpu_errata.c >>>> +++ b/arch/arm64/kernel/cpu_errata.c >>>> @@ -373,15 +373,18 @@ void verify_local_cpu_errata_workarounds(void) >>>> { >>>> const struct arm64_cpu_capabilities *caps = arm64_errata; >>>> - for (; caps->matches; caps++) >>>> - if (!cpus_have_cap(caps->capability) && >>>> - caps->matches(caps, SCOPE_LOCAL_CPU)) { >>>> + for (; caps->matches; caps++) { >>>> + if (cpus_have_cap(caps->capability)) { >>>> + if (caps->enable) >>>> + caps->enable((void *)caps); >>> >>> Do we really need this cast? >> >> Seems to me like the prototype for .enable needs updating. If any >> existing callback was actually using the (non-const) void* for some >> purpose (thankfully nothing seems to be), then passing the capability >> pointer into that would be unlikely to end well anyway. > > I agree. This was initially written such that we could call it via > on_each_cpu(). > But then we later switched to stop_machine(). And we weren't using the > argument until > very recently with the introduction of multiple entries for the same > capability. > > I will try to clean this up in a separate series, which would involve > cleaning up > all the enable(), quite invasive. I would like this to go in for 4.16, > as it is > needed for things like KPTI and some of the existing caps. OK, sounds good. For the sake of the immediate fix, perhaps it's cleaner to just pass NULL here if the current callbacks ignore it? Robin.
On 17/01/18 13:43, Robin Murphy wrote: > On 17/01/18 13:31, Suzuki K Poulose wrote: >> On 17/01/18 13:20, Robin Murphy wrote: >>> On 17/01/18 12:25, Dave Martin wrote: >>>> On Wed, Jan 17, 2018 at 10:05:56AM +0000, Suzuki K Poulose wrote: >>>>> When a CPU is brought up after we have finalised the system >>>>> wide capabilities (i.e, features and errata), we make sure the >>>>> new CPU doesn't need a new errata work around which has not been >>>>> detected already. However we don't run enable() method on the new >>>>> CPU for the errata work arounds already detected. This could >>>>> cause the new CPU running without potential work arounds. >>>>> It is upto the "enable()" method to decide if this CPU should >>>>> do something about the errata. >>>>> >>>>> Fixes: commit 6a6efbb45b7d95c84 ("arm64: Verify CPU errata work arounds on hotplugged CPU") >>>>> Cc: Will Deacon <will.deacon@arm.com> >>>>> Cc: Mark Rutland <mark.rutland@arm.com> >>>>> Cc: Andre Przywara <andre.przywara@arm.com> >>>>> Cc: Catalin Marinas <catalin.marinas@arm.com> >>>>> Cc: Dave Martin <dave.martin@arm.com> >>>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >>>>> --- >>>>> arch/arm64/kernel/cpu_errata.c | 9 ++++++--- >>>>> 1 file changed, 6 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c >>>>> index 90a9e465339c..54e41dfe41f6 100644 >>>>> --- a/arch/arm64/kernel/cpu_errata.c >>>>> +++ b/arch/arm64/kernel/cpu_errata.c >>>>> @@ -373,15 +373,18 @@ void verify_local_cpu_errata_workarounds(void) >>>>> { >>>>> const struct arm64_cpu_capabilities *caps = arm64_errata; >>>>> - for (; caps->matches; caps++) >>>>> - if (!cpus_have_cap(caps->capability) && >>>>> - caps->matches(caps, SCOPE_LOCAL_CPU)) { >>>>> + for (; caps->matches; caps++) { >>>>> + if (cpus_have_cap(caps->capability)) { >>>>> + if (caps->enable) >>>>> + caps->enable((void *)caps); >>>> >>>> Do we really need this cast? >>> >>> Seems to me like the prototype for .enable needs updating. If any existing callback was actually using the (non-const) void* for some purpose (thankfully nothing seems to be), then passing the capability pointer into that would be unlikely to end well anyway. >> >> I agree. This was initially written such that we could call it via on_each_cpu(). >> But then we later switched to stop_machine(). And we weren't using the argument until >> very recently with the introduction of multiple entries for the same capability. >> >> I will try to clean this up in a separate series, which would involve cleaning up >> all the enable(), quite invasive. I would like this to go in for 4.16, as it is>> needed for things like KPTI and some of the existing caps. Correction, s/KPTI/bp hardening/ > > OK, sounds good. For the sake of the immediate fix, perhaps it's cleaner to just pass NULL here if the current callbacks ignore it? As I said above, we have some users at the moment, so we cant do that. Suzuki
On Wed, Jan 17, 2018 at 01:22:19PM +0000, Suzuki K Poulose wrote: > On 17/01/18 12:25, Dave Martin wrote: > >On Wed, Jan 17, 2018 at 10:05:56AM +0000, Suzuki K Poulose wrote: > >>When a CPU is brought up after we have finalised the system > >>wide capabilities (i.e, features and errata), we make sure the > >>new CPU doesn't need a new errata work around which has not been > >>detected already. However we don't run enable() method on the new > >>CPU for the errata work arounds already detected. This could > >>cause the new CPU running without potential work arounds. > >>It is upto the "enable()" method to decide if this CPU should > >>do something about the errata. > >> > >>Fixes: commit 6a6efbb45b7d95c84 ("arm64: Verify CPU errata work arounds on hotplugged CPU") > >>Cc: Will Deacon <will.deacon@arm.com> > >>Cc: Mark Rutland <mark.rutland@arm.com> > >>Cc: Andre Przywara <andre.przywara@arm.com> > >>Cc: Catalin Marinas <catalin.marinas@arm.com> > >>Cc: Dave Martin <dave.martin@arm.com> > >>Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > >>--- > >> arch/arm64/kernel/cpu_errata.c | 9 ++++++--- > >> 1 file changed, 6 insertions(+), 3 deletions(-) > >> > >>diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c > >>index 90a9e465339c..54e41dfe41f6 100644 > >>--- a/arch/arm64/kernel/cpu_errata.c > >>+++ b/arch/arm64/kernel/cpu_errata.c > >>@@ -373,15 +373,18 @@ void verify_local_cpu_errata_workarounds(void) > >> { > >> const struct arm64_cpu_capabilities *caps = arm64_errata; > >>- for (; caps->matches; caps++) > >>- if (!cpus_have_cap(caps->capability) && > >>- caps->matches(caps, SCOPE_LOCAL_CPU)) { > >>+ for (; caps->matches; caps++) { > >>+ if (cpus_have_cap(caps->capability)) { > >>+ if (caps->enable) > >>+ caps->enable((void *)caps); > > > >Do we really need this cast? > > Yes, otherwise we would be passing a "const *" where a "void *" is expected, > and the compiler warns. Or we could simply change the prototype of the > enable() method to accept a const capability ptr. Hmmm, what is this argument for exactly? cpufeature.h doesn't explain what it is. Does any enable method use this for anything other than a struct arm64_cpu_capabilities const * ? If not, it would be better to specifiy that. Cheers ---Dave
On 17/01/18 14:38, Dave Martin wrote: > On Wed, Jan 17, 2018 at 01:22:19PM +0000, Suzuki K Poulose wrote: >> On 17/01/18 12:25, Dave Martin wrote: >>> On Wed, Jan 17, 2018 at 10:05:56AM +0000, Suzuki K Poulose wrote: >>>> When a CPU is brought up after we have finalised the system >>>> wide capabilities (i.e, features and errata), we make sure the >>>> new CPU doesn't need a new errata work around which has not been >>>> detected already. However we don't run enable() method on the new >>>> CPU for the errata work arounds already detected. This could >>>> cause the new CPU running without potential work arounds. >>>> It is upto the "enable()" method to decide if this CPU should >>>> do something about the errata. >>>> >>>> Fixes: commit 6a6efbb45b7d95c84 ("arm64: Verify CPU errata work arounds on hotplugged CPU") >>>> Cc: Will Deacon <will.deacon@arm.com> >>>> Cc: Mark Rutland <mark.rutland@arm.com> >>>> Cc: Andre Przywara <andre.przywara@arm.com> >>>> Cc: Catalin Marinas <catalin.marinas@arm.com> >>>> Cc: Dave Martin <dave.martin@arm.com> >>>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> >>>> --- >>>> arch/arm64/kernel/cpu_errata.c | 9 ++++++--- >>>> 1 file changed, 6 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c >>>> index 90a9e465339c..54e41dfe41f6 100644 >>>> --- a/arch/arm64/kernel/cpu_errata.c >>>> +++ b/arch/arm64/kernel/cpu_errata.c >>>> @@ -373,15 +373,18 @@ void verify_local_cpu_errata_workarounds(void) >>>> { >>>> const struct arm64_cpu_capabilities *caps = arm64_errata; >>>> - for (; caps->matches; caps++) >>>> - if (!cpus_have_cap(caps->capability) && >>>> - caps->matches(caps, SCOPE_LOCAL_CPU)) { >>>> + for (; caps->matches; caps++) { >>>> + if (cpus_have_cap(caps->capability)) { >>>> + if (caps->enable) >>>> + caps->enable((void *)caps); >>> >>> Do we really need this cast? >> >> Yes, otherwise we would be passing a "const *" where a "void *" is expected, >> and the compiler warns. Or we could simply change the prototype of the >> enable() method to accept a const capability ptr. > > Hmmm, what is this argument for exactly? cpufeature.h doesn't explain > what it is. This was introduced by commit 0a0d111d40fd1 ("arm64: cpufeature: Pass capability structure to ->enable callback"). The idea is to enable multiple entries in the table for a single capability. Some capabilities (read errata) could be detected in multiple ways. e.g, different MIDR ranges. (e.g, ARM64_HARDEN_BRANCH_PREDICTOR, ARM64_WORKAROUND_QCOM_FALKOR_E1003. Now, even though the errata is the same, there might be different work arounds for them in each "matching" cases. So, we need the "caps" passed on to the enable() method to see if the "specific work around" should be applied to the system/CPU (since CPU hwcap could be set by one of the cpu_capabilities entry.) (as we invoke enable() for all "available" capabilities.) Passing the caps information makes it easier to decide, by using the caps->matches(). > > Does any enable method use this for anything other than a struct > arm64_cpu_capabilities const * ? No, we use it only for const struct arm64_cpu_capability *. > If not, it would be better to specifiy that. Yes, that could be done. Cheers Suzuki
On Wed, Jan 17, 2018 at 02:52:09PM +0000, Suzuki K Poulose wrote: > On 17/01/18 14:38, Dave Martin wrote: > >On Wed, Jan 17, 2018 at 01:22:19PM +0000, Suzuki K Poulose wrote: > >>On 17/01/18 12:25, Dave Martin wrote: > >>>On Wed, Jan 17, 2018 at 10:05:56AM +0000, Suzuki K Poulose wrote: [...] > >>>>+ for (; caps->matches; caps++) { > >>>>+ if (cpus_have_cap(caps->capability)) { > >>>>+ if (caps->enable) > >>>>+ caps->enable((void *)caps); > >>> > >>>Do we really need this cast? > >> > >>Yes, otherwise we would be passing a "const *" where a "void *" is expected, > >>and the compiler warns. Or we could simply change the prototype of the > >>enable() method to accept a const capability ptr. > > > >Hmmm, what is this argument for exactly? cpufeature.h doesn't explain > >what it is. > > This was introduced by commit 0a0d111d40fd1 ("arm64: cpufeature: Pass capability > structure to ->enable callback"). > > The idea is to enable multiple entries in the table for a single capability. > Some capabilities (read errata) could be detected in multiple ways. e.g, different > MIDR ranges. (e.g, ARM64_HARDEN_BRANCH_PREDICTOR, ARM64_WORKAROUND_QCOM_FALKOR_E1003. > Now, even though the errata is the same, there might be different work arounds > for them in each "matching" cases. So, we need the "caps" passed on to the > enable() method to see if the "specific work around" should be applied > to the system/CPU (since CPU hwcap could be set by one of the cpu_capabilities > entry.) (as we invoke enable() for all "available" capabilities.) > > Passing the caps information makes it easier to decide, by using the caps->matches(). This makes sense (it's a common OOP idiom anyway: object->method(object, ...)) > > > >Does any enable method use this for anything other than a struct > >arm64_cpu_capabilities const * ? > > No, we use it only for const struct arm64_cpu_capability *. > > >If not, it would be better to specifiy that. > > Yes, that could be done. OK, I vote for doing that. Cheers ---Dave
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c index 90a9e465339c..54e41dfe41f6 100644 --- a/arch/arm64/kernel/cpu_errata.c +++ b/arch/arm64/kernel/cpu_errata.c @@ -373,15 +373,18 @@ void verify_local_cpu_errata_workarounds(void) { const struct arm64_cpu_capabilities *caps = arm64_errata; - for (; caps->matches; caps++) - if (!cpus_have_cap(caps->capability) && - caps->matches(caps, SCOPE_LOCAL_CPU)) { + for (; caps->matches; caps++) { + if (cpus_have_cap(caps->capability)) { + if (caps->enable) + caps->enable((void *)caps); + } else if (caps->matches(caps, SCOPE_LOCAL_CPU)) { pr_crit("CPU%d: Requires work around for %s, not detected" " at boot time\n", smp_processor_id(), caps->desc ? : "an erratum"); cpu_die_early(); } + } } void update_cpu_errata_workarounds(void)
When a CPU is brought up after we have finalised the system wide capabilities (i.e, features and errata), we make sure the new CPU doesn't need a new errata work around which has not been detected already. However we don't run enable() method on the new CPU for the errata work arounds already detected. This could cause the new CPU running without potential work arounds. It is upto the "enable()" method to decide if this CPU should do something about the errata. Fixes: commit 6a6efbb45b7d95c84 ("arm64: Verify CPU errata work arounds on hotplugged CPU") Cc: Will Deacon <will.deacon@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Andre Przywara <andre.przywara@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Dave Martin <dave.martin@arm.com> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> --- arch/arm64/kernel/cpu_errata.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)