[kvmtool PATCH v2 10/10] riscv: Allow including extensions in the min CPU type using command-line

[Anup Patel]

On Fri, Apr 25, 2025 at 6:13 PM Andrew Jones <ajones at ventanamicro.com> wrote:
>
> On Thu, Apr 24, 2025 at 09:01:59PM +0530, Anup Patel wrote:
> > It is useful to allow including extensions in the min CPU type on need
> > basis via command-line. To achieve this, parse extension names as comma
> > separated values appended to the "min" CPU type using command-line.
> >
> > For example, to include Sstc and Ssaia in the min CPU type use
> > "--cpu-type min,sstc,ssaia" command-line option.
> >
> > Signed-off-by: Anup Patel <apatel at ventanamicro.com>
> > ---
> >  riscv/fdt.c | 43 ++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 40 insertions(+), 3 deletions(-)
> >
> > diff --git a/riscv/fdt.c b/riscv/fdt.c
> > index 5d9b9bf..722493a 100644
> > --- a/riscv/fdt.c
> > +++ b/riscv/fdt.c
> > @@ -108,6 +108,20 @@ static bool __isa_ext_warn_disable_failure(struct kvm *kvm, struct isa_ext_info
> >       return true;
> >  }
> >
> > +static void __min_enable(const char *ext, size_t ext_len)
> > +{
> > +     struct isa_ext_info *info;
> > +     unsigned long i;
> > +
> > +     for (i = 0; i < ARRAY_SIZE(isa_info_arr); i++) {
> > +             info = &isa_info_arr[i];
> > +             if (strlen(info->name) != ext_len)
> > +                     continue;
> > +             if (!strncmp(ext, info->name, ext_len))
>
> Just strcmp since we already checked the length and wouldn't want
> something like info->name = mmmmnnnn to match ext = mmmm anyway.

(Like mentioned in my comment in the past revision ...)

The "ext" is not simply an extension name rather something
like "mmmm,xyz,abc" (suffix of comma separated string) so
a plain strcmp() will fail even if extension length is already
checked.

>
> > +                     info->min_enabled = true;
> > +     }
> > +}
> > +
> >  bool riscv__isa_extension_disabled(struct kvm *kvm, unsigned long isa_ext_id)
> >  {
> >       struct isa_ext_info *info = NULL;
> > @@ -128,15 +142,38 @@ bool riscv__isa_extension_disabled(struct kvm *kvm, unsigned long isa_ext_id)
> >  int riscv__cpu_type_parser(const struct option *opt, const char *arg, int unset)
> >  {
> >       struct kvm *kvm = opt->ptr;
> > +     const char *str, *nstr;
> > +     int len;
> >
> > -     if ((strncmp(arg, "min", 3) && strncmp(arg, "max", 3)) || strlen(arg) != 3)
> > +     if ((strncmp(arg, "min", 3) || strlen(arg) < 3) &&
>
> The strlen(arg) < 3 will never be true because if strncmp(arg, "min", 3)
> returns 0 (false), then that means arg starts with 'min', and therefore
> can't have a length less than 3. And, if strncmp(arg, "min", 3) returns
> nonzero, then the '|| expression' will be short-circuited.
>
> > +         (strncmp(arg, "max", 3) || strlen(arg) != 3))
>
> So
>
>  if (strncmp(arg, "min", 3) && (strncmp(arg, "max", 3) || strlen(arg) != 3))
>
> should do it.

Okay, I will update.

>
> >               die("Invalid CPU type %s\n", arg);
> >
> > -     if (!strcmp(arg, "max"))
> > +     if (!strcmp(arg, "max")) {
> >               kvm->cfg.arch.cpu_type = RISCV__CPU_TYPE_MAX;
> > -     else
> > +     } else {
> >               kvm->cfg.arch.cpu_type = RISCV__CPU_TYPE_MIN;
> >
> > +             str = arg;
> > +             str += 3;
> > +             while (*str) {
> > +                     if (*str == ',') {
> > +                             str++;
> > +                             continue;
> > +                     }
> > +
> > +                     nstr = strchr(str, ',');
> > +                     if (!nstr)
> > +                             nstr = str + strlen(str);
> > +
> > +                     len = nstr - str;
> > +                     if (len) {
> > +                             __min_enable(str, len);
> > +                             str += len;
> > +                     }
> > +             }
> > +     }
> > +
> >       return 0;
> >  }
> >
> > --
> > 2.43.0
> >
>
> Otherwise,
>
> Reviewed-by: Andrew Jones <ajones at ventanamicro.com>

Regards,
Anup