Message ID | 20240318151604.865025-3-conor@kernel.org |
---|---|
State | Accepted |
Commit | b90edde70127a824d7aa257c6a633c1a030bfb79 |
Delegated to: | Andes |
Headers | show |
Series | Support new RISC-V ISA extension properties | expand |
On Mon, Mar 18, 2024 at 03:16:02PM +0000, Conor Dooley wrote: > 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(-) LGTM! reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
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; }