From c7e7613e09ff10257dfc301ac1cd3bff807e6c02 Mon Sep 17 00:00:00 2001 From: Ruiyu Ni Date: Tue, 3 Nov 2015 02:34:21 +0000 Subject: [PATCH] MdeModulePkg: Fix a PciBusDxe hot plug bug 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 Reviewed-by: Feng Tian git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@18719 6f19259b-4bc3-4df7-8a09-765794883524 --- MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 13 ++-- .../Bus/Pci/PciBusDxe/PciResourceSupport.c | 60 ++++++++++++------- 2 files changed, 48 insertions(+), 25 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c index bfb7e5bee9..12647be85f 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c @@ -976,7 +976,8 @@ PciScanBus ( UINT8 Device; UINT8 Func; UINT64 Address; - UINTN SecondBus; + UINT8 SecondBus; + UINT8 PaddedSubBus; UINT16 Register; UINTN HpIndex; PCI_IO_DEVICE *PciDevice; @@ -1210,7 +1211,7 @@ PciScanBus ( Status = PciScanBus ( PciDevice, - (UINT8) (SecondBus), + SecondBus, SubBusNumber, PaddedBusRange ); @@ -1226,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 d8d988cbfc..e12d59f1d0 100644 --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c @@ -1,7 +1,7 @@ /** @file PCI resouces support functions implemntation for PCI Bus module. -Copyright (c) 2006 - 2013, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -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); } /**