Re: [edk2-devel] [PATCH v3 3/5] OvmfPkg: Extract functions for extra pci roots

["Jiahui Cen" <cenjiahui@huawei.com>]

Hi Laszlo,

Thanks for your detailed review!

On 2021/1/6 18:27, Laszlo Ersek wrote:
> On 12/22/20 10:59, Jiahui Cen via groups.io wrote:
>> Extract functions that support extra pci roots from
>> OvmfPkg/PciHostBridgeLib to share this feature with other packages.
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3059
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
>> Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
>> ---
>>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf               |   2 -
>>  OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf |   4 +
>>  OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h                   |  58 ++++++
>>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c                 | 158 +--------------
>>  OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c   | 212 ++++++++++++++++++++
>>  5 files changed, 285 insertions(+), 149 deletions(-)
>>
>> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> index 4c56f3c90b3b..8bdb76899111 100644
>> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
>> @@ -40,8 +40,6 @@ [LibraryClasses]
>>    DevicePathLib
>>    MemoryAllocationLib
>>    PciHostBridgeUtilityLib
>> -  PciLib
> 
> (1) Removing the PciLib class dependency is incorrect. The
> "XenSupport.c" file continues using PciRead32(), for example.

Will fix it.

> 
>> -  QemuFwCfgLib
>>
>>  [Pcd]
>>    gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
>> diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>> index 1ba8ec3e03c7..a10afbe30c6b 100644
>> --- a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>> +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>> @@ -30,8 +30,12 @@ [Sources]
>>    PciHostBridgeUtilityLib.c
>>
>>  [Packages]
>> +  MdeModulePkg/MdeModulePkg.dec
> 
> (2) Why is this needed? Both the MemoryAllocationLib and PciLib class
> headers are in MdePkg:
> 
>   MdePkg/Include/Library/MemoryAllocationLib.h
>   MdePkg/Include/Library/PciLib.h
> 
> ... Ah, wait, I see it. This is needed because, unfortunately, we cannot
> avoid a dependency on the PCI_ROOT_BRIDGE type, from file
> 
>   MdeModulePkg/Include/Library/PciHostBridgeLib.h
> 
> OK then.
> 
>>    MdePkg/MdePkg.dec
>>    OvmfPkg/OvmfPkg.dec
>>
>>  [LibraryClasses]
>>    DebugLib
>> +  MemoryAllocationLib
>> +  PciLib
>> +  QemuFwCfgLib
>> diff --git a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
>> index f932d412aa10..1d1c86c69064 100644
>> --- a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
>> +++ b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
>> @@ -14,6 +14,64 @@
>>  #define __PCI_HOST_BRIDGE_UTILITY_LIB_H__
>>
>>
>> +#include <Library/PciHostBridgeLib.h>
>> +
>> +
>> +/**
>> +  Utility function to free root bridge instances array.
>> +
>> +  @param  The root bridge instances array.
>> +  @param  The count of the array.
>> +**/
>> +VOID
>> +EFIAPI
>> +PciHostBridgeUtilityFreeRootBridges (
>> +  PCI_ROOT_BRIDGE *Bridges,
>> +  UINTN           Count
>> +  );
>> +
>> +
>> +/**
>> +  Utility function to return all the root bridge instances in an array.
>> +
>> +  @param      Count            The number of root bridge instances.
>> +
>> +  @param[in]  BusMin           The min bus number.
>> +
>> +  @param[in]  BusMax           The max bus number.
>> +
>> +  @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
>> +PciHostBridgeUtilityExtraRoots (
>> +  UINTN                    *Count,
>> +  UINT32                   BusMin,
>> +  UINT32                   BusMax,
>> +  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
>> +  );
>> +
>> +
>>  /**
>>    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 4a176347fd49..19c6e9fa421a 100644
>> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
>> @@ -21,8 +21,6 @@
>>  #include <Library/MemoryAllocationLib.h>
>>  #include <Library/PciHostBridgeLib.h>
>>  #include <Library/PciHostBridgeUtilityLib.h>
>> -#include <Library/PciLib.h>
>> -#include <Library/QemuFwCfgLib.h>
>>  #include "PciHostBridge.h"
>>
>>
>> @@ -160,23 +158,6 @@ InitRootBridge (
>>  }
>>
>>
>> -/**
>> -  Uninitialize a PCI_ROOT_BRIDGE structure set up with InitRootBridge().
>> -
>> -  param[in] RootBus  The PCI_ROOT_BRIDGE structure, allocated by the caller and
>> -                     initialized with InitRootBridge(), that should be
>> -                     uninitialized. This function doesn't free RootBus.
>> -**/
>> -STATIC
>> -VOID
>> -UninitRootBridge (
>> -  IN PCI_ROOT_BRIDGE *RootBus
>> -  )
>> -{
>> -  FreePool (RootBus->DevicePath);
>> -}
>> -
>> -
>>  /**
>>    Return all the root bridge instances in an array.
>>
>> @@ -192,14 +173,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;
>> @@ -239,117 +212,18 @@ PciHostBridgeGetRootBridges (
>>
>>    *Count = 0;
>>
>> -  //
>> -  // 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 = InitRootBridge (
>> -        Attributes,
>> -        Attributes,
>> -        AllocationAttributes,
>> -        (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 = InitRootBridge (
>> -    Attributes,
>> -    Attributes,
>> -    AllocationAttributes,
>> -    (UINT8) LastRootBridgeNumber,
>> +  return PciHostBridgeUtilityExtraRoots (
>> +    Count,
>> +    0,
>>      PCI_MAX_BUS,
>> -    &Io,
>> -    &Mem,
>> -    &MemAbove4G,
>> -    &mNonExistAperture,
>> -    &mNonExistAperture,
>> -    &Bridges[Initialized]
>> +    Attributes,
>> +    AllocationAttributes,
>> +    Io,
>> +    Mem,
>> +    MemAbove4G,
>> +    mNonExistAperture,
>> +    mNonExistAperture
>>      );
>> -  if (EFI_ERROR (Status)) {
>> -    goto FreeBridges;
>> -  }
>> -  ++Initialized;
>> -
>> -  *Count = Initialized;
>> -  return Bridges;
>> -
>> -FreeBridges:
>> -  while (Initialized > 0) {
>> -    --Initialized;
>> -    UninitRootBridge (&Bridges[Initialized]);
>> -  }
>> -
>> -  FreePool (Bridges);
>> -  return NULL;
>>  }
>>
>>
>> @@ -367,17 +241,7 @@ PciHostBridgeFreeRootBridges (
>>    UINTN           Count
>>    )
>>  {
>> -  if (Bridges == NULL && Count == 0) {
>> -    return;
>> -  }
>> -  ASSERT (Bridges != NULL && Count > 0);
>> -
>> -  do {
>> -    --Count;
>> -    UninitRootBridge (&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 bbaa5f830c98..a0cfd24ce477 100644
>> --- a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
>> +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
>> @@ -11,8 +11,13 @@
>>  **/
>>
>>  #include <IndustryStandard/Acpi10.h>
>> +#include <IndustryStandard/Pci.h>
>>  #include <Library/DebugLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>>  #include <Library/PciHostBridgeUtilityLib.h>
>> +#include <Library/PciLib.h>
>> +#include <Library/QemuFwCfgLib.h>
>> +#include "Library/PciHostBridgeLib/PciHostBridge.h"
> 
> (3) This approach is wrong.
> 
> Library instances in edk2 are supposed to be self-contained. It is
> non-idiomatic in edk2 for a consumed library class to expect a
> *dependent* (= consumer) library instance to provide a *callback*, with
> a particular funcion name. Whenever such generality is necessary, such
> that a callback cannot be avoided, then a typedef (for a function
> pointer type) must be introduced in the consumed library class, and the
> consumer (= dependent) library instance passes in the concrete callback
> as a pointer-to-function parameter -- wherever necessary.

Thanks for the explanation.

> 
> However, we do not need that level of generality here -- we don't need
> any callbacks.
> 
> (3a) You first need to extract the InitRootBridge() and
>      UninitRootBridge() functions to PciHostBridgeUtilityLib(), in a
>      separate patch.
> 
>      In the same patch, move the OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH
>      typedef, and the "mRootBridgeDevicePathTemplate" global variable
>      too. (Notice that the latter is identical to
>      "mEfiPciRootBridgeDevicePath" in
>      "ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c").
> 
>      No change of functionality; only rename the functions, in addition
>      to moving them.
> 
>      Note that his code extraction / rename will affect the
>      "XenSupport.c" file too!
> 
> (3b) Then, in another patch, you need to extend the parameter list of
>      PciHostBridgeUtilityInitRootBridge() with the following parameters:
> 
>      - DmaAbove4G
>      - NoExtendedConfigSpace
> 
> (3c) Then, in another patch, rebase ArmVirtPkg's PciHostBridgeLib
>      instance to the new PciHostBridgeUtilityInitRootBridge() /
>      PciHostBridgeUtilityUninitRootBridge() functions. Note that this
>      will affect the *single* root bus, at this point; namely the
>      PciHostBridgeGetRootBridges() function in
>      "ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c".
> 
>      This will allow you to verify whether the new (extended)
>      PciHostBridgeUtilityInitRootBridge() interface is flexible enough.
> 
>      The EFI_PCI_ROOT_BRIDGE_DEVICE_PATH typedef and the
>      "mEfiPciRootBridgeDevicePath" variable should disappear from
>      "ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c", at
>      this point.
> 
> (3d) Then you can proceed to extracting the fw_cfg-parsing logic from
>      OvmfPkg's PciHostBridgeLib instance to PciHostBridgeUtilityLib.
> 
>      At first, there should be no change in functionality; in
>      particular, the DmaAbove4G and NoExtendedConfigSpace parameters
>      should remain hard-coded, in the call(s) to
>      PciHostBridgeUtilityInitRootBridge().
> 
>      The name of the new functions should be
>      PciHostBridgeUtilityGetRootBridges() -- not
>      PciHostBridgeUtilityExtraRoots() --, and
>      PciHostBridgeUtilityFreeRootBridges() -- already named correctly in
>      your patch, apparently.
> 
> (3e) Extend PciHostBridgeUtilityGetRootBridges() with the DmaAbove4G and
>      NoExtendedConfigSpace parameters, so they are propagated from the
>      outside into PciHostBridgeUtilityInitRootBridge().
> 
> (3f) Call PciHostBridgeUtilityGetRootBridges() in ArmVirtPkg's
>      FdtPciHostBridgeLib.
> 

Got it. I will refactor the patches as you points.

> 
>>
>>
>>  GLOBAL_REMOVE_IF_UNREFERENCED
>> @@ -21,6 +26,213 @@ CHAR16 *mPciHostBridgeUtilityLibAcpiAddressSpaceTypeStr[] = {
>>  };
>>
>>
>> +/**
>> +  Uninitialize a PCI_ROOT_BRIDGE structure set up with InitRootBridge().
>> +
>> +  param[in] RootBus  The PCI_ROOT_BRIDGE structure, allocated by the caller and
>> +                     initialized with InitRootBridge(), that should be
>> +                     uninitialized. This function doesn't free RootBus.
>> +**/
>> +STATIC
>> +VOID
>> +UninitRootBridge (
>> +  IN PCI_ROOT_BRIDGE *RootBus
>> +  )
>> +{
>> +  FreePool (RootBus->DevicePath);
>> +}
>> +
>> +
>> +/**
>> +  Utility function to free root bridge instances array.
>> +
>> +  @param  The root bridge instances array.
>> +  @param  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;
>> +    UninitRootBridge (&Bridges[Count]);
>> +  } while (Count > 0);
>> +
>> +  FreePool (Bridges);
>> +}
>> +
>> +
>> +/**
>> +  Utility function to return all the root bridge instances in an array.
>> +
>> +  @param      Count            The number of root bridge instances.
> 
> (4) This should be @param[out].

Yes. Will fix.

Thanks,
Jiahui

> 
> Thanks
> Laszlo
> 
>> +
>> +  @param[in]  BusMin           The min bus number.
>> +
>> +  @param[in]  BusMax           The max bus number.
>> +
>> +  @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
>> +PciHostBridgeUtilityExtraRoots (
>> +  UINTN                    *Count,
>> +  UINT32                   BusMin,
>> +  UINT32                   BusMax,
>> +  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;
>> +  PCI_ROOT_BRIDGE      *Bridges;
>> +  FIRMWARE_CONFIG_ITEM FwCfgItem;
>> +  UINTN                FwCfgSize;
>> +  UINT64               ExtraRootBridges;
>> +  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 > BusMax - BusMin) {
>> +      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 = BusMin;
>> +
>> +  //
>> +  // 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 = BusMin + 1;
>> +       RootBridgeNumber <= BusMax && 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 = InitRootBridge (
>> +        Attributes,
>> +        Attributes,
>> +        AllocationAttributes,
>> +        (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 = InitRootBridge (
>> +    Attributes,
>> +    Attributes,
>> +    AllocationAttributes,
>> +    (UINT8) LastRootBridgeNumber,
>> +    BusMax,
>> +    &Io,
>> +    &Mem,
>> +    &MemAbove4G,
>> +    &PMem,
>> +    &PMemAbove4G,
>> +    &Bridges[Initialized]
>> +    );
>> +  if (EFI_ERROR (Status)) {
>> +    goto FreeBridges;
>> +  }
>> +  ++Initialized;
>> +
>> +  *Count = Initialized;
>> +  return Bridges;
>> +
>> +FreeBridges:
>> +  while (Initialized > 0) {
>> +    --Initialized;
>> +    UninitRootBridge (&Bridges[Initialized]);
>> +  }
>> +
>> +  FreePool (Bridges);
>> +  return NULL;
>> +}
>> +
>> +
>>  /**
>>    Utility function to inform the platform that the resource conflict happens.
>>
>>
> 
> .
>