linux-kernel - [PATCH v1 1/2] interconnect: qcom: icc-rpm: Fix for cached clock rate

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220416031029.693211-2-leo.yan@linaro.org>
Date:   Sat, 16 Apr 2022 11:10:28 +0800
From:   Leo Yan <leo.yan@...aro.org>
To:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Georgi Djakov <djakov@...nel.org>,
        Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Leo Yan <leo.yan@...aro.org>
Subject: [PATCH v1 1/2] interconnect: qcom: icc-rpm: Fix for cached clock rate

All nodes within an interconnect bus share interconnect bus clocks, but
every node has its own cached clock rate values, this can lead to
unexpected clock rate setting.

Let's see an example shown in below, in this case, a bus have two nodes
A and B, and its buswidth is 8:

  step1: vote bandwidth 1600M for node(A):

    aggregated(bw) = 1600M
    qcom_icc_node(A)->rate = 1600M / 8 = 200MHz

  step2: vote bandwidth 1600M for node(B):

    aggregated(bw) = 1600M + 1600M = 3200M
    qcom_icc_node(B)->rate = 3200M / 8 = 400MHz

  step3: unvote bandwidth 1600M for node(A)

    aggregated(bw) = 3200M - 1600M = 1600M
    target_clock = 1600M / 8 = 200MHz

The problem is in step 3, the calculated target clock rate is 200MHz,
which equals to the cached clock rate in node(A) (See step 1),
unfortunately, qcom_icc_set() skips to set the new clock rate 200MHz in
this case, so the bus clock rate will continue to stay at 400MHz.

To resolve the issue, one possible solution is to invoke clk_get_rate()
to retrieve the clock rates on the fly, thus we can totally remove the
cached clock rates.  But after review the code, many bus clock has set
the flag CLK_GET_RATE_NOCACHE, this results in the retrieving bus clock
rate is time cost for iterating parent clock nodes, and even challenges
bus clock drivers to provide recalc_rate() callbacks.

So this patch moves the cached rates into structure qcom_icc_provider,
we use it as a central place to maintain bus clock handlers and cached
clock rate, therefore, it can smoothly dismiss the mismatching problem.

Signed-off-by: Leo Yan <leo.yan@...aro.org>
---
 drivers/interconnect/qcom/icc-rpm.c | 4 ++--
 drivers/interconnect/qcom/icc-rpm.h | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/interconnect/qcom/icc-rpm.c b/drivers/interconnect/qcom/icc-rpm.c
index 34125e8f8b60..e0309e246523 100644
--- a/drivers/interconnect/qcom/icc-rpm.c
+++ b/drivers/interconnect/qcom/icc-rpm.c
@@ -274,7 +274,7 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
 	do_div(rate, qn->buswidth);
 	rate = min_t(u64, rate, LONG_MAX);
 
-	if (qn->rate == rate)
+	if (qp->bus_clk_rate == rate)
 		return 0;
 
 	for (i = 0; i < qp->num_clks; i++) {
@@ -286,7 +286,7 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
 		}
 	}
 
-	qn->rate = rate;
+	qp->bus_clk_rate = rate;
 
 	return 0;
 }
diff --git a/drivers/interconnect/qcom/icc-rpm.h b/drivers/interconnect/qcom/icc-rpm.h
index 26dad006034f..4457fcc5b84c 100644
--- a/drivers/interconnect/qcom/icc-rpm.h
+++ b/drivers/interconnect/qcom/icc-rpm.h
@@ -26,6 +26,7 @@ enum qcom_icc_type {
  * @type: the ICC provider type
  * @qos_offset: offset to QoS registers
  * @regmap: regmap for QoS registers read/write access
+ * @bus_clk_rate: bus clock rate in Hz
  */
 struct qcom_icc_provider {
 	struct icc_provider provider;
@@ -33,6 +34,7 @@ struct qcom_icc_provider {
 	enum qcom_icc_type type;
 	struct regmap *regmap;
 	unsigned int qos_offset;
+	u64 bus_clk_rate;
 	struct clk_bulk_data bus_clks[];
 };
 
@@ -66,7 +68,6 @@ struct qcom_icc_qos {
  * @mas_rpm_id:	RPM id for devices that are bus masters
  * @slv_rpm_id:	RPM id for devices that are bus slaves
  * @qos: NoC QoS setting parameters
- * @rate: current bus clock rate in Hz
  */
 struct qcom_icc_node {
 	unsigned char *name;
@@ -77,7 +78,6 @@ struct qcom_icc_node {
 	int mas_rpm_id;
 	int slv_rpm_id;
 	struct qcom_icc_qos qos;
-	u64 rate;
 };
 
 struct qcom_icc_desc {
-- 
2.25.1