Re: [PATCH v4 11/20] xen/pmstat: introduce CONFIG_PM_OP

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.06.2025 09:00, Penny, Zheng wrote:
> [Public]
> 
> Hi,
> 
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@xxxxxxxx>
>> Sent: Tuesday, June 10, 2025 9:41 PM
>> To: Penny, Zheng <penny.zheng@xxxxxxx>
>> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Andrew Cooper
>> <andrew.cooper3@xxxxxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>;
>> Anthony PERARD <anthony.perard@xxxxxxxxxx>; Orzel, Michal
>> <Michal.Orzel@xxxxxxx>; Julien Grall <julien@xxxxxxx>; Stefano Stabellini
>> <sstabellini@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
>> Subject: Re: [PATCH v4 11/20] xen/pmstat: introduce CONFIG_PM_OP
>>
>> On 28.05.2025 11:16, Penny Zheng wrote:
>>> --- a/xen/common/Kconfig
>>> +++ b/xen/common/Kconfig
>>> @@ -591,4 +591,11 @@ config SYSCTL
>>>       to reduce Xen footprint.
>>>  endmenu
>>>
>>> +config PM_OP
>>> +   bool "Enable Performance Management Operation"
>>> +   depends on ACPI && HAS_CPUFREQ && SYSCTL
>>> +   default y
>>> +   help
>>> +     This option shall enable userspace performance management control
>>> +     to do power/performance analyzing and tuning.
>>>  endmenu
>>
>> Why is this being added outside of the "Supported hypercall interfaces"
>> menu? (No matter where it's added, please make sure there's a separating 
>> blank
>> line ahdead of the subsequent "endmenu".)
> 
> Because, I intended to have the only five catogory under "Supported hypercall 
> interfaces":
>         - sysctl
>         - domctl
>         - hvm
>         - physdev
>         - platform
> Which will be introduced in a patch serie and may contain multiple different 
> features.
> PM_OP is more or less a feature to me. And it is only dependent on SYSCTL, 
> like LIVEPATCH, etc

Hmm, well, okay. Especially your reference to LIVEPATCH here suggests to me that
some re-ordering would then be quite desirable in the file. It is generally
pretty desirable for "depends on <feat>" to appear later than "<feat>". While
for the graphical UI interfaces of kconfig the difference may be marginal, for
the command line one you otherwise get to select the setting for the dependent
feature first, just for that setting potentially being undone by the later
selection for <feat>.

Then again the entire file could do with some rearrangements. Yet I guess to
avoid interfering with your work, I'd better postpone any tidying work there.

Jan