From: Nick Owens Subject: Re: riscv: Parse and classify the ISA extensions from ISA extension string from FDT To: Mike Larkin Cc: Doongar Singh , tech@openbsd.org Date: Wed, 4 Mar 2026 23:27:40 -0800 On Wed, Mar 4, 2026 at 11:13 PM Mike Larkin 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); > > } >