Re: [kvmtool PATCH v2 09/10] riscv: Add cpu-type command-line option

[Andrew Jones <ajones@ventanamicro.com>]

On Thu, Apr 24, 2025 at 09:01:58PM +0530, Anup Patel wrote:
> Currently, the KVMTOOL always creates a VM with all available
> ISA extensions virtualized by the in-kernel KVM module.
> 
> For better flexibility, add cpu-type command-line option using
> which users can select one of the available CPU types for VM.
> 
> There are two CPU types supported at the moment namely "min"
> and "max". The "min" CPU type implies VCPU with rv[64|32]imafdc
> ISA whereas the "max" CPU type implies VCPU with all available
> ISA extensions.
> 
> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> ---
>  riscv/aia.c                         |  2 +-
>  riscv/fdt.c                         | 68 +++++++++++++++++++++++++----
>  riscv/include/kvm/kvm-arch.h        |  2 +
>  riscv/include/kvm/kvm-config-arch.h | 10 +++++
>  4 files changed, 73 insertions(+), 9 deletions(-)
> 
> diff --git a/riscv/aia.c b/riscv/aia.c
> index 21d9704..cad53d4 100644
> --- a/riscv/aia.c
> +++ b/riscv/aia.c
> @@ -209,7 +209,7 @@ void aia__create(struct kvm *kvm)
>  		.flags = 0,
>  	};
>  
> -	if (kvm->cfg.arch.ext_disabled[KVM_RISCV_ISA_EXT_SSAIA])
> +	if (riscv__isa_extension_disabled(kvm, KVM_RISCV_ISA_EXT_SSAIA))
>  		return;
>  
>  	err = ioctl(kvm->vm_fd, KVM_CREATE_DEVICE, &aia_device);
> diff --git a/riscv/fdt.c b/riscv/fdt.c
> index c741fd8..5d9b9bf 100644
> --- a/riscv/fdt.c
> +++ b/riscv/fdt.c
> @@ -13,16 +13,17 @@ struct isa_ext_info {
>  	const char *name;
>  	unsigned long ext_id;
>  	bool single_letter;
> +	bool min_enabled;
>  };
>  
>  struct isa_ext_info isa_info_arr[] = {
>  	/* single-letter ordered canonically as "IEMAFDQCLBJTPVNSUHKORWXYZG" */
> -	{"i",		KVM_RISCV_ISA_EXT_I,	.single_letter = true},
> -	{"m",		KVM_RISCV_ISA_EXT_M,	.single_letter = true},
> -	{"a",		KVM_RISCV_ISA_EXT_A,	.single_letter = true},
> -	{"f",		KVM_RISCV_ISA_EXT_F,	.single_letter = true},
> -	{"d",		KVM_RISCV_ISA_EXT_D,	.single_letter = true},
> -	{"c",		KVM_RISCV_ISA_EXT_C,	.single_letter = true},
> +	{"i",		KVM_RISCV_ISA_EXT_I,	.single_letter = true, .min_enabled = true},
> +	{"m",		KVM_RISCV_ISA_EXT_M,	.single_letter = true, .min_enabled = true},
> +	{"a",		KVM_RISCV_ISA_EXT_A,	.single_letter = true, .min_enabled = true},
> +	{"f",		KVM_RISCV_ISA_EXT_F,	.single_letter = true, .min_enabled = true},
> +	{"d",		KVM_RISCV_ISA_EXT_D,	.single_letter = true, .min_enabled = true},
> +	{"c",		KVM_RISCV_ISA_EXT_C,	.single_letter = true, .min_enabled = true},
>  	{"v",		KVM_RISCV_ISA_EXT_V,	.single_letter = true},
>  	{"h",		KVM_RISCV_ISA_EXT_H,	.single_letter = true},
>  	/* multi-letter sorted alphabetically */
> @@ -89,6 +90,56 @@ struct isa_ext_info isa_info_arr[] = {
>  	{"zvkt",	KVM_RISCV_ISA_EXT_ZVKT},
>  };
>  
> +static bool __isa_ext_disabled(struct kvm *kvm, struct isa_ext_info *info)
> +{
> +	if (kvm->cfg.arch.cpu_type == RISCV__CPU_TYPE_MIN &&
> +	    !info->min_enabled)
> +		return true;
> +
> +	return kvm->cfg.arch.ext_disabled[info->ext_id];
> +}
> +
> +static bool __isa_ext_warn_disable_failure(struct kvm *kvm, struct isa_ext_info *info)
> +{
> +	if (kvm->cfg.arch.cpu_type == RISCV__CPU_TYPE_MIN &&
> +	    !info->min_enabled)
> +		return false;
> +
> +	return true;
> +}
> +
> +bool riscv__isa_extension_disabled(struct kvm *kvm, unsigned long isa_ext_id)
> +{
> +	struct isa_ext_info *info = NULL;
> +	unsigned long i;
> +
> +	for (i = 0; i < ARRAY_SIZE(isa_info_arr); i++) {
> +		if (isa_info_arr[i].ext_id == isa_ext_id) {
> +			info = &isa_info_arr[i];
> +		break;

indentation is off here

> +		}
> +	}
> +	if (!info)
> +		return true;
> +
> +	return __isa_ext_disabled(kvm, info);
> +}
> +
> +int riscv__cpu_type_parser(const struct option *opt, const char *arg, int unset)
> +{
> +	struct kvm *kvm = opt->ptr;
> +
> +	if ((strncmp(arg, "min", 3) && strncmp(arg, "max", 3)) || strlen(arg) != 3)
> +		die("Invalid CPU type %s\n", arg);
> +
> +	if (!strcmp(arg, "max"))
> +		kvm->cfg.arch.cpu_type = RISCV__CPU_TYPE_MAX;
> +	else
> +		kvm->cfg.arch.cpu_type = RISCV__CPU_TYPE_MIN;
> +
> +	return 0;
> +}
> +
>  static void dump_fdt(const char *dtb_file, void *fdt)
>  {
>  	int count, fd;
> @@ -139,9 +190,10 @@ static void generate_cpu_nodes(void *fdt, struct kvm *kvm)
>  				/* This extension is not available in hardware */
>  				continue;
>  
> -			if (kvm->cfg.arch.ext_disabled[isa_info_arr[i].ext_id]) {
> +			if (__isa_ext_disabled(kvm, &isa_info_arr[i])) {
>  				isa_ext_out = 0;
> -				if (ioctl(vcpu->vcpu_fd, KVM_SET_ONE_REG, &reg) < 0)
> +				if (ioctl(vcpu->vcpu_fd, KVM_SET_ONE_REG, &reg) < 0 &&
> +				    __isa_ext_warn_disable_failure(kvm, &isa_info_arr[i]))
>  					pr_warning("Failed to disable %s ISA exension\n",
>  						   isa_info_arr[i].name);
>  				continue;
> diff --git a/riscv/include/kvm/kvm-arch.h b/riscv/include/kvm/kvm-arch.h
> index f0f469f..1bb2d32 100644
> --- a/riscv/include/kvm/kvm-arch.h
> +++ b/riscv/include/kvm/kvm-arch.h
> @@ -90,6 +90,8 @@ enum irqchip_type {
>  	IRQCHIP_AIA
>  };
>  
> +bool riscv__isa_extension_disabled(struct kvm *kvm, unsigned long ext_id);
> +
>  extern enum irqchip_type riscv_irqchip;
>  extern bool riscv_irqchip_inkernel;
>  extern void (*riscv_irqchip_trigger)(struct kvm *kvm, int irq,
> diff --git a/riscv/include/kvm/kvm-config-arch.h b/riscv/include/kvm/kvm-config-arch.h
> index 7e54d8a..6d9a29a 100644
> --- a/riscv/include/kvm/kvm-config-arch.h
> +++ b/riscv/include/kvm/kvm-config-arch.h
> @@ -3,7 +3,13 @@
>  
>  #include "kvm/parse-options.h"
>  
> +enum riscv__cpu_type {
> +	RISCV__CPU_TYPE_MAX = 0,
> +	RISCV__CPU_TYPE_MIN
> +};
> +
>  struct kvm_config_arch {
> +	enum riscv__cpu_type cpu_type;
>  	const char	*dump_dtb_filename;
>  	u64		suspend_seconds;
>  	u64		custom_mvendorid;
> @@ -13,8 +19,12 @@ struct kvm_config_arch {
>  	bool		sbi_ext_disabled[KVM_RISCV_SBI_EXT_MAX];
>  };
>  
> +int riscv__cpu_type_parser(const struct option *opt, const char *arg, int unset);
> +
>  #define OPT_ARCH_RUN(pfx, cfg)						\
>  	pfx,								\
> +	OPT_CALLBACK('\0', "cpu-type", NULL, "min or max",		\
> +		     "Choose the cpu type (default is max).", riscv__cpu_type_parser, kvm),\
>  	OPT_STRING('\0', "dump-dtb", &(cfg)->dump_dtb_filename,		\
>  		   ".dtb file", "Dump generated .dtb to specified file"),\
>  	OPT_U64('\0', "suspend-seconds",				\
> -- 
> 2.43.0
>

Other than the indentation issue,

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