[v1,1/9] RISC-V: don't parse dt/acpi isa string to get rv32/rv64

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

When filling hwcap the kernel already expects the isa string to start with
rv32 if CONFIG_32BIT and rv64 if CONFIG_64BIT.

So when recreating the runtime isa-string we can also just go the other way
to get the correct starting point for it.

Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
Co-developed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/kernel/cpu.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

On Mon, Jun 26, 2023 at 12:19:39PM +0100, Conor Dooley wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> When filling hwcap the kernel already expects the isa string to start with
> rv32 if CONFIG_32BIT and rv64 if CONFIG_64BIT.
> 
> So when recreating the runtime isa-string we can also just go the other way
> to get the correct starting point for it.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> Co-developed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  arch/riscv/kernel/cpu.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index a2fc952318e9..742bb42e7e86 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -253,13 +253,16 @@ static void print_isa_ext(struct seq_file *f)
>   */
>  static const char base_riscv_exts[13] = "imafdqcbkjpvh";
>  
> -static void print_isa(struct seq_file *f, const char *isa)
> +static void print_isa(struct seq_file *f)
>  {
>  	int i;
>  
>  	seq_puts(f, "isa\t\t: ");
> -	/* Print the rv[64/32] part */
> -	seq_write(f, isa, 4);
> +	if (IS_ENABLED(CONFIG_32BIT))
> +		seq_write(f, "rv32", 4);
> +	else
> +		seq_write(f, "rv64", 4);
> +
>  	for (i = 0; i < sizeof(base_riscv_exts); i++) {
>  		if (__riscv_isa_extension_available(NULL, base_riscv_exts[i] - 'a'))
>  			/* Print only enabled the base ISA extensions */
> @@ -316,15 +319,14 @@ static int c_show(struct seq_file *m, void *v)
>  	unsigned long cpu_id = (unsigned long)v - 1;
>  	struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
>  	struct device_node *node;
> -	const char *compat, *isa;
> +	const char *compat;
>  
>  	seq_printf(m, "processor\t: %lu\n", cpu_id);
>  	seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id));
> +	print_isa(m);
>  
>  	if (acpi_disabled) {
>  		node = of_get_cpu_node(cpu_id, NULL);
> -		if (!of_property_read_string(node, "riscv,isa", &isa))
> -			print_isa(m, isa);
>  
>  		print_mmu(m);
>  		if (!of_property_read_string(node, "compatible", &compat) &&
> @@ -333,8 +335,6 @@ static int c_show(struct seq_file *m, void *v)
>  
>  		of_node_put(node);
>  	} else {
> -		if (!acpi_get_riscv_isa(NULL, cpu_id, &isa))
> -			print_isa(m, isa);
>  

Extra blank line here to remove. Actually the whole 'else' can be removed
because the print_mmu() call can be brought up above the
'if (acpi_disabled)'

>  		print_mmu(m);
>  	}
> -- 
> 2.40.1
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew

On Mon, Jun 26, 2023 at 05:16:09PM +0100, Conor Dooley wrote:
> On Mon, Jun 26, 2023 at 06:05:40PM +0200, Andrew Jones wrote:
> > On Mon, Jun 26, 2023 at 04:51:29PM +0100, Conor Dooley wrote:
> > > On Mon, Jun 26, 2023 at 05:14:15PM +0200, Andrew Jones wrote:
> > > > On Mon, Jun 26, 2023 at 12:19:39PM +0100, Conor Dooley wrote:
> > > > > From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > > > > @@ -333,8 +335,6 @@ static int c_show(struct seq_file *m, void *v)
> > > > >  
> > > > >  		of_node_put(node);
> > > > >  	} else {
> > > > > -		if (!acpi_get_riscv_isa(NULL, cpu_id, &isa))
> > > > > -			print_isa(m, isa);
> > > > >  
> > > > 
> > > > Extra blank line here to remove. Actually the whole 'else' can be removed
> > > > because the print_mmu() call can be brought up above the
> > > > 'if (acpi_disabled)'
> > > 
> > > Can it be? I intentionally did not make that change - wasn't sure
> > > whether re-ordering the fields in there was permissible.
> > 
> > I agree we shouldn't change the order, but moving print_mmu() up won't,
> > afaict.
> 
> D'oh, I'm an eejit. Sure, I'll do that for v2. Thanks!
> 
> > > One of the few things I know does parsing of /proc/cpuinfo is:
> > > https://github.com/google/cpu_features/blob/main/src/impl_riscv_linux.c
> > > and that doesn't seem to care about the mmu, but does rely on
> > > vendor/uarch ordering.
> > > 
> > > Makes me wonder, does ACPI break things by leaving out uarch/vendor
> > > fields, if there is something that expects them to exist? We should
> > > not intentionally break stuff in /proc/cpuinfo, but can't say I feel any
> > > sympathy for naively parsing it.
> > 
> > Yes, it would be nice for ACPI to be consistent. I'm not sure what can be
> > done about that.
> 
> Print "unknown", until there's a way of passing the info?
> Speaking of being an eejit, adding new fields to the file would probably
> break some really naive parsers & quite frankly that sort of thing can
> keep the pieces IMO. Ditto if adding more extensions breaks someone that
> expects _zicbom_zicboz that breaks when _zicbop is slid into the middle.

Hi Conor,

Instead of unknown, could you print "risc-v" or "riscv"? There is nothing
equivalent to compatible property in ACPI. Using mvendorid,
marchid and mimpid, people can determine the exact processor
<manufacturer>,<model>.

Thanks!
Sunil