Page MenuHomeFreeBSD
Differential D33206

x86: Defer LAPIC calibration until after timecounters are available
ClosedPublic
Actions

Authored by markj on Nov 30 2021, 10:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 10, 11:08 AM
Unknown Object (File)
Mon, Mar 2, 6:14 PM
Unknown Object (File)
Mon, Mar 2, 4:27 PM
Unknown Object (File)
Feb 18 2026, 9:25 PM
Unknown Object (File)
Feb 17 2026, 1:57 PM
Unknown Object (File)
Feb 17 2026, 1:57 PM
Unknown Object (File)
Feb 17 2026, 11:22 AM
Unknown Object (File)
Feb 17 2026, 11:22 AM
View All Files
Subscribers
None

Details

Reviewers
jhb
kib
cperciva
imp
mav
Commits
rG62d09b46ad75: x86: Defer LAPIC calibration until after timecounters are available
Summary

This ensures that we have a good reference timecounter for performing
calibration.

Change lapic_setup to avoid configuring the timer when booting, and move
calibration and initial configuration to a new lapic routine,
lapic_calibrate_timer. This calibration will be initiated from
cpu_initclocks(), before an eventtimer is selected.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 43084
Build 39972: arc lint + arc unit

Event Timeline

markj requested review of this revision.Nov 30 2021, 10:01 PM
markj created this revision.
markj added a parent revision: D33205: x86: Deduplicate clock.h.
Harbormaster completed remote builds in B43072: Diff 99268.Nov 30 2021, 10:01 PM
jhb accepted this revision.Nov 30 2021, 10:52 PM

Is Colin going to work on refining the actual calibration to not always take a full second? At the very least the calibration should probably be using sbinuptime() as the TSC calibration does instead of assuming the DELAY() waits exactly for a second. Seems sensible to refine the calibration in a separate commit though.

This revision is now accepted and ready to land.Nov 30 2021, 10:52 PM
cperciva added a comment.Nov 30 2021, 11:00 PM

Yes, I asked Mark to get this into the tree first so I could handle the TSC and LAPIC calibration at the same time.

markj added a comment.Nov 30 2021, 11:13 PM
In D33206#750402, @jhb wrote:

At the very least the calibration should probably be using sbinuptime() as the TSC calibration does instead of assuming the DELAY() waits exactly for a second.

Note that the next patch removes the use of sbinuptime(), since at SI_SUB_CLOCKS/SI_ORDER_FIRST we don't have usable timehands.

cperciva added inline comments.Nov 30 2021, 11:33 PM
sys/x86/x86/local_apic.c
357

Not sure why this line is being deleted?

markj updated this revision to Diff 99273.Nov 30 2021, 11:35 PM
markj marked an inline comment as done.

Restore a lost comment.

This revision now requires review to proceed.Nov 30 2021, 11:35 PM
Harbormaster completed remote builds in B43074: Diff 99273.Nov 30 2021, 11:35 PM
kib added inline comments.Dec 1 2021, 7:31 AM
sys/x86/x86/local_apic.c
1007

I still want to see the herald line when we use deadline mode, in bootverbose dmesg.

1017–1018

Unless I misread the change, this function (and corresponding switch) is now only executed for BSP, while before it was performed for each CPU. This is perhaps fine, because lapic_et_start() would do whatever is needed to ignite the timecounter.

But then, do we need lapic_setup_timer() at all?

markj added a child revision: D33209: x86: Perform late TSC calibration before LAPIC timer calibration.Dec 1 2021, 12:45 PM
markj updated this revision to Diff 99286.Dec 1 2021, 1:23 PM
markj marked 2 inline comments as done.
  • Don't bother with timer setup during boot.
  • Restore a bootverbose message.
Harbormaster completed remote builds in B43082: Diff 99286.Dec 1 2021, 1:23 PM
markj added inline comments.Dec 1 2021, 1:23 PM
sys/x86/x86/local_apic.c
1017–1018

Hmm, I think calibrate_timer() does not need to call lapic_setup_timer() at all, indeed. Note that without the change, the body of lapic_setup_timer() is not executed at all during boot, as the timer mode is not yet initialized. (It was implicitly set to LAT_MODE_UNDEF before, and this change makes the initialization explicit.) The mode is UNDEF until the timer is started, and that will happen after calibration. So I can remove lapic_setup_timer() and restore the inline implementation in native_lapic_setup().

kib accepted this revision.Dec 1 2021, 1:26 PM
kib added inline comments.
sys/x86/x86/local_apic.c
924

I suggest to move intr_restore() right after lapic_calibrate_initcount().

This revision is now accepted and ready to land.Dec 1 2021, 1:26 PM
markj updated this revision to Diff 99291.Dec 1 2021, 1:59 PM
markj marked an inline comment as done.

Move a verbose printf out of an interrupts-disabled section.

This revision now requires review to proceed.Dec 1 2021, 1:59 PM
Harbormaster completed remote builds in B43084: Diff 99291.Dec 1 2021, 1:59 PM
This revision was not accepted when it landed; it landed in state Needs Review.Dec 6 2021, 4:23 PM
Closed by commit rG62d09b46ad75: x86: Defer LAPIC calibration until after timecounters are available (authored by markj). · Explain Why
This revision was automatically updated to reflect the committed changes.
markj added a commit: rG62d09b46ad75: x86: Defer LAPIC calibration until after timecounters are available.

Revision Contents
Changeset List

PathSize
sys/
x86/
include/
10 lines
1 line
isa/
4 lines
x86/
57 lines
xen/
7 lines

Diff 99291

View Options

sys/x86/include/apicvar.h

Loading...
View Options

sys/x86/include/clock.h

Loading...
View Options

sys/x86/isa/clock.c

Loading...
View Options

sys/x86/x86/local_apic.c

Loading...
View Options

sys/x86/xen/xen_apic.c

Loading...