Re: [kvmtool PATCH 08/10] riscv: Include single-letter extensions in isa_info_arr[]

[Andrew Jones <ajones@ventanamicro.com>]

On Wed, Mar 26, 2025 at 12:26:42PM +0530, Anup Patel wrote:
> Currently, the isa_info_arr[] only include multi-letter extensions but
> the KVM ONE_REG interface covers both single-letter and multi-letter
> extensions so extend isa_info_arr[] to include single-letter extensions.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  riscv/fdt.c | 138 +++++++++++++++++++++++++++++-----------------------
>  1 file changed, 76 insertions(+), 62 deletions(-)
> 
> diff --git a/riscv/fdt.c b/riscv/fdt.c
> index 251821e..46efb47 100644
> --- a/riscv/fdt.c
> +++ b/riscv/fdt.c
> @@ -12,71 +12,81 @@
>  struct isa_ext_info {
>  	const char *name;
>  	unsigned long ext_id;
> +	bool multi_letter;
>  };
>  
>  struct isa_ext_info isa_info_arr[] = {
> -	/* sorted alphabetically */
> -	{"smnpm", KVM_RISCV_ISA_EXT_SMNPM},
> -	{"smstateen", KVM_RISCV_ISA_EXT_SMSTATEEN},
> -	{"ssaia", KVM_RISCV_ISA_EXT_SSAIA},
> -	{"sscofpmf", KVM_RISCV_ISA_EXT_SSCOFPMF},
> -	{"ssnpm", KVM_RISCV_ISA_EXT_SSNPM},
> -	{"sstc", KVM_RISCV_ISA_EXT_SSTC},
> -	{"svade", KVM_RISCV_ISA_EXT_SVADE},
> -	{"svadu", KVM_RISCV_ISA_EXT_SVADU},
> -	{"svinval", KVM_RISCV_ISA_EXT_SVINVAL},
> -	{"svnapot", KVM_RISCV_ISA_EXT_SVNAPOT},
> -	{"svpbmt", KVM_RISCV_ISA_EXT_SVPBMT},
> -	{"svvptc", KVM_RISCV_ISA_EXT_SVVPTC},
> -	{"zabha", KVM_RISCV_ISA_EXT_ZABHA},
> -	{"zacas", KVM_RISCV_ISA_EXT_ZACAS},
> -	{"zawrs", KVM_RISCV_ISA_EXT_ZAWRS},
> -	{"zba", KVM_RISCV_ISA_EXT_ZBA},
> -	{"zbb", KVM_RISCV_ISA_EXT_ZBB},
> -	{"zbc", KVM_RISCV_ISA_EXT_ZBC},
> -	{"zbkb", KVM_RISCV_ISA_EXT_ZBKB},
> -	{"zbkc", KVM_RISCV_ISA_EXT_ZBKC},
> -	{"zbkx", KVM_RISCV_ISA_EXT_ZBKX},
> -	{"zbs", KVM_RISCV_ISA_EXT_ZBS},
> -	{"zca", KVM_RISCV_ISA_EXT_ZCA},
> -	{"zcb", KVM_RISCV_ISA_EXT_ZCB},
> -	{"zcd", KVM_RISCV_ISA_EXT_ZCD},
> -	{"zcf", KVM_RISCV_ISA_EXT_ZCF},
> -	{"zcmop", KVM_RISCV_ISA_EXT_ZCMOP},
> -	{"zfa", KVM_RISCV_ISA_EXT_ZFA},
> -	{"zfh", KVM_RISCV_ISA_EXT_ZFH},
> -	{"zfhmin", KVM_RISCV_ISA_EXT_ZFHMIN},
> -	{"zicbom", KVM_RISCV_ISA_EXT_ZICBOM},
> -	{"zicboz", KVM_RISCV_ISA_EXT_ZICBOZ},
> -	{"ziccrse", KVM_RISCV_ISA_EXT_ZICCRSE},
> -	{"zicntr", KVM_RISCV_ISA_EXT_ZICNTR},
> -	{"zicond", KVM_RISCV_ISA_EXT_ZICOND},
> -	{"zicsr", KVM_RISCV_ISA_EXT_ZICSR},
> -	{"zifencei", KVM_RISCV_ISA_EXT_ZIFENCEI},
> -	{"zihintntl", KVM_RISCV_ISA_EXT_ZIHINTNTL},
> -	{"zihintpause", KVM_RISCV_ISA_EXT_ZIHINTPAUSE},
> -	{"zihpm", KVM_RISCV_ISA_EXT_ZIHPM},
> -	{"zimop", KVM_RISCV_ISA_EXT_ZIMOP},
> -	{"zknd", KVM_RISCV_ISA_EXT_ZKND},
> -	{"zkne", KVM_RISCV_ISA_EXT_ZKNE},
> -	{"zknh", KVM_RISCV_ISA_EXT_ZKNH},
> -	{"zkr", KVM_RISCV_ISA_EXT_ZKR},
> -	{"zksed", KVM_RISCV_ISA_EXT_ZKSED},
> -	{"zksh", KVM_RISCV_ISA_EXT_ZKSH},
> -	{"zkt", KVM_RISCV_ISA_EXT_ZKT},
> -	{"ztso", KVM_RISCV_ISA_EXT_ZTSO},
> -	{"zvbb", KVM_RISCV_ISA_EXT_ZVBB},
> -	{"zvbc", KVM_RISCV_ISA_EXT_ZVBC},
> -	{"zvfh", KVM_RISCV_ISA_EXT_ZVFH},
> -	{"zvfhmin", KVM_RISCV_ISA_EXT_ZVFHMIN},
> -	{"zvkb", KVM_RISCV_ISA_EXT_ZVKB},
> -	{"zvkg", KVM_RISCV_ISA_EXT_ZVKG},
> -	{"zvkned", KVM_RISCV_ISA_EXT_ZVKNED},
> -	{"zvknha", KVM_RISCV_ISA_EXT_ZVKNHA},
> -	{"zvknhb", KVM_RISCV_ISA_EXT_ZVKNHB},
> -	{"zvksed", KVM_RISCV_ISA_EXT_ZVKSED},
> -	{"zvksh", KVM_RISCV_ISA_EXT_ZVKSH},
> -	{"zvkt", KVM_RISCV_ISA_EXT_ZVKT},
> +	/* single-letter */
> +	{"a", KVM_RISCV_ISA_EXT_A, false},
> +	{"c", KVM_RISCV_ISA_EXT_C, false},
> +	{"d", KVM_RISCV_ISA_EXT_D, false},
> +	{"f", KVM_RISCV_ISA_EXT_F, false},
> +	{"h", KVM_RISCV_ISA_EXT_H, false},
> +	{"i", KVM_RISCV_ISA_EXT_I, false},
> +	{"m", KVM_RISCV_ISA_EXT_M, false},
> +	{"v", KVM_RISCV_ISA_EXT_V, false},
> +	/* multi-letter sorted alphabetically */
> +	{"smnpm", KVM_RISCV_ISA_EXT_SMNPM, true},
> +	{"smstateen", KVM_RISCV_ISA_EXT_SMSTATEEN, true},
> +	{"ssaia", KVM_RISCV_ISA_EXT_SSAIA, true},
> +	{"sscofpmf", KVM_RISCV_ISA_EXT_SSCOFPMF, true},
> +	{"ssnpm", KVM_RISCV_ISA_EXT_SSNPM, true},
> +	{"sstc", KVM_RISCV_ISA_EXT_SSTC, true},
> +	{"svade", KVM_RISCV_ISA_EXT_SVADE, true},
> +	{"svadu", KVM_RISCV_ISA_EXT_SVADU, true},
> +	{"svinval", KVM_RISCV_ISA_EXT_SVINVAL, true},
> +	{"svnapot", KVM_RISCV_ISA_EXT_SVNAPOT, true},
> +	{"svpbmt", KVM_RISCV_ISA_EXT_SVPBMT, true},
> +	{"svvptc", KVM_RISCV_ISA_EXT_SVVPTC, true},
> +	{"zabha", KVM_RISCV_ISA_EXT_ZABHA, true},
> +	{"zacas", KVM_RISCV_ISA_EXT_ZACAS, true},
> +	{"zawrs", KVM_RISCV_ISA_EXT_ZAWRS, true},
> +	{"zba", KVM_RISCV_ISA_EXT_ZBA, true},
> +	{"zbb", KVM_RISCV_ISA_EXT_ZBB, true},
> +	{"zbc", KVM_RISCV_ISA_EXT_ZBC, true},
> +	{"zbkb", KVM_RISCV_ISA_EXT_ZBKB, true},
> +	{"zbkc", KVM_RISCV_ISA_EXT_ZBKC, true},
> +	{"zbkx", KVM_RISCV_ISA_EXT_ZBKX, true},
> +	{"zbs", KVM_RISCV_ISA_EXT_ZBS, true},
> +	{"zca", KVM_RISCV_ISA_EXT_ZCA, true},
> +	{"zcb", KVM_RISCV_ISA_EXT_ZCB, true},
> +	{"zcd", KVM_RISCV_ISA_EXT_ZCD, true},
> +	{"zcf", KVM_RISCV_ISA_EXT_ZCF, true},
> +	{"zcmop", KVM_RISCV_ISA_EXT_ZCMOP, true},
> +	{"zfa", KVM_RISCV_ISA_EXT_ZFA, true},
> +	{"zfh", KVM_RISCV_ISA_EXT_ZFH, true},
> +	{"zfhmin", KVM_RISCV_ISA_EXT_ZFHMIN, true},
> +	{"zicbom", KVM_RISCV_ISA_EXT_ZICBOM, true},
> +	{"zicboz", KVM_RISCV_ISA_EXT_ZICBOZ, true},
> +	{"ziccrse", KVM_RISCV_ISA_EXT_ZICCRSE, true},
> +	{"zicntr", KVM_RISCV_ISA_EXT_ZICNTR, true},
> +	{"zicond", KVM_RISCV_ISA_EXT_ZICOND, true},
> +	{"zicsr", KVM_RISCV_ISA_EXT_ZICSR, true},
> +	{"zifencei", KVM_RISCV_ISA_EXT_ZIFENCEI, true},
> +	{"zihintntl", KVM_RISCV_ISA_EXT_ZIHINTNTL, true},
> +	{"zihintpause", KVM_RISCV_ISA_EXT_ZIHINTPAUSE, true},
> +	{"zihpm", KVM_RISCV_ISA_EXT_ZIHPM, true},
> +	{"zimop", KVM_RISCV_ISA_EXT_ZIMOP, true},
> +	{"zknd", KVM_RISCV_ISA_EXT_ZKND, true},
> +	{"zkne", KVM_RISCV_ISA_EXT_ZKNE, true},
> +	{"zknh", KVM_RISCV_ISA_EXT_ZKNH, true},
> +	{"zkr", KVM_RISCV_ISA_EXT_ZKR, true},
> +	{"zksed", KVM_RISCV_ISA_EXT_ZKSED, true},
> +	{"zksh", KVM_RISCV_ISA_EXT_ZKSH, true},
> +	{"zkt", KVM_RISCV_ISA_EXT_ZKT, true},
> +	{"ztso", KVM_RISCV_ISA_EXT_ZTSO, true},
> +	{"zvbb", KVM_RISCV_ISA_EXT_ZVBB, true},
> +	{"zvbc", KVM_RISCV_ISA_EXT_ZVBC, true},
> +	{"zvfh", KVM_RISCV_ISA_EXT_ZVFH, true},
> +	{"zvfhmin", KVM_RISCV_ISA_EXT_ZVFHMIN, true},
> +	{"zvkb", KVM_RISCV_ISA_EXT_ZVKB, true},
> +	{"zvkg", KVM_RISCV_ISA_EXT_ZVKG, true},
> +	{"zvkned", KVM_RISCV_ISA_EXT_ZVKNED, true},
> +	{"zvknha", KVM_RISCV_ISA_EXT_ZVKNHA, true},
> +	{"zvknhb", KVM_RISCV_ISA_EXT_ZVKNHB, true},
> +	{"zvksed", KVM_RISCV_ISA_EXT_ZVKSED, true},
> +	{"zvksh", KVM_RISCV_ISA_EXT_ZVKSH, true},
> +	{"zvkt", KVM_RISCV_ISA_EXT_ZVKT, true},

nit: I think I would add a 'misa' boolean member instead of 'multi_letter'
and then rework this table to look like this

	{"a",		KVM_RISCV_ISA_EXT_A, .misa = true	},
	{"c",		KVM_RISCV_ISA_EXT_C, .misa = true	},
	{"d",		KVM_RISCV_ISA_EXT_D, .misa = true	},
	{"f",		KVM_RISCV_ISA_EXT_F, .misa = true	},
	{"h",		KVM_RISCV_ISA_EXT_H, .misa = true	},
	{"i",		KVM_RISCV_ISA_EXT_I, .misa = true	},
	{"m",		KVM_RISCV_ISA_EXT_M, .misa = true	},
	{"v",		KVM_RISCV_ISA_EXT_V, .misa = true	},

	/* multi-letter sorted alphabetically */
	{"smnpm",	KVM_RISCV_ISA_EXT_SMNPM,		},
	{"smstateen",	KVM_RISCV_ISA_EXT_SMSTATEEN,		},
	...

The benefit is that only the misa extensions need another field.

>  };
>  
>  static void dump_fdt(const char *dtb_file, void *fdt)
> @@ -129,6 +139,10 @@ static void generate_cpu_nodes(void *fdt, struct kvm *kvm)
>  		}
>  
>  		for (i = 0; i < arr_sz; i++) {
> +			/* Skip single-letter extensions since these are taken care */

We should finish the comment with 'of by "whatever takes care of them"'

> +			if (!isa_info_arr[i].multi_letter)
> +				continue;
> +
>  			reg.id = RISCV_ISA_EXT_REG(isa_info_arr[i].ext_id);
>  			reg.addr = (unsigned long)&isa_ext_out;
>  			if (ioctl(vcpu->vcpu_fd, KVM_GET_ONE_REG, &reg) < 0)
> -- 
> 2.43.0
>

Thanks,
drew

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