RE: [PATCH v4 02/15] xen/cpufreq: extract _PSD info from "struct xen_processor_performance"

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

[Public]

Hi

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: Monday, May 12, 2025 11:43 PM
> To: Penny, Zheng <penny.zheng@xxxxxxx>
> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; Anthony PERARD <anthony.perard@xxxxxxxxxx>;
> Orzel, Michal <Michal.Orzel@xxxxxxx>; Julien Grall <julien@xxxxxxx>; Roger Pau
> Monné <roger.pau@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; 
> xen-
> devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH v4 02/15] xen/cpufreq: extract _PSD info from "struct
> xen_processor_performance"
>
> On 06.05.2025 07:56, Penny, Zheng wrote:
> > [Public]
> >
> > Hi,
> >
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@xxxxxxxx>
> >> Sent: Monday, April 28, 2025 11:32 PM
> >> To: Penny, Zheng <penny.zheng@xxxxxxx>
> >> Cc: Huang, Ray <Ray.Huang@xxxxxxx>; Andrew Cooper
> >> <andrew.cooper3@xxxxxxxxxx>; Anthony PERARD
> >> <anthony.perard@xxxxxxxxxx>; Orzel, Michal <Michal.Orzel@xxxxxxx>;
> >> Julien Grall <julien@xxxxxxx>; Roger Pau Monné
> >> <roger.pau@xxxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>;
> >> xen-devel@xxxxxxxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH v4 02/15] xen/cpufreq: extract _PSD info from
> >> "struct xen_processor_performance"
> >>
> >> On 14.04.2025 09:40, Penny Zheng wrote:
> >>> --- a/xen/include/public/platform.h
> >>> +++ b/xen/include/public/platform.h
> >>> @@ -440,6 +440,11 @@ struct xen_psd_package {
> >>>      uint64_t num_processors;
> >>>  };
> >>>
> >>> +/* Coordination type value */
> >>> +#define XEN_CPUPERF_SHARED_TYPE_HW   1 /* HW does needed
> >> coordination */
> >>> +#define XEN_CPUPERF_SHARED_TYPE_ALL  2 /* All dependent CPUs
> >> should
> >>> +set freq */ #define XEN_CPUPERF_SHARED_TYPE_ANY  3 /* Freq can be
> >> set
> >>> +from any dependent CPU */
> >>> +
> >>>  struct xen_processor_performance {
> >>>      uint32_t flags;     /* flag for Px sub info type */
> >>>      uint32_t platform_limit;  /* Platform limitation on freq usage
> >>> */ @@ -449,10 +454,7 @@ struct xen_processor_performance {
> >>>      XEN_GUEST_HANDLE(xen_processor_px_t) states;
> >>>      struct xen_psd_package domain_info;
> >>>      /* Coordination type of this processor */
> >>> -#define XEN_CPUPERF_SHARED_TYPE_HW   1 /* HW does needed
> >> coordination */
> >>> -#define XEN_CPUPERF_SHARED_TYPE_ALL  2 /* All dependent CPUs
> >> should
> >>> set freq */ -#define XEN_CPUPERF_SHARED_TYPE_ANY  3 /* Freq can be
> >>> set
> >> from any dependent CPU */
> >>> -    uint32_t shared_type;
> >>> +    uint32_t shared_type; /* XEN_CPUPERF_SHARED_TYPE_xxx */
> >>>  };
> >>>  typedef struct xen_processor_performance
> >>> xen_processor_performance_t;
> >>> DEFINE_XEN_GUEST_HANDLE(xen_processor_performance_t);
> >>
> >> What's this movement about? In the public interface nothing changes?
> >
> > As we will have shared type in "struct xen_processor_cppc" too, maybe
> > the definition would like to live at more common place, then it could be 
> > shared?
> > Living inside "struct xen_processor_performance" looks like internal values 
> > for
> internal field.
> > If it breaks the public interface some way, I'll change it back and
> > duplicate the definition in "struct xen_processor_cppc" too
>
> I don't think it would break anything, but I also don't see any need for the
> movement. And generally we prefer to avoid unnecessary code churn.

Understood, then I'll delete this change.

>
> Jan