riscv / riscv-fast-interrupt

Proposal for a RISC-V Core-Local Interrupt Controller (CLIC)
https://jira.riscv.org/browse/RVG-63
Creative Commons Attribution 4.0 International
237 stars 49 forks source link

Clarify isolation mechanism over CLIC memory mapped registers #156

Closed joxie closed 3 years ago

joxie commented 3 years ago

Per discussions in fast interrupt TG we are expecting memory mapped registers to be aliased in M/S mode systems, it would be good to provide some clarity in spec to avoid confusion.

For example, below statements may make people think that CLIC is enforcing priv mode identifier on memory bus when accessing these registers:

All CLIC-memory mapped registers are visible to M-mode. Interrupt registers clicintip[i], clicintie[i], clicintattr[i], clicintctrl[i] configured as M-mode interrupts are not acessible to S-mode and U-mode. Interrupt registers clicintip[i], clicintie[i], clicintattr[i], clicintctrl[i] configured as S-mode interrupts are not acessible to U-mode.

In S-mode, any interrupt i that is not accessible to S-mode appears as hard-wired zeros in clicintip[i], clicintie[i], clicintattr[i], and clicintctl[i].

Another example is that cliccfg might be RO for S mode and RW for M mode, but memory map says it is RW:

0x0000 1B RW cliccfg

dansmathers commented 3 years ago

Adding tech-fast-int email chain to document issue discussion:

allen baum: This is effectively implementing the access privilege of CSR (which are hart local) to MMIO (which are presumably system global, but don't really have to be?) You've got mode specific address ranges and something like restricted-view. CSRs

I agree with Joe: it sounds like you need to send the priv level along with the MMIO address on the interconnect, or the configuration must be hart local (and can't be shared between harts easily) or that the configuration is rarely changed (so you need to to interrupt configuration shoot-down, much like TLB shootdown).

Am I understanding this correctly?

Joe Xie: Does not feel like CLIC spec should/can enforce priv level identifier? This is not something most people will do IMHO.

Torbjorn Viem Ness Isn’t there supposed to be one CLIC per core (hence the “Core Local” part), so the restrictions could simply be determined by a privilege mode signal straight from the core to the CLIC?

This might be a bit more complicated in the case of cores with multiple harts, so the spec could perhaps use some clarification on what to do then…

Krste Asanovic The CLIC memory map has multiple regions, one per privilege mode. The intent is that only the necessary address regions are made accessible to each privilege mode using the system's standard memory protection mechanisms. This can be done either using PMPs in microcontroller systems, or page tables (and/or PMPs) in processors with virtual memory support. In neither case does this require interconnect to carry privilege mode of initiator.

Joe Xie I agree that makes most sense.

Do you mean all MMO registers will be duplicated, say there will be 3 copies of them (with slightly different behavior) for a system with U/S/M mode interrupt?

Krste Asanovic The registers aren't physically duplicated, but are aliased with modified accessibility in different address ranges.

The U-mode region would only be implemented if user-mode interrupts were supported. Systems with only M/S interrupts would only have two regions.

We're pausing work on the N extension for user-mode interrupts, as there is more interest in using bare S-mode, which has already been ratified, and which allows interrupts to be delegated from M-mode to S-mode. In this system, either less-trusted code runs in S-mode (ignore U-mode) or an RTOS can run in S-mode and RTOS tasks run in U-mode (with probably S-mode PMPs provide S/U isolation), with the security monitor underneath in M-mode.

Joe Xie: Thanks Krste, yeah should be aliased instead of duplication. Would be good to clarify that part more clearly in the spec, e.g. may expect cliccfg to be read only for S mode?

Krste Asanovic: OK - can you add issue to track requested clarification,

Joe Xie: Thanks again Krste. Just another thought, would it be even better to give them different prefix to further distinguish M/S alias e.g. [m/s]clicintattr and specify the behaviors clearly, given that the behavior in M/S mode is different, just like priv spec?

Allen Baum: The lack of that prefix is what made me think there was a single register rather than a restricted view implementation. And, yeah, the "L" in cLic should have made me realize it is hart local, and you can use PMA-like approaches to make it work. Sorry for the spam.

kasanovic commented 3 years ago

Will add clarification to spec about how memory apertures are intended to be protected across privilege modes.

kasanovic commented 3 years ago

In TG meeting 6/8/2021 we realized the discussion of memory apertures was incorrectly removed from the spec, and need to be reinstated to remove this confusion.

kasanovic commented 3 years ago

Added back text on memory apertures in commit https://github.com/riscv/riscv-fast-interrupt/commit/f2c9a1e7ef6fab9eb69bff39b0c0f1d677dc4561 . Still needs a little clean up to help clarify these issues.