Page MenuHomeFreeBSD

amd64: Set GSBASE before calling init_secondary()
ClosedPublic

Authored by markj on Jul 28 2021, 6:00 PM.

Details

Summary

The KMSAN instrumentation requires some thread-local storage to maintain
state for function parameters and return values. This buffer is checked
as part of each function's prologue. It is provided by the KMSAN
runtime, which looks up a pointer in the current thread's structure.

init_secondary() is instrumented, but it runs without a valid %gs
descriptor for some time, so the runtime cannot safely access curthread.
To work around this, load GSBASE before calling init_secondary(). Then
the runtime can check curthread == NULL and simply return a pointer to
some dummy storage.

Also change init_secondary() to initialize kernelGSBASE to 0. I don't
see any reason why we have to set it to the pcpu pointer.

Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 40730
Build 37619: arc lint + arc unit

Event Timeline

markj requested review of this revision.Jul 28 2021, 6:00 PM
sys/amd64/amd64/mpboot.S
263

I prefer to not put the conditional code there. I do not see a harm from doing this wrmsr unconditionally, and removing corresponding wrmsr from init_secondary().

sys/x86/x86/mp_x86.c
95 ↗(On Diff #92895)

The var is amd64-specific, am I right?

Why not put it into something amd64-specific then? At least, the definition.
As I understand, there is no useful amd64-only header.

sys/amd64/amd64/mpboot.S
263

Ok, but the wrmsr in init_secondary() cannot be removed. It seems that loading a selector into %gs causes the base register to be cleared, so we must write MSR_GSBASE again after calling lgdt().

sys/x86/x86/mp_x86.c
95 ↗(On Diff #92895)

Yes, it is amd64-specific, I just grouped it with bootSTK and bootAP.

Alternately, I can modify i386 to use bootpcpu, so amd64 and i386 maintain a bit of consistency. Which do you prefer?

sys/amd64/amd64/mpboot.S
263

Ok, perhaps expand this comment to say what you noted above.

sys/x86/x86/mp_x86.c
95 ↗(On Diff #92895)

For i386, I think the policy should be to not touching it unless required.

  • Make bootpcpu amd64-only.
  • Load GS.base unconditionally in entry_64.
This revision is now accepted and ready to land.Jul 28 2021, 8:11 PM