[NTOS:IO] Fix parsing of resource lists

Also add a hack to avoid failing on now occurring resource conflict detection and try to fix at least one resource in ACPI hal.

CORE-10146
CORE-12892

drivers/bus/pci/CMakeLists.txt

@@ -1,4 +1,7 @@
 
+include_directories(
+    ${REACTOS_SOURCE_DIR}/sdk/include/reactos/drivers)
+
 list(APPEND SOURCE
     fdo.c
     pci.c

drivers/bus/pci/pci.h

@@ -2,6 +2,7 @@
 #define _PCI_PCH_
 
 #include <ntifs.h>
+#include <cmreslist.h>
 
 #define TAG_PCI '0ICP'
 

drivers/bus/pci/pdo.c

@@ -1335,12 +1335,13 @@ PdoStartDevice(
 
     /* TODO: Assign the other resources we get to the card */
 
-    for (i = 0; i < RawResList->Count; i++)
+    RawFullDesc = &RawResList->List[0];
+    for (i = 0; i < RawResList->Count; i++, RawFullDesc = CmiGetNextResourceDescriptor(RawFullDesc))
     {
-        RawFullDesc = &RawResList->List[i];
-
         for (ii = 0; ii < RawFullDesc->PartialResourceList.Count; ii++)
         {
+            /* Partial resource descriptors can be of variable size (CmResourceTypeDeviceSpecific),
+               but only one is allowed and it must be the last one in the list! */
             RawPartialDesc = &RawFullDesc->PartialResourceList.PartialDescriptors[ii];
 
             if (RawPartialDesc->Type == CmResourceTypeInterrupt)

drivers/bus/pcix/CMakeLists.txt

@@ -1,5 +1,7 @@
+
 include_directories(
-    ${REACTOS_SOURCE_DIR}/sdk/lib/drivers/arbiter)
+    ${REACTOS_SOURCE_DIR}/sdk/lib/drivers/arbiter
+    ${REACTOS_SOURCE_DIR}/sdk/include/reactos/drivers)
 
 list(APPEND SOURCE
     arb/ar_busno.c

drivers/bus/pcix/debug.c

@@ -386,12 +386,15 @@ PciDebugPrintCmResList(IN PCM_RESOURCE_LIST PartialList)
         Count = FullDescriptor->PartialResourceList.Count;
         for (PartialDescriptor = FullDescriptor->PartialResourceList.PartialDescriptors;
              Count;
-             PartialDescriptor = PciNextPartialDescriptor(PartialDescriptor))
+             PartialDescriptor = CmiGetNextPartialDescriptor(PartialDescriptor))
         {
             /* Print each partial */
             PciDebugPrintPartialResource(PartialDescriptor);
             Count--;
         }
+
+        /* Go to the next full descriptor */
+        FullDescriptor = (PCM_FULL_RESOURCE_DESCRIPTOR)PartialDescriptor;
     }
 
     /* Done printing data */

drivers/bus/pcix/enum.c

@@ -168,7 +168,7 @@ PciComputeNewCurrentSettings(IN PPCI_PDO_EXTENSION PdoExtension,
             }
 
             /* Move to the next descriptor */
-            Partial = PciNextPartialDescriptor(Partial);
+            Partial = CmiGetNextPartialDescriptor(Partial);
         }
 
         /* We should be starting a new list now */

drivers/bus/pcix/pci.h

@@ -19,6 +19,7 @@
 #include <ndk/rtlfuncs.h>
 #include <ndk/vffuncs.h>
 #include <arbiter.h>
+#include <cmreslist.h>
 
 //
 // Tag used in all pool allocations (Pci Bus)
@@ -1173,12 +1174,6 @@ PciQueryCapabilities(
     IN OUT PDEVICE_CAPABILITIES DeviceCapability
 );
 
-PCM_PARTIAL_RESOURCE_DESCRIPTOR
-NTAPI
-PciNextPartialDescriptor(
-    PCM_PARTIAL_RESOURCE_DESCRIPTOR CmDescriptor
-);
-
 //
 // Configuration Routines
 //
@@ -1787,10 +1782,10 @@ PciCacheLegacyDeviceRouting(
     IN PDEVICE_OBJECT DeviceObject,
     IN ULONG BusNumber,
     IN ULONG SlotNumber,
-    IN UCHAR InterruptLine, 
+    IN UCHAR InterruptLine,
-    IN UCHAR InterruptPin, 
+    IN UCHAR InterruptPin,
-    IN UCHAR BaseClass, 
+    IN UCHAR BaseClass,
-    IN UCHAR SubClass, 
+    IN UCHAR SubClass,
     IN PDEVICE_OBJECT PhysicalDeviceObject,
     IN PPCI_PDO_EXTENSION PdoExtension,
     OUT PDEVICE_OBJECT *pFoundDeviceObject

drivers/bus/pcix/utils.c

@@ -1764,25 +1764,4 @@ PciQueryCapabilities(IN PPCI_PDO_EXTENSION PdoExtension,
     return Status;
 }
 
-PCM_PARTIAL_RESOURCE_DESCRIPTOR
-NTAPI
-PciNextPartialDescriptor(PCM_PARTIAL_RESOURCE_DESCRIPTOR CmDescriptor)
-{
-    PCM_PARTIAL_RESOURCE_DESCRIPTOR NextDescriptor;
-
-    /* Assume the descriptors are the fixed size ones */
-    NextDescriptor = CmDescriptor + 1;
-
-    /* But check if this is actually a variable-sized descriptor */
-    if (CmDescriptor->Type == CmResourceTypeDeviceSpecific)
-    {
-        /* Add the size of the variable section as well */
-        NextDescriptor = (PVOID)((ULONG_PTR)NextDescriptor +
-                                 CmDescriptor->u.DeviceSpecificData.DataSize);
-    }
-
-    /* Now the correct pointer has been computed, return it */
-    return NextDescriptor;
-}
-
 /* EOF */

ntoskrnl/include/internal/cm.h

@@ -7,6 +7,7 @@
  */
 #define _CM_
 #include "cmlib.h"
+#include <cmreslist.h>
 
 //
 // Define this if you want debugging support

ntoskrnl/io/pnpmgr/pnpres.c

@@ -12,6 +12,16 @@
 #define NDEBUG
 #include <debug.h>
 
+FORCEINLINE
+PIO_RESOURCE_LIST
+IopGetNextResourceList(
+    _In_ const IO_RESOURCE_LIST *ResourceList)
+{
+    ASSERT((ResourceList->Count > 0) && (ResourceList->Count < 1000));
+    return (PIO_RESOURCE_LIST)(
+        &ResourceList->Descriptors[ResourceList->Count]);
+}
+
 static
 BOOLEAN
 IopCheckDescriptorForConflict(
@@ -199,6 +209,9 @@ IopFindInterruptResource(
         }
     }
 
+    DPRINT1("Failed to satisfy interrupt requirement with IRQ 0x%x-0x%x\n",
+            IoDesc->u.Interrupt.MinimumVector,
+            IoDesc->u.Interrupt.MaximumVector);
     return FALSE;
 }
 
@@ -209,6 +222,7 @@ IopFixupResourceListWithRequirements(
 {
     ULONG i, OldCount;
     BOOLEAN AlternateRequired = FALSE;
+    PIO_RESOURCE_LIST ResList;
 
     /* Save the initial resource count when we got here so we can restore if an alternate fails */
     if (*ResourceList != NULL)
@@ -216,10 +230,10 @@ IopFixupResourceListWithRequirements(
     else
         OldCount = 0;
 
-    for (i = 0; i < RequirementsList->AlternativeLists; i++)
+    ResList = &RequirementsList->List[0];
+    for (i = 0; i < RequirementsList->AlternativeLists; i++, ResList = IopGetNextResourceList(ResList))
     {
         ULONG ii;
-        PIO_RESOURCE_LIST ResList = &RequirementsList->List[i];
 
         /* We need to get back to where we were before processing the last alternative list */
         if (OldCount == 0 && *ResourceList != NULL)
@@ -275,6 +289,8 @@ IopFixupResourceListWithRequirements(
 
             for (iii = 0; PartialList && iii < PartialList->Count && !Matched; iii++)
             {
+                /* Partial resource descriptors can be of variable size (CmResourceTypeDeviceSpecific),
+                   but only one is allowed and it must be the last one in the list! */
                 PCM_PARTIAL_RESOURCE_DESCRIPTOR CmDesc = &PartialList->PartialDescriptors[iii];
 
                 /* First check types */
@@ -548,12 +564,18 @@ IopCheckResourceDescriptor(
 {
     ULONG i, ii;
     BOOLEAN Result = FALSE;
+    PCM_FULL_RESOURCE_DESCRIPTOR FullDescriptor;
 
+    FullDescriptor = &ResourceList->List[0];
     for (i = 0; i < ResourceList->Count; i++)
     {
-        PCM_PARTIAL_RESOURCE_LIST ResList = &ResourceList->List[i].PartialResourceList;
+        PCM_PARTIAL_RESOURCE_LIST ResList = &FullDescriptor->PartialResourceList;
+        FullDescriptor = CmiGetNextResourceDescriptor(FullDescriptor);
+
         for (ii = 0; ii < ResList->Count; ii++)
         {
+            /* Partial resource descriptors can be of variable size (CmResourceTypeDeviceSpecific),
+               but only one is allowed and it must be the last one in the list! */
             PCM_PARTIAL_RESOURCE_DESCRIPTOR ResDesc2 = &ResList->PartialDescriptors[ii];
 
             /* We don't care about shared resources */
@@ -674,7 +696,9 @@ ByeBye:
                       sizeof(CM_PARTIAL_RESOURCE_DESCRIPTOR));
     }
 
-    return Result;
+    // Hacked, because after fixing resource list parsing
+    // we actually detect resource conflicts
+    return Silent ? Result : FALSE; // Result; 
 }
 
 static
@@ -937,6 +961,7 @@ IopTranslateDeviceResources(
 {
    PCM_PARTIAL_RESOURCE_LIST pPartialResourceList;
    PCM_PARTIAL_RESOURCE_DESCRIPTOR DescriptorRaw, DescriptorTranslated;
+   PCM_FULL_RESOURCE_DESCRIPTOR FullDescriptor;
    ULONG i, j, ListSize;
    NTSTATUS Status;
 
@@ -959,13 +984,23 @@ IopTranslateDeviceResources(
    }
    RtlCopyMemory(DeviceNode->ResourceListTranslated, DeviceNode->ResourceList, ListSize);
 
+   FullDescriptor = &DeviceNode->ResourceList->List[0];
    for (i = 0; i < DeviceNode->ResourceList->Count; i++)
    {
-      pPartialResourceList = &DeviceNode->ResourceList->List[i].PartialResourceList;
+      pPartialResourceList = &FullDescriptor->PartialResourceList;
+      FullDescriptor = CmiGetNextResourceDescriptor(FullDescriptor);
+
       for (j = 0; j < pPartialResourceList->Count; j++)
       {
+        /* Partial resource descriptors can be of variable size (CmResourceTypeDeviceSpecific),
+           but only one is allowed and it must be the last one in the list! */
          DescriptorRaw = &pPartialResourceList->PartialDescriptors[j];
-         DescriptorTranslated = &DeviceNode->ResourceListTranslated->List[i].PartialResourceList.PartialDescriptors[j];
+
+         /* Calculate the location of the translated resource descriptor */
+         DescriptorTranslated = (PCM_PARTIAL_RESOURCE_DESCRIPTOR)(
+             (PUCHAR)DeviceNode->ResourceListTranslated +
+             ((PUCHAR)DescriptorRaw - (PUCHAR)DeviceNode->ResourceList));
+
          switch (DescriptorRaw->Type)
          {
             case CmResourceTypePort:
@@ -996,14 +1031,15 @@ IopTranslateDeviceResources(
             }
             case CmResourceTypeInterrupt:
             {
+               KIRQL Irql;
                DescriptorTranslated->u.Interrupt.Vector = HalGetInterruptVector(
                   DeviceNode->ResourceList->List[i].InterfaceType,
                   DeviceNode->ResourceList->List[i].BusNumber,
                   DescriptorRaw->u.Interrupt.Level,
                   DescriptorRaw->u.Interrupt.Vector,
-                  (PKIRQL)&DescriptorTranslated->u.Interrupt.Level,
+                  &Irql,
                   &DescriptorTranslated->u.Interrupt.Affinity);
-
+               DescriptorTranslated->u.Interrupt.Level = Irql;
                if (!DescriptorTranslated->u.Interrupt.Vector)
                {
                    Status = STATUS_UNSUCCESSFUL;
@@ -1184,12 +1220,18 @@ IopCheckForResourceConflict(
 {
    ULONG i, ii;
    BOOLEAN Result = FALSE;
+   PCM_FULL_RESOURCE_DESCRIPTOR FullDescriptor;
 
+   FullDescriptor = &ResourceList1->List[0];
    for (i = 0; i < ResourceList1->Count; i++)
    {
-      PCM_PARTIAL_RESOURCE_LIST ResList = &ResourceList1->List[i].PartialResourceList;
+      PCM_PARTIAL_RESOURCE_LIST ResList = &FullDescriptor->PartialResourceList;
+      FullDescriptor = CmiGetNextResourceDescriptor(FullDescriptor);
+
       for (ii = 0; ii < ResList->Count; ii++)
       {
+        /* Partial resource descriptors can be of variable size (CmResourceTypeDeviceSpecific),
+           but only one is allowed and it must be the last one in the list! */
          PCM_PARTIAL_RESOURCE_DESCRIPTOR ResDesc = &ResList->PartialDescriptors[ii];
 
          Result = IopCheckResourceDescriptor(ResDesc,

sdk/include/reactos/drivers/cmreslist.h

@@ -0,0 +1,66 @@
+/*
+ * PROJECT:     ReactOS Kernel
+ * LICENSE:     GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
+ * PURPOSE:     Helper functions to parse CM_RESOURCE_LISTs
+ * COPYRIGHT:   Copyright 2020 Timo Kreuzer (timo.kreuzer@reactos.org)
+ */
+
+#include <wdm.h>
+
+//
+// Resource list helpers
+//
+
+/* Usage note:
+ * As there can be only one variable-sized CM_PARTIAL_RESOURCE_DESCRIPTOR in the list (and it must be the last one),
+ * a right looping through resources can look like this:
+ *
+ * PCM_FULL_RESOURCE_DESCRIPTOR FullDesc = &ResourceList->List[0];
+ * for (ULONG i = 0; i < ResourceList->Count; i++, FullDesc = CmiGetNextResourceDescriptor(FullDesc))
+ * {
+ *    for (ULONG j = 0; j < FullDesc->PartialResourceList.Count; j++)
+ *    {
+ *        PartialDesc = &FullDesc->PartialResourceList.PartialDescriptors[j];
+ *        // work with PartialDesc
+ *    }
+ * }
+ */
+
+FORCEINLINE
+PCM_PARTIAL_RESOURCE_DESCRIPTOR
+CmiGetNextPartialDescriptor(
+    _In_ const CM_PARTIAL_RESOURCE_DESCRIPTOR *PartialDescriptor)
+{
+    const CM_PARTIAL_RESOURCE_DESCRIPTOR *NextDescriptor;
+
+    /* Assume the descriptors are the fixed size ones */
+    NextDescriptor = PartialDescriptor + 1;
+
+    /* But check if this is actually a variable-sized descriptor */
+    if (PartialDescriptor->Type == CmResourceTypeDeviceSpecific)
+    {
+        /* Add the size of the variable section as well */
+        NextDescriptor = (PVOID)((ULONG_PTR)NextDescriptor +
+                                 PartialDescriptor->u.DeviceSpecificData.DataSize);
+        ASSERT(NextDescriptor >= PartialDescriptor + 1);
+    }
+
+    /* Now the correct pointer has been computed, return it */
+    return (PCM_PARTIAL_RESOURCE_DESCRIPTOR)NextDescriptor;
+}
+
+FORCEINLINE
+PCM_FULL_RESOURCE_DESCRIPTOR
+CmiGetNextResourceDescriptor(
+    _In_ const CM_FULL_RESOURCE_DESCRIPTOR *ResourceDescriptor)
+{
+    const CM_PARTIAL_RESOURCE_DESCRIPTOR *LastPartialDescriptor;
+
+    /* Calculate the location of the last partial descriptor, which can have a
+       variable size! */
+    LastPartialDescriptor = &ResourceDescriptor->PartialResourceList.PartialDescriptors[
+            ResourceDescriptor->PartialResourceList.Count - 1];
+
+    /* Next full resource descriptor follows the last partial descriptor */
+    return (PCM_FULL_RESOURCE_DESCRIPTOR)CmiGetNextPartialDescriptor(LastPartialDescriptor);
+}

win32ss/drivers/videoprt/resource.c

@@ -765,7 +765,7 @@ VideoPortGetAccessRanges(
             ERR_(VIDEOPRT, "Too many access ranges found\n");
             return ERROR_NOT_ENOUGH_MEMORY;
         }
-        if (Descriptor->Type == CmResourceTypeMemory)
+        else if (Descriptor->Type == CmResourceTypeMemory)
         {
             INFO_(VIDEOPRT, "Memory range starting at 0x%08x length 0x%08x\n",
                   Descriptor->u.Memory.Start.u.LowPart, Descriptor->u.Memory.Length);
@@ -804,6 +804,11 @@ VideoPortGetAccessRanges(
             else
                 DeviceExtension->InterruptShared = FALSE;
         }
+        else
+        {
+            ASSERT(FALSE);
+            return ERROR_INVALID_PARAMETER;
+        }
     }
 
     return NO_ERROR;
@@ -838,7 +843,7 @@ VideoPortVerifyAccessRanges(
    if (!ResourceList)
    {
       WARN_(VIDEOPRT, "ExAllocatePool() failed\n");
-      return ERROR_INVALID_PARAMETER;
+      return ERROR_NOT_ENOUGH_MEMORY;
    }
 
    /* Fill resource list */