[PATCH v1 1/2] riscv: don't read riscv, isa in the riscv cpu's get_desc()
Leo Liang
ycliang at andestech.com
Thu Mar 28 08:08:39 CET 2024
On Mon, Mar 18, 2024 at 03:16:02PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley at 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 at 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 at andestech.com>
More information about the U-Boot
mailing list