[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZqZ3Kp2NVctgstxs@vendhya2>
Date: Sun, 28 Jul 2024 17:51:54 +0100
From: Rob <rob@...endal.co.uk>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Christian Heusel <christian@...sel.eu>, Borislav Petkov <bp@...en8.de>,
regressions@...ts.linux.dev, x86@...nel.org,
Joerg Roedel <joro@...tes.org>, Tony Luck <tony.luck@...el.com>,
LKML <linux-kernel@...r.kernel.org>,
Paul Menzel <pmenzel@...gen.mpg.de>, Lyude Paul <lyude@...hat.com>
Subject: Re: [PATCH] x86/apic: Remove logical destination mode for 64-bit
* Thomas Gleixner (tglx@...utronix.de) wrote:
>Logical destination mode of the local APIC is used for systems with up to
>8 CPUs. It has an advantage over physical destination mode as it allows to
>target multiple CPUs at once with IPIs.
>
>That advantage was definitely worth it when systems with up to 8 CPUs
>were state of the art for servers and workstations, but that's history.
>
>Aside of that there are systems which fail to work with logical destination
>mode as the ACPI/DMI quirks show and there are AMD Zen1 systems out there
>which fail when interrupt remapping is enabled. The latter can be cured by
>firmware updates, but not all OEMs distribute the required changes.
>
>Physical destination mode is guaranteed to work because it is the only way
>to get a CPU up and running via the INIT/INIT/STARTUP sequence.
>
>As the number of CPUs keeps increasing, logical destination mode becomes a
>less used code path so there is no real good reason to keep it around.
>
>Therefore remove logical destination mode support for 64-bit and default to
>physical destination mode.
Thanks Chris for applying the patch for me.
Thomas - The patched kernel boots successfully. I held off updating the
BIOS so there can be no ambiguity.
Thanks,
Rob
>
>Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
>---
> arch/x86/include/asm/apic.h | 8 --
> arch/x86/kernel/apic/apic_flat_64.c | 119 ++----------------------------------
> 2 files changed, 7 insertions(+), 120 deletions(-)
>
>--- a/arch/x86/include/asm/apic.h
>+++ b/arch/x86/include/asm/apic.h
>@@ -345,20 +345,12 @@ extern struct apic *apic;
> * APIC drivers are probed based on how they are listed in the .apicdrivers
> * section. So the order is important and enforced by the ordering
> * of different apic driver files in the Makefile.
>- *
>- * For the files having two apic drivers, we use apic_drivers()
>- * to enforce the order with in them.
> */
> #define apic_driver(sym) \
> static const struct apic *__apicdrivers_##sym __used \
> __aligned(sizeof(struct apic *)) \
> __section(".apicdrivers") = { &sym }
>
>-#define apic_drivers(sym1, sym2) \
>- static struct apic *__apicdrivers_##sym1##sym2[2] __used \
>- __aligned(sizeof(struct apic *)) \
>- __section(".apicdrivers") = { &sym1, &sym2 }
>-
> extern struct apic *__apicdrivers[], *__apicdrivers_end[];
>
> /*
>--- a/arch/x86/kernel/apic/apic_flat_64.c
>+++ b/arch/x86/kernel/apic/apic_flat_64.c
>@@ -8,129 +8,25 @@
> * Martin Bligh, Andi Kleen, James Bottomley, John Stultz, and
> * James Cleverdon.
> */
>-#include <linux/cpumask.h>
> #include <linux/export.h>
>-#include <linux/acpi.h>
>
>-#include <asm/jailhouse_para.h>
> #include <asm/apic.h>
>
> #include "local.h"
>
>-static struct apic apic_physflat;
>-static struct apic apic_flat;
>-
>-struct apic *apic __ro_after_init = &apic_flat;
>-EXPORT_SYMBOL_GPL(apic);
>-
>-static int flat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
>-{
>- return 1;
>-}
>-
>-static void _flat_send_IPI_mask(unsigned long mask, int vector)
>-{
>- unsigned long flags;
>-
>- local_irq_save(flags);
>- __default_send_IPI_dest_field(mask, vector, APIC_DEST_LOGICAL);
>- local_irq_restore(flags);
>-}
>-
>-static void flat_send_IPI_mask(const struct cpumask *cpumask, int vector)
>-{
>- unsigned long mask = cpumask_bits(cpumask)[0];
>-
>- _flat_send_IPI_mask(mask, vector);
>-}
>-
>-static void
>-flat_send_IPI_mask_allbutself(const struct cpumask *cpumask, int vector)
>-{
>- unsigned long mask = cpumask_bits(cpumask)[0];
>- int cpu = smp_processor_id();
>-
>- if (cpu < BITS_PER_LONG)
>- __clear_bit(cpu, &mask);
>-
>- _flat_send_IPI_mask(mask, vector);
>-}
>-
>-static u32 flat_get_apic_id(u32 x)
>+static u32 physflat_get_apic_id(u32 x)
> {
> return (x >> 24) & 0xFF;
> }
>
>-static int flat_probe(void)
>+static int physflat_probe(void)
> {
> return 1;
> }
>
>-static struct apic apic_flat __ro_after_init = {
>- .name = "flat",
>- .probe = flat_probe,
>- .acpi_madt_oem_check = flat_acpi_madt_oem_check,
>-
>- .dest_mode_logical = true,
>-
>- .disable_esr = 0,
>-
>- .init_apic_ldr = default_init_apic_ldr,
>- .cpu_present_to_apicid = default_cpu_present_to_apicid,
>-
>- .max_apic_id = 0xFE,
>- .get_apic_id = flat_get_apic_id,
>-
>- .calc_dest_apicid = apic_flat_calc_apicid,
>-
>- .send_IPI = default_send_IPI_single,
>- .send_IPI_mask = flat_send_IPI_mask,
>- .send_IPI_mask_allbutself = flat_send_IPI_mask_allbutself,
>- .send_IPI_allbutself = default_send_IPI_allbutself,
>- .send_IPI_all = default_send_IPI_all,
>- .send_IPI_self = default_send_IPI_self,
>- .nmi_to_offline_cpu = true,
>-
>- .read = native_apic_mem_read,
>- .write = native_apic_mem_write,
>- .eoi = native_apic_mem_eoi,
>- .icr_read = native_apic_icr_read,
>- .icr_write = native_apic_icr_write,
>- .wait_icr_idle = apic_mem_wait_icr_idle,
>- .safe_wait_icr_idle = apic_mem_wait_icr_idle_timeout,
>-};
>-
>-/*
>- * Physflat mode is used when there are more than 8 CPUs on a system.
>- * We cannot use logical delivery in this case because the mask
>- * overflows, so use physical mode.
>- */
> static int physflat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
> {
>-#ifdef CONFIG_ACPI
>- /*
>- * Quirk: some x86_64 machines can only use physical APIC mode
>- * regardless of how many processors are present (x86_64 ES7000
>- * is an example).
>- */
>- if (acpi_gbl_FADT.header.revision >= FADT2_REVISION_ID &&
>- (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL)) {
>- printk(KERN_DEBUG "system APIC only can use physical flat");
>- return 1;
>- }
>-
>- if (!strncmp(oem_id, "IBM", 3) && !strncmp(oem_table_id, "EXA", 3)) {
>- printk(KERN_DEBUG "IBM Summit detected, will use apic physical");
>- return 1;
>- }
>-#endif
>-
>- return 0;
>-}
>-
>-static int physflat_probe(void)
>-{
>- return apic == &apic_physflat || num_possible_cpus() > 8 || jailhouse_paravirt();
>+ return 1;
> }
>
> static struct apic apic_physflat __ro_after_init = {
>@@ -146,7 +42,7 @@ static struct apic apic_physflat __ro_af
> .cpu_present_to_apicid = default_cpu_present_to_apicid,
>
> .max_apic_id = 0xFE,
>- .get_apic_id = flat_get_apic_id,
>+ .get_apic_id = physflat_get_apic_id,
>
> .calc_dest_apicid = apic_default_calc_apicid,
>
>@@ -166,8 +62,7 @@ static struct apic apic_physflat __ro_af
> .wait_icr_idle = apic_mem_wait_icr_idle,
> .safe_wait_icr_idle = apic_mem_wait_icr_idle_timeout,
> };
>+apic_driver(apic_physflat);
>
>-/*
>- * We need to check for physflat first, so this order is important.
>- */
>-apic_drivers(apic_physflat, apic_flat);
>+struct apic *apic __ro_after_init = &apic_physflat;
>+EXPORT_SYMBOL_GPL(apic);
--
Rob Newcater
Powered by blists - more mailing lists