Leif, Please test the patch I just sent to see if it resolves your issue.
Thanks, Mike >-----Original Message----- >From: edk2-devel [mailto:[email protected]] On Behalf Of Leif >Lindholm >Sent: Wednesday, October 28, 2015 7:51 AM >To: Tian, Feng >Cc: Ni, Ruiyu; [email protected]; Fan, Jeff >Subject: Re: [edk2] [Patch] MdeModulePkg: Fix a PciBusDxe hot plug bug > >Feng, > >Any update on the below? >The hard crash bug is still in SVN r18690. > >Regards, > >Leif > >On Mon, Oct 26, 2015 at 03:35:15PM +0000, Leif Lindholm wrote: >> Hi Ray, >> >> On Mon, Oct 26, 2015 at 03:17:22PM +0000, Ni, Ruiyu wrote: >> > Can you add a null pointer check to >> > PciIoDevice->ResourcePaddingDescriptors before calling >> > DumpPpbPaddingResource? Does it help? >> >> It removes the delay and the crash - thanks! >> >> But it does nothing for the commit history ;) >> >> Regards, >> >> Leif >> >> > > 在 2015年10月26日，21:13，Leif Lindholm <[email protected]> >写道： >> > > >> > > Hi Ruiyu, Feng, >> > > >> > > I am currently tracking down an issue on (at least) one of my >> > > platforms - that happens with this (now committed) patch, but not >> > > without it. >> > > >> > > Symptoms are a _long_ delay, followed by an unaligned access in (I >> > > think) DumpPpbPaddingResource. >> > > >> > > Anyway, there could be other things playing in here - I'm testing a >> > > new card in a platform I haven't previously tested cards. >> > > >> > > But this is one large patch, which could have been split up into >> > > multiple ones to make the introduced changes more reviewable: >> > > - There are both functional changes and whitespace fixups. >> > > - There are (text-only) changes to existing messages. >> > > - There is refactoring of internal APIs. >> > > >> > > It is certainly too invasive a change to be committed ~32 minutes >> > > after having first been posted to the list. >> > > >> > > Any chance we can revert this one and introduce it in smaller >> > > portions, making the individual change sets more reviewable? >> > > >> > > Regards, >> > > >> > > Leif >> > > >> > >> On Fri, Oct 23, 2015 at 03:57:40PM +0800, Ruiyu Ni wrote: >> > >> For a hot plug bridge with device attached, PciBusDxe driver reserves >> > >> the resources which equal to the total amount of padding resource >> > >> returned from HotPlug->GetResourcePadding() and the actual occupied >> > >> resource by the attached device. The behavior is incorrect. >> > >> Correct behavior is to reserve the bigger one between the padding >> > >> resource and the actual occupied resource. >> > >> >> > >> Contributed-under: TianoCore Contribution Agreement 1.0 >> > >> Signed-off-by: Ruiyu Ni <[email protected]> >> > >> Cc: Jeff Fan <[email protected]> >> > >> Cc: Feng Tian <[email protected]> >> > >> --- >> > >> .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c | 76 +++++++++- >> > >> .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.h | 13 ++ >> > >> MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 159 ++++++++++----- >------ >> > >> .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 58 +++++--- >> > >> 4 files changed, 204 insertions(+), 102 deletions(-) >> > >> >> > >> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c >b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c >> > >> index f7aea4f..030ef42 100644 >> > >> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c >> > >> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c >> > >> @@ -325,6 +325,77 @@ PciSearchDevice ( >> > >> } >> > >> >> > >> /** >> > >> + 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 (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. >> > >> >> > >> @param PciIoDevice PCI IO instance. >> > >> @@ -586,7 +657,10 @@ GatherPpbInfo ( >> > >> >> > >> GetResourcePaddingPpb (PciIoDevice); >> > >> >> > >> - DEBUG_CODE (DumpPciBars (PciIoDevice);); >> > >> + DEBUG_CODE ( >> > >> + DumpPpbPaddingResource (PciIoDevice, PciBarTypeUnknown); >> > >> + DumpPciBars (PciIoDevice); >> > >> + ); >> > >> >> > >> return PciIoDevice; >> > >> } >> > >> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h >b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h >> > >> index a4489b8..4d7b3b7 100644 >> > >> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h >> > >> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.h >> > >> @@ -460,4 +460,17 @@ 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 3e275e3..f4b6ebf 100644 >> > >> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c >> > >> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c >> > >> @@ -188,19 +188,21 @@ DumpBridgeResource ( >> > >> BridgeResource->PciDev->PciBar[BridgeResource->Bar].BaseAddress, >> > >> BridgeResource->Length, BridgeResource->Alignment >> > >> )); >> > >> - for ( Link = BridgeResource->ChildList.ForwardLink >> > >> - ; Link != &BridgeResource->ChildList >> > >> - ; Link = Link->ForwardLink >> > >> + for ( Link = GetFirstNode (&BridgeResource->ChildList) >> > >> + ; !IsNull (&BridgeResource->ChildList, Link) >> > >> + ; Link = GetNextNode (&BridgeResource->ChildList, Link) >> > >> ) { >> > >> 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 ", >> > >> + EFI_D_INFO, " Base = 0x%lx;\tLength = 0x%lx;\tAlignment = >0x%lx;\tOwner = %s [%02x|%02x|%02x:", >> > >> 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" >> > >> + L"PCI", >> > >> + Resource->PciDev->BusNumber, Resource->PciDev- >>DeviceNumber, >> > >> + Resource->PciDev->FunctionNumber >> > >> )); >> > >> >> > >> if ((!IS_PCI_BRIDGE (&Resource->PciDev->Pci) && >!IS_CARDBUS_BRIDGE (&Resource->PciDev->Pci)) || >> > >> @@ -210,24 +212,20 @@ DumpBridgeResource ( >> > >> // >> > >> // The resource requirement comes from the device itself. >> > >> // >> > >> - DEBUG (( >> > >> - EFI_D_INFO, " [%02x|%02x|%02x:%02x]\n", >> > >> - Resource->PciDev->BusNumber, Resource->PciDev- >>DeviceNumber, >> > >> - Resource->PciDev->FunctionNumber, Bar[Resource->Bar].Offset >> > >> - )); >> > >> + DEBUG ((EFI_D_INFO, "%02x]", Bar[Resource->Bar].Offset)); >> > >> } else { >> > >> // >> > >> // The resource requirement comes from the subordinate >devices. >> > >> // >> > >> - DEBUG (( >> > >> - EFI_D_INFO, " [%02x|%02x|%02x:**]\n", >> > >> - Resource->PciDev->BusNumber, Resource->PciDev- >>DeviceNumber, >> > >> - Resource->PciDev->FunctionNumber >> > >> - )); >> > >> + DEBUG ((EFI_D_INFO, "**]")); >> > >> } >> > >> } else { >> > >> - DEBUG ((EFI_D_INFO, " Padding:Length = 0x%lx;\tAlignment = >0x%lx\n", Resource->Length, Resource->Alignment)); >> > >> + DEBUG ((EFI_D_INFO, " Base = Padding;\tLength = >0x%lx;\tAlignment = 0x%lx", 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")); >> > >> } >> > >> } >> > >> } >> > >> @@ -235,63 +233,61 @@ 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[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. >> > >> >> > >> - @return !NULL The corresponding resource node for the Device. >> > >> - @return NULL No corresponding resource node for the Device. >> > >> + @return Count of the resource descriptors returned. >> > >> **/ >> > >> -PCI_RESOURCE_NODE * >> > >> +UINTN >> > >> FindResourceNode ( >> > >> - IN PCI_IO_DEVICE *Device, >> > >> - IN PCI_RESOURCE_NODE *BridgeResource >> > >> + IN PCI_IO_DEVICE *Device, >> > >> + IN PCI_RESOURCE_NODE *BridgeResource, >> > >> + OUT PCI_RESOURCE_NODE **DeviceResources OPTIONAL >> > >> ) >> > >> { >> > >> 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) { >> > >> - return Resource; >> > >> + if (DeviceResources != NULL) { >> > >> + DeviceResources[Count] = Resource; >> > >> + } >> > >> + Count++; >> > >> } >> > >> } >> > >> >> > >> - return NULL; >> > >> + return Count; >> > >> } >> > >> >> > >> /** >> > >> Dump the resource map of all the devices under Bridge. >> > >> >> > >> - @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. >> > >> + @param[in] Bridge Bridge device instance. >> > >> + @param[in] Resources Resource descriptors for the bridge device. >> > >> + @param[in] ResourceCount Count of resource descriptors. >> > >> **/ >> > >> VOID >> > >> DumpResourceMap ( >> > >> IN PCI_IO_DEVICE *Bridge, >> > >> - 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 >> > >> + IN PCI_RESOURCE_NODE **Resources, >> > >> + IN UINTN ResourceCount >> > >> ) >> > >> { >> > >> - 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; >> > >> + EFI_STATUS Status; >> > >> + LIST_ENTRY *Link; >> > >> + PCI_IO_DEVICE *Device; >> > >> + UINTN Index; >> > >> + CHAR16 *Str; >> > >> + PCI_RESOURCE_NODE **ChildResources; >> > >> + UINTN ChildResourceCount; >> > >> >> > >> DEBUG ((EFI_D_INFO, "PciBus: Resource Map for ")); >> > >> >> > >> @@ -320,11 +316,9 @@ DumpResourceMap ( >> > >> } >> > >> } >> > >> >> > >> - DumpBridgeResource (IoNode); >> > >> - DumpBridgeResource (Mem32Node); >> > >> - DumpBridgeResource (PMem32Node); >> > >> - DumpBridgeResource (Mem64Node); >> > >> - DumpBridgeResource (PMem64Node); >> > >> + for (Index = 0; Index < ResourceCount; Index++) { >> > >> + DumpBridgeResource (Resources[Index]); >> > >> + } >> > >> DEBUG ((EFI_D_INFO, "\n")); >> > >> >> > >> for ( Link = Bridge->ChildList.ForwardLink >> > >> @@ -334,20 +328,19 @@ DumpResourceMap ( >> > >> Device = PCI_IO_DEVICE_FROM_LINK (Link); >> > >> if (IS_PCI_BRIDGE (&Device->Pci)) { >> > >> >> > >> - 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 >> > >> - ); >> > >> + 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); >> > >> } >> > >> } >> > >> } >> > >> @@ -807,11 +800,11 @@ PciHostBridgeResourceAllocator ( >> > >> // Create the entire system resource map from the information >collected by >> > >> // enumerator. Several resource tree was created >> > >> // >> > >> - IoBridge = FindResourceNode (RootBridgeDev, &IoPool); >> > >> - Mem32Bridge = FindResourceNode (RootBridgeDev, &Mem32Pool); >> > >> - PMem32Bridge = FindResourceNode (RootBridgeDev, >&PMem32Pool); >> > >> - Mem64Bridge = FindResourceNode (RootBridgeDev, &Mem64Pool); >> > >> - PMem64Bridge = FindResourceNode (RootBridgeDev, >&PMem64Pool); >> > >> + FindResourceNode (RootBridgeDev, &IoPool, &IoBridge); >> > >> + FindResourceNode (RootBridgeDev, &Mem32Pool, &Mem32Bridge); >> > >> + FindResourceNode (RootBridgeDev, &PMem32Pool, >&PMem32Bridge); >> > >> + FindResourceNode (RootBridgeDev, &Mem64Pool, &Mem64Bridge); >> > >> + FindResourceNode (RootBridgeDev, &PMem64Pool, >&PMem64Bridge); >> > >> >> > >> ASSERT (IoBridge != NULL); >> > >> ASSERT (Mem32Bridge != NULL); >> > >> @@ -869,14 +862,13 @@ PciHostBridgeResourceAllocator ( >> > >> // Dump the resource map for current root bridge >> > >> // >> > >> DEBUG_CODE ( >> > >> - DumpResourceMap ( >> > >> - RootBridgeDev, >> > >> - IoBridge, >> > >> - Mem32Bridge, >> > >> - PMem32Bridge, >> > >> - Mem64Bridge, >> > >> - PMem64Bridge >> > >> - ); >> > >> + 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])); >> > >> ); >> > >> >> > >> FreePool (AcpiConfig); >> > >> @@ -984,7 +976,8 @@ PciScanBus ( >> > >> UINT8 Device; >> > >> UINT8 Func; >> > >> UINT64 Address; >> > >> - UINTN SecondBus; >> > >> + UINT8 SecondBus; >> > >> + UINT8 PaddedSubBus; >> > >> UINT16 Register; >> > >> UINTN HpIndex; >> > >> PCI_IO_DEVICE *PciDevice; >> > >> @@ -1218,7 +1211,7 @@ PciScanBus ( >> > >> >> > >> Status = PciScanBus ( >> > >> PciDevice, >> > >> - (UINT8) (SecondBus), >> > >> + SecondBus, >> > >> SubBusNumber, >> > >> PaddedBusRange >> > >> ); >> > >> @@ -1234,12 +1227,16 @@ 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 { >> > >> - Status = PciAllocateBusNumber (PciDevice, *SubBusNumber, >(UINT8) (BusRange), SubBusNumber); >> > >> + // >> > >> + // Reserve the larger one between the actual occupied bus >number and padded bus number >> > >> + // >> > >> + Status = PciAllocateBusNumber (PciDevice, StartBusNumber, >(UINT8) (BusRange), &PaddedSubBus); >> > >> 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 d8d988c..b106abe 100644 >> > >> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c >> > >> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c >> > >> @@ -196,6 +196,7 @@ CalculateApertureIo16 ( >> > >> PCI_RESOURCE_NODE *Node; >> > >> UINT64 Offset; >> > >> EFI_PCI_PLATFORM_POLICY PciPolicy; >> > >> + UINT64 PaddingAperture; >> > >> >> > >> if (!mPolicyDetermined) { >> > >> // >> > >> @@ -228,21 +229,27 @@ CalculateApertureIo16 ( >> > >> mPolicyDetermined = TRUE; >> > >> } >> > >> >> > >> - Aperture = 0; >> > >> + Aperture = 0; >> > >> + PaddingAperture = 0; >> > >> >> > >> if (Bridge == NULL) { >> > >> return ; >> > >> } >> > >> >> > >> - CurrentLink = Bridge->ChildList.ForwardLink; >> > >> - >> > >> // >> > >> // Assume the bridge is aligned >> > >> // >> > >> - while (CurrentLink != &Bridge->ChildList) { >> > >> + for ( CurrentLink = GetFirstNode (&Bridge->ChildList) >> > >> + ; !IsNull (&Bridge->ChildList, CurrentLink) >> > >> + ; CurrentLink = GetNextNode (&Bridge->ChildList, CurrentLink) >> > >> + ) { >> > >> >> > >> Node = RESOURCE_NODE_FROM_LINK (CurrentLink); >> > >> - >> > >> + if (Node->ResourceUsage == PciResUsagePadding) { >> > >> + ASSERT (PaddingAperture == 0); >> > >> + PaddingAperture = Node->Length; >> > >> + continue; >> > >> + } >> > >> // >> > >> // Consider the aperture alignment >> > >> // >> > >> @@ -293,13 +300,10 @@ CalculateApertureIo16 ( >> > >> // Increment aperture by the length of node >> > >> // >> > >> Aperture += Node->Length; >> > >> - >> > >> - CurrentLink = CurrentLink->ForwardLink; >> > >> } >> > >> >> > >> // >> > >> - // At last, adjust the aperture with the bridge's >> > >> - // alignment >> > >> + // Adjust the aperture with the bridge's alignment >> > >> // >> > >> Offset = Aperture & (Bridge->Alignment); >> > >> >> > >> @@ -319,6 +323,12 @@ 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); >> > >> } >> > >> >> > >> /** >> > >> @@ -336,10 +346,11 @@ CalculateResourceAperture ( >> > >> UINT64 Aperture; >> > >> LIST_ENTRY *CurrentLink; >> > >> PCI_RESOURCE_NODE *Node; >> > >> - >> > >> + UINT64 PaddingAperture; >> > >> UINT64 Offset; >> > >> >> > >> - Aperture = 0; >> > >> + Aperture = 0; >> > >> + PaddingAperture = 0; >> > >> >> > >> if (Bridge == NULL) { >> > >> return ; >> > >> @@ -351,14 +362,20 @@ CalculateResourceAperture ( >> > >> return ; >> > >> } >> > >> >> > >> - CurrentLink = Bridge->ChildList.ForwardLink; >> > >> - >> > >> // >> > >> // Assume the bridge is aligned >> > >> // >> > >> - while (CurrentLink != &Bridge->ChildList) { >> > >> + for ( CurrentLink = GetFirstNode (&Bridge->ChildList) >> > >> + ; !IsNull (&Bridge->ChildList, CurrentLink) >> > >> + ; CurrentLink = GetNextNode (&Bridge->ChildList, CurrentLink) >> > >> + ) { >> > >> >> > >> Node = RESOURCE_NODE_FROM_LINK (CurrentLink); >> > >> + if (Node->ResourceUsage == PciResUsagePadding) { >> > >> + ASSERT (PaddingAperture == 0); >> > >> + PaddingAperture = Node->Length; >> > >> + continue; >> > >> + } >> > >> >> > >> // >> > >> // Apply padding resource if available >> > >> @@ -381,11 +398,6 @@ CalculateResourceAperture ( >> > >> // Increment aperture by the length of node >> > >> // >> > >> Aperture += Node->Length; >> > >> - >> > >> - // >> > >> - // Consider the aperture alignment >> > >> - // >> > >> - CurrentLink = CurrentLink->ForwardLink; >> > >> } >> > >> >> > >> // >> > >> @@ -407,7 +419,7 @@ CalculateResourceAperture ( >> > >> } >> > >> >> > >> // >> > >> - // At last, adjust the bridge's alignment to the first child's >> > >> alignment >> > >> + // Adjust the bridge's alignment to the first child's alignment >> > >> // if the bridge has at least one child >> > >> // >> > >> CurrentLink = Bridge->ChildList.ForwardLink; >> > >> @@ -417,6 +429,12 @@ 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); >> > >> } >> > >> >> > >> /** >> > >> -- >> > >> 1.9.5.msysgit.1 >> > >> >> > >> _______________________________________________ >> > >> edk2-devel mailing list >> > >> [email protected] >> > >> https://lists.01.org/mailman/listinfo/edk2-devel >_______________________________________________ >edk2-devel mailing list >[email protected] >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
