RE: [PATCH v5 03/18] xen/cpufreq: extract _PSD info from "struct xen_processor_performance"

["Penny, Zheng" <penny.zheng@xxxxxxx>]

[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Monday, June 16, 2025 5:51 PM
> To: Penny, Zheng <penny.zheng@xxxxxxx>
> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 03/18] xen/cpufreq: extract _PSD info from "struct
> xen_processor_performance"
>
> On 16.06.2025 11:43, Penny, Zheng wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: Wednesday, June 11, 2025 11:34 PM
> >> To: Penny, Zheng <penny.zheng@xxxxxxx>
> >> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH v5 03/18] xen/cpufreq: extract _PSD info from
> >> "struct xen_processor_performance"
> >>
> >> On 27.05.2025 10:48, Penny Zheng wrote:
> >>> @@ -545,14 +597,9 @@ int set_px_pminfo(uint32_t acpi_id, struct
> >>> xen_processor_performance *perf)
> >>>
> >>>      if ( perf->flags & XEN_PX_PSD )
> >>>      {
> >>> -        /* check domain coordination */
> >>> -        if ( perf->shared_type != CPUFREQ_SHARED_TYPE_ALL &&
> >>> -             perf->shared_type != CPUFREQ_SHARED_TYPE_ANY &&
> >>> -             perf->shared_type != CPUFREQ_SHARED_TYPE_HW )
> >>> -        {
> >>> -            ret = -EINVAL;
> >>> +        ret = check_psd_pminfo(perf->shared_type);
> >>> +        if ( ret )
> >>>              goto out;
> >>> -        }
> >>>
> >>>          pxpt->shared_type = perf->shared_type;
> >>>          memcpy(&pxpt->domain_info, &perf->domain_info,
> >>
> >> ... the need for this change. And even if there is a need, a
> >> follow-on question would be how this relates to the subject of this patch.
> >
> > I extracted this snippet out for sharing the same checking logic both
> > in Px and later CPPC. They both need _PSD info
>
> Right, and that (iirc) becomes visible later in the series. But it needs 
> saying here. As
> it stands the description talks of only get_psd_info() right now. And the 
> change
> above is also unrelated to the "extract" mentioned in the title.
>
> > I could change title to "xen/cpufreq: make _PSD info common" and also
> > add description in commit message for introducing check_psd_pminfo()
>
> The title was probably fine; it's the description which was lacking. In fact 
> I'd deem
> "make ... common" misleading when there's no 2nd user (yet).
>

How about:
"
Title: xen/cpufreq: export _PSD info and checking

_PSD info, consisted of "shared_type" and "struct xen_psd_package", will not
only be provided from px-specific "struct xen_processor_performance", but also
in CPPC data.

In cpufreq_add/del_cpu(), a new helper get_psd_info() is introduced to
export _PSD info. While in set_px_pminfo(), check_psd_pminfo() is also 
introduced to
export _PSD value checking.
in the meantime, the following style corrections get applied at the same time: 
........
```

> Jan