Update Systick trait, implement it for e310x and ibex

[hudson-ayers]

Pull Request Overview

This PR updates the SysTick trait to be less specific to the ARM systick peripheral, to instead represent a System Timer abstraction that can be more effectively used by chips without this optional peripheral. It also adds an implementation of this trait for two RISC-V chips, ibex and the e310x, by virtualizing the single available hardware timer available on each such that it can be used by both the scheduler and by kernel capsules/userspace apps.

This includes renaming SysTick to SystemTimer (name subject to bikeshedding) and modifying the functions and documentation of the trait slightly. This PR also removes the reliance on a dedicated interrupt handler for system timer interrupts, instead relying on there simply being some interrupt that fires, and calling systick.expired() whenever userspace processes are interrupted to determine if there has been a timeslice expiration. This change is critical for enabling the ability to implement the system timer on top of a hardware peripheral that is shared with other timer functionality.

This PR also includes a new VirtualSystemTimer type which can be constructed by passing anything implementing the Alarm trait, allowing for VirtualMuxAlarms to be used to create a systick. Notably, the design of MuxAlarm is such that there is still constant-time overhead on all systick operations except for handling the systick interrupt when it fires (which is O(N) where N is the number of virtual alarms). This seems like a reasonable tradeoff to me, given that systick expirations already lead to the
higher overhead of a context switch, and a well designed system (and scheduler) should minimize the frequency of app preemption.

This PR moves opentitan and the hifive1 to use the new VirtualSystemTimer.

This PR also removes the reliance of the ARM systick on modifying a global static mut variable to signal timeslice expirations.

Finally, this PR also implements SystemTimer directly on the RISC-V mtimer, but this will only work if a different hardware timer is used to provide alarms to the kernel/userspace, and this implementation is untested given that we do not support doing so on any of our current RISC-V chips.

Testing Strategy

This pull request (well, a slightly earlier version) was tested on HiFive1 in QEMU and appeared to work (apps still functioned, and putting a busy loop in apps led to timeslice expirations that preempted the app).

I also tested it on OpenTitan in verilator, but have run into issues where after a timeslice expirations I receive spurious interrupts. I still need to debug this to determine whether it is an issue with this PR or the tock implementation of rv_timer().

I still need to test on existing ARM boards.

TODO or Help Wanted

This pull request still needs feedback on names, and feedback on the changes to the trait.
I learned today that @rajivr has also been working on modifying the SysTick trait, and he has taken a somewhat different approach, so it may be useful to compare and contrast these approaches.

More testing will be needed.

Documentation Updated

• Updated the relevant files in /docs, or no updates are required.

Formatting

• Ran make prepush.

[bradjc]

Why not enable_interrupt()?

[hudson-ayers]

@phil-levis didn't like the name enable_interrupt() because it might imply that all interrupts for the underlying hardware timer are disabled, which is not the case if the trait is on top of a virtualized timer.

[bradjc]

Ok overall I think the symmetry in arm/disarm probably makes for the most clear interface, since disarm doesn't have to actually disable anything. I think just capturing the importance of this interrupt in the documentation better to explain this subtlety will avoid any questions along this line in the future. I can add a commit.

[hudson-ayers]

Merge conflicts resolved, and I think all comments have been addressed.

The coupling between the VirtualSchedulerTimer and the implementation of VirtualMuxAlarm is a little strong still, but I would prefer to resolve that when the new time HIL arrives.

[bradjc]

I added the doc commit I promised. I also really like the name.

My last two questions:

• Why start_timer() instead of start()? start/reset seems like a more obvious pair. Also, now that it is called SchedulerTimer, start() should be pretty obvious it starts the timer.
• Why both us_remaining() and get_remaining_us()?

[hudson-ayers]

My last two questions:

• Why start_timer() instead of start()? start/reset seems like a more obvious pair. Also, now that it is called SchedulerTimer, start() should be pretty obvious it starts the timer.

Agreed, changed.

• Why both us_remaining() and get_remaining_us()?

The reason I kept at_least_us_remaining() was because I believed that it was possible for such an implementation to be more efficient -- in theory if we only ever call this function with a single value, the compiler should be able to precompute the us --> ticks conversion for that constant, so at runtime there is only a single register read followed by a comparison, while calling get_remaining_us() will always require converting the remaining tics to microseconds, which requires 64 bit multiplication and division. However I just benchmarked both approaches and found that the constant propagation I was expecting for at_least_us_remaining() does not happen, and in fact that approach takes more cycles (47, as compared to 22 for using get_remaining_us() < MIN_QUANTA_THRESHOLD), so I went ahead and removed at_least_us_remaining()

[hudson-ayers]

The updated version has now been tested on ARM by checking that apps are preempted as expected and that these preemptions are counted by the process console.

I will try to re-test on RISC-V before this is merged.

[ppannuto]

Looks good -- one comment on a name that I think would be better, but not blocking-worthy to change

[hudson-ayers]

rebased on master, changed expired() --> has_expired(). I tested opentitan on verilator and verified that a long running app was preempted by the scheduler timer, then resumed and finished. The timing felt a little weird to me, as it seemed that it probably should have been preempted multiple times, but I don't fully understand how verilator works so I am gonna chalk that up to weirdness with the emulated timer frequency.

I think this is ready to go.

[bradjc]

bors r+

[bors]

Build succeeded:

• ci-format (ubuntu-latest)
• ci-qemu
• ci-tests (macos-latest)
• ci-build (ubuntu-latest)
• ci-format (macos-latest)
• ci-tests (ubuntu-latest)
• ci-build (macos-latest)