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.
Details
Details
- Reviewers
br markj - Commits
- rS338914: Implement pmap_sync_icache().
- Used 'start' of a /bin/ls process in gdb running under FreeBSD/riscv in qemu.
Diff Detail
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 19708 Build 19272: arc lint + arc unit
Event Timeline
Comment Actions
"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.
| sys/riscv/riscv/pmap.c | ||
|---|---|---|
| 3251 | 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"? | |
Comment Actions
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?
| sys/riscv/riscv/pmap.c | ||
|---|---|---|
| 3251 | 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. | |
Comment Actions
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()?
Comment Actions
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?