Index | Thread | Search

From:
Nick Owens <mischief@offblast.org>
Subject:
Re: riscv: Parse and classify the ISA extensions from ISA extension string from FDT
To:
Mike Larkin <mlarkin@nested.page>
Cc:
Doongar Singh <doonbsd@gmail.com>, tech@openbsd.org
Date:
Wed, 4 Mar 2026 23:27:40 -0800

Download raw body.

Thread
On Wed, Mar 4, 2026 at 11:13 PM Mike Larkin <mlarkin@nested.page> wrote:
>
> On Wed, Feb 25, 2026 at 11:26:44PM +0530, Doongar Singh wrote:
> > Hi,
> >
> > For RISC-V, currently the ISA string buffer is just 32 bytes and the
> > ISA string is north of 300 bytes. As a result, junk characters are
> > printed during boot and in sysctl hw.model.
> >
> > The ISA string contains information about BASE ISA, User level Extension
> > and Supervisor Level Extensions. This patch series parses the long ISA
> > string and separates out BASE ISA (also expands it), User extensions,
> > and Suervisor Level extensions.
> >
> > "cpu_model" only contains the BASE ISA.
> > Two new machdeps - machdep.uextentions and machdep.sextensions print
> > the extension available in user level and supervisor level respectively.
> >
> > During boot following is the new output (Changed to fit in 72 columns):
> >
> > Base ISA: rv64imafdch (Mul/Div, Atomic, FPU (Single), FPU (Double),
> >       Compressed, Hypervisor)
> > Standard Userlevel Extensions: zic64b, zicbom, zicbop, zicboz, ziccamoa,
> >       ziccif, zicclsm, ziccrse, zicntr, zicsr, zifencei, zihintntl,
> >       zihintpause, zihpm, zmmul, za64rs, zaamo, zalrsc, zawrs, zfa,
> >       zca, zcd, zba, zbb, zbc, zbs
> > Standard Supervisor Level  Extensions: sdtrig, shcounterenw, shgatpa,
> >       shtvala, shvsatpa, shvstvala, shvstvecd, ssccptr, sscounterenw,
> >       ssstrict, sstc, sstvala, sstvecd, ssu64xl, svadu, svvptc
> >
> > Following is the output of the systcl machdep (changed to fit 72 columns)
> > machdep.compatible=riscv-virtio
> > machdep.uextensions=zic64b, zicbom, zicbop, zicboz, ziccamoa, ziccif,
> >       zicclsm, ziccrse, zicntr, zicsr, zifencei, zihintntl, zihintpause,
> >       zihpm, zmmul, za64rs, zaamo, zalrsc, zawrs, zfa, zca, zcd, zba,
> >       zbb, zbc, zbs
> > machdep.sextensions=sdtrig, shcounterenw, shgatpa, shtvala, shvsatpa,
> >       shvstvala, shvstvecd, ssccptr, sscounterenw, ssstrict, sstc,
> >       sstvala, sstvecd, ssu64xl, svadu, svvptc
> >
> > Does this look ok?
> >
> > /DS
>
> way too verbose. Also the sysctls aren't really needed.
>
> I suggest we just increase the size of the buffer to 512 and call it a day.
> Can be easily bumped later if need be.

see also previous thread at
https://marc.info/?l=openbsd-tech&m=175017594918295&w=2 ...

>
> -ml
>
> > diff --git sys/arch/riscv64/include/cpu.h sys/arch/riscv64/include/cpu.h
> > index 2b14083c31e..be0da2e3e70 100644
> > --- sys/arch/riscv64/include/cpu.h
> > +++ sys/arch/riscv64/include/cpu.h
> > @@ -26,11 +26,15 @@
> >
> >  /*  CTL_MACHDEP definitions. */
> >  #define      CPU_COMPATIBLE          1       /* compatible property */
> > -#define      CPU_MAXID               2       /* number of valid machdep ids */
> > +#define      CPU_STD_USER_EXT        2       /* Standard User Level  Extensions */
> > +#define      CPU_STD_SUPER_EXT       3       /* Standard Supervisor Extensions */
> > +#define      CPU_MAXID               4       /* number of valid machdep ids */
> >
> >  #define      CTL_MACHDEP_NAMES { \
> >       { 0, 0 }, \
> >       { "compatible", CTLTYPE_STRING }, \
> > +     { "uextensions", CTLTYPE_STRING }, \
> > +     { "sextensions", CTLTYPE_STRING }, \
> >  }
> >
> >  #ifdef _KERNEL
> > diff --git sys/arch/riscv64/riscv64/cpu.c sys/arch/riscv64/riscv64/cpu.c
> > index e8400d4eed3..37ee1c3b6b0 100644
> > --- sys/arch/riscv64/riscv64/cpu.c
> > +++ sys/arch/riscv64/riscv64/cpu.c
> > @@ -73,7 +73,6 @@ const struct vendor {
> >       { 0, NULL }
> >  };
> >
> > -char cpu_model[64];
> >  int cpu_node;
> >
> >  struct cpu_info *cpu_info_list = &cpu_info_primary;
> > @@ -99,16 +98,191 @@ void     thead_dcache_wb_range(paddr_t, psize_t);
> >
> >  size_t       thead_dcache_line_size;
> >
> > +char cpu_model[64];
> > +char expanded_base_isa[256];
> > +char cpu_user_ext[256];
> > +char cpu_super_ext[256];
> > +
> > +#define      TOKEN_MAX_SZ            64
> > +#define      PROCESS_BASE_ISA        0
> > +#define      PROCESS_ADD_EXT         1
> > +#define      PROCESS_S_MOD_EXT       2
> > +
> > +int tokenize(int start, char *str, char delim,
> > +    void (*token_cb)(char *, char *, char *), char *add_ext, char *s_ext)
> > +{
> > +     int i = start;
> > +     int len = strlen(str);
> > +     char token[TOKEN_MAX_SZ];
> > +     int token_sz;
> > +
> > +     if (start >= len)
> > +             return 0;
> > +
> > +     if (str[i] == '\0')
> > +             return 0;
> > +
> > +     while (i < len && (str[i] != delim && str[i] != '\0'))
> > +             i++;
> > +
> > +     if (str[i] == delim || str[i] == '\0') {
> > +             token_sz = i - start;
> > +             if (token_sz >= TOKEN_MAX_SZ) {
> > +                     return ENOSPC;
> > +             }
> > +
> > +             if (token_sz < TOKEN_MAX_SZ) {
> > +                     memcpy(token, (str + start), token_sz);
> > +                     token[token_sz] = '\0';
> > +             }
> > +
> > +             token_cb(token, add_ext, s_ext);
> > +     }
> > +
> > +     if (++i >= len)
> > +             return 0;
> > +
> > +     return i;
> > +}
> > +
> > +char *expand_baseisa(char *baisa, char *exp_isa, size_t exp_sz)
> > +{
> > +     int i = 0;
> > +     int len = strlen(baisa);
> > +     char *fmt;
> > +     int xlen;
> > +     int clen = 0;
> > +     int fin; /* Byte position in exp_isa. To check overflow */
> > +
> > +     for (i = 0; i <= len; i++) {
> > +             if (baisa[i] == 'i')
> > +                     break;
> > +     }
> > +
> > +     for (++i; i <= len; i++) {
> > +             if (clen == 0) {
> > +                     fmt = "%s";
> > +             } else {
> > +                     fmt = ", %s";
> > +             }
> > +
> > +             switch (baisa[i]) {
> > +             case 'm':
> > +                     xlen = strlen("Mul/Div");
> > +                     fin = clen + xlen + 2;
> > +                     /* enough space in buffer to print? */
> > +                     if (fin > (exp_sz - clen))
> > +                             return NULL;
> > +                     snprintf(exp_isa + clen, fin, fmt, "Mul/Div");
> > +                     break;
> > +
> > +             case 'a':
> > +                     xlen = strlen("Atomic");
> > +                     fin = clen + xlen + 2;
> > +                     /* enough space in buffer to print? */
> > +                     if (fin > (exp_sz - clen))
> > +                             return NULL;
> > +                     snprintf(exp_isa + clen, fin, fmt, "Atomic");
> > +                     break;
> > +
> > +             case 'f':
> > +                     xlen = strlen("FPU (Single)");
> > +                     fin = clen + xlen + 2;
> > +                     /* enough space in bufer to print? */
> > +                     if (fin > (exp_sz - clen))
> > +                             return NULL;
> > +                     snprintf(exp_isa + clen, fin, fmt, "FPU (Single)");
> > +                     break;
> > +
> > +             case 'd':
> > +                     xlen = strlen("FPU (Double)");
> > +                     fin = clen + xlen + 2;
> > +                     /* enough space in bufer to print? */
> > +                     if (fin > (exp_sz - clen))
> > +                             return NULL;
> > +                     snprintf(exp_isa + clen, fin, fmt, "FPU (Double)");
> > +                     break;
> > +
> > +             case 'c':
> > +                     xlen = strlen("Compressed");
> > +                     fin = clen + xlen + 2;
> > +                     /* enough space in bufer to print? */
> > +                     if (fin > (exp_sz - clen))
> > +                             return NULL;
> > +                     snprintf(exp_isa + clen, fin, fmt, "Compressed");
> > +                     break;
> > +
> > +             case 'h':
> > +                     xlen = strlen("Hypervisor");
> > +                     fin = clen + xlen + 2;
> > +                     /* enough space in bufer to print? */
> > +                     if (fin > (exp_sz - clen))
> > +                             return NULL;
> > +                     snprintf(exp_isa + clen, fin, fmt, "Hypervisor");
> > +                     break;
> > +             }
> > +
> > +             clen = strlen(exp_isa);
> > +     }
> > +
> > +     return exp_isa;
> > +}
> > +
> > +void create_isa_extension_list(char *ext, char *add_ext, char *s_ext)
> > +{
> > +     int state = PROCESS_BASE_ISA;
> > +     int curlen, xlen, nlen;
> > +     char *fmt;
> > +     int tocopy;
> > +
> > +     if (ext[0] == 'z')
> > +             state = PROCESS_ADD_EXT;
> > +     else if (ext[0] == 's')
> > +             state = PROCESS_S_MOD_EXT;
> > +
> > +     switch (state) {
> > +     case PROCESS_BASE_ISA:
> > +             tocopy = (strlen(ext) >= sizeof(cpu_model)
> > +                 ? sizeof(cpu_model) - 1 : strlen(ext));
> > +             strncpy(cpu_model, ext, tocopy);
> > +             cpu_model[tocopy] = '\0';
> > +             break;
> > +
> > +     case PROCESS_ADD_EXT:
> > +             if (add_ext[0] == '\0')
> > +                     curlen = 0;
> > +             else {
> > +                     curlen = strlen(add_ext);
> > +             }
> > +             xlen = strlen(ext) + 3;
> > +             nlen = curlen + xlen;
> > +             fmt = (curlen == 0 ? "%s" : ", %s");
> > +             snprintf(add_ext + curlen, xlen, fmt, ext);
> > +             break;
> > +
> > +     case PROCESS_S_MOD_EXT:
> > +             if (s_ext[0] == '\0')
> > +                     curlen = 0;
> > +             else
> > +                     curlen = strlen(s_ext);
> > +             xlen = strlen(ext) + 3;
> > +             nlen = curlen + xlen;
> > +             fmt = (curlen == 0 ? "%s" : ", %s");
> > +             snprintf(s_ext + curlen, xlen, fmt, ext);
> > +             break;
> > +     }
> > +}
> > +
> >  void
> >  cpu_identify(struct cpu_info *ci)
> >  {
> > -     char isa[32];
> > +     char *isa;
> >       uint64_t marchid, mimpid;
> >       uint32_t mvendorid;
> >       const char *vendor_name = NULL;
> >       const char *arch_name = NULL;
> >       struct arch *archlist = cpu_arch_none;
> > -     int i, len;
> > +     int i, len, plen;
> >
> >       mvendorid = sbi_get_mvendorid();
> >       marchid = sbi_get_marchid();
> > @@ -139,12 +313,31 @@ cpu_identify(struct cpu_info *ci)
> >               printf(" arch %llx", marchid);
> >       printf(" imp %llx", mimpid);
> >
> > -     len = OF_getprop(ci->ci_node, "riscv,isa", isa, sizeof(isa));
> > -     if (len != -1) {
> > -             printf(" %s", isa);
> > -             strlcpy(cpu_model, isa, sizeof(cpu_model));
> > +     plen = OF_getproplen(ci->ci_node, "riscv,isa");
> > +     isa = malloc(plen, M_TEMP, M_ZERO | M_NOWAIT);
> > +
> > +     if (isa) {
> > +             len = OF_getprop(ci->ci_node, "riscv,isa", isa, plen);
> > +             if (len != -1) {
> > +                     memset(cpu_user_ext, 0, sizeof(cpu_user_ext));
> > +                     memset(cpu_super_ext, 0, sizeof(cpu_super_ext));
> > +
> > +                     do {
> > +                             i = tokenize(i, isa, '_',
> > +                                 create_isa_extension_list, cpu_user_ext,
> > +                                 cpu_super_ext);
> > +                     } while (i > 0);
> > +
> > +                     if (i == 0) {
> > +                             printf("\n");
> > +                             expand_baseisa(cpu_model, expanded_base_isa, sizeof(expanded_base_isa));
> > +                             printf("Base ISA: %s (%s)\n", cpu_model, expanded_base_isa);
> > +                             printf("Standard Userlevel Extensions: %s\n", cpu_user_ext);
> > +                             printf("Standard Supervisor Level  Extensions: %s\n", cpu_super_ext);
> > +                     }
> > +             }
> > +             printf("\n");
> >       }
> > -     printf("\n");
> >
> >       /* Handle errata. */
> >       if (mvendorid == CPU_VENDOR_SIFIVE && marchid == CPU_ARCH_U7)
> > diff --git sys/arch/riscv64/riscv64/machdep.c sys/arch/riscv64/riscv64/machdep.c
> > index d61585cb000..5153bc9f10b 100644
> > --- sys/arch/riscv64/riscv64/machdep.c
> > +++ sys/arch/riscv64/riscv64/machdep.c
> > @@ -312,6 +312,8 @@ cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
> >  {
> >       char *compatible;
> >       int node, len, error;
> > +     extern char cpu_user_ext[];
> > +     extern char cpu_super_ext[];
> >
> >       /* all sysctl names at this level are terminal */
> >       if (namelen != 1)
> > @@ -329,6 +331,13 @@ cpu_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
> >               error = sysctl_rdstring(oldp, oldlenp, newp, compatible);
> >               free(compatible, M_TEMP, len);
> >               return error;
> > +
> > +     case CPU_STD_USER_EXT:
> > +             return (sysctl_rdstring(oldp, oldlenp, newp, cpu_user_ext));
> > +
> > +     case CPU_STD_SUPER_EXT:
> > +             return (sysctl_rdstring(oldp, oldlenp, newp, cpu_super_ext));
> > +
> >       default:
> >               return (EOPNOTSUPP);
> >       }
>