Page MenuHomeFreeBSD

riscv: improve parsing of riscv,isa property strings
ClosedPublic

Authored by mhorne on Sep 16 2022, 3:37 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 31, 4:31 PM
Unknown Object (File)
Sun, Oct 13, 11:00 AM
Unknown Object (File)
Sun, Oct 13, 11:00 AM
Unknown Object (File)
Sun, Oct 13, 10:59 AM
Unknown Object (File)
Sun, Oct 13, 10:59 AM
Unknown Object (File)
Sun, Oct 13, 10:49 AM
Unknown Object (File)
Oct 7 2024, 10:38 PM
Unknown Object (File)
Sep 29 2024, 10:14 PM

Details

Summary

This code was originally written under the assumption that the ISA
string would only contain single-letter extensions. The RISC-V
specification has extended its description of the format quite a bit,
allowing for much longer ISA strings containing multi-letter extension
names.

Newer versions of QEMU (7.1.0) will append to the riscv,isa property
indicating the presence of multi-letter standard extensions such as
Zfencei. This triggers a KASSERT about the expected length of the
string, preventing boot.

Increase the size of the isa array significantly, and teach the code
to parse multi-letter extensions, and optional extension version
numbers. We currently ignore them completely, but this will change in
the future as we start supporting supervisor-level extensions.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

The underscore isn't required before the first multi-character extension, and in fact the canonical form drops it (e.g. rv64iafdcxcheri, what we use for CheriBSD, is valid).

i.e. probably this should also stop at [sxz] to handle the multi-letter extensions, though there's a wrinkle that older QEMU bogusly treated S and U as single-letter extensions indicating the presence of those modes. Linux has a hack to spot that specific case (if -1 isn't _ and +1 is u).

The underscore isn't required before the first multi-character extension, and in fact the canonical form drops it (e.g. rv64iafdcxcheri, what we use for CheriBSD, is valid).

Okay, where can one learn the exact rules for how this ISA string is to be formatted? The device tree document Bindings/riscv/cpus.yaml is notably lacking in detail here.

The underscore isn't required before the first multi-character extension, and in fact the canonical form drops it (e.g. rv64iafdcxcheri, what we use for CheriBSD, is valid).

Okay, where can one learn the exact rules for how this ISA string is to be formatted? The device tree document Bindings/riscv/cpus.yaml is notably lacking in detail here.

It's as defined in the RISC-V unprivileged spec

The underscore isn't required before the first multi-character extension, and in fact the canonical form drops it (e.g. rv64iafdcxcheri, what we use for CheriBSD, is valid).

Okay, where can one learn the exact rules for how this ISA string is to be formatted? The device tree document Bindings/riscv/cpus.yaml is notably lacking in detail here.

It's as defined in the RISC-V unprivileged spec

Thank you, I had not kept up with these changes. I'll update with a more robust solution.

Rework the parsing logic to match the ISA string specification. Handle
multi-letter extensions by walking over them completely; same goes for version
numbers. I have tried to sketch this out in a way that anticipates future
changes.

Avoid dynamically allocating space for isa, but bump its size significantly.
We don't currently call fill_elf_hwcap() very early, but I believe that
eventually we may need to, in order to obtain information about relevant
extensions before pmap_bootstrap() (for example, svpbmt), or other early
initialization actions.

sys/riscv/riscv/identcpu.c
207–208

FPE-less builds need to parse these just not update hwcap

sys/riscv/riscv/identcpu.c
207–208

Yes, they will be skipped over by the default case then. Same with p, h, v as they are now.

mhorne retitled this revision from riscv: handle newer riscv,isa property strings to riscv: improve parsing of riscv,isa property strings.Oct 13 2022, 6:31 PM
mhorne edited the summary of this revision. (Show Details)

Any further feedback? I intend to commit this sometime before the end of the week.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 28 2022, 4:37 PM
This revision was automatically updated to reflect the committed changes.