[FREELDR] Improve DiskGetExtendedDriveParameters()

- Move BIOS-specific `EXTENDED_GEOMETRY` structure to pcdisk.c

- Zero out the transfer buffer for INT 13h, function 48h before
  invocation. This ensures that any random data is removed and
  that some broken BIOSes behave better.

- Use a more suitable type for the pointer to the transfer buffer,
  so that we can use more human-manageable member names in TRACEs
  instead of magic buffer offsets.

Author: HBelusca

boot/freeldr/freeldr/arch/i386/pc/pcdisk.c

@@ -27,7 +27,7 @@ DBG_DEFAULT_CHANNEL(DISK);
 /* Enable this line if you want to support multi-drive caching (increases FreeLdr size!) */
 // #define CACHE_MULTI_DRIVES
 
-#include <pshpack2.h>
+#include <pshpack1.h>
 
 typedef struct
 {
@@ -74,6 +74,22 @@ typedef struct
     UCHAR   Reserved;
 } I386_CDROM_SPEC_PACKET, *PI386_CDROM_SPEC_PACKET;
 
+/*
+ * Extended disk geometry (Int13 / AH=48h)
+ * See also ntdddisk.h DISK_EX_INT13_INFO
+ */
+typedef struct _EXTENDED_GEOMETRY
+{
+    USHORT      Size;
+    USHORT      Flags;
+    ULONG       Cylinders;
+    ULONG       Heads;
+    ULONG       SectorsPerTrack;
+    ULONGLONG   Sectors;
+    USHORT      BytesPerSector;
+    ULONG       PDPTE;
+} EXTENDED_GEOMETRY, *PEXTENDED_GEOMETRY;
+
 #include <poppack.h>
 
 typedef struct _PC_DISK_DRIVE
@@ -295,21 +311,27 @@ DiskInt13ExtensionsSupported(IN UCHAR DriveNumber)
 
 static BOOLEAN
 DiskGetExtendedDriveParameters(
-    IN UCHAR DriveNumber,
-    IN PPC_DISK_DRIVE DiskDrive,
-    OUT PVOID Buffer,
-    IN USHORT BufferSize)
+    _In_ UCHAR DriveNumber,
+    _In_ PPC_DISK_DRIVE DiskDrive,
+    _Out_ PVOID Buffer,
+    _In_ USHORT BufferSize)
 {
+    PEXTENDED_GEOMETRY Ptr = (PEXTENDED_GEOMETRY)BIOSCALLBUFFER;
     REGS RegsIn, RegsOut;
-    PUSHORT Ptr = (PUSHORT)(BIOSCALLBUFFER);
 
     TRACE("DiskGetExtendedDriveParameters(0x%x)\n", DriveNumber);
 
-    if (!DiskDrive->Int13ExtensionsSupported)
+    if (!DiskDrive->Int13ExtensionsSupported || (BufferSize < sizeof(*Ptr)))
         return FALSE;
 
-    /* Initialize transfer buffer */
-    *Ptr = BufferSize;
+    /*
+     * Initialize the transfer buffer.
+     * NOTE: Zeroing out the buffer also helps avoiding the bug where Dell
+     * machines using PhoenixBIOS 4.0 Release 6.0 fail to correctly handle
+     * this function if Ptr->Flags is not 0 on entry.
+     */
+    RtlZeroMemory(Ptr, sizeof(*Ptr)); // BufferSize;
+    Ptr->Size = BufferSize;
 
     /*
      * BIOS Int 13h, function 48h - Get drive parameters
@@ -325,7 +347,7 @@ DiskGetExtendedDriveParameters(
      */
     RegsIn.b.ah = 0x48;
     RegsIn.b.dl = DriveNumber;
-    RegsIn.x.ds = BIOSCALLBUFSEGMENT;   // DS:SI -> result buffer
+    RegsIn.x.ds = BIOSCALLBUFSEGMENT;
     RegsIn.w.si = BIOSCALLBUFOFFSET;
 
     /* Get drive parameters */
@@ -336,22 +358,25 @@ DiskGetExtendedDriveParameters(
     RtlCopyMemory(Buffer, Ptr, BufferSize);
 
 #if DBG
-    TRACE("Size of buffer:                          0x%x\n", Ptr[0]);
-    TRACE("Information flags:                       0x%x\n", Ptr[1]);
-    TRACE("Number of physical cylinders on drive:   %u\n", *(PULONG)&Ptr[2]);
-    TRACE("Number of physical heads on drive:       %u\n", *(PULONG)&Ptr[4]);
-    TRACE("Number of physical sectors per track:    %u\n", *(PULONG)&Ptr[6]);
-    TRACE("Total number of sectors on drive:        %I64u\n", *(PULONGLONG)&Ptr[8]);
-    TRACE("Bytes per sector:                        %u\n", Ptr[12]);
-    if (Ptr[0] >= 0x1e)
+    TRACE("Size of buffer:                          0x%x\n", Ptr->Size);
+    TRACE("Information flags:                       0x%x\n", Ptr->Flags);
+    TRACE("Number of physical cylinders on drive:   %u\n", Ptr->Cylinders);
+    TRACE("Number of physical heads on drive:       %u\n", Ptr->Heads);
+    TRACE("Number of physical sectors per track:    %u\n", Ptr->SectorsPerTrack);
+    TRACE("Total number of sectors on drive:        %I64u\n", Ptr->Sectors);
+    TRACE("Bytes per sector:                        %u\n", Ptr->BytesPerSector);
+    if (Ptr->Size >= 0x1E)
     {
-        // Ptr[13]: offset, Ptr[14]: segment
-        TRACE("EDD configuration parameters (DPTE):     %x:%x\n", Ptr[14], Ptr[13]);
+        // LOWORD(Ptr->PDPTE): offset, HIWORD(Ptr->PDPTE): segment
+        USHORT Off = (USHORT)(Ptr->PDPTE & 0xFFFF);         // ((PUSHORT)&Ptr->PDPTE)[0];
+        USHORT Seg = (USHORT)((Ptr->PDPTE >> 16) & 0xFFFF); // ((PUSHORT)&Ptr->PDPTE)[1];
+        TRACE("EDD configuration parameters (DPTE):     %x:%x\n", Seg, Off);
+
         /* The DPTE pointer is valid if it's != FFFF:FFFF (per the Enhanced Disk
          * Drive Specification), but also, when it's != 0000:0000 (broken BIOSes) */
-        if (!(Ptr[13] == 0xFFFF && Ptr[14] == 0xFFFF) && !(Ptr[13] == 0 && Ptr[14] == 0))
+        if (Ptr->PDPTE != 0xFFFFFFFF && Ptr->PDPTE != 0)
         {
-            PUCHAR SpecPtr = (PUCHAR)(ULONG_PTR)((Ptr[14] << 4) + Ptr[13]);
+            PUCHAR SpecPtr = (PUCHAR)(ULONG_PTR)((Seg << 4) + Off);
             TRACE("SpecPtr:                                 0x%x\n", SpecPtr);
             TRACE("Physical I/O port base address:          0x%x\n", *(PUSHORT)&SpecPtr[0]);
             TRACE("Disk-drive control port address:         0x%x\n", *(PUSHORT)&SpecPtr[2]);
@@ -364,9 +389,9 @@ DiskGetExtendedDriveParameters(
             TRACE("Drive options:                           0x%x\n", *(PUSHORT)&SpecPtr[10]);
         }
     }
-    if (Ptr[0] >= 0x42)
+    if (Ptr->Size >= 0x42)
     {
-        TRACE("Signature:                             0x%x\n", Ptr[15]);
+        TRACE("Signature:                             0x%x\n", ((PUSHORT)Ptr)[15]);
     }
 #endif // DBG
 
@@ -383,16 +408,11 @@ InitDriveGeometry(
     ULONG Cylinders;
 
     /* Get the extended geometry first */
-    DiskDrive->ExtGeometry.Size = sizeof(DiskDrive->ExtGeometry);
+    RtlZeroMemory(&DiskDrive->ExtGeometry, sizeof(DiskDrive->ExtGeometry));
     Success = DiskGetExtendedDriveParameters(DriveNumber, DiskDrive,
                                              &DiskDrive->ExtGeometry,
-                                             DiskDrive->ExtGeometry.Size);
-    if (!Success)
-    {
-        /* Failed, zero it out */
-        RtlZeroMemory(&DiskDrive->ExtGeometry, sizeof(DiskDrive->ExtGeometry));
-    }
-    else
+                                             sizeof(DiskDrive->ExtGeometry));
+    if (Success)
     {
         TRACE("DiskGetExtendedDriveParameters(0x%x) returned:\n"
               "Cylinders  : 0x%x\n"
@@ -575,9 +595,9 @@ PcDiskReadLogicalSectorsLBA(
     IN ULONG SectorCount,
     OUT PVOID Buffer)
 {
+    PI386_DISK_ADDRESS_PACKET Packet = (PI386_DISK_ADDRESS_PACKET)BIOSCALLBUFFER;
     REGS RegsIn, RegsOut;
     ULONG RetryCount;
-    PI386_DISK_ADDRESS_PACKET Packet = (PI386_DISK_ADDRESS_PACKET)(BIOSCALLBUFFER);
 
     /* Setup disk address packet */
     RtlZeroMemory(Packet, sizeof(*Packet));

boot/freeldr/freeldr/include/disk.h

@@ -33,22 +33,6 @@ typedef struct _GEOMETRY
 
 #include <pshpack1.h>
 
-/*
- * Extended disk geometry (Int13 / AH=48h)
- * See also ntdddisk.h DISK_EX_INT13_INFO
- */
-typedef struct _EXTENDED_GEOMETRY
-{
-    USHORT      Size;
-    USHORT      Flags;
-    ULONG       Cylinders;
-    ULONG       Heads;
-    ULONG       SectorsPerTrack;
-    ULONGLONG   Sectors;
-    USHORT      BytesPerSector;
-    ULONG       PDPTE;
-} EXTENDED_GEOMETRY, *PEXTENDED_GEOMETRY;
-
 /*
  * Define the structure of a partition table entry
  */