riscv vmm: implement SBI RFNC

[br]

Summary

This adds remote fence extension support.

Test Plan

Tested on EIC7700 although it is not clear if the SoC actually needs it: it works fine with and without this support included.

[markj]

This adds remote fence extension support.

Some further explanation, either here or in a comment, would be helpful.

sys/riscv/vmm/riscv.h

71

_INVALID would be a bit clearer as a name here IMO, assuming that "UNK" means "unknown".

sys/riscv/vmm/vmm_fence.c

132

atomic_readandclear_32() is a bit clearer here IMO.

170

Probably this should be atomic_store_32 instead of set_32. The latter is an atomic OR, but it doesn't seem necessary.

193

Why is the mb needed?

sys/riscv/vmm/vmm_riscv.c

750

Need to destroy the mutex here too I believe.

sys/riscv/vmm/vmm_sbi.c

120

Why atomic? This bitset is on the stack.

[br]

Address @markj comments

[br]

sys/riscv/vmm/vmm_fence.c

193

To ensure that the request is enqueued before the state of vcpu is obtained on out of order CPUs ?
vcpu_get_state() returns with vcpu unlocked, so the vcpu could change its state before we even return from vcpu_get_state() function ?
If these assumptions are correct, then vcpu could change from SLEEPING to RUNNING after return from vcpu_get_state(). And if things are reordered then an issue arise (i.e. not interrupting a CPU that we need to interrupt) ?

[jrtc27]

sys/riscv/vmm/riscv.h

104–105

Pack these two as a bitmask of the desired requests?

sys/riscv/vmm/vmm_fence.c

46

bool?

54

Normally one dequeues from the head and enqueues to the tail

56

What's wrong with *new_fence = *fence?

58–60

71

Same comments as dequeue

113

Why the parens?

150

bool

sys/riscv/vmm/vmm_sbi.c

100–101

Unused

224

... and the return value isn't even checked for these

[markj]

sys/riscv/vmm/vmm_fence.c

198

Here too, I don't see why this set needs to be atomic.

209

running_vcpus is a bitset of vcpu IDs, but smp_rendezvous() takes a bitset of physical CPU IDs, so this looks wrong. There is no reason to expect vCPU 0 to be running on CPU 0.

[markj]

sys/riscv/vmm/vmm_fence.c

209

(There is a vm_smp_rendezvous() on amd64 for this purpose, but it was not ported. I will eventually merge this into sys/dev/vmm, but would like to finish with D48270 first.)

[br]

Address @jrtc27 and @markj comments (Thanks!)

sys/riscv/vmm/vmm_sbi.c

100–101

It was used as a return value to the guest (instead of C function return code).
I've it fixed to use C return code, and will fix for the reset of SBI requests in a separate commit.

[markj]

sys/riscv/vmm/vmm_fence.c

209

This is still wrong, just less wrong than before. Once the vcpu is unlocked, there is no guarantee about which host CPU the vcpu is running on. That's why vm_smp_rendezvous() uses vcpu_notify_event().

[br]

sys/riscv/vmm/vmm_fence.c

209

I understand but once we have the request enqueued we don't care if vcpu change its host cpu, because in this case it will process enqueued SBI request on entry to the guest anyway. In this scenario we indeed interrupt wrong CPU without reason, but we don't loose instruction/data synchronization in the guest.