On 14.06.2023 20:02, Jason Andryuk wrote:
> Move some code around now that common xen_sysctl_pm_op get_para fields
> are together. In particular, the scaling governor information like
> scaling_available_governors is inside the union, so it is not always
> available.
>
> With that, gov_num may be 0, so bounce buffer handling needs
> to be modified.
>
> scaling_governor won't be filled for hwp, so this will simplify the
> change when it is introduced.
While I think this suitably describes the tool stack side changes, ...
> --- a/xen/drivers/acpi/pmstat.c
> +++ b/xen/drivers/acpi/pmstat.c
> @@ -239,11 +239,24 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> if ( ret )
> return ret;
>
> + op->u.get_para.cpuinfo_cur_freq =
> + cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
> + op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
> + op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
> + op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
> +
> + if ( cpufreq_driver.name[0] )
> + strlcpy(op->u.get_para.scaling_driver,
> + cpufreq_driver.name, CPUFREQ_NAME_LEN);
> + else
> + strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
> +
> if ( !(scaling_available_governors =
> xzalloc_array(char, gov_num * CPUFREQ_NAME_LEN)) )
> return -ENOMEM;
> - if ( (ret = read_scaling_available_governors(scaling_available_governors,
> - gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
> + if ( (ret = read_scaling_available_governors(
> + scaling_available_governors,
> + gov_num * CPUFREQ_NAME_LEN * sizeof(char))) )
> {
> xfree(scaling_available_governors);
> return ret;
> @@ -254,26 +267,16 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> if ( ret )
> return ret;
>
> - op->u.get_para.cpuinfo_cur_freq =
> - cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
> - op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
> - op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
> -
> op->u.get_para.u.s.scaling_cur_freq = policy->cur;
> op->u.get_para.u.s.scaling_max_freq = policy->max;
> op->u.get_para.u.s.scaling_min_freq = policy->min;
>
> - if ( cpufreq_driver.name[0] )
> - strlcpy(op->u.get_para.scaling_driver,
> - cpufreq_driver.name, CPUFREQ_NAME_LEN);
> - else
> - strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
> -
> if ( policy->governor->name[0] )
> strlcpy(op->u.get_para.u.s.scaling_governor,
> policy->governor->name, CPUFREQ_NAME_LEN);
> else
> - strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown",
> CPUFREQ_NAME_LEN);
> + strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown",
> + CPUFREQ_NAME_LEN);
>
> /* governor specific para */
> if ( !strncasecmp(op->u.get_para.u.s.scaling_governor,
> @@ -291,7 +294,6 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
> &op->u.get_para.u.s.u.ondemand.sampling_rate,
> &op->u.get_para.u.s.u.ondemand.up_threshold);
> }
> - op->u.get_para.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
>
> return ret;
> }
... all I see on the hypervisor side is re-ordering of steps and re-formatting
of over-long lines. It's not clear to me why what you do is necessary for your
purpose.
Jan
