[HALX86] acpi/madt.c: Rewrite it (#6032)

Especially HalpParseApicTables() which looked early-WIP and was buggy.
And keep smp/mps/mps.c in sync'.

hal/halx86/acpi/madt.c

@@ -3,6 +3,7 @@
  * LICENSE:     GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
  * PURPOSE:     Source File for MADT Table parsing
  * COPYRIGHT:   Copyright 2021 Justin Miller <justinmiller100@gmail.com>
+ *              Copyright 2023 Serge Gautherie <reactos-git_serge_171003@gautherie.fr>
  */
 
 /* INCLUDES *******************************************************************/
@@ -12,77 +13,250 @@
 /* ACPI_BIOS_ERROR defined in acoutput.h and bugcodes.h */
 #undef ACPI_BIOS_ERROR
 #include <smp.h>
+
 #define NDEBUG
 #include <debug.h>
 
+// See HalpParseApicTables(). Only enable this to local-debug it.
+// That needs, for example, to test-call the function later or to use the "FrLdrDbgPrint" hack.
+#if DBG && 0
+    #define DPRINT01        DPRINT1
+    #define DPRINT00        DPRINT
+#else
+#if defined(_MSC_VER)
+    #define DPRINT01        __noop
+    #define DPRINT00        __noop
+#else
+    #define DPRINT01(...)   do { if(0) { DbgPrint(__VA_ARGS__); } } while(0)
+    #define DPRINT00(...)   do { if(0) { DbgPrint(__VA_ARGS__); } } while(0)
+#endif // _MSC_VER
+#endif // DBG && 0
+
 /* GLOBALS ********************************************************************/
 
-PROCESSOR_IDENTITY HalpStaticProcessorIdentity[MAXIMUM_PROCESSORS] = {{0}};
-PPROCESSOR_IDENTITY HalpProcessorIdentity = NULL;
 HALP_APIC_INFO_TABLE HalpApicInfoTable;
-ACPI_TABLE_MADT *MadtTable;
+
-ACPI_SUBTABLE_HEADER *AcpiHeader;
+// ACPI_MADT_LOCAL_APIC.LapicFlags masks
-ACPI_MADT_LOCAL_APIC *LocalApic;
+#define LAPIC_FLAG_ENABLED          0x00000001
+#define LAPIC_FLAG_ONLINE_CAPABLE   0x00000002
+// Bits 2-31 are reserved.
+
+static PROCESSOR_IDENTITY HalpStaticProcessorIdentity[MAXIMUM_PROCESSORS];
+const PPROCESSOR_IDENTITY HalpProcessorIdentity = HalpStaticProcessorIdentity;
+
+#if 0
+extern ULONG HalpPicVectorRedirect[16];
+#endif
 
 /* FUNCTIONS ******************************************************************/
 
+// Note: HalpParseApicTables() is called early, so its DPRINT*() do nothing.
 VOID
 HalpParseApicTables(
     _In_ PLOADER_PARAMETER_BLOCK LoaderBlock)
 {
+    ACPI_TABLE_MADT *MadtTable;
+    ACPI_SUBTABLE_HEADER *AcpiHeader;
     ULONG_PTR TableEnd;
-    ULONG ValidProcessorCount;
 
-    /* We only support legacy APIC for now, this will be updated in the future */
+    MadtTable = HalAcpiGetTable(LoaderBlock, APIC_SIGNATURE);
-    HalpApicInfoTable.ApicMode = 0x10;
+    if (!MadtTable)
-    MadtTable = HalAcpiGetTable(LoaderBlock, 'CIPA');
-
-    AcpiHeader = (ACPI_SUBTABLE_HEADER*)MadtTable;
-    AcpiHeader->Length = sizeof(ACPI_TABLE_MADT);
-    TableEnd = (ULONG_PTR)MadtTable + MadtTable->Header.Length;
-
-    HalpApicInfoTable.ProcessorCount = 0;
-    HalpProcessorIdentity = HalpStaticProcessorIdentity;
-
-    AcpiHeader = (ACPI_SUBTABLE_HEADER*)MadtTable;
-    AcpiHeader->Length = sizeof(ACPI_TABLE_MADT);
-    TableEnd = (ULONG_PTR)MadtTable + MadtTable->Header.Length;
-
-    while ((ULONG_PTR)AcpiHeader <= TableEnd)
     {
-        LocalApic = (ACPI_MADT_LOCAL_APIC*)AcpiHeader;
+        DPRINT01("MADT table not found\n");
+        return;
+    }
 
-        if (LocalApic->Header.Type == ACPI_MADT_TYPE_LOCAL_APIC &&
+    if (MadtTable->Header.Length < sizeof(*MadtTable))
-            LocalApic->Header.Length == sizeof(ACPI_MADT_LOCAL_APIC))
     {
-            ValidProcessorCount = HalpApicInfoTable.ProcessorCount;
+        DPRINT01("Length is too short: %p, %u\n", MadtTable, MadtTable->Header.Length);
+        return;
+    }
 
-            HalpProcessorIdentity[ValidProcessorCount].LapicId = LocalApic->Id;
+    DPRINT00("MADT table: Address %08X, Flags %08X\n", MadtTable->Address, MadtTable->Flags);
-            HalpProcessorIdentity[ValidProcessorCount].ProcessorId = LocalApic->ProcessorId;
+
+#if 1
+
+    // TODO: We support only legacy APIC for now
+    HalpApicInfoTable.ApicMode = HALP_APIC_MODE_LEGACY;
+    // TODO: What about 'MadtTable->Flags & ACPI_MADT_PCAT_COMPAT'?
+
+#else // TODO: Is that correct?
+
+    if ((MadtTable->Flags & ACPI_MADT_PCAT_COMPAT) == ACPI_MADT_DUAL_PIC)
+    {
+        HalpApicInfoTable.ApicMode = HALP_APIC_MODE_LEGACY;
+    }
+    else // if ((MadtTable->Flags & ACPI_MADT_PCAT_COMPAT) == ACPI_MADT_MULTIPLE_APIC)
+    {
+#if 1
+        DPRINT01("ACPI_MADT_MULTIPLE_APIC support is UNIMPLEMENTED\n");
+        return;
+#else
+        HalpApicInfoTable.ApicMode = HALP_APIC_MODE_xyz;
+#endif
+    }
+
+#endif
+
+    HalpApicInfoTable.LocalApicPA = MadtTable->Address;
+
+    AcpiHeader = (ACPI_SUBTABLE_HEADER *)((ULONG_PTR)MadtTable + sizeof(*MadtTable));
+    TableEnd = (ULONG_PTR)MadtTable + MadtTable->Header.Length;
+    DPRINT00(" MadtTable %p, subtables %p - %p\n", MadtTable, AcpiHeader, (PVOID)TableEnd);
+
+    while ((ULONG_PTR)(AcpiHeader + 1) <= TableEnd)
+    {
+        if (AcpiHeader->Length < sizeof(*AcpiHeader))
+        {
+            DPRINT01("Length is too short: %p, %u\n", AcpiHeader, AcpiHeader->Length);
+            return;
+        }
+
+        if ((ULONG_PTR)AcpiHeader + AcpiHeader->Length > TableEnd)
+        {
+            DPRINT01("Length mismatch: %p, %u, %p\n",
+                     AcpiHeader, AcpiHeader->Length, (PVOID)TableEnd);
+            return;
+        }
+
+        switch (AcpiHeader->Type)
+        {
+            case ACPI_MADT_TYPE_LOCAL_APIC:
+            {
+                ACPI_MADT_LOCAL_APIC *LocalApic = (ACPI_MADT_LOCAL_APIC *)AcpiHeader;
+
+                if (AcpiHeader->Length != sizeof(*LocalApic))
+                {
+                    DPRINT01("Type/Length mismatch: %p, %u\n", AcpiHeader, AcpiHeader->Length);
+                    return;
+                }
+
+                DPRINT00(" Local Apic, Processor %lu: ProcessorId %u, Id %u, LapicFlags %08X\n",
+                         HalpApicInfoTable.ProcessorCount,
+                         LocalApic->ProcessorId, LocalApic->Id, LocalApic->LapicFlags);
+
+                if (!(LocalApic->LapicFlags & (LAPIC_FLAG_ONLINE_CAPABLE | LAPIC_FLAG_ENABLED)))
+                {
+                    DPRINT00("  Ignored: unusable\n");
+                    break;
+                }
+
+                if (HalpApicInfoTable.ProcessorCount == _countof(HalpStaticProcessorIdentity))
+                {
+                    DPRINT00("  Skipped: array is full\n");
+                    // We assume ignoring this processor is acceptable, until proven otherwise.
+                    break;
+                }
+
+                // Note: ProcessorId and Id are not validated in any way (yet).
+                HalpProcessorIdentity[HalpApicInfoTable.ProcessorCount].ProcessorId =
+                    LocalApic->ProcessorId;
+                HalpProcessorIdentity[HalpApicInfoTable.ProcessorCount].LapicId = LocalApic->Id;
 
                 HalpApicInfoTable.ProcessorCount++;
 
+                break;
+            }
+            case ACPI_MADT_TYPE_IO_APIC:
+            {
+                ACPI_MADT_IO_APIC *IoApic = (ACPI_MADT_IO_APIC *)AcpiHeader;
+
+                if (AcpiHeader->Length != sizeof(*IoApic))
+                {
+                    DPRINT01("Type/Length mismatch: %p, %u\n", AcpiHeader, AcpiHeader->Length);
+                    return;
+                }
+
+                DPRINT00(" Io Apic: Id %u, Address %08X, GlobalIrqBase %08X\n",
+                         IoApic->Id, IoApic->Address, IoApic->GlobalIrqBase);
+
+                // Ensure HalpApicInfoTable.IOAPICCount consistency.
+                if (HalpApicInfoTable.IoApicPA[IoApic->Id] != 0)
+                {
+                    DPRINT01("Id duplication: %p, %u\n", IoApic, IoApic->Id);
+                    return;
+                }
+
+                // Note: Address and GlobalIrqBase are not validated in any way (yet).
+                HalpApicInfoTable.IoApicPA[IoApic->Id] = IoApic->Address;
+                HalpApicInfoTable.IoApicIrqBase[IoApic->Id] = IoApic->GlobalIrqBase;
+
+                HalpApicInfoTable.IOAPICCount++;
+
+                break;
+            }
+            case ACPI_MADT_TYPE_INTERRUPT_OVERRIDE:
+            {
+                ACPI_MADT_INTERRUPT_OVERRIDE *InterruptOverride =
+                    (ACPI_MADT_INTERRUPT_OVERRIDE *)AcpiHeader;
+
+                if (AcpiHeader->Length != sizeof(*InterruptOverride))
+                {
+                    DPRINT01("Type/Length mismatch: %p, %u\n", AcpiHeader, AcpiHeader->Length);
+                    return;
+                }
+
+                DPRINT00(" Interrupt Override: Bus %u, SourceIrq %u, GlobalIrq %08X, IntiFlags %04X / UNIMPLEMENTED\n",
+                         InterruptOverride->Bus, InterruptOverride->SourceIrq,
+                         InterruptOverride->GlobalIrq, InterruptOverride->IntiFlags);
+
+                if (InterruptOverride->Bus != 0) // 0 = ISA
+                {
+                    DPRINT01("Invalid Bus: %p, %u\n", InterruptOverride, InterruptOverride->Bus);
+                    return;
+                }
+
+#if 1
+                // TODO: Implement it.
+#else // TODO: Is that correct?
+                if (InterruptOverride->SourceIrq > _countof(HalpPicVectorRedirect))
+                {
+                    DPRINT01("Invalid SourceIrq: %p, %u\n",
+                             InterruptOverride, InterruptOverride->SourceIrq);
+                    return;
+                }
+
+                // Note: GlobalIrq is not validated in any way (yet).
+                HalpPicVectorRedirect[InterruptOverride->SourceIrq] = InterruptOverride->GlobalIrq;
+                // TODO: What about 'InterruptOverride->IntiFlags'?
+#endif
+
+                break;
+            }
+            default:
+            {
+                DPRINT01(" UNIMPLEMENTED: Type %u, Length %u\n",
+                         AcpiHeader->Type, AcpiHeader->Length);
+                return;
+            }
+        }
+
         AcpiHeader = (ACPI_SUBTABLE_HEADER *)((ULONG_PTR)AcpiHeader + AcpiHeader->Length);
     }
-        else
+
+    if ((ULONG_PTR)AcpiHeader != TableEnd)
     {
-            /* End the parsing early if we don't use the currently selected table */
+        DPRINT01("Length mismatch: %p, %p, %p\n", MadtTable, AcpiHeader, (PVOID)TableEnd);
-            AcpiHeader = (ACPI_SUBTABLE_HEADER*)((ULONG_PTR)AcpiHeader + 1);
+        return;
-        }
     }
 }
 
 VOID
 HalpPrintApicTables(VOID)
 {
-    UINT32 i;
+#if DBG
+    ULONG i;
 
-    DPRINT1("HAL has detected a physical processor count of: %d\n", HalpApicInfoTable.ProcessorCount);
+    DPRINT1("Physical processor count: %lu\n", HalpApicInfoTable.ProcessorCount);
     for (i = 0; i < HalpApicInfoTable.ProcessorCount; i++)
     {
-        DPRINT1("Information about the following processor is for processors number: %d\n"
+        DPRINT1(" Processor %lu: ProcessorId %u, LapicId %u, ProcessorStarted %u, BSPCheck %u, ProcessorPrcb %p\n",
-                "   The BSPCheck is set to: %X\n"
+                i,
-                "   The LapicID is set to: %X\n",
+                HalpProcessorIdentity[i].ProcessorId,
-                i, HalpProcessorIdentity[i].BSPCheck, HalpProcessorIdentity[i].LapicId);
+                HalpProcessorIdentity[i].LapicId,
+                HalpProcessorIdentity[i].ProcessorStarted,
+                HalpProcessorIdentity[i].BSPCheck,
+                HalpProcessorIdentity[i].ProcessorPrcb);
     }
+#endif
 }

hal/halx86/include/smp.h

@@ -15,22 +15,25 @@ typedef struct _PROCESSOR_IDENTITY
     BOOLEAN ProcessorStarted;
     BOOLEAN BSPCheck;
     PKPRCB ProcessorPrcb;
-
 } PROCESSOR_IDENTITY, *PPROCESSOR_IDENTITY;
 
 /* This table is counter of the overall APIC constants acquired from madt */
+#define HALP_APIC_INFO_TABLE_IOAPIC_NUMBER 256 // ACPI_MADT_IO_APIC.Id is a UINT8.
 typedef struct _HALP_APIC_INFO_TABLE
 {
     ULONG ApicMode;
     ULONG ProcessorCount; /* Count of all physical cores, This includes BSP */
     ULONG IOAPICCount;
     ULONG LocalApicPA;                // The 32-bit physical address at which each processor can access its local interrupt controller
-    ULONG IoApicVA[256];
+    ULONG IoApicVA[HALP_APIC_INFO_TABLE_IOAPIC_NUMBER];
-    ULONG IoApicPA[256];
+    ULONG IoApicPA[HALP_APIC_INFO_TABLE_IOAPIC_NUMBER];
-    ULONG IoApicIrqBase[256]; // Global system interrupt base
+    ULONG IoApicIrqBase[HALP_APIC_INFO_TABLE_IOAPIC_NUMBER]; // Global system interrupt base
-
 } HALP_APIC_INFO_TABLE, *PHALP_APIC_INFO_TABLE;
 
+/* HALP_APIC_INFO_TABLE.ApicMode values */
+// TODO: What are the other modes/values?
+#define HALP_APIC_MODE_LEGACY 0x00000010
+
 VOID
 HalpParseApicTables(
     _In_ PLOADER_PARAMETER_BLOCK LoaderBlock);

hal/halx86/smp/mps/mps.c

@@ -9,14 +9,17 @@
 
 #include <hal.h>
 #include <smp.h>
+
 #define NDEBUG
 #include <debug.h>
 
 /* GLOBALS ********************************************************************/
 
-PROCESSOR_IDENTITY HalpStaticProcessorIdentity[MAXIMUM_PROCESSORS] = {{0}};
+static // TODO: While HalpParseApicTables() is UNIMPLEMENTED.
-PPROCESSOR_IDENTITY HalpProcessorIdentity = NULL;
+ULONG PhysicalProcessorCount;
-UINT32 PhysicalProcessorCount = 0;
+
+static PROCESSOR_IDENTITY HalpStaticProcessorIdentity[MAXIMUM_PROCESSORS];
+const PPROCESSOR_IDENTITY HalpProcessorIdentity = HalpStaticProcessorIdentity;
 
 /* FUNCTIONS ******************************************************************/
 
@@ -24,20 +27,28 @@ VOID
 HalpParseApicTables(
     _In_ PLOADER_PARAMETER_BLOCK LoaderBlock)
 {
+    UNREFERENCED_PARAMETER(LoaderBlock);
+
+    // TODO: Fill HalpStaticProcessorIdentity[].
     UNIMPLEMENTED;
 }
 
 VOID
 HalpPrintApicTables(VOID)
 {
-    UINT32 i;
+#if DBG
+    ULONG i;
 
-    DPRINT1("HAL has detected a physical processor count of: %d\n", PhysicalProcessorCount);
+    DPRINT1("Physical processor count: %lu\n", PhysicalProcessorCount);
     for (i = 0; i < PhysicalProcessorCount; i++)
     {
-        DPRINT1("Information about the following processor is for processors number: %d\n"
+        DPRINT1(" Processor %lu: ProcessorId %u, LapicId %u, ProcessorStarted %u, BSPCheck %u, ProcessorPrcb %p\n",
-                "   The BSPCheck is set to: %X\n"
+                i,
-                "   The LapicID is set to: %X\n",
+                HalpProcessorIdentity[i].ProcessorId,
-                i, HalpProcessorIdentity[i].BSPCheck, HalpProcessorIdentity[i].LapicId);
+                HalpProcessorIdentity[i].LapicId,
+                HalpProcessorIdentity[i].ProcessorStarted,
+                HalpProcessorIdentity[i].BSPCheck,
+                HalpProcessorIdentity[i].ProcessorPrcb);
     }
+#endif
 }