Page MenuHomeFreeBSD
Differential D17181

Use ifunc to resolve context switching mode on amd4.
ClosedPublic
Actions

Authored by kib on Sep 16 2018, 2:33 PM.
Tags
None
Referenced Files
F106161181: D17181.id48121.diff
Thu, Dec 26, 9:46 AM
F106127016: D17181.id48082.diff
Wed, Dec 25, 8:31 PM
Unknown Object (File)
Mon, Dec 9, 7:26 PM
Unknown Object (File)
Fri, Dec 6, 9:26 AM
Unknown Object (File)
Nov 17 2024, 12:33 AM
Unknown Object (File)
Oct 24 2024, 1:29 PM
Unknown Object (File)
Oct 24 2024, 1:29 PM
Unknown Object (File)
Oct 24 2024, 1:29 PM
View All 77 Files
Subscribers
emaste
imp

Details

Reviewers
alc
markj
jhb
Commits
rS338715: Use ifunc to resolve context switching mode on amd64.
rS338712: Calculate PTI, PCID and INVPCID modes earlier, before ifuncs are resolved.
rS338711: Make the PTI violation check to follow style of the SMAP check.
rS338699: Remove unneeded new line from the panic string.
Summary

Patch removes all checks for pti/pcid/invpcid from the context switch path. I verified this by looking at the generated code. The invpcid_works1 trick required inline attribute for pmap_activate_sw_pcid_pti() to work.

The change requires moving of the pti and pcid_enabled initialization before ifunc resolution.

Trap.c change is mostly unrelated and just puts the PTI trap check into the same style as the SMAP trap check, also removing excessive new line from the panic message.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib created this revision.Sep 16 2018, 2:33 PM
Herald added a subscriber: imp. · View Herald TranscriptSep 16 2018, 2:33 PM
Harbormaster completed remote builds in B19607: Diff 48081.Sep 16 2018, 2:33 PM
markj accepted this revision.Sep 16 2018, 3:28 PM
markj added inline comments.
sys/amd64/amd64/machdep.c
1583 ↗(On Diff #48081)

I'd add a comment explaining why we do this here.

sys/amd64/amd64/pmap.c
7466 ↗(On Diff #48081)

Perhaps make invpcid_works1 const as a further hint?

sys/amd64/amd64/trap.c
711 ↗(On Diff #48081)

Missing newline.

This revision is now accepted and ready to land.Sep 16 2018, 3:28 PM
kib updated this revision to Diff 48082.Sep 16 2018, 3:39 PM
kib marked 3 inline comments as done.

Handle Mark' notes.

This revision now requires review to proceed.Sep 16 2018, 3:39 PM
Harbormaster completed remote builds in B19608: Diff 48082.Sep 16 2018, 3:39 PM
markj accepted this revision.Sep 16 2018, 4:05 PM
This revision is now accepted and ready to land.Sep 16 2018, 4:05 PM
alc accepted this revision.Sep 16 2018, 6:23 PM
alc added inline comments.
sys/amd64/amd64/machdep.c
1585 ↗(On Diff #48082)

Rather than the also's and and's below, I would say it all up front: "Check for pti, pcid, and invpcid before ..."

1586 ↗(On Diff #48082)

"... select the implementation ..."

sys/amd64/amd64/pmap.c
7482 ↗(On Diff #48082)

I would say, "Explicitly", instead of "Manually". Then, add the word "automatically" to the next sentence, i.e., "... flushed automatically by ..."

This comment and the ones in the later functions have been deindented by a tab, so you might try reflowing the text.

7522 ↗(On Diff #48082)

"... used to handle an invalidate_all ..."

7524 ↗(On Diff #48082)

"The below sequence of operations has ..."

7526 ↗(On Diff #48082)

"... but the curpmap value has not yet been updated."

7527 ↗(On Diff #48082)

"... causes the invltlb ... handler, which is called between ..."

7528 ↗(On Diff #48082)

"... as a NOP ..."

7534 ↗(On Diff #48082)

"... and the IPI ..."

sys/amd64/amd64/trap.c
715 ↗(On Diff #48082)

Please insert a space into )==.

alc added a comment.Sep 16 2018, 6:27 PM

Do you plan to apply similar changes to the TLB invalidation code?

kib added a comment.Sep 16 2018, 6:36 PM
In D17181#366407, @alc wrote:

Do you plan to apply similar changes to the TLB invalidation code?

I will look at it certainly. We only need this for pmap_invalidate_XXX() functions, since IPI handlers already split.

Also I want to note D16736 which does the ifunc-ation for cache invalidation.

Closed by commit rS338699: Remove unneeded new line from the panic string. (authored by kib). · Explain WhySep 16 2018, 6:38 PM
This revision was automatically updated to reflect the committed changes.
kib reopened this revision.Sep 16 2018, 6:43 PM
kib marked 10 inline comments as done.
kib updated this revision to Diff 48085.Sep 16 2018, 6:46 PM

Updated diff after processing Alan' comments.

Harbormaster completed remote builds in B19610: Diff 48085.Sep 16 2018, 6:46 PM
markj accepted this revision.Sep 16 2018, 7:56 PM
This revision is now accepted and ready to land.Sep 16 2018, 7:56 PM
kib mentioned this in D17184: amd64: ifunc-ify pmap_invalidate_XXX() TLB invalidation functions..Sep 16 2018, 10:00 PM
alc accepted this revision.Sep 16 2018, 10:11 PM
Closed by commit rS338711: Make the PTI violation check to follow style of the SMAP check. (authored by kib). · Explain WhySep 17 2018, 2:59 PM
This revision was automatically updated to reflect the committed changes.
kib reopened this revision.Sep 17 2018, 3:05 PM
This revision was not accepted when it landed; it landed in state Needs Review.Sep 17 2018, 3:34 PM
Closed by commit rS338712: Calculate PTI, PCID and INVPCID modes earlier, before ifuncs are resolved. (authored by kib). · Explain Why
This revision was automatically updated to reflect the committed changes.
kib reopened this revision.Sep 17 2018, 3:36 PM
This revision was not accepted when it landed; it landed in state Needs Review.Sep 17 2018, 3:52 PM
Closed by commit rS338715: Use ifunc to resolve context switching mode on amd64. (authored by kib). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
sys/
amd64/
amd64/
17 lines

Diff 48113

View Options

head/sys/amd64/amd64/trap.c

Loading...