From: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
I have found these comments to not be at all helpful whenever I look at
the parser. Further, the comments in the default case (single letter
parser) are not quite right either.
Group the comments into a larger one at the start of each case, that
attempts to explain things at a higher level.
Reviewed-by: Andrew Jones <ajones@xxxxxxxxxxxxxxxx>
Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
---
arch/riscv/kernel/cpufeature.c | 70 ++++++++++++++++++++++++++++------
1 file changed, 59 insertions(+), 11 deletions(-)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index a79c5c52a174..cc5189c7c64e 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -146,7 +146,7 @@ void __init riscv_fill_hwcap(void)
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
@@ -163,53 +163,101 @@ void __init riscv_fill_hwcap(void)
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;
- /* Multi-letter extension must be delimited */
for (; *isa && *isa != '_'; ++isa)
if (unlikely(!isalnum(*isa)))
ext_err = true;
- /* Parse backwards */
+
ext_end = isa;
if (unlikely(ext_err))
break;
+
if (!isdigit(ext_end[-1]))
break;
- /* Skip the minor version */
+
while (isdigit(*--ext_end))
;
- if (tolower(ext_end[0]) != 'p'
- || !isdigit(ext_end[-1])) {
- /* Advance it to offset the pre-decrement */
+
+ if (tolower(ext_end[0]) != 'p' || !isdigit(ext_end[-1])) {
++ext_end;
break;
}
- /* Skip the major version */
+
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;
}
- /* Find next extension */
+
if (!isdigit(*isa))
break;
- /* Skip the minor version */
+
while (isdigit(*++isa))
;
+
if (tolower(*isa) != 'p')
break;
+
if (!isdigit(*++isa)) {
--isa;
break;
}
- /* Skip the major version */
+
while (isdigit(*++isa))
;
+
break;
}
+
+ /*
+ * The parser expects that at the start of an iteration isa points to the
+ * character before the start of the next extension. This will not be the
+ * case if we have just parsed a single-letter extension and the next
+ * extension is not a multi-letter extension prefixed with an "_". It is
+ * also not the case at the end of the string, where it will point to the
+ * terminating null character.
+ */
if (*isa != '_')
--isa;
--
2.39.2
