Generic PLIC peripheral

[romancardenas]

I developed a generic driver for interfacing PLIC peripherals, as mentioned in #124.

References

For developing this peripheral, I got inspired by the following resources:

• The PLIC specification
• The PLIC implementation in the e310x and e310x-hal crates.
• The driver for NVIC in cortex-m crate.

Particularities

For atomic operations, I use the volatile-register crate (as done in cortex-m).

Unlike the NVIC for Cortex-M microcontrollers, the standard leaves the base address as "implementation-specific". Thus, my implementation uses a const generic to define the base address of the PLIC. In PACs, developers can do something like this:

pub type PLIC = riscv::peripheral::PLIC<BASE_ADDRESS>;

I decided to use associated functions whenever possible, so we don't need to steal/pass the control of the PLIC. However, those functions that are not atomic are defined as methods.

Finally, I followed the safety guidelines of the NVIC implementation, so those associated functions that modify priorities/thresholds are marked as unsafe.

Let me know what you think. If you prefer any other style, I'll happily modify my implementation.

[dkhayes117]

Thank you for the PR. I'll look it over as soon as I can.

[dkhayes117]

Not all riscv targets will have a PLIC, should this be feature gated?

[dkhayes117]

Although, having to supply a PLIC base address would kind of act like a feature gate 🤔

[romancardenas]

I'm fine with both. I plan to develop other peripherals (e.g., CLIC and (A)CLINT) in the same fashion. From what I read, they all have an implementation-specific base address. Using feature gates to only import the peripherals implemented on each PAC sounds reasonable.

[romancardenas]

Maybe we need to reconsider the concept of a context for the PLIC peripheral. These are my thoughts:

• The main purpose of the contexts is to provide a "safe", isolated interface to the interrupts for each HART.
• Probably, there will be as many contexts as HARTs in the platform.
• Interrupt priorities are shared among contexts
• Each context enables/disables the interrupt sources for their context.
• Each HART attends interrupts via its context.

So I was thinking that the Context trait may be a bit annoying to maintain. Maybe it would be more useful to have one PLIC per context, and the context number could be a const generic too. Something like PLIC<BASE, CTX>. From what I saw, most boards only implement one context. Thus, their crate would have:

pub type PLIC = riscv::peripheral::PLIC<BASE_ADDRESS, 0>;

If the microcontroller has two contexts, then it would look like this:

pub type PLIC0 = riscv::peripheral::PLIC<BASE_ADDRESS, 0>;
pub type PLIC1 = riscv::peripheral::PLIC<BASE_ADDRESS, 1>;

And each HART can own a context. What do you think?

[romancardenas]

@dkhayes117 I developed an alternative implementation of the PLIC that stores the context as a const generic.
I personally like this approach more, because otherwise, I think that dealing with the context number would be non-ergonomic in practice.

I also added a feature gate for optionally adding the peripheral. Let me know what you think!

[romancardenas]

I added additional methods and generics to the current traits. The reason for that is that I am currently working on a port of RTIC for the E310X microcontrollers. As part of this work, I identified nice-to-have features that will ease the integration of RTIC to any platform with PLIC.

[dkhayes117]

Looks good. I left one comment about validating that the source priority != 0. @almindor @Disasm, It would be nice to have another pair of eyes on this if possible. Thanks!

[dkhayes117]

I think this is good to merge, but I'll give it a couple of days to see if the other members have any opinions.

[romancardenas]

@dkhayes117 take a look at my latest commit. Basically, I added utility functions to access the MIE and MIP CSRs. I also take advantage of the try_from methods of the traits to avoid returning raw numbers. This is aligned with the PLIC implementation of e310x-hal.

[romancardenas]

Sorry for all these changes, but I've been discovering some deficiencies in my proposal.

Unlike the cortex-m crate, we need to provide a new/default constructor for the peripherals, as they are all optional and PACs are responsible for creating them when needed.

I developed an alternative version of the e310x crate that uses this generic implementation and compiles OK. Take a look to see how this peripheral would work.

I think this PR is now good to go (fingers crossed).

[romancardenas]

I developed an alternative version of the e310x-hal that uses the generic PLIC peripheral. Later today I'll try a toy example on mi Sparkfun RED-V to confirm and illustrate that it works.

[romancardenas]

Yaaay it works :D
I'm having some issues with the RTC and debugging, but the PLIC works as expected.

https://github.com/romancardenas/hifive1/blob/master/examples/interrupt.rs

[romancardenas]

I opened a new PR with a clean commit history of this very same implementation