My last note is a suggestion, it's also fine as-is. I will probably work on changing the pmap to honor VM_PROT_EXECUTE next which will alter this a bit anyway and might use that approach then.

I think I would perhaps reword the commit message as "2nd Scheme" isn't very descriptive. Maybe something like:

• Drop commented out fence at the end of sfence_vma.
• Use static instead of PMAP_INLINE for pmap_invalidate_*().
• Use the SBI interface for remote fence.i for pmap_sync_icache.
• Flush the i-cache on all harts before writing a new PTE for an executable mapping.
• Don't use SFENCE.VMA after changing a mapping from invalid to valid.

• Fix whitespace in sbi_call().
• Drop fence() before single-threaded fence_i().
• Trim parens.

Hmm, so this should actually be part of the commit to libntpevent if we do that independent of the openssl merge (which I think we should)

My only suggestion might be to consider an explicit "disabled" tunable. For SMP we have 'kern.smp.disabled=1'. Having 'vm.numa.disabled=1' or 'kern.numa.disabled=1' might be more intuitive than setting ndomains to 1. (The implementation might be that you set ndomains to 1 if the tunable is set, but it's the UI I'm thinking of).

Looks good apart from a sorting nit.

Hmm, I think for this we want to actually wait to set PTE_A and PTE_D for user mappings until we get a fault? Presumably we'd want to check in the page fault handler if f the PTE was already valid before calling vm_fault and if it was set PTE_A (and PTE_D for a write) and then retry?

Actually, my plan is to just change them on powerpc and have them unified across the board. They should then really just live in sys/elf_common.h. I think if we want to move them now we should just move them and maybe add some #ifdef powerpc that we eventually remove when renumbering powerpc's vectors. I think the actual renumbering change is probably too big of a change during code freeze.

• Fix build on mips (32-bit and using GCC instead of clang)

Just commit already. openssh isn't buildable without this. :-P Note that if we merge this to head before openssl is updated we'd need a different config.h in head.

• More bjk feedback.

Thanks

Use "mips64el" instead of "mips64le" in the commit message. Good sleuthing!

In D17344#369936, @kib wrote:

Are the real changes only in syscall.masters ? The syscall.master' diffs look fine.

Agree with kib on the general point that all of freebsd32_ioctl is probably busted and should be moved to the actual ioctl handlers.

• Various fixes from bjk@

I did some very simple testing which seemed to work:

Fixed the various issues I noted earlier.

So the fact that some things were updated for the new name and that it clashed with the old name made just reverting to the old name messy. I already had done the union change as a followup to remove the compat macros, and it occurred to me that since I had it, I could actually go back to using kf_sa_local/peer in both places which would let both the old and new names work at the same time. This in theory shouldn't require reverting any of the ports changes or upstream port changes while still preserving the existing API. We could even go back and fix the kf_sock_ variants to remove the trailing '0' now if we wished to as well.

• Use an anonymous union and struct for FreeBSD 11 compat fields.
• Rename the kf_un.kf_sock variants back to their original names.

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

exp-run requested at https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=231525

• Use kf_sock_local/peer instead of kf_sa_local0/peer0.

Originally it was useful to disable this when a bug was encountered, but the window where that was useful probably ended several years ago.

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()?