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, 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

> Jan