On 05/09/16 08:26, Ruiyu Ni wrote:
> Change PciHostBridgeDxe driver to not install the
> PciHostBridgeResourceAllocation protocol and let
> PciRootBridgeIo.Configuration() return the correct PCI resource
> assignment information when the ResourceAssigned is TRUE.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <[email protected]>
> Cc: Jeff Fan <[email protected]>
> Cc: Laszlo Ersek <[email protected]>
> ---
> .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 92 +++++++++++++++------
> .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 4 +-
> .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 95
> ++++++++++++++--------
> 3 files changed, 133 insertions(+), 58 deletions(-)
This patch looks a bit more complex than the rest (although I haven't
looked at patch 6 yet), and I believe the issue that Gary reported could
require a change in this patch. So I'll postpone my review (for this
patch) until v2.
Thanks!
Laszlo
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> index 07ed54b..c866e88 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> @@ -338,6 +338,8 @@ InitializePciHostBridge (
> UINTN Index;
> PCI_ROOT_BRIDGE_APERTURE *MemApertures[4];
> UINTN MemApertureIndex;
> + BOOLEAN ResourceAssigned;
> + LIST_ENTRY *Link;
>
> RootBridges = PciHostBridgeGetRootBridges (&RootBridgeCount);
> if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
> @@ -358,27 +360,7 @@ InitializePciHostBridge (
> HostBridge->Signature = PCI_HOST_BRIDGE_SIGNATURE;
> HostBridge->CanRestarted = TRUE;
> InitializeListHead (&HostBridge->RootBridges);
> -
> - HostBridge->ResAlloc.NotifyPhase = NotifyPhase;
> - HostBridge->ResAlloc.GetNextRootBridge = GetNextRootBridge;
> - HostBridge->ResAlloc.GetAllocAttributes = GetAttributes;
> - HostBridge->ResAlloc.StartBusEnumeration = StartBusEnumeration;
> - HostBridge->ResAlloc.SetBusNumbers = SetBusNumbers;
> - HostBridge->ResAlloc.SubmitResources = SubmitResources;
> - HostBridge->ResAlloc.GetProposedResources = GetProposedResources;
> - HostBridge->ResAlloc.PreprocessController = PreprocessController;
> -
> - Status = gBS->InstallMultipleProtocolInterfaces (
> - &HostBridge->Handle,
> - &gEfiPciHostBridgeResourceAllocationProtocolGuid,
> &HostBridge->ResAlloc,
> - NULL
> - );
> - ASSERT_EFI_ERROR (Status);
> - if (EFI_ERROR (Status)) {
> - FreePool (HostBridge);
> - PciHostBridgeFreeRootBridges (RootBridges, RootBridgeCount);
> - return Status;
> - }
> + ResourceAssigned = FALSE;
>
> //
> // Create Root Bridge Device Handle in this Host Bridge
> @@ -387,18 +369,39 @@ InitializePciHostBridge (
> //
> // Create Root Bridge Handle Instance
> //
> - RootBridge = CreateRootBridge (&RootBridges[Index], HostBridge->Handle);
> + RootBridge = CreateRootBridge (&RootBridges[Index]);
> ASSERT (RootBridge != NULL);
> if (RootBridge == NULL) {
> continue;
> }
>
> + //
> + // Make sure all root bridges share the same ResourceAssigned value.
> + //
> + if (Index == 0) {
> + ResourceAssigned = RootBridges[Index].ResourceAssigned;
> + } else {
> + ASSERT (ResourceAssigned == RootBridges[Index].ResourceAssigned);
> + }
> +
> if (RootBridges[Index].Io.Base <= RootBridges[Index].Io.Limit) {
> Status = AddIoSpace (
> RootBridges[Index].Io.Base,
> RootBridges[Index].Io.Limit - RootBridges[Index].Io.Base + 1
> );
> ASSERT_EFI_ERROR (Status);
> + if (ResourceAssigned) {
> + Status = gDS->AllocateIoSpace (
> + EfiGcdAllocateAddress,
> + EfiGcdIoTypeIo,
> + 0,
> + RootBridges[Index].Io.Limit -
> RootBridges[Index].Io.Base + 1,
> + &RootBridges[Index].Io.Base,
> + gImageHandle,
> + NULL
> + );
> + ASSERT_EFI_ERROR (Status);
> + }
> }
>
> //
> @@ -428,11 +431,55 @@ InitializePciHostBridge (
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_WARN, "PciHostBridge driver failed to set
> EFI_MEMORY_UC to MMIO aperture - %r.\n", Status));
> }
> + if (ResourceAssigned) {
> + Status = gDS->AllocateMemorySpace (
> + EfiGcdAllocateAddress,
> + EfiGcdMemoryTypeMemoryMappedIo,
> + 0,
> + MemApertures[MemApertureIndex]->Limit -
> MemApertures[MemApertureIndex]->Base + 1,
> + &MemApertures[MemApertureIndex]->Base,
> + gImageHandle,
> + NULL
> + );
> + ASSERT_EFI_ERROR (Status);
> + }
> }
> }
> //
> // Insert Root Bridge Handle Instance
> //
> + InsertTailList (&HostBridge->RootBridges, &RootBridge->Link);
> + }
> +
> + //
> + // When resources were assigned, it's not needed to expose
> + // PciHostBridgeResourceAllocation protocol.
> + //
> + if (!ResourceAssigned) {
> + HostBridge->ResAlloc.NotifyPhase = NotifyPhase;
> + HostBridge->ResAlloc.GetNextRootBridge = GetNextRootBridge;
> + HostBridge->ResAlloc.GetAllocAttributes = GetAttributes;
> + HostBridge->ResAlloc.StartBusEnumeration = StartBusEnumeration;
> + HostBridge->ResAlloc.SetBusNumbers = SetBusNumbers;
> + HostBridge->ResAlloc.SubmitResources = SubmitResources;
> + HostBridge->ResAlloc.GetProposedResources = GetProposedResources;
> + HostBridge->ResAlloc.PreprocessController = PreprocessController;
> +
> + Status = gBS->InstallMultipleProtocolInterfaces (
> + &HostBridge->Handle,
> + &gEfiPciHostBridgeResourceAllocationProtocolGuid,
> &HostBridge->ResAlloc,
> + NULL
> + );
> + ASSERT_EFI_ERROR (Status);
> + }
> +
> + for (Link = GetFirstNode (&HostBridge->RootBridges)
> + ; !IsNull (&HostBridge->RootBridges, Link)
> + ; Link = GetNextNode (&HostBridge->RootBridges, Link)
> + ) {
> + RootBridge = ROOT_BRIDGE_FROM_LINK (Link);
> + RootBridge->RootBridgeIo.ParentHandle = HostBridge->Handle;
> +
> Status = gBS->InstallMultipleProtocolInterfaces (
> &RootBridge->Handle,
> &gEfiDevicePathProtocolGuid, RootBridge->DevicePath,
> @@ -440,7 +487,6 @@ InitializePciHostBridge (
> NULL
> );
> ASSERT_EFI_ERROR (Status);
> - InsertTailList (&HostBridge->RootBridges, &RootBridge->Link);
> }
> PciHostBridgeFreeRootBridges (RootBridges, RootBridgeCount);
> return Status;
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> index aa3f43a..13185b4 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> @@ -90,15 +90,13 @@ typedef struct {
> Construct the Pci Root Bridge instance.
>
> @param Bridge The root bridge instance.
> - @param HostBridgeHandle Handle to the HostBridge.
>
> @return The pointer to PCI_ROOT_BRIDGE_INSTANCE just created
> or NULL if creation fails.
> **/
> PCI_ROOT_BRIDGE_INSTANCE *
> CreateRootBridge (
> - IN PCI_ROOT_BRIDGE *Bridge,
> - IN EFI_HANDLE HostBridgeHandle
> + IN PCI_ROOT_BRIDGE *Bridge
> );
>
> //
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> index dbb415a..0bb922c 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> @@ -59,20 +59,19 @@ UINT8 mOutStride[] = {
> Construct the Pci Root Bridge instance.
>
> @param Bridge The root bridge instance.
> - @param HostBridgeHandle Handle to the HostBridge.
>
> @return The pointer to PCI_ROOT_BRIDGE_INSTANCE just created
> or NULL if creation fails.
> **/
> PCI_ROOT_BRIDGE_INSTANCE *
> CreateRootBridge (
> - IN PCI_ROOT_BRIDGE *Bridge,
> - IN EFI_HANDLE HostBridgeHandle
> + IN PCI_ROOT_BRIDGE *Bridge
> )
> {
> PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
> PCI_RESOURCE_TYPE Index;
> CHAR16 *DevicePathStr;
> + PCI_ROOT_BRIDGE_APERTURE *Aperture;
>
> DevicePathStr = NULL;
>
> @@ -120,32 +119,37 @@ CreateRootBridge (
> }
> }
>
> - if ((Bridge->AllocationAttributes & EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM)
> != 0) {
> - //
> - // If this bit is set, then the PCI Root Bridge does not
> - // support separate windows for Non-prefetchable and Prefetchable
> - // memory.
> - //
> - ASSERT (Bridge->PMem.Base > Bridge->PMem.Limit);
> - ASSERT (Bridge->PMemAbove4G.Base > Bridge->PMemAbove4G.Limit);
> - if ((Bridge->PMem.Base <= Bridge->PMem.Limit) ||
> - (Bridge->PMemAbove4G.Base <= Bridge->PMemAbove4G.Limit)
> - ) {
> - return NULL;
> + //
> + // Ignore AllocationAttributes when resources were already assigned.
> + //
> + if (!Bridge->ResourceAssigned) {
> + if ((Bridge->AllocationAttributes &
> EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM) != 0) {
> + //
> + // If this bit is set, then the PCI Root Bridge does not
> + // support separate windows for Non-prefetchable and Prefetchable
> + // memory.
> + //
> + ASSERT (Bridge->PMem.Base > Bridge->PMem.Limit);
> + ASSERT (Bridge->PMemAbove4G.Base > Bridge->PMemAbove4G.Limit);
> + if ((Bridge->PMem.Base <= Bridge->PMem.Limit) ||
> + (Bridge->PMemAbove4G.Base <= Bridge->PMemAbove4G.Limit)
> + ) {
> + return NULL;
> + }
> }
> - }
>
> - if ((Bridge->AllocationAttributes & EFI_PCI_HOST_BRIDGE_MEM64_DECODE) ==
> 0) {
> - //
> - // If this bit is not set, then the PCI Root Bridge does not support
> - // 64 bit memory windows.
> - //
> - ASSERT (Bridge->MemAbove4G.Base > Bridge->MemAbove4G.Limit);
> - ASSERT (Bridge->PMemAbove4G.Base > Bridge->PMemAbove4G.Limit);
> - if ((Bridge->MemAbove4G.Base <= Bridge->MemAbove4G.Limit) ||
> - (Bridge->PMemAbove4G.Base <= Bridge->PMemAbove4G.Limit)
> - ) {
> - return NULL;
> + if ((Bridge->AllocationAttributes & EFI_PCI_HOST_BRIDGE_MEM64_DECODE) ==
> 0) {
> + //
> + // If this bit is not set, then the PCI Root Bridge does not support
> + // 64 bit memory windows.
> + //
> + ASSERT (Bridge->MemAbove4G.Base > Bridge->MemAbove4G.Limit);
> + ASSERT (Bridge->PMemAbove4G.Base > Bridge->PMemAbove4G.Limit);
> + if ((Bridge->MemAbove4G.Base <= Bridge->MemAbove4G.Limit) ||
> + (Bridge->PMemAbove4G.Base <= Bridge->PMemAbove4G.Limit)
> + ) {
> + return NULL;
> + }
> }
> }
>
> @@ -174,14 +178,41 @@ CreateRootBridge (
> CopyMem (&RootBridge->PMemAbove4G, &Bridge->PMemAbove4G, sizeof
> (PCI_ROOT_BRIDGE_APERTURE));
>
> for (Index = TypeIo; Index < TypeMax; Index++) {
> - RootBridge->ResAllocNode[Index].Type = Index;
> - RootBridge->ResAllocNode[Index].Base = 0;
> - RootBridge->ResAllocNode[Index].Length = 0;
> - RootBridge->ResAllocNode[Index].Status = ResNone;
> + switch (Index) {
> + case TypeBus:
> + Aperture = &RootBridge->Bus;
> + break;
> + case TypeIo:
> + Aperture = &RootBridge->Io;
> + break;
> + case TypeMem32:
> + Aperture = &RootBridge->Mem;
> + break;
> + case TypeMem64:
> + Aperture = &RootBridge->MemAbove4G;
> + break;
> + case TypePMem32:
> + Aperture = &RootBridge->PMem;
> + break;
> + case TypePMem64:
> + Aperture = &RootBridge->PMemAbove4G;
> + break;
> + default:
> + break;
> + }
> + RootBridge->ResAllocNode[Index].Type = Index;
> + if (Bridge->ResourceAssigned && (Aperture->Limit >= Aperture->Base)) {
> + RootBridge->ResAllocNode[Index].Base = Aperture->Base;
> + RootBridge->ResAllocNode[Index].Length = Aperture->Limit -
> Aperture->Base + 1;
> + RootBridge->ResAllocNode[Index].Status = ResAllocated;
> + } else {
> + RootBridge->ResAllocNode[Index].Base = 0;
> + RootBridge->ResAllocNode[Index].Length = 0;
> + RootBridge->ResAllocNode[Index].Status = ResNone;
> + }
> }
>
> RootBridge->RootBridgeIo.SegmentNumber = Bridge->Segment;
> - RootBridge->RootBridgeIo.ParentHandle = HostBridgeHandle;
> RootBridge->RootBridgeIo.PollMem = RootBridgeIoPollMem;
> RootBridge->RootBridgeIo.PollIo = RootBridgeIoPollIo;
> RootBridge->RootBridgeIo.Mem.Read = RootBridgeIoMemRead;
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
