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.

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: define mtvec CSR with macro helpers #252

Merged
merged 5 commits into from
Jan 7, 2025

Conversation

rmsyn
Copy link
Contributor

@rmsyn rmsyn commented Dec 21, 2024

Uses the CSR macro helpers to define the mtvec CSR register.

Adds basic unit-tests for the mtvec CSR.

Ignores the warning for the clippy::identity_op to allow using ranged bitfield CSR macro arguments with a leading 0.

Changes the macro argument type to expr to allow using constants, expressions, and other initializers for CSR mask values.

Changes the macro argument type to `expr` to allow using constants,
expressions, and other initializers for CSR mask values.
Ignores the warning for the `clippy::identity_op` to allow using ranged
bitfield CSR macro arguments with a leading `0`.
@rmsyn rmsyn requested a review from a team as a code owner December 21, 2024 21:15
Comment on lines 39 to 42
/// The address is aligned to 4-bytes.
#[inline]
pub fn trap_mode(&self) -> Option<TrapMode> {
let mode = self.bits & 0b11;
match mode {
0 => Some(TrapMode::Direct),
1 => Some(TrapMode::Vectored),
_ => None,
}
pub fn set_address(&mut self, address: usize) {
self.bits = (address & !TRAP_MASK) | (self.bits & TRAP_MASK);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This 4-byte alignment is in reference to the CSR specification, however in riscv-rt::asm there is a reference to the trap table being 16-byte aligned.

Should we force 16-byte alignment here by masking the lower 4 bits?

Copy link
Contributor

Choose a reason for hiding this comment

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

According to the RISCV Priv spec:

The value in the BASE field must always be aligned on a 4-byte boundary, and the MODE setting may impose additional alignment constraints on the value in the BASE field.
...
An implementation may have different alignment constraints for different modes. In particular, MODE=Vectored may have stricter alignment constraints than MODE=Direct. Allowing coarser alignments in Vectored mode enables vectoring to be implemented without a hardware adder circuit.

So, without additional information about the target, we can only ensure that BASE is 4-byte aligned. For further versions of riscv-rt, we can think of a mechanism to apply a custom alignment depending on the target. For example, the E310x chip expects 64-byte alignment when working in vectored mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

In this way, we can add a try_set_address that checks that base is at least 4-byte aligned

riscv/src/register/mtvec.rs Outdated Show resolved Hide resolved
riscv/src/register/mtvec.rs Outdated Show resolved Hide resolved
Comment on lines 39 to 42
/// The address is aligned to 4-bytes.
#[inline]
pub fn trap_mode(&self) -> Option<TrapMode> {
let mode = self.bits & 0b11;
match mode {
0 => Some(TrapMode::Direct),
1 => Some(TrapMode::Vectored),
_ => None,
}
pub fn set_address(&mut self, address: usize) {
self.bits = (address & !TRAP_MASK) | (self.bits & TRAP_MASK);
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the RISCV Priv spec:

The value in the BASE field must always be aligned on a 4-byte boundary, and the MODE setting may impose additional alignment constraints on the value in the BASE field.
...
An implementation may have different alignment constraints for different modes. In particular, MODE=Vectored may have stricter alignment constraints than MODE=Direct. Allowing coarser alignments in Vectored mode enables vectoring to be implemented without a hardware adder circuit.

So, without additional information about the target, we can only ensure that BASE is 4-byte aligned. For further versions of riscv-rt, we can think of a mechanism to apply a custom alignment depending on the target. For example, the E310x chip expects 64-byte alignment when working in vectored mode.

Comment on lines 39 to 42
/// The address is aligned to 4-bytes.
#[inline]
pub fn trap_mode(&self) -> Option<TrapMode> {
let mode = self.bits & 0b11;
match mode {
0 => Some(TrapMode::Direct),
1 => Some(TrapMode::Vectored),
_ => None,
}
pub fn set_address(&mut self, address: usize) {
self.bits = (address & !TRAP_MASK) | (self.bits & TRAP_MASK);
Copy link
Contributor

Choose a reason for hiding this comment

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

In this way, we can add a try_set_address that checks that base is at least 4-byte aligned

Uses the CSR macro helpers to define the `mtvec` CSR register.

Authored-by: rmsyn <[email protected]>
Reviewed-by: romancardenas <[email protected]>
Adds basic unit-tests for the `mtvec` CSR.
@rmsyn rmsyn force-pushed the riscv/mtvec-csr-macro branch from 6d6b793 to 9fd2f73 Compare December 26, 2024 19:17
romancardenas
romancardenas previously approved these changes Dec 27, 2024
@romancardenas romancardenas added this pull request to the merge queue Jan 7, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 7, 2025
@romancardenas
Copy link
Contributor

Looks like Clippy added new lints. @rmsyn could you address them?

@romancardenas romancardenas added this pull request to the merge queue Jan 7, 2025
Merged via the queue into rust-embedded:master with commit 4038a13 Jan 7, 2025
119 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants