Details

Reviewers

jhb
jeff

Commits

rS359093: Do not spuriously re-enable disabled io_apic pin on EOI for some configurations.
rS359092: Stop (trying to) renumber io apics.
rS359090: Widen the stored io_apic_id to 8 bits.

Summary

There are actually three changes:

Widen the stored io_apic_id to 8 bits. It seems that the newer Intel chipset did that, and Linux reads 8 bits. The only detail is that all seen datasheets, even under NDA, claim that io apic id is 4 bits. (Submitted by jeff).
When matching PCIe IOAPIC device against MADT-enumerated IOAPICs, compare io_apic_id from BAR against io_apic_id read from MADT register page before renumbering. It seems that PCIe vs. MADT pointed registers are not coherent.
If EOI suppression is supported but reported ioapic version is so old that it does not has EOI register, fix Intel trick of eoi-ing by flipping pin type (edge/level) to account for the disabled pin. (Reported by: Juniper)

Items 1 and 2 tested by flo.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 29871

Event Timeline

kib created this revision.Mar 4 2020, 10:16 PM

Herald added a subscriber: imp. · View Herald TranscriptMar 4 2020, 10:16 PM

Harbormaster completed remote builds in B29791: Diff 69191.Mar 4 2020, 10:16 PM

As requested the verbose dmesg.boot with this patch applied.

rx4770m5.dmesg.boot.D23965492 KBDownload

More debugging.

Harbormaster completed remote builds in B29803: Diff 69215.Mar 5 2020, 2:32 PM

Print old value for apic id, not the new value written to it from MADT.

Harbormaster completed remote builds in B29805: Diff 69220.Mar 5 2020, 3:03 PM

Match id read from PCI BAR against old apic id before renumbering. It seems that PCI BAR might be not an alias for the MADT registers page.

Harbormaster completed remote builds in B29806: Diff 69224.Mar 5 2020, 4:07 PM

kib retitled this revision from x86: IO APICs on newer big Intel machines. to x86: Support IO APICs on newer big Intel machines and hypervisors..Mar 6 2020, 1:42 PM

kib edited the summary of this revision. (Show Details)

kib added reviewers: jhb, jeff.

jhb added inline comments.Mar 6 2020, 6:59 PM

sys/x86/x86/io_apic.c
1021	So this seems really odd. It should be the same register such that if we actually renumber, it should change in both places? Perhaps what is happening is that the write isn't working at all? Do you have output showing the MADT addresses and the BAR0 addresses for all of the I/O APIC devices and the ID's read from each one?
1029	This just prints the same `apic_id` twice?

kib added inline comments.Mar 6 2020, 7:34 PM

sys/x86/x86/io_apic.c
1021	BAR and MADT base yes, ids no. That said, I now asking myself why do we need to change the apic id at all ? Should we just leave the id as is and use just io_old_apic_id always.
1029	Will fix.

kib marked an inline comment as done.Mar 6 2020, 7:36 PM

kib added inline comments.Mar 7 2020, 10:22 PM

sys/x86/x86/io_apic.c
1021	After looking at more datasheets, I found the following description of the IOAPICID for the recent Intel CPU: APICID: APICID This register uniquely identifies an APIC in the system. This register is not used by OS'es anymore and is still implemented in hardware because of FUD. I think this supports idea of stopping trying to renumber IOAPICs.

jhb added inline comments.Mar 9 2020, 8:46 PM

sys/x86/x86/io_apic.c
1021	Yes, the renumbering only ever makes sense if you have systems with an actual APIC BUS (I think pre-PPro?) We do try to use it here to match devices, but mismatching isn't really fatal as the only role of this PCI device is to quiet it. It used to matter more before PCI devices implicitly reserved their BARs, but we could probably just assume mismatches won't ever happen and stub out the attach routine.

kib added inline comments.Mar 9 2020, 9:08 PM

sys/x86/x86/io_apic.c
1021	I think even with APIC bus (actually pre-PIV) it is only important to have unique APIC IDs assigned, so that the arbitration work. But, matching PCI devices to APICs is very important to have interrupt remapping working. The originator of the IOAPIC interrupt message is used by DMAR to select the table, and originator is named by the PCI device id. I will proceed with removing the renumbering.

Stop renumbering (writing to the IOAPIC ID register).

Harbormaster completed remote builds in B29855: Diff 69338.Mar 9 2020, 10:24 PM

jhb added inline comments.Mar 10 2020, 4:19 PM

sys/x86/x86/io_apic.c
1021	To be clear, I think the renumbering to match the table is how you ensure unique APIC IDs for systems with an APIC bus. However, I don't think I've ever seen a boot that actually renumbered anything. I suspect it was just paranoia in the early (3.x) SMP code as presumably the BIOS has to assign unique IDs as part of POST. I guess if you had a BIOS that didn't start the APs at all during POST and the OS had to do the raw bootstrap you might need to renumber, but those systems are probably very ancient at this point and not worth supporting.
1041	I think this should be io_hw_apic_id?

Use correct apic_id for comparision to warn about mismatch.

Harbormaster completed remote builds in B29871: Diff 69372.Mar 10 2020, 7:45 PM

kib marked an inline comment as done.Mar 10 2020, 7:45 PM

jhb accepted this revision.Mar 10 2020, 9:53 PM

This revision is now accepted and ready to land.Mar 10 2020, 9:53 PM

kib added a subscriber: pho.Mar 10 2020, 10:10 PM

I booted with this on:

CPU: Intel(R) Xeon(R) CPU E5-2620 0 @ 2.00GHz (1995.24-MHz K8-class CPU)
CPU: Intel(R) Xeon(R) CPU E5-1650 v3 @ 3.50GHz (3500.07-MHz K8-class CPU)
CPU: AMD Phenom(tm) 9150e Quad-Core Processor (1800.02-MHz 686-class CPU)

No ioapic warnings seen.

Closed by commit rS359090: Widen the stored io_apic_id to 8 bits. (authored by kib). · Explain WhyMar 18 2020, 9:24 PM

This revision was automatically updated to reflect the committed changes.

kib added a commit: rS359090: Widen the stored io_apic_id to 8 bits..

kib added a commit: rS359092: Stop (trying to) renumber io apics..Mar 18 2020, 9:31 PM

kib added a commit: rS359093: Do not spuriously re-enable disabled io_apic pin on EOI for some configurations..Mar 18 2020, 9:35 PM