[RFC,v3,0/4] riscv: cpufeature: Improvements for extended feature handling

Message

Tsukasa OI Dec. 2, 2021, 1:41 a.m. UTC

This is my v3 patchset about RISC-V device tree-based feature handling.

Background sections are mostly unmodified from RFC PATCH v1.  I repost
them so that this mail contains all important information and background.


Index
------

-   History
-   Background: New Extensions
-   Background: How Current ISA Handling is Broken?
-   Patches
-   Changes from RFC PATCH v2 -> v3


History
--------

RFC PATCH v1:
http://lists.infradead.org/pipermail/linux-riscv/2021-November/010252.html
RFC PATCH v2:
http://lists.infradead.org/pipermail/linux-riscv/2021-November/010350.html
RFC PATCH v3:
You are here.


Background: New Extensions
---------------------------

In this year, many of standard multi-letter extensions are being frozen,
reviewed and ratified.

They include:

-   Svpbmt (Page-Based Memory Types) 1.0
-   Zfinx / Zdinx / Zhinx (FP in Integer Registers) 1.0.0-rc
-   Zba / Zbb / Zbc / Zbs (Bit Manipulation) 1.0.0
-   Zkn / Zks / Zkr (Scalar Crypto) 1.0.0-rc6
-   Zve* / Zvl* (Vector Subextensions) 1.0

many of which introduce new instructions, new CSR (registers/bits)
and/or new features.

Also, it's highly plausible that distinguishing incompatible major versions
(version 2 and later) will be needed in the future.

In the device tree, we have "riscv,isa" string representing implemented ISA
and extensions supported by a hart.  Parsing this enables feature
detection. (Current kernel parses this string to detect presence of FPU.)

However, current ISA handling code does not allow handling those multi-
letter extensions and version numbers for multiple reasons.

(a) We haven't defined how do we encode those in ISA bitmap and
(b) current code simply does not support those

My series of patches are intended to resolve (b) and pave a path to easily
detect features noted by versioned and/or multi-letter extensions
(Extended Features) after (a) is resolved.


Background: How Current ISA Handling is Broken?
------------------------------------------------

Current arch/riscv/kernel/cpufeature.c (as of
commit 8bb7eca972ad ("Linux 5.15")) has two issues:

(a) It handles "riscv,isa" as prefix + array of single-letter extensions
(b) ignores every other characters (including version numbers)

For instance, ISA variant "rv32i2p1" means:

-   RISC-V, 32-bit
-   Base ISA "I" version 2.1

but current code parses this string (on 32-bit kernel) as:

-   RISC-V, 32-bit (which matches the kernel)
-   Base ISA "I"
-   Packed-SIMD extension "P"

This is clearly broken ...but it isn't very bad considering proposed "P"
draft is not yet frozen.  With non-versioned "rv64imazifencei_zdinx"
however (on 64-bit kernel), things can get really worse:

(intended meaning)
-   RISC-V, 64-bit
-   Base Instruction Set "I"
-   Single-letter Extensions "M", "A"
-   Multi-letter Extensions "Zifencei" and "Zdinx"

(parsed as)
-   RISC-V, 64-bit
-   Base Instruction **Sets** "I" and "E"
-   Extensions "M" and "A"
-   Compressed Instruction Extension "C"
-   Floating Point Extensions "F" and "D"
-   Reserved Extension "N" (removed user-level interrupts)
-   Supervisor Extension here? "S"

Incorrect detection of "F" and "D" is really bad ("C" is no good either).

Yes, I admit that "Zdinx" (double FP in GPR) is here to demonstrate the
worst kind of the problem intentionally.  But the problem stays.

Even if the extension does not need kernel support (e.g. extensions provide
new instructions but only general purpose registers are involved), wrong
host ISA detection can easily confuse the kernel and will cause problems
in the near future.  That's why I submit this series of patches now (not
during/after kernel support for multiple Extended Features is discussed).


Patches
--------

Patch 1/4: Change BITS_PER_LONG to number of alphabet letters
    Current ISA pretty-printing code assumes only bit 0-25 ('a'-'z') are
    used (it uses 'a' + BIT_OFFSET to print supported extensions).
    Although current parser does not set bit 26 and higher, it will be an
    annoying problem if we start to use those high feature bits.

Patch 2/4: Minimal "riscv,isa" Parser
    This patch implements minimal "riscv,isa" parser, which is designed to
    ignore multi-letter extensions and version numbers.  With this patch,
    you can safely ignore mutli-letter extensions and version numbers.

Patch 3/4: Full "riscv,isa" Parser (Part 1: Extension Names)
    This patch enables to extract multi-letter extension names. You can
    match any extension names to test some feature.
    Note that I implemented a backward parser to handle extension names
    with one or more digits such as "Zvl256b" (which is not valid per
    current ISA Manual not relaxing the rule is being discussed).
    cf. <https://github.com/riscv/riscv-isa-manual/pull/718>

Patch 4/4: Full "riscv,isa" Parser (Part 2: Version Numbers)
    In addition to Patch 3, this patch enables you to extract version
    information of an extension.  If version number handling is not needed,
    this patch may be omitted
    (I consider that Patch 1-3 satisfy most usecases).


Changes from RFC PATCH v2 -> v3
--------------------------------

-   'H'-extension is now parsed as a single-letter extension

    Because current RISC-V ISA Manual and software implementations seemed
    inconsistent, I decided to comply with the ISA Manual in
    RFC PATCH v1/v2 and reported an "issue" to riscv-isa-manual.
    cf. <https://github.com/riscv/riscv-isa-manual/issues/781>

    Greg Favor commented that 'H'-extension is in fact a single-letter
    extension and corresponding sections in the ISA Manual will be
    changed in a few month.

    Now, I'm confident enough to consider that 'H' is a single-letter
    extension and should not be handled as a prefix of multi-letter
    extensions "H*".  If ISA change didn't happen, we can discuss later.

    RISC-V KVM is now compatible with this patchset.

-   Changed parser error handling again

    It seemed that need to handling version numbers is rare and possible
    error number handling seemed a bit clumsy.  So, I simplified it
    (usually, we don't have to know *which* version number is invalid).
    Patch 4 is optional but I request to merge this too due to small
    overhead of version number handling and possible usecases involving
    custom and/or experimental extensions.

    `ext_err`:
        It indicates whether a valid extension token is parsed.
        If `false`, the extension token is valid.
    `ext_err_ver` (Patch 4 only):
        It indicates whether version number of the extension token is valid
        and correctly parsed by the implementation.
        If `false`, version number `ext_{major,minor}` are valid.
        Note that those version numbers have default values.
        `ext_major`:
            `UINT_MAX` means the token does not contain any version
            information.  Usually, implied major version is `1`
            (with some exceptions).
        `ext_minor`:
            `0` means either:
                (a) the token does not contain minor version number or
                (b) given minor version number is `0`.

-   `MATCH_EXT` macro is added

    I added `MATCH_EXT` macro to Patch 3.  This macro enables you to test
    whether certain extension name (in lowercase) is matched.

    For instance, `MATCH_EXT("zba")` will match extension "Zba".

    I didn't included an example for this but did in my GitHub branch
    (on the top of Patch 4).
    <https://github.com/a4lg/linux/tree/riscv-cpufeature.v3>

-   Added `unlikely` to error path




Tsukasa OI (4):
  riscv: cpufeature: Correctly print supported extensions
  riscv: cpufeature: Minimal parser for "riscv,isa" strings
  riscv: cpufeature: Extract extension names from "riscv,isa"
  riscv: cpufeature: Full parser for "riscv,isa" strings

 arch/riscv/kernel/cpufeature.c | 120 ++++++++++++++++++++++++++++-----
 1 file changed, 104 insertions(+), 16 deletions(-)


base-commit: 8bb7eca972ad531c9b149c0a51ab43a417385813