Bug 263124

Summary: sys/x86/isa/clock.c: lapic_calibrate_timer() in cpu_initclocks() breaks compiling for non-SMP
Product: Base System Reporter: Martin Birgmeier <d8zNeCFG>
Component: kernAssignee: Mark Johnston <markj>
Status: Closed FIXED    
Severity: Affects Only Me CC: jhb, markj
Priority: ---    
Version: 13.1-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
config file for non-SMP build none

Description Martin Birgmeier 2022-04-07 16:52:08 UTC 
Created attachment 233034 [details]
config file for non-SMP build

Scenario:
- FreeBSD 13.0 latest
- Checking out releng/13.1 (= RC1) into /usr/src
- Building a non-SMP kernel with a custom config previously used for 13.0 ("XYZZY")

Result:
- The build fails when linking the kernel with

linking kernel.full
ld: error: undefined symbol: apic_ops
>>> referenced by apicvar.h:386 (/net/hal/z/SRC/FreeBSD/src/MBi/releng/13.1/sys/x86/include/apicvar.h:386)
>>>               clock.o:(cpu_initclocks)
*** Error code 1

Expected result:
- The build should succeed even without "options SMP" and "device apic" being defined in the kernel configuration file

Note: Diffing sys/x86/isa/clock.c between 13.0 and 13.1 yields:

68a69
> #include <x86/apicvar.h>
413a415,417
> 
>       tsc_calibrate();
>       lapic_calibrate_timer();
427a432,433
>       tsc_calibrate();
>       lapic_calibrate_timer();
457c463
<     CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_NEEDGIANT,
---
>     CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_MPSAFE,

It seems the calls to lapic_calibrate_timer() should be made conditional on the SMP case.

The config file is attached.

-- Martin
Comment 1 John Baldwin freebsd_committer freebsd_triage 2022-04-07 18:10:42 UTC 
Probably the code in clock.c needs to be conditional on DEV_APIC (device apic on i386 sets this, and it is always on for amd64).

For your system you really want 'device apic' I suspect even if you don't want 'options SMP' as you get less IRQ sharing when using an I/O APIC in place of the old 8259A PICs even on a UP system.

Also, at this point removing 'options SMP' doesn't really buy you anything as we always compile the LOCK prefix in for atomic operations now.
Comment 2 Martin Birgmeier 2022-04-07 18:21:54 UTC 
Thank you for the quick reply.

I am considering moving to SMP kernels for these old systems.

Feel free to close this issue if it is of no further use.

-- Martin
Comment 3 Mark Johnston freebsd_committer freebsd_triage 2022-04-07 19:16:39 UTC 
https://reviews.freebsd.org/D34830
Comment 4 commit-hook freebsd_committer freebsd_triage 2022-04-08 16:10:49 UTC 
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=aa597d4049ffee69d413ea2154f4b312ffbaf646

commit aa597d4049ffee69d413ea2154f4b312ffbaf646
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-04-08 15:47:52 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-04-08 15:47:52 +0000

    i386: Fix the nodevice apic build

    PR:             263124
    Fixes:          62d09b46ad75 ("x86: Defer LAPIC calibration until after timecounters are available")
    Reviewed by:    kib, jhb, emaste
    MFC after:      3 days
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D34830

 sys/x86/isa/clock.c | 9 +++++++++
 1 file changed, 9 insertions(+)
Comment 5 commit-hook freebsd_committer freebsd_triage 2022-04-11 13:44:25 UTC 
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=85e36b3c9c003ba37156720091b3b385f07fb9d9

commit 85e36b3c9c003ba37156720091b3b385f07fb9d9
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-04-08 15:47:52 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-04-11 13:43:27 +0000

    i386: Fix the nodevice apic build

    PR:             263124
    Fixes:          62d09b46ad75 ("x86: Defer LAPIC calibration until after timecounters are available")
    Reviewed by:    kib, jhb, emaste
    Sponsored by:   The FreeBSD Foundation

    (cherry picked from commit aa597d4049ffee69d413ea2154f4b312ffbaf646)

 sys/x86/isa/clock.c | 9 +++++++++
 1 file changed, 9 insertions(+)
Comment 6 commit-hook freebsd_committer freebsd_triage 2022-04-11 14:11:31 UTC 
A commit in branch releng/13.1 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=21a2bfcafbff2a6bd3b29c5e6e75d35bdb1fb959

commit 21a2bfcafbff2a6bd3b29c5e6e75d35bdb1fb959
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2022-04-08 15:47:52 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-04-11 13:59:15 +0000

    i386: Fix the nodevice apic build

    Approved by:    re (gjb)
    PR:             263124
    Fixes:          62d09b46ad75 ("x86: Defer LAPIC calibration until after timecounters are available")
    Reviewed by:    kib, jhb, emaste
    Sponsored by:   The FreeBSD Foundation

    (cherry picked from commit aa597d4049ffee69d413ea2154f4b312ffbaf646)
    (cherry picked from commit 4f659ce4daf82ac3335abf3aab7181f2ac90a4cd)

 sys/x86/isa/clock.c | 9 +++++++++
 1 file changed, 9 insertions(+)