The following commit has been merged into the x86/urgent branch of tip:
Commit-ID: 1e1dd773644e9de88f54386f7147c1068375fc75
Gitweb: https://git.kernel.org/tip/1e1dd773644e9de88f54386f7147c1068375fc75
Author: Thomas Gleixner <[email protected]>
AuthorDate: Fri, 26 Apr 2024 00:30:36 +02:00
Committer: Borislav Petkov (AMD) <[email protected]>
CommitterDate: Mon, 29 Apr 2024 12:08:07 +02:00
x86/apic: Don't access the APIC when disabling X2APIC
With 'iommu=off' on the kernel command line and X2APIC enabled by the BIOS
the code which disables the X2APIC triggers an unchecked MSR access error:
RDMSR from 0x802 at rIP: 0xffffffff94079992 (native_apic_msr_read+0x12/0x50)
This is happens because default_acpi_madt_oem_check() selects an X2APIC
driver before the X2APIC is disabled.
When the X2APIC is disabled because interrupt remapping cannot be enabled
due to 'iommu=off' on the command line, x2apic_disable() invokes
apic_set_fixmap() which in turn tries to read the APIC ID. This triggers
the MSR warning because X2APIC is disabled, but the APIC driver is still
X2APIC based.
Prevent that by adding an argument to apic_set_fixmap() which makes the
APIC ID read out conditional and set it to false from the X2APIC disable
path. That's correct as the APIC ID has already been read out during early
discovery.
Fixes: d10a904435fa ("x86/apic: Consolidate boot_cpu_physical_apicid initialization sites")
Reported-by: Adrian Huang <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Borislav Petkov (AMD) <[email protected]>
Tested-by: Adrian Huang <[email protected]>
Cc: [email protected]
Link: https://lore.kernel.org/r/875xw5t6r7.ffs@tglx
---
arch/x86/kernel/apic/apic.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index c342c4a..b229648 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1771,7 +1771,7 @@ void x2apic_setup(void)
__x2apic_enable();
}
-static __init void apic_set_fixmap(void);
+static __init void apic_set_fixmap(bool read_apic);
static __init void x2apic_disable(void)
{
@@ -1793,7 +1793,12 @@ static __init void x2apic_disable(void)
}
__x2apic_disable();
- apic_set_fixmap();
+ /*
+ * Don't reread the APIC ID as it was already done from
+ * check_x2apic() and the apic driver still is a x2APIC variant,
+ * which fails to do the read after x2APIC was disabled.
+ */
+ apic_set_fixmap(false);
}
static __init void x2apic_enable(void)
@@ -2057,13 +2062,14 @@ void __init init_apic_mappings(void)
}
}
-static __init void apic_set_fixmap(void)
+static __init void apic_set_fixmap(bool read_apic)
{
set_fixmap_nocache(FIX_APIC_BASE, mp_lapic_addr);
apic_mmio_base = APIC_BASE;
apic_printk(APIC_VERBOSE, "mapped APIC to %16lx (%16lx)\n",
apic_mmio_base, mp_lapic_addr);
- apic_read_boot_cpu_id(false);
+ if (read_apic)
+ apic_read_boot_cpu_id(false);
}
void __init register_lapic_address(unsigned long address)
@@ -2073,7 +2079,7 @@ void __init register_lapic_address(unsigned long address)
mp_lapic_addr = address;
if (!x2apic_mode)
- apic_set_fixmap();
+ apic_set_fixmap(true);
}
/*
* tip-bot2 for Thomas Gleixner <[email protected]> wrote:
> - apic_set_fixmap();
> + /*
> + * Don't reread the APIC ID as it was already done from
> + * check_x2apic() and the apic driver still is a x2APIC variant,
> + * which fails to do the read after x2APIC was disabled.
> + */
> + apic_set_fixmap(false);
JFYI, I amended the commit with the fixlet below, to avoid the inevitable
followup trivial patch:
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index b229648b7a18..803dcfb0e346 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1795,7 +1795,7 @@ static __init void x2apic_disable(void)
__x2apic_disable();
/*
* Don't reread the APIC ID as it was already done from
- * check_x2apic() and the apic driver still is a x2APIC variant,
+ * check_x2apic() and the APIC driver still is a x2APIC variant,
* which fails to do the read after x2APIC was disabled.
*/
apic_set_fixmap(false);
Thanks,
Ingo