Re: [PATCH] platform/cpufreq: add public defines for CPUFREQ_SHARED_TYPE_

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.04.2022 17:16, Roger Pau Monne wrote:
> The values set in the shared_type field of xen_processor_performance
> have so far relied on Xen and Linux having the same
> CPUFREQ_SHARED_TYPE_ defines, as those have never been part of the
> public interface.
> 
> Formalize by adding the defines for the allowed values in the public
> header, while renaming them to use the XEN_PERF_SHARED_TYPE_ prefix
> for clarity.
> 
> Fixes: 2fa7bee0a0 ('Get ACPI Px from dom0 and choose Px controller')
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
> I wonder if we want to keep the CPUFREQ_SHARED_TYPE_ defines for
> internal usage (and define them based on XEN_PERF_SHARED_TYPE_) in
> case we need to pick up changes from Linux.

I think that would be desirable, also to limit code churn by this change.

> --- a/xen/include/public/platform.h
> +++ b/xen/include/public/platform.h
> @@ -465,7 +465,11 @@ struct xen_processor_performance {
>      uint32_t state_count;     /* total available performance states */
>      XEN_GUEST_HANDLE(xen_processor_px_t) states;
>      struct xen_psd_package domain_info;
> -    uint32_t shared_type;     /* coordination type of this processor */
> +    /* Coordination type of this processor */
> +#define XEN_PERF_SHARED_TYPE_HW   1 /* HW does needed coordination */
> +#define XEN_PERF_SHARED_TYPE_ALL  2 /* All dependent CPUs should set freq */
> +#define XEN_PERF_SHARED_TYPE_ANY  3 /* Freq can be set from any dependent 
> CPU */

While the names may then become a little long, I think it would be
relevant to have "processor" (or maybe "CPU") in the names, to have
a better match with struct xen_processor_performance. Also I'm not
sure you want to define these inside the struct - they're
supposedly suitable for Px, Cx, and Tx aiui (just like the underlying
ACPI constants are).

Jan