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é <[email protected]>
> ---
> 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
