Hi Ray, I can see that you rolled most of your earlier replies into this review, except maybe the subject clarification (i.e., say "completely nonexistent" rather than "completely missing"), I will follow up here.
On 02/26/16 05:39, Ni, Ruiyu wrote: > Laszlo, > Given the fact that every byte is covered by a descriptor in GCD, > the sorting can be avoided. Indeed. The sorting is only necessary for the gap checking, and the gap checking is superfluous, because the address space is fully covered by descriptors. I *vaguely* recalled this fact from earlier GCD debug messages (even the initializtion of the GCD so that it covers the entire address space with a single nonexistent descriptor), but I wasn't sure any more and I wanted to play it safe. So, I think this is a worthwhile simplification for the patch. Thanks a lot. I'm commenting more below: > > I added detailed comments (11 in total) in below. Please check. > > I am learning how to write mail in the style you are using. > It's a totally different style:) like talking. Wow, if you can adopt that style, that would be *extremely* helpful for me. Please go ahead with it! And thanks! > >> -----Original Message----- >> From: Laszlo Ersek [mailto:[email protected]] >> Sent: Friday, February 26, 2016 8:26 AM >> To: edk2-devel-01 <[email protected]> >> Cc: Ni, Ruiyu <[email protected]> >> Subject: [RFC PATCH v2] MdeModulePkg/PciHostBridge: Don't assume resources >> are completely missing >> >> 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); > > 1). Better to add comments here to say the two ranges doesn't overlap. Will do. > >> + if (IntersectionBase >= IntersectionEnd) { >> + return EFI_SUCCESS; >> + } >> + > > 2). Can just use ASSERT (Descriptor->GcdIoType == EfiGcdIoTypeNonExistent). > because failing to add resource is a fatal error. In general, I like to propagate errors, and I prefer to avoid sticking ASSERT()s down into utility functions. If there is an error here, it will be propagated out to the outermost AddXxxxxSpace() call, and there is an ASSERT_EFI_ERROR() out there. If you like, I can add an EFI_D_ERROR message to where I return EFI_INVALID_PARAMETER, with some data printed from the descriptor. > >> + 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)); > > 3). Good to know gEfiCallerBaseName:) Yes, it's very useful. I don't remember which was the edk2 module where I saw it first. I definitely stole it from some other code! >> + 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; >> + } > > 4). ASSERT_EFI_ERROR(Status) is enough. See (2). I will add another error message here instead. > >> + >> + PerformQuickSort (IoSpaceMap, NumberOfDescriptors, sizeof >> (EFI_GCD_IO_SPACE_DESCRIPTOR), >> DescriptorComparator); >> + > > 5). No sort is needed. I agree fully. The sort is only needed for the gap checking, > >> + PriorDescriptorEnd = 0; > >> + >> + Gap.GcdIoType = EfiGcdIoTypeNonExistent; >> + Gap.ImageHandle = NULL; >> + Gap.DeviceHandle = NULL; >> + > > 6). So no need to use PriorDescriptorEnd and Gap as well. Right. > >> + 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; >> + } >> + } >> + > > 7). the above logic is not needed as well. Agreed. > >> + Status = IntersectIoDescriptor (Base, Length, Descriptor); >> + if (EFI_ERROR (Status)) { >> + goto FreeIoSpaceMap; >> + } > > 8). ASSERT is enough. See (2) -- I think I'd like to leave this as-is, because in the next version IntersectIoDescriptor() will log error messages itself. > >> + 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); > > 9). above two assertion is not necessary. GetIoSpaceDescriptor() API follows > spec. Perfect. I only spelled the above out to clarify that the calculation just below will never underflow. > >> + DescriptorLeft = Descriptor.Length - >> + (CheckBase - Descriptor.BaseAddress); >> + >> + ASSERT (Descriptor.GcdIoType == EfiGcdIoTypeIo); >> + >> + Step = MIN (DescriptorLeft, Remaining); >> + CheckBase += Step; >> + Remaining -= Step; > > 10). We can just let CheckBase = Descriptor.BaseAddress + Descriptor.Length > since > there is no gap in GCD. > So the while(Remaining > 0) can be while (CheckBase < Base + Length). You are correct. Namely, if Step = MIN (DescriptorLeft, Remaining) selects DescriptorLeft as the minimum, then the assignment CheckBase += Step is equivalent to CheckBase = CheckBase + Descriptor.Length - (CheckBase - Descriptor.BaseAddress) which is exactly equivalent to CheckBase = Descriptor.Length + Descriptor.BaseAddress which is what you propose. And, in this case, Remaining will not underflow. In the *other* case (i.e., if Remaining were chosen as minimum, for Step), your suggestion would underflow Remaining -- but, as you say, we can completely eliminate Remaining from the loop. We don't have to arrive *exactly* at the end of the aperture with our verification loop; it's perfectly fine for termination if the last descriptor overshoots it. So, I'll do this. > >> + } >> + ); >> + >> +FreeIoSpaceMap: >> + FreePool (IoSpaceMap); >> + >> + return Status; >> +} >> + > > 11). Similar comments for the MMIO operation in below. Right. So, I think that the only part where I disagree is the ASSERT()s in the deeper functions. In the next version I'd like to stick with the error propagation (because any failure will be ultimately caught anyway). But, I will add more error messages, to communicate the cause more clearly. Thanks! Laszlo > >> +/** >> + 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
