riscv/pmap.c: Don't pass hartid map to 'smp_rendezvous_cpus'
AcceptedPublic
Actions

Authored by bnovkov on Sun, Sep 27, 7:07 PM.

Details

Reviewers

markj

Group Reviewers

riscv

Summary

The pm_active bitmask is indexed by hart IDs which can differ from
CPU IDs. pmap_invalidate_range_svinval assumes that the map is
indexed by CPU IDs, which is wrong and causes remote TLB invalidations
on unrelated CPUs.

Fix this by adding a routine that converts a hart-indexed bitmask
to a CPU ID-indexed bitmask. While we're here, fix a similar issue
in pmap_active_cpus.

Fixes: 99360212c739 ("riscv/pmap.c: Add an Svinval-aware variant of pmap_invalidate_range")
Reported by: markj

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 77387
Build 74270: arc lint + arc unit

Event Timeline

bnovkov created this revision.Sun, Sep 27, 7:07 PM

Herald added a subscriber: imp. · View Herald TranscriptSun, Sep 27, 7:07 PM

bnovkov requested review of this revision.Sun, Sep 27, 7:07 PM

Harbormaster completed remote builds in B77387: Diff 187833.Sun, Sep 27, 7:07 PM

bnovkov mentioned this in D57624: riscv/pmap.c: Add an Svinval-aware variant of pmap_invalidate_range.Sun, Sep 27, 7:08 PM

I suspect that using pm_active to store hart IDs instead of CPU IDs is too clever and will cause more problems down the road. We should instead store CPU IDs there and make the SBI routines handle translation to hart IDs. I think it could be done more cheaply than with a CPU_FOREACH_ISSET loop, if that matters: translating between a cpuset and a hart ID set is just a rotation of the bitmap. (Right?)

I am ok with this as an intermediate step however.

sys/riscv/riscv/mp_machdep.c
123
sys/riscv/riscv/pmap.c
1171	This might be too expensive if the cpuset is large. Right now it is not (MAXCPU is 16 on riscv), but that may well change someday. I would add an XXX comment pointing this out.