Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c5e49ef29c50715f8fa5086fec999c0fa6d84dac.camel@redhat.com>
Subject: Re: [PATCH] Revert "x86/acpi/boot: Do not register processors that
 cannot be onlined for x2APIC"
From:   Lyude Paul <lyude@redhat.com>
To:     linux-acpi@vger.kernel.org
Cc:     Leo Duran <leo.duran@amd.com>,
        Kishon Vijay Abraham I <kvijayab@amd.com>,
        Borislav Petkov <bp@alien8.de>,
        Zhang Rui <rui.zhang@intel.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Len Brown <lenb@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@vger.kernel.org>
Date:   Fri, 31 Mar 2023 19:56:03 -0400
In-Reply-To: <20230331235328.4312-1-lyude@redhat.com>
References: <20230331235328.4312-1-lyude@redhat.com>
Organization: Red Hat Inc.
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2023-03-31 at 19:53 -0400, Lyude Paul wrote:
> This reverts commit e2869bd7af608c343988429ceb1c2fe99644a01f. This commit
> unfortunately seems to have resulted in one of my machines no longer
> booting. Specifically, this machine is a custom build with a MS-7A39/A320=
M
> GAMING PRO motherboard with firmware version v1.10. I'm not entirely sure
> of the cause yet, but starting it up with "earlycon=3Defifb keep_bootcon"=
 has
> informed me that the kernel panics like so:
>=20
> Call Trace:
>   <TASK>
>   dump_stack_lvl+0x33/0x46
>   panic+0x105/0x2b1
>   ? timer_irq_works+0x53/0xef
>   panic_if_irq_remap.cold+0x5/0x5
>   setup_IO_APIC+0x3c4/0x831
>   ? __pfx_native_io_apic_read+0x10/0x10
>   ? __ioapic_read_entry+0x34/0x50
>   ? _raw_spin_unlock_irqrestore+0x1b/0x40
>   ? clear_IO_APIC_pin+0x16b/0x240
>   apic_intr_mode_init+0x101/0x106
>   x86_late_time_init+0x20/0x34
>   start_kernel+0x8b4/0x95f
>   secondary_startup_64_no_verify+0x5e/0xeb
>   </TASK>
> ---[ end Kernel panic - not syncing: timer doesn't work through
>  interrupt-mapped IO-APIC ]---

Agh, I totally forgot to actually decode the stacktrace on this before send=
ing
it out. I can do that if anyone would think it would help, but I have a
feeling the stacktrace here isn't particularly useful in the first place
considering the culprit commit here.

As well, hopefully it goes without saying but: I'm happy to try any kind of
fixes or provide any more information from this machine. Just let me know =
=E2=99=A5

>=20
> My assumption is there's probably something funky with the firmware on th=
e
> machine seeing as it's a random gaming motherboard, but that also probabl=
y
> means there are other boards out there like this that are cold, afraid, a=
nd
> unable to boot. We could warm their hearts by reverting this, or maybe by
> figuring out a proper fix.
>=20
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Fixes: e2869bd7af60 ("x86/acpi/boot: Do not register processors that cann=
ot be onlined for x2APIC")
> Cc: Leo Duran <leo.duran@amd.com>
> Cc: Kishon Vijay Abraham I <kvijayab@amd.com>
> Cc: Borislav Petkov (AMD) <bp@alien8.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> ---
>  arch/x86/kernel/acpi/boot.c | 19 +++----------------
>  1 file changed, 3 insertions(+), 16 deletions(-)
>=20
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 1c38174b5f019..4177577c173bf 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -188,17 +188,6 @@ static int acpi_register_lapic(int id, u32 acpiid, u=
8 enabled)
>  	return cpu;
>  }
> =20
> -static bool __init acpi_is_processor_usable(u32 lapic_flags)
> -{
> -	if (lapic_flags & ACPI_MADT_ENABLED)
> -		return true;
> -
> -	if (acpi_support_online_capable && (lapic_flags & ACPI_MADT_ONLINE_CAPA=
BLE))
> -		return true;
> -
> -	return false;
> -}
> -
>  static int __init
>  acpi_parse_x2apic(union acpi_subtable_headers *header, const unsigned lo=
ng end)
>  {
> @@ -223,10 +212,6 @@ acpi_parse_x2apic(union acpi_subtable_headers *heade=
r, const unsigned long end)
>  	if (apic_id =3D=3D 0xffffffff)
>  		return 0;
> =20
> -	/* don't register processors that cannot be onlined */
> -	if (!acpi_is_processor_usable(processor->lapic_flags))
> -		return 0;
> -
>  	/*
>  	 * We need to register disabled CPU as well to permit
>  	 * counting disabled CPUs. This allows us to size
> @@ -265,7 +250,9 @@ acpi_parse_lapic(union acpi_subtable_headers * header=
, const unsigned long end)
>  		return 0;
> =20
>  	/* don't register processors that can not be onlined */
> -	if (!acpi_is_processor_usable(processor->lapic_flags))
> +	if (acpi_support_online_capable &&
> +	    !(processor->lapic_flags & ACPI_MADT_ENABLED) &&
> +	    !(processor->lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
>  		return 0;
> =20
>  	/*

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