Page MenuHomeFreeBSD
Differential D57624

riscv/pmap.c: Add an Svinval-aware variant of pmap_invalidate_range
ClosedPublic
Actions

Authored by bnovkov on Jun 17 2026, 9:23 PM.
Tags
None
Referenced Files
F175177753: D57624.id181310.diff
Thu, Oct 8, 9:45 PM
Unknown Object (File)
Tue, Oct 6, 10:03 PM
Unknown Object (File)
Sun, Oct 4, 10:47 PM
Unknown Object (File)
Sun, Oct 4, 7:03 PM
Unknown Object (File)
Thu, Oct 1, 6:59 PM
Unknown Object (File)
Wed, Sep 30, 9:32 AM
Unknown Object (File)
Wed, Sep 30, 6:48 AM
Unknown Object (File)
Sat, Sep 26, 9:16 AM
View All 93 Files
Subscribers
imp
mhorne

Details

Reviewers
markj
mhorne
Group Reviewers
riscv
Commits
rG99360212c739: riscv/pmap.c: Add an Svinval-aware variant of pmap_invalidate_range
Summary

This change introduces a variant of pmap_invalidate_range that uses
the fine-grained TLB invalidation instructions introduced by the
Svinval extension. These instructions allow for more efficient TLB
flushing on certain implementations. Under this new scheme,
pmap_invalidate_range was converted to an ifunc that selects the
appropriate variant during boot.

Event: BSDCan 2026

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bnovkov created this revision.Jun 17 2026, 9:23 PM
Herald added a subscriber: imp. · View Herald TranscriptJun 17 2026, 9:23 PM
bnovkov requested review of this revision.Jun 17 2026, 9:23 PM
Harbormaster completed remote builds in B73949: Diff 179962.Jun 17 2026, 9:23 PM
bnovkov added a parent revision: D57623: riscv: Add support for the Svinval extension.Jun 17 2026, 9:24 PM
mhorne accepted this revision.Jun 24 2026, 2:03 PM
mhorne added a subscriber: mhorne.
mhorne added inline comments.
sys/riscv/riscv/pmap.c
1115–1116
This revision is now accepted and ready to land.Jun 24 2026, 2:03 PM
mhorne added inline comments.Jun 24 2026, 2:06 PM
sys/riscv/riscv/pmap.c
1102

The relationship between this check and the definition above was a momentary confusion for me.

You might do away with the atop() and PAGE_SIZE division altogether.

markj added a comment.Jun 24 2026, 5:28 PM

Why not use this for pmap_invalidate_page() as well?

bnovkov updated this revision to Diff 181310.Jul 5 2026, 12:45 PM

Address @markj 's and @mhorne 's comments:

  • add an svinval-aware pmap_invalidate_page variant
  • remove awkward atop conversion
This revision now requires review to proceed.Jul 5 2026, 12:45 PM
mhorne accepted this revision.Jul 9 2026, 5:21 PM
This revision is now accepted and ready to land.Jul 9 2026, 5:21 PM
markj added inline comments.Jul 13 2026, 2:45 PM
sys/riscv/riscv/pmap.c
1129

Why is this done in the callback? Shouldn't we just fall back to a direct pmap_invalidate_all() call?

1152

I think you want to pass smp_no_rendezvous_barrier instead of NULL here, otherwise all CPUs will needlessly synchronize before invoking the callback, and again after. See smp_rendezvous_action().

bnovkov updated this revision to Diff 181919.Jul 13 2026, 6:20 PM

Address @markj 's comments

This revision now requires review to proceed.Jul 13 2026, 6:20 PM
Harbormaster completed remote builds in B74776: Diff 181919.Jul 13 2026, 6:20 PM
bnovkov marked 2 inline comments as done.Jul 13 2026, 6:22 PM
bnovkov added inline comments.
sys/riscv/riscv/pmap.c
1129

No particular reason, it seemed like the most straightforward solution at the time.
I have no preferences here, I've worked your suggestion into the patch.

markj added inline comments.Jul 13 2026, 6:24 PM
sys/riscv/riscv/pmap.c
1151

Why exactly do we need this fence? Is it just copied from the SBI implementation?

1157

So above we invoked the callback on all harts except this one, waiting for them to finish, and now we're doing the invalidation on the current hart. Why not do them all in parallel?

bnovkov marked an inline comment as done.Jul 13 2026, 7:37 PM
bnovkov added inline comments.
sys/riscv/riscv/pmap.c
1151

The fence here follows the convention outlined in the comment above pmap_invalidate_page_sbi, which was introduced back in D17414.
I assume it's here because sfence_vmas that target an address only apply to the leaf PTEs, but I'm not sure.

1157

Right, that pattern only makes sense in the sbi_* variants where I assume we want to avoid calling into openSBI.
I'll include the original cpu in the mask as well.

bnovkov updated this revision to Diff 181986.Jul 14 2026, 6:48 PM

Avoid clearing CPU mask for range invalidation

Harbormaster completed remote builds in B74799: Diff 181986.Jul 14 2026, 6:48 PM
markj accepted this revision.Jul 15 2026, 2:01 PM

Did you microbenchmark the new implementation? I'd be curious to know, say, how many cycles it takes to execute 10000 back-to-back 4KB page invalidations using the old and new implementations respectively, when the target CPU mask is just the current CPU and when it's the set of all CPUs.

This revision is now accepted and ready to land.Jul 15 2026, 2:01 PM
Closed by commit rG99360212c739: riscv/pmap.c: Add an Svinval-aware variant of pmap_invalidate_range (authored by bnovkov). · Explain WhyAug 10 2026, 4:02 PM
This revision was automatically updated to reflect the committed changes.
bnovkov added a commit: rG99360212c739: riscv/pmap.c: Add an Svinval-aware variant of pmap_invalidate_range.
bnovkov added a comment.Aug 10 2026, 4:02 PM
In D57624#1336128, @markj wrote:

Did you microbenchmark the new implementation? I'd be curious to know, say, how many cycles it takes to execute 10000 back-to-back 4KB page invalidations using the old and new implementations respectively, when the target CPU mask is just the current CPU and when it's the set of all CPUs.

Unfortunately no, I haven't had the time to run a proper microbenchmark yet.

markj added a comment.Fri, Sep 25, 10:12 PM

There is an unfortunate bug in this change: the pm_active bitmask is indexed by hart IDs, not CPU IDs, and these are not the same in general, see e.g., the output from QEMU below:

FreeBSD/SMP: Multiprocessor System Detected: 16 CPUs                                                                                                            
CPU 1  : Vendor=Unspecified Core=Unknown (Hart 14)                                                                                                             
CPU 2  : Vendor=Unspecified Core=Unknown (Hart 15)                                                                                                                                                                                                                                                                            
CPU 3  : Vendor=Unspecified Core=Unknown (Hart 0)                                                                                                              
CPU 4  : Vendor=Unspecified Core=Unknown (Hart 1)                                                                                                              
CPU 5  : Vendor=Unspecified Core=Unknown (Hart 2)                                                                                                                                                                                                                                                                             
CPU 6  : Vendor=Unspecified Core=Unknown (Hart 3)                                                                                                              
CPU 7  : Vendor=Unspecified Core=Unknown (Hart 4)                                                                                                                                                                                                                                                                             
CPU 8  : Vendor=Unspecified Core=Unknown (Hart 5)                                                                                                              
CPU 9  : Vendor=Unspecified Core=Unknown (Hart 6)                                                                                                              
CPU 10 : Vendor=Unspecified Core=Unknown (Hart 7)                                                                                                              
CPU 11 : Vendor=Unspecified Core=Unknown (Hart 8)                                                                                                              
CPU 12 : Vendor=Unspecified Core=Unknown (Hart 9)                                                                                                              
CPU 13 : Vendor=Unspecified Core=Unknown (Hart 10)                                                                                                             
CPU 14 : Vendor=Unspecified Core=Unknown (Hart 11)                                                                                                             
CPU 15 : Vendor=Unspecified Core=Unknown (Hart 12)

So, when we pass pm_active as the target cpuset to smp_rendezvous_cpus(), we end up signaling an unrelated set of CPUs.

markj added a comment.Fri, Sep 25, 10:13 PM
In D57624#1377340, @markj wrote:

There is an unfortunate bug in this change: the pm_active bitmask is indexed by hart IDs, not CPU IDs.

This also means that pmap_activate_cpus() is broken, but that has relatively limited fallout.

bnovkov added a comment.Sun, Sep 27, 7:08 PM
In D57624#1377340, @markj wrote:

There is an unfortunate bug in this change: the pm_active bitmask is indexed by hart IDs, not CPU IDs, and these are not the same in general.
So, when we pass pm_active as the target cpuset to smp_rendezvous_cpus(), we end up signaling an unrelated set of CPUs.

Oof, that is a really unfortunate bug, thank you for catching this. D60072 should fix this.

Revision Contents
Changeset List

PathSize
sys/
riscv/
riscv/
73 lines

Diff 183783

View Options

sys/riscv/riscv/pmap.c

Loading...