[HALX86] Address code review comments

boot/freeldr/freeldr/ntldr/arch/i386/winldr.c

@@ -64,41 +64,6 @@ typedef struct
 #define TYPE_CODE   (0x10 | DESCRIPTOR_CODE | DESCRIPTOR_EXECUTE_READ)
 #define TYPE_DATA   (0x10 | DESCRIPTOR_READ_WRITE)
 
-VOID
-KiSetGdtEntryEx(
-    IN OUT PKGDTENTRY Entry,
-    IN ULONG32 Base,
-    IN ULONG Limit,
-    IN UCHAR Type,
-    IN UCHAR Dpl,
-    IN BOOLEAN Granularity,
-    IN UCHAR SegMode) // 0: 16-bit, 1: 32-bit, 2: 64-bit
-{
-    KiSetGdtDescriptorBase(Entry, Base);
-    KiSetGdtDescriptorLimit(Entry, Limit);
-    Entry->HighWord.Bits.Type = (Type & 0x1f);
-    Entry->HighWord.Bits.Dpl  = (Dpl & 0x3);
-    Entry->HighWord.Bits.Pres = (Type != 0); // Present, must be 1 when the GDT entry is valid.
-    Entry->HighWord.Bits.Sys  = 0;           // System
-    Entry->HighWord.Bits.Reserved_0  = 0;    // LongMode = !!(SegMode & 1);
-    Entry->HighWord.Bits.Default_Big = !!(SegMode & 2);
-    Entry->HighWord.Bits.Granularity |= !!Granularity; // The flag may have been already set by KiSetGdtDescriptorLimit().
-    // Entry->MustBeZero = 0;
-}
-
-FORCEINLINE
-VOID
-KiSetGdtEntry(
-    IN OUT PKGDTENTRY Entry,
-    IN ULONG32 Base,
-    IN ULONG Limit,
-    IN UCHAR Type,
-    IN UCHAR Dpl,
-    IN UCHAR SegMode) // 0: 16-bit, 1: 32-bit, 2: 64-bit
-{
-    KiSetGdtEntryEx(Entry, Base, Limit, Type, Dpl, FALSE, SegMode);
-}
-
 #if 0
 VOID
 DumpGDTEntry(ULONG_PTR Base, ULONG Selector)

hal/halx86/acpi/halacpi.c

@@ -851,12 +851,12 @@ HalpSetupAcpiPhase0(IN PLOADER_PARAMETER_BLOCK LoaderBlock)
         /* Allocate it */
         HalpLowStubPhysicalAddress.QuadPart = HalpAllocPhysicalMemory(LoaderBlock,
                                                                       0x100000,
-                                                                      1,
+                                                                      HALP_LOW_STUB_SIZE_IN_PAGES,
                                                                       FALSE);
         if (HalpLowStubPhysicalAddress.QuadPart)
         {
             /* Map it */
-            HalpLowStub = HalpMapPhysicalMemory64(HalpLowStubPhysicalAddress, 1);
+            HalpLowStub = HalpMapPhysicalMemory64(HalpLowStubPhysicalAddress, HALP_LOW_STUB_SIZE_IN_PAGES);
         }
     }
 

hal/halx86/apic/apicsmp.c

@@ -3,14 +3,15 @@
  * LICENSE:     GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
  * PURPOSE:     SMP specific APIC code
  * COPYRIGHT:   Copyright 2021 Timo Kreuzer <[email protected]>
- *              Copyright 2021 Justin Miller <[email protected]>
+ *              Copyright 2023 Justin Miller <[email protected]>
  */
 
 /* INCLUDES *******************************************************************/
 
 #include <hal.h>
 #include "apicp.h"
 #include <smp.h>
+
 #define NDEBUG
 #include <debug.h>
 
@@ -40,7 +41,7 @@ extern PPROCESSOR_IDENTITY HalpProcessorIdentity;
             local APIC(s) specified in Destination field. Vector specifies
             the startup address.
         APIC_MT_ExtInt - Delivers an external interrupt to the target local
-            APIC specified in Destination field. 
+            APIC specified in Destination field.
 
     \param TriggerMode - The trigger mode of the interrupt. Can be:
         APIC_TGM_Edge - The interrupt is edge triggered.
@@ -68,6 +69,12 @@ ApicRequestGlobalInterrupt(
 {
     APIC_INTERRUPT_COMMAND_REGISTER Icr;
 
+    /* Wait for the APIC to be idle */
+    do
+    {
+        Icr.Long0 = ApicRead(APIC_ICR0);
+    } while (Icr.DeliveryStatus);
+
     /* Setup the command register */
     Icr.LongLong = 0;
     Icr.Vector = Vector;
@@ -90,14 +97,16 @@ ApicRequestGlobalInterrupt(
 
 VOID
 NTAPI
-HalpRequestIpi(KAFFINITY TargetProcessors)
+HalpRequestIpi(_In_ KAFFINITY TargetProcessors)
 {
     UNIMPLEMENTED;
     __debugbreak();
 }
 
 VOID
-ApicStartApplicationProcessor(ULONG NTProcessorNumber, PHYSICAL_ADDRESS StartupLoc)
+ApicStartApplicationProcessor(
+    _In_ ULONG NTProcessorNumber,
+    _In_ PHYSICAL_ADDRESS StartupLoc)
 {
     ASSERT(StartupLoc.HighPart == 0);
     ASSERT((StartupLoc.QuadPart & 0xFFF) == 0);
@@ -107,6 +116,10 @@ ApicStartApplicationProcessor(ULONG NTProcessorNumber, PHYSICAL_ADDRESS StartupL
     ApicRequestGlobalInterrupt(HalpProcessorIdentity[NTProcessorNumber].LapicId, 0,
         APIC_MT_INIT, APIC_TGM_Edge, APIC_DSH_Destination);
 
+    /* De-Assert Init IPI */
+    ApicRequestGlobalInterrupt(HalpProcessorIdentity[NTProcessorNumber].LapicId, 0,
+        APIC_MT_INIT, APIC_TGM_Level, APIC_DSH_Destination);
+
     /* Stall execution for a bit to give APIC time: MPS Spec - B.4 */
     KeStallExecutionProcessor(200);
 

hal/halx86/generic/up.c

@@ -1,21 +1,23 @@
 /*
  * PROJECT:     ReactOS Kernel
  * LICENSE:     GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
- * PURPOSE:     Core source file for UP alternative functions
+ * PURPOSE:     Core source file for Uniprocessor (UP) alternative functions
  * COPYRIGHT:   Copyright 2021 Justin Miller <[email protected]>
  */
 
 /* INCLUDES ******************************************************************/
 
 #include <hal.h>
+
 #define NDEBUG
 #include <debug.h>
 
 /* FUNCTIONS *****************************************************************/
 
 VOID
 NTAPI
-HalRequestIpi(KAFFINITY TargetProcessors)
+HalRequestIpi(
+    _In_ KAFFINITY TargetProcessors)
 {
     /* This should never be called in UP mode */
     __debugbreak();
@@ -24,8 +26,8 @@ HalRequestIpi(KAFFINITY TargetProcessors)
 BOOLEAN
 NTAPI
 HalStartNextProcessor(
-    IN PLOADER_PARAMETER_BLOCK LoaderBlock,
-    IN PKPROCESSOR_STATE ProcessorState)
+    _In_ PLOADER_PARAMETER_BLOCK LoaderBlock,
+    _In_ PKPROCESSOR_STATE ProcessorState)
 {
     /* Always return false on UP systems */
     return FALSE;

hal/halx86/include/smp.h

@@ -45,8 +45,11 @@ HalpPrintApicTables(VOID);
 /* APIC specific functions inside apic/apicsmp.c */
 
 VOID
-ApicStartApplicationProcessor(ULONG NTProcessorNumber, PHYSICAL_ADDRESS StartupLoc);
+ApicStartApplicationProcessor(
+    _In_ ULONG NTProcessorNumber,
+    _In_ PHYSICAL_ADDRESS StartupLoc);
 
 VOID
 NTAPI
-HalpRequestIpi(KAFFINITY TargetProcessors);
+HalpRequestIpi(
+    _In_ KAFFINITY TargetProcessors);

hal/halx86/smp.cmake

@@ -10,10 +10,7 @@ if(ARCH STREQUAL "i386")
         smp/i386/apentry.S)
     list(APPEND HAL_SMP_SOURCE
         smp/i386/spinup.c)
-endif()
-
-
-if(ARCH STREQUAL "amd64")
+elseif(ARCH STREQUAL "amd64")
     list(APPEND HAL_SMP_ASM_SOURCE
         smp/amd64/apentry.S)
     list(APPEND HAL_SMP_SOURCE

hal/halx86/smp/amd64/apentry.S

@@ -1,7 +1,7 @@
 /*
  * PROJECT:     ReactOS Kernel
  * LICENSE:     GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
- * PURPOSE:     Assembly file for real mode AP code
+ * PURPOSE:     AMD64 Application Processor (AP) spinup setup
  * COPYRIGHT:   Copyright 2021 Justin Miller <[email protected]>
  */
 

hal/halx86/smp/amd64/spinup.c

@@ -9,6 +9,7 @@
 
 #include <hal.h>
 #include <smp.h>
+
 #define NDEBUG
 #include <debug.h>
 

hal/halx86/smp/i386/apentry.S

@@ -1,7 +1,7 @@
 /*
  * PROJECT:     ReactOS HAL
  * LICENSE:     GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
- * PURPOSE:     Application processor startup code for i386
+ * PURPOSE:     i386 Application Processor (AP) spinup setup
  * COPYRIGHT:   Copyright 2021 Justin Miller <[email protected]>
  *              Copyright 2021 Victor Perevertkin <[email protected]>
  */
@@ -34,7 +34,7 @@ _HalpAPEntry16:
 #else
     data32 lgdt cs:[ZERO_OFFSET(Gdtr)]
 #endif
-    
+
     /* Load temp page table */
     mov eax, cs:[ZERO_OFFSET(PageTableRoot)]
     mov cr3, eax
@@ -63,11 +63,11 @@ Gdtr_Pad:
 Gdtr:
     .short 0 // Limit
     .long 0 // Base
-_HalpAPEntry16End:  
+_HalpAPEntry16End:
 .endcode16
 
 .code32
-_HalpAPEntry32: 
+_HalpAPEntry32:
     /* Set the Ring 0 DS/ES/SS Segment */
     mov ax, HEX(10)
     mov ds, ax