| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e5dc4906-2b28-4adc-8e8e-ddd5d7cc985c@amd.com>
Date: Wed, 9 Apr 2025 13:59:23 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Lifeng Zheng <zhenglifeng1@...wei.com>, rafael@...nel.org,
lenb@...nel.org, robert.moore@...el.com, viresh.kumar@...aro.org,
gautham.shenoy@....com, ray.huang@....com, perry.yuan@....com,
pierre.gondois@....com
Cc: acpica-devel@...ts.linux.dev, linux-acpi@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, linuxarm@...wei.com,
jonathan.cameron@...wei.com, zhanjie9@...ilicon.com, lihuisong@...wei.com,
cenxinghai@...artners.com, hepeng68@...wei.com
Subject: Re: [PATCH v6 8/8] ACPI: CPPC: Add three functions related to
autonomous selection
On 4/9/2025 1:57 AM, Lifeng Zheng wrote:
> cppc_set_epp - write energy performance preference register value, based on
> ACPI 6.5, s8.4.6.1.7
>
> cppc_get_auto_act_window - read autonomous activity window register value,
> based on ACPI 6.5, s8.4.6.1.6
>
> cppc_set_auto_act_window - write autonomous activity window register value,
> based on ACPI 6.5, s8.4.6.1.6
>
> Reviewed-by: Pierre Gondois <pierre.gondois@....com>
> Signed-off-by: Lifeng Zheng <zhenglifeng1@...wei.com>
> ---
> drivers/acpi/cppc_acpi.c | 80 ++++++++++++++++++++++++++++++++++++++++
> include/acpi/cppc_acpi.h | 24 ++++++++++++
> 2 files changed, 104 insertions(+)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index ef2394c074e3..3d5eace44af5 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1608,6 +1608,86 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
> }
> EXPORT_SYMBOL_GPL(cppc_set_epp_perf);
>
> +/**
> + * cppc_set_epp() - Write the EPP register.
> + * @cpu: CPU on which to write register.
> + * @epp_val: Value to write to the EPP register.
> + */
> +int cppc_set_epp(int cpu, u64 epp_val)
Any reason this is a u64 argument when the biggest value you can support
is 0xFF? Presumably you could drop the the bounds check below if you
limited the variable size.
> +{
> + if (epp_val > CPPC_ENERGY_PERF_MAX)
> + return -EINVAL;
> +
> + return cppc_set_reg_val(cpu, ENERGY_PERF, epp_val);
> +}
> +EXPORT_SYMBOL_GPL(cppc_set_epp);
> +
> +/**
> + * cppc_get_auto_act_window() - Read autonomous activity window register.
> + * @cpu: CPU from which to read register.
> + * @auto_act_window: Return address.
> + *
> + * According to ACPI 6.5, s8.4.6.1.6, the value read from the autonomous
> + * activity window register consists of two parts: a 7 bits value indicate
> + * significand and a 3 bits value indicate exponent.
> + */
> +int cppc_get_auto_act_window(int cpu, u64 *auto_act_window)
> +{
> + unsigned int exp;
> + u64 val, sig;
> + int ret;
> +
> + ret = cppc_get_reg_val(cpu, AUTO_ACT_WINDOW, &val);
> + if (ret)
> + return ret;
> +
> + sig = val & CPPC_AUTO_ACT_WINDOW_MAX_SIG;
> + exp = (val >> CPPC_AUTO_ACT_WINDOW_SIG_BIT_SIZE) & CPPC_AUTO_ACT_WINDOW_MAX_EXP;
> + *auto_act_window = sig * int_pow(10, exp);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(cppc_get_auto_act_window);
Since this is exported code, do you perhaps want a check that
auto_act_window is not NULL to avoid a possible accidental NULL pointer
dereference?
> +
> +/**
> + * cppc_set_auto_act_window() - Write autonomous activity window register.
> + * @cpu: CPU on which to write register.
> + * @auto_act_window: usec value to write to the autonomous activity window register.
> + *
> + * According to ACPI 6.5, s8.4.6.1.6, the value to write to the autonomous
> + * activity window register consists of two parts: a 7 bits value indicate
> + * significand and a 3 bits value indicate exponent.
> + */
> +int cppc_set_auto_act_window(int cpu, u64 auto_act_window)
> +{
> + /* The max value to stroe is 1270000000 */
store
> + u64 max_val = CPPC_AUTO_ACT_WINDOW_MAX_SIG * int_pow(10, CPPC_AUTO_ACT_WINDOW_MAX_EXP);
> + int exp = 0;
> + u64 val;
> +
> + if (auto_act_window > max_val)
> + return -EINVAL;
> +
> + /*
> + * The max significand is 127, when auto_act_window is larger than
> + * 129, discard the precision of the last digit and increase the
> + * exponent by 1.
> + */
> + while (auto_act_window > CPPC_AUTO_ACT_WINDOW_SIG_CARRY_THRESH) {
> + auto_act_window /= 10;
> + exp += 1;
> + }
> +
> + /* For 128 and 129, cut it to 127. */
> + if (auto_act_window > CPPC_AUTO_ACT_WINDOW_MAX_SIG)
> + auto_act_window = CPPC_AUTO_ACT_WINDOW_MAX_SIG;
> +
> + val = (exp << CPPC_AUTO_ACT_WINDOW_SIG_BIT_SIZE) + auto_act_window;
> +
> + return cppc_set_reg_val(cpu, AUTO_ACT_WINDOW, val);
> +}
> +EXPORT_SYMBOL_GPL(cppc_set_auto_act_window);
> +
> /**
> * cppc_get_auto_sel() - Read autonomous selection register.
> * @cpu: CPU from which to read register.
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 31767c65be20..325e9543e08f 100644
> --- a/include/acpi/cppc_acpi.h
> +++ b/include/acpi/cppc_acpi.h
> @@ -32,6 +32,15 @@
> #define CMD_READ 0
> #define CMD_WRITE 1
>
> +#define CPPC_AUTO_ACT_WINDOW_SIG_BIT_SIZE (7)
> +#define CPPC_AUTO_ACT_WINDOW_EXP_BIT_SIZE (3)
> +#define CPPC_AUTO_ACT_WINDOW_MAX_SIG ((1 << CPPC_AUTO_ACT_WINDOW_SIG_BIT_SIZE) - 1)
> +#define CPPC_AUTO_ACT_WINDOW_MAX_EXP ((1 << CPPC_AUTO_ACT_WINDOW_EXP_BIT_SIZE) - 1)
> +/* CPPC_AUTO_ACT_WINDOW_MAX_SIG is 127, so 128 and 129 will decay to 127 when writing */
> +#define CPPC_AUTO_ACT_WINDOW_SIG_CARRY_THRESH 129
> +
> +#define CPPC_ENERGY_PERF_MAX (0xFF)
> +
> /* Each register has the folowing format. */
> struct cpc_reg {
> u8 descriptor;
> @@ -159,6 +168,9 @@ extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
> extern int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val);
> extern int cppc_get_epp_perf(int cpunum, u64 *epp_perf);
> extern int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable);
> +extern int cppc_set_epp(int cpu, u64 epp_val);
> +extern int cppc_get_auto_act_window(int cpu, u64 *auto_act_window);
> +extern int cppc_set_auto_act_window(int cpu, u64 auto_act_window);
> extern int cppc_get_auto_sel(int cpu, bool *enable);
> extern int cppc_set_auto_sel(int cpu, bool enable);
> extern int amd_get_highest_perf(unsigned int cpu, u32 *highest_perf);
> @@ -229,6 +241,18 @@ static inline int cppc_get_epp_perf(int cpunum, u64 *epp_perf)
> {
> return -EOPNOTSUPP;
> }
> +static inline int cppc_set_epp(int cpu, u64 epp_val)
> +{
> + return -EOPNOTSUPP;
> +}
> +static inline int cppc_get_auto_act_window(int cpu, u64 *auto_act_window)
> +{
> + return -EOPNOTSUPP;
> +}
> +static inline int cppc_set_auto_act_window(int cpu, u64 auto_act_window)
> +{
> + return -EOPNOTSUPP;
> +}
> static inline int cppc_get_auto_sel(int cpu, bool *enable)
> {
> return -EOPNOTSUPP;
Powered by blists - more mailing lists