bogus MADT X2APIC entry [Re: illegal instruction in kernel at boot]

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Thu, Sep 26, 2024 at 05:48:30PM +0200, Manuel Bouyer wrote:
> > [...]
> 
> I made some progress.  Actually it's in aesni_probe, and more specifically
> when it calls fpu_area_restore() from fpu_kern_enter().
> cr4 is 0x678 and xsave_feature is 0x2e7 at fpu_area_restore() entry
> so CR4_OSFXSR is there.
> I tried disabling FPU_SAVE_XSAVEOPT, keeping only FPU_SAVE_XSAVE but this
> didn't help. Disabling XSAVE completely avoids the illegal instruction
> (which means that fxrstor is working) but a NULL function pointer
> cause a panic at a later point.
> 
> I suspect that, for this CPU some more bits needs to be enabled in one of the
> config registers, or our fxsavel is called with inappropriate arguments
> (x86_xsave_feature would have a wrong value). I'm not familiar with x86 FPU
> so some help is welcome

It turns out that no CPUs are detected, so cpu_init() is not called and the
FPU is not initialised.
CPUs are skipped because of this in mpacpi_config_cpu():
               /* ACPI spec: "Logical processors with APIC ID values
                * less than 255 must use the Processor Local APIC
                * structure to convey their APIC information to OSPM."
                * But Xen with PVH dom0 breaks this ACPI spec.
                */
               if (x2apic->LocalApicId <= 0xff && vm_guest != VM_GUEST_XENPVH) {
                       printf("bogus MADT X2APIC entry (id = 0x%"PRIx32")\n",
                           x2apic->LocalApicId);
                       break;
               }

Removing this (see attached patch) makes the kernel boot up to starting init.

I think I've seen a discussion about this not long ago but I couldn't
find it in the archives. Is this check still relevant ?
Should I commit the attached patch ?
It may help for kern/57737 too.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: arch/x86/x86/mpacpi.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/mpacpi.c,v
retrieving revision 1.109.4.1
diff -u -p -u -r1.109.4.1 mpacpi.c
--- arch/x86/x86/mpacpi.c	30 Mar 2023 11:43:17 -0000	1.109.4.1
+++ arch/x86/x86/mpacpi.c	28 Sep 2024 15:17:47 -0000
@@ -393,17 +393,6 @@ mpacpi_config_cpu(ACPI_SUBTABLE_HEADER *
 	case ACPI_MADT_TYPE_LOCAL_X2APIC:
 		x2apic = (ACPI_MADT_LOCAL_X2APIC *)hdrp;
 
-		/* ACPI spec: "Logical processors with APIC ID values
-		 * less than 255 must use the Processor Local APIC
-		 * structure to convey their APIC information to OSPM."
-		 * But Xen with PVH dom0 breaks this ACPI spec.
-		 */
-		if (x2apic->LocalApicId <= 0xff && vm_guest != VM_GUEST_XENPVH) {
-			printf("bogus MADT X2APIC entry (id = 0x%"PRIx32")\n",
-			    x2apic->LocalApicId);
-			break;
-		}
-
 		if (x2apic->LapicFlags & ACPI_MADT_ENABLED) {
 			if (x2apic->LocalApicId != cpunum)
 				caa.cpu_role = CPU_ROLE_AP;