public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: u-boot@lists.denx.de
Cc: conor@kernel.org, Conor Dooley <conor.dooley@microchip.com>,
	Rick Chen <rick@andestech.com>, Leo <ycliang@andestech.com>,
	Tom Rini <trini@konsulko.com>,
	Heinrich Schuchardt <xypron.glpk@gmx.de>
Subject: [PATCH v1 1/2] riscv: don't read riscv, isa in the riscv cpu's get_desc()
Date: Mon, 18 Mar 2024 15:16:02 +0000	[thread overview]
Message-ID: <20240318151604.865025-3-conor@kernel.org> (raw)
In-Reply-To: <20240318151604.865025-2-conor@kernel.org>

From: Conor Dooley <conor.dooley@microchip.com>

cpu_get_desc() for the RISC-V CPU currently reads "riscv,isa" to get
the description, but it is no longer a required property and cannot be
assummed to always be present, as the new "riscv,isa-extensions" and
"riscv,isa-base" properties may be present instead.

On RISC-V, cpu_get_desc() has two main uses - firstly providing an
informational name for the CPU for smbios or at boot with
DISPLAY_CPUINFO etc and secondly it forms the basis of ISA extension
detection in supports_extension() as it returns (a portion of) an ISA
string.

cpu_get_desc() returns a string, which aligned with "riscv,isa" but
the new property is a list of strings. Rather than add support for
the list of strings property, which would require creating an isa
string from "riscv,isa-extensions", modify the RISC-V CPU's
implementaion of cpu_get_desc() return the first compatible as the
cpu description instead. This may be fine for the informational cases,
but it would break extension dtection, given supports_extension()
expects cpu_get_desc() to return an ISA string.

Call dev_read_string() directly in supports_extension() to get the
contents of "riscv,isa" so that extension detection remains functional.
As a knock-on affect of this change, extension detection is no longer
broken for long ISA strings. Previously if the ISA string exceeded the
32 element array that supports_extension() passed to cpu_get_desc(),
it would return ENOSPC and no extensions would be detected.
This bug probably had no impact as U-Boot does not currently do anything
meaningful with the results of supports_extension() and most SoCs
supported by U-Boot don't have anywhere near that complex of an ISA
string. The QEMU virt machine's CPUs do however, so extension detection
doesn't work there.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
I'm not really sure if I am happy with this patch - people could
definitely have got use out of the cpu info printout of the ISA string
before this patch - they'd have seen something like
CPU:	rv64imafdc
at boot, but now they will see
CPU:	sifive,u74
If it really is desired, cpu_get_desc() could be made to assemble
an isa string out of "riscv,isa-extensions", but I think it's always
gonna be a bit flawed, since that string can run to almost arbitrary
length now - one I saw for a CPU last week was 320 characters long
and these things are only going to grow.
---
 arch/riscv/cpu/cpu.c    | 12 +++++++-----
 drivers/cpu/riscv_cpu.c |  8 ++++----
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
index ecfefa1a02..99083e11df 100644
--- a/arch/riscv/cpu/cpu.c
+++ b/arch/riscv/cpu/cpu.c
@@ -39,7 +39,7 @@ static inline bool supports_extension(char ext)
 	return csr_read(CSR_MISA) & (1 << (ext - 'a'));
 #elif CONFIG_CPU
 	struct udevice *dev;
-	char desc[32];
+	const char *isa;
 	int i;
 
 	uclass_find_first_device(UCLASS_CPU, &dev);
@@ -47,12 +47,14 @@ static inline bool supports_extension(char ext)
 		debug("unable to find the RISC-V cpu device\n");
 		return false;
 	}
-	if (!cpu_get_desc(dev, desc, sizeof(desc))) {
+
+	isa = dev_read_string(dev, "riscv,isa");
+	if (isa) {
 		/*
 		 * skip the first 4 characters (rv32|rv64)
 		 */
-		for (i = 4; i < sizeof(desc); i++) {
-			switch (desc[i]) {
+		for (i = 4; i < sizeof(isa); i++) {
+			switch (isa[i]) {
 			case 's':
 			case 'x':
 			case 'z':
@@ -64,7 +66,7 @@ static inline bool supports_extension(char ext)
 				 */
 				return false;
 			default:
-				if (desc[i] == ext)
+				if (isa[i] == ext)
 					return true;
 			}
 		}
diff --git a/drivers/cpu/riscv_cpu.c b/drivers/cpu/riscv_cpu.c
index 5d1026b37d..9b1950efe0 100644
--- a/drivers/cpu/riscv_cpu.c
+++ b/drivers/cpu/riscv_cpu.c
@@ -21,13 +21,13 @@ DECLARE_GLOBAL_DATA_PTR;
 
 static int riscv_cpu_get_desc(const struct udevice *dev, char *buf, int size)
 {
-	const char *isa;
+	const char *cpu;
 
-	isa = dev_read_string(dev, "riscv,isa");
-	if (size < (strlen(isa) + 1))
+	cpu = dev_read_string(dev, "compatible");
+	if (size < (strlen(cpu) + 1))
 		return -ENOSPC;
 
-	strcpy(buf, isa);
+	strcpy(buf, cpu);
 
 	return 0;
 }
-- 
2.43.0


  reply	other threads:[~2024-03-18 15:17 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-18 15:16 [PATCH v1 0/2] Support new RISC-V ISA extension properties Conor Dooley
2024-03-18 15:16 ` Conor Dooley [this message]
2024-03-28  7:08   ` [PATCH v1 1/2] riscv: don't read riscv, isa in the riscv cpu's get_desc() Leo Liang
2024-03-18 15:16 ` [PATCH v1 2/2] riscv: support extension probing using riscv, isa-extensions Conor Dooley
2024-03-28  7:12   ` Leo Liang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240318151604.865025-3-conor@kernel.org \
    --to=conor@kernel.org \
    --cc=conor.dooley@microchip.com \
    --cc=rick@andestech.com \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=xypron.glpk@gmx.de \
    --cc=ycliang@andestech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox