Commits · 7816ebc1ddd16b5cc95febb75f778bf88411a365 · Kirill Smelkov / linux

21 Jun, 2023 5 commits

RISC-V: remove decrement/increment dance in ISA string parser · 7816ebc1

Conor Dooley authored Jun 07, 2023

While expanding on the comments in the ISA string parsing code, I
noticed that the conditional decrement of `isa` at the end of the loop
was a bit odd.
The parsing code expects that at the start of the for loop, `isa` will
point to the first character of the next unparsed extension.
However, depending on what the next extension is, this may not be true.
Unless the next extension is a multi-letter extension preceded by an
underscore, `isa` will either point to the string's null-terminator or
to the first character of the next extension, once the switch statement
has been evaluated.
Obviously incrementing `isa` at the end of the loop could cause it to
increment past the null terminator or miss a single letter extension, so
`isa` is conditionally decremented, just so that the loop can increment
it again.

It's easier to understand the code if, instead of this decrement +
increment dance, we instead use a while loop & rely on the handling of
individual extension types to leave `isa` pointing to the first
character of the next extension.
As already mentioned, this won't be the case where the following
extension is multi-letter & preceded by an underscore. To handle that,
invert the check and increment rather than decrement.
Hopefully this eliminates a "huh?!?" moment the next time somebody tries
to understand this code.
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Link: https://lore.kernel.org/r/20230607-estate-left-f20faabefb89@spudSigned-off-by: Palmer Dabbelt <palmer@rivosinc.com>

7816ebc1

RISC-V: rework comments in ISA string parser · 6b913e3d

Conor Dooley authored Jun 07, 2023

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@ventanamicro.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Link: https://lore.kernel.org/r/20230607-headpiece-tannery-83ed5cc4856a@spudSigned-off-by: Palmer Dabbelt <palmer@rivosinc.com>

6b913e3d

RISC-V: validate riscv,isa at boot, not during ISA string parsing · 069b0d51

Conor Dooley authored Jun 07, 2023

Since riscv_fill_hwcap() now only iterates over possible cpus, the
basic validation of whether riscv,isa contains "rv<width>" can be moved
to riscv_early_of_processor_hartid().

Further, "ima" support is required by the kernel, so reject any CPU not
fitting the bill.
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Link: https://lore.kernel.org/r/20230607-guts-blurry-67e711acf328@spudSigned-off-by: Palmer Dabbelt <palmer@rivosinc.com>

069b0d51

RISC-V: split early & late of_node to hartid mapping · 2ac87434

Conor Dooley authored Jun 07, 2023

Some back and forth with Drew [1] about riscv_fill_hwcap() resulted in
the realisation that it is not very useful to parse the DT & perform
validation of riscv,isa every time we would like to get the id for a
hart.

Although it is no longer called in riscv_fill_hwcap(),
riscv_of_processor_hartid() is called in several other places.
Notably in setup_smp() it forms part of the logic for filling the mask
of possible CPUs. Since a possible CPU must have passed this basic
validation of riscv,isa, a repeat validation is not required.

Rename riscv_of_processor_id() to riscv_early_of_processor_id(),
which will be called from setup_smp() & introduce a new
riscv_of_processor_id() which makes use of the pre-populated mask of
possible cpus.

Link: https://lore.kernel.org/linux-riscv/xvdswl3iyikwvamny7ikrxo2ncuixshtg3f6uucjahpe3xpc5c@ud4cz4fkg5dj/ [1]
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Link: https://lore.kernel.org/r/20230607-glade-pastel-d8cbd9d9f3c6@spudSigned-off-by: Palmer Dabbelt <palmer@rivosinc.com>

2ac87434

RISC-V: simplify register width check in ISA string parsing · fed14be4

Conor Dooley authored Jun 07, 2023

Saving off the `isa` pointer to a temp variable, followed by checking if
it has been incremented is a bit of an odd pattern. Perhaps it was done
to avoid a funky looking if statement mixed with the ifdeffery.

Now that we use IS_ENABLED() here just return from the parser as soon as
we detect a mismatch between the string and the currently running
kernel.
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Sunil V L <sunilvl@ventanamicro.com>
Link: https://lore.kernel.org/r/20230607-splatter-bacterium-a75bb9f0d0b7@spudSigned-off-by: Palmer Dabbelt <palmer@rivosinc.com>

fed14be4

06 Jun, 2023 3 commits

Merge patch series "riscv: allow case-insensitive ISA string parsing" · 748462b5

Palmer Dabbelt authored Jun 06, 2023

Yangyu Chen <cyy@cyyself.name> says:

This patchset allows case-insensitive ISA string parsing, which is
needed in the ACPI environment. As the RISC-V Hart Capabilities Table
(RHCT) description in UEFI Forum ECR[1] shows the format of the ISA
string is defined in the RISC-V unprivileged specification[2]. However,
the RISC-V unprivileged specification defines the ISA naming strings are
case-insensitive while the current ISA string parser in the kernel only
accepts lowercase letters. In this case, the kernel should allow
case-insensitive ISA string parsing. Moreover, this reason has been
discussed in Conor's patch[3]. And I have also checked the current ISA
string parsing in the recent ACPI support patch[4] will also call
`riscv_fill_hwcap` function as DT we use now.

The original motivation for my patch v1[5] is that some SoC generators
will provide generated DT with illegal ISA string in dt-binding such as
rocket-chip, which will even cause kernel panic in some cases as I
mentioned in v1[5]. Now, the rocket-chip has been fixed in PR #3333[6].
However, when using some specific version of rocket-chip with
illegal ISA string in DT, this patchset will also work for parsing
uppercase letters correctly in DT, thus will have better compatibility.

In summary, this patch not only works for case-insensitive ISA string
parsing to meet the requirements in ECR[1] but also can be a workaround
for some specific versions of rocket-chip.

* b4-shazam-merge:
dt-bindings: riscv: drop invalid comment about riscv,isa lower-case reasoning
riscv: allow case-insensitive ISA string parsing

Link: https://lore.kernel.org/r/tencent_E6911C8D71F5624E432A1AFDF86804C3B509@qq.comSigned-off-by: Palmer Dabbelt <palmer@rivosinc.com>

748462b5

dt-bindings: riscv: drop invalid comment about riscv,isa lower-case reasoning · 9e320d7c

Conor Dooley authored May 02, 2023

"Ease of parsing" may have been the initial argument for keeping this
string in lower-case, but parsers may have been written that expect
lower-case only.
For example, the one in released kernels currently does not behave
correctly for multi-letter extensions that begin with a capital letter.
Allowing upper-case here brings about no benefit but would break
compatibility between new devicetrees and older kernels.

Drop the comment to avoid confusing people.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Yangyu Chen <cyy@cyyself.name>
Link: https://lore.kernel.org/r/tencent_3B8290DDC66D3E624132ED39C7465CDC9807@qq.comSigned-off-by: Palmer Dabbelt <palmer@rivosinc.com>

9e320d7c

riscv: allow case-insensitive ISA string parsing · 255b34d7

Yangyu Chen authored May 02, 2023

According to RISC-V Hart Capabilities Table (RHCT) description in UEFI
Forum ECR, the format of the ISA string is defined in the RISC-V
unprivileged specification which is case-insensitive. However, the
current ISA string parser in the kernel does not support ISA strings
with uppercase letters.

This patch modifies the ISA string parser in the kernel to support
case-insensitive ISA string parsing.
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Yangyu Chen <cyy@cyyself.name>
Link: https://lore.kernel.org/r/tencent_B30EED51C7235CA1988890E5C658BE35C107@qq.comSigned-off-by: Palmer Dabbelt <palmer@rivosinc.com>

255b34d7

01 Jun, 2023 22 commits

Merge patch series "Add basic ACPI support for RISC-V" · 90502d51

Palmer Dabbelt authored Jun 01, 2023

Sunil V L <sunilvl@ventanamicro.com> says:

This patch series enables the basic ACPI infrastructure for RISC-V.
Supporting external interrupt controllers is in progress and hence it is
tested using poll based HVC SBI console and RAM disk.

The first patch in this series is one of the patch from Jisheng's
series [1] which is not merged yet. This patch is required to support
ACPI since efi_init() which gets called before sbi_init() can enable
static branches and hits a panic.

Below are two ECRs approved by ASWG.
RINTC - https://drive.google.com/file/d/1R6k4MshhN3WTT-hwqAquu5nX6xSEqK2l/view
RHCT - https://drive.google.com/file/d/1nP3nFiH4jkPMp6COOxP6123DCZKR-tia/view

Testing:
1) Build latest Qemu

2) Build EDK2 as per instructions in
https://github.com/vlsunil/riscv-uefi-edk2-docs/wiki/RISC-V-Qemu-Virt-support

3) Build Linux after enabling SBI HVC and SBI earlycon
CONFIG_RISCV_SBI_V01=y
CONFIG_SERIAL_EARLYCON_RISCV_SBI=y
CONFIG_HVC_RISCV_SBI=y

4) Build buildroot.

Run with below command.
qemu-system-riscv64   -nographic \
-drive file=Build/RiscVVirtQemu/RELEASE_GCC5/FV/RISCV_VIRT.fd,if=pflash,format=raw,unit=1 \
-machine virt -smp 16 -m 2G \
-kernel arch/riscv/boot/Image \
-initrd buildroot/output/images/rootfs.cpio \
-append "root=/dev/ram ro console=hvc0 earlycon=sbi"

* b4-shazam-merge:
  RISC-V: Enable ACPI in defconfig
  RISC-V: time.c: Add ACPI support for time_init()
  clocksource/timer-riscv: Add ACPI support
  clocksource/timer-riscv: Refactor riscv_timer_init_dt()
  irqchip/riscv-intc: Add ACPI support
  RISC-V: cpu: Enable cpuinfo for ACPI systems
  RISC-V: cpufeature: Add ACPI support in riscv_fill_hwcap()
  RISC-V: only iterate over possible CPUs in ISA string parser
  RISC-V: smpboot: Add ACPI support in setup_smp()
  RISC-V: smpboot: Create wrapper setup_smp()
  drivers/acpi: RISC-V: Add RHCT related code
  RISC-V: ACPI: Cache and retrieve the RINTC structure
  RISC-V: Add ACPI initialization in setup_arch()
  ACPI: processor_core: RISC-V: Enable mapping processor to the hartid
  RISC-V: Add support to build the ACPI core
  ACPI: OSL: Make should_use_kmap() 0 for RISC-V
  ACPI: tables: Print RINTC information when MADT is parsed
  crypto: hisilicon/qm: Fix to enable build with RISC-V clang
  platform/surface: Disable for RISC-V
  riscv: move sbi_init() earlier before jump_label_init()

Link: https://lore.kernel.org/r/20230515054928.2079268-1-sunilvl@ventanamicro.comSigned-off-by: Palmer Dabbelt <palmer@rivosinc.com>

90502d51