From b82bf8ce167ae96dff2868461e82136b90b8c45f Mon Sep 17 00:00:00 2001 From: Timo Kreuzer Date: Sun, 28 Jan 2018 23:44:28 +0100 Subject: [PATCH] [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 | 3 ++ drivers/bus/pci/pci.h | 1 + drivers/bus/pci/pdo.c | 7 +-- drivers/bus/pcix/CMakeLists.txt | 4 +- drivers/bus/pcix/debug.c | 5 +- drivers/bus/pcix/enum.c | 2 +- drivers/bus/pcix/pci.h | 15 ++---- drivers/bus/pcix/utils.c | 21 -------- ntoskrnl/include/internal/cm.h | 1 + ntoskrnl/io/pnpmgr/pnpres.c | 60 ++++++++++++++++++---- sdk/include/reactos/drivers/cmreslist.h | 66 +++++++++++++++++++++++++ win32ss/drivers/videoprt/resource.c | 9 +++- 12 files changed, 146 insertions(+), 48 deletions(-) create mode 100644 sdk/include/reactos/drivers/cmreslist.h diff --git a/drivers/bus/pci/CMakeLists.txt b/drivers/bus/pci/CMakeLists.txt index 5bd36b7bb7c..3e8fe17c8e6 100644 --- a/drivers/bus/pci/CMakeLists.txt +++ b/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 diff --git a/drivers/bus/pci/pci.h b/drivers/bus/pci/pci.h index f3394c7ca44..3cd2204bf30 100644 --- a/drivers/bus/pci/pci.h +++ b/drivers/bus/pci/pci.h @@ -2,6 +2,7 @@ #define _PCI_PCH_ #include +#include #define TAG_PCI '0ICP' diff --git a/drivers/bus/pci/pdo.c b/drivers/bus/pci/pdo.c index bedaac36fc0..1d6b8c54ecf 100644 --- a/drivers/bus/pci/pdo.c +++ b/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) diff --git a/drivers/bus/pcix/CMakeLists.txt b/drivers/bus/pcix/CMakeLists.txt index 9389bde01eb..cdbcf5f8514 100644 --- a/drivers/bus/pcix/CMakeLists.txt +++ b/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 diff --git a/drivers/bus/pcix/debug.c b/drivers/bus/pcix/debug.c index 971cdadf498..4b7e0154f3c 100644 --- a/drivers/bus/pcix/debug.c +++ b/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 */ diff --git a/drivers/bus/pcix/enum.c b/drivers/bus/pcix/enum.c index edb8a261d97..fc3563d2c21 100644 --- a/drivers/bus/pcix/enum.c +++ b/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 */ diff --git a/drivers/bus/pcix/pci.h b/drivers/bus/pcix/pci.h index fc6b578c24d..eb7f517d48b 100644 --- a/drivers/bus/pcix/pci.h +++ b/drivers/bus/pcix/pci.h @@ -19,6 +19,7 @@ #include #include #include +#include // // 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 InterruptPin, - IN UCHAR BaseClass, - IN UCHAR SubClass, + IN UCHAR InterruptLine, + IN UCHAR InterruptPin, + IN UCHAR BaseClass, + IN UCHAR SubClass, IN PDEVICE_OBJECT PhysicalDeviceObject, IN PPCI_PDO_EXTENSION PdoExtension, OUT PDEVICE_OBJECT *pFoundDeviceObject diff --git a/drivers/bus/pcix/utils.c b/drivers/bus/pcix/utils.c index 32a03339e63..fe231559264 100644 --- a/drivers/bus/pcix/utils.c +++ b/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 */ diff --git a/ntoskrnl/include/internal/cm.h b/ntoskrnl/include/internal/cm.h index a65e5e8ba2a..19f70265585 100644 --- a/ntoskrnl/include/internal/cm.h +++ b/ntoskrnl/include/internal/cm.h @@ -7,6 +7,7 @@ */ #define _CM_ #include "cmlib.h" +#include // // Define this if you want debugging support diff --git a/ntoskrnl/io/pnpmgr/pnpres.c b/ntoskrnl/io/pnpmgr/pnpres.c index 2711e1a1dac..b417072b087 100644 --- a/ntoskrnl/io/pnpmgr/pnpres.c +++ b/ntoskrnl/io/pnpmgr/pnpres.c @@ -12,6 +12,16 @@ #define NDEBUG #include +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, diff --git a/sdk/include/reactos/drivers/cmreslist.h b/sdk/include/reactos/drivers/cmreslist.h new file mode 100644 index 00000000000..5b52b8b6656 --- /dev/null +++ b/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 + +// +// 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); +} diff --git a/win32ss/drivers/videoprt/resource.c b/win32ss/drivers/videoprt/resource.c index 4a44392a0f3..f6ac53c6842 100644 --- a/win32ss/drivers/videoprt/resource.c +++ b/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 */