[ros-diffs] [reactos] 01/01: [HALX86] acpi/madt.c: Rewrite it (#6032)

[Serge Gautherie]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=2c8d083fc019cdfb465b8…

commit 2c8d083fc019cdfb465b8e64fd1438d394629c4c
Author:     Serge Gautherie <32623169+SergeGautherie(a)users.noreply.github.com>
AuthorDate: Sat Aug 31 23:35:50 2024 +0200
Commit:     GitHub <noreply(a)github.com>
CommitDate: Sat Aug 31 14:35:50 2024 -0700

    [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   | 248 ++++++++++++++++++++++++++++++++++++++++-------
 hal/halx86/include/smp.h |  13 ++-
 hal/halx86/smp/mps/mps.c |  29 ++++--
 3 files changed, 239 insertions(+), 51 deletions(-)

diff --git a/hal/halx86/acpi/madt.c b/hal/halx86/acpi/madt.c
index cd26be67b21..f61d51b5d02 100644
--- a/hal/halx86/acpi/madt.c
+++ b/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(a)gmail.com>
+ *              Copyright 2023 Serge Gautherie
<reactos-git_serge_171003(a)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 *LocalApic;
+
+// ACPI_MADT_LOCAL_APIC.LapicFlags masks
+#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 */
-    HalpApicInfoTable.ApicMode = 0x10;
-    MadtTable = HalAcpiGetTable(LoaderBlock, 'CIPA');
+    MadtTable = HalAcpiGetTable(LoaderBlock, APIC_SIGNATURE);
+    if (!MadtTable)
+    {
+        DPRINT01("MADT table not found\n");
+        return;
+    }
+
+    if (MadtTable->Header.Length < sizeof(*MadtTable))
+    {
+        DPRINT01("Length is too short: %p, %u\n", MadtTable,
MadtTable->Header.Length);
+        return;
+    }
 
-    AcpiHeader = (ACPI_SUBTABLE_HEADER*)MadtTable;
-    AcpiHeader->Length = sizeof(ACPI_TABLE_MADT);
-    TableEnd = (ULONG_PTR)MadtTable + MadtTable->Header.Length;
+    DPRINT00("MADT table: Address %08X, Flags %08X\n", MadtTable->Address,
MadtTable->Flags);
 
-    HalpApicInfoTable.ProcessorCount = 0;
-    HalpProcessorIdentity = HalpStaticProcessorIdentity;
+#if 1
 
-    AcpiHeader = (ACPI_SUBTABLE_HEADER*)MadtTable;
-    AcpiHeader->Length = sizeof(ACPI_TABLE_MADT);
-    TableEnd = (ULONG_PTR)MadtTable + MadtTable->Header.Length;
+    // TODO: We support only legacy APIC for now
+    HalpApicInfoTable.ApicMode = HALP_APIC_MODE_LEGACY;
+    // TODO: What about 'MadtTable->Flags & ACPI_MADT_PCAT_COMPAT'?
 
-    while ((ULONG_PTR)AcpiHeader <= TableEnd)
+#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)
     {
-        LocalApic = (ACPI_MADT_LOCAL_APIC*)AcpiHeader;
+#if 1
+        DPRINT01("ACPI_MADT_MULTIPLE_APIC support is UNIMPLEMENTED\n");
+        return;
+#else
+        HalpApicInfoTable.ApicMode = HALP_APIC_MODE_xyz;
+#endif
+    }
 
-        if (LocalApic->Header.Type == ACPI_MADT_TYPE_LOCAL_APIC &&
-            LocalApic->Header.Length == sizeof(ACPI_MADT_LOCAL_APIC))
-        {
-            ValidProcessorCount = HalpApicInfoTable.ProcessorCount;
+#endif
+
+    HalpApicInfoTable.LocalApicPA = MadtTable->Address;
 
-            HalpProcessorIdentity[ValidProcessorCount].LapicId = LocalApic->Id;
-            HalpProcessorIdentity[ValidProcessorCount].ProcessorId =
LocalApic->ProcessorId;
+    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);
 
-            HalpApicInfoTable.ProcessorCount++;
+    while ((ULONG_PTR)(AcpiHeader + 1) <= TableEnd)
+    {
+        if (AcpiHeader->Length < sizeof(*AcpiHeader))
+        {
+            DPRINT01("Length is too short: %p, %u\n", AcpiHeader,
AcpiHeader->Length);
+            return;
+        }
 
-            AcpiHeader = (ACPI_SUBTABLE_HEADER*)((ULONG_PTR)AcpiHeader +
AcpiHeader->Length);
+        if ((ULONG_PTR)AcpiHeader + AcpiHeader->Length > TableEnd)
+        {
+            DPRINT01("Length mismatch: %p, %u, %p\n",
+                     AcpiHeader, AcpiHeader->Length, (PVOID)TableEnd);
+            return;
         }
-        else
+
+        switch (AcpiHeader->Type)
         {
-            /* End the parsing early if we don't use the currently selected table */
-            AcpiHeader = (ACPI_SUBTABLE_HEADER*)((ULONG_PTR)AcpiHeader + 1);
+            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);
+    }
+
+    if ((ULONG_PTR)AcpiHeader != TableEnd)
+    {
+        DPRINT01("Length mismatch: %p, %p, %p\n", MadtTable, AcpiHeader,
(PVOID)TableEnd);
+        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"
-                "   The BSPCheck is set to: %X\n"
-                "   The LapicID is set to: %X\n",
-                i, HalpProcessorIdentity[i].BSPCheck, HalpProcessorIdentity[i].LapicId);
+        DPRINT1(" Processor %lu: ProcessorId %u, LapicId %u, ProcessorStarted %u,
BSPCheck %u, ProcessorPrcb %p\n",
+                i,
+                HalpProcessorIdentity[i].ProcessorId,
+                HalpProcessorIdentity[i].LapicId,
+                HalpProcessorIdentity[i].ProcessorStarted,
+                HalpProcessorIdentity[i].BSPCheck,
+                HalpProcessorIdentity[i].ProcessorPrcb);
     }
+#endif
 }
diff --git a/hal/halx86/include/smp.h b/hal/halx86/include/smp.h
index 7efa12afc39..d38a5be4f16 100644
--- a/hal/halx86/include/smp.h
+++ b/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 IoApicPA[256];
-    ULONG IoApicIrqBase[256]; // Global system interrupt base
-
+    ULONG IoApicVA[HALP_APIC_INFO_TABLE_IOAPIC_NUMBER];
+    ULONG IoApicPA[HALP_APIC_INFO_TABLE_IOAPIC_NUMBER];
+    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);
diff --git a/hal/halx86/smp/mps/mps.c b/hal/halx86/smp/mps/mps.c
index a55a962f02d..7b1a279772f 100644
--- a/hal/halx86/smp/mps/mps.c
+++ b/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}};
-PPROCESSOR_IDENTITY HalpProcessorIdentity = NULL;
-UINT32 PhysicalProcessorCount = 0;
+static // TODO: While HalpParseApicTables() is UNIMPLEMENTED.
+ULONG PhysicalProcessorCount;
+
+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"
-                "   The BSPCheck is set to: %X\n"
-                "   The LapicID is set to: %X\n",
-                i, HalpProcessorIdentity[i].BSPCheck, HalpProcessorIdentity[i].LapicId);
+        DPRINT1(" Processor %lu: ProcessorId %u, LapicId %u, ProcessorStarted %u,
BSPCheck %u, ProcessorPrcb %p\n",
+                i,
+                HalpProcessorIdentity[i].ProcessorId,
+                HalpProcessorIdentity[i].LapicId,
+                HalpProcessorIdentity[i].ProcessorStarted,
+                HalpProcessorIdentity[i].BSPCheck,
+                HalpProcessorIdentity[i].ProcessorPrcb);
     }
+#endif
 }