All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.