Re: [PATCH v4 0/4] riscv: add support for SBI Supervisor Software Events

["Björn Töpel" <bjorn@kernel.org>]

Clément!

Clément Léger <cleger@rivosinc.com> writes:

> The SBI Supervisor Software Events (SSE) extensions provides a mechanism
> to inject software events from an SBI implementation to supervisor
> software such that it preempts all other supervisor level traps and
> interrupts. This extension is introduced by the SBI v3.0 specification[1].
>
> Various events are defined and can be send asynchronously to supervisor
> software (RAS, PMU, DEBUG, Asynchronous page fault) from SBI as well
> as platform specific events. Events can be either local (per-hart) or
> global. Events can be nested on top of each other based on priority and
> can interrupt the kernel at any time.
>
> First patch adds the SSE definitions. Second one adds support for SSE
> at arch level (entry code and stack allocations) and third one at driver
> level. Finally, the last patch add support for SSE events in the SBI PMU
> driver. Additional testing for that part is highly welcomed since there
> are a lot of possible path that needs to be exercised.
>
> Amongst the specific points that needs to be handle is the interruption
> at any point of the kernel execution and more specifically at the
> beginning of exception handling. Due to the fact that the exception entry
> implementation uses the SCRATCH CSR as both the current task struct and
> as the temporary register to switch the stack and save register, it is
> difficult to reliably get the current task struct if we get interrupted
> at this specific moment (ie, it might contain 0, the task pointer or tp).
> A fixup-like mechanism is not possible due to the nested nature of SSE
> which makes it really hard to obtain the original interruption site. In
> order to retrieve the task in a reliable manner, add an additional
> __sse_entry_task per_cpu array which stores the current task. Ideally,
> we would need to modify the way we retrieve/store the current task in
> exception handling so that it does not depend on the place where it's
> interrupted.
>
> Contrary to pseudo NMI [2], SSE does not modifies the way interrupts are
> handled and does not adds any overhead to existing code. Moreover, it
> provides "true" NMI-like interrupts which can interrupt the kernel at
> any time (even in exception handling). This is particularly crucial for
> RAS errors which needs to be handled as fast as possible to avoid any
> fault propagation.
>
> While OpenSBI SSE support is already upstream, an additional patch is
> needed for the PMU perf driver to work as expected [2].
>
> Link: https://github.com/riscv-non-isa/riscv-sbi-doc/releases/download/v3.0-rc7/riscv-sbi.pdf [1]
> Link: https://github.com/rivosinc/opensbi/tree/dev/cleger/sse_pmu_irq [2]

Finally got around having a look/test!

Thanks for adding all the SSE tests to kvm-unit-tests [1]! What a hidden
gem -- at least to me!

It would be nice with similar tests in kselftest, so that we can
exercise the SSE paths on the CI worker!

...and a couple of general comments that apply to all patches.

  * There are a bunch of checkpatch warnings/errors -- not all make
    sense, but some do!
  * Minor spelling errors in the commit messages, that codespell
    catches.
  * There are some new unused variables warnings in the build.

Most of that (modulo the spellchecks) in PW [2].
  
I'll have some minor things in the other patches.


Thanks for working on this, and looking forward to having it land!
Björn

[1] https://gitlab.com/kvm-unit-tests/kvm-unit-tests
[2] https://patchwork.kernel.org/project/linux-riscv/list/?series=963680