Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

riscv-rt: compatibility with RV32E and RV64E #243

Merged
merged 8 commits into from
Dec 12, 2024
Merged

riscv-rt: compatibility with RV32E and RV64E #243

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d6365e0
compatibility with RV32E
romancardenas Nov 9, 2024
289206a
Add riscv-target-parser crate
romancardenas Nov 18, 2024
1e71338
Use conditional dependencies in riscv-rt depending on base extension
romancardenas Nov 19, 2024
fb14952
Use environment variables for finding out the base ISA on riscv_rt_ma…
romancardenas Nov 27, 2024
7815cd7
Cleaner implementation of LLVM patch and changelog
romancardenas Nov 28, 2024
ef18149
In case of collision, prioritize riscvi over riscve
romancardenas Nov 28, 2024
aee2106
Adjust stack alignment to 4-byte for E base ISAs
romancardenas Dec 4, 2024
da3b8ac
enforce 16-byte alignment
romancardenas Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions riscv-rt/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Changed

- Add documentation to trap frame fields.
- Avoid using `t3`+ in startup assembly to ensure compatibility with RVE32.
- `link.x.in`: remove references to `eh_frame`.
- Rename start/end section symbols to align with `cortex-m-rt`:
- `_stext`: it remains, as linker files can modify it.
Expand Down
27 changes: 13 additions & 14 deletions riscv-rt/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,12 @@ cfg_global_asm!(
#[cfg(riscvm)]
"mul t0, t2, t0",
#[cfg(not(riscvm))]
"beqz t2, 2f // Jump if single-hart
mv t1, t2
mv t3, t0
"beqz t2, 2f // skip if hart ID is 0
mv t1, t0
1:
add t0, t0, t3
addi t1, t1, -1
bnez t1, 1b
add t0, t0, t1
addi t2, t2, -1
bnez t2, 1b
2: ",
);
cfg_global_asm!(
Expand Down Expand Up @@ -153,22 +152,22 @@ cfg_global_asm!(
"call __pre_init
// Copy .data from flash to RAM
la t0, __sdata
la t2, __edata
la a0, __edata
la t1, __sidata
bgeu t0, t2, 2f
bgeu t0, a0, 2f
1: ",
#[cfg(target_arch = "riscv32")]
"lw t3, 0(t1)
"lw t2, 0(t1)
addi t1, t1, 4
sw t3, 0(t0)
sw t2, 0(t0)
addi t0, t0, 4
bltu t0, t2, 1b",
bltu t0, a0, 1b",
#[cfg(target_arch = "riscv64")]
"ld t3, 0(t1)
"ld t2, 0(t1)
addi t1, t1, 8
sd t3, 0(t0)
sd t2, 0(t0)
addi t0, t0, 8
bltu t0, t2, 1b",
bltu t0, a0, 1b",
"
2: // Zero out .bss
la t0, __sbss
Expand Down
17 changes: 16 additions & 1 deletion riscv-rt/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,25 +567,40 @@ pub use riscv_rt_macros::core_interrupt_riscv64 as core_interrupt; // just for d
pub static __ONCE__: () = ();

/// Registers saved in trap handler
#[allow(missing_docs)]
#[repr(C)]
#[derive(Debug)]
pub struct TrapFrame {
/// `x1`: return address, stores the address to return to after a function call or interrupt.
pub ra: usize,
/// `x5`: temporary register `t0`, used for intermediate values.
pub t0: usize,
/// `x6`: temporary register `t1`, used for intermediate values.
pub t1: usize,
/// `x7`: temporary register `t2`, used for intermediate values.
pub t2: usize,
/// `x28`: temporary register `t3`, used for intermediate values.
pub t3: usize,
/// `x29`: temporary register `t4`, used for intermediate values.
pub t4: usize,
/// `x30`: temporary register `t5`, used for intermediate values.
pub t5: usize,
/// `x31`: temporary register `t6`, used for intermediate values.
pub t6: usize,
/// `x10`: argument register `a0`. Used to pass the first argument to a function.
pub a0: usize,
/// `x11`: argument register `a1`. Used to pass the second argument to a function.
pub a1: usize,
/// `x12`: argument register `a2`. Used to pass the third argument to a function.
pub a2: usize,
/// `x13`: argument register `a3`. Used to pass the fourth argument to a function.
pub a3: usize,
/// `x14`: argument register `a4`. Used to pass the fifth argument to a function.
pub a4: usize,
/// `x15`: argument register `a5`. Used to pass the sixth argument to a function.
pub a5: usize,
/// `x16`: argument register `a6`. Used to pass the seventh argument to a function.
pub a6: usize,
/// `x17`: argument register `a7`. Used to pass the eighth argument to a function.
pub a7: usize,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be a good idea to order these fields by there x* name (similar to the spec)?

Also, maybe we use coniditional compilation to not include the x16-x31 registers for RV32E builds?

Mostly thinking about downstream users that may use a pointer to the TrapFrame, which they may or may not try to map to memory layout.

It may not be necessary, but could be a helpful reminder that those registers don't exist on RV32E platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd try to avoid changing the order of the attributes in case users use C or assembly code that relies on the current order.

I'm working on feature-gating registers for RVE32. It's a bit tricky, as we need to add "trash" padding to ensure that the stack is 16-byte aligned. I'll open a PR shortly with all this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd try to avoid changing the order of the attributes in case users use C or assembly code that relies on the current order.

I guess that's also part of what I'm asking, "should we intentionally break downstream users compiling RV32E?" Especially regarding not providing them in-memory registers that don't exist on the platform.

For non-RV32E users, I agree with you, we should avoid breaking them without a good reason.

I'm working on feature-gating registers for RVE32. It's a bit tricky, as we need to add "trash" padding to ensure that the stack is 16-byte aligned. I'll open a PR shortly with all this.

Sounds good, I'll keep an eye out for it.

Maybe for the missing registers on RV32E targets:

pub struct TrapFrame {
    #[cfg(not(rv32e))]
    pub a6: usize,
    #[cfg(rv32e)]
    _a6: usize,
    ...
}

Copy link

@hegza hegza Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working on feature-gating registers for RVE32. It's a bit tricky, as we need to add "trash" padding to ensure that the stack is 16-byte aligned. I'll open a PR shortly with all this.

Wait, 16-byte aligned? You mean for RV32E? Stack on RV32E is specified to be 32-bit / 4-byte aligned. It's a specific feature of RV32E and in contrast to RV32I. References in LLVM & Clang:

}

Expand Down
Loading