Implement CPU hotplug on Arm

Currently on Arm the desc->affinity mask of an irq is never updated, which makes it hard to know the actual affinity of an interrupt.

Fix this by updating the field in irq_set_affinity.

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

---
v5->v6:
* add missing locking around irq_set_affinity calls

v4->v5:
* add locking

v3->v4:
* patch introduced
---

xen/arch/arm/gic-vgic.c          |  2 ++
xen/arch/arm/irq.c               |  9 +++++++--
xen/arch/arm/vgic.c              | 14 ++++++++++++--
xen/arch/arm/vgic/vgic-mmio-v2.c | 11 +++++------
xen/arch/arm/vgic/vgic.c         | 15 ++++++++-------
5 files changed, 34 insertions(+), 17 deletions(-)

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

index ea48c5375a..5253caf002 100644
@@ -232,7 +232,9 @@ static void gic_update_one_lr(struct vcpu *v, int i)
            if ( test_bit(GIC_IRQ_GUEST_MIGRATING, &p->status) )
            {
                struct vcpu *v_target = vgic_get_target_vcpu(v, irq);
+                spin_lock(&p->desc->lock);
                irq_set_affinity(p->desc, cpumask_of(v_target->processor));
+                spin_unlock(&p->desc->lock);
                clear_bit(GIC_IRQ_GUEST_MIGRATING, &p->status);
            }
        }

xen/arch/arm/irq.c +7 -2

index 73e58a5108..7204bc2b68 100644
@@ -216,10 +216,15 @@ static inline struct domain *irq_get_domain(struct irq_desc *desc)
    return irq_get_guest_info(desc)->d;
 }
 
+/* Must be called with desc->lock held */
 void irq_set_affinity(struct irq_desc *desc, const cpumask_t *mask)
 {
-    if ( desc != NULL )
-        desc->handler->set_affinity(desc, mask);
+    if ( desc == NULL )
+        return;
+
+    ASSERT(spin_is_locked(&desc->lock));
+    cpumask_copy(desc->affinity, mask);
+    desc->handler->set_affinity(desc, mask);
 }
 
 int request_irq(unsigned int irq, unsigned int irqflags,

xen/arch/arm/vgic.c +12 -2

index 6647071ad4..c59f6873db 100644
@@ -445,7 +445,9 @@ bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq)
 
    if ( list_empty(&p->inflight) )
    {
+        spin_lock(&p->desc->lock);
        irq_set_affinity(p->desc, cpumask_of(new->processor));
+        spin_unlock(&p->desc->lock);
        spin_unlock_irqrestore(&old->arch.vgic.lock, flags);
        return true;
    }

@@ -453,7 +455,9 @@ bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq)
    if ( !list_empty(&p->lr_queue) )
    {
        vgic_remove_irq_from_queues(old, p);
+        spin_lock(&p->desc->lock);
        irq_set_affinity(p->desc, cpumask_of(new->processor));
+        spin_unlock(&p->desc->lock);
        spin_unlock_irqrestore(&old->arch.vgic.lock, flags);
        vgic_inject_irq(new->domain, new, irq, true);
        return true;

@@ -473,6 +477,7 @@ void arch_move_irqs(struct vcpu *v)
    struct domain *d = v->domain;
    struct pending_irq *p;
    struct vcpu *v_target;
+    unsigned long flags;
    int i;
 
    /*

@@ -494,7 +499,13 @@ void arch_move_irqs(struct vcpu *v)
        p = irq_to_pending(v_target, virq);
 
        if ( v_target == v && !test_bit(GIC_IRQ_GUEST_MIGRATING, &p->status) )
+        {
+            if ( !p->desc )
+                continue;
+            spin_lock_irqsave(&p->desc->lock, flags);
            irq_set_affinity(p->desc, cpu_mask);
+            spin_unlock_irqrestore(&p->desc->lock, flags);
+        }
    }
 }
 

@@ -574,8 +585,8 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, unsigned int n)
        spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
        if ( p->desc != NULL )
        {
-            irq_set_affinity(p->desc, cpumask_of(v_target->processor));
            spin_lock_irqsave(&p->desc->lock, flags);
+            irq_set_affinity(p->desc, cpumask_of(v_target->processor));
            /*
             * The irq cannot be a PPI, we only support delivery of SPIs
             * to guests.

@@ -944,4 +955,3 @@ void vgic_check_inflight_irqs_pending(struct vcpu *v, unsigned int rank, uint32_
 * indent-tabs-mode: nil
 * End:
 */
-

xen/arch/arm/vgic/vgic-mmio-v2.c +5 -6

index b7c2d7ce99..fc04741ca1 100644
@@ -159,24 +159,23 @@ static void vgic_mmio_write_target(struct vcpu *vcpu,
    for ( i = 0; i < len; i++ )
    {
        struct vgic_irq *irq = vgic_get_irq(vcpu->domain, NULL, intid + i);
+        struct irq_desc *desc = irq_to_desc(irq->hwintid);
 
-        spin_lock_irqsave(&irq->irq_lock, flags);
+        spin_lock_irqsave(&desc->lock, flags);
+        spin_lock(&irq->irq_lock);
 
        irq->targets = (val >> (i * 8)) & cpu_mask;
        if ( irq->targets )
        {
            irq->target_vcpu = vcpu->domain->vcpu[ffs(irq->targets) - 1];
            if ( irq->hw )
-            {
-                struct irq_desc *desc = irq_to_desc(irq->hwintid);
-
                irq_set_affinity(desc, cpumask_of(irq->target_vcpu->processor));
-            }
        }
        else
            irq->target_vcpu = NULL;
 
-        spin_unlock_irqrestore(&irq->irq_lock, flags);
+        spin_unlock(&irq->irq_lock);
+        spin_unlock_irqrestore(&desc->lock, flags);
        vgic_put_irq(vcpu->domain, irq);
    }
 }

xen/arch/arm/vgic/vgic.c +8 -7

index b2c0e1873a..81ba4099ef 100644
@@ -812,21 +812,22 @@ void arch_move_irqs(struct vcpu *v)
    {
        struct vgic_irq *irq = vgic_get_irq(d, NULL, i + VGIC_NR_PRIVATE_IRQS);
        unsigned long flags;
+        irq_desc_t *desc;
 
        if ( !irq )
            continue;
 
-        spin_lock_irqsave(&irq->irq_lock, flags);
+        desc = irq_to_desc(irq->hwintid);
 
-        /* Only hardware mapped vIRQs that are targeting this vCPU. */
-        if ( irq->hw && irq->target_vcpu == v)
-        {
-            irq_desc_t *desc = irq_to_desc(irq->hwintid);
+        spin_lock_irqsave(&desc->lock, flags);
+        spin_lock(&irq->irq_lock);
 
+        /* Only hardware mapped vIRQs that are targeting this vCPU. */
+        if ( irq->hw && irq->target_vcpu == v )
            irq_set_affinity(desc, cpumask_of(v->processor));
-        }
 
-        spin_unlock_irqrestore(&irq->irq_lock, flags);
+        spin_unlock(&irq->irq_lock);
+        spin_unlock_irqrestore(&desc->lock, flags);
        vgic_put_irq(d, irq);
    }
 }

Signature · 1 line

2.51.2

Currently on Arm the desc->affinity mask of an irq is never updated, which makes it hard to know the actual affinity of an interrupt.

Fix this by updating the field in irq_set_affinity.

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

---
v5->v6:
* add missing locking around irq_set_affinity calls

v4->v5:
* add locking

v3->v4:
* patch introduced

---
 xen/arch/arm/gic-vgic.c          |  2 ++
 xen/arch/arm/irq.c               |  9 +++++++--
 xen/arch/arm/vgic.c              | 14 ++++++++++++--
 xen/arch/arm/vgic/vgic-mmio-v2.c | 11 +++++------
 xen/arch/arm/vgic/vgic.c         | 15 ++++++++-------
 5 files changed, 34 insertions(+), 17 deletions(-)

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

index ea48c5375a..5253caf002 100644
@@ -232,7 +232,9 @@ static void gic_update_one_lr(struct vcpu *v, int i)
             if ( test_bit(GIC_IRQ_GUEST_MIGRATING, &p->status) )
             {
                 struct vcpu *v_target = vgic_get_target_vcpu(v, irq);
+                spin_lock(&p->desc->lock);
                 irq_set_affinity(p->desc, cpumask_of(v_target->processor));
+                spin_unlock(&p->desc->lock);
                 clear_bit(GIC_IRQ_GUEST_MIGRATING, &p->status);
             }
         }

xen/arch/arm/irq.c +7 -2

index 73e58a5108..7204bc2b68 100644
@@ -216,10 +216,15 @@ static inline struct domain *irq_get_domain(struct irq_desc *desc)
     return irq_get_guest_info(desc)->d;
 }
 
+/* Must be called with desc->lock held */
 void irq_set_affinity(struct irq_desc *desc, const cpumask_t *mask)
 {
-    if ( desc != NULL )
-        desc->handler->set_affinity(desc, mask);
+    if ( desc == NULL )
+        return;
+
+    ASSERT(spin_is_locked(&desc->lock));
+    cpumask_copy(desc->affinity, mask);
+    desc->handler->set_affinity(desc, mask);
 }
 
 int request_irq(unsigned int irq, unsigned int irqflags,

xen/arch/arm/vgic.c +12 -2

index 6647071ad4..c59f6873db 100644
@@ -445,7 +445,9 @@ bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq)
 
     if ( list_empty(&p->inflight) )
     {
+        spin_lock(&p->desc->lock);
         irq_set_affinity(p->desc, cpumask_of(new->processor));
+        spin_unlock(&p->desc->lock);
         spin_unlock_irqrestore(&old->arch.vgic.lock, flags);
         return true;
     }

@@ -453,7 +455,9 @@ bool vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int irq)
     if ( !list_empty(&p->lr_queue) )
     {
         vgic_remove_irq_from_queues(old, p);
+        spin_lock(&p->desc->lock);
         irq_set_affinity(p->desc, cpumask_of(new->processor));
+        spin_unlock(&p->desc->lock);
         spin_unlock_irqrestore(&old->arch.vgic.lock, flags);
         vgic_inject_irq(new->domain, new, irq, true);
         return true;

@@ -473,6 +477,7 @@ void arch_move_irqs(struct vcpu *v)
     struct domain *d = v->domain;
     struct pending_irq *p;
     struct vcpu *v_target;
+    unsigned long flags;
     int i;
 
     /*

@@ -494,7 +499,13 @@ void arch_move_irqs(struct vcpu *v)
         p = irq_to_pending(v_target, virq);
 
         if ( v_target == v && !test_bit(GIC_IRQ_GUEST_MIGRATING, &p->status) )
+        {
+            if ( !p->desc )
+                continue;
+            spin_lock_irqsave(&p->desc->lock, flags);
             irq_set_affinity(p->desc, cpu_mask);
+            spin_unlock_irqrestore(&p->desc->lock, flags);
+        }
     }
 }
 

@@ -574,8 +585,8 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, unsigned int n)
         spin_unlock_irqrestore(&v_target->arch.vgic.lock, flags);
         if ( p->desc != NULL )
         {
-            irq_set_affinity(p->desc, cpumask_of(v_target->processor));
             spin_lock_irqsave(&p->desc->lock, flags);
+            irq_set_affinity(p->desc, cpumask_of(v_target->processor));
             /*
              * The irq cannot be a PPI, we only support delivery of SPIs
              * to guests.

@@ -944,4 +955,3 @@ void vgic_check_inflight_irqs_pending(struct vcpu *v, unsigned int rank, uint32_
  * indent-tabs-mode: nil
  * End:
  */
-

xen/arch/arm/vgic/vgic-mmio-v2.c +5 -6

index b7c2d7ce99..fc04741ca1 100644
@@ -159,24 +159,23 @@ static void vgic_mmio_write_target(struct vcpu *vcpu,
     for ( i = 0; i < len; i++ )
     {
         struct vgic_irq *irq = vgic_get_irq(vcpu->domain, NULL, intid + i);
+        struct irq_desc *desc = irq_to_desc(irq->hwintid);
 
-        spin_lock_irqsave(&irq->irq_lock, flags);
+        spin_lock_irqsave(&desc->lock, flags);
+        spin_lock(&irq->irq_lock);
 
         irq->targets = (val >> (i * 8)) & cpu_mask;
         if ( irq->targets )
         {
             irq->target_vcpu = vcpu->domain->vcpu[ffs(irq->targets) - 1];
             if ( irq->hw )
-            {
-                struct irq_desc *desc = irq_to_desc(irq->hwintid);
-
                 irq_set_affinity(desc, cpumask_of(irq->target_vcpu->processor));
-            }
         }
         else
             irq->target_vcpu = NULL;
 
-        spin_unlock_irqrestore(&irq->irq_lock, flags);
+        spin_unlock(&irq->irq_lock);
+        spin_unlock_irqrestore(&desc->lock, flags);
         vgic_put_irq(vcpu->domain, irq);
     }
 }

xen/arch/arm/vgic/vgic.c +8 -7

index b2c0e1873a..81ba4099ef 100644
@@ -812,21 +812,22 @@ void arch_move_irqs(struct vcpu *v)
     {
         struct vgic_irq *irq = vgic_get_irq(d, NULL, i + VGIC_NR_PRIVATE_IRQS);
         unsigned long flags;
+        irq_desc_t *desc;
 
         if ( !irq )
             continue;
 
-        spin_lock_irqsave(&irq->irq_lock, flags);
+        desc = irq_to_desc(irq->hwintid);
 
-        /* Only hardware mapped vIRQs that are targeting this vCPU. */
-        if ( irq->hw && irq->target_vcpu == v)
-        {
-            irq_desc_t *desc = irq_to_desc(irq->hwintid);
+        spin_lock_irqsave(&desc->lock, flags);
+        spin_lock(&irq->irq_lock);
 
+        /* Only hardware mapped vIRQs that are targeting this vCPU. */
+        if ( irq->hw && irq->target_vcpu == v )
             irq_set_affinity(desc, cpumask_of(v->processor));
-        }
 
-        spin_unlock_irqrestore(&irq->irq_lock, flags);
+        spin_unlock(&irq->irq_lock);
+        spin_unlock_irqrestore(&desc->lock, flags);
         vgic_put_irq(d, irq);
     }
 }

Hi Mykyta,

Quoted 2 lines · Mykyta Poturai

Currently on Arm the desc->affinity mask of an irq is never updated, which makes it hard to know the actual affinity of an interrupt.

With this change you'll track affinity of hardware interrupts, but pure virtual interrupts still will not be tracked. Is it intended behaviour? I believe it should be mentioned in the commit message.

Move IRQs from dying CPU to the online ones when a CPU is getting offlined. When onlining, rebalance all IRQs in a round-robin fashion. Guest-bound IRQs are already handled by scheduler in the process of moving vCPUs to active pCPUs, so we only need to handle IRQs used by Xen itself.

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

---
v5->v6:
* don't do any balancing on boot
* only do balancing when cpu hotplug is enabled

v4->v5:
* handle CPU onlining as well
* more comments
* fix crash when ESPI is disabled
* don't assume CPU 0 is a boot CPU
* use insigned int for irq number
* remove assumption that all irqs a bound to CPU 0 by default from the commit message

v3->v4:
* patch introduced
---

xen/arch/arm/include/asm/irq.h |  4 +++
xen/arch/arm/irq.c             | 60 ++++++++++++++++++++++++++++++++++
xen/arch/arm/smpboot.c         |  8 +++++
3 files changed, 72 insertions(+)

xen/arch/arm/include/asm/irq.h +0 -0

index 09788dbfeb..a3897ec62d 100644

xen/arch/arm/include/asm/irq.h +1 -0

+
 #endif /* _ASM_HW_IRQ_H */
 /*
 * Local variables:

xen/arch/arm/irq.c +54 -0

index 7204bc2b68..d428d3118b 100644
@@ -158,6 +158,60 @@ static int init_local_irq_data(unsigned int cpu)
    return 0;
 }
 
+#ifdef CONFIG_CPU_HOTPLUG
+static int cpu_next;
+
+static void balance_irq(int irq, unsigned int from, bool up)
+{
+    struct irq_desc *desc = irq_to_desc(irq);
+    unsigned long flags;
+
+    ASSERT(!cpumask_empty(&cpu_online_map));
+
+    spin_lock_irqsave(&desc->lock, flags);
+    if ( likely(!desc->action) )
+        goto out;
+
+    if ( likely(test_bit(_IRQ_GUEST, &desc->status) ||
+                test_bit(_IRQ_MOVE_PENDING, &desc->status)) )
+        goto out;
+
+    /*
+     * Setting affinity to a mask of multiple CPUs causes the GIC drivers to
+     * select one CPU from that mask. If the dying CPU was included in the IRQ's
+     * affinity mask, we cannot determine exactly which CPU the interrupt is
+     * currently routed to, as GIC drivers lack a concrete get_affinity API. So
+     * to be safe we must reroute it to a new, definitely online, CPU. In the
+     * case of CPU going down, we move only the interrupt that could reside on
+     * it. Otherwise, we rearrange all interrupts in a round-robin fashion.
+     */
+    if ( !up && !cpumask_test_cpu(from, desc->affinity) )
+        goto out;
+
+    cpu_next = cpumask_cycle(cpu_next, &cpu_online_map);
+    irq_set_affinity(desc, cpumask_of(cpu_next));
+
+out:
+    spin_unlock_irqrestore(&desc->lock, flags);
+}
+
+void rebalance_irqs(unsigned int from, bool up)
+{
+    int irq;
+
+    if ( cpumask_empty(&cpu_online_map) )
+        return;
+
+    for ( irq = NR_LOCAL_IRQS; irq < NR_IRQS; irq++ )
+        balance_irq(irq, from, up);
+
+#ifdef CONFIG_GICV3_ESPI
+    for ( irq = ESPI_BASE_INTID; irq < ESPI_MAX_INTID; irq++ )
+        balance_irq(irq, from, up);
+#endif
+}
+#endif /* CONFIG_CPU_HOTPLUG */
+
 static int cpu_callback(struct notifier_block *nfb, unsigned long action,
                        void *hcpu)
 {

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

    }
 
    return notifier_from_errno(rc);

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

index 7f3cfa812e..f17e88e678 100644

xen/arch/arm/smpboot.c +1 -0

+
    /* Return to caller; eventually the IPI mechanism will unwind and the 
     * scheduler will drop to the idle loop, which will call stop_cpu(). */
 }

Signature · 1 line

2.51.2

Move XEN_SYSCTL_CPU_HOTPLUG_{ONLINE,OFFLINE} handlers to common code to allow for enabling/disabling CPU cores in runtime on Arm64.

SMT-disable enforcement check is moved into a separate architecture-specific function.

For now this operations only support Arm64. For proper Arm32 support, there needs to be a mechanism to free per-cpu page tables, allocated in init_domheap_mappings. Also, hotplug is not supported if ITS, FFA, or TEE is enabled, as they use non-static IRQ actions.

Create a Kconfig option CPU_HOTPLUG that reflects this constraints. On X86 the option is enabled unconditionally.

As cpu hotplug now has its own config option, switch flask to allow XEN_SYSCTL_cpu_hotplug depending on CONFIG_CPU_HOTPLUG, so it can work not only on x86.

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

---

v5->v6:
* fix style issues
* rename arch_smt_cpu_disable -> arch_cpu_can_stay_online and invert the logic
* use IS_ENABLED istead of ifdef
* remove explicit list af arch-specific SYSCTL_CPU_HOTPLUG_* options from the common handler
* fix flask issue

v4->v5:
* move handling to common code
* rename config to CPU_HOTPUG
* merge with "smp: Move cpu_up/down helpers to common code"

v3->v4:
* don't reimplement cpu_up/down helpers
* add Kconfig option
* fixup formatting

v2->v3:
* no changes

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

xen/arch/arm/smp.c                |  9 ++++++
xen/arch/ppc/stubs.c              |  4 +++
xen/arch/riscv/stubs.c            |  5 ++++
xen/arch/x86/include/asm/smp.h    |  3 --
xen/arch/x86/platform_hypercall.c | 12 ++++++++
xen/arch/x86/smp.c                | 33 ++--------------------
xen/arch/x86/sysctl.c             | 21 ++++++++------
xen/common/Kconfig                |  6 ++++
xen/common/smp.c                  | 35 +++++++++++++++++++++++
xen/common/sysctl.c               | 46 +++++++++++++++++++++++++++++++
xen/include/xen/smp.h             |  4 +++
xen/xsm/flask/hooks.c             |  2 +-
12 files changed, 137 insertions(+), 43 deletions(-)

xen/arch/arm/smp.c +9 -0

index b372472188..984f863a9a 100644
@@ -44,6 +44,15 @@ void smp_send_call_function_mask(const cpumask_t *mask)
    }
 }
 
+/*
+ * We currently don't support SMT on ARM so we don't need any special logic for
+ * CPU disabling
+ */
+bool arch_cpu_can_stay_online(unsigned int cpu)
+{
+    return true;
+}
+
 /*
 * Local variables:
 * mode: C

xen/arch/ppc/stubs.c +4 -0

index a333f06119..8f280ba080 100644
@@ -101,6 +101,10 @@ void smp_send_call_function_mask(const cpumask_t *mask)
    BUG_ON("unimplemented");
 }
 
+bool arch_cpu_can_stay_online(unsigned int cpu)
+{
+    BUG_ON("unimplemented");
+}
 /* irq.c */
 
 void irq_ack_none(struct irq_desc *desc)

xen/arch/riscv/stubs.c +5 -0

index daadff0138..7c3cda7bc5 100644
@@ -70,6 +70,11 @@ void smp_send_call_function_mask(const cpumask_t *mask)
    BUG_ON("unimplemented");
 }
 
+bool arch_cpu_can_stay_online(unsigned int cpu)
+{
+    BUG_ON("unimplemented");
+}
+
 /* irq.c */
 
 void irq_ack_none(struct irq_desc *desc)

xen/arch/x86/include/asm/smp.h +0 -3

index 3f16e62696..cb3e0fed19 100644
@@ -50,9 +50,6 @@ int cpu_add(uint32_t apic_id, uint32_t acpi_id, uint32_t pxm);
 
 void __stop_this_cpu(void);
 
-long cf_check cpu_up_helper(void *data);
-long cf_check cpu_down_helper(void *data);
-
 long cf_check core_parking_helper(void *data);
 bool core_parking_remove(unsigned int cpu);
 uint32_t get_cur_idle_nums(void);

xen/arch/x86/platform_hypercall.c +12 -0

index cd4f0ae5e5..e745151790 100644
@@ -735,6 +735,12 @@ ret_t do_platform_op(
    {
        int cpu = op->u.cpu_ol.cpuid;
 
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+        {
+            ret = -EOPNOTSUPP;
+            break;
+        }
+
        ret = xsm_resource_plug_core(XSM_HOOK);
        if ( ret )
            break;

@@ -761,6 +767,12 @@ ret_t do_platform_op(
    {
        int cpu = op->u.cpu_ol.cpuid;
 
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+        {
+            ret = -EOPNOTSUPP;
+            break;
+        }
+
        ret = xsm_resource_unplug_core(XSM_HOOK);
        if ( ret )
            break;

xen/arch/x86/smp.c +3 -30

index 7936294f5f..b781e933f2 100644
@@ -418,35 +418,8 @@ void cf_check call_function_interrupt(void)
    smp_call_function_interrupt();
 }
 
-long cf_check cpu_up_helper(void *data)
+bool arch_cpu_can_stay_online(unsigned int cpu)
 {
-    unsigned int cpu = (unsigned long)data;
-    int ret = cpu_up(cpu);
-
-    /* Have one more go on EBUSY. */
-    if ( ret == -EBUSY )
-        ret = cpu_up(cpu);
-
-    if ( !ret && !opt_smt &&
-         cpu_data[cpu].compute_unit_id == INVALID_CUID &&
-         cpumask_weight(per_cpu(cpu_sibling_mask, cpu)) > 1 )
-    {
-        ret = cpu_down_helper(data);
-        if ( ret )
-            printk("Could not re-offline CPU%u (%d)\n", cpu, ret);
-        else
-            ret = -EPERM;
-    }
-
-    return ret;
-}
-
-long cf_check cpu_down_helper(void *data)
-{
-    int cpu = (unsigned long)data;
-    int ret = cpu_down(cpu);
-    /* Have one more go on EBUSY. */
-    if ( ret == -EBUSY )
-        ret = cpu_down(cpu);
-    return ret;
+    return opt_smt || cpu_data[cpu].compute_unit_id != INVALID_CUID ||
+           cpumask_weight(per_cpu(cpu_sibling_mask, cpu)) <= 1;
 }

xen/arch/x86/sysctl.c +12 -9

index 1b04947516..35239b73c1 100644
@@ -49,6 +49,7 @@ static void cf_check l3_cache_get(void *arg)
 
 static long cf_check smt_up_down_helper(void *data)
 {
+    #ifdef CONFIG_CPU_HOTPLUG
    bool up = (bool)data;
    unsigned int cpu, sibling_mask = boot_cpu_data.x86_num_siblings - 1;
    int ret = 0;

@@ -89,6 +90,8 @@ static long cf_check smt_up_down_helper(void *data)
               up ? "enabled" : "disabled", CPUMASK_PR(&cpu_online_map));
 
    return ret;
+    #endif /* CONFIG_CPU_HOTPLUG */
+    return 0;
 }
 
 void arch_do_physinfo(struct xen_sysctl_physinfo *pi)

@@ -115,24 +118,24 @@ long arch_do_sysctl(
 
    case XEN_SYSCTL_cpu_hotplug:
    {
-        unsigned int cpu = sysctl->u.cpu_hotplug.cpu;
        unsigned int op  = sysctl->u.cpu_hotplug.op;
        bool plug;
        long (*fn)(void *data);
        void *hcpu;
 
-        switch ( op )
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
        {
-        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
-            plug = true;
-            fn = cpu_up_helper;
-            hcpu = _p(cpu);
+            ret = -EOPNOTSUPP;
            break;
+        }
 
+        switch ( op )
+        {
+        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
        case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
-            plug = false;
-            fn = cpu_down_helper;
-            hcpu = _p(cpu);
+            /* Handled by common code */
+            ASSERT_UNREACHABLE();
+            ret = -EOPNOTSUPP;
            break;
 
        case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE:

xen/common/Kconfig +0 -0

index d7e79e752a..bb73990355 100644

xen/common/Kconfig +2 -0

+
+

menu "Supported hypercall interfaces"
visible if EXPERT

xen/common/smp.c +35 -0

index a011f541f1..e2bf82856e 100644
@@ -16,6 +16,7 @@
 * GNU General Public License for more details.
 */
 
+#include <xen/cpu.h>
 #include <asm/hardirq.h>
 #include <asm/processor.h>
 #include <xen/spinlock.h>

@@ -104,6 +105,40 @@ void smp_call_function_interrupt(void)
    irq_exit();
 }
 
+#ifdef CONFIG_CPU_HOTPLUG
+long cf_check cpu_up_helper(void *data)
+{
+    unsigned int cpu = (unsigned long)data;
+    int ret = cpu_up(cpu);
+
+    /* Have one more go on EBUSY. */
+    if ( ret == -EBUSY )
+        ret = cpu_up(cpu);
+
+    if ( !ret && !arch_cpu_can_stay_online(cpu) )
+    {
+        ret = cpu_down_helper(data);
+        if ( ret )
+            printk("Could not re-offline CPU%u (%d)\n", cpu, ret);
+        else
+            ret = -EPERM;
+    }
+
+    return ret;
+}
+
+long cf_check cpu_down_helper(void *data)
+{
+    unsigned int cpu = (unsigned long)data;
+    int ret = cpu_down(cpu);
+
+    /* Have one more go on EBUSY. */
+    if ( ret == -EBUSY )
+        ret = cpu_down(cpu);
+    return ret;
+}
+#endif /* CONFIG_CPU_HOTPLUG */
+
 /*
 * Local variables:
 * mode: C

xen/common/sysctl.c +46 -0

index 5207664252..51a3dd699a 100644
@@ -483,6 +483,52 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
            copyback = 1;
        break;
 
+    case XEN_SYSCTL_cpu_hotplug:
+    {
+        unsigned int cpu = op->u.cpu_hotplug.cpu;
+        unsigned int hp_op = op->u.cpu_hotplug.op;
+        bool plug;
+        long (*fn)(void *data);
+        void *hcpu;
+
+        ret = -EOPNOTSUPP;
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+            break;
+
+        switch ( hp_op )
+        {
+        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
+            plug = true;
+            fn = cpu_up_helper;
+            hcpu = _p(cpu);
+            break;
+
+        case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
+            plug = false;
+            fn = cpu_down_helper;
+            hcpu = _p(cpu);
+            break;
+
+        default:
+            fn = NULL;
+            break;
+        }
+
+        if ( fn )
+        {
+            ret = plug ? xsm_resource_plug_core(XSM_HOOK)
+                       : xsm_resource_unplug_core(XSM_HOOK);
+
+            if ( !ret )
+                ret = continue_hypercall_on_cpu(0, fn, hcpu);
+
+            break;
+        }
+
+        /* Use the arch handler for cases not handled here */
+        fallthrough;
+    }
+
    default:
        ret = arch_do_sysctl(op, u_sysctl);
        copyback = 0;

xen/include/xen/smp.h +4 -0

index 2ca9ff1bfc..04530738c9 100644
@@ -76,4 +76,8 @@ extern void *stack_base[NR_CPUS];
 void initialize_cpu_data(unsigned int cpu);
 int setup_cpu_root_pgt(unsigned int cpu);
 
+bool arch_cpu_can_stay_online(unsigned int cpu);
+long cf_check cpu_up_helper(void *data);
+long cf_check cpu_down_helper(void *data);
+
 #endif /* __XEN_SMP_H__ */

xen/xsm/flask/hooks.c +1 -1

index b250b27065..36d357cae8 100644
@@ -835,7 +835,7 @@ static int cf_check flask_sysctl(int cmd)
    case XEN_SYSCTL_getdomaininfolist:
    case XEN_SYSCTL_page_offline_op:
    case XEN_SYSCTL_scheduler_op:
-#ifdef CONFIG_X86
+#ifdef CONFIG_CPU_HOTPLUG
    case XEN_SYSCTL_cpu_hotplug:
 #endif
        return 0;

Signature · 1 line

2.51.2

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

@@ -44,6 +44,15 @@ void smp_send_call_function_mask(const cpumask_t *mask)
     }

xen/arch/x86/platform_hypercall.c +6 -0

@@ -735,6 +735,12 @@ ret_t do_platform_op(
     {
         int cpu = op->u.cpu_ol.cpuid;
 
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+        {
+            ret = -EOPNOTSUPP;
+            break;
+        }
+
         ret = xsm_resource_plug_core(XSM_HOOK);
         if ( ret )
             break;

@@ -761,6 +767,12 @@ ret_t do_platform_op(
     {
         int cpu = op->u.cpu_ol.cpuid;

xen/arch/x86/smp.c +1 -23

@@ -418,35 +418,8 @@ void cf_check call_function_interrupt(void)
     smp_call_function_interrupt();
 }
 
-long cf_check cpu_up_helper(void *data)
+bool arch_cpu_can_stay_online(unsigned int cpu)
 {
-    unsigned int cpu = (unsigned long)data;
-    int ret = cpu_up(cpu);
-
-    /* Have one more go on EBUSY. */
-    if ( ret == -EBUSY )
-        ret = cpu_up(cpu);
-
-    if ( !ret && !opt_smt &&
-         cpu_data[cpu].compute_unit_id == INVALID_CUID &&
-         cpumask_weight(per_cpu(cpu_sibling_mask, cpu)) > 1 )
-    {
-        ret = cpu_down_helper(data);
-        if ( ret )
-            printk("Could not re-offline CPU%u (%d)\n", cpu, ret);
-        else
-            ret = -EPERM;
-    }
-
-    return ret;
-}
-
-long cf_check cpu_down_helper(void *data)

xen/arch/x86/sysctl.c +1 -0

@@ -49,6 +49,7 @@ static void cf_check l3_cache_get(void *arg)
 
 static long cf_check smt_up_down_helper(void *data)
 {
+    #ifdef CONFIG_CPU_HOTPLUG
     bool up = (bool)data;
     unsigned int cpu, sibling_mask = boot_cpu_data.x86_num_siblings - 1;
     int ret = 0;

xen/arch/x86/sysctl.c +0 -1

@@ -115,24 +118,24 @@ long arch_do_sysctl(
 
     case XEN_SYSCTL_cpu_hotplug:
     {
-        unsigned int cpu = sysctl->u.cpu_hotplug.cpu;
         unsigned int op  = sysctl->u.cpu_hotplug.op;
         bool plug;
         long (*fn)(void *data);
         void *hcpu;

xen/common/sysctl.c +31 -0

+        ret = -EOPNOTSUPP;
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+            break;
+
+        switch ( hp_op )
+        {
+        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
+            plug = true;
+            fn = cpu_up_helper;
+            hcpu = _p(cpu);
+            break;
+
+        case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
+            plug = false;
+            fn = cpu_down_helper;
+            hcpu = _p(cpu);
+            break;
+
+        default:
+            fn = NULL;
+            break;
+        }
+
+        if ( fn )
+        {
+            ret = plug ? xsm_resource_plug_core(XSM_HOOK)
+                       : xsm_resource_unplug_core(XSM_HOOK);
+
+            if ( !ret )
+                ret = continue_hypercall_on_cpu(0, fn, hcpu);
+

Quoted 4 lines · Mykyta Poturai

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

@@ -44,6 +44,15 @@ void smp_send_call_function_mask(const cpumask_t *mask)
      }

xen/arch/arm/smp.c void smp_send_call_function_mask(const cpumask_t *mask) · line 54

Quoted 10 lines

xen/arch/arm/smp.c +8 -0

  }
 
+/*
+ * We currently don't support SMT on ARM so we don't need any special logic for
+ * CPU disabling
+ */
+bool arch_cpu_can_stay_online(unsigned int cpu)
+{
+    return true;
+}

Something as simple as this would be nice to be an inline function (or, less desirably, a macro).

Quoted 19 lines

xen/arch/x86/platform_hypercall.c +6 -0

@@ -735,6 +735,12 @@ ret_t do_platform_op(
      {
          int cpu = op->u.cpu_ol.cpuid;
 
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+        {
+            ret = -EOPNOTSUPP;
+            break;
+        }
+
          ret = xsm_resource_plug_core(XSM_HOOK);
          if ( ret )
              break;

@@ -761,6 +767,12 @@ ret_t do_platform_op(
      {
          int cpu = op->u.cpu_ol.cpuid;
 

xen/arch/x86/platform_hypercall.c ret_t do_platform_op( · line 778

Quoted 9 lines

xen/arch/x86/platform_hypercall.c +6 -0

+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+        {
+            ret = -EOPNOTSUPP;
+            break;
+        }
+
          ret = xsm_resource_unplug_core(XSM_HOOK);
          if ( ret )
              break;

I wonder whether on x86 this really should become an optional thing (and if so, whether that wouldn't better be a separate change with proper justification). See also the comment on common/Kconfig further down - by the name of the option, and given the support status the change above may be legitimate, but not some of the similar restrictions added elsewhere.

Quoted 31 lines

xen/arch/x86/smp.c +1 -23

@@ -418,35 +418,8 @@ void cf_check call_function_interrupt(void)
      smp_call_function_interrupt();
  }
 
-long cf_check cpu_up_helper(void *data)
+bool arch_cpu_can_stay_online(unsigned int cpu)
  {
-    unsigned int cpu = (unsigned long)data;
-    int ret = cpu_up(cpu);
-
-    /* Have one more go on EBUSY. */
-    if ( ret == -EBUSY )
-        ret = cpu_up(cpu);
-
-    if ( !ret && !opt_smt &&
-         cpu_data[cpu].compute_unit_id == INVALID_CUID &&
-         cpumask_weight(per_cpu(cpu_sibling_mask, cpu)) > 1 )
-    {
-        ret = cpu_down_helper(data);
-        if ( ret )
-            printk("Could not re-offline CPU%u (%d)\n", cpu, ret);
-        else
-            ret = -EPERM;
-    }
-
-    return ret;
-}
-
-long cf_check cpu_down_helper(void *data)

xen/arch/x86/smp.c void cf_check call_function_interrupt(void) · line 425

Quoted 10 lines

xen/arch/x86/smp.c +2 -7

-{
-    int cpu = (unsigned long)data;
-    int ret = cpu_down(cpu);
-    /* Have one more go on EBUSY. */
-    if ( ret == -EBUSY )
-        ret = cpu_down(cpu);
-    return ret;
+    return opt_smt || cpu_data[cpu].compute_unit_id != INVALID_CUID ||
+           cpumask_weight(per_cpu(cpu_sibling_mask, cpu)) <= 1;
  }

Unlike for Arm, this may indeed better be an out-of-line function.

Quoted 11 lines

xen/arch/x86/sysctl.c +1 -0

@@ -49,6 +49,7 @@ static void cf_check l3_cache_get(void *arg)
 
  static long cf_check smt_up_down_helper(void *data)
  {
+    #ifdef CONFIG_CPU_HOTPLUG
      bool up = (bool)data;
      unsigned int cpu, sibling_mask = boot_cpu_data.x86_num_siblings - 1;
      int ret = 0;

xen/arch/x86/sysctl.c static long cf_check smt_up_down_helper(void *data) · line 95

Quoted 6 lines

xen/arch/x86/sysctl.c +2 -0

@@ -89,6 +90,8 @@ static long cf_check smt_up_down_helper(void *data)
                 up ? "enabled" : "disabled", CPUMASK_PR(&cpu_online_map));
 
      return ret;
+    #endif /* CONFIG_CPU_HOTPLUG */
+    return 0;
  }

The #-es or pre-processor directives want to be in the very first column.

Sharing "return ret" would also be nice, imo. Would require ret's decl to move ahead of the #ifdef. Actually - is there anything preventing

if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
return 0;

at the top of the function? Perhaps even with ASSERT_UNREACHABLE() added in?

Quoted 9 lines

xen/arch/x86/sysctl.c +0 -1

@@ -115,24 +118,24 @@ long arch_do_sysctl(
 
      case XEN_SYSCTL_cpu_hotplug:
      {
-        unsigned int cpu = sysctl->u.cpu_hotplug.cpu;
          unsigned int op  = sysctl->u.cpu_hotplug.op;
          bool plug;
          long (*fn)(void *data);
          void *hcpu;

xen/arch/x86/sysctl.c long arch_do_sysctl( · line 129

Quoted 10 lines

xen/arch/x86/sysctl.c +2 -5

 
-        switch ( op )
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
          {
-        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
-            plug = true;
-            fn = cpu_up_helper;
-            hcpu = _p(cpu);
+            ret = -EOPNOTSUPP;
              break;

ASSERT_UNREACHABLE() looks to also be valid to be added here, seeing how do_sysctl() now works.

xen/common/Kconfig config SYSTEM_SUSPEND · line 641

Quoted 8 lines

xen/common/Kconfig +2 -0

@@ -637,6 +637,12 @@ config SYSTEM_SUSPEND
 
  	  If unsure, say N.
 
+config CPU_HOTPLUG
+	bool "Enable CPU hotplug"

I'm not happy with this prompt. For x86 SUPPORT.md declares (ACPI) CPU hotplug as experimental. That's physical hotplug. The code you're fiddling with, however, is also used for soft-{off,on}lining. Which, e.g. to disable SMT on x86, may need to be used for security purposes.

xen/common/Kconfig config SYSTEM_SUSPEND · line 642

Quoted 1 line

xen/common/Kconfig +1 -0

+	depends on (X86 || ARM_64) && !FFA && !TEE && !HAS_ITS

What if on x86 FFA, TEE, or ITS gain a meaning?

xen/common/Kconfig config SYSTEM_SUSPEND · line 645

Quoted 3 lines

xen/common/Kconfig +3 -0

+	default y
+
+

Nit: No double blank lines please.

xen/common/sysctl.c long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl) · line 488

Quoted 9 lines

xen/common/sysctl.c +3 -0

@@ -483,6 +483,52 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
              copyback = 1;
          break;
 
+    case XEN_SYSCTL_cpu_hotplug:
+    {
+        unsigned int cpu = op->u.cpu_hotplug.cpu;

I don't think this variable is very useful to keep. Instead use ...

xen/common/sysctl.c long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl) · line 492

Quoted 4 lines

xen/common/sysctl.c +4 -0

+        unsigned int hp_op = op->u.cpu_hotplug.op;
+        bool plug;
+        long (*fn)(void *data);
+        void *hcpu;

         void *hcpu = _p(op->u.cpu_hotplug.op);

right here, dropping the assignments further down.

Quoted 31 lines

xen/common/sysctl.c +31 -0

+        ret = -EOPNOTSUPP;
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+            break;
+
+        switch ( hp_op )
+        {
+        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
+            plug = true;
+            fn = cpu_up_helper;
+            hcpu = _p(cpu);
+            break;
+
+        case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
+            plug = false;
+            fn = cpu_down_helper;
+            hcpu = _p(cpu);
+            break;
+
+        default:
+            fn = NULL;
+            break;
+        }
+
+        if ( fn )
+        {
+            ret = plug ? xsm_resource_plug_core(XSM_HOOK)
+                       : xsm_resource_unplug_core(XSM_HOOK);
+
+            if ( !ret )
+                ret = continue_hypercall_on_cpu(0, fn, hcpu);
+

xen/common/sysctl.c long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl) · line 534

Quoted 10 lines

xen/common/sysctl.c +7 -0

+            break;
+        }
+
+        /* Use the arch handler for cases not handled here */
+        fallthrough;
+    }
+
      default:
          ret = arch_do_sysctl(op, u_sysctl);
          copyback = 0;

This form of falling through may be a little risky, towards someone not looking closely enough and inserting another case label immediately ahead of the default one. While I don't think there's a really good solution to this, please consider

     }
         /* Use the arch handler for cases not handled above */
         fallthrough;
     default:

instead.

xen/xsm/flask/hooks.c static int cf_check flask_sysctl(int cmd) · line 841

Quoted 11 lines

xen/xsm/flask/hooks.c +1 -1

@@ -835,7 +835,7 @@ static int cf_check flask_sysctl(int cmd)
      case XEN_SYSCTL_getdomaininfolist:
      case XEN_SYSCTL_page_offline_op:
      case XEN_SYSCTL_scheduler_op:
-#ifdef CONFIG_X86
+#ifdef CONFIG_CPU_HOTPLUG
      case XEN_SYSCTL_cpu_hotplug:
  #endif
          return 0;

Is there a reason the #ifdef can't simply be dropped?

Jan

Quoted 20 lines · Jan Beulich

Quoted 19 lines · Mykyta Poturai

xen/arch/x86/platform_hypercall.c +6 -0

@@ -735,6 +735,12 @@ ret_t do_platform_op(
      {
          int cpu = op->u.cpu_ol.cpuid;
 
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+        {
+            ret = -EOPNOTSUPP;
+            break;
+        }
+
          ret = xsm_resource_plug_core(XSM_HOOK);
          if ( ret )
              break;

@@ -761,6 +767,12 @@ ret_t do_platform_op(
      {
          int cpu = op->u.cpu_ol.cpuid;
 

Quoted 15 lines

xen/arch/x86/platform_hypercall.c ret_t do_platform_op( · line 778

Quoted 9 lines

xen/arch/x86/platform_hypercall.c +6 -0

+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+        {
+            ret = -EOPNOTSUPP;
+            break;
+        }
+
          ret = xsm_resource_unplug_core(XSM_HOOK);
          if ( ret )
              break;

I wonder whether on x86 this really should become an optional thing (and if so, whether that wouldn't better be a separate change with proper justification). See also the comment on common/Kconfig further down - by the name of the option, and given the support status the change above may be legitimate, but not some of the similar restrictions added elsewhere.

Maybe force it to be always on like x86 then? I don't really have a justification for making it optional on x86, it just happened as side effect of creating a config option.

Quoted 68 lines

xen/common/sysctl.c long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl) · line 488

Quoted 9 lines

xen/common/sysctl.c +3 -0

@@ -483,6 +483,52 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
              copyback = 1;
          break;
 
+    case XEN_SYSCTL_cpu_hotplug:
+    {
+        unsigned int cpu = op->u.cpu_hotplug.cpu;

I don't think this variable is very useful to keep. Instead use ...

xen/common/sysctl.c long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl) · line 492

Quoted 4 lines

xen/common/sysctl.c +4 -0

+        unsigned int hp_op = op->u.cpu_hotplug.op;
+        bool plug;
+        long (*fn)(void *data);
+        void *hcpu;

         void *hcpu = _p(op->u.cpu_hotplug.op);

right here, dropping the assignments further down.

Quoted 31 lines

xen/common/sysctl.c +31 -0

+        ret = -EOPNOTSUPP;
+        if ( !IS_ENABLED(CONFIG_CPU_HOTPLUG) )
+            break;
+
+        switch ( hp_op )
+        {
+        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
+            plug = true;
+            fn = cpu_up_helper;
+            hcpu = _p(cpu);
+            break;
+
+        case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
+            plug = false;
+            fn = cpu_down_helper;
+            hcpu = _p(cpu);
+            break;
+
+        default:
+            fn = NULL;
+            break;
+        }
+
+        if ( fn )
+        {
+            ret = plug ? xsm_resource_plug_core(XSM_HOOK)
+                       : xsm_resource_unplug_core(XSM_HOOK);
+
+            if ( !ret )
+                ret = continue_hypercall_on_cpu(0, fn, hcpu);
+

xen/common/sysctl.c long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl) · line 534

Quoted 10 lines

xen/common/sysctl.c +7 -0

+            break;
+        }
+
+        /* Use the arch handler for cases not handled here */
+        fallthrough;
+    }
+
      default:
          ret = arch_do_sysctl(op, u_sysctl);
          copyback = 0;

This form of falling through may be a little risky, towards someone not looking closely enough and inserting another case label immediately ahead of the default one. While I don't think there's a really good solution to this, please consider

Quoted 7 lines

     }
         /* Use the arch handler for cases not handled above */
         fallthrough;
     default:

instead.

Just want to clarirfy if I got the idea. Is this what you meant?

     switch ( op->cmd )
     {
     ....
     case XEN_SYSCTL_cpu_hotplug:
     {
     ....
     }

         /* Use the arch handler for cases not handled here */
         fallthrough;
     default:
         ret = arch_do_sysctl(op, u_sysctl);
         copyback = 0;
         break;
     }

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

---

v5->v6:
* no changes

v4->v5:
* s/supported/implemented/
* update SUPPORT.md

v3->v4:
* update configuration section

v2->v3:
* patch introduced
---
SUPPORT.md | 1 +
docs/misc/cpu-hotplug.txt | 50 +++++++++++++++++++++++++++++++++++++++

2 files changed, 51 insertions(+)

create mode 100644 docs/misc/cpu-hotplug.txt

SUPPORT.md +1 -0

index d441bccf37..7b93ae69e7 100644
@@ -52,6 +52,7 @@ For the Cortex A77 r0p0 - r1p0, see Errata 1508412.
 ### ACPI CPU Hotplug
 
    Status, x86: Experimental
+    Status, Arm64: Experimental
 
 ### Physical Memory
 

docs/misc/cpu-hotplug.txt new file +40 -0

new file mode 100644
index 0000000000..45f20c2002
@@ -0,0 +1,50 @@
+CPU Hotplug
+===========
+
+CPU hotplug is a feature that allows pCPU cores to be added to or removed from a
+running system without requiring a reboot. It is implemented on x86 and Arm64
+architectures.
+
+Implementation Details
+----------------------
+
+CPU hotplug is implemented through the `XEN_SYSCTL_CPU_HOTPLUG_*` sysctl calls.
+The specific calls are:
+
+- `XEN_SYSCTL_CPU_HOTPLUG_ONLINE`: Brings a pCPU online
+- `XEN_SYSCTL_CPU_HOTPLUG_OFFLINE`: Takes a pCPU offline
+- `XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE`: Enables SMT threads (x86 only)
+- `XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE`: Disables SMT threads (x86 only)
+
+All cores can be disabled, assuming hardware support, except for the boot core.
+Sysctl calls are routed to the boot core before doing any actual up/down
+operations on other cores.
+
+Configuration
+-------------
+
+The presence of the feature is controlled by CONFIG_CPU_HOTPLUG option. It is
+enabled by default on x86 architecture. On Arm64, the option is enabled by
+default when ITS, FFA, and TEE configs are disabled.
+xen-hptool userspace tool is built unconditionally.
+
+Usage
+-----
+
+Disable core:
+
+$ xen-hptool cpu-offline 2
+Prepare to offline CPU 2
+(XEN) Removing cpu 2 from runqueue 0
+CPU 2 offlined successfully
+

Signature · 1 line

2.51.2