| 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: <aJMCgGt5zu5Dhrd5@arm.com>
Date: Wed, 6 Aug 2025 09:21:36 +0200
From: Beata Michalska <beata.michalska@....com>
To: Prashant Malani <pmalani@...gle.com>
Cc: Viresh Kumar <viresh.kumar@...aro.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Jie Zhan <zhanjie9@...ilicon.com>,
Ionela Voinescu <ionela.voinescu@....com>,
Ben Segall <bsegall@...gle.com>,
Dietmar Eggemann <dietmar.eggemann@....com>,
Ingo Molnar <mingo@...hat.com>, Juri Lelli <juri.lelli@...hat.com>,
open list <linux-kernel@...r.kernel.org>,
"open list:CPU FREQUENCY SCALING FRAMEWORK" <linux-pm@...r.kernel.org>,
Mel Gorman <mgorman@...e.de>, Peter Zijlstra <peterz@...radead.org>,
Steven Rostedt <rostedt@...dmis.org>,
Valentin Schneider <vschneid@...hat.com>,
Vincent Guittot <vincent.guittot@...aro.org>,
z00813676 <zhenglifeng1@...wei.com>, sudeep.holla@....com
Subject: Re: [PATCH v2 2/2] cpufreq: CPPC: Dont read counters for idle CPUs
On Mon, Aug 04, 2025 at 01:55:37PM -0700, Prashant Malani wrote:
> On Fri, 1 Aug 2025 at 02:16, Prashant Malani <pmalani@...gle.com> wrote:
> > On Thu, 31 Jul 2025 at 13:31, Beata Michalska <beata.michalska@....com> wrote:
> > > Thank you for the info, but I'm exploring ways that will not increase the time
> > > window between the reads.
> >
> > IMO this issue is intractable on non-RT OSes like Linux (at least,
> > Linux when it is not compiled for RT), since we basically need to
> > ensure atomicity for the reading of both ref and del registers together.
> > We can't disable preemption here, since some of
> > the code paths (like PCC regs) acquire semaphores [2].
>
> Actually, minor correction here. The PCC path is not the issue.
> It's the ffh read path[3], which requires interrupts to be enabled.
> Larger point still stands.
>
> [3] https://elixir.bootlin.com/linux/v6.16-rc7/source/arch/arm64/kernel/topology.c#L451
>
> BR,
>
> -Prashant
Would you mind giving it a go and see whether that improves things on your end ?
Note that this is a quick and semi-dirty hack though.
---
BR
Beata
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
index 5d07ee85bdae..65adb78a9a87 100644
--- a/arch/arm64/kernel/topology.c
+++ b/arch/arm64/kernel/topology.c
@@ -479,6 +479,11 @@ bool cpc_ffh_supported(void)
return true;
}
+bool cpc_burst_read_supported(void)
+{
+ return cpc_ffh_supported();
+}
+
int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
{
int ret = -EOPNOTSUPP;
@@ -501,6 +506,61 @@ int cpc_read_ffh(int cpu, struct cpc_reg *reg, u64 *val)
return ret;
}
+
+struct cpc_burst_read {
+ struct cpc_reg_sample *samples;
+ size_t count;
+};
+
+void counters_burst_read_on_cpu(void *arg)
+{
+ struct cpc_burst_read *desc = arg;
+ u64 value;
+ int i;
+
+ for (i = 0; i < desc->count; ++i) {
+ switch ((u64)desc->samples[i].reg->address) {
+ case 0x0:
+ value = read_corecnt();
+ break;
+ case 0x1:
+ value = read_constcnt();
+ break;
+ }
+ *(u64 *)desc->samples[i].sample_value = value;
+ }
+
+ for (i = 0; i < desc->count; ++i) {
+ struct cpc_reg *reg = desc->samples[i].reg;
+
+ value = *(u64 *)desc->samples[i].sample_value;
+ value &= GENMASK_ULL(reg->bit_offset + reg->bit_width - 1,
+ reg->bit_offset);
+ value >>= reg->bit_offset;
+ *(u64 *)desc->samples[i].sample_value = value;
+ }
+}
+
+static inline bool cpc_reg_supported(struct cpc_reg *reg)
+{
+ return !((u64)reg->address != 0x0 || (u64)reg->address != 0x1);
+}
+
+int cpc_burst_read_ffh(int cpu, struct cpc_reg_sample *samples, size_t count)
+{
+ struct cpc_burst_read desc = { samples, count };
+ int ret = -EOPNOTSUPP;
+ int i;
+
+ for (i = 0; i < count; ++i) {
+ if (!cpc_reg_supported(samples[i].reg))
+ return ret;
+ }
+
+ smp_call_function_single(cpu, counters_burst_read_on_cpu, &desc, 1);
+ return 0;
+}
+
int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
{
return -EOPNOTSUPP;
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 6b649031808f..c070627e4a1e 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -617,6 +617,11 @@ bool __weak cpc_supported_by_cpu(void)
return false;
}
+bool __weak cpc_burst_read_supported(void)
+{
+ return false;
+}
+
/**
* pcc_data_alloc() - Allocate the pcc_data memory for pcc subspace
* @pcc_ss_id: PCC Subspace index as in the PCC client ACPI package.
@@ -1077,6 +1082,21 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)
return 0;
}
+static int cpc_burst_read(int cpu, struct cpc_reg_sample *samples, size_t count)
+{
+ int i;
+
+ // Just for now - only ffh
+ if (!cpc_ffh_supported())
+ return -EINVAL;
+
+ for (i = 0; i < count; ++i)
+ if (samples[i].reg->space_id != ACPI_ADR_SPACE_FIXED_HARDWARE)
+ return -EINVAL;
+ return cpc_burst_read_ffh(cpu, samples, count);
+}
+
+
static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
{
int ret_val = 0;
@@ -1515,8 +1535,21 @@ int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs)
}
}
- cpc_read(cpunum, delivered_reg, &delivered);
- cpc_read(cpunum, reference_reg, &reference);
+ // Verify whether the registers can be requested in one go
+ if (delivered_reg->type != ACPI_TYPE_INTEGER &&
+ reference_reg->type != ACPI_TYPE_INTEGER &&
+ cpc_burst_read_supported()) {
+
+ struct cpc_reg_sample samples[] = {
+ { &delivered_reg->cpc_entry.reg, &delivered },
+ { &reference_reg->cpc_entry.reg, &reference }
+ };
+
+ cpc_burst_read(cpunum, samples, ARRAY_SIZE(samples));
+ } else {
+ cpc_read(cpunum, delivered_reg, &delivered);
+ cpc_read(cpunum, reference_reg, &reference);
+ }
cpc_read(cpunum, ref_perf_reg, &ref_perf);
/*
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 325e9543e08f..f094e275834a 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -52,6 +52,12 @@ struct cpc_reg {
u64 address;
} __packed;
+
+struct cpc_reg_sample {
+ struct cpc_reg *reg;
+ void *sample_value;
+};
+
/*
* Each entry in the CPC table is either
* of type ACPI_TYPE_BUFFER or
@@ -165,6 +171,7 @@ extern unsigned int cppc_get_transition_latency(int cpu);
extern bool cpc_ffh_supported(void);
extern bool cpc_supported_by_cpu(void);
extern int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val);
+extern int cpc_burst_read_ffh(int cpunum, struct cpc_reg_sample *samples, size_t count);
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);
@@ -229,6 +236,10 @@ static inline int cpc_read_ffh(int cpunum, struct cpc_reg *reg, u64 *val)
{
return -EOPNOTSUPP;
}
+static inline int cpc_burst_read_ffh(int cpunum, struct cpc_reg_sample *samples, size_t count)
+{
+ return -EOPNOTSUPP;
+}
static inline int cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
{
return -EOPNOTSUPP;
--
Powered by blists - more mailing lists