[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 10/10] xen/arm: Enable errata for secondary CPU on hotplug after the boot





On 15/05/18 12:44, Mirela Simonovic wrote:
On boot, enabling errata workarounds will be triggered by the boot CPU
from start_xen(). On CPU hotplug (non-boot scenario) this would not be
done. This patch adds the code required to enable errata workarounds for
a CPU being hotplugged after the system boots. This is triggered using
a notifier. If the CPU fails to enable workarounds the notifier will
return an error and Xen will hit the BUG_ON() in notify_cpu_starting().
To avoid the BUG_ON() in an error case either enabling notifiers should
be fixed to return void (not propagate error to notify_cpu_starting())
and the errata notifier will always return success for CPU_STARTING
event, or the notify_cpu_starting() and other common code should be
fixed to expect an error at CPU_STARTING phase.

Signed-off-by: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>

Reviewed-by: Julien Grall <julien.grall@xxxxxxx>


---
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>
---
Changes in v4:
-Add includes alphabetically
-Added newline before the return in cpu_errata_notifier_init()
-Enabling capabilities returns an error if enabling a capability fails
  (enable_nonboot_cpu_caps() returns int instead of void). When enabling
  any of the capability fails the error is remembered into a variable and
  the remaining capabilities are enabled. If enabling multiple capabilities
  fails the error returned by enable_nonboot_cpu_caps() represents the
  error code of the last failure.
-Callback enable_nonboot_cpu_caps() can return an error when CPU_STARTING
  fires. This is not right because of the assumption that starting a CPU
  cannot fail at this phase. Consequently, if an error happens it will
  cause Xen to hit the BUG_ON() in notify_cpu_starting(). In future,
  either this notifier/enabling capabilities should be fixed to always
  return success/void, or notify_cpu_starting() and other common code
  should be fixed to expect an error at CPU_STARTING phase.
-Fix commit message to reflect changes in v4
---
  xen/arch/arm/cpuerrata.c         | 49 ++++++++++++++++++++++++++++++++++++++++
  xen/arch/arm/cpufeature.c        | 29 ++++++++++++++++++++++++
  xen/include/asm-arm/cpufeature.h |  1 +
  3 files changed, 79 insertions(+)

diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 1baa20654b..b829d226ef 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -1,3 +1,4 @@
+#include <xen/cpu.h>
  #include <xen/cpumask.h>
  #include <xen/mm.h>
  #include <xen/sizes.h>
@@ -5,6 +6,7 @@
  #include <xen/spinlock.h>
  #include <xen/vmap.h>
  #include <xen/warning.h>
+#include <xen/notifier.h>
  #include <asm/cpufeature.h>
  #include <asm/cpuerrata.h>
  #include <asm/psci.h>
@@ -349,6 +351,53 @@ void __init enable_errata_workarounds(void)
      enable_cpu_capabilities(arm_errata);
  }
+static int cpu_errata_callback(struct notifier_block *nfb,
+                               unsigned long action,
+                               void *hcpu)
+{
+    int rc = 0;
+
+    switch ( action )
+    {
+    case CPU_STARTING:
+        /*
+         * At CPU_STARTING phase no notifier shall return an error, because the
+         * system is designed with the assumption that starting a CPU cannot
+         * fail at this point. If an error happens here it will cause Xen to 
hit
+         * the BUG_ON() in notify_cpu_starting(). In future, either this
+         * notifier/enabling capabilities should be fixed to always return
+         * success/void or notify_cpu_starting() and other common code should 
be
+         * fixed to expect an error at CPU_STARTING phase.
+         */
+        ASSERT(system_state != SYS_STATE_boot);
+        rc = enable_nonboot_cpu_caps(arm_errata);
+        break;
+    default:
+        break;
+    }
+
+    return !rc ? NOTIFY_DONE : notifier_from_errno(rc);
+}
+
+static struct notifier_block cpu_errata_nfb = {
+    .notifier_call = cpu_errata_callback,
+};
+
+static int __init cpu_errata_notifier_init(void)
+{
+    register_cpu_notifier(&cpu_errata_nfb);
+
+    return 0;
+}
+/*
+ * Initialization has to be done at init rather than presmp_init phase because
+ * the callback should execute only after the secondary CPUs are initially
+ * booted (in hotplug scenarios when the system state is not boot). On boot,
+ * the enabling of errata workarounds will be triggered by the boot CPU from
+ * start_xen().
+ */
+__initcall(cpu_errata_notifier_init);
+
  /*
   * Local variables:
   * mode: C
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 525b45e22f..3aaff4c0e6 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -69,6 +69,35 @@ void __init enable_cpu_capabilities(const struct 
arm_cpu_capabilities *caps)
  }
/*
+ * Run through the enabled capabilities and enable() them on the calling CPU.
+ * If enabling of any capability fails the error is returned. After enabling a
+ * capability fails the error will be remembered into 'rc' and the remaining
+ * capabilities will be enabled. If enabling multiple capabilities fail the
+ * error returned by this function represents the error code of the last
+ * failure.
+ */
+int enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps)
+{
+    int rc = 0;
+
+    for ( ; caps->matches; caps++ )
+    {
+        if ( !cpus_have_cap(caps->capability) )
+            continue;
+
+        if ( caps->enable )
+        {
+            int ret = caps->enable((void *)caps);
+
+            if ( ret )
+                rc = ret;
+        }
+    }
+
+    return rc;
+}
+
+/*
   * Local variables:
   * mode: C
   * c-file-style: "BSD"
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index e557a095af..c5d046218b 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -88,6 +88,7 @@ void update_cpu_capabilities(const struct 
arm_cpu_capabilities *caps,
                               const char *info);
void enable_cpu_capabilities(const struct arm_cpu_capabilities *caps);
+int enable_nonboot_cpu_caps(const struct arm_cpu_capabilities *caps);
#endif /* __ASSEMBLY__ */

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.