[PATCH] x86/apic: Remove logical destination mode for 64-bit

[PATCH] x86/apic: Remove logical destination mode for 64-bit

[Thomas Gleixner]

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.

Signed-off-by: Thomas Gleixner <tglx@linutronix.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);

Re: [PATCH] x86/apic: Remove logical destination mode for 64-bit

[Rob]

* Thomas Gleixner (tglx@linutronix.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@linutronix.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

Re: [PATCH] x86/apic: Remove logical destination mode for 64-bit

[Borislav Petkov]

On Sun, Jul 28, 2024 at 01:06:10PM +0200, Thomas Gleixner 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.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/include/asm/apic.h         |    8 --
>  arch/x86/kernel/apic/apic_flat_64.c |  119 ++----------------------------------
>  2 files changed, 7 insertions(+), 120 deletions(-)

Boots on my Zen1 but then again, it never had those Zen1 client problems to
begin with...

Anyway

Tested-by: Borislav Petkov (AMD) <bp@alien8.de>

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/apic: Remove logical destination mode for 64-bit

[Christian Heusel]

[-- Attachment #1: Type: text/plain, Size: 1498 bytes --]

On 24/07/28 05:51PM, Rob wrote:
> * Thomas Gleixner (tglx@linutronix.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

As far as I can tell this patch did not get applied anywhere so far,
right? Or did I miss anything?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] x86/apic: Remove logical destination mode for 64-bit

[Thomas Gleixner]

On Fri, Aug 09 2024 at 11:37, Christian Heusel wrote:
> On 24/07/28 05:51PM, Rob wrote:
>> * Thomas Gleixner (tglx@linutronix.de) wrote:
>> > 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
>
> As far as I can tell this patch did not get applied anywhere so far,
> right? Or did I miss anything?

I forgot about it. Let me find it again and stick it into the tip tree.

Thanks,

        tglx

[tip: x86/apic] x86/apic: Remove logical destination mode for 64-bit

[tip-bot2 for Thomas Gleixner]

The following commit has been merged into the x86/apic branch of tip:

Commit-ID:     838ba7733e4e3a94a928e8d0a058de1811a58621
Gitweb:        https://git.kernel.org/tip/838ba7733e4e3a94a928e8d0a058de1811a58621
Author:        Thomas Gleixner <tglx@linutronix.de>
AuthorDate:    Sun, 28 Jul 2024 13:06:10 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Fri, 09 Aug 2024 14:34:16 +02:00

x86/apic: Remove logical destination mode for 64-bit

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 as reported by Rob and
Christian. The latter problem 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.

Reported-by: Rob Newcater <rob@durendal.co.uk>
Reported-by: Christian Heusel <christian@heusel.eu>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Borislav Petkov (AMD) <bp@alien8.de>
Tested-by: Rob Newcater <rob@durendal.co.uk>
Link: https://lore.kernel.org/all/877cd5u671.ffs@tglx

---
 arch/x86/include/asm/apic.h         |   8 +--
 arch/x86/kernel/apic/apic_flat_64.c | 119 +--------------------------
 2 files changed, 7 insertions(+), 120 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 9dd22ee..53f0844 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -350,20 +350,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[];
 
 /*
diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
index f37ad33..e0308d8 100644
--- 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_after_init = {
 	.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_after_init = {
 	.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);

Re: [PATCH] x86/apic: Remove logical destination mode for 64-bit

[Christian Heusel]

[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]

On 24/08/09 02:18PM, Thomas Gleixner wrote:
> On Fri, Aug 09 2024 at 11:37, Christian Heusel wrote:
> > On 24/07/28 05:51PM, Rob wrote:
> >> * Thomas Gleixner (tglx@linutronix.de) wrote:
> >> > 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
> >
> > As far as I can tell this patch did not get applied anywhere so far,
> > right? Or did I miss anything?
> 
> I forgot about it. Let me find it again and stick it into the tip tree.

I think the patch did not make it past the tip tree after that if I
checked correctly.

Now since it's summer people are maybe on vacation and I hope it got
stuck there because of cocktails on the beach and so on 😆🍹

In any case I wanted to send a reminder about it ..

Cheers,
Chris

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] x86/apic: Remove logical destination mode for 64-bit

[Borislav Petkov]

On Thu, Sep 05, 2024 at 03:28:47PM +0200, Christian Heusel wrote:
> Now since it's summer people are maybe on vacation and I hope it got
> stuck there because of cocktails on the beach and so on 😆🍹

Yep, check. :)

The patch is on its way to 6.12:

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/apic&id=838ba7733e4e3a94a928e8d0a058de1811a58621

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] x86/apic: Remove logical destination mode for 64-bit

[Linux regression tracking (Thorsten Leemhuis)]

On 05.09.24 16:04, Borislav Petkov wrote:
> On Thu, Sep 05, 2024 at 03:28:47PM +0200, Christian Heusel wrote:
>> Now since it's summer people are maybe on vacation and I hope it got
>> stuck there because of cocktails on the beach and so on 😆🍹
> 
> Yep, check. :)
> The patch is on its way to 6.12:
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/apic&id=838ba7733e4e3a94a928e8d0a058de1811a58621

Hmmm. Please help me out here: why was that fix queued for -next and not
for this cycle?

Was that patch when it was committed considered too dangerous for
mainlining this cycle (at this point of the cycle I guess it might)? I
mean, it's afaics (not totally sure here, the change is missing a Fixes:
tag as well as Closes: tags pointing to the report) fixing a regression
with f0551af02130 that Christian reported (see start of this thread, e.g.,
https://lore.kernel.org/all/12df8b45-6100-4c8b-b82a-a6a75bed2e05@heusel.eu/
). And f0551af02130 is from v6.9-rc1, so given what Linus wrote in
https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQUBQ@mail.gmail.com/
that fix should likely have been (or still should be?) merged in this
cycle, unless it's really dangerous.

Or did I misunderstood something here?

Ciao, Thorsten

P.S.: While at it:

#regzbot fix: x86/apic: Remove logical destination mode for 64-bit

Re: [PATCH] x86/apic: Remove logical destination mode for 64-bit

[Thomas Gleixner]

On Fri, Sep 06 2024 at 10:34, Linux regression tracking wrote:
> On 05.09.24 16:04, Borislav Petkov wrote:
>> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86/apic&id=838ba7733e4e3a94a928e8d0a058de1811a58621
>
> Hmmm. Please help me out here: why was that fix queued for -next and not
> for this cycle?
>
> Was that patch when it was committed considered too dangerous for
> mainlining this cycle (at this point of the cycle I guess it might)?

Yes.

> I mean, it's afaics (not totally sure here, the change is missing a Fixes:
> tag as well as Closes: tags pointing to the report) fixing a regression
> with f0551af02130 that Christian reported (see start of this thread, e.g.,
> https://lore.kernel.org/all/12df8b45-6100-4c8b-b82a-a6a75bed2e05@heusel.eu/
> ). And f0551af02130 is from v6.9-rc1, so given what Linus wrote in
> https://lore.kernel.org/all/CAHk-=wis_qQy4oDNynNKi5b7Qhosmxtoj1jxo5wmB6SRUwQUBQ@mail.gmail.com/
> that fix should likely have been (or still should be?) merged in this
> cycle, unless it's really dangerous.
>
> Or did I misunderstood something here?

I really wanted to cook it first. Aside of that f0551af02130 unearthed a
firmware bug as the reporters confirmed. So I didn't see an immediate
reason to send it to Linus.

My rationale for writing the patch was to avoid this issue in the future
for those who can't update firmware and have it in the next LTS release,
which is what distros will ship in their stable offerings.

Thanks,

        tglx