Index | Thread | Search

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

Download raw body.

Thread
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.

-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);
>  	}