Re: [PATCH] riscv: Add "g" as an instruction alias

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4777 bytes --]

On Fri, Jun 26, 2026 at 11:24:46PM -0700, Charlie Jenkins wrote:
> "G" is an official alias for "IMAFDZicsr_Zifencei" [1]. Many common

What does [1] reference?

> tools like LLVM, GCC, OpenSBI, QEMU support this alias so make Linux
> follow the status quo and allow users to pass it in the isa string. In
> the ISA string, "G" is expected to written lowercase as "g" like the
> other extensions. Since "g" is a simple alias, follow what OpenSBI does
> and expose "imafd_zicsr_zifencei" instead of "g".

What does OpenSBI do? Does it covert "g" in Kconfig etc into imafd... in
a devicetree?
Do you mean that your intention is for a dts with "g" in it to only show
the constituent parts in /proc/cpuinfo etc?

> 
> Signed-off-by: Charlie Jenkins <thecharlesjenkins@gmail.com>
> ---
> This can be tested with a device tree that passes in "g" to the isa
> string like:
> 
> riscv,isa-extensions = "gc";

Are you sure this is what you tried? It should not work.
The code dealing with this works on exact matches, so it should end up
comparing "gc" with "g" and "c", detecting neither.
Needs to be ... = "g", "c";

Also, you need to document this in the dt binding, along with the exact
meaning. Probably do what b has done and mandate filling in the
constituent parts, retrofitting something that permits the removal of
other extensions will create devicetrees that do not play nicely with
other OSes.

> or
> 
> riscv,isa = "rv64gc";

And I think this definitely does not work properly, cos you only
modified the riscv,isa-extensions part of
riscv_early_of_processor_hartid(). I'm not convinced that this is worth
permitting at all though, riscv,isa should probably not get any
behavioural changes at this point. It's deprecated after all.
Sure ACPI uses the same format and parser etc, but that's obviously not
affected by what we do in riscv_early_of_processor_hartid()!

> 
> Example test case using qemu:
> 	1. Run QEMU with the additional arg "-machine dumpdtb=qemu.dtb"
> 	2. Decompile the dts "dtc -O dts -I dtb qemu.dtb -o qemu.dts"
> 	3. Set riscv,isa-extensions to "gc"
> 	4. Compile the dtb "dtc -O dtb -I dts qemu.dts -o qemu.dtb"
> 	5. Boot qemu with "-dtc qemu.dtb"
> 	6. Look at /proc/cpuinfo
> ---
>  arch/riscv/kernel/cpu.c        |  7 ++++---
>  arch/riscv/kernel/cpufeature.c | 11 +++++++++++
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 3dbc8cc557dd..0a2df97a1fd6 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -81,9 +81,10 @@ int __init riscv_early_of_processor_hartid(struct device_node *node, unsigned lo
>  	if (!of_property_present(node, "riscv,isa-extensions"))
>  		return -ENODEV;
>  
> -	if (of_property_match_string(node, "riscv,isa-extensions", "i") < 0 ||
> -	    of_property_match_string(node, "riscv,isa-extensions", "m") < 0 ||
> -	    of_property_match_string(node, "riscv,isa-extensions", "a") < 0) {
> +	if (of_property_match_string(node, "riscv,isa-extensions", "g") < 0 &&
> +	    (of_property_match_string(node, "riscv,isa-extensions", "i") < 0 ||
> +	     of_property_match_string(node, "riscv,isa-extensions", "m") < 0 ||
> +	     of_property_match_string(node, "riscv,isa-extensions", "a") < 0)) {
>  		pr_warn("CPU with hartid=%lu does not support ima", *hart);
>  		return -ENODEV;
>  	}
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index f46aa5602d74..f78cbf5ade1e 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -322,6 +322,16 @@ static const unsigned int riscv_a_exts[] = {
>  	RISCV_ISA_EXT_ZALRSC,
>  };
>  
> +static const unsigned int riscv_g_bundled_exts[] = {
> +	RISCV_ISA_EXT_i,
> +	RISCV_ISA_EXT_m,
> +	RISCV_ISA_EXT_a,
> +	RISCV_ISA_EXT_f,
> +	RISCV_ISA_EXT_d,
> +	RISCV_ISA_EXT_ZICSR,
> +	RISCV_ISA_EXT_ZIFENCEI
> +};
> +
>  #define RISCV_ISA_EXT_ZKN	\
>  	RISCV_ISA_EXT_ZBKB,	\
>  	RISCV_ISA_EXT_ZBKC,	\
> @@ -495,6 +505,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>  	__RISCV_ISA_EXT_SUPERSET(a, RISCV_ISA_EXT_a, riscv_a_exts),
>  	__RISCV_ISA_EXT_DATA_VALIDATE(f, RISCV_ISA_EXT_f, riscv_ext_f_validate),
>  	__RISCV_ISA_EXT_DATA_VALIDATE(d, RISCV_ISA_EXT_d, riscv_ext_d_validate),
> +	__RISCV_ISA_EXT_BUNDLE(g, riscv_g_bundled_exts),
>  	__RISCV_ISA_EXT_DATA(q, RISCV_ISA_EXT_q),
>  	__RISCV_ISA_EXT_SUPERSET(c, RISCV_ISA_EXT_c, riscv_c_exts),
>  	__RISCV_ISA_EXT_SUPERSET_VALIDATE(v, RISCV_ISA_EXT_v, riscv_v_exts, riscv_ext_vector_float_validate),
> 
> ---
> base-commit: 5a66900afbd6b2a063eebad35294038a654de2b0
> change-id: 20260626-g_ext-0ca7d6223ff8
> 
> Best regards,
> --  
> - Charlie
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]