From b3800cfd10abe8033e16147ad16b01cd16aae0d7 Mon Sep 17 00:00:00 2001 From: Ruiyu Ni Date: Tue, 3 Nov 2015 02:33:05 +0000 Subject: [PATCH] Revert "MdeModulePkg: Fix a PciBusDxe hot plug bug" Leif suggested to split the big patch to smaller ones. This reverts commit 73b7f115c653c807b9d0be97bf516871d8aff7ba. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ruiyu Ni Reviewed-by: Feng Tian git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18717 6f19259b-4bc3-4df7-8a09-765794883524 --- .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 80 +-------- .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.h | 13 -- MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 159 +++++++++--------- .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 58 +++---- 4 files changed, 102 insertions(+), 208 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c index a6ade26e3a..f7aea4fd80 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c @@ -324,81 +324,6 @@ PciSearchDevice ( return EFI_SUCCESS; } -/** - Dump the PPB padding resource information. - - @param PciIoDevice PCI IO instance. - @param ResourceType The desired resource type to dump. - PciBarTypeUnknown means to dump all types of resources. -**/ -VOID -DumpPpbPaddingResource ( - IN PCI_IO_DEVICE *PciIoDevice, - IN PCI_BAR_TYPE ResourceType - ) -{ - EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor; - PCI_BAR_TYPE Type; - - if (PciIoDevice->ResourcePaddingDescriptors == NULL) { - return; - } - - if (ResourceType == PciBarTypeIo16 || ResourceType == PciBarTypeIo32) { - ResourceType = PciBarTypeIo; - } - - for (Descriptor = PciIoDevice->ResourcePaddingDescriptors; Descriptor->Desc != ACPI_END_TAG_DESCRIPTOR; Descriptor++) { - - Type = PciBarTypeUnknown; - if (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR && Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_IO) { - Type = PciBarTypeIo; - } else if (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR && Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) { - - if (Descriptor->AddrSpaceGranularity == 32) { - // - // prefechable - // - if (Descriptor->SpecificFlag == EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE) { - Type = PciBarTypePMem32; - } - - // - // Non-prefechable - // - if (Descriptor->SpecificFlag == 0) { - Type = PciBarTypeMem32; - } - } - - if (Descriptor->AddrSpaceGranularity == 64) { - // - // prefechable - // - if (Descriptor->SpecificFlag == EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE) { - Type = PciBarTypePMem64; - } - - // - // Non-prefechable - // - if (Descriptor->SpecificFlag == 0) { - Type = PciBarTypeMem64; - } - } - } - - if ((Type != PciBarTypeUnknown) && ((ResourceType == PciBarTypeUnknown) || (ResourceType == Type))) { - DEBUG (( - EFI_D_INFO, - " Padding: Type = %s; Alignment = 0x%lx;\tLength = 0x%lx\n", - mBarTypeStr[Type], Descriptor->AddrRangeMax, Descriptor->AddrLen - )); - } - } - -} - /** Dump the PCI BAR information. @@ -661,10 +586,7 @@ GatherPpbInfo ( GetResourcePaddingPpb (PciIoDevice); - DEBUG_CODE ( - DumpPpbPaddingResource (PciIoDevice, PciBarTypeUnknown); - DumpPciBars (PciIoDevice); - ); + DEBUG_CODE (DumpPciBars (PciIoDevice);); return PciIoDevice; } diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h index 4d7b3b754a..a4489b895f 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h @@ -460,17 +460,4 @@ ResetAllPpbBusNumber ( IN UINT8 StartBusNumber ); -/** - Dump the PPB padding resource information. - - @param PciIoDevice PCI IO instance. - @param ResourceType The desired resource type to dump. - PciBarTypeUnknown means to dump all types of resources. -**/ -VOID -DumpPpbPaddingResource ( - IN PCI_IO_DEVICE *PciIoDevice, - IN PCI_BAR_TYPE ResourceType - ); - #endif diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c index f4b6ebfc0d..3e275e34ec 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c @@ -188,21 +188,19 @@ DumpBridgeResource ( BridgeResource->PciDev->PciBar[BridgeResource->Bar].BaseAddress, BridgeResource->Length, BridgeResource->Alignment )); - for ( Link = GetFirstNode (&BridgeResource->ChildList) - ; !IsNull (&BridgeResource->ChildList, Link) - ; Link = GetNextNode (&BridgeResource->ChildList, Link) + for ( Link = BridgeResource->ChildList.ForwardLink + ; Link != &BridgeResource->ChildList + ; Link = Link->ForwardLink ) { Resource = RESOURCE_NODE_FROM_LINK (Link); if (Resource->ResourceUsage == PciResUsageTypical) { Bar = Resource->Virtual ? Resource->PciDev->VfPciBar : Resource->PciDev->PciBar; DEBUG (( - EFI_D_INFO, " Base = 0x%lx;\tLength = 0x%lx;\tAlignment = 0x%lx;\tOwner = %s [%02x|%02x|%02x:", + EFI_D_INFO, " Base = 0x%lx;\tLength = 0x%lx;\tAlignment = 0x%lx;\tOwner = %s ", Bar[Resource->Bar].BaseAddress, Resource->Length, Resource->Alignment, IS_PCI_BRIDGE (&Resource->PciDev->Pci) ? L"PPB" : IS_CARDBUS_BRIDGE (&Resource->PciDev->Pci) ? L"P2C" : - L"PCI", - Resource->PciDev->BusNumber, Resource->PciDev->DeviceNumber, - Resource->PciDev->FunctionNumber + L"PCI" )); if ((!IS_PCI_BRIDGE (&Resource->PciDev->Pci) && !IS_CARDBUS_BRIDGE (&Resource->PciDev->Pci)) || @@ -212,20 +210,24 @@ DumpBridgeResource ( // // The resource requirement comes from the device itself. // - DEBUG ((EFI_D_INFO, "%02x]", Bar[Resource->Bar].Offset)); + DEBUG (( + EFI_D_INFO, " [%02x|%02x|%02x:%02x]\n", + Resource->PciDev->BusNumber, Resource->PciDev->DeviceNumber, + Resource->PciDev->FunctionNumber, Bar[Resource->Bar].Offset + )); } else { // // The resource requirement comes from the subordinate devices. // - DEBUG ((EFI_D_INFO, "**]")); + DEBUG (( + EFI_D_INFO, " [%02x|%02x|%02x:**]\n", + Resource->PciDev->BusNumber, Resource->PciDev->DeviceNumber, + Resource->PciDev->FunctionNumber + )); } } else { - DEBUG ((EFI_D_INFO, " Base = Padding;\tLength = 0x%lx;\tAlignment = 0x%lx", Resource->Length, Resource->Alignment)); + DEBUG ((EFI_D_INFO, " Padding:Length = 0x%lx;\tAlignment = 0x%lx\n", Resource->Length, Resource->Alignment)); } - if (BridgeResource->ResType != Resource->ResType) { - DEBUG ((EFI_D_INFO, "; Type = %s", mBarTypeStr[MIN (Resource->ResType, PciBarTypeMaxType)])); - } - DEBUG ((EFI_D_INFO, "\n")); } } } @@ -233,61 +235,63 @@ DumpBridgeResource ( /** Find the corresponding resource node for the Device in child list of BridgeResource. - @param[in] Device Pointer to PCI_IO_DEVICE. - @param[in] BridgeResource Pointer to PCI_RESOURCE_NODE. - @param[out] DeviceResources Pointer to a buffer to receive resources for the Device. + @param[in] Device Pointer to PCI_IO_DEVICE. + @param[in] BridgeResource Pointer to PCI_RESOURCE_NODE. - @return Count of the resource descriptors returned. + @return !NULL The corresponding resource node for the Device. + @return NULL No corresponding resource node for the Device. **/ -UINTN +PCI_RESOURCE_NODE * FindResourceNode ( - IN PCI_IO_DEVICE *Device, - IN PCI_RESOURCE_NODE *BridgeResource, - OUT PCI_RESOURCE_NODE **DeviceResources OPTIONAL + IN PCI_IO_DEVICE *Device, + IN PCI_RESOURCE_NODE *BridgeResource ) { LIST_ENTRY *Link; PCI_RESOURCE_NODE *Resource; - UINTN Count; - Count = 0; for ( Link = BridgeResource->ChildList.ForwardLink ; Link != &BridgeResource->ChildList ; Link = Link->ForwardLink ) { Resource = RESOURCE_NODE_FROM_LINK (Link); if (Resource->PciDev == Device) { - if (DeviceResources != NULL) { - DeviceResources[Count] = Resource; - } - Count++; + return Resource; } } - return Count; + return NULL; } /** Dump the resource map of all the devices under Bridge. - @param[in] Bridge Bridge device instance. - @param[in] Resources Resource descriptors for the bridge device. - @param[in] ResourceCount Count of resource descriptors. + @param[in] Bridge Bridge device instance. + @param[in] IoNode IO resource descriptor for the bridge device. + @param[in] Mem32Node Mem32 resource descriptor for the bridge device. + @param[in] PMem32Node PMem32 resource descriptor for the bridge device. + @param[in] Mem64Node Mem64 resource descriptor for the bridge device. + @param[in] PMem64Node PMem64 resource descriptor for the bridge device. **/ VOID DumpResourceMap ( IN PCI_IO_DEVICE *Bridge, - IN PCI_RESOURCE_NODE **Resources, - IN UINTN ResourceCount + IN PCI_RESOURCE_NODE *IoNode, + IN PCI_RESOURCE_NODE *Mem32Node, + IN PCI_RESOURCE_NODE *PMem32Node, + IN PCI_RESOURCE_NODE *Mem64Node, + IN PCI_RESOURCE_NODE *PMem64Node ) { - EFI_STATUS Status; - LIST_ENTRY *Link; - PCI_IO_DEVICE *Device; - UINTN Index; - CHAR16 *Str; - PCI_RESOURCE_NODE **ChildResources; - UINTN ChildResourceCount; + EFI_STATUS Status; + LIST_ENTRY *Link; + PCI_IO_DEVICE *Device; + PCI_RESOURCE_NODE *ChildIoNode; + PCI_RESOURCE_NODE *ChildMem32Node; + PCI_RESOURCE_NODE *ChildPMem32Node; + PCI_RESOURCE_NODE *ChildMem64Node; + PCI_RESOURCE_NODE *ChildPMem64Node; + CHAR16 *Str; DEBUG ((EFI_D_INFO, "PciBus: Resource Map for ")); @@ -316,9 +320,11 @@ DumpResourceMap ( } } - for (Index = 0; Index < ResourceCount; Index++) { - DumpBridgeResource (Resources[Index]); - } + DumpBridgeResource (IoNode); + DumpBridgeResource (Mem32Node); + DumpBridgeResource (PMem32Node); + DumpBridgeResource (Mem64Node); + DumpBridgeResource (PMem64Node); DEBUG ((EFI_D_INFO, "\n")); for ( Link = Bridge->ChildList.ForwardLink @@ -328,19 +334,20 @@ DumpResourceMap ( Device = PCI_IO_DEVICE_FROM_LINK (Link); if (IS_PCI_BRIDGE (&Device->Pci)) { - ChildResourceCount = 0; - for (Index = 0; Index < ResourceCount; Index++) { - ChildResourceCount += FindResourceNode (Device, Resources[Index], NULL); - } - ChildResources = AllocatePool (sizeof (PCI_RESOURCE_NODE *) * ChildResourceCount); - ASSERT (ChildResources != NULL); - ChildResourceCount = 0; - for (Index = 0; Index < ResourceCount; Index++) { - ChildResourceCount += FindResourceNode (Device, Resources[Index], &ChildResources[ChildResourceCount]); - } - - DumpResourceMap (Device, ChildResources, ChildResourceCount); - FreePool (ChildResources); + ChildIoNode = (IoNode == NULL ? NULL : FindResourceNode (Device, IoNode)); + ChildMem32Node = (Mem32Node == NULL ? NULL : FindResourceNode (Device, Mem32Node)); + ChildPMem32Node = (PMem32Node == NULL ? NULL : FindResourceNode (Device, PMem32Node)); + ChildMem64Node = (Mem64Node == NULL ? NULL : FindResourceNode (Device, Mem64Node)); + ChildPMem64Node = (PMem64Node == NULL ? NULL : FindResourceNode (Device, PMem64Node)); + + DumpResourceMap ( + Device, + ChildIoNode, + ChildMem32Node, + ChildPMem32Node, + ChildMem64Node, + ChildPMem64Node + ); } } } @@ -800,11 +807,11 @@ PciHostBridgeResourceAllocator ( // Create the entire system resource map from the information collected by // enumerator. Several resource tree was created // - FindResourceNode (RootBridgeDev, &IoPool, &IoBridge); - FindResourceNode (RootBridgeDev, &Mem32Pool, &Mem32Bridge); - FindResourceNode (RootBridgeDev, &PMem32Pool, &PMem32Bridge); - FindResourceNode (RootBridgeDev, &Mem64Pool, &Mem64Bridge); - FindResourceNode (RootBridgeDev, &PMem64Pool, &PMem64Bridge); + IoBridge = FindResourceNode (RootBridgeDev, &IoPool); + Mem32Bridge = FindResourceNode (RootBridgeDev, &Mem32Pool); + PMem32Bridge = FindResourceNode (RootBridgeDev, &PMem32Pool); + Mem64Bridge = FindResourceNode (RootBridgeDev, &Mem64Pool); + PMem64Bridge = FindResourceNode (RootBridgeDev, &PMem64Pool); ASSERT (IoBridge != NULL); ASSERT (Mem32Bridge != NULL); @@ -862,13 +869,14 @@ PciHostBridgeResourceAllocator ( // Dump the resource map for current root bridge // DEBUG_CODE ( - PCI_RESOURCE_NODE *Resources[5]; - Resources[0] = IoBridge; - Resources[1] = Mem32Bridge; - Resources[2] = PMem32Bridge; - Resources[3] = Mem64Bridge; - Resources[4] = PMem64Bridge; - DumpResourceMap (RootBridgeDev, Resources, sizeof (Resources) / sizeof (Resources[0])); + DumpResourceMap ( + RootBridgeDev, + IoBridge, + Mem32Bridge, + PMem32Bridge, + Mem64Bridge, + PMem64Bridge + ); ); FreePool (AcpiConfig); @@ -976,8 +984,7 @@ PciScanBus ( UINT8 Device; UINT8 Func; UINT64 Address; - UINT8 SecondBus; - UINT8 PaddedSubBus; + UINTN SecondBus; UINT16 Register; UINTN HpIndex; PCI_IO_DEVICE *PciDevice; @@ -1211,7 +1218,7 @@ PciScanBus ( Status = PciScanBus ( PciDevice, - SecondBus, + (UINT8) (SecondBus), SubBusNumber, PaddedBusRange ); @@ -1227,16 +1234,12 @@ PciScanBus ( if ((Attributes == EfiPaddingPciRootBridge) && (State & EFI_HPC_STATE_ENABLED) != 0 && (State & EFI_HPC_STATE_INITIALIZED) != 0) { - *PaddedBusRange = (UINT8) ((UINT8) (BusRange) + *PaddedBusRange); + *PaddedBusRange = (UINT8) ((UINT8) (BusRange) +*PaddedBusRange); } else { - // - // Reserve the larger one between the actual occupied bus number and padded bus number - // - Status = PciAllocateBusNumber (PciDevice, StartBusNumber, (UINT8) (BusRange), &PaddedSubBus); + Status = PciAllocateBusNumber (PciDevice, *SubBusNumber, (UINT8) (BusRange), SubBusNumber); if (EFI_ERROR (Status)) { return Status; } - *SubBusNumber = MAX (PaddedSubBus, *SubBusNumber); } } diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c index b106abe0fa..d8d988cbfc 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c @@ -196,7 +196,6 @@ CalculateApertureIo16 ( PCI_RESOURCE_NODE *Node; UINT64 Offset; EFI_PCI_PLATFORM_POLICY PciPolicy; - UINT64 PaddingAperture; if (!mPolicyDetermined) { // @@ -229,27 +228,21 @@ CalculateApertureIo16 ( mPolicyDetermined = TRUE; } - Aperture = 0; - PaddingAperture = 0; + Aperture = 0; if (Bridge == NULL) { return ; } + CurrentLink = Bridge->ChildList.ForwardLink; + // // Assume the bridge is aligned // - for ( CurrentLink = GetFirstNode (&Bridge->ChildList) - ; !IsNull (&Bridge->ChildList, CurrentLink) - ; CurrentLink = GetNextNode (&Bridge->ChildList, CurrentLink) - ) { + while (CurrentLink != &Bridge->ChildList) { Node = RESOURCE_NODE_FROM_LINK (CurrentLink); - if (Node->ResourceUsage == PciResUsagePadding) { - ASSERT (PaddingAperture == 0); - PaddingAperture = Node->Length; - continue; - } + // // Consider the aperture alignment // @@ -300,10 +293,13 @@ CalculateApertureIo16 ( // Increment aperture by the length of node // Aperture += Node->Length; + + CurrentLink = CurrentLink->ForwardLink; } // - // Adjust the aperture with the bridge's alignment + // At last, adjust the aperture with the bridge's + // alignment // Offset = Aperture & (Bridge->Alignment); @@ -323,12 +319,6 @@ CalculateApertureIo16 ( Bridge->Alignment = Node->Alignment; } } - - // - // Hotplug controller needs padding resources. - // Use the larger one between the padding resource and actual occupied resource. - // - Bridge->Length = MAX (Bridge->Length, PaddingAperture); } /** @@ -346,11 +336,10 @@ CalculateResourceAperture ( UINT64 Aperture; LIST_ENTRY *CurrentLink; PCI_RESOURCE_NODE *Node; - UINT64 PaddingAperture; + UINT64 Offset; - Aperture = 0; - PaddingAperture = 0; + Aperture = 0; if (Bridge == NULL) { return ; @@ -362,20 +351,14 @@ CalculateResourceAperture ( return ; } + CurrentLink = Bridge->ChildList.ForwardLink; + // // Assume the bridge is aligned // - for ( CurrentLink = GetFirstNode (&Bridge->ChildList) - ; !IsNull (&Bridge->ChildList, CurrentLink) - ; CurrentLink = GetNextNode (&Bridge->ChildList, CurrentLink) - ) { + while (CurrentLink != &Bridge->ChildList) { Node = RESOURCE_NODE_FROM_LINK (CurrentLink); - if (Node->ResourceUsage == PciResUsagePadding) { - ASSERT (PaddingAperture == 0); - PaddingAperture = Node->Length; - continue; - } // // Apply padding resource if available @@ -398,6 +381,11 @@ CalculateResourceAperture ( // Increment aperture by the length of node // Aperture += Node->Length; + + // + // Consider the aperture alignment + // + CurrentLink = CurrentLink->ForwardLink; } // @@ -419,7 +407,7 @@ CalculateResourceAperture ( } // - // Adjust the bridge's alignment to the first child's alignment + // At last, adjust the bridge's alignment to the first child's alignment // if the bridge has at least one child // CurrentLink = Bridge->ChildList.ForwardLink; @@ -429,12 +417,6 @@ CalculateResourceAperture ( Bridge->Alignment = Node->Alignment; } } - - // - // Hotplug controller needs padding resources. - // Use the larger one between the padding resource and actual occupied resource. - // - Bridge->Length = MAX (Bridge->Length, PaddingAperture); } /** -- 2.39.2