On Fri, 2021-06-04 at 12:31 +0300, Nick Kossifidis wrote:

Στις 2021-06-03 18:13, Anup Patel έγραψε:

This patch adds SBI direct memory access synchronize (DSYN))
extension
which allows S-mode (or VS-mode) software to explicitly synchronize
memory with assistance from the M-mode (or HS-mode).

Signed-off-by: Anup Patel <anup.patel@...>

Thanks for working on this, it seems simple and clean, some thoughts:

a) I also prefer DMAS or something with DMA in the name, and fixed-
sized
arguments.

Agreed.

b) Device and CPU don't necessarily have the same view of the memory,
we
need to define that physical address is the address the CPU sees.

c) Custom CMOs may only accept virtual addresses instead of physical,
in
which case we'll need to switch them back to virtual in the firmware.
Upon registration SBI may tell the OS if it accepts physical or virtual
addresses and the OS can act accordingly (switch cpu_addr to physical
or
not).

d) Since these operations may also be implemented with custom
instructions (instead of e.g. a register write somewhere) I agree that
keeping the code in the firmware makes more sense than allowing custom
instructions in the kernel, on the other hand these operations are
supposed to be performed on S-mode and doing an ecall for them adds a
bit of an overhead. This extension would be a good candidate for using
the vDSO-like interface we discussed at some point. M-mode could share
a
code region with S-mode (both PMP and ePMP allow this scenario) and
during registration of the extension, SBI will return the physical
address of the region, its size and a set of offsets for the different
functions in there (in this case only one function). 

The firmware code will still be executed while the priv mode is S-mode
right ?

Wouldn't that violate the priv spec ?

I'm not very
passionate about this, after all an ecall isn't that expensive and a
DMA
sync is not an operation that happens very frequently, but maybe it's a
good opportunity to talk about this approach.

That's what I am thinking. The only additional cost is just a "ecall
and mret".

IMO, there will be noticeable difference in performance in vDSO-like
interface where S-mode is trying to read something that M-mode
provides. Thus, the base function list are likely candidates [1].
However, the OS makes those SBI calls once during the boot. Thus, it
wouldn't benefit that much.


[1] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.adoc#function-listing

Regards,
Nick

--
Regards,
Atish