Felix Held submitted this change.

View Change

Approvals: Matt DeVillier: Looks good to me, approved build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Nico Huber: Looks good to me, but someone else must approve
arch/x86/ioapic: always write IOAPIC ID in set_ioapic_id

Back in the days of the APIC bus, the IOAPIC IDs mustn't overlap with
the LAPIC IDs (0 to CONFIG_MAX_CPUS - 1), but since the IOAPIC and LAPIC
nowadays talk to each other via the system bus, an IOAPIC ID of 0 is
valid. When set_ioapic_id gets called with an IOAPIC ID of 0, it skipped
writing the IOAPIC ID to the corresponding IOAPIC register, so the code
was relying of the register having the expected default value of the
IOAPIC IO 0 for things to work as expected. The case of the IOAPIC ID
being 0 is the most common case in coreboot, since that's what
register_new_ioapic_gsi0 will end up doing. Fix this issue by not making
the io_apic_write call conditional on ioapic_id being non-zero. The only
southbridge that doesn't call register_new_ioapic_gsi0, calls
set_ioapic_id with the IOAPIC ID 2 for which this won't cause any
changes in behavior.

Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: Ic8538f82a6b10f16eeb228669db197dc8e326ffd
Reviewed-on: https://review.coreboot.org/c/coreboot/+/80330
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Matt DeVillier <matt.devillier@amd.corp-partner.google.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
---
M src/arch/x86/ioapic.c
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/arch/x86/ioapic.c b/src/arch/x86/ioapic.c
index 04c852b..df97a50 100644
--- a/src/arch/x86/ioapic.c
+++ b/src/arch/x86/ioapic.c
@@ -131,12 +131,8 @@
ioapic_base);
printk(BIOS_DEBUG, "IOAPIC: ID = 0x%02x\n", ioapic_id);

- if (ioapic_id) {
- /* Set IOAPIC ID if it has been specified. */
- io_apic_write(ioapic_base, 0x00,
- (io_apic_read(ioapic_base, 0x00) & 0xf0ffffff) |
- (ioapic_id << 24));
- }
+ io_apic_write(ioapic_base, 0x00,
+ (io_apic_read(ioapic_base, 0x00) & 0xf0ffffff) | (ioapic_id << 24));

printk(BIOS_SPEW, "IOAPIC: Dumping registers\n");
for (i = 0; i < 3; i++)

To view, visit change 80330. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: Ic8538f82a6b10f16eeb228669db197dc8e326ffd
Gerrit-Change-Number: 80330
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Held <felix-coreboot@felixheld.de>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Felix Held <felix-coreboot@felixheld.de>
Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella@intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier@amd.corp-partner.google.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged