[edk2-devel] [PATCH v3] IntelSiliconPkg/IntelVTdDxe: Support Multi PCI Root Bus

Sheng Wei posted 1 patch 2 years, 6 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
.../Feature/VTd/IntelVTdDxe/DmaProtection.h        | 17 +++++
.../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c        |  2 +-
.../Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf        |  1 +
.../Feature/VTd/IntelVTdDxe/PciInfo.c              | 72 ++++++++++++++++++++++
4 files changed, 91 insertions(+), 1 deletion(-)
[edk2-devel] [PATCH v3] IntelSiliconPkg/IntelVTdDxe: Support Multi PCI Root Bus
Posted by Sheng Wei 2 years, 6 months ago
Some system may has multi PCI root bus. It needs to use PciRootBridgeIo
 protocol to get the root bus count. Scan each root bus to get all devices.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3695

Signed-off-by: Robert Kowalewski <robert.kowalewski@intel.com>
Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Jenny Huang <jenny.huang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Robert Kowalewski <robert.kowalewski@intel.com>
Cc: Albecki Mateusz <mateusz.albecki@intel.com>
Cc: Kolakowski Jacek <Jacek.Kolakowski@intel.com>
---
 .../Feature/VTd/IntelVTdDxe/DmaProtection.h        | 17 +++++
 .../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c        |  2 +-
 .../Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf        |  1 +
 .../Feature/VTd/IntelVTdDxe/PciInfo.c              | 72 ++++++++++++++++++++++
 4 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
index a24fbc37..7dd29a24 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
@@ -33,6 +33,7 @@
 #include <Protocol/PciEnumerationComplete.h>
 #include <Protocol/PlatformVtdPolicy.h>
 #include <Protocol/IoMmu.h>
+#include <Protocol/PciRootBridgeIo.h>
 
 #include <IndustryStandard/Pci.h>
 #include <IndustryStandard/DmaRemappingReportingTable.h>
@@ -341,6 +342,22 @@ ScanPciBus (
   IN SCAN_BUS_FUNC_CALLBACK_FUNC  Callback
   );
 
+/**
+  Scan PCI bus and invoke callback function for each PCI devices under all root bus.
+
+  @param[in]  Context               The context of the callback function.
+  @param[in]  Segment               The segment of the source.
+  @param[in]  Callback              The callback function in PCI scan.
+
+  @retval EFI_SUCCESS           The PCI devices under the bus are scaned.
+**/
+EFI_STATUS
+ScanAllPciBus (
+  IN VOID                         *Context,
+  IN UINT16                       Segment,
+  IN SCAN_BUS_FUNC_CALLBACK_FUNC  Callback
+  );
+
 /**
   Dump the PCI device information managed by this VTd engine.
 
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
index 2d9b4374..1ee290b7 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
@@ -692,7 +692,7 @@ ProcessDhrd (
     mVtdUnitInformation[VtdIndex].PciDeviceInfo.IncludeAllFlag = TRUE;
     DEBUG ((DEBUG_INFO,"  ProcessDhrd: with INCLUDE ALL\n"));
 
-    Status = ScanPciBus((VOID *)VtdIndex, DmarDrhd->SegmentNumber, 0, ScanBusCallbackRegisterPciDevice);
+    Status = ScanAllPciBus((VOID *)VtdIndex, DmarDrhd->SegmentNumber, ScanBusCallbackRegisterPciDevice);
     if (EFI_ERROR (Status)) {
       return Status;
     }
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
index 220636ad..387f90e3 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
@@ -70,6 +70,7 @@
   gEfiPciIoProtocolGuid                       ## CONSUMES
   gEfiPciEnumerationCompleteProtocolGuid      ## CONSUMES
   gEdkiiPlatformVTdPolicyProtocolGuid         ## SOMETIMES_CONSUMES
+  gEfiPciRootBridgeIoProtocolGuid             ## CONSUMES
 
 [Pcd]
   gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask   ## CONSUMES
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
index 4af376b3..cffb9f61 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
@@ -279,6 +279,78 @@ ScanPciBus (
   return EFI_SUCCESS;
 }
 
+/**
+  Scan PCI bus and invoke callback function for each PCI devices under all root bus.
+
+  @param[in]  Context               The context of the callback function.
+  @param[in]  Segment               The segment of the source.
+  @param[in]  Callback              The callback function in PCI scan.
+
+  @retval EFI_SUCCESS           The PCI devices under the bus are scaned.
+**/
+EFI_STATUS
+ScanAllPciBus (
+  IN VOID                         *Context,
+  IN UINT16                       Segment,
+  IN SCAN_BUS_FUNC_CALLBACK_FUNC  Callback
+  )
+{
+  EFI_STATUS                        Status;
+  UINTN                             Index;
+  UINTN                             HandleCount;
+  EFI_HANDLE                        *HandleBuffer;
+  EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL   *PciRootBridgeIo;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptors;
+
+  DEBUG ((DEBUG_INFO, "ScanAllPciBus ()\n"));
+
+  Status = gBS->LocateHandleBuffer (
+                  ByProtocol,
+                  &gEfiPciRootBridgeIoProtocolGuid,
+                  NULL,
+                  &HandleCount,
+                  &HandleBuffer
+                  );
+  if (EFI_ERROR (Status)) {
+    //
+    // If PciRootBridgeIo protocol is not support, scan PCI device from root bus 0x00.
+    //
+    Status = ScanPciBus(Context, Segment, 0x00, Callback);
+    return Status;
+  }
+
+  DEBUG ((DEBUG_INFO,"Find %d root bridges\n", HandleCount));
+
+  for (Index = 0; Index < HandleCount; Index++) {
+    Status = gBS->HandleProtocol (
+                    HandleBuffer[Index],
+                    &gEfiPciRootBridgeIoProtocolGuid,
+                    (VOID **) &PciRootBridgeIo
+                    );
+    ASSERT_EFI_ERROR (Status);
+
+    Status = PciRootBridgeIo->Configuration (PciRootBridgeIo, (VOID **) &Descriptors);
+    ASSERT_EFI_ERROR (Status);
+
+    while (Descriptors->Desc != ACPI_END_TAG_DESCRIPTOR) {
+      if (Descriptors->ResType == ACPI_ADDRESS_SPACE_TYPE_BUS) {
+        break;
+      }
+      Descriptors++;
+    }
+
+    DEBUG ((DEBUG_INFO,"Scan root bridges : %d, Segment : %d, Bus : 0x%02X\n", Index, PciRootBridgeIo->SegmentNumber, Descriptors->AddrRangeMin));
+    Status = ScanPciBus(Context, (UINT16) PciRootBridgeIo->SegmentNumber, (UINT8) Descriptors->AddrRangeMin, Callback);
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+  }
+
+  FreePool(HandleBuffer);
+
+  return Status;
+}
+
 /**
   Dump the PCI device information managed by this VTd engine.
 
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82451): https://edk2.groups.io/g/devel/message/82451
Mute This Topic: https://groups.io/mt/86487827/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v3] IntelSiliconPkg/IntelVTdDxe: Support Multi PCI Root Bus
Posted by Ni, Ray 2 years, 6 months ago
Wei,

> +  if (EFI_ERROR (Status)) {
> +    //
> +    // If PciRootBridgeIo protocol is not support, scan PCI device from root bus 0x00.
> +    //
> +    Status = ScanPciBus(Context, Segment, 0x00, Callback);

1. Is the error handling necessary?
In a simple client platform, there is only one PciRootBridgeIo protocol instance.
In a complex client or server platform, there are multiple PciRootBridgeIo protocol instances.

Additionally, the INF has a dependency on PciRootBridgeIo protocol.
The driver doesn't run if the protocol doesn't exist.


> +
> +    while (Descriptors->Desc != ACPI_END_TAG_DESCRIPTOR) {
> +      if (Descriptors->ResType == ACPI_ADDRESS_SPACE_TYPE_BUS) {
> +        break;
> +      }
> +      Descriptors++;
> +    }
> +

2. You might need to add the error handling in case the ACPI descriptor doesn't contain
record for ACPI_ADDRESS_SPACE_TYPE_BUS.
That would be a fatal error. So, even an assertion is ok to me.

Thanks,
Ray


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82469): https://edk2.groups.io/g/devel/message/82469
Mute This Topic: https://groups.io/mt/86487827/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v3] IntelSiliconPkg/IntelVTdDxe: Support Multi PCI Root Bus
Posted by Huang, Jenny 2 years, 6 months ago
Reviewed-by: Jenny Huang <jenny.huang@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sheng Wei
Sent: Thursday, October 21, 2021 5:11 AM
To: devel@edk2.groups.io
Cc: Kowalewski, Robert <robert.kowalewski@intel.com>; Huang, Jenny <jenny.huang@intel.com>; Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Albecki, Mateusz <mateusz.albecki@intel.com>; Kolakowski, Jacek <Jacek.Kolakowski@intel.com>
Subject: [edk2-devel] [PATCH v3] IntelSiliconPkg/IntelVTdDxe: Support Multi PCI Root Bus

Some system may has multi PCI root bus. It needs to use PciRootBridgeIo  protocol to get the root bus count. Scan each root bus to get all devices.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3695

Signed-off-by: Robert Kowalewski <robert.kowalewski@intel.com>
Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Jenny Huang <jenny.huang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Robert Kowalewski <robert.kowalewski@intel.com>
Cc: Albecki Mateusz <mateusz.albecki@intel.com>
Cc: Kolakowski Jacek <Jacek.Kolakowski@intel.com>
---
 .../Feature/VTd/IntelVTdDxe/DmaProtection.h        | 17 +++++
 .../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c        |  2 +-
 .../Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf        |  1 +
 .../Feature/VTd/IntelVTdDxe/PciInfo.c              | 72 ++++++++++++++++++++++
 4 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
index a24fbc37..7dd29a24 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.h
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtectio
+++ n.h
@@ -33,6 +33,7 @@
 #include <Protocol/PciEnumerationComplete.h>
 #include <Protocol/PlatformVtdPolicy.h>  #include <Protocol/IoMmu.h>
+#include <Protocol/PciRootBridgeIo.h>
 
 #include <IndustryStandard/Pci.h>
 #include <IndustryStandard/DmaRemappingReportingTable.h>
@@ -341,6 +342,22 @@ ScanPciBus (
   IN SCAN_BUS_FUNC_CALLBACK_FUNC  Callback
   );
 
+/**
+  Scan PCI bus and invoke callback function for each PCI devices under all root bus.
+
+  @param[in]  Context               The context of the callback function.
+  @param[in]  Segment               The segment of the source.
+  @param[in]  Callback              The callback function in PCI scan.
+
+  @retval EFI_SUCCESS           The PCI devices under the bus are scaned.
+**/
+EFI_STATUS
+ScanAllPciBus (
+  IN VOID                         *Context,
+  IN UINT16                       Segment,
+  IN SCAN_BUS_FUNC_CALLBACK_FUNC  Callback
+  );
+
 /**
   Dump the PCI device information managed by this VTd engine.
 
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
index 2d9b4374..1ee290b7 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTabl
+++ e.c
@@ -692,7 +692,7 @@ ProcessDhrd (
     mVtdUnitInformation[VtdIndex].PciDeviceInfo.IncludeAllFlag = TRUE;
     DEBUG ((DEBUG_INFO,"  ProcessDhrd: with INCLUDE ALL\n"));
 
-    Status = ScanPciBus((VOID *)VtdIndex, DmarDrhd->SegmentNumber, 0, ScanBusCallbackRegisterPciDevice);
+    Status = ScanAllPciBus((VOID *)VtdIndex, DmarDrhd->SegmentNumber, 
+ ScanBusCallbackRegisterPciDevice);
     if (EFI_ERROR (Status)) {
       return Status;
     }
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
index 220636ad..387f90e3 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.
+++ inf
@@ -70,6 +70,7 @@
   gEfiPciIoProtocolGuid                       ## CONSUMES
   gEfiPciEnumerationCompleteProtocolGuid      ## CONSUMES
   gEdkiiPlatformVTdPolicyProtocolGuid         ## SOMETIMES_CONSUMES
+  gEfiPciRootBridgeIoProtocolGuid             ## CONSUMES
 
 [Pcd]
   gIntelSiliconPkgTokenSpaceGuid.PcdVTdPolicyPropertyMask   ## CONSUMES
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
index 4af376b3..cffb9f61 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/PciInfo.c
@@ -279,6 +279,78 @@ ScanPciBus (
   return EFI_SUCCESS;
 }
 
+/**
+  Scan PCI bus and invoke callback function for each PCI devices under all root bus.
+
+  @param[in]  Context               The context of the callback function.
+  @param[in]  Segment               The segment of the source.
+  @param[in]  Callback              The callback function in PCI scan.
+
+  @retval EFI_SUCCESS           The PCI devices under the bus are scaned.
+**/
+EFI_STATUS
+ScanAllPciBus (
+  IN VOID                         *Context,
+  IN UINT16                       Segment,
+  IN SCAN_BUS_FUNC_CALLBACK_FUNC  Callback
+  )
+{
+  EFI_STATUS                        Status;
+  UINTN                             Index;
+  UINTN                             HandleCount;
+  EFI_HANDLE                        *HandleBuffer;
+  EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL   *PciRootBridgeIo;
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptors;
+
+  DEBUG ((DEBUG_INFO, "ScanAllPciBus ()\n"));
+
+  Status = gBS->LocateHandleBuffer (
+                  ByProtocol,
+                  &gEfiPciRootBridgeIoProtocolGuid,
+                  NULL,
+                  &HandleCount,
+                  &HandleBuffer
+                  );
+  if (EFI_ERROR (Status)) {
+    //
+    // If PciRootBridgeIo protocol is not support, scan PCI device from root bus 0x00.
+    //
+    Status = ScanPciBus(Context, Segment, 0x00, Callback);
+    return Status;
+  }
+
+  DEBUG ((DEBUG_INFO,"Find %d root bridges\n", HandleCount));
+
+  for (Index = 0; Index < HandleCount; Index++) {
+    Status = gBS->HandleProtocol (
+                    HandleBuffer[Index],
+                    &gEfiPciRootBridgeIoProtocolGuid,
+                    (VOID **) &PciRootBridgeIo
+                    );
+    ASSERT_EFI_ERROR (Status);
+
+    Status = PciRootBridgeIo->Configuration (PciRootBridgeIo, (VOID **) &Descriptors);
+    ASSERT_EFI_ERROR (Status);
+
+    while (Descriptors->Desc != ACPI_END_TAG_DESCRIPTOR) {
+      if (Descriptors->ResType == ACPI_ADDRESS_SPACE_TYPE_BUS) {
+        break;
+      }
+      Descriptors++;
+    }
+
+    DEBUG ((DEBUG_INFO,"Scan root bridges : %d, Segment : %d, Bus : 0x%02X\n", Index, PciRootBridgeIo->SegmentNumber, Descriptors->AddrRangeMin));
+    Status = ScanPciBus(Context, (UINT16) PciRootBridgeIo->SegmentNumber, (UINT8) Descriptors->AddrRangeMin, Callback);
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+  }
+
+  FreePool(HandleBuffer);
+
+  return Status;
+}
+
 /**
   Dump the PCI device information managed by this VTd engine.
 
--
2.16.2.windows.1








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82740): https://edk2.groups.io/g/devel/message/82740
Mute This Topic: https://groups.io/mt/86487827/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-