rust-embedded / riscv

Low level access to RISC-V processors
818 stars 160 forks source link

`riscv-rt`: Support rv32e #178

Closed hegza closed 4 months ago

hegza commented 6 months ago

BLUF

riscv-rt fails to link on riscv32emc. GCC complains about "mis-matched ISA string to merge 'i' and 'e'". This PR changes riscv-rt such that it does link on riscv32emc, by removing accesses to registers that don't exist on RVE.

Contents

As defined in RISC-V Unprivileged Spec. 20191213, the only change in RVE is to reduce the number of integer registers to 16 (x0-x15). This requires also a separate calling convention that seems a little underspecified.

Somebody on the internet said these are also implied by the respective ilp32e calling convention:

This PR does the following:

WIP

  1. The latest riscv-rt master branch fails to compile when codegen-units > 1 in Cargo.toml' [profile.*].

    .../riscv32-unknown-elf/bin/ld:link.x:77: undefined symbol `default_start_trap' referenced in expression

    But I think that is unrelated to the issue at hand. Perhaps worthy of its own issue.

  2. I think this patch might not be all that is required for RVE support, but this is at least the minimal amount of changes that makes riscv-rt link on RVE, allowing further testing.
  3. FPGA testing pending. I won't be able to test the interrupts yet, but I should be able to confirm the runtime works otherwise. Done.

Extra context

We're building an RV32EMC in the lab. It's not supported by latest Rust, so we're rolling our own toolchain https://github.com/soc-hub-fi/rust-rv32emc-docker 😃

hegza commented 6 months ago

Eh, curiously enough, something like the original problem re-surfaces if built with --release. I'm doing the testing on a virtual machine without any extra package caching, and I've done a clean build between debug & release.

/opt/riscv/riscv32-unknown-elf/bin/ld: error: /root/riscv-rt-test/target/riscv32emc-unknown-none-elf/release/deps/libriscv_rt-88803cdbe53aa305.rlib(riscv_rt-88803cdbe53aa305.riscv_rt.2debb9bc8c5d66ff-cgu.0.rcgu.o): mis-matched ISA string to merge 'i' and 'e' /opt/riscv/riscv32-unknown-elf/bin/ld: failed to merge target specific data of file /root/riscv-rt-test/target/riscv32emc-unknown-none-elf/release/deps/libriscv_rt-88803cdbe53aa305.rlib(riscv_rt-88803cdbe53aa305.riscv_rt.2debb9bc8c5d66ff-cgu.0.rcgu.o)

hegza commented 6 months ago

Latest commit fixes the issue with --release. It seems the LLVM spurious error affects us too 🙂

hegza commented 6 months ago

By disabling -Fsingle-hart I figured also the M-instructions broke similarly to what happened before with E. I added the .attributes hack for rv32em and that fixed the issue. I presume same would happen for C-type instructions so I pre-emptively added also "emc" and "ec".

hegza commented 6 months ago

FPGA testing done. riscv-rt is capable of running a led blinker on an Ibex-type RV32EMC 🥳

romancardenas commented 6 months ago

Looks good to me!! @hegza Should we leave it as WIP until you can test it further?

hegza commented 6 months ago

I think it's up to you whether you can accept the incremental support for emc without guarantees that the interrupt behavior is sound. If that's fine, please merge. It's easier for me to progress incrementally from here.

In time, I am likely to test some emc interrupts based on my fork at http://github.com/soc-hub-fi/pulpissimo-riscv/ which is a port of riscv-rt for Ibex. Ibex does not support direct-mode interrups so vectored must be the default.

If there's anything minor regarding code style I'd like to address it.

romancardenas commented 6 months ago

The main issue I see is that the official Rust toolchain does not support these targets yet, so it is not possible to keep a simple CI to check that we don't break anything in the future.

hegza commented 6 months ago

Yeah I'd agree with that. I think it's a valid idea to wait for the official toolchain to catch up, then add the CI checks, then merge.