cpu_auxmsr: assert caller is preventing CPU migration
ClosedPublic
Actions

Authored by adam_fenn.io on Aug 23 2020, 9:27 PM.

Details

Reviewers

kib
grehan

Commits

rS364656: cpu_auxmsr: assert caller is preventing CPU migration.

Test Plan

Peformed the following sanity-checks:

i386
- Confirmed assertion is not triggered during boot of a 4-CPU system (a bhyve VM).
amd64
- Confirmed assertion is not triggered during boot of a 4-CPU system (a VMWare VM).
- Confirmed assertion is not triggered in the VM entry/exit code path (used an invocation of dtrace -f fbt:kernel:cpu_auxmsr to confirm this routine was, as expected, being called while a bhyve VM was running).

The resume codepaths were not sanity-checked, but, by inspection, these appear equally clear as contexts with interrupts disabled as the cases that were sanity-checked.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 33127
Build 30489: arc lint + arc unit

Event Timeline

adam_fenn.io created this revision.Aug 23 2020, 9:27 PM

adam_fenn.io held this revision as a draft.

Harbormaster completed remote builds in B33123: Diff 76121.Aug 23 2020, 9:27 PM

adam_fenn.io requested review of this revision.Aug 23 2020, 9:30 PM

kib added inline comments.Aug 23 2020, 10:00 PM

sys/amd64/amd64/initcpu.c
224	I suggest to trim this sentence to just Caller should prevent CPU migration.
234	I am not sure the comment is useful. If somebody is using this in context that only prevents scheduling, e.g. in critical section, he should be qualified enough to change assert to accommodate new situation. Comment above the function gives enough clue, if needed.
236	Indent of contig lines should be +4 spaces, no alignment to opening braces. Also I believe the message could be made much shorter without loosing the information.

Incorporate @kib's review feedback.

Harbormaster completed remote builds in B33127: Diff 76127.Aug 23 2020, 10:36 PM

adam_fenn.io marked 2 inline comments as done.Aug 23 2020, 10:37 PM

kib added inline comments.Aug 23 2020, 10:55 PM

sys/amd64/amd64/initcpu.c
229	I suggest to write the message `Context switch possible` instead of `interrupts enabled` which just repeats the assert code. It is more to the point IMO.
230	Remove blank line.