From: Ruiyu Ni <[email protected]> The patch removes the assumption that the resources claimed by root bridges should not exist. Because resources might have been added: 1. by platform modules either in PEI through resource HOB, or in DXE, before the PCI host bridge driver runs. 2. Resources claimed by different root bridges may overlap so that resource adding operation for latter root bridges may fail if we assume the resource should not exist.
In real world, this patch is to fit OVMF platform needs because different root bridges in OVMF platform shares the same resources. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni <[email protected]> [[email protected]: intersection-based implementation] Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek <[email protected]> Cc: Ruiyu Ni <[email protected]> --- Notes: Untested (only build tested). I will have to complete the rebase the OVMF PCI host bridge / root bridge driver on top of PciHostBridgeDxe to test this patch. But, I think it's worth posting; we can discuss it, and Ray could even test it sooner. Thanks! MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h | 1 + MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 428 +++++++++++++++++++- 3 files changed, 419 insertions(+), 11 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf index ab5d87e1cf0a..a11f5a4b01fa 100644 --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf @@ -42,6 +42,7 @@ [LibraryClasses] BaseLib PciSegmentLib PciHostBridgeLib + SortLib [Protocols] gEfiMetronomeArchProtocolGuid ## CONSUMES diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h index 9a8ca21f4819..6a9bb0a2ee1c 100644 --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h @@ -22,6 +22,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include <Library/UefiDriverEntryPoint.h> #include <Library/MemoryAllocationLib.h> #include <Library/PciHostBridgeLib.h> +#include <Library/SortLib.h> #include <Protocol/PciHostBridgeResourceAllocation.h> #include "PciRootBridge.h" diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c index edf042cc2547..6fe41eac25a8 100644 --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c @@ -29,6 +29,413 @@ GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mPciResourceTypeStr[] = { }; /** + Comparator for memory or IO space descriptor. + Because the comparator only compares BaseAddress and both + EFI_GCD_MEMORY_SPACE_DESCRIPTOR and EFI_GCD_IO_SPACE_DESCRIPTOR + put the BaseAddress in the same offset (0), this comparator can be + used by both structures. + + @param Left Pointer to the memory or IO space descriptor. + @param Right Pointer to the memory or IO space descriptor. + + @retval 0 Left equal to Right. + @retval -1 Left is less than Right. + @retval 1 Left is greater than Right. +**/ +INTN +EFIAPI +DescriptorComparator ( + IN CONST VOID *Left, + IN CONST VOID *Right + ) +{ + CONST EFI_PHYSICAL_ADDRESS *LeftBaseAddress; + CONST EFI_PHYSICAL_ADDRESS *RightBaseAddress; + + LeftBaseAddress = Left; + RightBaseAddress = Right; + + if (*LeftBaseAddress < *RightBaseAddress) { + return -1; + } else if (*LeftBaseAddress == *RightBaseAddress) { + return 0; + } else { + return 1; + } +} + +/** + Ensure the compatibility of an IO space descriptor with the IO aperture. + + The IO space descriptor can come from the GCD IO space map, or it can + represent a gap between two neighboring IO space descriptors. In the latter + case, the GcdIoType field is expected to be EfiGcdIoTypeNonExistent. + + If the IO space descriptor already has type EfiGcdIoTypeIo, then no action is + taken -- it is by definition compatible with the aperture. + + Otherwise, the intersection of the IO space descriptor is calculated with the + aperture. If the intersection is the empty set (no overlap), no action is + taken; the IO space descriptor is compatible with the aperture. + + Otherwise, the type of the descriptor is investigated again. If the type is + EfiGcdIoTypeNonExistent (representing a gap, or a genuine descriptor with + such a type), then an attempt is made to add the intersection as IO space to + the GCD IO space map. This ensures continuity for the aperture, and the + descriptor is deemed compatible with the aperture. + + Otherwise, the IO space descriptor is incompatible with the IO aperture. + + @param[in] Base Base address of the aperture. + @param[in] Length Length of the aperture. + @param[in] Descriptor The descriptor to ensure compatibility with the + aperture for. + + @retval EFI_SUCCESS The descriptor is compatible. The GCD IO space + map may have been updated, for continuity + within the aperture. + @retval EFI_INVALID_PARAMETER The descriptor is incompatible. + @return Error codes from gDS->AddIoSpace(). +**/ +EFI_STATUS +IntersectIoDescriptor ( + IN UINT64 Base, + IN UINT64 Length, + IN CONST EFI_GCD_IO_SPACE_DESCRIPTOR *Descriptor + ) +{ + UINT64 IntersectionBase; + UINT64 IntersectionEnd; + EFI_STATUS Status; + + if (Descriptor->GcdIoType == EfiGcdIoTypeIo) { + return EFI_SUCCESS; + } + + IntersectionBase = MAX (Base, Descriptor->BaseAddress); + IntersectionEnd = MIN (Base + Length, + Descriptor->BaseAddress + Descriptor->Length); + if (IntersectionBase >= IntersectionEnd) { + return EFI_SUCCESS; + } + + if (Descriptor->GcdIoType == EfiGcdIoTypeNonExistent) { + Status = gDS->AddIoSpace (EfiGcdIoTypeIo, IntersectionBase, + IntersectionEnd - IntersectionBase); + + DEBUG ((EFI_ERROR (Status) ? EFI_D_ERROR : EFI_D_VERBOSE, + "%a: %a: add [%Lx, %Lx): %r\n", gEfiCallerBaseName, __FUNCTION__, + IntersectionBase, IntersectionEnd, Status)); + return Status; + } + + return EFI_INVALID_PARAMETER; +} + +/** + Add IO space to GCD. + The routine checks the GCD database and only adds those which are + not added in the specified range to GCD. + + @param Base Base address of the IO space. + @param Length Length of the IO space. + + @retval EFI_SUCCES The IO space was added successfully. +**/ +EFI_STATUS +AddIoSpace ( + IN UINT64 Base, + IN UINT64 Length + ) +{ + EFI_STATUS Status; + UINTN Index; + UINTN NumberOfDescriptors; + EFI_GCD_IO_SPACE_DESCRIPTOR *IoSpaceMap; + CONST EFI_GCD_IO_SPACE_DESCRIPTOR *Descriptor; + UINT64 PriorDescriptorEnd; + EFI_GCD_IO_SPACE_DESCRIPTOR Gap; + + Status = gDS->GetIoSpaceMap (&NumberOfDescriptors, &IoSpaceMap); + if (EFI_ERROR (Status)) { + return Status; + } + + PerformQuickSort (IoSpaceMap, NumberOfDescriptors, sizeof (EFI_GCD_IO_SPACE_DESCRIPTOR), DescriptorComparator); + + PriorDescriptorEnd = 0; + + Gap.GcdIoType = EfiGcdIoTypeNonExistent; + Gap.ImageHandle = NULL; + Gap.DeviceHandle = NULL; + + for (Index = 0; Index < NumberOfDescriptors; Index++) { + Descriptor = &IoSpaceMap[Index]; + + // + // If there is a gap between the prior descriptor and the current one, + // check it. Note that this covers the case when the first descriptor + // begins above the start of the aperture. + // + if (PriorDescriptorEnd < Descriptor->BaseAddress) { + Gap.BaseAddress = PriorDescriptorEnd; + Gap.Length = Descriptor->BaseAddress - PriorDescriptorEnd; + + Status = IntersectIoDescriptor (Base, Length, &Gap); + if (EFI_ERROR (Status)) { + goto FreeIoSpaceMap; + } + } + + Status = IntersectIoDescriptor (Base, Length, Descriptor); + if (EFI_ERROR (Status)) { + goto FreeIoSpaceMap; + } + PriorDescriptorEnd = Descriptor->BaseAddress + Descriptor->Length; + } + + // + // Add the gap after the last descriptor. + // + Gap.BaseAddress = PriorDescriptorEnd; + Gap.Length = MAX_UINT64 - PriorDescriptorEnd; + Status = IntersectIoDescriptor (Base, Length, &Gap); + if (EFI_ERROR (Status)) { + goto FreeIoSpaceMap; + } + + DEBUG_CODE ( + // + // Make sure there are adjacent descriptors covering [Base, Base + Length). + // It is possible that they have not been merged; merging can be prevented + // by allocation. + // + UINT64 CheckBase; + UINT64 Remaining; + EFI_STATUS CheckStatus; + EFI_GCD_IO_SPACE_DESCRIPTOR Descriptor; + UINT64 DescriptorLeft; + UINT64 Step; + + CheckBase = Base; + Remaining = Length; + while (Remaining > 0) { + CheckStatus = gDS->GetIoSpaceDescriptor (CheckBase, &Descriptor); + ASSERT_EFI_ERROR (Status); + + ASSERT (Descriptor.BaseAddress <= CheckBase); + ASSERT (CheckBase < Descriptor.BaseAddress + Descriptor.Length); + DescriptorLeft = Descriptor.Length - + (CheckBase - Descriptor.BaseAddress); + + ASSERT (Descriptor.GcdIoType == EfiGcdIoTypeIo); + + Step = MIN (DescriptorLeft, Remaining); + CheckBase += Step; + Remaining -= Step; + } + ); + +FreeIoSpaceMap: + FreePool (IoSpaceMap); + + return Status; +} + +/** + Ensure the compatibility of a memory space descriptor with the MMIO aperture. + + The memory space descriptor can come from the GCD memory space map, or it can + represent a gap between two neighboring memory space descriptors. In the + latter case, the GcdMemoryType field is expected to be + EfiGcdMemoryTypeNonExistent. + + If the memory space descriptor already has type + EfiGcdMemoryTypeMemoryMappedIo, and its capabilities are a superset of the + required capabilities, then no action is taken -- it is by definition + compatible with the aperture. + + Otherwise, the intersection of the memory space descriptor is calculated with + the aperture. If the intersection is the empty set (no overlap), no action is + taken; the memory space descriptor is compatible with the aperture. + + Otherwise, the type of the descriptor is investigated again. If the type is + EfiGcdMemoryTypeNonExistent (representing a gap, or a genuine descriptor with + such a type), then an attempt is made to add the intersection as MMIO space + to the GCD memory space map, with the specified capabilities. This ensures + continuity for the aperture, and the descriptor is deemed compatible with the + aperture. + + Otherwise, the memory space descriptor is incompatible with the MMIO + aperture. + + @param[in] Base Base address of the aperture. + @param[in] Length Length of the aperture. + @param[in] Capabilities Capabilities required by the aperture. + @param[in] Descriptor The descriptor to ensure compatibility with the + aperture for. + + @retval EFI_SUCCESS The descriptor is compatible. The GCD memory + space map may have been updated, for + continuity within the aperture. + @retval EFI_INVALID_PARAMETER The descriptor is incompatible. + @return Error codes from gDS->AddMemorySpace(). +**/ +EFI_STATUS +IntersectMemoryDescriptor ( + IN UINT64 Base, + IN UINT64 Length, + IN UINT64 Capabilities, + IN CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Descriptor + ) +{ + UINT64 IntersectionBase; + UINT64 IntersectionEnd; + EFI_STATUS Status; + + if (Descriptor->GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo && + (Descriptor->Capabilities & Capabilities) == Capabilities) { + return EFI_SUCCESS; + } + + IntersectionBase = MAX (Base, Descriptor->BaseAddress); + IntersectionEnd = MIN (Base + Length, + Descriptor->BaseAddress + Descriptor->Length); + if (IntersectionBase >= IntersectionEnd) { + return EFI_SUCCESS; + } + + if (Descriptor->GcdMemoryType == EfiGcdMemoryTypeNonExistent) { + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, + IntersectionBase, IntersectionEnd - IntersectionBase, + Capabilities); + + DEBUG ((EFI_ERROR (Status) ? EFI_D_ERROR : EFI_D_VERBOSE, + "%a: %a: add [%Lx, %Lx): %r\n", gEfiCallerBaseName, __FUNCTION__, + IntersectionBase, IntersectionEnd, Status)); + return Status; + } + + return EFI_INVALID_PARAMETER; +} + +/** + Add MMIO space to GCD. + The routine checks the GCD database and only adds those which are + not added in the specified range to GCD. + + @param Base Base address of the MMIO space. + @param Length Length of the MMIO space. + @param Capabilities Capabilities of the MMIO space. + + @retval EFI_SUCCES The MMIO space was added successfully. +**/ +EFI_STATUS +AddMemoryMappedIoSpace ( + IN UINT64 Base, + IN UINT64 Length, + IN UINT64 Capabilities + ) +{ + EFI_STATUS Status; + UINTN Index; + UINTN NumberOfDescriptors; + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *MemorySpaceMap; + CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Descriptor; + UINT64 PriorDescriptorEnd; + EFI_GCD_MEMORY_SPACE_DESCRIPTOR Gap; + + Status = gDS->GetMemorySpaceMap (&NumberOfDescriptors, &MemorySpaceMap); + if (EFI_ERROR (Status)) { + return Status; + } + + PerformQuickSort (MemorySpaceMap, NumberOfDescriptors, sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR), DescriptorComparator); + + PriorDescriptorEnd = 0; + + Gap.Capabilities = 0; + Gap.Attributes = 0; + Gap.GcdMemoryType = EfiGcdMemoryTypeNonExistent; + Gap.ImageHandle = NULL; + Gap.DeviceHandle = NULL; + + for (Index = 0; Index < NumberOfDescriptors; Index++) { + Descriptor = &MemorySpaceMap[Index]; + + // + // If there is a gap between the prior descriptor and the current one, + // check it. Note that this covers the case when the first descriptor + // begins above the start of the aperture. + // + if (PriorDescriptorEnd < Descriptor->BaseAddress) { + Gap.BaseAddress = PriorDescriptorEnd; + Gap.Length = Descriptor->BaseAddress - PriorDescriptorEnd; + + Status = IntersectMemoryDescriptor (Base, Length, Capabilities, &Gap); + if (EFI_ERROR (Status)) { + goto FreeMemorySpaceMap; + } + } + + Status = IntersectMemoryDescriptor (Base, Length, Capabilities, + Descriptor); + if (EFI_ERROR (Status)) { + goto FreeMemorySpaceMap; + } + PriorDescriptorEnd = Descriptor->BaseAddress + Descriptor->Length; + } + + // + // Add the gap after the last descriptor. + // + Gap.BaseAddress = PriorDescriptorEnd; + Gap.Length = MAX_UINT64 - PriorDescriptorEnd; + Status = IntersectMemoryDescriptor (Base, Length, Capabilities, &Gap); + if (EFI_ERROR (Status)) { + goto FreeMemorySpaceMap; + } + + DEBUG_CODE ( + // + // Make sure there are adjacent descriptors covering [Base, Base + Length). + // It is possible that they have not been merged; merging can be prevented + // by allocation and different capabilities. + // + UINT64 CheckBase; + UINT64 Remaining; + EFI_STATUS CheckStatus; + EFI_GCD_MEMORY_SPACE_DESCRIPTOR Descriptor; + UINT64 DescriptorLeft; + UINT64 Step; + + CheckBase = Base; + Remaining = Length; + while (Remaining > 0) { + CheckStatus = gDS->GetMemorySpaceDescriptor (CheckBase, &Descriptor); + ASSERT_EFI_ERROR (Status); + + ASSERT (Descriptor.BaseAddress <= CheckBase); + ASSERT (CheckBase < Descriptor.BaseAddress + Descriptor.Length); + DescriptorLeft = Descriptor.Length - + (CheckBase - Descriptor.BaseAddress); + + ASSERT (Descriptor.GcdMemoryType == EfiGcdMemoryTypeMemoryMappedIo); + ASSERT ((Descriptor.Capabilities & Capabilities) == Capabilities); + + Step = MIN (DescriptorLeft, Remaining); + CheckBase += Step; + Remaining -= Step; + } + ); + +FreeMemorySpaceMap: + FreePool (MemorySpaceMap); + + return Status; +} + +/** Entry point of this driver. @@ -89,6 +496,7 @@ InitializePciHostBridge ( &gEfiPciHostBridgeResourceAllocationProtocolGuid, &HostBridge->ResAlloc, NULL ); + ASSERT_EFI_ERROR (Status); if (EFI_ERROR (Status)) { FreePool (HostBridge); PciHostBridgeFreeRootBridges (RootBridges, RootBridgeCount); @@ -109,11 +517,10 @@ InitializePciHostBridge ( } if (RootBridges[Index].Io.Limit > RootBridges[Index].Io.Base) { - Status = gDS->AddIoSpace ( - EfiGcdIoTypeIo, - RootBridges[Index].Io.Base, - RootBridges[Index].Io.Limit - RootBridges[Index].Io.Base + 1 - ); + Status = AddIoSpace ( + RootBridges[Index].Io.Base, + RootBridges[Index].Io.Limit - RootBridges[Index].Io.Base + 1 + ); ASSERT_EFI_ERROR (Status); } @@ -130,12 +537,11 @@ InitializePciHostBridge ( for (MemApertureIndex = 0; MemApertureIndex < sizeof (MemApertures) / sizeof (MemApertures[0]); MemApertureIndex++) { if (MemApertures[MemApertureIndex]->Limit > MemApertures[MemApertureIndex]->Base) { - Status = gDS->AddMemorySpace ( - EfiGcdMemoryTypeMemoryMappedIo, - MemApertures[MemApertureIndex]->Base, - MemApertures[MemApertureIndex]->Limit - MemApertures[MemApertureIndex]->Base + 1, - EFI_MEMORY_UC - ); + Status = AddMemoryMappedIoSpace ( + MemApertures[MemApertureIndex]->Base, + MemApertures[MemApertureIndex]->Limit - MemApertures[MemApertureIndex]->Base + 1, + EFI_MEMORY_UC + ); ASSERT_EFI_ERROR (Status); Status = gDS->SetMemorySpaceAttributes ( MemApertures[MemApertureIndex]->Base, -- 1.8.3.1 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
