Re: [PATCH v9 4/6] arm/sysctl: Implement cpu hotplug ops
Jan Beulich <[email protected]>
| Newsgroups | org.xenproject.lists.xen-devel |
|---|---|
| Message-ID | <[email protected]> |
On 18.08.2026 10:48, Mykyta Poturai wrote: > 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 enabled, > and partially supported FFA, or TEE is enabled, as they use non-static > IRQ actions. > > Remove ifdef guards for x86 in flask, as cpu hotplug is now > supported on more architectures. > > Signed-off-by: Mykyta Poturai <[email protected]> > --- > v8->v9: > * simplify dependencies of config CPU_ONLINE_OFFLINE again You must also have re-based over XSA-499. As non-trivial re-basing can go wrong, I think at least in those cases it wants noting in the revlog. (Personally I try to remember to always add the remark, even if the re-base was the trivial resolution of e.g. fuzz.) > @@ -104,6 +105,40 @@ void smp_call_function_interrupt(void) > irq_exit(); > } > > +#ifdef CONFIG_CPU_ONLINE_OFFLINE > +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) ) I'm sorry, I should have noticed this already before: This isn't "can", at least on x86. It is a requirement in certain situations that CPUs be kept kind-of- online. Since arch_cpu_must_stay_online() feels clumsy as a name, and since that might also conflict with another arch perhaps really meaning "can", not "must", maybe arch_cpu_keep_online()? > --- a/xen/common/sysctl.c > +++ b/xen/common/sysctl.c > @@ -475,6 +475,40 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl) > copyback = 1; > break; > > + case XEN_SYSCTL_cpu_hotplug: > + { > + unsigned int hp_op = op->u.cpu_hotplug.op; This variable is used ... > + long (*fn)(void *data); > + void *hcpu = _p(op->u.cpu_hotplug.cpu); > + > + ret = -EOPNOTSUPP; > + if ( !IS_ENABLED(CONFIG_CPU_ONLINE_OFFLINE) ) > + break; > + > + switch ( hp_op ) ... solely here. Is such a variable really warranted? > + { > + case XEN_SYSCTL_CPU_HOTPLUG_ONLINE: > + fn = cpu_up_helper; > + break; > + > + case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE: > + fn = cpu_down_helper; > + break; > + > + default: > + fn = NULL; > + break; > + } > + > + if ( fn ) > + { > + ret = continue_hypercall_on_cpu(0, fn, hcpu); Same (maybe to slightly lesser degree) for "hcpu", used solely here. For both variables, the situation is/was different in the original code. Jan