The recent change to ignore MADT entries with invalid APIC IDs also
affected logging of MADT entries. That's not desired [1] [2], so restore
the old behavior.

Fixes: 47342d8f490c ("x86/ACPI: Ignore entries with invalid APIC IDs when parsing MADT")
Link: https://lore.kernel.org/xen-devel/0bd3583c-a55d-9a68-55b1-c383499d46d8@suse.com/ # [1]
Link: https://lore.kernel.org/xen-devel/f780d40e-c828-c57a-b19c-16ee15c1454a@suse.com/ # [2]
Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>
---
 xen/arch/x86/acpi/boot.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/acpi/boot.c b/xen/arch/x86/acpi/boot.c
index ead41bd535..170f9783c5 100644
--- a/xen/arch/x86/acpi/boot.c
+++ b/xen/arch/x86/acpi/boot.c
@@ -87,21 +87,22 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 	if (BAD_MADT_ENTRY(processor, end))
 		return -EINVAL;
 
-	/* Ignore entries with invalid x2APIC ID */
-	if (processor->local_apic_id == 0xffffffff)
-		return 0;
-
 	/* Don't register processors that cannot be onlined. */
 	if (madt_revision >= 5 &&
 	    !(processor->lapic_flags & ACPI_MADT_ENABLED) &&
 	    !(processor->lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
 		return 0;
 
-	if ((processor->lapic_flags & ACPI_MADT_ENABLED) || opt_cpu_info) {
+	if ((processor->lapic_flags & ACPI_MADT_ENABLED) ||
+	    processor->local_apic_id != 0xffffffff || opt_cpu_info) {
 		acpi_table_print_madt_entry(header);
 		log = true;
 	}
 
+	/* Ignore entries with invalid x2APIC ID */
+	if (processor->local_apic_id == 0xffffffff)
+		return 0;
+
 	/* Record local apic id only when enabled and fitting. */
 	if (processor->local_apic_id >= MAX_APICS ||
 	    processor->uid >= MAX_MADT_ENTRIES) {
@@ -146,19 +147,20 @@ acpi_parse_lapic(struct acpi_subtable_header * header, const unsigned long end)
 	if (BAD_MADT_ENTRY(processor, end))
 		return -EINVAL;
 
-	/* Ignore entries with invalid APIC ID */
-	if (processor->id == 0xff)
-		return 0;
-
 	/* Don't register processors that cannot be onlined. */
 	if (madt_revision >= 5 &&
 	    !(processor->lapic_flags & ACPI_MADT_ENABLED) &&
 	    !(processor->lapic_flags & ACPI_MADT_ONLINE_CAPABLE))
 		return 0;
 
-	if ((processor->lapic_flags & ACPI_MADT_ENABLED) || opt_cpu_info)
+	if ((processor->lapic_flags & ACPI_MADT_ENABLED) ||
+	    processor->id != 0xff || opt_cpu_info)
 		acpi_table_print_madt_entry(header);
 
+	/* Ignore entries with invalid APIC ID */
+	if (processor->id == 0xff)
+		return 0;
+
 	/* Record local apic id only when enabled */
 	if (processor->lapic_flags & ACPI_MADT_ENABLED) {
 		x86_acpiid_to_apicid[processor->processor_id] = processor->id;
-- 
2.40.1

On 13.09.2023 10:13, Simon Gaiser wrote:
> The recent change to ignore MADT entries with invalid APIC IDs also
> affected logging of MADT entries. That's not desired [1] [2], so restore
> the old behavior.
> 
> Fixes: 47342d8f490c ("x86/ACPI: Ignore entries with invalid APIC IDs when parsing MADT")
> Link: https://lore.kernel.org/xen-devel/0bd3583c-a55d-9a68-55b1-c383499d46d8@suse.com/ # [1]
> Link: https://lore.kernel.org/xen-devel/f780d40e-c828-c57a-b19c-16ee15c1454a@suse.com/ # [2]
> Signed-off-by: Simon Gaiser <simon@invisiblethingslab.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with the implied ack assuming that the support status of physical hotplug
is going to be changed, and hence a revert of the earlier commit isn't
going to be necessary. Stefano - what's the status of your respective
patch to SUPPORT.md (which probably wants re-sending as the root of a new
thread anyway)?

Jan