On 27/02/2023 7:43 pm, Andrew Cooper wrote: > On 09/09/2022 3:30 pm, Jan Beulich wrote: >> select HAS_ALTERNATIVE >> select HAS_COMPAT >> select HAS_CPUFREQ >> --- a/xen/arch/x86/platform_hypercall.c >> +++ b/xen/arch/x86/platform_hypercall.c >> @@ -727,12 +727,17 @@ ret_t do_platform_op( >> case XEN_CORE_PARKING_SET: >> idle_nums = min_t(uint32_t, >> op->u.core_parking.idle_nums, num_present_cpus() - >> 1); >> - ret = continue_hypercall_on_cpu( >> - 0, core_parking_helper, (void *)(unsigned >> long)idle_nums); >> + if ( CONFIG_NR_CPUS > 1 ) >> + ret = continue_hypercall_on_cpu( >> + 0, core_parking_helper, >> + (void *)(unsigned long)idle_nums); >> + else if ( idle_nums ) >> + ret = -EINVAL; >> break; >> >> case XEN_CORE_PARKING_GET: >> - op->u.core_parking.idle_nums = get_cur_idle_nums(); >> + op->u.core_parking.idle_nums = CONFIG_NR_CPUS > 1 >> + ? get_cur_idle_nums() : 0; > These don't look right. > > If the core parking feature isn't available, it should uniformly fail, > not report success on the get side and fail on the set side.
Huh, and in extra fun. It turns out we've had core parking unconditionally disabled in XenServer for ~9 years now. It looks at the idleness of dom0 and starts taking CPUs offline, even when the VMs are busy. I don't see how this functionality can ever have worked suitably... I think there's a good argument to be made for having it user selectable, and frankly, off-by-default. ~Andrew
