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
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJZ5v0go8y7E2kCDbPYKcwppp0iGzZb3WiKAMhcMRMf_wrUVGA@mail.gmail.com>
Date: Tue, 14 Jan 2025 19:24:59 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Lifeng Zheng <zhenglifeng1@...wei.com>
Cc: rafael@...nel.org, lenb@...nel.org, robert.moore@...el.com, 
	viresh.kumar@...aro.org, mario.limonciello@....com, gautham.shenoy@....com, 
	ray.huang@....com, pierre.gondois@....com, 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, hepeng68@...wei.com, 
	fanghao11@...wei.com
Subject: Re: [PATCH v4 5/6] ACPI: CPPC: Add autonomous selection ABIs

On Mon, Jan 13, 2025 at 1:21 PM Lifeng Zheng <zhenglifeng1@...wei.com> wrote:

This should mention the specification revision and section number(s)
for the specification material the code is based on.

> cppc_set_epp - write energy performance preference register value
>
> cppc_get_auto_act_window - read autonomous activity window register value
>
> cppc_set_auto_act_window - write autonomous activity window register value
>
> cppc_get_auto_sel - read autonomous selection enable register value,
> modified from cppc_get_auto_sel_caps()

It would be better to move the modification part into a separate patch.

> Signed-off-by: Lifeng Zheng <zhenglifeng1@...wei.com>
> ---
>  drivers/acpi/cppc_acpi.c     | 82 ++++++++++++++++++++++++++++++++----
>  drivers/cpufreq/amd-pstate.c |  3 +-
>  include/acpi/cppc_acpi.h     | 30 +++++++++++--
>  3 files changed, 103 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 03134613311d..7bfe30f7b40f 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1568,23 +1568,89 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
>  EXPORT_SYMBOL_GPL(cppc_set_epp_perf);
>
>  /**
> - * cppc_get_auto_sel_caps - Read autonomous selection register.
> - * @cpunum : CPU from which to read register.
> - * @perf_caps : struct where autonomous selection register value is updated.
> + * 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_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps)
> +int cppc_set_epp(int cpu, u64 epp_val)
>  {
> -       u64 auto_sel;
> +       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.

It would be good to describe the autonomous activity window encoding.

> + */
> +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);
> +
> +/**
> + * 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.
> + */
> +int cppc_set_auto_act_window(int cpu, u64 auto_act_window)
> +{
> +       u64 max_val = CPPC_AUTO_ACT_WINDOW_MAX_SIG * int_pow(10, CPPC_AUTO_ACT_WINDOW_MAX_EXP);
> +       int digits = 0;
> +       u64 val;
> +
> +       if (auto_act_window > max_val)
> +               return -EINVAL;
> +
> +       while (auto_act_window > CPPC_AUTO_ACT_WINDOW_SIG_CARRY_THRESH) {
> +               auto_act_window /= 10;
> +               digits += 1;
> +       }
> +
> +       if (auto_act_window > CPPC_AUTO_ACT_WINDOW_MAX_SIG)
> +               auto_act_window = CPPC_AUTO_ACT_WINDOW_MAX_SIG;

It looks like this may clobber the most significant bit, or am I mistaken?

> +
> +       val = (digits << 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.
> + * @enable: Return address.
> + */
> +int cppc_get_auto_sel(int cpu, bool *enable)
> +{
> +       u64 val;
>         int ret;
>
> -       ret = cppc_get_reg_val(cpunum, AUTO_SEL_ENABLE, &auto_sel);
> +       ret = cppc_get_reg_val(cpu, AUTO_SEL_ENABLE, &val);
>         if (ret)
>                 return ret;
>
> -       perf_caps->auto_sel = (bool)auto_sel;
> +       *enable = (bool)val;
> +
>         return 0;
>  }
> -EXPORT_SYMBOL_GPL(cppc_get_auto_sel_caps);
> +EXPORT_SYMBOL_GPL(cppc_get_auto_sel);
>
>  /**
>   * cppc_set_auto_sel - Write autonomous selection register.
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 66e5dfc711c0..8bc11d0618f8 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -399,6 +399,7 @@ static int shmem_init_perf(struct amd_cpudata *cpudata)
>  {
>         struct cppc_perf_caps cppc_perf;
>         u64 numerator;
> +       bool auto_sel;
>
>         int ret = cppc_get_perf_caps(cpudata->cpu, &cppc_perf);
>         if (ret)
> @@ -420,7 +421,7 @@ static int shmem_init_perf(struct amd_cpudata *cpudata)
>         if (cppc_state == AMD_PSTATE_ACTIVE)
>                 return 0;
>
> -       ret = cppc_get_auto_sel_caps(cpudata->cpu, &cppc_perf);
> +       ret = cppc_get_auto_sel(cpudata->cpu, &auto_sel);
>         if (ret) {
>                 pr_warn("failed to get auto_sel, ret: %d\n", ret);
>                 return 0;
> diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
> index 62d368bcd9ec..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,7 +168,10 @@ 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_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps);
> +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);
>  extern int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator);
> @@ -229,11 +241,23 @@ static inline int cppc_get_epp_perf(int cpunum, u64 *epp_perf)
>  {
>         return -EOPNOTSUPP;
>  }
> -static inline int cppc_set_auto_sel(int cpu, bool enable)
> +static inline int cppc_set_epp(int cpu, u64 epp_val)
>  {
>         return -EOPNOTSUPP;
>  }
> -static inline int cppc_get_auto_sel_caps(int cpunum, struct cppc_perf_caps *perf_caps)
> +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;
> +}
> +static inline int cppc_set_auto_sel(int cpu, bool enable)
>  {
>         return -EOPNOTSUPP;
>  }
> --
> 2.33.0
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ