On 14.04.2025 09:40, Penny Zheng wrote:
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -515,7 +515,7 @@ If set, force use of the performance counters for
> oprofile, rather than detectin
> available support.
>
> ### cpufreq
> -> `= none | {{ <boolean> | xen } {
> [:[powersave|performance|ondemand|userspace][,[<maxfreq>]][,[<minfreq>]]] }
> [,verbose]} | dom0-kernel | hwp[:[<hdc>][,verbose]] | amd-cppc[:[verbose]]`
> +> `= none | {{ <boolean> | xen } {
> [:[powersave|performance|ondemand|userspace][,[<maxfreq>]][,[<minfreq>]]] }
> [,verbose]} | dom0-kernel | hwp[:[<hdc>][,verbose]] |
> amd-cppc[:[active][,verbose]]`
>
> > Default: `xen`
>
> @@ -537,6 +537,12 @@ choice of `dom0-kernel` is deprecated and not supported
> by all Dom0 kernels.
> * `amd-cppc` selects ACPI Collaborative Performance and Power Control (CPPC)
> on supported AMD hardware to provide finer grained frequency control
> mechanism. The default is disabled.
> +* `active` is to enable amd-cppc driver in active(autonomous) mode. In this
> + mode, users could write to energy performance preference register(epp) to
> + tell hardware if they want to bias toward performance or energy efficiency.
> + Then built-in CPPC power algorithm will calculate the runtime workload and
> + adjust cores frequency automatically according to the power supply,
> thermal,
> + core voltage and some other hardware conditions.
This section of the cmdline doc may not be of the best quality, but may I
nevertheless ask that for this new sub-option you make explicit what the
default is (and that it's of boolean nature)?
> @@ -434,12 +464,82 @@ static int cf_check amd_cppc_cpufreq_cpu_init(struct
> cpufreq_policy *policy)
>
> amd_cppc_boost_init(policy, data);
>
> + return 0;
> +}
> +
> +static int cf_check amd_cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
> +{
> + int ret;
> +
> + ret = amd_cppc_cpufreq_init_perf(policy);
> + if ( ret )
> + return ret;
> +
> amd_cppc_verbose(policy->cpu,
> "CPU initialized with amd-cppc passive mode\n");
>
> return 0;
> }
>
> +static int cf_check amd_cppc_epp_cpu_init(struct cpufreq_policy *policy)
> +{
> + int ret;
> +
> + ret = amd_cppc_cpufreq_init_perf(policy);
> + if ( ret )
> + return ret;
> +
> + policy->policy = cpufreq_policy_from_governor(policy->governor);
This is the init part, which is fine. What if the governor is being changed,
though? The struct field is new, and there's no other place I can spot where
it would be adjusted.
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -242,6 +242,7 @@
> #define MSR_AMD_CPPC_ENABLE 0xc00102b1U
> #define AMD_CPPC_ENABLE (_AC(1, ULL) << 0)
> #define MSR_AMD_CPPC_REQ 0xc00102b3U
> +#define AMD_CPPC_EPP_MASK (_AC(0xff, ULL) << 24)
Nit: Excess blank after << (there's no such issue with AMD_CPPC_ENABLE,
ftaod).
Jan
