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


Sheng Wei
 

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


Ni, Ray
 

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


Huang, Jenny
 

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