[PATCH v2] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs()

Yufan Dou posted 1 patch 2 weeks, 2 days ago
drivers/acpi/riscv/cppc.c | 102 ++++++++++++++++++++++++++++++++++++--
1 file changed, 97 insertions(+), 5 deletions(-)
[PATCH v2] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs()
Posted by Yufan Dou 2 weeks, 2 days ago
On RISC-V, reading FFH feedback counters on a remote hart through
cpc_read_ffh() requires a separate IPI for each register.
cppc_get_perf_ctrs() therefore samples the delivered and reference
counters in two separate IPIs. Variation in the delay between these
reads distorts the delivered/reference ratio and thus the frequency
reported by cpufreq.

Implement cpc_read_ffh_fb_ctrs() to read both counters back-to-back
in a single callback on the target hart. For remote reads, this
reduces two IPIs to one and narrows the separation between the
counter samples. Support CSR-type, SBI-type and mixed-type pairs.
Pairing SBI reads does not reduce the number of SBI calls, but
still avoids a separate IPI for each counter.

Factor the execution-context handling into cppc_ffh_read_on_cpu(),
shared by cpc_read_ffh() and cpc_read_ffh_fb_ctrs(). CPPC frequency
invariance (FIE) reads non-PCC counters directly from the scheduler
tick, with local interrupts disabled. Unconditionally rejecting
such reads prevents the frequency scale from being updated.

Following arm64's counters_read_on_cpu(), invoke the callback
directly when interrupts are disabled and the target is the current
CPU. Reject remote reads in that context. Otherwise, use
smp_call_function_single() and propagate its return value before
accessing the callback results.

Frequency accuracy testing used CSR-type delivered and reference
counters, with CPU1 kept busy by:

  # stress-ng --cpu 1 --taskset 1

On a CPU with cpuinfo_max_freq of 3000000 kHz:

Before:

  Maximum observed cpuinfo_cur_freq: 3201369 kHz
  Maximum observed deviation:        +201369 kHz (+6.71%)

After:

  Maximum observed cpuinfo_cur_freq: 3009646 kHz
  Maximum observed deviation:          +9646 kHz (+0.32%)

The maximum observed deviation from cpuinfo_max_freq decreased
by 95.2%.

Additional testing covered the CPPC FIE tick path and SBI-type
FFH register reads.

Co-developed-by: Yicong Yang <yang.yicong@picoheart.com>
Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
Signed-off-by: Yufan Dou <douyufan@picoheart.com>
Reviewed-by: Sunil V L <sunilvl@oss.qualcomm.com>
---
Changes in v2:
- Support SBI-type and mixed-type counter pairs in a single callback
  on the target hart, reducing remote reads to one IPI.
- Factor out cppc_ffh_read_on_cpu() for both FFH read interfaces.
  Allow local reads with interrupts disabled and reject remote reads
  in that context, following arm64's counters_read_on_cpu().
- Propagate synchronous cross-CPU call errors in cpc_read_ffh().
- Test the CPPC FIE tick path and SBI-type FFH register reads.

v1:
https://lore.kernel.org/all/20260902083450.2348-1-douyufan@picoheart.com/

 drivers/acpi/riscv/cppc.c | 102 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 97 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/riscv/cppc.c b/drivers/acpi/riscv/cppc.c
index 42c1a9052470..0f580cd6c21e 100644
--- a/drivers/acpi/riscv/cppc.c
+++ b/drivers/acpi/riscv/cppc.c
@@ -85,6 +85,55 @@ static void cppc_ffh_csr_write(void *write_data)
 	data->ret.error = -EINVAL;
 }
 
+struct cppc_ffh_ctr {
+	struct sbi_cppc_data data;
+	u64 type;
+};
+
+struct cppc_ffh_fb_ctrs_data {
+	struct cppc_ffh_ctr first;
+	struct cppc_ffh_ctr second;
+};
+
+static void cppc_ffh_read_fb_ctrs(void *read_data)
+{
+	struct cppc_ffh_fb_ctrs_data *data = read_data;
+
+	if (data->first.type == FFH_CPPC_SBI)
+		sbi_cppc_read(&data->first.data);
+	else
+		cppc_ffh_csr_read(&data->first.data);
+
+	if (data->second.type == FFH_CPPC_SBI)
+		sbi_cppc_read(&data->second.data);
+	else
+		cppc_ffh_csr_read(&data->second.data);
+}
+
+static int cppc_ffh_ctr_errno(const struct cppc_ffh_ctr *ctr)
+{
+	if (!ctr->data.ret.error)
+		return 0;
+
+	return ctr->type == FFH_CPPC_SBI ?
+	       sbi_err_map_linux_errno(ctr->data.ret.error) :
+	       ctr->data.ret.error;
+}
+
+static int cppc_ffh_read_on_cpu(int cpu, smp_call_func_t func, void *data)
+{
+	if (irqs_disabled()) {
+		/* Remote reads require IPIs, which are unsafe with IRQs disabled. */
+		if (WARN_ON_ONCE(cpu != smp_processor_id()))
+			return -EPERM;
+
+		func(data);
+		return 0;
+	}
+
+	return smp_call_function_single(cpu, func, data, 1);
+}
+
 /*
  * Refer to drivers/acpi/cppc_acpi.c for the description of the functions
  * below.
@@ -97,9 +146,7 @@ bool cpc_ffh_supported(void)
 int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
 {
 	struct sbi_cppc_data data;
-
-	if (WARN_ON_ONCE(irqs_disabled()))
-		return -EPERM;
+	int ret;
 
 	if (FFH_CPPC_TYPE(reg->address) == FFH_CPPC_SBI) {
 		if (!cppc_ext_present)
@@ -107,7 +154,9 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
 
 		data.reg = FFH_CPPC_SBI_REG(reg->address);
 
-		smp_call_function_single(cpu, sbi_cppc_read, &data, 1);
+		ret = cppc_ffh_read_on_cpu(cpu, sbi_cppc_read, &data);
+		if (ret)
+			return ret;
 
 		*val = data.ret.value;
 
@@ -115,7 +164,9 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
 	} else if (FFH_CPPC_TYPE(reg->address) == FFH_CPPC_CSR) {
 		data.reg = FFH_CPPC_CSR_NUM(reg->address);
 
-		smp_call_function_single(cpu, cppc_ffh_csr_read, &data, 1);
+		ret = cppc_ffh_read_on_cpu(cpu, cppc_ffh_csr_read, &data);
+		if (ret)
+			return ret;
 
 		*val = data.ret.value;
 
@@ -125,6 +176,47 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
 	return -EINVAL;
 }
 
+int cpc_read_ffh_fb_ctrs(int cpu, struct cpc_reg *reg1, u64 *val1,
+			 struct cpc_reg *reg2, u64 *val2)
+{
+	struct cppc_ffh_fb_ctrs_data data;
+	int ret;
+
+	data.first.type = FFH_CPPC_TYPE(reg1->address);
+	data.second.type = FFH_CPPC_TYPE(reg2->address);
+
+	if ((data.first.type != FFH_CPPC_SBI && data.first.type != FFH_CPPC_CSR) ||
+	    (data.second.type != FFH_CPPC_SBI && data.second.type != FFH_CPPC_CSR))
+		return -EINVAL;
+
+	if ((data.first.type == FFH_CPPC_SBI || data.second.type == FFH_CPPC_SBI) &&
+	    !cppc_ext_present)
+		return -EINVAL;
+
+	data.first.data.reg = data.first.type == FFH_CPPC_SBI ?
+			      FFH_CPPC_SBI_REG(reg1->address) :
+			      FFH_CPPC_CSR_NUM(reg1->address);
+	data.second.data.reg = data.second.type == FFH_CPPC_SBI ?
+			       FFH_CPPC_SBI_REG(reg2->address) :
+			       FFH_CPPC_CSR_NUM(reg2->address);
+
+	ret = cppc_ffh_read_on_cpu(cpu, cppc_ffh_read_fb_ctrs, &data);
+	if (ret)
+		return ret;
+
+	ret = cppc_ffh_ctr_errno(&data.first);
+	if (ret)
+		return ret;
+	ret = cppc_ffh_ctr_errno(&data.second);
+	if (ret)
+		return ret;
+
+	*val1 = data.first.data.ret.value;
+	*val2 = data.second.data.ret.value;
+
+	return 0;
+}
+
 int cpc_write_ffh(int cpu, struct cpc_reg *reg, u64 val)
 {
 	struct sbi_cppc_data data;

base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04
-- 
2.34.1
Re: [PATCH v2] ACPI: RISC-V: CPPC: Implement cpc_read_ffh_fb_ctrs()
Posted by Jie Zhan 1 week, 1 day ago

On 9/9/2026 3:15 PM, Yufan Dou wrote:
> On RISC-V, reading FFH feedback counters on a remote hart through
> cpc_read_ffh() requires a separate IPI for each register.
> cppc_get_perf_ctrs() therefore samples the delivered and reference
> counters in two separate IPIs. Variation in the delay between these
> reads distorts the delivered/reference ratio and thus the frequency
> reported by cpufreq.
> 
> Implement cpc_read_ffh_fb_ctrs() to read both counters back-to-back
> in a single callback on the target hart. For remote reads, this
> reduces two IPIs to one and narrows the separation between the
> counter samples. Support CSR-type, SBI-type and mixed-type pairs.
> Pairing SBI reads does not reduce the number of SBI calls, but
> still avoids a separate IPI for each counter.
> 
> Factor the execution-context handling into cppc_ffh_read_on_cpu(),
> shared by cpc_read_ffh() and cpc_read_ffh_fb_ctrs(). CPPC frequency
> invariance (FIE) reads non-PCC counters directly from the scheduler
> tick, with local interrupts disabled. Unconditionally rejecting
> such reads prevents the frequency scale from being updated.
> 
> Following arm64's counters_read_on_cpu(), invoke the callback
> directly when interrupts are disabled and the target is the current
> CPU. Reject remote reads in that context. Otherwise, use
> smp_call_function_single() and propagate its return value before
> accessing the callback results.
> 
> Frequency accuracy testing used CSR-type delivered and reference
> counters, with CPU1 kept busy by:
> 
>   # stress-ng --cpu 1 --taskset 1
> 
> On a CPU with cpuinfo_max_freq of 3000000 kHz:
> 
> Before:
> 
>   Maximum observed cpuinfo_cur_freq: 3201369 kHz
>   Maximum observed deviation:        +201369 kHz (+6.71%)
> 
> After:
> 
>   Maximum observed cpuinfo_cur_freq: 3009646 kHz
>   Maximum observed deviation:          +9646 kHz (+0.32%)
> 
> The maximum observed deviation from cpuinfo_max_freq decreased
> by 95.2%.
> 
> Additional testing covered the CPPC FIE tick path and SBI-type
> FFH register reads.
> 
> Co-developed-by: Yicong Yang <yang.yicong@picoheart.com>
> Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
> Signed-off-by: Yufan Dou <douyufan@picoheart.com>
> Reviewed-by: Sunil V L <sunilvl@oss.qualcomm.com>
LGTM, thanks!
Reviewed-by: Jie Zhan <zhanjie9@hisilicon.com> 
> ---
> Changes in v2:
> - Support SBI-type and mixed-type counter pairs in a single callback
>   on the target hart, reducing remote reads to one IPI.
> - Factor out cppc_ffh_read_on_cpu() for both FFH read interfaces.
>   Allow local reads with interrupts disabled and reject remote reads
>   in that context, following arm64's counters_read_on_cpu().
> - Propagate synchronous cross-CPU call errors in cpc_read_ffh().
> - Test the CPPC FIE tick path and SBI-type FFH register reads.
> 
> v1:
> https://lore.kernel.org/all/20260902083450.2348-1-douyufan@picoheart.com/
> 
>  drivers/acpi/riscv/cppc.c | 102 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 97 insertions(+), 5 deletions(-)
> 
[ ... ]