Page MenuHomeFreeBSD

amd64: be more precise when enabling the AlderLake small core PCID workaround
ClosedPublic

Authored by kib on Jan 4 2023, 5:20 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sep 28 2024, 6:08 PM
Unknown Object (File)
Sep 24 2024, 6:37 AM
Unknown Object (File)
Sep 17 2024, 7:18 AM
Unknown Object (File)
Sep 14 2024, 8:20 AM
Unknown Object (File)
Aug 31 2024, 1:45 AM
Unknown Object (File)
Aug 29 2024, 6:28 PM
Unknown Object (File)
Aug 26 2024, 12:25 PM
Unknown Object (File)
Aug 18 2024, 2:40 AM
Subscribers

Details

Summary

In particular, do not enable the workaround if INVPCID is not supported by the core.

Reported by: "Chen, Alvin W" <Weike.Chen@Dell.com>

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Jan 4 2023, 5:20 AM
sys/amd64/amd64/initcpu.c
264

pmap_pcid_invlpg_workaround_uena is updated from a tunable after this point, so we always apply the workaround on the BSP. Is the BSP always a big core?

kib marked an inline comment as done.Jan 4 2023, 10:22 AM
kib added inline comments.
sys/amd64/amd64/initcpu.c
264

So far yes, but I believe it is not for JasperLake. This is what the report was about.

kib marked an inline comment as done.

Read the tunable before initializing BSP pcpu small core bools.

markj added inline comments.
sys/amd64/amd64/machdep.c
1349

"uena" means "user-enabled"? It defaults to 1 so this name seems wrong.

This revision is now accepted and ready to land.Jan 4 2023, 3:11 PM
kib marked an inline comment as done.Jan 4 2023, 3:17 PM
kib added inline comments.
sys/amd64/amd64/machdep.c
1349

Why?

If you have a suggestion, I can rename the variable. But it reflects the user setting, if any. 0 is to disable workaround.

sys/amd64/amd64/machdep.c
1349

It seems wrong because a flag named "user-enabled" sounds like it means, "did the system user set this flag?" and the default value used by the kernel is "yes".

I cannot really see why pmap_pcid_invlpg_workaround_uena is even needed, so I'm not sure what the right name is. It looks like pmap_pcid_invlpg_workaround should default to 1, and be overridden by the vm.pmap.pmap_pcid_invlpg_workaround tunable if it is set.

kib marked an inline comment as done.Jan 5 2023, 2:43 AM
kib added inline comments.
sys/amd64/amd64/machdep.c
1349

It was _udis initially.
But regardless of that, I need tri-state. The sysctl must report the actual state, which is calculated by visiting all CPUs from initializecpu(), where we take the user (uena/udis) value into the account. If there are no small cores but user enabled the workaround (even implicitly), with single var I cannot properly report that the workaround is disabled.