* [PATCH v2 0/4] Add extra pci roots support for Arm
@ 2020-11-07 7:40 Jiahui Cen
2020-11-07 7:40 ` [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib cenjiahui
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Jiahui Cen @ 2020-11-07 7:40 UTC (permalink / raw)
To: devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, leif, xieyingtai,
miaoyubo, Jiahui Cen
Changes with v1
v1->v2:
Separated into four patches.
Factor the same logic parts into a new library.
v1: https://edk2.groups.io/g/devel/topic/72723351#56901
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3059
QEMU: https://lore.kernel.org/qemu-devel/20201103120157.2286-1-cenjiahui@huawei.com/
This patch series adds support for extra pci roots for ARM.
In order to avoid duplicated codes, we introduce a new library
PciHostBridgeUtilityLib which extracts common interfaces from
OvmfPkg/PciHostBridgeLib. It provides conflicts informing and extra pci
roots scanning. Using the utility lib, the uefi could scan for extra
root buses and recognize multiple roots for ARM.
Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Yubo Miao (4):
OvmfPkg: Extract functions form PciHostBridgeLib
ArmVirtPkg: Use extracted PciHostBridgeUtilityLib
OvmfPkg: Extract functions of extra pci roots
ArmVirtPkg: Support extra pci roots
ArmVirtPkg/ArmVirt.dsc.inc | 1 +
.../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 223 +++++++-------
.../FdtPciHostBridgeLib.inf | 5 +
.../Include/Library/PciHostBridgeUtilityLib.h | 98 ++++++
.../PciHostBridgeLib/PciHostBridgeLib.c | 230 +-------------
.../PciHostBridgeLib/PciHostBridgeLib.inf | 1 +
.../PciHostBridgeUtilityLib.c | 280 ++++++++++++++++++
.../PciHostBridgeUtilityLib.inf | 51 ++++
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
12 files changed, 560 insertions(+), 333 deletions(-)
create mode 100644 OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
create mode 100644 OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
create mode 100644 OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
--
2.19.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib
2020-11-07 7:40 [PATCH v2 0/4] Add extra pci roots support for Arm Jiahui Cen
@ 2020-11-07 7:40 ` cenjiahui
2020-11-07 7:40 ` [PATCH v2 2/4] ArmVirtPkg: Use extracted PciHostBridgeUtilityLib Jiahui Cen
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: cenjiahui @ 2020-11-07 7:40 UTC (permalink / raw)
To: devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, leif, xieyingtai,
miaoyubo, Jiahui Cen
From: Yubo Miao <miaoyubo@huawei.com>
Introduce a new PciHostBridgeUtilityLib class to share duplicate code
between OvmfPkg and ArmVirtPkg.
Extract function PciHostBridgeResourceConflict from
OvmfPkg/PciHostBridgeLib.
Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
.../Include/Library/PciHostBridgeUtilityLib.h | 33 +++++++++
.../PciHostBridgeLib/PciHostBridgeLib.c | 62 +---------------
.../PciHostBridgeLib/PciHostBridgeLib.inf | 1 +
.../PciHostBridgeUtilityLib.c | 71 +++++++++++++++++++
.../PciHostBridgeUtilityLib.inf | 49 +++++++++++++
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfXen.dsc | 1 +
9 files changed, 159 insertions(+), 61 deletions(-)
create mode 100644 OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
create mode 100644 OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
create mode 100644 OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
diff --git a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
new file mode 100644
index 0000000000..d2622fd907
--- /dev/null
+++ b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
@@ -0,0 +1,33 @@
+/** @file
+ PCI Host Bridge Library consumed by PciHostBridgeDxe driver returning
+ the platform specific information about the PCI Host Bridge.
+
+ Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#ifndef __PCI_HOST_BRIDGE_UTILITY_LIB_H__
+#define __PCI_HOST_BRIDGE_UTILITY_LIB_H__
+
+/**
+ Inform the platform that the resource conflict happens.
+
+ @param HostBridgeHandle Handle of the Host Bridge.
+ @param Configuration Pointer to PCI I/O and PCI memory resource
+ descriptors. The Configuration contains the resources
+ for all the root bridges. The resource for each root
+ bridge is terminated with END descriptor and an
+ additional END is appended indicating the end of the
+ entire resources. The resource descriptor field
+ values follow the description in
+ EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL
+ .SubmitResources().
+**/
+VOID
+EFIAPI
+PciHostBridgeResourceConflict (
+ EFI_HANDLE HostBridgeHandle,
+ VOID *Configuration
+ );
+
+#endif
diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
index e850f7d183..d6b0e00f80 100644
--- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
+++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
@@ -23,6 +23,7 @@
#include <Library/PciLib.h>
#include <Library/QemuFwCfgLib.h>
#include "PciHostBridge.h"
+#include <Library/PciHostBridgeUtilityLib.h>
#pragma pack(1)
@@ -34,10 +35,6 @@ typedef struct {
GLOBAL_REMOVE_IF_UNREFERENCED
-CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
- L"Mem", L"I/O", L"Bus"
-};
-
STATIC
CONST
@@ -384,60 +381,3 @@ PciHostBridgeFreeRootBridges (
FreePool (Bridges);
}
-
-
-/**
- Inform the platform that the resource conflict happens.
-
- @param HostBridgeHandle Handle of the Host Bridge.
- @param Configuration Pointer to PCI I/O and PCI memory resource
- descriptors. The Configuration contains the resources
- for all the root bridges. The resource for each root
- bridge is terminated with END descriptor and an
- additional END is appended indicating the end of the
- entire resources. The resource descriptor field
- values follow the description in
- EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL
- .SubmitResources().
-**/
-VOID
-EFIAPI
-PciHostBridgeResourceConflict (
- EFI_HANDLE HostBridgeHandle,
- VOID *Configuration
- )
-{
- EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
- UINTN RootBridgeIndex;
- DEBUG ((DEBUG_ERROR, "PciHostBridge: Resource conflict happens!\n"));
-
- RootBridgeIndex = 0;
- Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) Configuration;
- while (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) {
- DEBUG ((DEBUG_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++));
- for (; Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR; Descriptor++) {
- ASSERT (Descriptor->ResType <
- ARRAY_SIZE (mPciHostBridgeLibAcpiAddressSpaceTypeStr)
- );
- DEBUG ((DEBUG_ERROR, " %s: Length/Alignment = 0x%lx / 0x%lx\n",
- mPciHostBridgeLibAcpiAddressSpaceTypeStr[Descriptor->ResType],
- Descriptor->AddrLen, Descriptor->AddrRangeMax
- ));
- if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
- DEBUG ((DEBUG_ERROR, " Granularity/SpecificFlag = %ld / %02x%s\n",
- Descriptor->AddrSpaceGranularity, Descriptor->SpecificFlag,
- ((Descriptor->SpecificFlag &
- EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE
- ) != 0) ? L" (Prefetchable)" : L""
- ));
- }
- }
- //
- // Skip the END descriptor for root bridge
- //
- ASSERT (Descriptor->Desc == ACPI_END_TAG_DESCRIPTOR);
- Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)(
- (EFI_ACPI_END_TAG_DESCRIPTOR *)Descriptor + 1
- );
- }
-}
diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
index 6ec9ec7514..7d01528c94 100644
--- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
+++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
@@ -41,6 +41,7 @@
MemoryAllocationLib
PciLib
QemuFwCfgLib
+ PciHostBridgeUtilityLib
[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
new file mode 100644
index 0000000000..4b34f92fd8
--- /dev/null
+++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
@@ -0,0 +1,71 @@
+/** @file
+ OVMF's instance of the PCI Host Bridge Library.
+
+ Copyright (c) 2020, Huawei Corporation. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include <Library/DebugLib.h>
+#include <Library/PciHostBridgeUtilityLib.h>
+
+CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
+ L"Mem", L"I/O", L"Bus"
+};
+
+/**
+ Inform the platform that the resource conflict happens.
+
+ @param HostBridgeHandle Handle of the Host Bridge.
+ @param Configuration Pointer to PCI I/O and PCI memory resource
+ descriptors. The Configuration contains the resources
+ for all the root bridges. The resource for each root
+ bridge is terminated with END descriptor and an
+ additional END is appended indicating the end of the
+ entire resources. The resource descriptor field
+ values follow the description in
+ EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL
+ .SubmitResources().
+**/
+VOID
+EFIAPI
+PciHostBridgeResourceConflict (
+ EFI_HANDLE HostBridgeHandle,
+ VOID *Configuration
+ )
+{
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
+ UINTN RootBridgeIndex;
+ DEBUG ((DEBUG_ERROR, "PciHostBridge: Resource conflict happens!\n"));
+
+ RootBridgeIndex = 0;
+ Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) Configuration;
+ while (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) {
+ DEBUG ((DEBUG_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++));
+ for (; Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR; Descriptor++) {
+ ASSERT (Descriptor->ResType <
+ ARRAY_SIZE (mPciHostBridgeLibAcpiAddressSpaceTypeStr)
+ );
+ DEBUG ((DEBUG_ERROR, " %s: Length/Alignment = 0x%lx / 0x%lx\n",
+ mPciHostBridgeLibAcpiAddressSpaceTypeStr[Descriptor->ResType],
+ Descriptor->AddrLen, Descriptor->AddrRangeMax
+ ));
+ if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
+ DEBUG ((DEBUG_ERROR, " Granularity/SpecificFlag = %ld / %02x%s\n",
+ Descriptor->AddrSpaceGranularity, Descriptor->SpecificFlag,
+ ((Descriptor->SpecificFlag &
+ EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE
+ ) != 0) ? L" (Prefetchable)" : L""
+ ));
+ }
+ }
+ //
+ // Skip the END descriptor for root bridge
+ //
+ ASSERT (Descriptor->Desc == ACPI_END_TAG_DESCRIPTOR);
+ Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)(
+ (EFI_ACPI_END_TAG_DESCRIPTOR *)Descriptor + 1
+ );
+ }
+}
+
diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
new file mode 100644
index 0000000000..c88ab8e415
--- /dev/null
+++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
@@ -0,0 +1,49 @@
+## @file
+# OVMF and Arm's instance of the PCI Host Bridge Utility Library.
+#
+# Copyright (C) 2016, Red Hat, Inc.
+# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = PciHostBridgeUtilityLib
+ FILE_GUID = e3aa5932-527a-42e7-86f5-81b144c7e5f1
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = PciHostBridgeUtilityLib
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+# VALID_ARCHITECTURES = IA32 X64 EBC
+#
+
+[Sources]
+ PciHostBridgeUtilityLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ DevicePathLib
+ MemoryAllocationLib
+ PciLib
+ QemuFwCfgLib
+
+[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base
+ gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size
+ gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base
+ gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 58d9f292f9..0c2bf0b13c 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -738,6 +738,7 @@
<LibraryClasses>
PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
+ PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
}
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
<LibraryClasses>
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 3551f9710a..baf36a4f7a 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -752,6 +752,7 @@
<LibraryClasses>
PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
+ PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
}
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
<LibraryClasses>
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 7a8bdb8a86..219b5f559b 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -748,6 +748,7 @@
<LibraryClasses>
PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
+ PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
}
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
<LibraryClasses>
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 34c9de19df..442c0730ef 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -544,6 +544,7 @@
<LibraryClasses>
PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
+ PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
}
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
<LibraryClasses>
--
2.19.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/4] ArmVirtPkg: Use extracted PciHostBridgeUtilityLib
2020-11-07 7:40 [PATCH v2 0/4] Add extra pci roots support for Arm Jiahui Cen
2020-11-07 7:40 ` [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib cenjiahui
@ 2020-11-07 7:40 ` Jiahui Cen
2020-11-07 7:40 ` [PATCH v2 3/4] OvmfPkg: Extract functions of extra pci roots Jiahui Cen
2020-11-07 7:40 ` [PATCH v2 4/4] ArmVirtPkg: Support " Jiahui Cen
3 siblings, 0 replies; 7+ messages in thread
From: Jiahui Cen @ 2020-11-07 7:40 UTC (permalink / raw)
To: devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, leif, xieyingtai,
miaoyubo, Jiahui Cen
From: Yubo Miao <miaoyubo@huawei.com>
Eliminate the currently duplicated code in ArmVirtPkg and use the
extracted PciHostBridgeResourceConflict from PciHostBridgeUtilityLib.
Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
---
ArmVirtPkg/ArmVirt.dsc.inc | 1 +
.../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 61 +------------------
.../FdtPciHostBridgeLib.inf | 2 +
3 files changed, 4 insertions(+), 60 deletions(-)
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 4dafd1fa0f..593a523171 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -144,6 +144,7 @@
PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+ PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
# USB Libraries
UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index 496b192d22..3952f511b4 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -14,6 +14,7 @@
#include <Library/MemoryAllocationLib.h>
#include <Library/PcdLib.h>
#include <Library/UefiBootServicesTableLib.h>
+#include <Library/PciHostBridgeUtilityLib.h>
#include <Protocol/FdtClient.h>
#include <Protocol/PciRootBridgeIo.h>
@@ -51,9 +52,6 @@ STATIC EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mEfiPciRootBridgeDevicePath = {
};
GLOBAL_REMOVE_IF_UNREFERENCED
-CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
- L"Mem", L"I/O", L"Bus"
-};
//
// We expect the "ranges" property of "pci-host-ecam-generic" to consist of
@@ -414,60 +412,3 @@ PciHostBridgeFreeRootBridges (
ASSERT (Count == 1);
}
-/**
- Inform the platform that the resource conflict happens.
-
- @param HostBridgeHandle Handle of the Host Bridge.
- @param Configuration Pointer to PCI I/O and PCI memory resource
- descriptors. The Configuration contains the resources
- for all the root bridges. The resource for each root
- bridge is terminated with END descriptor and an
- additional END is appended indicating the end of the
- entire resources. The resource descriptor field
- values follow the description in
- EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL
- .SubmitResources().
-**/
-VOID
-EFIAPI
-PciHostBridgeResourceConflict (
- EFI_HANDLE HostBridgeHandle,
- VOID *Configuration
- )
-{
- EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
- UINTN RootBridgeIndex;
- DEBUG ((EFI_D_ERROR, "PciHostBridge: Resource conflict happens!\n"));
-
- RootBridgeIndex = 0;
- Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) Configuration;
- while (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) {
- DEBUG ((EFI_D_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++));
- for (; Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR; Descriptor++) {
- ASSERT (Descriptor->ResType <
- (sizeof (mPciHostBridgeLibAcpiAddressSpaceTypeStr) /
- sizeof (mPciHostBridgeLibAcpiAddressSpaceTypeStr[0])
- )
- );
- DEBUG ((EFI_D_ERROR, " %s: Length/Alignment = 0x%lx / 0x%lx\n",
- mPciHostBridgeLibAcpiAddressSpaceTypeStr[Descriptor->ResType],
- Descriptor->AddrLen, Descriptor->AddrRangeMax
- ));
- if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
- DEBUG ((EFI_D_ERROR, " Granularity/SpecificFlag = %ld / %02x%s\n",
- Descriptor->AddrSpaceGranularity, Descriptor->SpecificFlag,
- ((Descriptor->SpecificFlag &
- EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE
- ) != 0) ? L" (Prefetchable)" : L""
- ));
- }
- }
- //
- // Skip the END descriptor for root bridge
- //
- ASSERT (Descriptor->Desc == ACPI_END_TAG_DESCRIPTOR);
- Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)(
- (EFI_ACPI_END_TAG_DESCRIPTOR *)Descriptor + 1
- );
- }
-}
diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
index 277ccfd245..97e9368c8e 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
@@ -31,6 +31,7 @@
ArmVirtPkg/ArmVirtPkg.dec
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
[LibraryClasses]
DebugLib
@@ -38,6 +39,7 @@
DxeServicesTableLib
MemoryAllocationLib
PciPcdProducerLib
+ PciHostBridgeUtilityLib
[FixedPcd]
gArmTokenSpaceGuid.PcdPciMmio32Translation
--
2.19.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/4] OvmfPkg: Extract functions of extra pci roots
2020-11-07 7:40 [PATCH v2 0/4] Add extra pci roots support for Arm Jiahui Cen
2020-11-07 7:40 ` [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib cenjiahui
2020-11-07 7:40 ` [PATCH v2 2/4] ArmVirtPkg: Use extracted PciHostBridgeUtilityLib Jiahui Cen
@ 2020-11-07 7:40 ` Jiahui Cen
2020-11-07 7:40 ` [PATCH v2 4/4] ArmVirtPkg: Support " Jiahui Cen
3 siblings, 0 replies; 7+ messages in thread
From: Jiahui Cen @ 2020-11-07 7:40 UTC (permalink / raw)
To: devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, leif, xieyingtai,
miaoyubo, Jiahui Cen
From: Yubo Miao <miaoyubo@huawei.com>
To use extra pci roots in other lib, it is necessary to extract
UninitRootBridge, PciHostBridgeFreeRootBridges and
PciHostBridgeExtraRoots from Ovmf/PciHostBridgeLib.
Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
.../Include/Library/PciHostBridgeUtilityLib.h | 65 ++++++
.../PciHostBridgeLib/PciHostBridgeLib.c | 168 +-------------
.../PciHostBridgeUtilityLib.c | 209 ++++++++++++++++++
.../PciHostBridgeUtilityLib.inf | 2 +
4 files changed, 286 insertions(+), 158 deletions(-)
diff --git a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
index d2622fd907..35fe237b08 100644
--- a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
+++ b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h
@@ -9,6 +9,71 @@
#ifndef __PCI_HOST_BRIDGE_UTILITY_LIB_H__
#define __PCI_HOST_BRIDGE_UTILITY_LIB_H__
+/**
+ 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.
+**/
+VOID
+UninitRootBridge (
+ IN PCI_ROOT_BRIDGE *RootBus
+ );
+
+/**
+ Free the root bridge instances array returned from
+ PciHostBridgeGetRootBridges().
+
+ @param The root bridge instances array.
+ @param The count of the array.
+**/
+VOID
+EFIAPI
+PciHostBridgeFreeRootBridges (
+ PCI_ROOT_BRIDGE *Bridges,
+ UINTN Count
+ );
+
+/**
+ Return all the root bridge instances in an array.
+
+ @param Count Return the count of root bridge instances.
+
+ @param[in] PMem Prefetchable MMIO aperture.
+
+ @param[in] PMemAbove4G Prefetchable MMIO aperture above 4G.
+
+ @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.
+
+ @return All the root bridge instances in an array.
+ The array should be passed into PciHostBridgeFreeRootBridges()
+ when it's not used.
+**/
+PCI_ROOT_BRIDGE *
+EFIAPI
+PciHostBridgeExtraRoots (
+ UINTN *Count,
+ PCI_ROOT_BRIDGE_APERTURE PMem,
+ PCI_ROOT_BRIDGE_APERTURE PMemAbove4G,
+ UINT32 BusMax,
+ UINT64 Attributes,
+ UINT64 AllocationAttributes,
+ PCI_ROOT_BRIDGE_APERTURE Io,
+ PCI_ROOT_BRIDGE_APERTURE Mem,
+ PCI_ROOT_BRIDGE_APERTURE MemAbove4G
+);
+
/**
Inform the platform that the resource conflict happens.
diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
index d6b0e00f80..48a4c089b7 100644
--- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
+++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
@@ -161,24 +161,6 @@ InitRootBridge (
return EFI_SUCCESS;
}
-
-/**
- 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.
@@ -194,14 +176,7 @@ 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;
@@ -241,143 +216,20 @@ 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,
+ Bridges = PciHostBridgeExtraRoots (
+ Count,
+ mNonExistAperture,
+ mNonExistAperture,
+ PCI_MAX_BUS,
Attributes,
AllocationAttributes,
- (UINT8) LastRootBridgeNumber,
- PCI_MAX_BUS,
- &Io,
- &Mem,
- &MemAbove4G,
- &mNonExistAperture,
- &mNonExistAperture,
- &Bridges[Initialized]
+ Io,
+ Mem,
+ MemAbove4G
);
- if (EFI_ERROR (Status)) {
- goto FreeBridges;
- }
- ++Initialized;
-
- *Count = Initialized;
- return Bridges;
-
-FreeBridges:
- while (Initialized > 0) {
- --Initialized;
- UninitRootBridge (&Bridges[Initialized]);
+ if (Bridges) {
+ return Bridges;
}
-
- FreePool (Bridges);
return NULL;
}
-
-/**
- Free the root bridge instances array returned from
- PciHostBridgeGetRootBridges().
-
- @param The root bridge instances array.
- @param The count of the array.
-**/
-VOID
-EFIAPI
-PciHostBridgeFreeRootBridges (
- 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);
-}
diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
index 4b34f92fd8..5ed91daf1e 100644
--- a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
+++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
@@ -6,13 +6,222 @@
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
+#include <IndustryStandard/Pci.h>
#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PciHostBridgeLib.h>
#include <Library/PciHostBridgeUtilityLib.h>
+#include <Library/QemuFwCfgLib.h>
+#include <Library/PciLib.h>
+#include "Library/PciHostBridgeLib/PciHostBridge.h"
CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
L"Mem", L"I/O", L"Bus"
};
+/**
+ 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.
+**/
+VOID
+UninitRootBridge (
+ IN PCI_ROOT_BRIDGE *RootBus
+ )
+{
+ FreePool (RootBus->DevicePath);
+}
+
+/**
+ Free the root bridge instances array returned from
+ PciHostBridgeGetRootBridges().
+
+ @param The root bridge instances array.
+ @param The count of the array.
+**/
+VOID
+EFIAPI
+PciHostBridgeFreeRootBridges (
+ 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);
+}
+
+/**
+ Return all the root bridge instances in an array.
+
+ @param Count Return the count of root bridge instances.
+
+ @param[in] PMem Prefetchable MMIO aperture.
+
+ @param[in] PMemAbove4G Prefetchable MMIO aperture above 4G.
+
+ @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.
+
+ @return All the root bridge instances in an array.
+ The array should be passed into PciHostBridgeFreeRootBridges()
+ when it's not used.
+**/
+
+PCI_ROOT_BRIDGE *
+EFIAPI
+PciHostBridgeExtraRoots (
+ UINTN *Count,
+ PCI_ROOT_BRIDGE_APERTURE PMem,
+ PCI_ROOT_BRIDGE_APERTURE PMemAbove4G,
+ UINT32 BusMax,
+ UINT64 Attributes,
+ UINT64 AllocationAttributes,
+ PCI_ROOT_BRIDGE_APERTURE Io,
+ PCI_ROOT_BRIDGE_APERTURE Mem,
+ PCI_ROOT_BRIDGE_APERTURE MemAbove4G
+)
+{
+ 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) {
+ 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 <= 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;
+}
/**
Inform the platform that the resource conflict happens.
diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
index c88ab8e415..1a667870ee 100644
--- a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
+++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
@@ -28,6 +28,7 @@
PciHostBridgeUtilityLib.c
[Packages]
+ MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
OvmfPkg/OvmfPkg.dec
@@ -38,6 +39,7 @@
MemoryAllocationLib
PciLib
QemuFwCfgLib
+ PciHostBridgeLib
[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase
--
2.19.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 4/4] ArmVirtPkg: Support extra pci roots
2020-11-07 7:40 [PATCH v2 0/4] Add extra pci roots support for Arm Jiahui Cen
` (2 preceding siblings ...)
2020-11-07 7:40 ` [PATCH v2 3/4] OvmfPkg: Extract functions of extra pci roots Jiahui Cen
@ 2020-11-07 7:40 ` Jiahui Cen
3 siblings, 0 replies; 7+ messages in thread
From: Jiahui Cen @ 2020-11-07 7:40 UTC (permalink / raw)
To: devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, leif, xieyingtai,
miaoyubo, Jiahui Cen
From: Yubo Miao <miaoyubo@huawei.com>
As the implementation of extra pci roots is shared in
PciHostBridgeUtilityLib, let's call PciHostBridgeExtraRoots to support it.
Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
---
.../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 164 ++++++++++++------
.../FdtPciHostBridgeLib.inf | 3 +
2 files changed, 112 insertions(+), 55 deletions(-)
diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index 3952f511b4..d33f833a50 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -15,6 +15,11 @@
#include <Library/PcdLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/PciHostBridgeUtilityLib.h>
+#include <Library/QemuFwCfgLib.h>
+#include <Library/PciLib.h>
+#include <IndustryStandard/Pci.h>
+#include <Library/BaseMemoryLib.h>
+#include "Library/PciHostBridgeLib/PciHostBridge.h"
#include <Protocol/FdtClient.h>
#include <Protocol/PciRootBridgeIo.h>
@@ -304,7 +309,60 @@ ProcessPciHost (
return Status;
}
-STATIC PCI_ROOT_BRIDGE mRootBridge;
+EFI_STATUS
+InitRootBridge (
+ IN UINT64 Supports,
+ IN UINT64 Attributes,
+ IN UINT64 AllocAttributes,
+ IN UINT8 RootBusNumber,
+ IN UINT8 MaxSubBusNumber,
+ IN PCI_ROOT_BRIDGE_APERTURE *Io,
+ IN PCI_ROOT_BRIDGE_APERTURE *Mem,
+ IN PCI_ROOT_BRIDGE_APERTURE *MemAbove4G,
+ IN PCI_ROOT_BRIDGE_APERTURE *PMem,
+ IN PCI_ROOT_BRIDGE_APERTURE *PMemAbove4G,
+ OUT PCI_ROOT_BRIDGE *RootBus
+ )
+{
+ EFI_PCI_ROOT_BRIDGE_DEVICE_PATH *DevicePath;
+
+ //
+ // Be safe if other fields are added to PCI_ROOT_BRIDGE later.
+ //
+ ZeroMem (RootBus, sizeof *RootBus);
+
+ RootBus->Segment = 0;
+ RootBus->ResourceAssigned = FALSE;
+ RootBus->Supports = Supports;
+ RootBus->Attributes = Attributes;
+
+ RootBus->DmaAbove4G = TRUE;
+
+ RootBus->AllocationAttributes = AllocAttributes;
+ RootBus->Bus.Base = RootBusNumber;
+ RootBus->Bus.Limit = MaxSubBusNumber;
+ CopyMem (&RootBus->Io, Io, sizeof (*Io));
+ CopyMem (&RootBus->Mem, Mem, sizeof (*Mem));
+ CopyMem (&RootBus->MemAbove4G, MemAbove4G, sizeof (*MemAbove4G));
+ CopyMem (&RootBus->PMem, PMem, sizeof (*PMem));
+ CopyMem (&RootBus->PMemAbove4G, PMemAbove4G, sizeof (*PMemAbove4G));
+
+ RootBus->NoExtendedConfigSpace = FALSE;
+
+ DevicePath = AllocateCopyPool (sizeof mEfiPciRootBridgeDevicePath,
+ &mEfiPciRootBridgeDevicePath);
+ if (DevicePath == NULL) {
+ DEBUG ((EFI_D_ERROR, "%a: %r\n", __FUNCTION__, EFI_OUT_OF_RESOURCES));
+ return EFI_OUT_OF_RESOURCES;
+ }
+ DevicePath->AcpiDevicePath.UID = RootBusNumber;
+ RootBus->DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)DevicePath;
+
+ DEBUG ((EFI_D_INFO,
+ "%a: populated root bus %d, with room for %d subordinate bus(es)\n",
+ __FUNCTION__, RootBusNumber, MaxSubBusNumber - RootBusNumber));
+ return EFI_SUCCESS;
+}
/**
Return all the root bridge instances in an array.
@@ -321,11 +379,19 @@ PciHostBridgeGetRootBridges (
UINTN *Count
)
{
- UINT64 IoBase, IoSize;
- UINT64 Mmio32Base, Mmio32Size;
- UINT64 Mmio64Base, Mmio64Size;
- UINT32 BusMin, BusMax;
- EFI_STATUS Status;
+ UINT64 IoBase, IoSize;
+ UINT64 Mmio32Base, Mmio32Size;
+ UINT64 Mmio64Base, Mmio64Size;
+ UINT32 BusMin, BusMax;
+ PCI_ROOT_BRIDGE *Bridges;
+ UINT64 Attributes;
+ UINT64 AllocationAttributes;
+ EFI_STATUS Status;
+ PCI_ROOT_BRIDGE_APERTURE Io;
+ PCI_ROOT_BRIDGE_APERTURE PMem;
+ PCI_ROOT_BRIDGE_APERTURE PMemAbove4G;
+ PCI_ROOT_BRIDGE_APERTURE Mem;
+ PCI_ROOT_BRIDGE_APERTURE MemAbove4G;
if (PcdGet64 (PcdPciExpressBaseAddress) == 0) {
DEBUG ((EFI_D_INFO, "%a: PCI host bridge not present\n", __FUNCTION__));
@@ -343,33 +409,27 @@ PciHostBridgeGetRootBridges (
return NULL;
}
- *Count = 1;
+ ZeroMem (&Io, sizeof (Io));
+ ZeroMem (&Mem, sizeof (Mem));
+ ZeroMem (&MemAbove4G, sizeof (MemAbove4G));
- mRootBridge.Segment = 0;
- mRootBridge.Supports = EFI_PCI_ATTRIBUTE_ISA_IO_16 |
- EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO |
- EFI_PCI_ATTRIBUTE_VGA_IO_16 |
- EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16;
- mRootBridge.Attributes = mRootBridge.Supports;
+ Attributes = EFI_PCI_ATTRIBUTE_ISA_IO_16 |
+ EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO |
+ EFI_PCI_ATTRIBUTE_VGA_IO_16 |
+ EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16;
- mRootBridge.DmaAbove4G = TRUE;
- mRootBridge.NoExtendedConfigSpace = FALSE;
- mRootBridge.ResourceAssigned = FALSE;
+ AllocationAttributes = EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM;
- mRootBridge.AllocationAttributes = EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM;
-
- mRootBridge.Bus.Base = BusMin;
- mRootBridge.Bus.Limit = BusMax;
- mRootBridge.Io.Base = IoBase;
- mRootBridge.Io.Limit = IoBase + IoSize - 1;
- mRootBridge.Mem.Base = Mmio32Base;
- mRootBridge.Mem.Limit = Mmio32Base + Mmio32Size - 1;
+ Io.Base = IoBase;
+ Io.Limit = IoBase + IoSize - 1;
+ Mem.Base = Mmio32Base;
+ Mem.Limit = Mmio32Base + Mmio32Size - 1;
if (sizeof (UINTN) == sizeof (UINT64)) {
- mRootBridge.MemAbove4G.Base = Mmio64Base;
- mRootBridge.MemAbove4G.Limit = Mmio64Base + Mmio64Size - 1;
+ MemAbove4G.Base = Mmio64Base;
+ MemAbove4G.Limit = Mmio64Base + Mmio64Size - 1;
if (Mmio64Size > 0) {
- mRootBridge.AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
+ AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
}
} else {
//
@@ -378,37 +438,31 @@ PciHostBridgeGetRootBridges (
// BARs unless they are allocated below 4 GB. So ignore the range above
// 4 GB in this case.
//
- mRootBridge.MemAbove4G.Base = MAX_UINT64;
- mRootBridge.MemAbove4G.Limit = 0;
+ MemAbove4G.Base = MAX_UINT64;
+ MemAbove4G.Limit = 0;
}
//
// No separate ranges for prefetchable and non-prefetchable BARs
//
- mRootBridge.PMem.Base = MAX_UINT64;
- mRootBridge.PMem.Limit = 0;
- mRootBridge.PMemAbove4G.Base = MAX_UINT64;
- mRootBridge.PMemAbove4G.Limit = 0;
-
- mRootBridge.DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)&mEfiPciRootBridgeDevicePath;
-
- return &mRootBridge;
-}
-
-/**
- Free the root bridge instances array returned from
- PciHostBridgeGetRootBridges().
-
- @param Bridges The root bridge instances array.
- @param Count The count of the array.
-**/
-VOID
-EFIAPI
-PciHostBridgeFreeRootBridges (
- PCI_ROOT_BRIDGE *Bridges,
- UINTN Count
- )
-{
- ASSERT (Count == 1);
+ PMem.Base = MAX_UINT64;
+ PMem.Limit = 0;
+ PMemAbove4G.Base = MAX_UINT64;
+ PMemAbove4G.Limit = 0;
+
+ Bridges = PciHostBridgeExtraRoots (
+ Count,
+ PMem,
+ PMemAbove4G,
+ BusMax,
+ Attributes,
+ AllocationAttributes,
+ Io,
+ Mem,
+ MemAbove4G
+ );
+ if (Bridges) {
+ return Bridges;
+ }
+ return NULL;
}
-
diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
index 97e9368c8e..1b0f9a2b90 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
@@ -39,6 +39,9 @@
DxeServicesTableLib
MemoryAllocationLib
PciPcdProducerLib
+ PciHostBridgeLib
+ BaseMemoryLib
+ QemuFwCfgLib
PciHostBridgeUtilityLib
[FixedPcd]
--
2.19.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/4] ArmVirtPkg: Use extracted PciHostBridgeUtilityLib
2020-11-09 13:05 [PATCH v2 0/4] Add extra pci roots support for Arm Jiahui Cen
@ 2020-11-09 13:05 ` Jiahui Cen
2020-11-11 17:27 ` Laszlo Ersek
0 siblings, 1 reply; 7+ messages in thread
From: Jiahui Cen @ 2020-11-09 13:05 UTC (permalink / raw)
To: devel
Cc: jordan.l.justen, lersek, ard.biesheuvel, leif, xieyingtai,
miaoyubo, Jiahui Cen
From: Yubo Miao <miaoyubo@huawei.com>
Eliminate the currently duplicated code in ArmVirtPkg and use the
extracted PciHostBridgeResourceConflict from PciHostBridgeUtilityLib.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
---
ArmVirtPkg/ArmVirt.dsc.inc | 1 +
ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf | 2 +
ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 61 +-------------------
3 files changed, 4 insertions(+), 60 deletions(-)
diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 4dafd1fa0f1d..593a523171ff 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -144,6 +144,7 @@ [LibraryClasses.common]
PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+ PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
# USB Libraries
UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
index 277ccfd24546..97e9368c8e9f 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
@@ -31,6 +31,7 @@ [Packages]
ArmVirtPkg/ArmVirtPkg.dec
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
[LibraryClasses]
DebugLib
@@ -38,6 +39,7 @@ [LibraryClasses]
DxeServicesTableLib
MemoryAllocationLib
PciPcdProducerLib
+ PciHostBridgeUtilityLib
[FixedPcd]
gArmTokenSpaceGuid.PcdPciMmio32Translation
diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index 496b192d2291..3952f511b4d2 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -14,6 +14,7 @@
#include <Library/MemoryAllocationLib.h>
#include <Library/PcdLib.h>
#include <Library/UefiBootServicesTableLib.h>
+#include <Library/PciHostBridgeUtilityLib.h>
#include <Protocol/FdtClient.h>
#include <Protocol/PciRootBridgeIo.h>
@@ -51,9 +52,6 @@ STATIC EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mEfiPciRootBridgeDevicePath = {
};
GLOBAL_REMOVE_IF_UNREFERENCED
-CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
- L"Mem", L"I/O", L"Bus"
-};
//
// We expect the "ranges" property of "pci-host-ecam-generic" to consist of
@@ -414,60 +412,3 @@ PciHostBridgeFreeRootBridges (
ASSERT (Count == 1);
}
-/**
- Inform the platform that the resource conflict happens.
-
- @param HostBridgeHandle Handle of the Host Bridge.
- @param Configuration Pointer to PCI I/O and PCI memory resource
- descriptors. The Configuration contains the resources
- for all the root bridges. The resource for each root
- bridge is terminated with END descriptor and an
- additional END is appended indicating the end of the
- entire resources. The resource descriptor field
- values follow the description in
- EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL
- .SubmitResources().
-**/
-VOID
-EFIAPI
-PciHostBridgeResourceConflict (
- EFI_HANDLE HostBridgeHandle,
- VOID *Configuration
- )
-{
- EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
- UINTN RootBridgeIndex;
- DEBUG ((EFI_D_ERROR, "PciHostBridge: Resource conflict happens!\n"));
-
- RootBridgeIndex = 0;
- Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) Configuration;
- while (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) {
- DEBUG ((EFI_D_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++));
- for (; Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR; Descriptor++) {
- ASSERT (Descriptor->ResType <
- (sizeof (mPciHostBridgeLibAcpiAddressSpaceTypeStr) /
- sizeof (mPciHostBridgeLibAcpiAddressSpaceTypeStr[0])
- )
- );
- DEBUG ((EFI_D_ERROR, " %s: Length/Alignment = 0x%lx / 0x%lx\n",
- mPciHostBridgeLibAcpiAddressSpaceTypeStr[Descriptor->ResType],
- Descriptor->AddrLen, Descriptor->AddrRangeMax
- ));
- if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
- DEBUG ((EFI_D_ERROR, " Granularity/SpecificFlag = %ld / %02x%s\n",
- Descriptor->AddrSpaceGranularity, Descriptor->SpecificFlag,
- ((Descriptor->SpecificFlag &
- EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE
- ) != 0) ? L" (Prefetchable)" : L""
- ));
- }
- }
- //
- // Skip the END descriptor for root bridge
- //
- ASSERT (Descriptor->Desc == ACPI_END_TAG_DESCRIPTOR);
- Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)(
- (EFI_ACPI_END_TAG_DESCRIPTOR *)Descriptor + 1
- );
- }
-}
--
2.28.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/4] ArmVirtPkg: Use extracted PciHostBridgeUtilityLib
2020-11-09 13:05 ` [PATCH v2 2/4] ArmVirtPkg: Use extracted PciHostBridgeUtilityLib Jiahui Cen
@ 2020-11-11 17:27 ` Laszlo Ersek
0 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2020-11-11 17:27 UTC (permalink / raw)
To: Jiahui Cen, devel
Cc: jordan.l.justen, ard.biesheuvel, leif, xieyingtai, miaoyubo
On 11/09/20 14:05, Jiahui Cen wrote:
> From: Yubo Miao <miaoyubo@huawei.com>
>
> Eliminate the currently duplicated code in ArmVirtPkg and use the
> extracted PciHostBridgeResourceConflict from PciHostBridgeUtilityLib.
(1) Please update the function name above, to
"PciHostBridgeUtilityResourceConflict".
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
> Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
> ---
> ArmVirtPkg/ArmVirt.dsc.inc | 1 +
> ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf | 2 +
> ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 61 +-------------------
> 3 files changed, 4 insertions(+), 60 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 4dafd1fa0f1d..593a523171ff 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -144,6 +144,7 @@ [LibraryClasses.common]
> PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
> PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
> PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> + PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>
> # USB Libraries
> UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
(2) Please place this lib class resolution in the following files,
instead:
- ArmVirtPkg/ArmVirtKvmTool.dsc
- ArmVirtPkg/ArmVirtQemu.dsc
- ArmVirtPkg/ArmVirtQemuKernel.dsc
just below where each one of those files resolves "PciHostBridgeLib" to
"ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf".
("ArmVirtXen.dsc" consumes "ArmVirt.dsc.inc" but does not use
"FdtPciHostBridgeLib.inf", therefore it does not need to inherit a
PciHostBridgeUtilityLib resolution from "ArmVirt.dsc.inc".)
> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> index 277ccfd24546..97e9368c8e9f 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> @@ -31,6 +31,7 @@ [Packages]
> ArmVirtPkg/ArmVirtPkg.dec
> MdeModulePkg/MdeModulePkg.dec
> MdePkg/MdePkg.dec
> + OvmfPkg/OvmfPkg.dec
>
> [LibraryClasses]
> DebugLib
> @@ -38,6 +39,7 @@ [LibraryClasses]
> DxeServicesTableLib
> MemoryAllocationLib
> PciPcdProducerLib
> + PciHostBridgeUtilityLib
>
> [FixedPcd]
> gArmTokenSpaceGuid.PcdPciMmio32Translation
(3) Please keep the entries under [LibraryClasses] sorted.
> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> index 496b192d2291..3952f511b4d2 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> @@ -14,6 +14,7 @@
> #include <Library/MemoryAllocationLib.h>
> #include <Library/PcdLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> +#include <Library/PciHostBridgeUtilityLib.h>
>
> #include <Protocol/FdtClient.h>
> #include <Protocol/PciRootBridgeIo.h>
(4) Okay, I'm going to cheat a bit here... Because the #include list is
not sorted in the first place, unfortunately. So please do this:
> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> index 496b192d2291..1c0321ce404b 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> @@ -7,12 +7,13 @@
>
> **/
> #include <PiDxe.h>
> -#include <Library/PciHostBridgeLib.h>
> #include <Library/DebugLib.h>
> #include <Library/DevicePathLib.h>
> #include <Library/DxeServicesTableLib.h>
> #include <Library/MemoryAllocationLib.h>
> #include <Library/PcdLib.h>
> +#include <Library/PciHostBridgeLib.h>
> +#include <Library/PciHostBridgeUtilityLib.h>
> #include <Library/UefiBootServicesTableLib.h>
>
> #include <Protocol/FdtClient.h>
Normally we should keep such unrelated changes separate, but in itself,
it's moving just one line, so I prefer sneaking it into this patch.
Back to your patch:
On 11/09/20 14:05, Jiahui Cen wrote:
> @@ -51,9 +52,6 @@ STATIC EFI_PCI_ROOT_BRIDGE_DEVICE_PATH mEfiPciRootBridgeDevicePath = {
> };
>
> GLOBAL_REMOVE_IF_UNREFERENCED
> -CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = {
> - L"Mem", L"I/O", L"Bus"
> -};
>
> //
> // We expect the "ranges" property of "pci-host-ecam-generic" to consist of
OK.
> @@ -414,60 +412,3 @@ PciHostBridgeFreeRootBridges (
> ASSERT (Count == 1);
> }
>
> -/**
> - Inform the platform that the resource conflict happens.
> -
> - @param HostBridgeHandle Handle of the Host Bridge.
> - @param Configuration Pointer to PCI I/O and PCI memory resource
> - descriptors. The Configuration contains the resources
> - for all the root bridges. The resource for each root
> - bridge is terminated with END descriptor and an
> - additional END is appended indicating the end of the
> - entire resources. The resource descriptor field
> - values follow the description in
> - EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL
> - .SubmitResources().
> -**/
> -VOID
> -EFIAPI
> -PciHostBridgeResourceConflict (
> - EFI_HANDLE HostBridgeHandle,
> - VOID *Configuration
> - )
> -{
> - EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
> - UINTN RootBridgeIndex;
> - DEBUG ((EFI_D_ERROR, "PciHostBridge: Resource conflict happens!\n"));
> -
> - RootBridgeIndex = 0;
> - Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *) Configuration;
> - while (Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR) {
> - DEBUG ((EFI_D_ERROR, "RootBridge[%d]:\n", RootBridgeIndex++));
> - for (; Descriptor->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR; Descriptor++) {
> - ASSERT (Descriptor->ResType <
> - (sizeof (mPciHostBridgeLibAcpiAddressSpaceTypeStr) /
> - sizeof (mPciHostBridgeLibAcpiAddressSpaceTypeStr[0])
> - )
> - );
> - DEBUG ((EFI_D_ERROR, " %s: Length/Alignment = 0x%lx / 0x%lx\n",
> - mPciHostBridgeLibAcpiAddressSpaceTypeStr[Descriptor->ResType],
> - Descriptor->AddrLen, Descriptor->AddrRangeMax
> - ));
> - if (Descriptor->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
> - DEBUG ((EFI_D_ERROR, " Granularity/SpecificFlag = %ld / %02x%s\n",
> - Descriptor->AddrSpaceGranularity, Descriptor->SpecificFlag,
> - ((Descriptor->SpecificFlag &
> - EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE
> - ) != 0) ? L" (Prefetchable)" : L""
> - ));
> - }
> - }
> - //
> - // Skip the END descriptor for root bridge
> - //
> - ASSERT (Descriptor->Desc == ACPI_END_TAG_DESCRIPTOR);
> - Descriptor = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)(
> - (EFI_ACPI_END_TAG_DESCRIPTOR *)Descriptor + 1
> - );
> - }
> -}
>
(5) Please replace only the body of the function, with a call to
PciHostBridgeUtilityResourceConflict().
I'm finishing the review of v2 at this point -- I'd like to ask you to
address the requests I made thus far, and post v3:
- I've asked for trimming the Library #include lists in C files, and the
[LibraryClasses] sections in INF files, to the required minimums.
This means that some dependencies will move from patch#1 to patch#3,
and I wouldn't like to re-review those parts in patch#3.
- Some points I've made thus far apply to the rest of the series, and I
would like you to ensure them globally, before I first look at the
rest of the patches. In particular:
- Keep #include lists and [LibraryClasses] sections (and almost all
other INF file sections) sorted.
- Keep #include lists and [LibraryClasses] sections (and other INF
file sections) minimal.
This applies to such modules too that you move code *from* --
removing code may mean that you no longer need various #includes and
lib classes in the original module!
- Functions declared in the PciHostBridgeUtilityLib class header
should be called PciHostBridgeUtility*.
Among other things this means that you can't / shouldn't move
PciHostBridgeLib function imlementations whole-sale; you should only
move their bodies.
It also implies the renaming of functions that are currently
internal to OVMF's PciHostBridgeLib instance.
- The tree should build at every stage throughout the series.
In general, please go through my points under patch #1, and see if / how
each applies to patches #3 and #4.
(6) Finally: it seems you have most of the necessary git settings
implemented in your edk2 clone.
Can you please check if you have "sendemail.transferEncoding=8bit" as
well?
(You might have to pass "--transfer-encoding=8bit" to git-send-email
manually.)
Beyond "8bit", "base64" is also an option; just avoid the default
"quoted-printable", please.
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-11 17:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-07 7:40 [PATCH v2 0/4] Add extra pci roots support for Arm Jiahui Cen
2020-11-07 7:40 ` [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib cenjiahui
2020-11-07 7:40 ` [PATCH v2 2/4] ArmVirtPkg: Use extracted PciHostBridgeUtilityLib Jiahui Cen
2020-11-07 7:40 ` [PATCH v2 3/4] OvmfPkg: Extract functions of extra pci roots Jiahui Cen
2020-11-07 7:40 ` [PATCH v2 4/4] ArmVirtPkg: Support " Jiahui Cen
-- strict thread matches above, loose matches on Subject: below --
2020-11-09 13:05 [PATCH v2 0/4] Add extra pci roots support for Arm Jiahui Cen
2020-11-09 13:05 ` [PATCH v2 2/4] ArmVirtPkg: Use extracted PciHostBridgeUtilityLib Jiahui Cen
2020-11-11 17:27 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox