In D56925#1303888, @jrtc27 wrote:

The Vector spec does not require any ordering on the memory accesses performed, so a legal implementation could alternate which address faults and this would never make forward progress.

But also, I don't think we should be doing anything that would allow spurious kernel faults like this in the first place? For userspace, sure, but for the kernel?

Well, e.g., pmap_enter() doesn't issue sfence_vma() after installing the new PTE, so Bojan's explanation seems plausible. Does the panic go away if we do that instead, something like the patch below?

diff --git a/sys/riscv/riscv/pmap.c b/sys/riscv/riscv/pmap.c
index 1f3fd8c24b81..a74d99d5646b 100644
--- a/sys/riscv/riscv/pmap.c
+++ b/sys/riscv/riscv/pmap.c
@@ -3466,6 +3466,8 @@ pmap_enter(pmap_t pmap, vm_offset_t va, vm_page_t m, vm_prot_t prot,
                        vm_page_dirty(m);
        } else {
                pmap_store(l3, new_l3);
+               if (pmap == kernel_pmap)
+                       sfence_vma(); /* only updates the local TLB */
        }
 
 #if VM_NRESERVLEVEL > 0

Alternately, we can try to make pmap_fault() avoid acquiring the lock for the kernel pmap, see how it uses pmap_klookup() on arm64 to deal with a similar problem.

In D56925#1303888, @jrtc27 wrote:

The Vector spec does not require any ordering on the memory accesses performed, so a legal implementation could alternate which address faults and this would never make forward progress.

Thank you for pointing this out, I wasn't aware of that limitation.

In D56925#1303898, @markj wrote:

Well, e.g., pmap_enter() doesn't issue sfence_vma() after installing the new PTE, so Bojan's explanation seems plausible. Does the panic go away if we do that instead, something like the patch below?
Alternately, we can try to make pmap_fault() avoid acquiring the lock for the kernel pmap, see how it uses pmap_klookup() on arm64 to deal with a similar problem.

This does fix the vtbuf panic, but I immediately run into a similar thing when the APs are launched:

panic: mtx_lock() by idle thread 0xffffffc05ce01140 on mutex 0xffffffc000bd8c80 @ /usr/src/sys/riscv/riscv/pmap.c:2942
cpuid = 2
time = 1
KDB: stack backtrace:
db_trace_self() at db_trace_self
db_trace_self_wrapper() at db_trace_self_wrapper+0x36
kdb_backtrace() at kdb_backtrace+0x2c
vpanic() at vpanic+0x134
panic() at panic+0x26
$x() at $x+0x6e
pmap_fault() at pmap_fault+0x52
page_fault_handler() at page_fault_handler+0x11e
do_trap_supervisor() at do_trap_supervisor+0x6c
cpu_exception_handler_supervisor() at cpu_exception_handler_supervisor+0x74
--- exception 13, tval = 0xffffffc002719210
cpu_search_highest() at cpu_search_highest+0xdc
sched_ule_idletd() at sched_ule_idletd+0x154
fork_exit() at fork_exit+0x68
fork_trampoline() at fork_trampoline+0xa
Uptime: 1s
timeout stopping cpus
panic: mtx_lock() by idle thread 0xffffffc05ce

However even if it did fix those panics, I still don't think it would've been an adequate solution since that situation (i.e., spurious fault in a non-pmap lock-safe context) can still happen.
Thank you for suggesting pmap_klookup, I think something along those lines is the proper solution here, I'll see what I can do.

In D56925#1303962, @bnovkov wrote:

In D56925#1303888, @jrtc27 wrote:

The Vector spec does not require any ordering on the memory accesses performed, so a legal implementation could alternate which address faults and this would never make forward progress.

Thank you for pointing this out, I wasn't aware of that limitation.

In D56925#1303898, @markj wrote:

Well, e.g., pmap_enter() doesn't issue sfence_vma() after installing the new PTE, so Bojan's explanation seems plausible. Does the panic go away if we do that instead, something like the patch below?
Alternately, we can try to make pmap_fault() avoid acquiring the lock for the kernel pmap, see how it uses pmap_klookup() on arm64 to deal with a similar problem.

This does fix the vtbuf panic, but I immediately run into a similar thing when the APs are launched:

panic: mtx_lock() by idle thread 0xffffffc05ce01140 on mutex 0xffffffc000bd8c80 @ /usr/src/sys/riscv/riscv/pmap.c:2942
cpuid = 2
time = 1
KDB: stack backtrace:
db_trace_self() at db_trace_self
db_trace_self_wrapper() at db_trace_self_wrapper+0x36
kdb_backtrace() at kdb_backtrace+0x2c
vpanic() at vpanic+0x134
panic() at panic+0x26
$x() at $x+0x6e
pmap_fault() at pmap_fault+0x52
page_fault_handler() at page_fault_handler+0x11e
do_trap_supervisor() at do_trap_supervisor+0x6c
cpu_exception_handler_supervisor() at cpu_exception_handler_supervisor+0x74
--- exception 13, tval = 0xffffffc002719210
cpu_search_highest() at cpu_search_highest+0xdc
sched_ule_idletd() at sched_ule_idletd+0x154
fork_exit() at fork_exit+0x68
fork_trampoline() at fork_trampoline+0xa
Uptime: 1s
timeout stopping cpus
panic: mtx_lock() by idle thread 0xffffffc05ce

However even if it did fix those panics, I still don't think it would've been an adequate solution since that situation (i.e., spurious fault in a non-pmap lock-safe context) can still happen.
Thank you for suggesting pmap_klookup, I think something along those lines is the proper solution here, I'll see what I can do.

I'll point out though that there's a lot of complexity introduced by the possibility of transient faults on arbitrary kernel addresses. Like, if it's possible to take a transient fault while dereferencing curthread, life becomes tricky; see the uses of NO_PERTHREAD_SSP in arm64 platform code. So I do wonder if it wouldn't be better to just issue a TLB shootdown when a kernel_pmap PTE changes from invalid to valid.

The approach seems sensible to me, given the discussion above.

I think the same/similar short explanatory comment ought to be added to all instances of this handling, especially since the behaviour deviates from other pmap implementations.

Between you and markj I will trust that all relevant entry points are covered here.

@markj after chasing down a few issues you pointed out, I've decided to opt for the pmap_klookup approach instead since I think it's a more straightforward solution.

The patch survived a full buildkernel run.