Re: [PATCH v2] core-parking: fix build with gcc12 and NR_CPUS=1

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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