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.
Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com>
---
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/Kconfig | 1 +
xen/arch/arm/smp.c | 9 +++++++
xen/arch/ppc/stubs.c | 4 +++
xen/arch/riscv/stubs.c | 5 ++++
xen/arch/x86/Kconfig | 1 +
xen/arch/x86/include/asm/smp.h | 3 ---
xen/arch/x86/smp.c | 33 +++----------------------
xen/arch/x86/sysctl.c | 12 +++------
xen/common/Kconfig | 3 +++
xen/common/smp.c | 34 +++++++++++++++++++++++++
xen/common/sysctl.c | 45 ++++++++++++++++++++++++++++++++++
xen/include/xen/smp.h | 4 +++
12 files changed, 112 insertions(+), 42 deletions(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index cf6af68299..5144e9c8d5 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -7,6 +7,7 @@ config ARM_64
def_bool y
depends on !ARM_32
select 64BIT
+ select CPU_HOTPLUG if !TEE && !FFA && !HAS_ITS
select HAS_FAST_MULTIPLY
select HAS_VPCI_GUEST_SUPPORT if PCI_PASSTHROUGH
diff --git a/xen/arch/arm/smp.c b/xen/arch/arm/smp.c
index b372472188..075da9aeb3 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_smt_cpu_disable(unsigned int cpu)
+{
+ return false;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/ppc/stubs.c b/xen/arch/ppc/stubs.c
index f7f6e7ed97..ed75d06dd9 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_smt_cpu_disable(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 29bdb65afb..8a9503ec94 100644
--- a/xen/arch/riscv/stubs.c
+++ b/xen/arch/riscv/stubs.c
@@ -75,6 +75,11 @@ void smp_send_call_function_mask(const cpumask_t *mask)
BUG_ON("unimplemented");
}
+bool arch_smt_cpu_disable(unsigned int cpu)
+{
+ BUG_ON("unimplemented");
+}
+
/* irq.c */
void irq_ack_none(struct irq_desc *desc)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index c808c989fc..826aa2512d 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -12,6 +12,7 @@ config X86
select ARCH_PAGING_MEMPOOL
select ARCH_SUPPORTS_INT128
imply CORE_PARKING
+ select CPU_HOTPLUG
select FUNCTION_ALIGNMENT_16B
select GENERIC_BUG_FRAME
select HAS_ALTERNATIVE
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/smp.c b/xen/arch/x86/smp.c
index 7936294f5f..d64b533cc0 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_smt_cpu_disable(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..87a4b7ac63 100644
--- a/xen/arch/x86/sysctl.c
+++ b/xen/arch/x86/sysctl.c
@@ -115,7 +115,6 @@ 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);
@@ -124,15 +123,10 @@ long arch_do_sysctl(
switch ( 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);
+ /* 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 38320b248a..1a28c2dafe 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -176,6 +176,9 @@ config LIBFDT
config MEM_ACCESS_ALWAYS_ON
bool
+config CPU_HOTPLUG
+ bool
+
config VM_EVENT
def_bool MEM_ACCESS_ALWAYS_ON
prompt "Memory Access and VM events" if !MEM_ACCESS_ALWAYS_ON
diff --git a/xen/common/smp.c b/xen/common/smp.c
index a011f541f1..8ff81197cb 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,39 @@ 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_smt_cpu_disable(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)
+{
+ 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..2acf47723d 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -483,6 +483,51 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
copyback = 1;
break;
+#ifdef CONFIG_CPU_HOTPLUG
+ 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;
+
+ 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;
+
+ case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE:
+ case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE:
+ /* Use arch specific handlers as SMT is very arch-dependent */
+ ret = arch_do_sysctl(op, u_sysctl);
+ copyback = 0;
+ goto out;
+
+ default:
+ ret = -EOPNOTSUPP;
+ break;
+ }
+
+ if ( !ret )
+ 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;
+ }
+#endif
+
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..c734033bfb 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_smt_cpu_disable(unsigned int cpu);
+long cf_check cpu_up_helper(void *data);
+long cf_check cpu_down_helper(void *data);
+
#endif /* __XEN_SMP_H__ */
--
2.51.2On 13.01.2026 09:45, Mykyta Poturai 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.
For all of these "not supported" cases, what if a user nevertheless tries?
Wouldn't the request better be outright denied, rather leaving the system in
a questionable state? Hmm, I see you ...
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -7,6 +7,7 @@ config ARM_64
> def_bool y
> depends on !ARM_32
> select 64BIT
> + select CPU_HOTPLUG if !TEE && !FFA && !HAS_ITS
... make the select conditional. But do TEE, FFA, and HAS_ITS each mean the
feature is actually in use when the hypervisor runs?
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -176,6 +176,9 @@ config LIBFDT
> config MEM_ACCESS_ALWAYS_ON
> bool
>
> +config CPU_HOTPLUG
> + bool
Nit: Indentation by a single tab please. See adjacent entries.
> @@ -104,6 +105,39 @@ 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;
Note this for the first comment below on cpu_down_helper().
> + int ret = cpu_up(cpu);
> +
> + /* Have one more go on EBUSY. */
> + if ( ret == -EBUSY )
> + ret = cpu_up(cpu);
> +
> + if ( !ret && arch_smt_cpu_disable(cpu) )
As you validly note in a comment in do_sysctl(), SMT is an arch-specific concept
and perhaps even an arch-specific term. Hence using it in the name of an arch
hook feels inappropriate. Plus - the hook could be used for other purposes. What
the arch needs to indicate is whether the CPU that was brought up may actually
stay online. That more generic purpose is what imo the name wants to cover.
> + {
> + 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;
Why is this left as plain int? Yes, it was like this in the original code,
but wrongly so.
> + int ret = cpu_down(cpu);
> + /* Have one more go on EBUSY. */
Also please add the missing blank line after the declarations.
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -483,6 +483,51 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
> copyback = 1;
> break;
>
> +#ifdef CONFIG_CPU_HOTPLUG
> + case XEN_SYSCTL_cpu_hotplug:
Please see the pretty recent
https://lists.xen.org/archives/html/xen-devel/2026-01/msg00329.html
(scroll down to the xen/arch/x86/platform_hypercall.c change).
> + {
> + 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;
> +
> + 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;
> +
> + case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE:
> + case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE:
> + /* Use arch specific handlers as SMT is very arch-dependent */
> + ret = arch_do_sysctl(op, u_sysctl);
> + copyback = 0;
> + goto out;
I wonder if it wouldn't be neater for this and actually also ...
> + default:
> + ret = -EOPNOTSUPP;
> + break;
... this to fall through to ...
> + }
> +
> + if ( !ret )
> + 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;
> + }
> +#endif
> +
> default:
> ret = arch_do_sysctl(op, u_sysctl);
... here. (Minimally the earlier default case wants uniformly forwarding to
the arch handler, or else arch-specific additions would always require
adjustment here.)
Jan
On 14.01.26 11:49, Jan Beulich wrote:
> On 13.01.2026 09:45, Mykyta Poturai 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.
>
> For all of these "not supported" cases, what if a user nevertheless tries?
> Wouldn't the request better be outright denied, rather leaving the system in
> a questionable state? Hmm, I see you ...
>
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -7,6 +7,7 @@ config ARM_64
>> def_bool y
>> depends on !ARM_32
>> select 64BIT
>> + select CPU_HOTPLUG if !TEE && !FFA && !HAS_ITS
>
> ... make the select conditional. But do TEE, FFA, and HAS_ITS each mean the
> feature is actually in use when the hypervisor runs?
>
The way interrupts are requested in these modules causes Xen to crash
when trying to offline a cpu. It’s a fairly simple fix and I plan to
send them eventually. I’ve decided to omit them now and do these fixes
only for supported code to keep the series from ballooning with too many
patches.
>> + int ret = cpu_up(cpu);
>> +
>> + /* Have one more go on EBUSY. */
>> + if ( ret == -EBUSY )
>> + ret = cpu_up(cpu);
>> +
>> + if ( !ret && arch_smt_cpu_disable(cpu) )
>
> As you validly note in a comment in do_sysctl(), SMT is an arch-specific concept
> and perhaps even an arch-specific term. Hence using it in the name of an arch
> hook feels inappropriate. Plus - the hook could be used for other purposes. What
> the arch needs to indicate is whether the CPU that was brought up may actually
> stay online. That more generic purpose is what imo the name wants to cover.
>
Would arch_cpu_online_allowed() be okay, or maybe you have something
more specific in mind?
>> + case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE:
>> + case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE:
>> + /* Use arch specific handlers as SMT is very arch-dependent */
>> + ret = arch_do_sysctl(op, u_sysctl);
>> + copyback = 0;
>> + goto out;
>
> I wonder if it wouldn't be neater for this and actually also ...
>
>> + default:
>> + ret = -EOPNOTSUPP;
>> + break;
>
> ... this to fall through to ...
>
>> + }
>> +
>> + if ( !ret )
>> + 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;
>> + }
>> +#endif
>> +
>> default:
>> ret = arch_do_sysctl(op, u_sysctl);
>
> ... here. (Minimally the earlier default case wants uniformly forwarding to
> the arch handler, or else arch-specific additions would always require
> adjustment here.)
>
> Jan
Would it be okay to mix goto and switch like this, or is it too convoluted?
case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
plug = false;
fn = cpu_down_helper;
hcpu = _p(cpu);
break;
default:
goto outer_default;
}
if ( !ret )
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;
}
#endif
default:
outer_default:
ret = arch_do_sysctl(op, u_sysctl);
copyback = 0;
break;
}
--
Mykyta
On 03.02.2026 11:30, Mykyta Poturai wrote:
> On 14.01.26 11:49, Jan Beulich wrote:
>> On 13.01.2026 09:45, Mykyta Poturai 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.
>>
>> For all of these "not supported" cases, what if a user nevertheless tries?
>> Wouldn't the request better be outright denied, rather leaving the system in
>> a questionable state? Hmm, I see you ...
>>
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -7,6 +7,7 @@ config ARM_64
>>> def_bool y
>>> depends on !ARM_32
>>> select 64BIT
>>> + select CPU_HOTPLUG if !TEE && !FFA && !HAS_ITS
>>
>> ... make the select conditional. But do TEE, FFA, and HAS_ITS each mean the
>> feature is actually in use when the hypervisor runs?
>>
> The way interrupts are requested in these modules causes Xen to crash
> when trying to offline a cpu. It’s a fairly simple fix and I plan to
> send them eventually. I’ve decided to omit them now and do these fixes
> only for supported code to keep the series from ballooning with too many
> patches.
I disagree with such an approach, but it'll be the Arm maintainers to judge here.
>>> + int ret = cpu_up(cpu);
>>> +
>>> + /* Have one more go on EBUSY. */
>>> + if ( ret == -EBUSY )
>>> + ret = cpu_up(cpu);
>>> +
>>> + if ( !ret && arch_smt_cpu_disable(cpu) )
>>
>> As you validly note in a comment in do_sysctl(), SMT is an arch-specific concept
>> and perhaps even an arch-specific term. Hence using it in the name of an arch
>> hook feels inappropriate. Plus - the hook could be used for other purposes. What
>> the arch needs to indicate is whether the CPU that was brought up may actually
>> stay online. That more generic purpose is what imo the name wants to cover.
>
> Would arch_cpu_online_allowed() be okay, or maybe you have something
> more specific in mind?
The name is already much better, just that it gives the impression that it perhaps
rather would want using ahead of calling cpu_up().
>>> + case XEN_SYSCTL_CPU_HOTPLUG_SMT_ENABLE:
>>> + case XEN_SYSCTL_CPU_HOTPLUG_SMT_DISABLE:
>>> + /* Use arch specific handlers as SMT is very arch-dependent */
>>> + ret = arch_do_sysctl(op, u_sysctl);
>>> + copyback = 0;
>>> + goto out;
>>
>> I wonder if it wouldn't be neater for this and actually also ...
>>
>>> + default:
>>> + ret = -EOPNOTSUPP;
>>> + break;
>>
>> ... this to fall through to ...
>>
>>> + }
>>> +
>>> + if ( !ret )
>>> + 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;
>>> + }
>>> +#endif
>>> +
>>> default:
>>> ret = arch_do_sysctl(op, u_sysctl);
>>
>> ... here. (Minimally the earlier default case wants uniformly forwarding to
>> the arch handler, or else arch-specific additions would always require
>> adjustment here.)
>
> Would it be okay to mix goto and switch like this, or is it too convoluted?
I'm not a fan of using goto, but maybe it would be acceptable here. By ...
> case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
> plug = false;
> fn = cpu_down_helper;
> hcpu = _p(cpu);
> break;
>
> default:
> goto outer_default;
> }
>
> if ( !ret )
> 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;
... wrapping everything past the switch() block in "if ( fn )" you'd already
get what is wanted.
> }
> #endif
>
> default:
> outer_default:
Nit: See ./CODING_STYLE.
Jan
> ret = arch_do_sysctl(op, u_sysctl);
> copyback = 0;
> break;
> }
>
>
>
© 2016 - 2026 Red Hat, Inc.