Implement pmap_sync_icache().

[jhb]

Summary

This uses an IPI to execute "fence.i" on all processors similar to the
implementation on MIPS.

This is required to support userland debuggers setting breakpoints in
user processes.

Test Plan

• Used 'start' of a /bin/ls process in gdb running under FreeBSD/riscv in qemu.

[markj]

"To make a store to instruction memory visible to all RISC-V harts, the writing hart has to execute a data FENCE before requesting that all remote RISC-V harts execute a FENCE.I."

Doesn't that imply that we should execute a bare "fence" before the SMP rendezvous?

BTW, there are some intrinsics in cpufunc_asm.S, but I don't think there's a good reason for them to be proper subroutines. It might be nice to make a fence_i(void) inline function in cpufunc.h, similar to what we have on amd64.

[jhb]

sys/riscv/riscv/pmap.c

3251 ↗

(On Diff #47968)

It's not clear to me why the cpufunc version of this is implemented as a function in assembly rather than as an inline function? Also, the function in assembly adds arguments that it doesn't use that would have had to be faked if I reused it here. I wonder if we don't want a simple function without any arguments that is an inline asm wrapper around "fence.i"?

[jhb]

In D17139#365472, @markj wrote:

"To make a store to instruction memory visible to all RISC-V harts, the writing hart has to execute a data FENCE before requesting that all remote RISC-V harts execute a FENCE.I."

Doesn't that imply that we should execute a bare "fence" before the SMP rendezvous?

Mmm, yes, though I wonder if smp_rendezvous doesn't already do a fence already? Doesn't hurt to be explicit though.

BTW, there are some intrinsics in cpufunc_asm.S, but I don't think there's a good reason for them to be proper subroutines. It might be nice to make a fence_i(void) inline function in cpufunc.h, similar to what we have on amd64.

Sorry, I had a pending comment for that that didn't get published until just now. Also, it seems like the existing use of fence.i doesn't have a fence?

[markj]

sys/riscv/riscv/pmap.c

3251 ↗

(On Diff #47968)

I don't think there's a real reason. I asked br@ about it a while ago and he noted that it was based on what arm64 does.

[markj]

In D17139#365780, @jhb wrote:

In D17139#365472, @markj wrote:

"To make a store to instruction memory visible to all RISC-V harts, the writing hart has to execute a data FENCE before requesting that all remote RISC-V harts execute a FENCE.I."

Doesn't that imply that we should execute a bare "fence" before the SMP rendezvous?

Mmm, yes, though I wonder if smp_rendezvous doesn't already do a fence already? Doesn't hurt to be explicit though.

BTW, there are some intrinsics in cpufunc_asm.S, but I don't think there's a good reason for them to be proper subroutines. It might be nice to make a fence_i(void) inline function in cpufunc.h, similar to what we have on amd64.

Sorry, I had a pending comment for that that didn't get published until just now. Also, it seems like the existing use of fence.i doesn't have a fence?

How about having cpu_icache_sync_range() issue a fence and fence.i, and modify pmap_sync_icache() to issue the fence before the smp_rendezvous()?

[jhb]

In D17139#365825, @markj wrote:

In D17139#365780, @jhb wrote:

In D17139#365472, @markj wrote:

"To make a store to instruction memory visible to all RISC-V harts, the writing hart has to execute a data FENCE before requesting that all remote RISC-V harts execute a FENCE.I."

Doesn't that imply that we should execute a bare "fence" before the SMP rendezvous?

Mmm, yes, though I wonder if smp_rendezvous doesn't already do a fence already? Doesn't hurt to be explicit though.

BTW, there are some intrinsics in cpufunc_asm.S, but I don't think there's a good reason for them to be proper subroutines. It might be nice to make a fence_i(void) inline function in cpufunc.h, similar to what we have on amd64.

Sorry, I had a pending comment for that that didn't get published until just now. Also, it seems like the existing use of fence.i doesn't have a fence?

How about having cpu_icache_sync_range() issue a fence and fence.i, and modify pmap_sync_icache() to issue the fence before the smp_rendezvous()?

Hmmm, cpu_icache_sync_range() is currently using a sfence.vma (which is a TLB flush). I think it should probably just be a 'fence; fence.i'. In fact, all of the '*cache_*inv_range functions should probably not be using sfence.vma, but instead use fence. I think probably though that we should do a followup change to add inline asm functions for the various fence instructions in cpufunc.h (perhaps under #ifdef KERNEL as fence() is probably too generic of a name for userland) and then use those explicitly in places where we need fences and retire the abstraction macros in cpufunc.h. As part of that, we might also want to use more of the sbi routines (e.g. for TLB shutdowns).

Hmm, looks like we don't do any TLB shootdowns at all right now. ARMv8 doesn't require them since it has magic instructions for that, so perhaps that is missing due to this port being based on aarch64?

[jhb]

• Use a regular 'fence' in pmap_sync_icache() before the IPI.