Re: [PATCH v6 3/5] arm/sysctl: Implement cpu hotplug ops

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Mykyta,

> On 12 Mar 2026, at 10:39, Mykyta Poturai <Mykyta_Poturai@xxxxxxxx> wrote:
> 
> 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 <mykyta_poturai@xxxxxxxx>
> 
> ---
> 
> 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(-)
> 
> diff --git a/xen/arch/arm/smp.c b/xen/arch/arm/smp.c
> index b372472188..984f863a9a 100644
> --- a/xen/arch/arm/smp.c
> +++ b/xen/arch/arm/smp.c
> @@ -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
> diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
> index a333f06119..8f280ba080 100644
> --- a/xen/arch/ppc/stubs.c
> +++ b/xen/arch/ppc/stubs.c
> @@ -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)
> diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
> index daadff0138..7c3cda7bc5 100644
> --- a/xen/arch/riscv/stubs.c
> +++ b/xen/arch/riscv/stubs.c
> @@ -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)
> diff --git a/xen/arch/x86/include/asm/smp.h b/xen/arch/x86/include/asm/smp.h
> index 3f16e62696..cb3e0fed19 100644
> --- a/xen/arch/x86/include/asm/smp.h
> +++ b/xen/arch/x86/include/asm/smp.h
> @@ -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);
> diff --git a/xen/arch/x86/platform_hypercall.c 
> b/xen/arch/x86/platform_hypercall.c
> index cd4f0ae5e5..e745151790 100644
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -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;
> diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c
> index 7936294f5f..b781e933f2 100644
> --- a/xen/arch/x86/smp.c
> +++ b/xen/arch/x86/smp.c
> @@ -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;
> }
> diff --git a/xen/arch/x86/sysctl.c b/xen/arch/x86/sysctl.c
> index 1b04947516..35239b73c1 100644
> --- a/xen/arch/x86/sysctl.c
> +++ b/xen/arch/x86/sysctl.c
> @@ -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:
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index d7e79e752a..bb73990355 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -637,6 +637,12 @@ config SYSTEM_SUSPEND
> 
>  If unsure, say N.
> 
> +config CPU_HOTPLUG
> + bool "Enable CPU hotplug"
> + depends on (X86 || ARM_64) && !FFA && !TEE && !HAS_ITS
> + default y

I do not see this as something that should be enabled on arm or
depend on any of this.

FFA could work, conditions depend more on the rest of the system
than on FFA code, same for tee which depends on TEE (and actually
FFA depends on TEE).

So i would rather see this as default n on ARM64 and only depend
on HAS_ITS and maybe flag this feature as EXPERT on arm64.

What are others thinking here ?

Cheers
Bertrand

> +
> +
> menu "Supported hypercall interfaces"
> visible if EXPERT
> 
> diff --git a/xen/common/smp.c b/xen/common/smp.c
> index a011f541f1..e2bf82856e 100644
> --- a/xen/common/smp.c
> +++ b/xen/common/smp.c
> @@ -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
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index 5207664252..51a3dd699a 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -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;
> diff --git a/xen/include/xen/smp.h b/xen/include/xen/smp.h
> index 2ca9ff1bfc..04530738c9 100644
> --- a/xen/include/xen/smp.h
> +++ b/xen/include/xen/smp.h
> @@ -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__ */
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index b250b27065..36d357cae8 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -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;
> -- 
> 2.51.2