| 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: <20230526-topic-smd_icc-v6-22-263283111e66@linaro.org>
Date: Wed, 14 Jun 2023 20:04:41 +0200
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Georgi Djakov <djakov@...nel.org>,
Leo Yan <leo.yan@...aro.org>,
Evan Green <evgreen@...omium.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>
Cc: Marijn Suijten <marijn.suijten@...ainline.org>,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-clk@...r.kernel.org, linux-pm@...r.kernel.org,
devicetree@...r.kernel.org,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Stephan Gerhold <stephan@...hold.net>
Subject: [PATCH v6 22/22] interconnect: qcom: icc-rpm: Fix bandwidth
calculations
Up until now, we've been aggregating the bandwidth values and only
dividing them by the bus width of the source node. This was completely
wrong, as different nodes on a given path may (and usually do) have
varying bus widths. That in turn, resulted in the calculated clock rates
being completely bogus - usually they ended up being much higher, as
NoC_A<->NoC_B links are very wide.
Since we're not using the aggregate bandwidth value for anything other
than clock rate calculations, remodel qcom_icc_bus_aggregate() to
calculate the per-context clock rate for a given provider, taking into
account the bus width of every individual node.
Fixes: 30c8fa3ec61a ("interconnect: qcom: Add MSM8916 interconnect provider driver")
Reported-by: Stephan Gerhold <stephan@...hold.net>
Reviewed-by: Stephan Gerhold <stephan@...hold.net>
Signed-off-by: Konrad Dybcio <konrad.dybcio@...aro.org>
---
drivers/interconnect/qcom/icc-rpm.c | 59 ++++++++++++-------------------------
1 file changed, 19 insertions(+), 40 deletions(-)
diff --git a/drivers/interconnect/qcom/icc-rpm.c b/drivers/interconnect/qcom/icc-rpm.c
index 5733261d9407..3209d8de709b 100644
--- a/drivers/interconnect/qcom/icc-rpm.c
+++ b/drivers/interconnect/qcom/icc-rpm.c
@@ -292,58 +292,44 @@ static int qcom_icc_bw_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
}
/**
- * qcom_icc_bus_aggregate - aggregate bandwidth by traversing all nodes
+ * qcom_icc_bus_aggregate - calculate bus clock rates by traversing all nodes
* @provider: generic interconnect provider
- * @agg_avg: an array for aggregated average bandwidth of buckets
- * @agg_peak: an array for aggregated peak bandwidth of buckets
- * @max_agg_avg: pointer to max value of aggregated average bandwidth
+ * @agg_clk_rate: array containing the aggregated clock rates in kHz
*/
-static void qcom_icc_bus_aggregate(struct icc_provider *provider,
- u64 *agg_avg, u64 *agg_peak,
- u64 *max_agg_avg)
+static void qcom_icc_bus_aggregate(struct icc_provider *provider, u64 *agg_clk_rate)
{
- struct icc_node *node;
+ u64 agg_avg_rate, agg_rate;
struct qcom_icc_node *qn;
- u64 sum_avg[QCOM_SMD_RPM_STATE_NUM];
+ struct icc_node *node;
int i;
- /* Initialise aggregate values */
- for (i = 0; i < QCOM_SMD_RPM_STATE_NUM; i++) {
- agg_avg[i] = 0;
- agg_peak[i] = 0;
- }
-
- *max_agg_avg = 0;
-
/*
- * Iterate nodes on the interconnect and aggregate bandwidth
- * requests for every bucket.
+ * Iterate nodes on the provider, aggregate bandwidth requests for
+ * every bucket and convert them into bus clock rates.
*/
list_for_each_entry(node, &provider->nodes, node_list) {
qn = node->data;
for (i = 0; i < QCOM_SMD_RPM_STATE_NUM; i++) {
if (qn->channels)
- sum_avg[i] = div_u64(qn->sum_avg[i], qn->channels);
+ agg_avg_rate = div_u64(qn->sum_avg[i], qn->channels);
else
- sum_avg[i] = qn->sum_avg[i];
- agg_avg[i] += sum_avg[i];
- agg_peak[i] = max_t(u64, agg_peak[i], qn->max_peak[i]);
+ agg_avg_rate = qn->sum_avg[i];
+
+ agg_rate = max_t(u64, agg_avg_rate, qn->max_peak[i]);
+ do_div(agg_rate, qn->buswidth);
+
+ agg_clk_rate[i] = max_t(u64, agg_clk_rate[i], agg_rate);
}
}
-
- /* Find maximum values across all buckets */
- for (i = 0; i < QCOM_SMD_RPM_STATE_NUM; i++)
- *max_agg_avg = max_t(u64, *max_agg_avg, agg_avg[i]);
}
static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
{
- struct qcom_icc_provider *qp;
struct qcom_icc_node *src_qn = NULL, *dst_qn = NULL;
+ u64 agg_clk_rate[QCOM_SMD_RPM_STATE_NUM] = { 0 };
struct icc_provider *provider;
+ struct qcom_icc_provider *qp;
u64 active_rate, sleep_rate;
- u64 agg_avg[QCOM_SMD_RPM_STATE_NUM], agg_peak[QCOM_SMD_RPM_STATE_NUM];
- u64 max_agg_avg;
int ret;
src_qn = src->data;
@@ -352,7 +338,9 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
provider = src->provider;
qp = to_qcom_provider(provider);
- qcom_icc_bus_aggregate(provider, agg_avg, agg_peak, &max_agg_avg);
+ qcom_icc_bus_aggregate(provider, agg_clk_rate);
+ active_rate = agg_clk_rate[QCOM_SMD_RPM_ACTIVE_STATE];
+ sleep_rate = agg_clk_rate[QCOM_SMD_RPM_SLEEP_STATE];
ret = qcom_icc_rpm_set(src_qn, src_qn->sum_avg);
if (ret)
@@ -368,15 +356,6 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
if (!qp->bus_clk_desc && !qp->bus_clk)
return 0;
- /* Intentionally keep the rates in kHz as that's what RPM accepts */
- active_rate = max(agg_avg[QCOM_SMD_RPM_ACTIVE_STATE],
- agg_peak[QCOM_SMD_RPM_ACTIVE_STATE]);
- do_div(active_rate, src_qn->buswidth);
-
- sleep_rate = max(agg_avg[QCOM_SMD_RPM_SLEEP_STATE],
- agg_peak[QCOM_SMD_RPM_SLEEP_STATE]);
- do_div(sleep_rate, src_qn->buswidth);
-
/*
* Downstream checks whether the requested rate is zero, but it makes little sense
* to vote for a value that's below the lower threshold, so let's not do so.
--
2.41.0
Powered by blists - more mailing lists