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: <20241203173908.3148794-2-etienne.carriere@foss.st.com>
Date: Tue, 3 Dec 2024 18:39:07 +0100
From: Etienne Carriere <etienne.carriere@...s.st.com>
To: <linux-kernel@...r.kernel.org>
CC: Sudeep Holla <sudeep.holla@....com>,
        Cristian Marussi
	<cristian.marussi@....com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, <arm-scmi@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-clk@...r.kernel.org>,
        Etienne
 Carriere <etienne.carriere@...s.st.com>
Subject: [PATCH v2 1/2] firmware: arm_scmi: get only min/max clock rates

Remove limitation of 16 clock rates max for discrete clock rates
description when the SCMI firmware supports SCMI Clock protocol v2.0
or later.

Driver clk-scmi.c is only interested in the min and max clock rates.
Get these by querying the first and last discrete rates with SCMI
clock protocol message ID CLOCK_DESCRIBE_RATES since the SCMI
specification v2.0 and later states that rates enumerated by this
command are to be enumerated in "numeric ascending order" [1].

Preserve the implementation that queries all discrete rates (16 rates
max) to support SCMI firmware built on SCMI specification v1.0 [2]
where SCMI Clock protocol v1.0 does not explicitly require rates
described with CLOCK_DESCRIBE_RATES to be in ascending order.

Link: https://developer.arm.com/documentation/den0056 [1]
Link: https://developer.arm.com/documentation/den0056/a [2]
Signed-off-by: Etienne Carriere <etienne.carriere@...s.st.com>
---
Changes since patch series v1:
- Preserve the original implementation and keep using it for SCMI
  Clock protocol v1.0.

---
 drivers/clk/clk-scmi.c            |   4 +-
 drivers/firmware/arm_scmi/clock.c | 112 ++++++++++++++++++++++++++++--
 include/linux/scmi_protocol.h     |   4 +-
 3 files changed, 110 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/clk-scmi.c b/drivers/clk/clk-scmi.c
index 15510c2ff21c..09ccd6cea7f2 100644
--- a/drivers/clk/clk-scmi.c
+++ b/drivers/clk/clk-scmi.c
@@ -244,8 +244,8 @@ static int scmi_clk_ops_init(struct device *dev, struct scmi_clk *sclk,
 		if (num_rates <= 0)
 			return -EINVAL;
 
-		min_rate = sclk->info->list.rates[0];
-		max_rate = sclk->info->list.rates[num_rates - 1];
+		min_rate = sclk->info->list.min_rate;
+		max_rate = sclk->info->list.max_rate;
 	} else {
 		min_rate = sclk->info->range.min_rate;
 		max_rate = sclk->info->range.max_rate;
diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
index 2ed2279388f0..34fde0b88098 100644
--- a/drivers/firmware/arm_scmi/clock.c
+++ b/drivers/firmware/arm_scmi/clock.c
@@ -223,10 +223,21 @@ scmi_clock_protocol_attributes_get(const struct scmi_protocol_handle *ph,
 	return ret;
 }
 
+/*
+ * Support SCMI_CLOCK protocol v1.0 as described in SCMI specification v1.0
+ * that do not explicitly require clock rates described with command
+ * CLOCK_DESCRIBE_RATES to be in ascending order. The Linux legacy
+ * implementation for these clock supports a max of 16 rates.
+ * In SCMI specification v2.0 and later, rates must be in ascending order
+ * to we query only to min and max rates values.
+ */
+#define SCMI_MAX_NUM_RATES_V1		16
+
 struct scmi_clk_ipriv {
 	struct device *dev;
 	u32 clk_id;
 	struct scmi_clock_info *clk;
+	u64 rates[SCMI_MAX_NUM_RATES_V1];
 };
 
 static void iter_clk_possible_parents_prepare_message(void *message, unsigned int desc_index,
@@ -493,7 +504,7 @@ iter_clk_describe_process_response(const struct scmi_protocol_handle *ph,
 			break;
 		}
 	} else {
-		u64 *rate = &p->clk->list.rates[st->desc_index + st->loop_idx];
+		u64 *rate = &p->rates[st->desc_index + st->loop_idx];
 
 		*rate = RATE_TO_U64(r->rate[st->loop_idx]);
 		p->clk->list.num_rates++;
@@ -519,7 +530,7 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
 		.dev = ph->dev,
 	};
 
-	iter = ph->hops->iter_response_init(ph, &ops, SCMI_MAX_NUM_RATES,
+	iter = ph->hops->iter_response_init(ph, &ops, ARRAY_SIZE(cpriv.rates),
 					    CLOCK_DESCRIBE_RATES,
 					    sizeof(struct scmi_msg_clock_describe_rates),
 					    &cpriv);
@@ -535,10 +546,95 @@ scmi_clock_describe_rates_get(const struct scmi_protocol_handle *ph, u32 clk_id,
 			clk->range.min_rate, clk->range.max_rate,
 			clk->range.step_size);
 	} else if (clk->list.num_rates) {
-		sort(clk->list.rates, clk->list.num_rates,
-		     sizeof(clk->list.rates[0]), rate_cmp_func, NULL);
+		sort(cpriv.rates, clk->list.num_rates,
+		     sizeof(cpriv.rates[0]), rate_cmp_func, NULL);
+		clk->list.min_rate = cpriv.rates[0];
+		clk->list.max_rate = cpriv.rates[clk->list.num_rates - 1];
+	}
+
+	return ret;
+}
+
+static int scmi_clock_get_rates_bound(const struct scmi_protocol_handle *ph,
+				      u32 clk_id, struct scmi_clock_info *clk)
+{
+	struct scmi_msg_clock_describe_rates *msg;
+	const struct scmi_msg_resp_clock_describe_rates *resp;
+	unsigned int num_returned, num_remaining;
+	struct scmi_xfer *t;
+	int ret;
+
+	/* Get either the range triplet or the min rate & rates count */
+	ret = ph->xops->xfer_get_init(ph, CLOCK_DESCRIBE_RATES, sizeof(*msg), 0,
+				      &t);
+	if (ret)
+		return ret;
+
+	msg = t->tx.buf;
+	msg->id = cpu_to_le32(clk_id);
+	msg->rate_index = 0;
+
+	resp = t->rx.buf;
+
+	ret = ph->xops->do_xfer(ph, t);
+	if (ret)
+		goto out;
+
+	clk->rate_discrete = RATE_DISCRETE(resp->num_rates_flags);
+	num_returned = NUM_RETURNED(resp->num_rates_flags);
+	num_remaining = NUM_REMAINING(resp->num_rates_flags);
+
+	if (clk->rate_discrete) {
+		clk->list.num_rates = num_returned + num_remaining;
+		clk->list.min_rate = RATE_TO_U64(resp->rate[0]);
+
+		if (num_remaining) {
+			ph->xops->reset_rx_to_maxsz(ph, t);
+			msg->id = cpu_to_le32(clk_id);
+			msg->rate_index = cpu_to_le32(clk->list.num_rates - 1);
+			ret = ph->xops->do_xfer(ph, t);
+			if (!ret)
+				clk->list.max_rate = RATE_TO_U64(resp->rate[0]);
+		} else {
+			u64 max = RATE_TO_U64(resp->rate[num_returned - 1]);
+
+			clk->list.max_rate = max;
+		}
+	} else {
+		/* We expect a triplet, warn about out of spec replies ... */
+		if (num_returned != 3 || num_remaining != 0) {
+			dev_warn(ph->dev,
+				 "Out-of-spec CLOCK_DESCRIBE_RATES reply for %s - returned:%d remaining:%d rx_len:%zd\n",
+				 clk->name, num_returned, num_remaining,
+				 t->rx.len);
+
+			/*
+			 * A known quirk: a triplet is returned but
+			 * num_returned != 3, check for a safe payload
+			 * size to use returned info.
+			 */
+			if (num_remaining != 0 ||
+			    t->rx.len != sizeof(*resp) +
+					 sizeof(__le32) * 2 * 3) {
+				dev_err(ph->dev,
+					"Cannot fix out-of-spec reply !\n");
+				ret = -EPROTO;
+				goto out;
+			}
+		}
+
+		clk->range.min_rate = RATE_TO_U64(resp->rate[0]);
+		clk->range.max_rate = RATE_TO_U64(resp->rate[1]);
+		clk->range.step_size = RATE_TO_U64(resp->rate[2]);
+
+		dev_dbg(ph->dev, "Min %llu Max %llu Step %llu Hz\n",
+			clk->range.min_rate, clk->range.max_rate,
+			clk->range.step_size);
 	}
 
+out:
+	ph->xops->xfer_put(ph, t);
+
 	return ret;
 }
 
@@ -1089,8 +1185,12 @@ static int scmi_clock_protocol_init(const struct scmi_protocol_handle *ph)
 		struct scmi_clock_info *clk = cinfo->clk + clkid;
 
 		ret = scmi_clock_attributes_get(ph, clkid, cinfo, version);
-		if (!ret)
-			scmi_clock_describe_rates_get(ph, clkid, clk);
+		if (!ret) {
+			if (PROTOCOL_REV_MAJOR(version) >= 0x2)
+				scmi_clock_get_rates_bound(ph, clkid, clk);
+			else
+				scmi_clock_describe_rates_get(ph, clkid, clk);
+		}
 	}
 
 	if (PROTOCOL_REV_MAJOR(version) >= 0x3) {
diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h
index 688466a0e816..240478bb8476 100644
--- a/include/linux/scmi_protocol.h
+++ b/include/linux/scmi_protocol.h
@@ -15,7 +15,6 @@
 
 #define SCMI_MAX_STR_SIZE		64
 #define SCMI_SHORT_NAME_MAX_SIZE	16
-#define SCMI_MAX_NUM_RATES		16
 
 /**
  * struct scmi_revision_info - version information structure
@@ -54,7 +53,8 @@ struct scmi_clock_info {
 	union {
 		struct {
 			int num_rates;
-			u64 rates[SCMI_MAX_NUM_RATES];
+			u64 min_rate;
+			u64 max_rate;
 		} list;
 		struct {
 			u64 min_rate;
-- 
2.25.1


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ