Implement CPU hotplug on Arm

When stopping a core deinit_timer_interrupt is called in non-alloc context, which causes xfree in release_irq to fail an assert.

To fix this, switch to a statically allocated irqaction that does not need to be freed in release_irq.

Signed-off-by Mykyta Poturai <[email protected]>

v2->v3:
* no changes

v1->v2:
* Use percpu actions

---
 xen/arch/arm/time.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

xen/arch/arm/time.c +0 -0

index e74d30d258..59349467de 100644

xen/arch/arm/time.c +15 -5

+
 /* Set up the timer interrupt on this CPU */
 void init_timer_interrupt(void)
 {
+    struct irqaction *hyp_action = &this_cpu(irq_hyp);
+    struct irqaction *virt_action = &this_cpu(irq_virt);
+
     /* Sensible defaults */
     WRITE_SYSREG64(0, CNTVOFF_EL2);     /* No VM-specific offset */
     /* Do not let the VMs program the physical timer, only read the physical counter */

@@ -314,10 +320,17 @@ void init_timer_interrupt(void)
     WRITE_SYSREG(0, CNTHP_CTL_EL2);   /* Hypervisor's timer disabled */
     isb();
 
-    request_irq(timer_irq[TIMER_HYP_PPI], 0, htimer_interrupt,
-                "hyptimer", NULL);
-    request_irq(timer_irq[TIMER_VIRT_PPI], 0, vtimer_interrupt,
-                   "virtimer", NULL);
+    hyp_action->name = "hyptimer";
+    hyp_action->handler = htimer_interrupt;
+    hyp_action->dev_id = NULL;
+    hyp_action->free_on_release = 0;
+    setup_irq(timer_irq[TIMER_HYP_PPI], 0, hyp_action);
+
+    virt_action->name = "virtimer";
+    virt_action->handler = vtimer_interrupt;
+    virt_action->dev_id = NULL;
+    virt_action->free_on_release = 0;
+    setup_irq(timer_irq[TIMER_VIRT_PPI], 0, virt_action);
 
     check_timer_irq_cfg(timer_irq[TIMER_HYP_PPI], "hypervisor");
     check_timer_irq_cfg(timer_irq[TIMER_VIRT_PPI], "virtual");
--

2.34.1

When stopping a core deinit_timer_interrupt is called in non-alloc context, which causes xfree in release_irq to fail an assert.

To fix this, switch to a statically allocated irqaction that does not need to be freed in release_irq.

Signed-off-by Mykyta Poturai <[email protected]>

When stopping a core cpu_gic_callback is called in non-alloc context, which causes xfree in release_irq to fail an assert.

To fix this, switch to a statically allocated irqaction that does not need to be freed in release_irq.

Signed-off-by Mykyta Poturai <[email protected]>

v2->v3:
* no changes

v1->v2:
* use percpu actions

---
 xen/arch/arm/gic.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

xen/arch/arm/gic.c +0 -0

index 260ee64cca..ed6853bb32 100644

xen/arch/arm/gic.c +8 -3

+
 void init_maintenance_interrupt(void)
 {
-    request_irq(gic_hw_ops->info->maintenance_irq, 0, maintenance_interrupt,
-                "irq-maintenance", NULL);
+    struct irqaction *maintenance = &this_cpu(irq_maintenance);
+
+    maintenance->name = "irq-maintenance";
+    maintenance->handler = maintenance_interrupt;
+    maintenance->dev_id = NULL;
+    maintenance->free_on_release = 0;
+    setup_irq(gic_hw_ops->info->maintenance_irq, 0, maintenance);
 }
 
 int gic_make_hwdom_dt_node(const struct domain *d,
--

2.34.1

When stopping a core cpu_gic_callback is called in non-alloc context, which causes xfree in release_irq to fail an assert.

To fix this, switch to a statically allocated irqaction that does not need to be freed in release_irq.

Signed-off-by Mykyta Poturai <[email protected]>

Implement XEN_SYSCTL_CPU_HOTPLUG_{ONLINE,OFFLINE} calls to allow for enabling/disabling CPU cores in runtime.

Signed-off-by Mykyta Poturai <[email protected]>

v2->v3:
* no changes

v1->v2:
* remove SMT ops
* remove cpu == 0 checks
* add XSM hooks
* only implement for 64bit Arm

---
  xen/arch/arm/sysctl.c | 45 +++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 45 insertions(+)

xen/arch/arm/sysctl.c +27 -0

index 32cab4feff..fecd649db1 100644
@@ -12,6 +12,8 @@
  #include <xen/dt-overlay.h>
  #include <xen/errno.h>
  #include <xen/hypercall.h>
+#include <xen/cpu.h>
+#include <xsm/xsm.h>
  #include <asm/arm64/sve.h>
  #include <public/sysctl.h>
 

@@ -23,6 +25,42 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
                                         XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
  }
 
+#ifdef CONFIG_ARM_64
+static long cpu_up_helper(void *data)
+{
+    unsigned long cpu = (unsigned long) data;
+    return cpu_up(cpu);
+}
+
+static long cpu_down_helper(void *data)
+{
+    unsigned long cpu = (unsigned long) data;
+    return cpu_down(cpu);
+}
+
+static long cpu_hotplug_sysctl(struct xen_sysctl_cpu_hotplug *hotplug)
+{
+    int ret;
+
+    switch (hotplug->op) {
+        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
+            ret = xsm_resource_plug_core(XSM_HOOK);
+            if ( ret )
+                return ret;
+            return continue_hypercall_on_cpu(0, cpu_up_helper, _p(hotplug->cpu));
+
+        case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:

This series implements support for CPU hotplug/unplug on Arm. To achieve this, several things need to be done:

1. XEN_SYSCTL_CPU_HOTPLUG_* calls implemented.
2. timer and GIC maintenance interrupts switched to static irqactions to remove the need for freeing them during release_irq.
3. Enabled the build of xen-hptool on Arm.

Tested on QEMU.

v2->v3:
* add docs

v1->v2:
* see individual patches

Mykyta Poturai (5):
arm/time: Use static irqaction
arm/gic: Use static irqaction
arm/sysctl: Implement cpu hotplug ops
tools: Allow building xen-hptool without CONFIG_MIGRATE
docs: Document CPU hotplug

 config/Tools.mk.in               |  1 +
 docs/misc/cpu-hotplug.txt        | 51 ++++++++++++++++++++++++++++++++
 tools/configure                  | 30 +++++++++++++++++++
 tools/configure.ac               |  1 +
 tools/libs/guest/Makefile.common |  4 +++
 tools/misc/Makefile              |  2 +-
 xen/arch/arm/gic.c               | 11 +++++--
 xen/arch/arm/sysctl.c            | 45 ++++++++++++++++++++++++++++
 xen/arch/arm/time.c              | 21 ++++++++++---
 9 files changed, 159 insertions(+), 7 deletions(-)
 create mode 100644 docs/misc/cpu-hotplug.txt
 mode change 100755 => 100644 tools/configure

--
2.34.1

Hi Mykyta,

Thanks for the series.

It seems there might be issues here -- please take a look and let me know if my concerns are valid:

1. FF-A notification IRQ: after a CPU down->up cycle the IRQ configuration may be lost.

2. GICv3 LPIs: a CPU may fail to come back up unless its LPI pending table exists (is allocated) on bring-up. See gicv3_lpi_allocate_pendtable() and its call chain.

3. IRQ migration on CPU down: if an IRQ targets a CPU being offlined, its affinity should be moved to an online CPU before completing the offlining.

4. Race between the new hypercalls and disable/enable_nonboot_cpus():
disable_nonboot_cpus is called, enable_nonboot_cpus() reads frozen_cpus, and before it calls cpu_up() a hypercall onlines the CPU. cpu_up() then fails as "already online", but the CPU_RESUME_FAILED path may still run for an already-online CPU, risking use-after-free of per-CPU state (e.g. via free_percpu_area()) and other issues related to CPU_RESUME_FAILED notification.

Best regards,
Mykola

Quoted 4 lines · Mykola Kvach

Hi Mykyta,

Thanks for the series.

Quoted 5 lines

It seems there might be issues here -- please take a look and let me know if my concerns are valid:

1. FF-A notification IRQ: after a CPU down->up cycle the IRQ configuration may be lost.

OPTEE and FFA are marked as unsupported.

Quoted 4 lines

2. GICv3 LPIs: a CPU may fail to come back up unless its LPI pending table exists (is allocated) on bring-up. See gicv3_lpi_allocate_pendtable() and its call chain.

ITS is marked as unsupported. I have a plan to deal with this, but it is out of scope of this series.

Quoted 4 lines

3. IRQ migration on CPU down: if an IRQ targets a CPU being offlined, its affinity should be moved to an online CPU before completing the offlining.

All guest tied IRQ migration is handled by the scheduler. Regarding the irqs used by Xen, I didn't find any with affinity to other CPUs than CPU 0, which can't be disabled. I think theoretically it is possible for them to have different affinity, but it seems unlikely considering that x86 hotplug code also doesn't seem to do any Xen irq migration AFAIU.

Quoted 9 lines

4. Race between the new hypercalls and disable/enable_nonboot_cpus():
disable_nonboot_cpus is called, enable_nonboot_cpus() reads frozen_cpus, and before it calls cpu_up() a hypercall onlines the CPU. cpu_up() then fails as "already online", but the CPU_RESUME_FAILED path may still run for an already-online CPU, risking use-after-free of per-CPU state (e.g. via free_percpu_area()) and other issues related to CPU_RESUME_FAILED notification.

There don't seem to be any calls to disable/enable_nonboot_cpus() on Arm. If we take x86 as an example, then they are called with all domains already paused, and I don't see how paused domains can issue hypercalls.

Quoted 4 lines

Best regards,
Mykola

--
Mykyta

Thank you for your answers

Quoted 9 lines · Mykyta Poturai

Quoted 7 lines · Mykola Kvach

Hi Mykyta,

Thanks for the series.

It seems there might be issues here -- please take a look and let me know if my concerns are valid:

Quoted 4 lines

Quoted 2 lines

1. FF-A notification IRQ: after a CPU down->up cycle the IRQ configuration may be lost.

OPTEE and FFA are marked as unsupported.

Understood, thanks. Would it be worth documenting this?

Quoted 14 lines

Quoted 4 lines

2. GICv3 LPIs: a CPU may fail to come back up unless its LPI pending table exists (is allocated) on bring-up. See gicv3_lpi_allocate_pendtable() and its call chain.

ITS is marked as unsupported. I have a plan to deal with this, but it is out of scope of this series.
Thanks for the clarification. Should we document this somewhere?

Quoted 3 lines

3. IRQ migration on CPU down: if an IRQ targets a CPU being offlined, its affinity should be moved to an online CPU before completing the offlining.

Quoted 5 lines

All guest tied IRQ migration is handled by the scheduler. Regarding the irqs used by Xen, I didn't find any with affinity to other CPUs than CPU 0, which can't be disabled. I think theoretically it is possible for them to have different affinity, but it seems unlikely considering that x86 hotplug code also doesn't seem to do any Xen irq migration AFAIU.

What about arm_smmu_init_domain_context and its related call chains? As far as I can see, some of these paths touch XEN_DOMCTL_* hypercalls, and my understanding is they can be issued on any CPU.

Should we add a
check that no enabled (e)SPIs owned by Xen are pinned to the offlining CPU?

Quoted 10 lines

Quoted 9 lines

4. Race between the new hypercalls and disable/enable_nonboot_cpus():
disable_nonboot_cpus is called, enable_nonboot_cpus() reads frozen_cpus, and before it calls cpu_up() a hypercall onlines the CPU. cpu_up() then fails as "already online", but the CPU_RESUME_FAILED path may still run for an already-online CPU, risking use-after-free of per-CPU state (e.g. via free_percpu_area()) and other issues related to CPU_RESUME_FAILED notification.

Quoted 2 lines

There don't seem to be any calls to disable/enable_nonboot_cpus() on Arm.

If we take x86 as an example, then they are called with all domains

Quoted 1 line

already paused, and I don't see how paused domains can issue hypercalls.