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

[Charlie Jenkins <thecharlesjenkins@gmail.com>]

On Sat, Jun 27, 2026 at 02:58:14PM +0100, Conor Dooley wrote:
> 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?

Actually, I didn't test properly. OpenSBI doesn't crash and it does
parse "g" from the devicetree but it pulls from misa to print what
extensions are supported in the boot log.

> Do you mean that your intention is for a dts with "g" in it to only show
> the constituent parts in /proc/cpuinfo etc?

Yeah that's what I meant, I'll remove the opensbi wording. 

> 
> > 
> > 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";

Yeah... I did test "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.

Okay sounds good.

> 
> > 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()!

I should have done a better job reviewing myself. Yes you are right.
Since ACPI still uses "riscv,isa" I'm hesitant to say that we shouldn't
support it. This has always been a pain that we don't have a good
extension story for acpi. My preference would be to support it in
riscv,isa at least for ACPI. Maybe it's fine to not allow it for
non-ACPI by rejecting it in riscv_early_of_processor_hartid().

- Charlie

> 
> > 
> > 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
> > 



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv