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

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

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

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

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

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

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