[edk2-devel] [PATCH v4 6/9] OvmfPkg/PciHostBridgeLib: Extract GetRootBridges/FreeRootBridges
Laszlo Ersek
lersek at redhat.com
Thu Jan 14 09:12:27 UTC 2021
On 01/12/21 10:45, Jiahui Cen via groups.io wrote:
> Extract PciHostBridgeGetRootBridges() / PciHostBridgeFreeRootBridges() to
> PciHostBridgeUtilityLib as common utility functions.
>
> No change of functionality.
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3059
>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
> Signed-off-by: Jiahui Cen <cenjiahui at huawei.com>
> Signed-off-by: Yubo Miao <miaoyubo at huawei.com>
> ---
> OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf | 1 -
> OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf | 5 +
> OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h | 50 +++++
> OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c | 136 +-------------
> OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c | 193 ++++++++++++++++++++
> 5 files changed, 252 insertions(+), 133 deletions(-)
>
> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
> index 463c05c94b07..93ba440c9009 100644
> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
> @@ -40,7 +40,6 @@ [LibraryClasses]
> MemoryAllocationLib
> PciHostBridgeUtilityLib
> PciLib
> - QemuFwCfgLib
>
> [Pcd]
> gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
> diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
> index 4d6764b702f4..4cdf367ffee2 100644
> --- a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
> +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
> @@ -39,3 +39,8 @@ [LibraryClasses]
> DebugLib
> DevicePathLib
> MemoryAllocationLib
> + PciLib
> + QemuFwCfgLib
> +
> +[Pcd]
> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
(1) We'll have to re-add the PcdLib dependency then (#include too).
> diff --git a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
> index 13c591641d7a..29ea19f2d7e5 100644
> --- a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
> +++ b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
> @@ -97,6 +97,56 @@ PciHostBridgeUtilityUninitRootBridge (
> );
>
>
> +/**
> + Utility function to return all the root bridge instances in an array.
> +
> + @param[out] Count The number of root bridge instances.
> +
> + @param[in] Attributes Initial attributes.
> +
> + @param[in] AllocAttributes Allocation attributes.
> +
> + @param[in] Io IO aperture.
> +
> + @param[in] Mem MMIO aperture.
> +
> + @param[in] MemAbove4G MMIO aperture above 4G.
> +
> + @param[in] PMem Prefetchable MMIO aperture.
> +
> + @param[in] PMemAbove4G Prefetchable MMIO aperture above 4G.
> +
> + @return All the root bridge instances in an array.
> +**/
> +PCI_ROOT_BRIDGE *
> +EFIAPI
> +PciHostBridgeUtilityGetRootBridges (
> + UINTN *Count,
> + UINT64 Attributes,
> + UINT64 AllocationAttributes,
> + PCI_ROOT_BRIDGE_APERTURE *Io,
> + PCI_ROOT_BRIDGE_APERTURE *Mem,
> + PCI_ROOT_BRIDGE_APERTURE *MemAbove4G,
> + PCI_ROOT_BRIDGE_APERTURE *PMem,
> + PCI_ROOT_BRIDGE_APERTURE *PMemAbove4G
> + );
(2) You missed annotating the parameters as IN / OUT (should be in sync
with the @param[...] comments above). Don't forget to sync the C file too.
> +
> +
> +/**
> + Utility function to free root bridge instances array from
> + PciHostBridgeUtilityGetRootBridges().
> +
> + @param[in] Bridges The root bridge instances array.
> + @param[in] Count The count of the array.
> +**/
> +VOID
> +EFIAPI
> +PciHostBridgeUtilityFreeRootBridges (
> + PCI_ROOT_BRIDGE *Bridges,
> + UINTN Count
> + );
> +
(3) You missed annotating the parameters as IN / OUT (should be in sync
with the @param[...] comments above). Don't forget to sync the C file too.
> +
> /**
> Utility function to inform the platform that the resource conflict happens.
>
> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
> index d9c057021598..0a1d94a29bf3 100644
> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
> @@ -9,9 +9,6 @@
> **/
> #include <PiDxe.h>
>
> -#include <IndustryStandard/Pci.h>
> -#include <IndustryStandard/Q35MchIch9.h>
> -
> #include <Protocol/PciHostBridgeResourceAllocation.h>
> #include <Protocol/PciRootBridgeIo.h>
>
> @@ -20,8 +17,6 @@
> #include <Library/MemoryAllocationLib.h>
> #include <Library/PciHostBridgeLib.h>
> #include <Library/PciHostBridgeUtilityLib.h>
> -#include <Library/PciLib.h>
> -#include <Library/QemuFwCfgLib.h>
> #include "PciHostBridge.h"
>
>
> @@ -43,14 +38,6 @@ PciHostBridgeGetRootBridges (
> UINTN *Count
> )
> {
> - EFI_STATUS Status;
> - FIRMWARE_CONFIG_ITEM FwCfgItem;
> - UINTN FwCfgSize;
> - UINT64 ExtraRootBridges;
> - PCI_ROOT_BRIDGE *Bridges;
> - UINTN Initialized;
> - UINTN LastRootBridgeNumber;
> - UINTN RootBridgeNumber;
> UINT64 Attributes;
> UINT64 AllocationAttributes;
> PCI_ROOT_BRIDGE_APERTURE Io;
> @@ -90,121 +77,16 @@ PciHostBridgeGetRootBridges (
>
> *Count = 0;
(4) Please move this assignment as well into the extracted function.
Thanks
Laszlo
>
> - //
> - // QEMU provides the number of extra root buses, shortening the exhaustive
> - // search below. If there is no hint, the feature is missing.
> - //
> - Status = QemuFwCfgFindFile ("etc/extra-pci-roots", &FwCfgItem, &FwCfgSize);
> - if (EFI_ERROR (Status) || FwCfgSize != sizeof ExtraRootBridges) {
> - ExtraRootBridges = 0;
> - } else {
> - QemuFwCfgSelectItem (FwCfgItem);
> - QemuFwCfgReadBytes (FwCfgSize, &ExtraRootBridges);
> -
> - if (ExtraRootBridges > PCI_MAX_BUS) {
> - DEBUG ((DEBUG_ERROR, "%a: invalid count of extra root buses (%Lu) "
> - "reported by QEMU\n", __FUNCTION__, ExtraRootBridges));
> - return NULL;
> - }
> - DEBUG ((DEBUG_INFO, "%a: %Lu extra root buses reported by QEMU\n",
> - __FUNCTION__, ExtraRootBridges));
> - }
> -
> - //
> - // Allocate the "main" root bridge, and any extra root bridges.
> - //
> - Bridges = AllocatePool ((1 + (UINTN)ExtraRootBridges) * sizeof *Bridges);
> - if (Bridges == NULL) {
> - DEBUG ((DEBUG_ERROR, "%a: %r\n", __FUNCTION__, EFI_OUT_OF_RESOURCES));
> - return NULL;
> - }
> - Initialized = 0;
> -
> - //
> - // The "main" root bus is always there.
> - //
> - LastRootBridgeNumber = 0;
> -
> - //
> - // Scan all other root buses. If function 0 of any device on a bus returns a
> - // VendorId register value different from all-bits-one, then that bus is
> - // alive.
> - //
> - for (RootBridgeNumber = 1;
> - RootBridgeNumber <= PCI_MAX_BUS && Initialized < ExtraRootBridges;
> - ++RootBridgeNumber) {
> - UINTN Device;
> -
> - for (Device = 0; Device <= PCI_MAX_DEVICE; ++Device) {
> - if (PciRead16 (PCI_LIB_ADDRESS (RootBridgeNumber, Device, 0,
> - PCI_VENDOR_ID_OFFSET)) != MAX_UINT16) {
> - break;
> - }
> - }
> - if (Device <= PCI_MAX_DEVICE) {
> - //
> - // Found the next root bus. We can now install the *previous* one,
> - // because now we know how big a bus number range *that* one has, for any
> - // subordinate buses that might exist behind PCI bridges hanging off it.
> - //
> - Status = PciHostBridgeUtilityInitRootBridge (
> - Attributes,
> - Attributes,
> - AllocationAttributes,
> - FALSE,
> - PcdGet16 (PcdOvmfHostBridgePciDevId) != INTEL_Q35_MCH_DEVICE_ID,
> - (UINT8) LastRootBridgeNumber,
> - (UINT8) (RootBridgeNumber - 1),
> - &Io,
> - &Mem,
> - &MemAbove4G,
> - &mNonExistAperture,
> - &mNonExistAperture,
> - &Bridges[Initialized]
> - );
> - if (EFI_ERROR (Status)) {
> - goto FreeBridges;
> - }
> - ++Initialized;
> - LastRootBridgeNumber = RootBridgeNumber;
> - }
> - }
> -
> - //
> - // Install the last root bus (which might be the only, ie. main, root bus, if
> - // we've found no extra root buses).
> - //
> - Status = PciHostBridgeUtilityInitRootBridge (
> - Attributes,
> + return PciHostBridgeUtilityGetRootBridges (
> + Count,
> Attributes,
> AllocationAttributes,
> - FALSE,
> - PcdGet16 (PcdOvmfHostBridgePciDevId) != INTEL_Q35_MCH_DEVICE_ID,
> - (UINT8) LastRootBridgeNumber,
> - PCI_MAX_BUS,
> &Io,
> &Mem,
> &MemAbove4G,
> &mNonExistAperture,
> - &mNonExistAperture,
> - &Bridges[Initialized]
> + &mNonExistAperture
> );
> - if (EFI_ERROR (Status)) {
> - goto FreeBridges;
> - }
> - ++Initialized;
> -
> - *Count = Initialized;
> - return Bridges;
> -
> -FreeBridges:
> - while (Initialized > 0) {
> - --Initialized;
> - PciHostBridgeUtilityUninitRootBridge (&Bridges[Initialized]);
> - }
> -
> - FreePool (Bridges);
> - return NULL;
> }
>
>
> @@ -222,17 +104,7 @@ PciHostBridgeFreeRootBridges (
> UINTN Count
> )
> {
> - if (Bridges == NULL && Count == 0) {
> - return;
> - }
> - ASSERT (Bridges != NULL && Count > 0);
> -
> - do {
> - --Count;
> - PciHostBridgeUtilityUninitRootBridge (&Bridges[Count]);
> - } while (Count > 0);
> -
> - FreePool (Bridges);
> + PciHostBridgeUtilityFreeRootBridges (Bridges, Count);
> }
>
>
> diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
> index 9101aedc8888..629ebcd7a5be 100644
> --- a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
> +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
> @@ -11,11 +11,15 @@
> **/
>
> #include <IndustryStandard/Acpi10.h>
> +#include <IndustryStandard/Pci.h>
> +#include <IndustryStandard/Q35MchIch9.h>
> #include <Library/BaseMemoryLib.h>
> #include <Library/DebugLib.h>
> #include <Library/DevicePathLib.h>
> #include <Library/MemoryAllocationLib.h>
> #include <Library/PciHostBridgeUtilityLib.h>
> +#include <Library/PciLib.h>
> +#include <Library/QemuFwCfgLib.h>
>
>
> #pragma pack(1)
> @@ -182,6 +186,195 @@ PciHostBridgeUtilityUninitRootBridge (
> }
>
>
> +/**
> + Utility function to return all the root bridge instances in an array.
> +
> + @param[out] Count The number of root bridge instances.
> +
> + @param[in] Attributes Initial attributes.
> +
> + @param[in] AllocAttributes Allocation attributes.
> +
> + @param[in] Io IO aperture.
> +
> + @param[in] Mem MMIO aperture.
> +
> + @param[in] MemAbove4G MMIO aperture above 4G.
> +
> + @param[in] PMem Prefetchable MMIO aperture.
> +
> + @param[in] PMemAbove4G Prefetchable MMIO aperture above 4G.
> +
> + @return All the root bridge instances in an array.
> +**/
> +PCI_ROOT_BRIDGE *
> +EFIAPI
> +PciHostBridgeUtilityGetRootBridges (
> + UINTN *Count,
> + UINT64 Attributes,
> + UINT64 AllocationAttributes,
> + PCI_ROOT_BRIDGE_APERTURE *Io,
> + PCI_ROOT_BRIDGE_APERTURE *Mem,
> + PCI_ROOT_BRIDGE_APERTURE *MemAbove4G,
> + PCI_ROOT_BRIDGE_APERTURE *PMem,
> + PCI_ROOT_BRIDGE_APERTURE *PMemAbove4G
> + )
> +{
> + EFI_STATUS Status;
> + FIRMWARE_CONFIG_ITEM FwCfgItem;
> + UINTN FwCfgSize;
> + UINT64 ExtraRootBridges;
> + PCI_ROOT_BRIDGE *Bridges;
> + UINTN Initialized;
> + UINTN LastRootBridgeNumber;
> + UINTN RootBridgeNumber;
> +
> + //
> + // QEMU provides the number of extra root buses, shortening the exhaustive
> + // search below. If there is no hint, the feature is missing.
> + //
> + Status = QemuFwCfgFindFile ("etc/extra-pci-roots", &FwCfgItem, &FwCfgSize);
> + if (EFI_ERROR (Status) || FwCfgSize != sizeof ExtraRootBridges) {
> + ExtraRootBridges = 0;
> + } else {
> + QemuFwCfgSelectItem (FwCfgItem);
> + QemuFwCfgReadBytes (FwCfgSize, &ExtraRootBridges);
> +
> + if (ExtraRootBridges > PCI_MAX_BUS) {
> + DEBUG ((DEBUG_ERROR, "%a: invalid count of extra root buses (%Lu) "
> + "reported by QEMU\n", __FUNCTION__, ExtraRootBridges));
> + return NULL;
> + }
> + DEBUG ((DEBUG_INFO, "%a: %Lu extra root buses reported by QEMU\n",
> + __FUNCTION__, ExtraRootBridges));
> + }
> +
> + //
> + // Allocate the "main" root bridge, and any extra root bridges.
> + //
> + Bridges = AllocatePool ((1 + (UINTN)ExtraRootBridges) * sizeof *Bridges);
> + if (Bridges == NULL) {
> + DEBUG ((DEBUG_ERROR, "%a: %r\n", __FUNCTION__, EFI_OUT_OF_RESOURCES));
> + return NULL;
> + }
> + Initialized = 0;
> +
> + //
> + // The "main" root bus is always there.
> + //
> + LastRootBridgeNumber = 0;
> +
> + //
> + // Scan all other root buses. If function 0 of any device on a bus returns a
> + // VendorId register value different from all-bits-one, then that bus is
> + // alive.
> + //
> + for (RootBridgeNumber = 1;
> + RootBridgeNumber <= PCI_MAX_BUS && Initialized < ExtraRootBridges;
> + ++RootBridgeNumber) {
> + UINTN Device;
> +
> + for (Device = 0; Device <= PCI_MAX_DEVICE; ++Device) {
> + if (PciRead16 (PCI_LIB_ADDRESS (RootBridgeNumber, Device, 0,
> + PCI_VENDOR_ID_OFFSET)) != MAX_UINT16) {
> + break;
> + }
> + }
> + if (Device <= PCI_MAX_DEVICE) {
> + //
> + // Found the next root bus. We can now install the *previous* one,
> + // because now we know how big a bus number range *that* one has, for any
> + // subordinate buses that might exist behind PCI bridges hanging off it.
> + //
> + Status = PciHostBridgeUtilityInitRootBridge (
> + Attributes,
> + Attributes,
> + AllocationAttributes,
> + FALSE,
> + PcdGet16 (PcdOvmfHostBridgePciDevId) != INTEL_Q35_MCH_DEVICE_ID,
> + (UINT8) LastRootBridgeNumber,
> + (UINT8) (RootBridgeNumber - 1),
> + Io,
> + Mem,
> + MemAbove4G,
> + PMem,
> + PMemAbove4G,
> + &Bridges[Initialized]
> + );
> + if (EFI_ERROR (Status)) {
> + goto FreeBridges;
> + }
> + ++Initialized;
> + LastRootBridgeNumber = RootBridgeNumber;
> + }
> + }
> +
> + //
> + // Install the last root bus (which might be the only, ie. main, root bus, if
> + // we've found no extra root buses).
> + //
> + Status = PciHostBridgeUtilityInitRootBridge (
> + Attributes,
> + Attributes,
> + AllocationAttributes,
> + FALSE,
> + PcdGet16 (PcdOvmfHostBridgePciDevId) != INTEL_Q35_MCH_DEVICE_ID,
> + (UINT8) LastRootBridgeNumber,
> + PCI_MAX_BUS,
> + Io,
> + Mem,
> + MemAbove4G,
> + PMem,
> + PMemAbove4G,
> + &Bridges[Initialized]
> + );
> + if (EFI_ERROR (Status)) {
> + goto FreeBridges;
> + }
> + ++Initialized;
> +
> + *Count = Initialized;
> + return Bridges;
> +
> +FreeBridges:
> + while (Initialized > 0) {
> + --Initialized;
> + PciHostBridgeUtilityUninitRootBridge (&Bridges[Initialized]);
> + }
> +
> + FreePool (Bridges);
> + return NULL;
> +}
> +
> +
> +/**
> + Utility function to free root bridge instances array from
> + PciHostBridgeUtilityGetRootBridges().
> +
> + @param[in] Bridges The root bridge instances array.
> + @param[in] Count The count of the array.
> +**/
> +VOID
> +EFIAPI
> +PciHostBridgeUtilityFreeRootBridges (
> + PCI_ROOT_BRIDGE *Bridges,
> + UINTN Count
> + )
> +{
> + if (Bridges == NULL && Count == 0) {
> + return;
> + }
> + ASSERT (Bridges != NULL && Count > 0);
> +
> + do {
> + --Count;
> + PciHostBridgeUtilityUninitRootBridge (&Bridges[Count]);
> + } while (Count > 0);
> +
> + FreePool (Bridges);
> +}
> +
> +
> /**
> Utility function to inform the platform that the resource conflict happens.
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70278): https://edk2.groups.io/g/devel/message/70278
Mute This Topic: https://groups.io/mt/79619792/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list