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 <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Andre Przywara <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Dave Martin <[email protected]>
Signed-off-by: Suzuki K Poulose <[email protected]>
---
  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

Reply via email to