[Public]
Hi
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: Monday, May 12, 2025 11:43 PM
> To: Penny, Zheng <penny.zh...@amd.com>
> Cc: Huang, Ray <ray.hu...@amd.com>; Andrew Cooper
> <andrew.coop...@citrix.com>; Anthony PERARD <anthony.per...@vates.tech>;
> Orzel, Michal <michal.or...@amd.com>; Julien Grall <jul...@xen.org>; Roger Pau
> Monné <roger....@citrix.com>; Stefano Stabellini <sstabell...@kernel.org>;
> xen-
> de...@lists.xenproject.org
> 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 <jbeul...@suse.com>
> >> Sent: Monday, April 28, 2025 11:32 PM
> >> To: Penny, Zheng <penny.zh...@amd.com>
> >> Cc: Huang, Ray <ray.hu...@amd.com>; Andrew Cooper
> >> <andrew.coop...@citrix.com>; Anthony PERARD
> >> <anthony.per...@vates.tech>; Orzel, Michal <michal.or...@amd.com>;
> >> Julien Grall <jul...@xen.org>; Roger Pau Monné
> >> <roger....@citrix.com>; Stefano Stabellini <sstabell...@kernel.org>;
> >> xen-devel@lists.xenproject.org
> >> 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
