Page MenuHomeFreeBSD

amd64: fix INVLPGB range invalidation
ClosedPublic

Authored by kevans on Fri, Apr 17, 1:24 AM.
Tags
None
Referenced Files
F153897981: D56458.diff
Fri, Apr 24, 3:14 PM
Unknown Object (File)
Thu, Apr 23, 8:45 PM
Unknown Object (File)
Thu, Apr 23, 2:17 AM
Unknown Object (File)
Thu, Apr 23, 1:01 AM
Unknown Object (File)
Wed, Apr 22, 11:13 AM
Unknown Object (File)
Tue, Apr 21, 5:16 AM
Unknown Object (File)
Mon, Apr 20, 4:28 AM
Unknown Object (File)
Sun, Apr 19, 5:51 PM
Subscribers

Details

Summary

AMD64 Architecture Programmer's Manual Volume 3 says the following:

ECX[15:0] contains a count of the number of sequential pages to
invalidate in addition to the original virtual address, starting from
the virtual address specified in rAX. A count of 0 invalidates a
single page. ECX[31]=0 indicates to increment the virtual address at
the 4K boundary. ECX[31]=1 indicates to increment the virtual address
at the 2M boundary. The maximum count supported is reported in
CPUID function 8000_0008h, EDX[15:0].

ECX[31] being what we call INVLPGB_2M_CNT, signaling to increment the
VA by 2M.

This instruction invalidates the TLB entry or entries, regardless of
the page size (4 Kbytes, 2 Mbytes, 4 Mbytes, or 1 Gbyte). [...]

Combined with this, my interpretation of the current code is: if
<va> is aligned on a PDE boundary, we'll use INVLPGB_2M_CNT to try and
invalidate <cnt> PDEs with a single call, but that only works if <va> is
the start of at least <cnt> 2M pages. Otherwise, if <va> or any of the
subsequent PDEs isn't actually a superpage, then we would actually only
invalidate the *first* page within the PDE before skipping to the next
PDE, leaving the remainder of the 4K pages in between as they were.

The implication would seem to be that we would need to inspect the range
that we're trying to invalidate if we're planning on using
INVLPGB_2M_CNT at all, so this patch just simplifies it to a series of
4K invalidations. My gut feeling is that we likely still come out on
top vs. the TLB shootdown we're avoiding.

This seems to explain some issues we've seen lately with fdgrowtable()
and kqueue on recent Zen4/Zen5 EPYC hardware, where we'd experience
corruption that we can't explain.

PR: 293382

Diff Detail

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

Event Timeline

To be clear: I'm definitely not asserting that my interpretation of the language here is correct. In particular, I'm not confident that the second paragraph quoted means that it will only invalidate a 4K page if <va> isn't a superpage; I think the verbiage about 2M increment is clear, albeit a little verbose.

kib added inline comments.
sys/amd64/amd64/mp_machdep.c
743

I think a one-line comment that we always do page increments because '....' is due there.

This revision is now accepted and ready to land.Fri, Apr 17, 2:09 AM
sys/amd64/amd64/mp_machdep.c
743

My proposal to keep it at 80 columns:

/* 4K increments because these may not be superpages. */

Presumably that's a good enough hint to look at the commit message if you want more detail

sys/amd64/amd64/mp_machdep.c
743

Sounds good.

I'm going to plan to commit this within the next day or so. I think linux's use of "stride" instead of size or shift for their naming of the bit is very telling, and their general range invalidation function seems to specifically just use PTE stride as well.

I'm going to plan to commit this within the next day or so. I think linux's use of "stride" instead of size or shift for their naming of the bit is very telling, and their general range invalidation function seems to specifically just use PTE stride as well.

Yes, the implementation of invlpgb_kernel_range_flush provides compelling evidence.

This revision was automatically updated to reflect the committed changes.