diff mbox series

[v1,7/9] RISC-V: split riscv_fill_hwcap() in 3

Message ID 20230626-prevalent-heaviness-e35188de1225@wendy (mailing list archive)
State Superseded
Headers show
Series RISC-V: Probe DT extension support using riscv,isa-extensions & riscv,isa-base | expand

Checks

Context Check Description
conchuod/cover_letter success Series has a cover letter
conchuod/tree_selection success Guessed tree name to be for-next at HEAD 488833ccdcac
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 6 and now 6
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/build_rv64_clang_allmodconfig fail Errors and warnings before: 8 this patch: 14
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig fail Errors and warnings before: 8 this patch: 14
conchuod/build_rv32_defconfig success Build OK
conchuod/dtb_warn_rv64 success Errors and warnings before: 20 this patch: 20
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch success total: 0 errors, 0 warnings, 0 checks, 375 lines checked
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success No Fixes tag
conchuod/build_rv64_nommu_virt_defconfig success Build OK

Commit Message

Conor Dooley June 26, 2023, 11:19 a.m. UTC
Before adding more complexity to it, split riscv_fill_hwcap() into 3
distinct sections:
- riscv_fill_hwcap() still is the top level function, into which the
  additional complexity will be added.
- riscv_fill_hwcap_from_isa_string() handles getting the information
  from the riscv,isa/ACPI equivalent across harts & the various quirks
  there
- riscv_parse_isa_string() does what it says on the tin.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/kernel/cpufeature.c | 350 +++++++++++++++++----------------
 1 file changed, 182 insertions(+), 168 deletions(-)

Comments

Andrew Jones June 26, 2023, 4:17 p.m. UTC | #1
On Mon, Jun 26, 2023 at 12:19:45PM +0100, Conor Dooley wrote:
> Before adding more complexity to it, split riscv_fill_hwcap() into 3
> distinct sections:
> - riscv_fill_hwcap() still is the top level function, into which the
>   additional complexity will be added.
> - riscv_fill_hwcap_from_isa_string() handles getting the information
>   from the riscv,isa/ACPI equivalent across harts & the various quirks
>   there
> - riscv_parse_isa_string() does what it says on the tin.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  arch/riscv/kernel/cpufeature.c | 350 +++++++++++++++++----------------
>  1 file changed, 182 insertions(+), 168 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 5405d8a58537..366477ba1eea 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -180,29 +180,172 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>  
>  const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
>  
> -void __init riscv_fill_hwcap(void)
> +static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo,
> +					  unsigned long *isa2hwcap, const char *isa)
> +{
> +	/*
> +	 * For all possible cpus, we have already validated in
> +	 * the boot process that they at least contain "rv" and
> +	 * whichever of "32"/"64" this kernel supports, and so this
> +	 * section can be skipped.
> +	 */
> +	isa += 4;
> +
> +	while (*isa) {
> +		const char *ext = isa++;
> +		const char *ext_end = isa;
> +		bool ext_long = false, ext_err = false;
> +
> +		switch (*ext) {
> +		case 's':
> +			/*
> +			 * Workaround for invalid single-letter 's' & 'u'(QEMU).
> +			 * No need to set the bit in riscv_isa as 's' & 'u' are
> +			 * not valid ISA extensions. It works until multi-letter
> +			 * extension starting with "Su" appears.
> +			 */
> +			if (ext[-1] != '_' && ext[1] == 'u') {
> +				++isa;
> +				ext_err = true;
> +				break;
> +			}
> +			fallthrough;
> +		case 'S':
> +		case 'x':
> +		case 'X':
> +		case 'z':
> +		case 'Z':
> +			/*
> +			 * Before attempting to parse the extension itself, we find its end.
> +			 * As multi-letter extensions must be split from other multi-letter
> +			 * extensions with an "_", the end of a multi-letter extension will
> +			 * either be the null character or the "_" at the start of the next
> +			 * multi-letter extension.
> +			 *
> +			 * Next, as the extensions version is currently ignored, we
> +			 * eliminate that portion. This is done by parsing backwards from
> +			 * the end of the extension, removing any numbers. This may be a
> +			 * major or minor number however, so the process is repeated if a
> +			 * minor number was found.
> +			 *
> +			 * ext_end is intended to represent the first character *after* the
> +			 * name portion of an extension, but will be decremented to the last
> +			 * character itself while eliminating the extensions version number.
> +			 * A simple re-increment solves this problem.
> +			 */
> +			ext_long = true;
> +			for (; *isa && *isa != '_'; ++isa)
> +				if (unlikely(!isalnum(*isa)))
> +					ext_err = true;
> +
> +			ext_end = isa;
> +			if (unlikely(ext_err))
> +				break;
> +
> +			if (!isdigit(ext_end[-1]))
> +				break;
> +
> +			while (isdigit(*--ext_end))
> +				;
> +
> +			if (tolower(ext_end[0]) != 'p' || !isdigit(ext_end[-1])) {
> +				++ext_end;
> +				break;
> +			}
> +
> +			while (isdigit(*--ext_end))
> +				;
> +
> +			++ext_end;
> +			break;
> +		default:
> +			/*
> +			 * Things are a little easier for single-letter extensions, as they
> +			 * are parsed forwards.
> +			 *
> +			 * After checking that our starting position is valid, we need to
> +			 * ensure that, when isa was incremented at the start of the loop,
> +			 * that it arrived at the start of the next extension.
> +			 *
> +			 * If we are already on a non-digit, there is nothing to do. Either
> +			 * we have a multi-letter extension's _, or the start of an
> +			 * extension.
> +			 *
> +			 * Otherwise we have found the current extension's major version
> +			 * number. Parse past it, and a subsequent p/minor version number
> +			 * if present. The `p` extension must not appear immediately after
> +			 * a number, so there is no fear of missing it.
> +			 *
> +			 */
> +			if (unlikely(!isalpha(*ext))) {
> +				ext_err = true;
> +				break;
> +			}
> +
> +			if (!isdigit(*isa))
> +				break;
> +
> +			while (isdigit(*++isa))
> +				;
> +
> +			if (tolower(*isa) != 'p')
> +				break;
> +
> +			if (!isdigit(*++isa)) {
> +				--isa;
> +				break;
> +			}
> +
> +			while (isdigit(*++isa))
> +				;
> +
> +			break;
> +		}
> +
> +		/*
> +		 * The parser expects that at the start of an iteration isa points to the
> +		 * first character of the next extension. As we stop parsing an extension
> +		 * on meeting a non-alphanumeric character, an extra increment is needed
> +		 * where the succeeding extension is a multi-letter prefixed with an "_".
> +		 */
> +		if (*isa == '_')
> +			++isa;
> +
> +#define SET_ISA_EXT_MAP(name, bit)						\
> +		do {								\
> +			if ((ext_end - ext == sizeof(name) - 1) &&		\
> +			     !strncasecmp(ext, name, sizeof(name) - 1) &&	\
> +			     riscv_isa_extension_check(bit))			\
> +				set_bit(bit, isainfo->isa);			\
> +		} while (false)							\
> +
> +		if (unlikely(ext_err))
> +			continue;
> +		if (!ext_long) {
> +			int nr = tolower(*ext) - 'a';
> +
> +			if (riscv_isa_extension_check(nr)) {
> +				*this_hwcap |= isa2hwcap[nr];
> +				set_bit(nr, isainfo->isa);
> +			}
> +		} else {
> +			for (int i = 0; i < riscv_isa_ext_count; i++)
> +				SET_ISA_EXT_MAP(riscv_isa_ext[i].name,
> +						riscv_isa_ext[i].id);
> +		}
> +#undef SET_ISA_EXT_MAP
> +	}
> +}
> +
> +static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
>  {
>  	struct device_node *node;
>  	const char *isa;
> -	char print_str[NUM_ALPHA_EXTS + 1];
> -	int i, j, rc;
> -	unsigned long isa2hwcap[26] = {0};
> +	int rc;
>  	struct acpi_table_header *rhct;
>  	acpi_status status;
>  	unsigned int cpu;
>  
> -	isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
> -	isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
> -	isa2hwcap['a' - 'a'] = COMPAT_HWCAP_ISA_A;
> -	isa2hwcap['f' - 'a'] = COMPAT_HWCAP_ISA_F;
> -	isa2hwcap['d' - 'a'] = COMPAT_HWCAP_ISA_D;
> -	isa2hwcap['c' - 'a'] = COMPAT_HWCAP_ISA_C;
> -	isa2hwcap['v' - 'a'] = COMPAT_HWCAP_ISA_V;
> -
> -	elf_hwcap = 0;
> -
> -	bitmap_zero(riscv_isa, RISCV_ISA_EXT_MAX);
> -
>  	if (!acpi_disabled) {
>  		status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct);
>  		if (ACPI_FAILURE(status))
> @@ -234,158 +377,7 @@ void __init riscv_fill_hwcap(void)
>  			}
>  		}
>  
> -		/*
> -		 * For all possible cpus, we have already validated in
> -		 * the boot process that they at least contain "rv" and
> -		 * whichever of "32"/"64" this kernel supports, and so this
> -		 * section can be skipped.
> -		 */
> -		isa += 4;
> -
> -		while (*isa) {
> -			const char *ext = isa++;
> -			const char *ext_end = isa;
> -			bool ext_long = false, ext_err = false;
> -
> -			switch (*ext) {
> -			case 's':
> -				/*
> -				 * Workaround for invalid single-letter 's' & 'u'(QEMU).
> -				 * No need to set the bit in riscv_isa as 's' & 'u' are
> -				 * not valid ISA extensions. It works until multi-letter
> -				 * extension starting with "Su" appears.
> -				 */
> -				if (ext[-1] != '_' && ext[1] == 'u') {
> -					++isa;
> -					ext_err = true;
> -					break;
> -				}
> -				fallthrough;
> -			case 'S':
> -			case 'x':
> -			case 'X':
> -			case 'z':
> -			case 'Z':
> -				/*
> -				 * Before attempting to parse the extension itself, we find its end.
> -				 * As multi-letter extensions must be split from other multi-letter
> -				 * extensions with an "_", the end of a multi-letter extension will
> -				 * either be the null character or the "_" at the start of the next
> -				 * multi-letter extension.
> -				 *
> -				 * Next, as the extensions version is currently ignored, we
> -				 * eliminate that portion. This is done by parsing backwards from
> -				 * the end of the extension, removing any numbers. This may be a
> -				 * major or minor number however, so the process is repeated if a
> -				 * minor number was found.
> -				 *
> -				 * ext_end is intended to represent the first character *after* the
> -				 * name portion of an extension, but will be decremented to the last
> -				 * character itself while eliminating the extensions version number.
> -				 * A simple re-increment solves this problem.
> -				 */
> -				ext_long = true;
> -				for (; *isa && *isa != '_'; ++isa)
> -					if (unlikely(!isalnum(*isa)))
> -						ext_err = true;
> -
> -				ext_end = isa;
> -				if (unlikely(ext_err))
> -					break;
> -
> -				if (!isdigit(ext_end[-1]))
> -					break;
> -
> -				while (isdigit(*--ext_end))
> -					;
> -
> -				if (tolower(ext_end[0]) != 'p' || !isdigit(ext_end[-1])) {
> -					++ext_end;
> -					break;
> -				}
> -
> -				while (isdigit(*--ext_end))
> -					;
> -
> -				++ext_end;
> -				break;
> -			default:
> -				/*
> -				 * Things are a little easier for single-letter extensions, as they
> -				 * are parsed forwards.
> -				 *
> -				 * After checking that our starting position is valid, we need to
> -				 * ensure that, when isa was incremented at the start of the loop,
> -				 * that it arrived at the start of the next extension.
> -				 *
> -				 * If we are already on a non-digit, there is nothing to do. Either
> -				 * we have a multi-letter extension's _, or the start of an
> -				 * extension.
> -				 *
> -				 * Otherwise we have found the current extension's major version
> -				 * number. Parse past it, and a subsequent p/minor version number
> -				 * if present. The `p` extension must not appear immediately after
> -				 * a number, so there is no fear of missing it.
> -				 *
> -				 */
> -				if (unlikely(!isalpha(*ext))) {
> -					ext_err = true;
> -					break;
> -				}
> -
> -				if (!isdigit(*isa))
> -					break;
> -
> -				while (isdigit(*++isa))
> -					;
> -
> -				if (tolower(*isa) != 'p')
> -					break;
> -
> -				if (!isdigit(*++isa)) {
> -					--isa;
> -					break;
> -				}
> -
> -				while (isdigit(*++isa))
> -					;
> -
> -				break;
> -			}
> -
> -			/*
> -			 * The parser expects that at the start of an iteration isa points to the
> -			 * first character of the next extension. As we stop parsing an extension
> -			 * on meeting a non-alphanumeric character, an extra increment is needed
> -			 * where the succeeding extension is a multi-letter prefixed with an "_".
> -			 */
> -			if (*isa == '_')
> -				++isa;
> -
> -#define SET_ISA_EXT_MAP(name, bit)							\
> -			do {								\
> -				if ((ext_end - ext == sizeof(name) - 1) &&		\
> -				     !strncasecmp(ext, name, sizeof(name) - 1) &&	\
> -				     riscv_isa_extension_check(bit))			\
> -					set_bit(bit, isainfo->isa);			\
> -			} while (false)							\
> -
> -			if (unlikely(ext_err))
> -				continue;
> -			if (!ext_long) {
> -				int nr = tolower(*ext) - 'a';
> -
> -				if (riscv_isa_extension_check(nr)) {
> -					this_hwcap |= isa2hwcap[nr];
> -					set_bit(nr, isainfo->isa);
> -				}
> -			} else {
> -				for (int i = 0; i < riscv_isa_ext_count; i++)
> -					SET_ISA_EXT_MAP(riscv_isa_ext[i].name,
> -							riscv_isa_ext[i].id);
> -			}
> -#undef SET_ISA_EXT_MAP
> -		}
> +		riscv_parse_isa_string(&this_hwcap, isainfo, isa2hwcap, isa);
>  
>  		/*
>  		 * Linux requires the following extensions, so we may as well
> @@ -422,6 +414,28 @@ void __init riscv_fill_hwcap(void)
>  
>  	if (!acpi_disabled && rhct)
>  		acpi_put_table((struct acpi_table_header *)rhct);
> +}
> +
> +void __init riscv_fill_hwcap(void)
> +{
> +	struct device_node *node;
> +	const char *isa;
> +	char print_str[NUM_ALPHA_EXTS + 1];
> +	int i, j, rc;
> +	unsigned long isa2hwcap[26] = {0};
> +	struct acpi_table_header *rhct;
> +	acpi_status status;
> +	unsigned int cpu;

I see all these unused variables get removed in the next patch, but they
should get removed here, lest they trigger some warnings and bots come
after you!

> +
> +	isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
> +	isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
> +	isa2hwcap['a' - 'a'] = COMPAT_HWCAP_ISA_A;
> +	isa2hwcap['f' - 'a'] = COMPAT_HWCAP_ISA_F;
> +	isa2hwcap['d' - 'a'] = COMPAT_HWCAP_ISA_D;
> +	isa2hwcap['c' - 'a'] = COMPAT_HWCAP_ISA_C;
> +	isa2hwcap['v' - 'a'] = COMPAT_HWCAP_ISA_V;
> +
> +	riscv_fill_hwcap_from_isa_string(isa2hwcap);
>  
>  	/* We don't support systems with F but without D, so mask those out
>  	 * here. */
> -- 
> 2.40.1
>

Otherwise,

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

Thanks,
drew
Conor Dooley June 27, 2023, 5:42 p.m. UTC | #2
On Mon, Jun 26, 2023 at 06:17:51PM +0200, Andrew Jones wrote:
> > +void __init riscv_fill_hwcap(void)
> > +{
> > +	struct device_node *node;
> > +	const char *isa;
> > +	char print_str[NUM_ALPHA_EXTS + 1];
> > +	int i, j, rc;
> > +	unsigned long isa2hwcap[26] = {0};
> > +	struct acpi_table_header *rhct;
> > +	acpi_status status;
> > +	unsigned int cpu;
> 
> I see all these unused variables get removed in the next patch, but they
> should get removed here, lest they trigger some warnings and bots come
> after you!

Funnily enough, I'd pushed this out for LKP and it never complained
about the unused variables - but my own stuff on patchwork did.
diff mbox series

Patch

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 5405d8a58537..366477ba1eea 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -180,29 +180,172 @@  const struct riscv_isa_ext_data riscv_isa_ext[] = {
 
 const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
 
-void __init riscv_fill_hwcap(void)
+static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo,
+					  unsigned long *isa2hwcap, const char *isa)
+{
+	/*
+	 * For all possible cpus, we have already validated in
+	 * the boot process that they at least contain "rv" and
+	 * whichever of "32"/"64" this kernel supports, and so this
+	 * section can be skipped.
+	 */
+	isa += 4;
+
+	while (*isa) {
+		const char *ext = isa++;
+		const char *ext_end = isa;
+		bool ext_long = false, ext_err = false;
+
+		switch (*ext) {
+		case 's':
+			/*
+			 * Workaround for invalid single-letter 's' & 'u'(QEMU).
+			 * No need to set the bit in riscv_isa as 's' & 'u' are
+			 * not valid ISA extensions. It works until multi-letter
+			 * extension starting with "Su" appears.
+			 */
+			if (ext[-1] != '_' && ext[1] == 'u') {
+				++isa;
+				ext_err = true;
+				break;
+			}
+			fallthrough;
+		case 'S':
+		case 'x':
+		case 'X':
+		case 'z':
+		case 'Z':
+			/*
+			 * Before attempting to parse the extension itself, we find its end.
+			 * As multi-letter extensions must be split from other multi-letter
+			 * extensions with an "_", the end of a multi-letter extension will
+			 * either be the null character or the "_" at the start of the next
+			 * multi-letter extension.
+			 *
+			 * Next, as the extensions version is currently ignored, we
+			 * eliminate that portion. This is done by parsing backwards from
+			 * the end of the extension, removing any numbers. This may be a
+			 * major or minor number however, so the process is repeated if a
+			 * minor number was found.
+			 *
+			 * ext_end is intended to represent the first character *after* the
+			 * name portion of an extension, but will be decremented to the last
+			 * character itself while eliminating the extensions version number.
+			 * A simple re-increment solves this problem.
+			 */
+			ext_long = true;
+			for (; *isa && *isa != '_'; ++isa)
+				if (unlikely(!isalnum(*isa)))
+					ext_err = true;
+
+			ext_end = isa;
+			if (unlikely(ext_err))
+				break;
+
+			if (!isdigit(ext_end[-1]))
+				break;
+
+			while (isdigit(*--ext_end))
+				;
+
+			if (tolower(ext_end[0]) != 'p' || !isdigit(ext_end[-1])) {
+				++ext_end;
+				break;
+			}
+
+			while (isdigit(*--ext_end))
+				;
+
+			++ext_end;
+			break;
+		default:
+			/*
+			 * Things are a little easier for single-letter extensions, as they
+			 * are parsed forwards.
+			 *
+			 * After checking that our starting position is valid, we need to
+			 * ensure that, when isa was incremented at the start of the loop,
+			 * that it arrived at the start of the next extension.
+			 *
+			 * If we are already on a non-digit, there is nothing to do. Either
+			 * we have a multi-letter extension's _, or the start of an
+			 * extension.
+			 *
+			 * Otherwise we have found the current extension's major version
+			 * number. Parse past it, and a subsequent p/minor version number
+			 * if present. The `p` extension must not appear immediately after
+			 * a number, so there is no fear of missing it.
+			 *
+			 */
+			if (unlikely(!isalpha(*ext))) {
+				ext_err = true;
+				break;
+			}
+
+			if (!isdigit(*isa))
+				break;
+
+			while (isdigit(*++isa))
+				;
+
+			if (tolower(*isa) != 'p')
+				break;
+
+			if (!isdigit(*++isa)) {
+				--isa;
+				break;
+			}
+
+			while (isdigit(*++isa))
+				;
+
+			break;
+		}
+
+		/*
+		 * The parser expects that at the start of an iteration isa points to the
+		 * first character of the next extension. As we stop parsing an extension
+		 * on meeting a non-alphanumeric character, an extra increment is needed
+		 * where the succeeding extension is a multi-letter prefixed with an "_".
+		 */
+		if (*isa == '_')
+			++isa;
+
+#define SET_ISA_EXT_MAP(name, bit)						\
+		do {								\
+			if ((ext_end - ext == sizeof(name) - 1) &&		\
+			     !strncasecmp(ext, name, sizeof(name) - 1) &&	\
+			     riscv_isa_extension_check(bit))			\
+				set_bit(bit, isainfo->isa);			\
+		} while (false)							\
+
+		if (unlikely(ext_err))
+			continue;
+		if (!ext_long) {
+			int nr = tolower(*ext) - 'a';
+
+			if (riscv_isa_extension_check(nr)) {
+				*this_hwcap |= isa2hwcap[nr];
+				set_bit(nr, isainfo->isa);
+			}
+		} else {
+			for (int i = 0; i < riscv_isa_ext_count; i++)
+				SET_ISA_EXT_MAP(riscv_isa_ext[i].name,
+						riscv_isa_ext[i].id);
+		}
+#undef SET_ISA_EXT_MAP
+	}
+}
+
+static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
 {
 	struct device_node *node;
 	const char *isa;
-	char print_str[NUM_ALPHA_EXTS + 1];
-	int i, j, rc;
-	unsigned long isa2hwcap[26] = {0};
+	int rc;
 	struct acpi_table_header *rhct;
 	acpi_status status;
 	unsigned int cpu;
 
-	isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
-	isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
-	isa2hwcap['a' - 'a'] = COMPAT_HWCAP_ISA_A;
-	isa2hwcap['f' - 'a'] = COMPAT_HWCAP_ISA_F;
-	isa2hwcap['d' - 'a'] = COMPAT_HWCAP_ISA_D;
-	isa2hwcap['c' - 'a'] = COMPAT_HWCAP_ISA_C;
-	isa2hwcap['v' - 'a'] = COMPAT_HWCAP_ISA_V;
-
-	elf_hwcap = 0;
-
-	bitmap_zero(riscv_isa, RISCV_ISA_EXT_MAX);
-
 	if (!acpi_disabled) {
 		status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct);
 		if (ACPI_FAILURE(status))
@@ -234,158 +377,7 @@  void __init riscv_fill_hwcap(void)
 			}
 		}
 
-		/*
-		 * For all possible cpus, we have already validated in
-		 * the boot process that they at least contain "rv" and
-		 * whichever of "32"/"64" this kernel supports, and so this
-		 * section can be skipped.
-		 */
-		isa += 4;
-
-		while (*isa) {
-			const char *ext = isa++;
-			const char *ext_end = isa;
-			bool ext_long = false, ext_err = false;
-
-			switch (*ext) {
-			case 's':
-				/*
-				 * Workaround for invalid single-letter 's' & 'u'(QEMU).
-				 * No need to set the bit in riscv_isa as 's' & 'u' are
-				 * not valid ISA extensions. It works until multi-letter
-				 * extension starting with "Su" appears.
-				 */
-				if (ext[-1] != '_' && ext[1] == 'u') {
-					++isa;
-					ext_err = true;
-					break;
-				}
-				fallthrough;
-			case 'S':
-			case 'x':
-			case 'X':
-			case 'z':
-			case 'Z':
-				/*
-				 * Before attempting to parse the extension itself, we find its end.
-				 * As multi-letter extensions must be split from other multi-letter
-				 * extensions with an "_", the end of a multi-letter extension will
-				 * either be the null character or the "_" at the start of the next
-				 * multi-letter extension.
-				 *
-				 * Next, as the extensions version is currently ignored, we
-				 * eliminate that portion. This is done by parsing backwards from
-				 * the end of the extension, removing any numbers. This may be a
-				 * major or minor number however, so the process is repeated if a
-				 * minor number was found.
-				 *
-				 * ext_end is intended to represent the first character *after* the
-				 * name portion of an extension, but will be decremented to the last
-				 * character itself while eliminating the extensions version number.
-				 * A simple re-increment solves this problem.
-				 */
-				ext_long = true;
-				for (; *isa && *isa != '_'; ++isa)
-					if (unlikely(!isalnum(*isa)))
-						ext_err = true;
-
-				ext_end = isa;
-				if (unlikely(ext_err))
-					break;
-
-				if (!isdigit(ext_end[-1]))
-					break;
-
-				while (isdigit(*--ext_end))
-					;
-
-				if (tolower(ext_end[0]) != 'p' || !isdigit(ext_end[-1])) {
-					++ext_end;
-					break;
-				}
-
-				while (isdigit(*--ext_end))
-					;
-
-				++ext_end;
-				break;
-			default:
-				/*
-				 * Things are a little easier for single-letter extensions, as they
-				 * are parsed forwards.
-				 *
-				 * After checking that our starting position is valid, we need to
-				 * ensure that, when isa was incremented at the start of the loop,
-				 * that it arrived at the start of the next extension.
-				 *
-				 * If we are already on a non-digit, there is nothing to do. Either
-				 * we have a multi-letter extension's _, or the start of an
-				 * extension.
-				 *
-				 * Otherwise we have found the current extension's major version
-				 * number. Parse past it, and a subsequent p/minor version number
-				 * if present. The `p` extension must not appear immediately after
-				 * a number, so there is no fear of missing it.
-				 *
-				 */
-				if (unlikely(!isalpha(*ext))) {
-					ext_err = true;
-					break;
-				}
-
-				if (!isdigit(*isa))
-					break;
-
-				while (isdigit(*++isa))
-					;
-
-				if (tolower(*isa) != 'p')
-					break;
-
-				if (!isdigit(*++isa)) {
-					--isa;
-					break;
-				}
-
-				while (isdigit(*++isa))
-					;
-
-				break;
-			}
-
-			/*
-			 * The parser expects that at the start of an iteration isa points to the
-			 * first character of the next extension. As we stop parsing an extension
-			 * on meeting a non-alphanumeric character, an extra increment is needed
-			 * where the succeeding extension is a multi-letter prefixed with an "_".
-			 */
-			if (*isa == '_')
-				++isa;
-
-#define SET_ISA_EXT_MAP(name, bit)							\
-			do {								\
-				if ((ext_end - ext == sizeof(name) - 1) &&		\
-				     !strncasecmp(ext, name, sizeof(name) - 1) &&	\
-				     riscv_isa_extension_check(bit))			\
-					set_bit(bit, isainfo->isa);			\
-			} while (false)							\
-
-			if (unlikely(ext_err))
-				continue;
-			if (!ext_long) {
-				int nr = tolower(*ext) - 'a';
-
-				if (riscv_isa_extension_check(nr)) {
-					this_hwcap |= isa2hwcap[nr];
-					set_bit(nr, isainfo->isa);
-				}
-			} else {
-				for (int i = 0; i < riscv_isa_ext_count; i++)
-					SET_ISA_EXT_MAP(riscv_isa_ext[i].name,
-							riscv_isa_ext[i].id);
-			}
-#undef SET_ISA_EXT_MAP
-		}
+		riscv_parse_isa_string(&this_hwcap, isainfo, isa2hwcap, isa);
 
 		/*
 		 * Linux requires the following extensions, so we may as well
@@ -422,6 +414,28 @@  void __init riscv_fill_hwcap(void)
 
 	if (!acpi_disabled && rhct)
 		acpi_put_table((struct acpi_table_header *)rhct);
+}
+
+void __init riscv_fill_hwcap(void)
+{
+	struct device_node *node;
+	const char *isa;
+	char print_str[NUM_ALPHA_EXTS + 1];
+	int i, j, rc;
+	unsigned long isa2hwcap[26] = {0};
+	struct acpi_table_header *rhct;
+	acpi_status status;
+	unsigned int cpu;
+
+	isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
+	isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
+	isa2hwcap['a' - 'a'] = COMPAT_HWCAP_ISA_A;
+	isa2hwcap['f' - 'a'] = COMPAT_HWCAP_ISA_F;
+	isa2hwcap['d' - 'a'] = COMPAT_HWCAP_ISA_D;
+	isa2hwcap['c' - 'a'] = COMPAT_HWCAP_ISA_C;
+	isa2hwcap['v' - 'a'] = COMPAT_HWCAP_ISA_V;
+
+	riscv_fill_hwcap_from_isa_string(isa2hwcap);
 
 	/* We don't support systems with F but without D, so mask those out
 	 * here. */