On Fri, Apr 25, 2025 at 6:13 PM Andrew Jones <ajones@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxxxxxxxx>
Regards,
Anup
