ACPI: CPPC: Add CPPC v4 support (ACPI 6.6)

CPPC v4 (ACPI 6.6, Section 8.4.6) adds two optional entries to the _CPC package:

1. OSPM Nominal Performance (8.4.6.1.2.6): A write-only register that lets OSPM inform the platform what it considers nominal performance. The platform classifies performance above this level as boost and below as throttle for its power/thermal decisions.

2. Resource Priority (8.4.6.1.2.7): A Package of Resource Priority Register Descriptor sub-packages that allow OSPM to set relative priority among processors for shared resources (boost, throttle, L2/L3 cache, memory bandwidth). Parsing the full structure is not yet supported; such entries are marked as unsupported.

Add v4 _CPC table parsing (25 entries) and update REG_OPTIONAL to mark the two new registers as optional.

Signed-off-by Sumit Gupta <[email protected]>

Reviewed-by Mario Limonciello (AMD) <[email protected]>

Add acpi_cppc/ospm_nominal_perf sysfs attribute (read-write) and cppc_set_ospm_nominal_perf() API for the OSPM Nominal Performance register (ACPI 6.6, Section 8.4.6.1.2.6).

The register conveys the desired nominal performance level at which the platform may run. OSPM can request a lower level than platform nominal. Valid range is [Lowest Performance, Nominal Performance]. The value tells the platform what OSPM considers nominal. The platform classifies performance above this as boosted and below as throttled. It uses that for its power/thermal decisions.

Although the register is write-only per spec, cache the OSPM-written value in cpc_desc so userspace can observe it via sysfs, and to skip redundant writes.

Initialize to platform nominal at policy init. Override via sysfs if needed.

Signed-off-by Sumit Gupta <[email protected]>

---
  drivers/acpi/cppc_acpi.c       | 69 ++++++++++++++++++++++++++++++++++
  drivers/cpufreq/cppc_cpufreq.c | 10 +++++
  include/acpi/cppc_acpi.h       |  6 +++
  3 files changed, 85 insertions(+)

drivers/acpi/cppc_acpi.c +69 -0

index a1c91ce20cc8..fbc620adafad 100644
@@ -155,6 +155,10 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
  static struct kobj_attribute _name =		\
  __ATTR(_name, 0444, show_##_name, NULL)
 
+#define define_one_cppc_rw(_name)		\
+static struct kobj_attribute _name =		\
+__ATTR(_name, 0644, show_##_name, store_##_name)
+
  #define to_cpc_desc(a) container_of(a, struct cpc_desc, kobj)
 
  #define show_cppc_data(access_fn, struct_name, member_name)		\

@@ -211,6 +215,38 @@ static ssize_t show_feedback_ctrs(struct kobject *kobj,
  }
  define_one_cppc_ro(feedback_ctrs);
 
+static ssize_t show_ospm_nominal_perf(struct kobject *kobj,
+				      struct kobj_attribute *attr, char *buf)
+{
+	struct cpc_desc *cpc_ptr = to_cpc_desc(kobj);
+	u64 val = READ_ONCE(cpc_ptr->ospm_nominal_perf);
+
+	if (!val)
+		return -ENODATA;
+
+	return sysfs_emit(buf, "%llu\n", val);
+}
+
+static ssize_t store_ospm_nominal_perf(struct kobject *kobj,
+				       struct kobj_attribute *attr,
+				       const char *buf, size_t count)
+{
+	struct cpc_desc *cpc_ptr = to_cpc_desc(kobj);
+	u64 val;
+	int ret;
+
+	ret = kstrtou64(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = cppc_set_ospm_nominal_perf(cpc_ptr->cpu_id, val);
+	if (ret)
+		return ret;
+
+	return count;
+}
+define_one_cppc_rw(ospm_nominal_perf);
+
  static struct attribute *cppc_attrs[] = {
  	&feedback_ctrs.attr,
  	&reference_perf.attr,

@@ -222,6 +258,7 @@ static struct attribute *cppc_attrs[] = {
  	&nominal_perf.attr,
  	&nominal_freq.attr,
  	&lowest_freq.attr,
+	&ospm_nominal_perf.attr,
  	NULL
  };
  ATTRIBUTE_GROUPS(cppc);

@@ -1683,6 +1720,38 @@ int cppc_set_epp(int cpu, u64 epp_val)
  }
  EXPORT_SYMBOL_GPL(cppc_set_epp);
 
+/**
+ * cppc_set_ospm_nominal_perf() - Write OSPM Nominal Performance register.
+ * @cpu: CPU on which to write register.
+ * @ospm_nominal_perf: Value to write to the OSPM Nominal Performance register.
+ *
+ * OSPM Nominal Performance allows OSPM to inform the platform of the nominal
+ * performance level it intends to maintain.
+ *
+ * Return: 0 for success, -EINVAL on invalid input, -EOPNOTSUPP if not
+ * supported, -EIO otherwise.
+ */
+int cppc_set_ospm_nominal_perf(int cpu, u64 ospm_nominal_perf)
+{
+	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+	int ret;
+
+	if (!ospm_nominal_perf || ospm_nominal_perf > U32_MAX)
+		return -EINVAL;
+
+	if (cpc_desc &&
+	    READ_ONCE(cpc_desc->ospm_nominal_perf) == ospm_nominal_perf)
+		return 0;
+
+	ret = cppc_set_reg_val(cpu, OSPM_NOMINAL_PERF, ospm_nominal_perf);
+	if (ret)
+		return ret;
+
+	WRITE_ONCE(cpc_desc->ospm_nominal_perf, ospm_nominal_perf);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cppc_set_ospm_nominal_perf);
+
  /**
   * cppc_get_auto_act_window() - Read autonomous activity window register.
   * @cpu: CPU from which to read register.

drivers/cpufreq/cppc_cpufreq.c +10 -0

index 7e7f9dfb7a24..d06cba963550 100644
@@ -715,6 +715,16 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
  		goto out;
  	}
 
+	/*
+	 * Initialize OSPM Nominal Performance to inform firmware of
+	 * OSPM's nominal level. Performance above this value = boost;
+	 * below = throttle. Uses platform nominal by default.
+	 */
+	ret = cppc_set_ospm_nominal_perf(cpu, caps->nominal_perf);
+	if (ret && ret != -EOPNOTSUPP)
+		pr_debug("Failed to set ospm_nominal_perf for CPU%d: %d\n",
+			 cpu, ret);
+
  	cppc_cpufreq_cpu_fie_init(policy);
  	return 0;
 

include/acpi/cppc_acpi.h +6 -0

index 8693890a7275..3771e2ed507d 100644
@@ -86,6 +86,7 @@ struct cpc_desc {
  	struct cpc_register_resource cpc_regs[MAX_CPC_REG_ENT];
  	struct acpi_psd_package domain_info;
  	struct kobject kobj;
+	u32 ospm_nominal_perf;
  };
 
  /* These are indexes into the per-cpu cpc_regs[]. Order is important. */

@@ -180,6 +181,7 @@ 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_set_ospm_nominal_perf(int cpu, u64 ospm_nominal_perf);
  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);

@@ -266,6 +268,10 @@ static inline int cppc_set_epp(int cpu, u64 epp_val)
  {
  	return -EOPNOTSUPP;
  }
+static inline int cppc_set_ospm_nominal_perf(int cpu, u64 ospm_nominal_perf)
+{
+	return -EOPNOTSUPP;
+}
  static inline int cppc_get_auto_act_window(int cpu, u64 *auto_act_window)
  {
  	return -EOPNOTSUPP;

Add acpi_cppc/ospm_nominal_perf sysfs attribute (read-write) and cppc_set_ospm_nominal_perf() API for the OSPM Nominal Performance register (ACPI 6.6, Section 8.4.6.1.2.6).

The register conveys the desired nominal performance level at which the platform may run. OSPM can request a lower level than platform nominal. Valid range is [Lowest Performance, Nominal Performance]. The value tells the platform what OSPM considers nominal. The platform classifies performance above this as boosted and below as throttled. It uses that for its power/thermal decisions.

Although the register is write-only per spec, cache the OSPM-written value in cpc_desc so userspace can observe it via sysfs, and to skip redundant writes.

Initialize to platform nominal at policy init. Override via sysfs if needed.

Signed-off-by Sumit Gupta <[email protected]>

---
  drivers/acpi/cppc_acpi.c       | 69 ++++++++++++++++++++++++++++++++++
  drivers/cpufreq/cppc_cpufreq.c | 10 +++++
  include/acpi/cppc_acpi.h       |  6 +++
  3 files changed, 85 insertions(+)

drivers/acpi/cppc_acpi.c +45 -0

index a1c91ce20cc8..fbc620adafad 100644
@@ -155,6 +155,10 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
  static struct kobj_attribute _name =		\
  __ATTR(_name, 0444, show_##_name, NULL)
 
+#define define_one_cppc_rw(_name)		\
+static struct kobj_attribute _name =		\
+__ATTR(_name, 0644, show_##_name, store_##_name)
+
  #define to_cpc_desc(a) container_of(a, struct cpc_desc, kobj)
 
  #define show_cppc_data(access_fn, struct_name, member_name)		\

@@ -211,6 +215,38 @@ static ssize_t show_feedback_ctrs(struct kobject *kobj,
  }
  define_one_cppc_ro(feedback_ctrs);
 
+static ssize_t show_ospm_nominal_perf(struct kobject *kobj,
+				      struct kobj_attribute *attr, char *buf)
+{
+	struct cpc_desc *cpc_ptr = to_cpc_desc(kobj);
+	u64 val = READ_ONCE(cpc_ptr->ospm_nominal_perf);
+
+	if (!val)
+		return -ENODATA;
+
+	return sysfs_emit(buf, "%llu\n", val);
+}
+
+static ssize_t store_ospm_nominal_perf(struct kobject *kobj,
+				       struct kobj_attribute *attr,
+				       const char *buf, size_t count)
+{
+	struct cpc_desc *cpc_ptr = to_cpc_desc(kobj);
+	u64 val;
+	int ret;
+
+	ret = kstrtou64(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	ret = cppc_set_ospm_nominal_perf(cpc_ptr->cpu_id, val);
+	if (ret)
+		return ret;
+
+	return count;
+}
+define_one_cppc_rw(ospm_nominal_perf);
+
  static struct attribute *cppc_attrs[] = {
  	&feedback_ctrs.attr,
  	&reference_perf.attr,

@@ -222,6 +258,7 @@ static struct attribute *cppc_attrs[] = {
  	&nominal_perf.attr,
  	&nominal_freq.attr,
  	&lowest_freq.attr,
+	&ospm_nominal_perf.attr,
  	NULL
  };
  ATTRIBUTE_GROUPS(cppc);

@@ -1683,6 +1720,38 @@ int cppc_set_epp(int cpu, u64 epp_val)
  }
  EXPORT_SYMBOL_GPL(cppc_set_epp);
 
+/**
+ * cppc_set_ospm_nominal_perf() - Write OSPM Nominal Performance register.
+ * @cpu: CPU on which to write register.
+ * @ospm_nominal_perf: Value to write to the OSPM Nominal Performance register.
+ *
+ * OSPM Nominal Performance allows OSPM to inform the platform of the nominal
+ * performance level it intends to maintain.
+ *

drivers/acpi/cppc_acpi.c +5 -0

+	WRITE_ONCE(cpc_desc->ospm_nominal_perf, ospm_nominal_perf);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(cppc_set_ospm_nominal_perf);
+
  /**
   * cppc_get_auto_act_window() - Read autonomous activity window register.
   * @cpu: CPU from which to read register.

drivers/cpufreq/cppc_cpufreq.c +0 -0

index 7e7f9dfb7a24..d06cba963550 100644
@@ -715,6 +715,16 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy)
  		goto out;
  	}
 

drivers/cpufreq/cppc_cpufreq.c +0 -0

  	cppc_cpufreq_cpu_fie_init(policy);
  	return 0;
 

include/acpi/cppc_acpi.h +6 -0

index 8693890a7275..3771e2ed507d 100644
@@ -86,6 +86,7 @@ struct cpc_desc {
  	struct cpc_register_resource cpc_regs[MAX_CPC_REG_ENT];
  	struct acpi_psd_package domain_info;
  	struct kobject kobj;
+	u32 ospm_nominal_perf;
  };
 
  /* These are indexes into the per-cpu cpc_regs[]. Order is important. */

@@ -180,6 +181,7 @@ 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_set_ospm_nominal_perf(int cpu, u64 ospm_nominal_perf);
  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);

@@ -266,6 +268,10 @@ static inline int cppc_set_epp(int cpu, u64 epp_val)
  {
  	return -EOPNOTSUPP;
  }
+static inline int cppc_set_ospm_nominal_perf(int cpu, u64 ospm_nominal_perf)
+{
+	return -EOPNOTSUPP;
+}
  static inline int cppc_get_auto_act_window(int cpu, u64 *auto_act_window)
  {
  	return -EOPNOTSUPP;

External email: Use caution opening links or attachments

Hello Sumit,

Quoted 108 lines · Sumit Gupta

Add acpi_cppc/ospm_nominal_perf sysfs attribute (read-write) and cppc_set_ospm_nominal_perf() API for the OSPM Nominal Performance register (ACPI 6.6, Section 8.4.6.1.2.6).

The register conveys the desired nominal performance level at which the platform may run. OSPM can request a lower level than platform nominal. Valid range is [Lowest Performance, Nominal Performance]. The value tells the platform what OSPM considers nominal. The platform classifies performance above this as boosted and below as throttled. It uses that for its power/thermal decisions.

Although the register is write-only per spec, cache the OSPM-written value in cpc_desc so userspace can observe it via sysfs, and to skip redundant writes.

Initialize to platform nominal at policy init. Override via sysfs if needed.

Signed-off-by Sumit Gupta <[email protected]>

---
  drivers/acpi/cppc_acpi.c       | 69 ++++++++++++++++++++++++++++++++++ drivers/cpufreq/cppc_cpufreq.c | 10 +++++ include/acpi/cppc_acpi.h       |  6 +++

  3 files changed, 85 insertions(+)

drivers/acpi/cppc_acpi.c +4 -0

index a1c91ce20cc8..fbc620adafad 100644
@@ -155,6 +155,10 @@ static DEFINE_PER_CPU(struct cpc_desc *, 
 cpc_desc_ptr);
   static struct kobj_attribute _name =                \
   __ATTR(_name, 0444, show_##_name, NULL)
 
+#define define_one_cppc_rw(_name)            \
+static struct kobj_attribute _name =         \
+__ATTR(_name, 0644, show_##_name, store_##_name)
+
   #define to_cpc_desc(a) container_of(a, struct cpc_desc, kobj)
 

  #define show_cppc_data(access_fn, struct_name, member_name)         \

drivers/acpi/cppc_acpi.c +32 -0

@@ -211,6 +215,38 @@ static ssize_t show_feedback_ctrs(struct kobject 
 *kobj,
   }
   define_one_cppc_ro(feedback_ctrs);
 
+static ssize_t show_ospm_nominal_perf(struct kobject *kobj,
+                                   struct kobj_attribute *attr, char 
 *buf)
+{
+     struct cpc_desc *cpc_ptr = to_cpc_desc(kobj);
+     u64 val = READ_ONCE(cpc_ptr->ospm_nominal_perf);
+
+     if (!val)
+             return -ENODATA;
+
+     return sysfs_emit(buf, "%llu\n", val);
+}
+
+static ssize_t store_ospm_nominal_perf(struct kobject *kobj,
+                                    struct kobj_attribute *attr,
+                                    const char *buf, size_t count)
+{
+     struct cpc_desc *cpc_ptr = to_cpc_desc(kobj);
+     u64 val;
+     int ret;
+
+     ret = kstrtou64(buf, 0, &val);
+     if (ret)
+             return ret;
+
+     ret = cppc_set_ospm_nominal_perf(cpc_ptr->cpu_id, val);
+     if (ret)
+             return ret;
+
+     return count;
+}
+define_one_cppc_rw(ospm_nominal_perf);
+
   static struct attribute *cppc_attrs[] = {

      &feedback_ctrs.attr,
      &reference_perf.attr,

drivers/acpi/cppc_acpi.c +1 -0

@@ -222,6 +258,7 @@ static struct attribute *cppc_attrs[] = {
       &nominal_perf.attr,
       &nominal_freq.attr,
       &lowest_freq.attr,
+     &ospm_nominal_perf.attr,

      NULL
  };
  ATTRIBUTE_GROUPS(cppc);

drivers/acpi/cppc_acpi.c +2 -0

@@ -1683,6 +1720,38 @@ int cppc_set_epp(int cpu, u64 epp_val)
   }
   EXPORT_SYMBOL_GPL(cppc_set_epp);
 
+/**
+ * cppc_set_ospm_nominal_perf() - Write OSPM Nominal Performance 

register.
+ * @cpu: CPU on which to write register.
+ * @ospm_nominal_perf: Value to write to the OSPM Nominal Performance register.
+ *
+ * OSPM Nominal Performance allows OSPM to inform the platform of the nominal
+ * performance level it intends to maintain.
+ *
+ * Return: 0 for success, -EINVAL on invalid input, -EOPNOTSUPP if not
+ * supported, -EIO otherwise.
+ */

drivers/acpi/cppc_acpi.c int cppc_set_epp(int cpu, u64 epp_val) · line 1740

Quoted 7 lines

drivers/acpi/cppc_acpi.c +7 -0

+int cppc_set_ospm_nominal_perf(int cpu, u64 ospm_nominal_perf)
+{
+     struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
+     int ret;
+
+     if (!ospm_nominal_perf || ospm_nominal_perf > U32_MAX)
+             return -EINVAL;

I think the spec also requests to have a value in the range

[lowest:nominal]. As these registers are read-only it should

be ok to read the values here ?

drivers/acpi/cppc_acpi.c int cppc_set_epp(int cpu, u64 epp_val) · line 1749

Quoted 10 lines

drivers/acpi/cppc_acpi.c +9 -0

+
+     if (cpc_desc &&
+         READ_ONCE(cpc_desc->ospm_nominal_perf) == ospm_nominal_perf)
+             return 0;
+
+     ret = cppc_set_reg_val(cpu, OSPM_NOMINAL_PERF, ospm_nominal_perf);
+     if (ret)
+             return ret;
+

Shouldn't we have some protection against concurrent accesses ?

Quoted 17 lines

+ WRITE_ONCE(cpc_desc->ospm_nominal_perf, ospm_nominal_perf); +     return 0;

drivers/acpi/cppc_acpi.c +3 -0

+}
+EXPORT_SYMBOL_GPL(cppc_set_ospm_nominal_perf);
+

  /**
   * cppc_get_auto_act_window() - Read autonomous activity window register.
   * @cpu: CPU from which to read register.

drivers/cpufreq/cppc_cpufreq.c +0 -0

index 7e7f9dfb7a24..d06cba963550 100644
@@ -715,6 +715,16 @@ static int cppc_cpufreq_cpu_init(struct 
 cpufreq_policy *policy)
               goto out;
       }
 

drivers/cpufreq/cppc_cpufreq.c static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) · line 727

Quoted 11 lines

drivers/cpufreq/cppc_cpufreq.c +10 -0

+     /*
+      * Initialize OSPM Nominal Performance to inform firmware of
+      * OSPM's nominal level. Performance above this value = boost;
+      * below = throttle. Uses platform nominal by default.
+      */
+     ret = cppc_set_ospm_nominal_perf(cpu, caps->nominal_perf);
+     if (ret && ret != -EOPNOTSUPP)
+             pr_debug("Failed to set ospm_nominal_perf for CPU%d: 
 %d\n",
+                      cpu, ret);
+

IIUC, if (ospm_nominal_perf == nominal_perf), the firmware should not behave differently. Is this really useful ?

------------

Also this seems like there will need some synchronization mechanism to keep-up with the boost state.

If the ospm_nominal_perf is lowered and boost is disabled, a freq. update should happen. IMO it looks like this could be handled with (another) freq_qos_request.

This new freq_qos_request, if we name it ospm_nominal_freq_req, should only be taken into account if boost is disabled. Otherwise, if boost is enabled, ospm_nominal_freq_req should be ignored.

------------

Also, the function seems to set the ospm_nominal_freq for a single CPU when the policy might be common for multiple CPUs right ?

The issues this field raises seems similar to the auto_sel ones. I.e. :

- concurrency accesses + need for a scratch value

- what should happen when unloading the driver

- the value can be set for single CPUs but we might want to have the same value for the whole policy

Maybe a common solution should be found.
(I m not suggesting anything right now unfortunately).

Quoted 10 lines

Quoted 10 lines

drivers/cpufreq/cppc_cpufreq.c +0 -0

index 7e7f9dfb7a24..d06cba963550 100644
@@ -715,6 +715,16 @@ static int cppc_cpufreq_cpu_init(struct 
 cpufreq_policy *policy)
               goto out;
       }
 

Quoted 15 lines

drivers/cpufreq/cppc_cpufreq.c static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) · line 727

Quoted 11 lines

drivers/cpufreq/cppc_cpufreq.c +10 -0

+     /*
+      * Initialize OSPM Nominal Performance to inform firmware of
+      * OSPM's nominal level. Performance above this value = boost;
+      * below = throttle. Uses platform nominal by default.
+      */
+     ret = cppc_set_ospm_nominal_perf(cpu, caps->nominal_perf);
+     if (ret && ret != -EOPNOTSUPP)
+             pr_debug("Failed to set ospm_nominal_perf for CPU%d: 
 %d\n",
+                      cpu, ret);
+

IIUC, if (ospm_nominal_perf == nominal_perf), the firmware should not behave differently. Is this really useful ?

Right, it's a no-op from the firmware's side. The init was only so that sysfs would show a value (platform nominal) before any userspace write. Will drop it in v3 and return 0 from sysfs until userspace writes a value.

Quoted 5 lines

------------

Also this seems like there will need some synchronization mechanism to keep-up with the boost state.

Quoted 9 lines

If the ospm_nominal_perf is lowered and boost is disabled, a freq. update should happen. IMO it looks like this could be handled with (another) freq_qos_request.

This new freq_qos_request, if we name it ospm_nominal_freq_req, should only be taken into account if boost is disabled. Otherwise, if boost is enabled, ospm_nominal_freq_req should be ignored.

Agreed, will add the new freq_qos_request in a follow-up patch.

Quoted 5 lines

------------

Also, the function seems to set the ospm_nominal_freq for a single CPU when the policy might be common for multiple CPUs right ?

In v3, after dropping the change from cppc_cpufreq_cpu_init, the problem won't come in this specific instance.

Quoted 11 lines

The issues this field raises seems similar to the auto_sel ones. I.e. :

- concurrency accesses + need for a scratch value

- what should happen when unloading the driver

- the value can be set for single CPUs but we might want to have the same value for the whole policy

Quoted 3 lines

Maybe a common solution should be found.
(I m not suggesting anything right now unfortunately).

One way to address this is to move the sysfs from per-CPU acpi_cppc to a per-policy node under cpufreq (ospm_nominal_perf_freq, kHz).

In the sysfs callback, we can convert kHz to perf and write the register on every CPU in policy->cpus.
Concurrency is already covered by policy->rwsem at the cpufreq layer. This is similar to how we were handling min/max_perf in earlier version.

Does this approach make sense?

Thank you,
Sumit Gupta

....

External email: Use caution opening links or attachments

Quoted 75 lines

Quoted 11 lines

Quoted 11 lines

drivers/cpufreq/cppc_cpufreq.c +0 -0

index 7e7f9dfb7a24..d06cba963550 100644
@@ -715,6 +715,16 @@ static int cppc_cpufreq_cpu_init(struct
 cpufreq_policy *policy)
               goto out;
       }
 

Quoted 15 lines

drivers/cpufreq/cppc_cpufreq.c static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) · line 727

Quoted 11 lines

drivers/cpufreq/cppc_cpufreq.c +10 -0

+     /*
+      * Initialize OSPM Nominal Performance to inform firmware of
+      * OSPM's nominal level. Performance above this value = boost;
+      * below = throttle. Uses platform nominal by default.
+      */
+     ret = cppc_set_ospm_nominal_perf(cpu, caps->nominal_perf);
+     if (ret && ret != -EOPNOTSUPP)
+             pr_debug("Failed to set ospm_nominal_perf for CPU%d:
 %d\n",
+                      cpu, ret);
+

IIUC, if (ospm_nominal_perf == nominal_perf), the firmware should not behave differently. Is this really useful ?

Right, it's a no-op from the firmware's side. The init was only so that sysfs would show a value (platform nominal) before any userspace write. Will drop it in v3 and return 0 from sysfs until userspace writes a value.

Quoted 5 lines

------------

Also this seems like there will need some synchronization mechanism to keep-up with the boost state.

Quoted 9 lines

If the ospm_nominal_perf is lowered and boost is disabled, a freq. update should happen. IMO it looks like this could be handled with (another) freq_qos_request.

This new freq_qos_request, if we name it ospm_nominal_freq_req, should only be taken into account if boost is disabled. Otherwise, if boost is enabled, ospm_nominal_freq_req should be ignored.

Agreed, will add the new freq_qos_request in a follow-up patch.

Quoted 5 lines

------------

Also, the function seems to set the ospm_nominal_freq for a single CPU when the policy might be common for multiple CPUs right ?

In v3, after dropping the change from cppc_cpufreq_cpu_init, the problem won't come in this specific instance.

Quoted 14 lines

The issues this field raises seems similar to the auto_sel ones. I.e. :

- concurrency accesses + need for a scratch value

- what should happen when unloading the driver

- the value can be set for single CPUs but we might want to have the same value for the whole policy

Maybe a common solution should be found.
(I m not suggesting anything right now unfortunately).

Quoted 2 lines

One way to address this is to move the sysfs from per-CPU acpi_cppc to a per-policy node under cpufreq (ospm_nominal_perf_freq, kHz).

Yes right, would it make sense to also move the "ospm_nominal_perf" to the cpu_data ?

Quoted 5 lines

In the sysfs callback, we can convert kHz to perf and write the register on every CPU in policy->cpus.
Concurrency is already covered by policy->rwsem at the cpufreq layer. This is similar to how we were handling min/max_perf in earlier version.

Yes right. Something I don't really understand is that similarly to auto_sel,
the sysfs rwsem protects the state of the cpu_data->auto_sel field, but we don't really have any guarantee that another driver won't modify the firmware state right ? (maybe it's just not worth ?)

Quoted 2 lines

Does this approach make sense?

Yes right thanks !

Quoted 7 lines

Thank you,
Sumit Gupta

....

Add initial kernel support for CPPC v4 (ACPI 6.6, Section 8.4.6), which extends the _CPC package from 23 to 25 entries with two optional fields:

- OSPM Nominal Performance (8.4.6.1.2.6): register used by OSPM to tell the platform what it considers nominal. The platform classifies performance above this as boost and below as throttle for power/thermal decisions.

- Resource Priority (8.4.6.1.2.7): Package of Resource Priority Register Descriptor sub-packages. Full parsing is not yet implemented; such entries are marked as unsupported.

Patch 1: Add v4 _CPC parsing - validate the 25-entry layout, handle the Resource Priority package, and mark the two new registers optional.

Patch 2: Add acpi_cppc/ospm_nominal_perf as a read-write sysfs attribute, and initialize it to the platform nominal value during cppc_cpufreq policy init.

---
v1[1] -> v2:
- Patch 1: added Reviewed-by from Mario Limonciello.
- Patch 2:
- Make ospm_nominal_perf sysfs read-write; cache last write in cpc_desc and skip redundant register writes.
- Validate input in cppc_set_ospm_nominal_perf.

Sumit Gupta (2):
ACPI: CPPC: Add support for CPPC v4
ACPI: CPPC: Add ospm_nominal_perf support

 drivers/acpi/cppc_acpi.c       | 93 +++++++++++++++++++++++++++++++---
 drivers/cpufreq/cppc_cpufreq.c | 10 ++++
 include/acpi/cppc_acpi.h       | 14 ++++-
 3 files changed, 109 insertions(+), 8 deletions(-)

[1] https://lore.kernel.org/lkml/[email protected]/

--

External email: Use caution opening links or attachments

Quoted 38 lines · Sumit Gupta

Add initial kernel support for CPPC v4 (ACPI 6.6, Section 8.4.6), which extends the _CPC package from 23 to 25 entries with two optional fields:

- OSPM Nominal Performance (8.4.6.1.2.6): register used by OSPM to tell the platform what it considers nominal. The platform classifies performance above this as boost and below as throttle for power/thermal decisions.

- Resource Priority (8.4.6.1.2.7): Package of Resource Priority Register Descriptor sub-packages. Full parsing is not yet implemented; such entries are marked as unsupported.

Patch 1: Add v4 _CPC parsing - validate the 25-entry layout, handle the Resource Priority package, and mark the two new registers optional.

Patch 2: Add acpi_cppc/ospm_nominal_perf as a read-write sysfs attribute, and initialize it to the platform nominal value during cppc_cpufreq policy init.

---
v1[1] -> v2:
- Patch 1: added Reviewed-by from Mario Limonciello.
- Patch 2:
- Make ospm_nominal_perf sysfs read-write; cache last write in cpc_desc and skip redundant register writes.
- Validate input in cppc_set_ospm_nominal_perf.

Sumit Gupta (2):
ACPI: CPPC: Add support for CPPC v4
ACPI: CPPC: Add ospm_nominal_perf support

  drivers/acpi/cppc_acpi.c       | 93 +++++++++++++++++++++++++++++++---
  drivers/cpufreq/cppc_cpufreq.c | 10 ++++
  include/acpi/cppc_acpi.h       | 14 ++++-
  3 files changed, 109 insertions(+), 8 deletions(-)

Quoted 3 lines

[1] https://lore.kernel.org/lkml/[email protected]/

--

Can you please see the sashiko.dev feedback on this set:

https://sashiko.dev/#/patchset/20260430142430.755437-1-sumitg%40nvidia.com

and let me know what you think? Especially regarding the second patch?

The commit message states the valid range is [Lowest Performance, Nominal Performance]. Does this code allow writing arbitrary values outside that range by only checking against U32_MAX, without fetching the CPU's capabilities to validate the input?

If the hardware loses state during a logical CPU hotplug or system suspend, but the software cache is not invalidated, will this check prevent the register from being correctly re-initialized when the CPU comes back online?

Can concurrent sysfs writes permanently desynchronize the software cache from the hardware register?
...
Is a lock needed around the read-modify-write cycle?

Additionally, can a time-of-check to time-of-use race lead to a NULL pointer dereference if cpc_desc_ptr is initialized concurrently?
...
Would this cause the WRITE_ONCE() to dereference the locally fetched NULL cpc_desc pointer? Should this explicitly return -ENODEV early if !cpc_desc?

For shared cpufreq policies where policy->cpus contains multiple logical cores (such as CPUFREQ_SHARED_TYPE_ANY), does this skip initializing the secondary CPUs in the domain?

If they are uninitialized, will their local cache remain 0, causing sysfs reads for those secondary CPUs to incorrectly return -ENODATA?

Also, since the sysfs attribute is tied to the physical CPU device lifetime and persists independently of cpufreq policy teardowns, will unconditionally setting the nominal performance here silently clobber any persistent userspace configurations when a CPU is taken offline and online?