x86: disallow APIC IDs over 255 on AMD hardware

[emaste]

Summary

Lack of an AMD IOMMU driver means we cannot successfully route interrupts to APIC IDs over 255.

See also commits fa5f94140a83 and 4258eb5a0d97.

[emaste]

@cperciva I believe this is a committable version of the hack I sent you.

[markj]

sys/x86/x86/mp_x86.c

156

A default of -1 is more natural. Otherwise you can't use this tunable to limit interrupts to only APIC 0. (Not that you'd want to do that probably, but the limitation doesn't make sense.)

[emaste]

use -1 for unlimited as suggested by markj

[jhb]

sys/x86/x86/mp_x86.c

266

I'd almost rather the implementation somehow be that the default value is 255 always unless an IOMMU driver raises it. That is, even on Intel you'd want this if DMAR was disabled or not present, etc. I'm not quite sure how to reasonably structure that. It would be nice perhaps if the IOMMU drivers had a chance to set apic_id_limit in the -1 case, and that after whatever hook/SYSINIT does that, a subsequent one on x86 would apply this logic to use 255.

I also think there's a suitable constant (MAX_APIC_ID or some such) that can be used in place of 255?

[jhb]

sys/x86/x86/mp_x86.c

158

This name is a bit ambiguous as we aren't disabling CPUs with high APIC IDs from functioning as CPUs (executing threads, etc.) just servicing interrupts. Ideally something about interrupts or "intr" would be in the name and description.

266

Hmm, we don't actually permit large APIC IDs until dmar0 attaches which is well after set_interrupt_apic_ids.

We should probably be initializing the IOMMU much earlier based on the table similar to how MADT works rather than a new-bus driver hung off of acpi0, but that's not something to fix easily. At the least this deserves a giant XXX comment about this being an expedient but not very correct hack.

[emaste]

sys/x86/x86/mp_x86.c

266

OK, will update the comment. I want to make a minimal change to get these systems to boot/function; we need to implement the AMD IOMMU either way and can look at those sort of changes as part of that work.

266

Maybe IOAPIC_MAX_ID which is xAPIC_MAX_APIC_ID which is 0xfe.

[emaste]

jhb suggestions

[cperciva]

I tested the version of this patch which I downloaded at 18:19 UTC, and it works in EC2 -- at least, it works with boot_verbose turned off. With boot_verbose="YES" we get a "spin lock ... (ap boot) held ... too long" panic because bringing up lots of CPUs verbosely over a slow serial port takes too long, but that's not anything APIC-related.

[kib]

Then add a bump if DMAR IR is enabled?

[emaste]

Use 255 not IOAPIC_MAX_ID which isn't the proper limit

[emaste]

Then add a bump if DMAR IR is enabled?

This is the issue @jhb was getting at, we should initialize the driver early so that we can default to 255 and increase once DMAR IR / AMD equivalent is functional. But I don't know enough to make those changes. Or, do you mean we should be able to do this with the current initialization ordering?

[kib]

In D41618#948775, @emaste wrote:

Then add a bump if DMAR IR is enabled?

This is the issue @jhb was getting at, we should initialize the driver early so that we can default to 255 and increase once DMAR IR / AMD equivalent is functional. But I don't know enough to make those changes. Or, do you mean we should be able to do this with the current initialization ordering?

There is already a function in x86/intr_machdep.c that is called when DMAR IR is ready, it is intr_reprogram(). It is needed to ensure that DMAR remap entries are created for already configured interrupts. On the other hand, it does not re-balance interrupts over cores, so if interrupts were allocated for low APICs, they would stay there until re-rebalanced, which is off by default, it seems.

[jhb]

In D41618#948778, @kib wrote:

In D41618#948775, @emaste wrote:

Then add a bump if DMAR IR is enabled?

This is the issue @jhb was getting at, we should initialize the driver early so that we can default to 255 and increase once DMAR IR / AMD equivalent is functional. But I don't know enough to make those changes. Or, do you mean we should be able to do this with the current initialization ordering?

There is already a function in x86/intr_machdep.c that is called when DMAR IR is ready, it is intr_reprogram(). It is needed to ensure that DMAR remap entries are created for already configured interrupts. On the other hand, it does not re-balance interrupts over cores, so if interrupts were allocated for low APICs, they would stay there until re-rebalanced, which is off by default, it seems.

I think that sort of change is probably something to do later. Possibly it would work for Ed's case since dmar0 is off of acpi0 and hopefully attached before PCI devices are attached.

However, that change also requires further work as you'd have to now go in and add in more CPUs via intr_add_cpu and that conflicts a bit with the earlier hack to disable domain-specific CPUs for interrupts if we have domains without any valid CPUs.

sys/x86/x86/mp_x86.c

278

Hmm, xAPIC_MAX_APIC_ID is indeed the right limit. 255 is the broadcast APIC ID (xAPIC_ID_ALL), so you can't use it for MSI or I/O APIC pins either. I think if you fix that I'm fine with it though. This comment is good enough for now so that we can eventually fix this so that that IOMMUs increase the limit at runtime.

[emaste]

Use xAPIC_MAX_APIC_ID. IOAPIC_MAX_ID had the right value, but the wrong name.