Re: Early boot regression from f0551af0213 ("x86/topology: Ignore non-present APIC IDs in a present package")

[Lyude Paul <lyude@redhat.com>]

Awesome! This patch does seem to make the system boot, thank you for your help
<3

Tested-by: Lyude Paul <lyude@redhat.com>

On Thu, 2024-05-16 at 15:38 +0200, Thomas Gleixner wrote:
> Lyude!
> 
> On Wed, May 15 2024 at 19:15, Lyude Paul wrote:
> > On Tue, 2024-05-14 at 10:25 +0200, Thomas Gleixner wrote:
> > > 
> > > Which one of the debug patches did you use?
> > 
> > The one you sent on 4/18, when you also asked me for the output of
> > /sys/kernel/debug/x86/topo/
> > 
> > (I thought I remembered that patch not booting on previous kernels but maybe
> > I'm misremembering)
> 
> It booted with the -rc kernel too. :)
> 
> I found an interesting and probably related difference in the boot logs
> though. Up to 8 possible CPUs the kernel uses logical destination mode
> for the APIC. With more than 8 it uses physical destination mode.
> 
> Can you please test the patch below on top of 6.9 and validate that it
> boots w/o any magic command line parameter?
> 
> Thanks,
> 
>         tglx
> ---
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 66fd4b2a37a3..db5e93a7c194 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1883,6 +1883,8 @@ static inline void try_to_enable_x2apic(int remap_mode) { }
>  static inline void __x2apic_enable(void) { }
>  #endif /* !CONFIG_X86_X2APIC */
>  
> +int irq_remap_mode = -1;
> +
>  void __init enable_IR_x2apic(void)
>  {
>  	unsigned long flags;
> @@ -1915,6 +1917,8 @@ void __init enable_IR_x2apic(void)
>  
>  	if (ir_stat < 0)
>  		restore_ioapic_entries();
> +	else
> +		irq_remap_mode = ir_stat;
>  	legacy_pic->restore_mask();
>  	local_irq_restore(flags);
>  }
> diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
> index f37ad3392fec..5cc386db3557 100644
> --- a/arch/x86/kernel/apic/apic_flat_64.c
> +++ b/arch/x86/kernel/apic/apic_flat_64.c
> @@ -12,6 +12,7 @@
>  #include <linux/export.h>
>  #include <linux/acpi.h>
>  
> +#include <asm/irq_remapping.h>
>  #include <asm/jailhouse_para.h>
>  #include <asm/apic.h>
>  
> @@ -130,7 +131,8 @@ static int physflat_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
>  
>  static int physflat_probe(void)
>  {
> -	return apic == &apic_physflat || num_possible_cpus() > 8 || jailhouse_paravirt();
> +	return apic == &apic_physflat || irq_remap_mode == IRQ_REMAP_XAPIC_MODE ||
> +		num_possible_cpus() > 8 || jailhouse_paravirt();
>  }
>  
>  static struct apic apic_physflat __ro_after_init = {
> diff --git a/arch/x86/kernel/apic/local.h b/arch/x86/kernel/apic/local.h
> index 842fe28496be..f633ab6dfa9f 100644
> --- a/arch/x86/kernel/apic/local.h
> +++ b/arch/x86/kernel/apic/local.h
> @@ -22,6 +22,8 @@ void x2apic_send_IPI_allbutself(int vector);
>  void x2apic_send_IPI_self(int vector);
>  extern u32 x2apic_max_apicid;
>  
> +extern int irq_remap_mode;
> +
>  /* IPI */
>  
>  DECLARE_STATIC_KEY_FALSE(apic_use_ipi_shorthand);
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat