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

[Lyude Paul]

On Sat, 2024-04-20 at 00:15 +0200, Thomas Gleixner wrote:
> Paul!

Lyude is fine BTW :P (I get the confusion though, Paul is usually not a
last name lol)

Anyway - unfortunately it doesn't seem like this patch helps :s, I'm
still not seeing any difference and the backtrace I'm seeing at early
boot looks the same. Any more information I can provide?

> 
> On Fri, Apr 19 2024 at 13:38, Lyude Paul wrote:
> > Awesome - can confirm the patch does indeed make the machine boot.
> > Full
> > dmesg from boot attached.
> 
> Thanks for providing the data.
> 
> [    0.089286] CPU topo: APIC ID 0 present 1
> [    0.089294] CPU topo: APIC ID 0 present 0
> [    0.089296] CPU topo: Hot-pluggable APIC ID 0 in present package.
> 
> ACPI is really a wonderland.
> 
> Can you please test the patch below?
> 
> Thanks,
> 
>         tglx
> ---
> Subject: x86/topology: Deal with more broken ACPI tables
> From: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Date: Thu, 18 Apr 2024 21:02:39 +0200
> 
> Paul reported a regression which waas caused by the handling of non-
> present
> CPUs in a present package. It's caused by the ACPI table on the
> system
> which advertises APICs twice, present and non-present:
> 
>   CPU topo: APIC ID 0 present 1
>   CPU topo: APIC ID 0 present 0
>   CPU topo: Hot-pluggable APIC ID 0 in present
> package.                                                             
>                                                                      
>                                        
> Which causes the topology to get confused to the point that it fails
> to
> bring the system up because the target APIC for the IOAPIC is not
> available.
> 
> Prevent this by checking whether a non-present CPU has been already
> registered as present before. If so emit a firmware warning and
> ignore the
> registration request.
> 
> Fixes: f0551af0213 ("x86/topology: Ignore non-present APIC IDs in a
> present package")
> Reported-by: Lyude Paul <lyude@xxxxxxxxxx>
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> ---
>  arch/x86/kernel/cpu/topology.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> --- a/arch/x86/kernel/cpu/topology.c
> +++ b/arch/x86/kernel/cpu/topology.c
> @@ -195,6 +195,12 @@ static __init void topo_register_apic(u3
>  	} else {
>  		u32 pkgid = topo_apicid(apic_id, TOPO_PKG_DOMAIN);
>  
> +		if (test_bit(apic_id, phys_cpu_present_map)) {
> +			pr_warn_once(FW_BUG "Already present APIC ID
> %x registered again as non-present\n",
> +				     apic_id);
> +			return;
> +		}
> +
>  		/*
>  		 * Check for present APICs in the same package when
> running
>  		 * on bare metal. Allow the bogosity in a guest.
> 

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