[PATCH v1 0/4] Implement CPU hotplug on Arm

[PATCH v1 0/4] Implement CPU hotplug on Arm

[Mykyta Poturai]

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.

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

 config/arm64.mk                  |  1 +
 config/x86_32.mk                 |  1 +
 config/x86_64.mk                 |  1 +
 tools/libs/guest/Makefile.common |  4 +-
 tools/misc/Makefile              |  2 +-
 xen/arch/arm/gic.c               | 10 ++++-
 xen/arch/arm/sysctl.c            | 67 ++++++++++++++++++++++++++++++++
 xen/arch/arm/time.c              | 20 ++++++++--
 8 files changed, 98 insertions(+), 8 deletions(-)

-- 
2.34.1

[PATCH v1 1/4] arm/time: Use static irqaction

[Mykyta Poturai]

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 <mykyta_poturai@epam.com>
---
 xen/arch/arm/time.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index e74d30d258..6f215de210 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -303,6 +303,20 @@ static void check_timer_irq_cfg(unsigned int irq, const char *which)
            "WARNING: %s-timer IRQ%u is not level triggered.\n", which, irq);
 }
 
+static struct irqaction __read_mostly irq_hyp = {
+    .name = "hyptimer",
+    .handler = htimer_interrupt,
+    .dev_id = NULL,
+    .free_on_release = 0,
+};
+
+static struct irqaction __read_mostly irq_virt = {
+    .name = "virtimer",
+    .handler = vtimer_interrupt,
+    .dev_id = NULL,
+    .free_on_release = 0,
+};
+
 /* Set up the timer interrupt on this CPU */
 void init_timer_interrupt(void)
 {
@@ -314,10 +328,8 @@ 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);
+    setup_irq(timer_irq[TIMER_HYP_PPI], 0, &irq_hyp);
+    setup_irq(timer_irq[TIMER_VIRT_PPI], 0, &irq_virt);
 
     check_timer_irq_cfg(timer_irq[TIMER_HYP_PPI], "hypervisor");
     check_timer_irq_cfg(timer_irq[TIMER_VIRT_PPI], "virtual");
-- 
2.34.1

Re: [PATCH v1 1/4] arm/time: Use static irqaction

[Julien Grall]

Hi Mykyta,

On 18/09/2025 13:16, Mykyta Poturai wrote:
> 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 <mykyta_poturai@epam.com>
> ---
>   xen/arch/arm/time.c | 20 ++++++++++++++++----
>   1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
> index e74d30d258..6f215de210 100644
> --- a/xen/arch/arm/time.c
> +++ b/xen/arch/arm/time.c
> @@ -303,6 +303,20 @@ static void check_timer_irq_cfg(unsigned int irq, const char *which)
>              "WARNING: %s-timer IRQ%u is not level triggered.\n", which, irq);
>   }
>   
> +static struct irqaction __read_mostly irq_hyp = {
> +    .name = "hyptimer",
> +    .handler = htimer_interrupt,
> +    .dev_id = NULL,
> +    .free_on_release = 0,
> +};
> +
> +static struct irqaction __read_mostly irq_virt = {
> +    .name = "virtimer",
> +    .handler = vtimer_interrupt,
> +    .dev_id = NULL,
> +    .free_on_release = 0,
> +};

setup_irq() will update the field "next" in irqaction. So we need one 
instance per call. Effectively, this means one per CPU. Therefore, we 
want to use DEFINE_PER_CPU. This applies to the rest of the series.

Cheers,

-- 
Julien Grall

[PATCH v1 3/4] arm/sysctl: Implement cpu hotplug ops

[Mykyta Poturai]

Implement XEN_SYSCTL_CPU_HOTPLUG_* calls to allow for enabling/disabling
CPU cores in runtime.

Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
---
 xen/arch/arm/sysctl.c | 67 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
index 32cab4feff..ca8fb550fd 100644
--- a/xen/arch/arm/sysctl.c
+++ b/xen/arch/arm/sysctl.c
@@ -12,6 +12,7 @@
 #include <xen/dt-overlay.h>
 #include <xen/errno.h>
 #include <xen/hypercall.h>
+#include <xen/cpu.h>
 #include <asm/arm64/sve.h>
 #include <public/sysctl.h>
 
@@ -23,6 +24,68 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
                                        XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
 }
 
+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 smt_up_down_helper(void *data)
+{
+    bool up = (bool) data;
+    unsigned int cpu;
+    int ret;
+
+    for_each_present_cpu ( cpu )
+    {
+        if ( cpu == 0 )
+            continue;
+
+        if ( up )
+            ret = cpu_up(cpu);
+        else
+            ret = cpu_down(cpu);
+
+        if ( ret )
+            return ret;
+    }
+
+    return 0;
+}
+
+static long cpu_hotplug_sysctl(struct xen_sysctl_cpu_hotplug *hotplug)
+{
+    bool up;
+
+    switch (hotplug->op) {
+        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
+            if ( hotplug->cpu == 0 )
+                return -EINVAL;
+            return continue_hypercall_on_cpu(0, cpu_up_helper, _p(hotplug->cpu));
+
+        case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
+            if ( hotplug->cpu == 0 )
+                return -EINVAL;
+            return continue_hypercall_on_cpu(0, cpu_down_helper, _p(hotplug->cpu));
+
+        case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE:
+        case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE:
+            if ( CONFIG_NR_CPUS <= 1 )
+                return 0;
+            up = hotplug->op == XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE;
+            return continue_hypercall_on_cpu(0, smt_up_down_helper, _p(up));
+
+        default:
+            return -EINVAL;
+    }
+}
+
 long arch_do_sysctl(struct xen_sysctl *sysctl,
                     XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
 {
@@ -34,6 +97,10 @@ long arch_do_sysctl(struct xen_sysctl *sysctl,
         ret = dt_overlay_sysctl(&sysctl->u.dt_overlay);
         break;
 
+    case XEN_SYSCTL_cpu_hotplug:
+        ret = cpu_hotplug_sysctl(&sysctl->u.cpu_hotplug);
+        break;
+
     default:
         ret = -ENOSYS;
         break;
-- 
2.34.1

Re: [PATCH v1 3/4] arm/sysctl: Implement cpu hotplug ops

[Julien Grall]

Hi Mykyta,

On 18/09/2025 13:16, Mykyta Poturai wrote:
> Implement XEN_SYSCTL_CPU_HOTPLUG_* calls to allow for enabling/disabling
> CPU cores in runtime.
> 
> Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
> ---
>   xen/arch/arm/sysctl.c | 67 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 67 insertions(+)
> 
> diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
> index 32cab4feff..ca8fb550fd 100644
> --- a/xen/arch/arm/sysctl.c
> +++ b/xen/arch/arm/sysctl.c
> @@ -12,6 +12,7 @@
>   #include <xen/dt-overlay.h>
>   #include <xen/errno.h>
>   #include <xen/hypercall.h>
> +#include <xen/cpu.h>
>   #include <asm/arm64/sve.h>
>   #include <public/sysctl.h>
>   
> @@ -23,6 +24,68 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>                                          XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
>   }
>   
> +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 smt_up_down_helper(void *data)

Looking at the code, you will effectively disable all the CPUs but CPU0. 
But I don't understand why. From the name is goal seems to be disable 
SMT threading.

> +{
> +    bool up = (bool) data;
> +    unsigned int cpu;
> +    int ret;
> +
> +    for_each_present_cpu ( cpu )
> +    {
> +        if ( cpu == 0 )
> +            continue;
> +
> +        if ( up )
> +            ret = cpu_up(cpu);
> +        else
> +            ret = cpu_down(cpu);
> +

Regardless what I wrote above, you likely want to handle preemption.

> +        if ( ret )
> +            return ret;
 > +    }
> +
> +    return 0;
> +}
> +
> +static long cpu_hotplug_sysctl(struct xen_sysctl_cpu_hotplug *hotplug)
> +{
> +    bool up;
> +
> +    switch (hotplug->op) {
> +        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
> +            if ( hotplug->cpu == 0 )

I can't find a similar check on x86. Do you have any pointer?

> +                return -EINVAL;

On x86, they seem to check for XSM permission before continuing. Can you 
explain why this is not necessary? Same questions applies below.

> +            return continue_hypercall_on_cpu(0, cpu_up_helper, _p(hotplug->cpu));
> +
> +        case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
> +            if ( hotplug->cpu == 0 )
> +                return -EINVAL;
> +            return continue_hypercall_on_cpu(0, cpu_down_helper, _p(hotplug->cpu));
> +
> +        case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE:
> +        case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE:

Why are we implementing those helpers on Arm?

> +            if ( CONFIG_NR_CPUS <= 1 )
> +                return 0;
> +            up = hotplug->op == XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE;
> +            return continue_hypercall_on_cpu(0, smt_up_down_helper, _p(up));
> +
> +        default:
> +            return -EINVAL;
> +    }
> +}
> +
>   long arch_do_sysctl(struct xen_sysctl *sysctl,
>                       XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>   {
> @@ -34,6 +97,10 @@ long arch_do_sysctl(struct xen_sysctl *sysctl,
>           ret = dt_overlay_sysctl(&sysctl->u.dt_overlay);
>           break;
>   
> +    case XEN_SYSCTL_cpu_hotplug:

This will also enable CPU hotplug on 32-bit Arm. Is this what you 
intended? (I see patch #4 only mention 64-bit Arm).

> +        ret = cpu_hotplug_sysctl(&sysctl->u.cpu_hotplug);
> +        break;
> +
>       default:
>           ret = -ENOSYS;
>           break;

Cheers,

-- 
Julien Grall

Re: [PATCH v1 3/4] arm/sysctl: Implement cpu hotplug ops

[Jan Beulich]

On 18.09.2025 15:35, Julien Grall wrote:
> On 18/09/2025 13:16, Mykyta Poturai wrote:
>> +static long cpu_hotplug_sysctl(struct xen_sysctl_cpu_hotplug *hotplug)
>> +{
>> +    bool up;
>> +
>> +    switch (hotplug->op) {
>> +        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
>> +            if ( hotplug->cpu == 0 )
> 
> I can't find a similar check on x86. Do you have any pointer?

When CPU 0 cannot be brought down (see cpu_down()), tryin to bring it up
is kind of pointless, and hence can perhaps be short circuited like this?

Jan

Re: [PATCH v1 3/4] arm/sysctl: Implement cpu hotplug ops

[Julien Grall]

Hi Jan,

On 18/09/2025 15:51, Jan Beulich wrote:
> On 18.09.2025 15:35, Julien Grall wrote:
>> On 18/09/2025 13:16, Mykyta Poturai wrote:
>>> +static long cpu_hotplug_sysctl(struct xen_sysctl_cpu_hotplug *hotplug)
>>> +{
>>> +    bool up;
>>> +
>>> +    switch (hotplug->op) {
>>> +        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
>>> +            if ( hotplug->cpu == 0 )
>>
>> I can't find a similar check on x86. Do you have any pointer?
> 
> When CPU 0 cannot be brought down (see cpu_down()), tryin to bring it up
> is kind of pointless, and hence can perhaps be short circuited like this?

Thanks for the clarification, I missed the check in cpu_down(). That 
said, I don't see any value to short circuit it. In fact, I see this as 
more a risk because if we ever decide to allow CPU 0 to be offlined, 
then it would be more difficult to find places where we short circuit it.

So I would rather prefer if we remove the checks.

Cheers,

-- 
Julien Grall

Re: [PATCH v1 3/4] arm/sysctl: Implement cpu hotplug ops

[Jan Beulich]

On 23.09.2025 20:41, Julien Grall wrote:
> On 18/09/2025 15:51, Jan Beulich wrote:
>> On 18.09.2025 15:35, Julien Grall wrote:
>>> On 18/09/2025 13:16, Mykyta Poturai wrote:
>>>> +static long cpu_hotplug_sysctl(struct xen_sysctl_cpu_hotplug *hotplug)
>>>> +{
>>>> +    bool up;
>>>> +
>>>> +    switch (hotplug->op) {
>>>> +        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
>>>> +            if ( hotplug->cpu == 0 )
>>>
>>> I can't find a similar check on x86. Do you have any pointer?
>>
>> When CPU 0 cannot be brought down (see cpu_down()), tryin to bring it up
>> is kind of pointless, and hence can perhaps be short circuited like this?
> 
> Thanks for the clarification, I missed the check in cpu_down(). That 
> said, I don't see any value to short circuit it. In fact, I see this as 
> more a risk because if we ever decide to allow CPU 0 to be offlined, 
> then it would be more difficult to find places where we short circuit it.
> 
> So I would rather prefer if we remove the checks.

In fact I agree (and I merely wanted to point out the present situation):
CPU0 not (normally) being possible to be brought down is, I think, pretty
much an x86 thing. I.e. I think the check would want to go away from
common code.

Jan

Re: [PATCH v1 3/4] arm/sysctl: Implement cpu hotplug ops

[Mykyta Poturai]

On 18.09.25 16:35, Julien Grall wrote:
> Hi Mykyta,
> 
> On 18/09/2025 13:16, Mykyta Poturai wrote:
>> Implement XEN_SYSCTL_CPU_HOTPLUG_* calls to allow for enabling/disabling
>> CPU cores in runtime.
>>
>> Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
>> ---
>>   xen/arch/arm/sysctl.c | 67 +++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 67 insertions(+)
>>
>> diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
>> index 32cab4feff..ca8fb550fd 100644
>> --- a/xen/arch/arm/sysctl.c
>> +++ b/xen/arch/arm/sysctl.c
>> @@ -12,6 +12,7 @@
>>   #include <xen/dt-overlay.h>
>>   #include <xen/errno.h>
>>   #include <xen/hypercall.h>
>> +#include <xen/cpu.h>
>>   #include <asm/arm64/sve.h>
>>   #include <public/sysctl.h>
>> @@ -23,6 +24,68 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>>                                          
>> XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
>>   }
>> +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 smt_up_down_helper(void *data)
> 
> Looking at the code, you will effectively disable all the CPUs but CPU0. 
> But I don't understand why. From the name is goal seems to be disable 
> SMT threading.
>

Sorry I have slightly misunderstood the x86 implementation/reasoning of 
this ops. I will drop them in V2.

>> +{
>> +    bool up = (bool) data;
>> +    unsigned int cpu;
>> +    int ret;
>> +
>> +    for_each_present_cpu ( cpu )
>> +    {
>> +        if ( cpu == 0 )
>> +            continue;
>> +
>> +        if ( up )
>> +            ret = cpu_up(cpu);
>> +        else
>> +            ret = cpu_down(cpu);
>> +
> 
> Regardless what I wrote above, you likely want to handle preemption.
> 
>> +        if ( ret )
>> +            return ret;
>  > +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static long cpu_hotplug_sysctl(struct xen_sysctl_cpu_hotplug *hotplug)
>> +{
>> +    bool up;
>> +
>> +    switch (hotplug->op) {
>> +        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
>> +            if ( hotplug->cpu == 0 )
> 
> I can't find a similar check on x86. Do you have any pointer?

Jan correctly mentioned that CPU0 can't be disabled so this is a short 
circuit for clarity.

> 
>> +                return -EINVAL;
> 
> On x86, they seem to check for XSM permission before continuing. Can you 
> explain why this is not necessary? Same questions applies below.

I will add XSM checks thanks for pointing this out.

> 
>> +            return continue_hypercall_on_cpu(0, cpu_up_helper, 
>> _p(hotplug->cpu));
>> +
>> +        case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
>> +            if ( hotplug->cpu == 0 )
>> +                return -EINVAL;
>> +            return continue_hypercall_on_cpu(0, cpu_down_helper, 
>> _p(hotplug->cpu));
>> +
>> +        case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE:
>> +        case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE:
> 
> Why are we implementing those helpers on Arm?
> 
>> +            if ( CONFIG_NR_CPUS <= 1 )
>> +                return 0;
>> +            up = hotplug->op == XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE;
>> +            return continue_hypercall_on_cpu(0, smt_up_down_helper, 
>> _p(up));
>> +
>> +        default:
>> +            return -EINVAL;
>> +    }
>> +}
>> +
>>   long arch_do_sysctl(struct xen_sysctl *sysctl,
>>                       XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>>   {
>> @@ -34,6 +97,10 @@ long arch_do_sysctl(struct xen_sysctl *sysctl,
>>           ret = dt_overlay_sysctl(&sysctl->u.dt_overlay);
>>           break;
>> +    case XEN_SYSCTL_cpu_hotplug:
> 
> This will also enable CPU hotplug on 32-bit Arm. Is this what you 
> intended? (I see patch #4 only mention 64-bit Arm).

It wasn't intended. I will additionally check if it works on arm32 end 
explicitly specify it.

> 
>> +        ret = cpu_hotplug_sysctl(&sysctl->u.cpu_hotplug);
>> +        break;
>> +
>>       default:
>>           ret = -ENOSYS;
>>           break;
> 
> Cheers,
> 

-- 
Mykyta

Re: [PATCH v1 3/4] arm/sysctl: Implement cpu hotplug ops

[Julien Grall]

Hi Mykyta,

On 23/09/2025 14:37, Mykyta Poturai wrote:
> On 18.09.25 16:35, Julien Grall wrote:
>> Hi Mykyta,
>>
>> On 18/09/2025 13:16, Mykyta Poturai wrote:
>>> Implement XEN_SYSCTL_CPU_HOTPLUG_* calls to allow for enabling/disabling
>>> CPU cores in runtime.
>>>
>>> Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
>>> ---
>>>    xen/arch/arm/sysctl.c | 67 +++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 67 insertions(+)
>>>
>>> diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
>>> index 32cab4feff..ca8fb550fd 100644
>>> --- a/xen/arch/arm/sysctl.c
>>> +++ b/xen/arch/arm/sysctl.c
>>> @@ -12,6 +12,7 @@
>>>    #include <xen/dt-overlay.h>
>>>    #include <xen/errno.h>
>>>    #include <xen/hypercall.h>
>>> +#include <xen/cpu.h>
>>>    #include <asm/arm64/sve.h>
>>>    #include <public/sysctl.h>
>>> @@ -23,6 +24,68 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>>>                                           
>>> XEN_SYSCTL_PHYSCAP_ARM_SVE_MASK);
>>>    }
>>> +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 smt_up_down_helper(void *data)
>>
>> Looking at the code, you will effectively disable all the CPUs but CPU0.
>> But I don't understand why. From the name is goal seems to be disable
>> SMT threading.
>>
> 
> Sorry I have slightly misunderstood the x86 implementation/reasoning of
> this ops. I will drop them in V2.
> 
>>> +{
>>> +    bool up = (bool) data;
>>> +    unsigned int cpu;
>>> +    int ret;
>>> +
>>> +    for_each_present_cpu ( cpu )
>>> +    {
>>> +        if ( cpu == 0 )
>>> +            continue;
>>> +
>>> +        if ( up )
>>> +            ret = cpu_up(cpu);
>>> +        else
>>> +            ret = cpu_down(cpu);
>>> +
>>
>> Regardless what I wrote above, you likely want to handle preemption.
>>
>>> +        if ( ret )
>>> +            return ret;
>>   > +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static long cpu_hotplug_sysctl(struct xen_sysctl_cpu_hotplug *hotplug)
>>> +{
>>> +    bool up;
>>> +
>>> +    switch (hotplug->op) {
>>> +        case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
>>> +            if ( hotplug->cpu == 0 )
>>
>> I can't find a similar check on x86. Do you have any pointer?
> 
> Jan correctly mentioned that CPU0 can't be disabled so this is a short
> circuit for clarity.

I have replied to Jan. In short, the clarify you are referring is what 
would make more difficult to support offlining CPU0. So I would rather 
prefer if they are not present.
>>
>>> +            return continue_hypercall_on_cpu(0, cpu_up_helper,
>>> _p(hotplug->cpu));
>>> +
>>> +        case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
>>> +            if ( hotplug->cpu == 0 )
>>> +                return -EINVAL;
>>> +            return continue_hypercall_on_cpu(0, cpu_down_helper,
>>> _p(hotplug->cpu));
>>> +
>>> +        case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE:
>>> +        case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE:
>>
>> Why are we implementing those helpers on Arm?
>>
>>> +            if ( CONFIG_NR_CPUS <= 1 )
>>> +                return 0;
>>> +            up = hotplug->op == XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE;
>>> +            return continue_hypercall_on_cpu(0, smt_up_down_helper,
>>> _p(up));
>>> +
>>> +        default:
>>> +            return -EINVAL;
>>> +    }
>>> +}
>>> +
>>>    long arch_do_sysctl(struct xen_sysctl *sysctl,
>>>                        XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>>>    {
>>> @@ -34,6 +97,10 @@ long arch_do_sysctl(struct xen_sysctl *sysctl,
>>>            ret = dt_overlay_sysctl(&sysctl->u.dt_overlay);
>>>            break;
>>> +    case XEN_SYSCTL_cpu_hotplug:
>>
>> This will also enable CPU hotplug on 32-bit Arm. Is this what you
>> intended? (I see patch #4 only mention 64-bit Arm).
> 
> It wasn't intended. I will additionally check if it works on arm32 end
> explicitly specify it.

It will not work properly on arm32 because of the page table code. We 
have per-CPU pagetables (see init_domheap_mappings()) and they will need 
to be freed.

Note this is not a request to add support for arm32 CPU offlining.

Cheers,

-- 
Julien Grall

[PATCH v1 2/4] arm/gic: Use static irqaction

[Mykyta Poturai]

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 <mykyta_poturai@epam.com>
---
 xen/arch/arm/gic.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 260ee64cca..b00747a250 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -386,10 +386,16 @@ void gic_dump_info(struct vcpu *v)
     gic_hw_ops->dump_state(v);
 }
 
+static struct irqaction __read_mostly irq_maintenance = {
+    .name = "irq-maintenance",
+    .handler = maintenance_interrupt,
+    .dev_id = NULL,
+    .free_on_release = 0,
+};
+
 void init_maintenance_interrupt(void)
 {
-    request_irq(gic_hw_ops->info->maintenance_irq, 0, maintenance_interrupt,
-                "irq-maintenance", NULL);
+    setup_irq(gic_hw_ops->info->maintenance_irq, 0, &irq_maintenance);
 }
 
 int gic_make_hwdom_dt_node(const struct domain *d,
-- 
2.34.1

[PATCH v1 4/4] tools: Allow building xen-hptool without CONFIG_MIGRATE

[Mykyta Poturai]

With CPU hotplug sysctls implemented on Arm it becomes useful to have a tool
for calling them. Introduce new CONFIG_HOTPLUG to allow building hptool
separately from other migration tools and enable it for Arm.

Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
---
 config/arm64.mk                  | 1 +
 config/x86_32.mk                 | 1 +
 config/x86_64.mk                 | 1 +
 tools/libs/guest/Makefile.common | 4 +++-
 tools/misc/Makefile              | 2 +-
 5 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/config/arm64.mk b/config/arm64.mk
index c4662f67d0..d4995c8991 100644
--- a/config/arm64.mk
+++ b/config/arm64.mk
@@ -1,5 +1,6 @@
 CONFIG_ARM := y
 CONFIG_ARM_64 := y
+CONFIG_HOTPLUG := y
 
 CONFIG_XEN_INSTALL_SUFFIX :=
 
diff --git a/config/x86_32.mk b/config/x86_32.mk
index 3cc046d9bc..0c88862ad3 100644
--- a/config/x86_32.mk
+++ b/config/x86_32.mk
@@ -3,6 +3,7 @@ CONFIG_X86_32 := y
 
 CONFIG_MIGRATE := y
 CONFIG_XCUTILS := y
+CONFIG_HOTPLUG := y
 
 CFLAGS += -m32 -march=i686
 
diff --git a/config/x86_64.mk b/config/x86_64.mk
index 8614457b03..25cf965507 100644
--- a/config/x86_64.mk
+++ b/config/x86_64.mk
@@ -3,6 +3,7 @@ CONFIG_X86_64 := y
 
 CONFIG_MIGRATE := y
 CONFIG_XCUTILS := y
+CONFIG_HOTPLUG := y
 
 CONFIG_XEN_INSTALL_SUFFIX := .gz
 
diff --git a/tools/libs/guest/Makefile.common b/tools/libs/guest/Makefile.common
index a026a2f662..96a1141511 100644
--- a/tools/libs/guest/Makefile.common
+++ b/tools/libs/guest/Makefile.common
@@ -7,6 +7,9 @@ OBJS-y += xg_private.o
 OBJS-y += xg_domain.o
 OBJS-y += xg_suspend.o
 OBJS-y += xg_resume.o
+
+OBJS-$(CONFIG_HOTPLUG) += xg_offline_page.o
+
 ifeq ($(CONFIG_MIGRATE),y)
 OBJS-y += xg_sr_common.o
 OBJS-$(CONFIG_X86) += xg_sr_common_x86.o
@@ -17,7 +20,6 @@ OBJS-$(CONFIG_X86) += xg_sr_save_x86_pv.o
 OBJS-$(CONFIG_X86) += xg_sr_save_x86_hvm.o
 OBJS-y += xg_sr_restore.o
 OBJS-y += xg_sr_save.o
-OBJS-y += xg_offline_page.o
 else
 OBJS-y += xg_nomigrate.o
 endif
diff --git a/tools/misc/Makefile b/tools/misc/Makefile
index c26e544e83..64b4d77051 100644
--- a/tools/misc/Makefile
+++ b/tools/misc/Makefile
@@ -16,7 +16,7 @@ INSTALL_BIN                    += xencov_split
 INSTALL_BIN += $(INSTALL_BIN-y)
 
 # Everything to be installed in regular sbin/
-INSTALL_SBIN-$(CONFIG_MIGRATE) += xen-hptool
+INSTALL_SBIN-$(CONFIG_HOTPLUG) += xen-hptool
 INSTALL_SBIN-$(CONFIG_X86)     += xen-hvmcrash
 INSTALL_SBIN-$(CONFIG_X86)     += xen-hvmctx
 INSTALL_SBIN-$(CONFIG_X86)     += xen-lowmemd
-- 
2.34.1

Re: [PATCH v1 4/4] tools: Allow building xen-hptool without CONFIG_MIGRATE

[Jan Beulich]

On 18.09.2025 14:16, Mykyta Poturai wrote:
> --- a/config/arm64.mk
> +++ b/config/arm64.mk
> @@ -1,5 +1,6 @@
>  CONFIG_ARM := y
>  CONFIG_ARM_64 := y
> +CONFIG_HOTPLUG := y
>  
>  CONFIG_XEN_INSTALL_SUFFIX :=
>  
> --- a/config/x86_32.mk
> +++ b/config/x86_32.mk
> @@ -3,6 +3,7 @@ CONFIG_X86_32 := y
>  
>  CONFIG_MIGRATE := y
>  CONFIG_XCUTILS := y
> +CONFIG_HOTPLUG := y
>  
>  CFLAGS += -m32 -march=i686
>  
> --- a/config/x86_64.mk
> +++ b/config/x86_64.mk
> @@ -3,6 +3,7 @@ CONFIG_X86_64 := y
>  
>  CONFIG_MIGRATE := y
>  CONFIG_XCUTILS := y
> +CONFIG_HOTPLUG := y
>  
>  CONFIG_XEN_INSTALL_SUFFIX := .gz

I realize you only do what is already being done, but I question this
way of doing things when the scope is tools-only. Any CONFIG_* put here
cannot have an identically named, but potentially set differently
Kconfig setting in xen/: Any use of such in a Makefile could end up being
wrong, and would pretty surely end up being confusing.

Jan

Re: [PATCH v1 4/4] tools: Allow building xen-hptool without CONFIG_MIGRATE

[Mykyta Poturai]

On 18.09.25 17:59, Jan Beulich wrote:
> On 18.09.2025 14:16, Mykyta Poturai wrote:
>> --- a/config/arm64.mk
>> +++ b/config/arm64.mk
>> @@ -1,5 +1,6 @@
>>   CONFIG_ARM := y
>>   CONFIG_ARM_64 := y
>> +CONFIG_HOTPLUG := y
>>   
>>   CONFIG_XEN_INSTALL_SUFFIX :=
>>   
>> --- a/config/x86_32.mk
>> +++ b/config/x86_32.mk
>> @@ -3,6 +3,7 @@ CONFIG_X86_32 := y
>>   
>>   CONFIG_MIGRATE := y
>>   CONFIG_XCUTILS := y
>> +CONFIG_HOTPLUG := y
>>   
>>   CFLAGS += -m32 -march=i686
>>   
>> --- a/config/x86_64.mk
>> +++ b/config/x86_64.mk
>> @@ -3,6 +3,7 @@ CONFIG_X86_64 := y
>>   
>>   CONFIG_MIGRATE := y
>>   CONFIG_XCUTILS := y
>> +CONFIG_HOTPLUG := y
>>   
>>   CONFIG_XEN_INSTALL_SUFFIX := .gz
> 
> I realize you only do what is already being done, but I question this
> way of doing things when the scope is tools-only. Any CONFIG_* put here
> cannot have an identically named, but potentially set differently
> Kconfig setting in xen/: Any use of such in a Makefile could end up being
> wrong, and would pretty surely end up being confusing.
> 
> Jan

I have checked the whole codebase and didn't find any other 
CONFIG_HOTPLUG, the only similar thing is CONFIG_HOTPLUG_CPU. Anyway, I 
can change it to something more specific like CONFIG_HPTOOL for example.

-- 
Mykyta

Re: [PATCH v1 4/4] tools: Allow building xen-hptool without CONFIG_MIGRATE

[Jan Beulich]

On 23.09.2025 15:19, Mykyta Poturai wrote:
> On 18.09.25 17:59, Jan Beulich wrote:
>> On 18.09.2025 14:16, Mykyta Poturai wrote:
>>> --- a/config/arm64.mk
>>> +++ b/config/arm64.mk
>>> @@ -1,5 +1,6 @@
>>>   CONFIG_ARM := y
>>>   CONFIG_ARM_64 := y
>>> +CONFIG_HOTPLUG := y
>>>   
>>>   CONFIG_XEN_INSTALL_SUFFIX :=
>>>   
>>> --- a/config/x86_32.mk
>>> +++ b/config/x86_32.mk
>>> @@ -3,6 +3,7 @@ CONFIG_X86_32 := y
>>>   
>>>   CONFIG_MIGRATE := y
>>>   CONFIG_XCUTILS := y
>>> +CONFIG_HOTPLUG := y
>>>   
>>>   CFLAGS += -m32 -march=i686
>>>   
>>> --- a/config/x86_64.mk
>>> +++ b/config/x86_64.mk
>>> @@ -3,6 +3,7 @@ CONFIG_X86_64 := y
>>>   
>>>   CONFIG_MIGRATE := y
>>>   CONFIG_XCUTILS := y
>>> +CONFIG_HOTPLUG := y
>>>   
>>>   CONFIG_XEN_INSTALL_SUFFIX := .gz
>>
>> I realize you only do what is already being done, but I question this
>> way of doing things when the scope is tools-only. Any CONFIG_* put here
>> cannot have an identically named, but potentially set differently
>> Kconfig setting in xen/: Any use of such in a Makefile could end up being
>> wrong, and would pretty surely end up being confusing.
> 
> I have checked the whole codebase and didn't find any other 
> CONFIG_HOTPLUG, the only similar thing is CONFIG_HOTPLUG_CPU. Anyway, I 
> can change it to something more specific like CONFIG_HPTOOL for example.

Iff no other way (configure?) can be found, then yes, please use this more
specific name. But not extending this legacy way of "configuring" things
would be much preferred.

Jan

Re: [PATCH v1 0/4] Implement CPU hotplug on Arm

[Grygorii Strashko]

Hi Mykyta,

Thank a lot for your patches.

On 18.09.25 15:16, Mykyta Poturai wrote:
> 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.
> 
> Mykyta Poturai (4):
>    arm/time: Use static irqaction
>    arm/gic: Use static irqaction
>    arm/sysctl: Implement cpu hotplug ops
>    tools: Allow building xen-hptool without CONFIG_MIGRATE
> 
>   config/arm64.mk                  |  1 +
>   config/x86_32.mk                 |  1 +
>   config/x86_64.mk                 |  1 +
>   tools/libs/guest/Makefile.common |  4 +-
>   tools/misc/Makefile              |  2 +-
>   xen/arch/arm/gic.c               | 10 ++++-
>   xen/arch/arm/sysctl.c            | 67 ++++++++++++++++++++++++++++++++
>   xen/arch/arm/time.c              | 20 ++++++++--
>   8 files changed, 98 insertions(+), 8 deletions(-)
> 

Hence you introducing new feature for ARM I'd very much appreciated if you
add corresponding documentation under docs/hypervisor-guide/arm/.

-- 
Best regards,
-grygorii

Re: [PATCH v1 0/4] Implement CPU hotplug on Arm

[Julien Grall]

Hi Grygorii,

On 23/09/2025 17:09, Grygorii Strashko wrote:
> On 18.09.25 15:16, Mykyta Poturai wrote:
>> 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.
>>
>> Mykyta Poturai (4):
>>    arm/time: Use static irqaction
>>    arm/gic: Use static irqaction
>>    arm/sysctl: Implement cpu hotplug ops
>>    tools: Allow building xen-hptool without CONFIG_MIGRATE
>>
>>   config/arm64.mk                  |  1 +
>>   config/x86_32.mk                 |  1 +
>>   config/x86_64.mk                 |  1 +
>>   tools/libs/guest/Makefile.common |  4 +-
>>   tools/misc/Makefile              |  2 +-
>>   xen/arch/arm/gic.c               | 10 ++++-
>>   xen/arch/arm/sysctl.c            | 67 ++++++++++++++++++++++++++++++++
>>   xen/arch/arm/time.c              | 20 ++++++++--
>>   8 files changed, 98 insertions(+), 8 deletions(-)
>>
> 
> Hence you introducing new feature for ARM I'd very much appreciated if you
> add corresponding documentation under docs/hypervisor-guide/arm/.

I think some documentation is good. But why does this need to be Arm 
specific?

Cheers,

-- 
Julien Grall

Re: [PATCH v1 0/4] Implement CPU hotplug on Arm

[Grygorii Strashko]

On 23.09.25 21:38, Julien Grall wrote:
> Hi Grygorii,
> 
> On 23/09/2025 17:09, Grygorii Strashko wrote:
>> On 18.09.25 15:16, Mykyta Poturai wrote:
>>> 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.
>>>
>>> Mykyta Poturai (4):
>>>    arm/time: Use static irqaction
>>>    arm/gic: Use static irqaction
>>>    arm/sysctl: Implement cpu hotplug ops
>>>    tools: Allow building xen-hptool without CONFIG_MIGRATE
>>>
>>>   config/arm64.mk                  |  1 +
>>>   config/x86_32.mk                 |  1 +
>>>   config/x86_64.mk                 |  1 +
>>>   tools/libs/guest/Makefile.common |  4 +-
>>>   tools/misc/Makefile              |  2 +-
>>>   xen/arch/arm/gic.c               | 10 ++++-
>>>   xen/arch/arm/sysctl.c            | 67 ++++++++++++++++++++++++++++++++
>>>   xen/arch/arm/time.c              | 20 ++++++++--
>>>   8 files changed, 98 insertions(+), 8 deletions(-)
>>>
>>
>> Hence you introducing new feature for ARM I'd very much appreciated if you
>> add corresponding documentation under docs/hypervisor-guide/arm/.
> 
> I think some documentation is good. But why does this need to be Arm specific?

Only because this series is for ARM, if it could be generic (at least partially) - even better.


-- 
Best regards,
-grygorii