diff mbox

[edk2,07/17] OvmfPkg: PciHostBridgeLib: convert main loop from PciHostBridgeDxe

Message ID 1456532616-32475-8-git-send-email-lersek@redhat.com
State Superseded
Headers show

Commit Message

Laszlo Ersek Feb. 27, 2016, 12:23 a.m. UTC
In this patch we import the scan for extra root buses from the
InitializePciHostBridge() function, in file
"OvmfPkg/PciHostBridgeDxe/PciHostBridge.c".

For the time being, the InitRootBridge() and UninitRootBridge() functions
are just placeholders.

The PciHostBridgeGetRootBridges() API expects us to return the
PCI_ROOT_BRIDGE structures in a contiguous array, instead of a linked
list. Therefore the following bits have to be converted manually:

(1) The array is allocated in advance, in a single step.

(2) The calculation of the array size depends on an explicit
    multiplication, which we must check against overflow. Since more than
    255 extra root bridges make no sense anyway, we use (1 + 255) as the
    limit on the main plus all extra root bridges. This also ensures that
    the UINTN multiplication doesn't overflow.

(3) The PciHostBridgeDxe code decrements "ExtraRootBridgesLeft" to
    terminate the scanning early. Here we need track the increasing count
    of used array elements as well, so we employ "ExtraRootBridges" as a
    constant limit, and increment the new local variable "Initialized".

(4) The prototypes of InitRootBridge() and UninitRootBridge() reflect that
    the PCI_ROOT_BRIDGE structure is allocated by the caller; only
    in-place initialization is necessary.

Additionally, MAX_PCI_DEVICE_NUMBER is open coded as 31 (decimal) form
"OvmfPkg/PciHostBridgeDxe/PciHostBridge.h".

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>

---
 OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf |   9 +-
 OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c   | 165 +++++++++++++++++++-
 2 files changed, 172 insertions(+), 2 deletions(-)

-- 
1.8.3.1


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek March 1, 2016, 8:40 a.m. UTC | #1
On 03/01/16 08:16, Jordan Justen wrote:
> On 2016-02-26 16:23:26, Laszlo Ersek wrote:

>> In this patch we import the scan for extra root buses from the

>> InitializePciHostBridge() function, in file

>> "OvmfPkg/PciHostBridgeDxe/PciHostBridge.c".

>>

>> For the time being, the InitRootBridge() and UninitRootBridge() functions

>> are just placeholders.

>>

>> The PciHostBridgeGetRootBridges() API expects us to return the

>> PCI_ROOT_BRIDGE structures in a contiguous array, instead of a linked

>> list. Therefore the following bits have to be converted manually:

>>

>> (1) The array is allocated in advance, in a single step.

>>

>> (2) The calculation of the array size depends on an explicit

>>     multiplication, which we must check against overflow. Since more than

>>     255 extra root bridges make no sense anyway, we use (1 + 255) as the

>>     limit on the main plus all extra root bridges. This also ensures that

>>     the UINTN multiplication doesn't overflow.

>>

>> (3) The PciHostBridgeDxe code decrements "ExtraRootBridgesLeft" to

>>     terminate the scanning early. Here we need track the increasing count

>>     of used array elements as well, so we employ "ExtraRootBridges" as a

>>     constant limit, and increment the new local variable "Initialized".

>>

>> (4) The prototypes of InitRootBridge() and UninitRootBridge() reflect that

>>     the PCI_ROOT_BRIDGE structure is allocated by the caller; only

>>     in-place initialization is necessary.

>>

>> Additionally, MAX_PCI_DEVICE_NUMBER is open coded as 31 (decimal) form

>> "OvmfPkg/PciHostBridgeDxe/PciHostBridge.h".

>>

> 

> Do we need a different macro than PCI_MAX_DEVICE?


No, that's the one we should use. I wasn't aware of its existence in
"MdePkg/Include/IndustryStandard/Pci22.h".

Thanks
Laszlo

> 

> -Jordan

> 

>> Cc: Jordan Justen <jordan.l.justen@intel.com>

>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>

>> Cc: Marcel Apfelbaum <marcel@redhat.com>

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

>> ---

>>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf |   9 +-

>>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c   | 165 +++++++++++++++++++-

>>  2 files changed, 172 insertions(+), 2 deletions(-)

>>

>> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf

>> index 1f3930195a1d..b83693191261 100644

>> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf

>> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf

>> @@ -34,5 +34,12 @@ [Sources]

>>    PciHostBridgeLib.c

>>  

>>  [Packages]

>> -  MdePkg/MdePkg.dec

>>    MdeModulePkg/MdeModulePkg.dec

>> +  MdePkg/MdePkg.dec

>> +  OvmfPkg/OvmfPkg.dec

>> +

>> +[LibraryClasses]

>> +  DebugLib

>> +  MemoryAllocationLib

>> +  PciLib

>> +  QemuFwCfgLib

>> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c

>> index 3752993f65aa..b1f7e50ce781 100644

>> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c

>> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c

>> @@ -14,14 +14,77 @@

>>  

>>  **/

>>  #include <PiDxe.h>

>> -#include <Library/PciHostBridgeLib.h>

>> +

>> +#include <IndustryStandard/Pci.h>

>> +

>>  #include <Library/DebugLib.h>

>> +#include <Library/MemoryAllocationLib.h>

>> +#include <Library/PciHostBridgeLib.h>

>> +#include <Library/PciLib.h>

>> +#include <Library/QemuFwCfgLib.h>

>> +

>> +#define MAX_PCI_DEVICE_NUMBER 31

>> +

>>  

>>  GLOBAL_REMOVE_IF_UNREFERENCED

>>  CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {

>>    L"Mem", L"I/O", L"Bus"

>>  };

>>  

>> +

>> +/**

>> +  Initialize a PCI_ROOT_BRIDGE structure.

>> +

>> +  param[in]  RootBusNumber     The bus number to store in RootBus.

>> +

>> +  param[in]  MaxSubBusNumber   The inclusive maximum bus number that can be

>> +                               assigned to any subordinate bus found behind any

>> +                               PCI bridge hanging off this root bus.

>> +

>> +                               The caller is repsonsible for ensuring that

>> +                               RootBusNumber <= MaxSubBusNumber. If

>> +                               RootBusNumber equals MaxSubBusNumber, then the

>> +                               root bus has no room for subordinate buses.

>> +

>> +  param[out] RootBus           The PCI_ROOT_BRIDGE structure (allocated by the

>> +                               caller) that should be filled in by this

>> +                               function.

>> +

>> +  @retval EFI_SUCCESS           Initialization successful. A device path

>> +                                consisting of an ACPI device path node, with

>> +                                UID = RootBusNumber, has been allocated and

>> +                                linked into RootBus.

>> +

>> +  @retval EFI_OUT_OF_RESOURCES  Memory allocation failed.

>> +**/

>> +STATIC

>> +EFI_STATUS

>> +InitRootBridge (

>> +  IN  UINT8           RootBusNumber,

>> +  IN  UINT8           MaxSubBusNumber,

>> +  OUT PCI_ROOT_BRIDGE *RootBus

>> +  )

>> +{

>> +  return EFI_OUT_OF_RESOURCES;

>> +}

>> +

>> +

>> +/**

>> +  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

>> +  )

>> +{

>> +}

>> +

>> +

>>  /**

>>    Return all the root bridge instances in an array.

>>  

>> @@ -37,10 +100,109 @@ PciHostBridgeGetRootBridges (

>>    UINTN *Count

>>    )

>>  {

>> +  EFI_STATUS           Status;

>> +  FIRMWARE_CONFIG_ITEM FwCfgItem;

>> +  UINTN                FwCfgSize;

>> +  UINT64               ExtraRootBridges;

>> +  PCI_ROOT_BRIDGE      *Bridges;

>> +  UINTN                Initialized;

>> +  UINTN                LastRootBridgeNumber;

>> +  UINTN                RootBridgeNumber;

>> +

>>    *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 > 255) {

>> +      DEBUG ((EFI_D_ERROR, "%a: invalid count of extra root buses (%Lu) "

>> +        "reported by QEMU\n", __FUNCTION__, ExtraRootBridges));

>> +      return NULL;

>> +    }

>> +    DEBUG ((EFI_D_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 ((EFI_D_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 < 256 && Initialized < ExtraRootBridges;

>> +       ++RootBridgeNumber) {

>> +    UINTN Device;

>> +

>> +    for (Device = 0; Device <= MAX_PCI_DEVICE_NUMBER; ++Device) {

>> +      if (PciRead16 (PCI_LIB_ADDRESS (RootBridgeNumber, Device, 0,

>> +                       PCI_VENDOR_ID_OFFSET)) != MAX_UINT16) {

>> +        break;

>> +      }

>> +    }

>> +    if (Device <= MAX_PCI_DEVICE_NUMBER) {

>> +      //

>> +      // 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 ((UINT8)LastRootBridgeNumber,

>> +                 (UINT8)(RootBridgeNumber - 1), &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 ((UINT8)LastRootBridgeNumber, 255,

>> +             &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;

>>  }

>>  

>> +

>>  /**

>>    Free the root bridge instances array returned from

>>    PciHostBridgeGetRootBridges().

>> @@ -58,6 +220,7 @@ PciHostBridgeFreeRootBridges (

>>    return;

>>  }

>>  

>> +

>>  /**

>>    Inform the platform that the resource conflict happens.

>>  

>> -- 

>> 1.8.3.1

>>

>>


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
index 1f3930195a1d..b83693191261 100644
--- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
+++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
@@ -34,5 +34,12 @@  [Sources]
   PciHostBridgeLib.c
 
 [Packages]
-  MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  DebugLib
+  MemoryAllocationLib
+  PciLib
+  QemuFwCfgLib
diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
index 3752993f65aa..b1f7e50ce781 100644
--- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
+++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
@@ -14,14 +14,77 @@ 
 
 **/
 #include <PiDxe.h>
-#include <Library/PciHostBridgeLib.h>
+
+#include <IndustryStandard/Pci.h>
+
 #include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PciHostBridgeLib.h>
+#include <Library/PciLib.h>
+#include <Library/QemuFwCfgLib.h>
+
+#define MAX_PCI_DEVICE_NUMBER 31
+
 
 GLOBAL_REMOVE_IF_UNREFERENCED
 CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
   L"Mem", L"I/O", L"Bus"
 };
 
+
+/**
+  Initialize a PCI_ROOT_BRIDGE structure.
+
+  param[in]  RootBusNumber     The bus number to store in RootBus.
+
+  param[in]  MaxSubBusNumber   The inclusive maximum bus number that can be
+                               assigned to any subordinate bus found behind any
+                               PCI bridge hanging off this root bus.
+
+                               The caller is repsonsible for ensuring that
+                               RootBusNumber <= MaxSubBusNumber. If
+                               RootBusNumber equals MaxSubBusNumber, then the
+                               root bus has no room for subordinate buses.
+
+  param[out] RootBus           The PCI_ROOT_BRIDGE structure (allocated by the
+                               caller) that should be filled in by this
+                               function.
+
+  @retval EFI_SUCCESS           Initialization successful. A device path
+                                consisting of an ACPI device path node, with
+                                UID = RootBusNumber, has been allocated and
+                                linked into RootBus.
+
+  @retval EFI_OUT_OF_RESOURCES  Memory allocation failed.
+**/
+STATIC
+EFI_STATUS
+InitRootBridge (
+  IN  UINT8           RootBusNumber,
+  IN  UINT8           MaxSubBusNumber,
+  OUT PCI_ROOT_BRIDGE *RootBus
+  )
+{
+  return EFI_OUT_OF_RESOURCES;
+}
+
+
+/**
+  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
+  )
+{
+}
+
+
 /**
   Return all the root bridge instances in an array.
 
@@ -37,10 +100,109 @@  PciHostBridgeGetRootBridges (
   UINTN *Count
   )
 {
+  EFI_STATUS           Status;
+  FIRMWARE_CONFIG_ITEM FwCfgItem;
+  UINTN                FwCfgSize;
+  UINT64               ExtraRootBridges;
+  PCI_ROOT_BRIDGE      *Bridges;
+  UINTN                Initialized;
+  UINTN                LastRootBridgeNumber;
+  UINTN                RootBridgeNumber;
+
   *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 > 255) {
+      DEBUG ((EFI_D_ERROR, "%a: invalid count of extra root buses (%Lu) "
+        "reported by QEMU\n", __FUNCTION__, ExtraRootBridges));
+      return NULL;
+    }
+    DEBUG ((EFI_D_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 ((EFI_D_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 < 256 && Initialized < ExtraRootBridges;
+       ++RootBridgeNumber) {
+    UINTN Device;
+
+    for (Device = 0; Device <= MAX_PCI_DEVICE_NUMBER; ++Device) {
+      if (PciRead16 (PCI_LIB_ADDRESS (RootBridgeNumber, Device, 0,
+                       PCI_VENDOR_ID_OFFSET)) != MAX_UINT16) {
+        break;
+      }
+    }
+    if (Device <= MAX_PCI_DEVICE_NUMBER) {
+      //
+      // 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 ((UINT8)LastRootBridgeNumber,
+                 (UINT8)(RootBridgeNumber - 1), &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 ((UINT8)LastRootBridgeNumber, 255,
+             &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;
 }
 
+
 /**
   Free the root bridge instances array returned from
   PciHostBridgeGetRootBridges().
@@ -58,6 +220,7 @@  PciHostBridgeFreeRootBridges (
   return;
 }
 
+
 /**
   Inform the platform that the resource conflict happens.