* [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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
* [PATCH v2 0/4] Add extra pci roots support for Arm @ 2020-11-09 13:05 Jiahui Cen 2020-11-09 13:05 ` [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib Jiahui Cen 0 siblings, 1 reply; 8+ 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 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. 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> Signed-off-by: Yubo Miao <miaoyubo@huawei.com> Signed-off-by: Jiahui Cen <cenjiahui@huawei.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 + OvmfPkg/OvmfPkgIa32.dsc | 1 + OvmfPkg/OvmfPkgIa32X64.dsc | 1 + OvmfPkg/OvmfPkgX64.dsc | 1 + OvmfPkg/OvmfXen.dsc | 1 + ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf | 5 + OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf | 1 + OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf | 51 ++++ OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h | 98 +++++++ ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 221 ++++++++------- OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c | 234 +--------------- OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c | 283 ++++++++++++++++++++ 12 files changed, 563 insertions(+), 335 deletions(-) create mode 100644 OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf create mode 100644 OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h create mode 100644 OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c -- 2.28.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib 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 16:45 ` Laszlo Ersek 0 siblings, 1 reply; 8+ 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> Introduce a new PciHostBridgeUtilityLib class to share duplicate code between OvmfPkg and ArmVirtPkg. Extract function PciHostBridgeResourceConflict from OvmfPkg/PciHostBridgeLib. Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> Signed-off-by: Yubo Miao <miaoyubo@huawei.com> Signed-off-by: Jiahui Cen <cenjiahui@huawei.com> --- OvmfPkg/OvmfPkgIa32.dsc | 1 + OvmfPkg/OvmfPkgIa32X64.dsc | 1 + OvmfPkg/OvmfPkgX64.dsc | 1 + OvmfPkg/OvmfXen.dsc | 1 + OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf | 1 + OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf | 49 +++++++++++++ OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h | 33 +++++++++ OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c | 64 +---------------- OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c | 74 ++++++++++++++++++++ 9 files changed, 162 insertions(+), 63 deletions(-) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index 58d9f292f9ac..0c2bf0b13c34 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -738,6 +738,7 @@ [Components] <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 3551f9710a6c..baf36a4f7a54 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -752,6 +752,7 @@ [Components.X64] <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 7a8bdb8a8697..219b5f559b53 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -748,6 +748,7 @@ [Components] <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 34c9de19dfba..442c0730ef32 100644 --- a/OvmfPkg/OvmfXen.dsc +++ b/OvmfPkg/OvmfXen.dsc @@ -544,6 +544,7 @@ [Components] <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/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf index 6ec9ec751478..7d01528c94f1 100644 --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf @@ -41,6 +41,7 @@ [LibraryClasses] MemoryAllocationLib PciLib QemuFwCfgLib + PciHostBridgeUtilityLib [Pcd] gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf new file mode 100644 index 000000000000..c88ab8e4155d --- /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/Include/Library/PciHostBridgeUtilityLib.h b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h new file mode 100644 index 000000000000..d2622fd907e6 --- /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 e850f7d183ee..1c8f465834f3 100644 --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c @@ -22,6 +22,7 @@ #include <Library/PciHostBridgeLib.h> #include <Library/PciLib.h> #include <Library/QemuFwCfgLib.h> +#include <Library/PciHostBridgeUtilityLib.h> #include "PciHostBridge.h" @@ -33,12 +34,6 @@ typedef struct { #pragma pack () -GLOBAL_REMOVE_IF_UNREFERENCED -CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = { - L"Mem", L"I/O", L"Bus" -}; - - STATIC CONST OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH mRootBridgeDevicePathTemplate = { @@ -384,60 +379,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/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c new file mode 100644 index 000000000000..7e9512dc08f1 --- /dev/null +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c @@ -0,0 +1,74 @@ +/** @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> + + +GLOBAL_REMOVE_IF_UNREFERENCED +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 + ); + } +} + -- 2.28.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib 2020-11-09 13:05 ` [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib Jiahui Cen @ 2020-11-11 16:45 ` Laszlo Ersek 2020-11-12 3:21 ` Jiahui Cen 0 siblings, 1 reply; 8+ messages in thread From: Laszlo Ersek @ 2020-11-11 16:45 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> > > Introduce a new PciHostBridgeUtilityLib class to share duplicate code > between OvmfPkg and ArmVirtPkg. > > Extract function PciHostBridgeResourceConflict from > OvmfPkg/PciHostBridgeLib. > > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> > Signed-off-by: Yubo Miao <miaoyubo@huawei.com> > Signed-off-by: Jiahui Cen <cenjiahui@huawei.com> > --- > OvmfPkg/OvmfPkgIa32.dsc | 1 + > OvmfPkg/OvmfPkgIa32X64.dsc | 1 + > OvmfPkg/OvmfPkgX64.dsc | 1 + > OvmfPkg/OvmfXen.dsc | 1 + > OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf | 1 + > OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf | 49 +++++++++++++ > OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h | 33 +++++++++ > OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c | 64 +---------------- > OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c | 74 ++++++++++++++++++++ > 9 files changed, 162 insertions(+), 63 deletions(-) (1) There is a typo in the subject: s/form/from/ (2) "OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf" is consumed in the following platform DSC file as well -- please update it too: OvmfPkg/Bhyve/BhyveX64.dsc (This will affect the list of necessary CC's on this patch.) (3) In my v1 review at <https://edk2.groups.io/g/devel/message/57062> (msgid <57fd0043-d63a-55b0-9c55-4ca079331885@redhat.com>), I mentioned "OvmfPkg/OvmfPkg.dec". You forgot to extend that file, with the new library class. Please add the following hunk to this patch: > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec > index 118c968fda4e..b89252eeab2a 100644 > --- a/OvmfPkg/OvmfPkg.dec > +++ b/OvmfPkg/OvmfPkg.dec > @@ -49,6 +49,10 @@ [LibraryClasses] > # access. > PciCapPciSegmentLib|Include/Library/PciCapPciSegmentLib.h > > + ## @libraryclass Provide common utility functions to PciHostBridgeLib > + # instances in ArmVirtPkg and OvmfPkg. > + PciHostBridgeUtilityLib|Include/Library/PciHostBridgeUtilityLib.h > + > ## @libraryclass Register a status code handler for printing the Boot > # Manager's LoadImage() and StartImage() preparations, and > # return codes, to the UEFI console. (4) Issue (3) above would have been caught by CI, namely by the "LibraryClassCheck" plugin: https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md#library-declaration-test---libraryclasscheck In order to put your patches through CI before posting, I suggest pushing them to a branch in your edk2 clone on github.com, and submitting a pull request against the central repository. The pull request will be auto-closed in the end, unconditionally, but it will give you CI results. (You can also run CI locally, but setting that up is laborious.) Back to your patch: On 11/09/20 14:05, Jiahui Cen wrote: > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 58d9f292f9ac..0c2bf0b13c34 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -738,6 +738,7 @@ [Components] > <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 3551f9710a6c..baf36a4f7a54 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -752,6 +752,7 @@ [Components.X64] > <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 7a8bdb8a8697..219b5f559b53 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -748,6 +748,7 @@ [Components] > <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 34c9de19dfba..442c0730ef32 100644 > --- a/OvmfPkg/OvmfXen.dsc > +++ b/OvmfPkg/OvmfXen.dsc > @@ -544,6 +544,7 @@ [Components] > <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> (5) Please keep the PciHostBridgeUtilityLib class resolution right next to the PciHostBridgeLib one. > diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf > index 6ec9ec751478..7d01528c94f1 100644 > --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf > +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf > @@ -41,6 +41,7 @@ [LibraryClasses] > MemoryAllocationLib > PciLib > QemuFwCfgLib > + PciHostBridgeUtilityLib > > [Pcd] > gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase (6) Please keep the entries under [LibraryClasses] alphabetically sorted. > diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf > new file mode 100644 > index 000000000000..c88ab8e4155d > --- /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. (7) I suggest the following description: Provide common utility functions to PciHostBridgeLib instances in ArmVirtPkg and OvmfPkg. > +# > +# Copyright (C) 2016, Red Hat, Inc. > +# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR> (8) It makes sense to duplicate the (C) notices from the original file (namely "OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf"), but this is still a new file due to your contribution. Therefore, please add a proper Huawei copyright notice. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 (9) The latest specified INF version is 1.29, according to <https://edk2-docs.gitbook.io/edk-ii-inf-specification/#edk-ii-module-information-inf-file-specification>; please write INF_VERSION = 1.29 > + 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 > +# > + (10) EBC is gone, plus we'll want to plug this lib into "ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf", which itself names AARCH64 and ARM for VALID_ARCHITECTURES. Therefore, please state "IA32 X64 AARCH64 ARM" here. > +[Sources] > + PciHostBridgeUtilityLib.c > + > +[Packages] > + MdePkg/MdePkg.dec > + OvmfPkg/OvmfPkg.dec > + > +[LibraryClasses] > + BaseMemoryLib > + DebugLib > + DevicePathLib > + MemoryAllocationLib > + PciLib > + QemuFwCfgLib (11) The list of required (consumed) library classes, along with the library class header files included in "PciHostBridgeUtilityLib.c", should be as minimal as possible (and alphabetically sorted). Specifically, you only need to keep "DebugLib"; everything else should be removed, as far as I can tell. (We can extend the [LibraryClasses] section in later patches, if necessary.) > + > +[Pcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase > + gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize > + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base > + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size > + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base > + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId (12) The [Pcd] section should be dropped altogether. > diff --git a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h > new file mode 100644 > index 000000000000..d2622fd907e6 > --- /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. (13) This file-top comment should say: Provide common utility functions to PciHostBridgeLib instances in ArmVirtPkg and OvmfPkg. > + > + Copyright (c) 2016, Intel Corporation. All rights reserved.<BR> (14) It's fine to keep the (C) notices from the file that you are using as template, but you should please add a Huawei (C) notice as well. > + 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 > + ); > + (15) Please rename this function to "PciHostBridgeUtilityResourceConflict". The idea is that no PciHostBridgeUtilityLib instance should directly implement a PciHostBridgeLib API. PciHostBridgeUtilityLib can offer APIs that are as "fat" as necessary, taking over as much work as possible, but we should keep the namespaces isolated. (16) In our helper library, the "HostBridgeHandle" parameter is not used; please remove it from both the leading comment on the function, and also from the function declaration. > +#endif (17) Please append a comment to this line: // __PCI_HOST_BRIDGE_UTILITY_LIB_H__ > diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c > index e850f7d183ee..1c8f465834f3 100644 > --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c > +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c > @@ -22,6 +22,7 @@ > #include <Library/PciHostBridgeLib.h> > #include <Library/PciLib.h> > #include <Library/QemuFwCfgLib.h> > +#include <Library/PciHostBridgeUtilityLib.h> > #include "PciHostBridge.h" > > (18) Please keep the #include list of the Library class headers alphabetically sorted. > @@ -33,12 +34,6 @@ typedef struct { > #pragma pack () > > > -GLOBAL_REMOVE_IF_UNREFERENCED > -CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = { > - L"Mem", L"I/O", L"Bus" > -}; > - > - > STATIC > CONST > OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH mRootBridgeDevicePathTemplate = { > @@ -384,60 +379,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 > - ); > - } > -} (19) Please do not remove the entire definition of this function. According to my point (15), only replace the body of this function with the following function call: PciHostBridgeUtilityResourceConflict (Configuration); > diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c > new file mode 100644 > index 000000000000..7e9512dc08f1 > --- /dev/null > +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c > @@ -0,0 +1,74 @@ > +/** @file > + OVMF's instance of the PCI Host Bridge Library. > + > + Copyright (c) 2020, Huawei Corporation. All rights reserved.<BR> (20) Please preserve the (C) notices from the original file (from where the code is coming); namely "OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c". > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > +#include <Library/DebugLib.h> > +#include <Library/PciHostBridgeUtilityLib.h> (21) Is this #include list really sufficient to compile the tree when only this patch is applied? I would think the following directive is needed, additionally: #include <IndustryStandard/Acpi10.h> (I suggest placing it above the Library #includes.) > + > + > +GLOBAL_REMOVE_IF_UNREFERENCED > +CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = { > + L"Mem", L"I/O", L"Bus" > +}; > + > + (22) Please rename this object to "mPciHostBridgeUtilityLibAcpiAddressSpaceTypeStr" (you'll have to modify the reference to it below as well). > +/** > + 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 ( (23) Please rename to "PciHostBridgeUtilityResourceConflict". (24) Please drop "HostBridgeHandle", corresponding to point (16). > + 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 > + ); > + } > +} > + > Thanks! Laszlo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/4] OvmfPkg: Extract functions form PciHostBridgeLib 2020-11-11 16:45 ` Laszlo Ersek @ 2020-11-12 3:21 ` Jiahui Cen 0 siblings, 0 replies; 8+ messages in thread From: Jiahui Cen @ 2020-11-12 3:21 UTC (permalink / raw) To: Laszlo Ersek, devel Cc: jordan.l.justen, ard.biesheuvel, leif, xieyingtai, miaoyubo Hi Laszlo, Thanks for your detailed review. Will fix them in v3. Jiahui On 2020/11/12 0:45, Laszlo Ersek wrote: > On 11/09/20 14:05, Jiahui Cen wrote: >> 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. >> >> Cc: Jordan Justen <jordan.l.justen@intel.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> >> Signed-off-by: Yubo Miao <miaoyubo@huawei.com> >> Signed-off-by: Jiahui Cen <cenjiahui@huawei.com> >> --- >> OvmfPkg/OvmfPkgIa32.dsc | 1 + >> OvmfPkg/OvmfPkgIa32X64.dsc | 1 + >> OvmfPkg/OvmfPkgX64.dsc | 1 + >> OvmfPkg/OvmfXen.dsc | 1 + >> OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf | 1 + >> OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf | 49 +++++++++++++ >> OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h | 33 +++++++++ >> OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c | 64 +---------------- >> OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c | 74 ++++++++++++++++++++ >> 9 files changed, 162 insertions(+), 63 deletions(-) > > (1) There is a typo in the subject: s/form/from/ > > > (2) "OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf" is consumed > in the following platform DSC file as well -- please update it too: > > OvmfPkg/Bhyve/BhyveX64.dsc > > (This will affect the list of necessary CC's on this patch.) > > > (3) In my v1 review at <https://edk2.groups.io/g/devel/message/57062> > (msgid <57fd0043-d63a-55b0-9c55-4ca079331885@redhat.com>), I mentioned > "OvmfPkg/OvmfPkg.dec". > > You forgot to extend that file, with the new library class. Please add > the following hunk to this patch: > >> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec >> index 118c968fda4e..b89252eeab2a 100644 >> --- a/OvmfPkg/OvmfPkg.dec >> +++ b/OvmfPkg/OvmfPkg.dec >> @@ -49,6 +49,10 @@ [LibraryClasses] >> # access. >> PciCapPciSegmentLib|Include/Library/PciCapPciSegmentLib.h >> >> + ## @libraryclass Provide common utility functions to PciHostBridgeLib >> + # instances in ArmVirtPkg and OvmfPkg. >> + PciHostBridgeUtilityLib|Include/Library/PciHostBridgeUtilityLib.h >> + >> ## @libraryclass Register a status code handler for printing the Boot >> # Manager's LoadImage() and StartImage() preparations, and >> # return codes, to the UEFI console. > > > (4) Issue (3) above would have been caught by CI, namely by the > "LibraryClassCheck" plugin: > > https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md#library-declaration-test---libraryclasscheck > > In order to put your patches through CI before posting, I suggest > pushing them to a branch in your edk2 clone on github.com, and > submitting a pull request against the central repository. The > pull request will be auto-closed in the end, unconditionally, but it > will give you CI results. > > (You can also run CI locally, but setting that up is laborious.) > > Back to your patch: > > > On 11/09/20 14:05, Jiahui Cen wrote: >> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >> index 58d9f292f9ac..0c2bf0b13c34 100644 >> --- a/OvmfPkg/OvmfPkgIa32.dsc >> +++ b/OvmfPkg/OvmfPkgIa32.dsc >> @@ -738,6 +738,7 @@ [Components] >> <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 3551f9710a6c..baf36a4f7a54 100644 >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >> @@ -752,6 +752,7 @@ [Components.X64] >> <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 7a8bdb8a8697..219b5f559b53 100644 >> --- a/OvmfPkg/OvmfPkgX64.dsc >> +++ b/OvmfPkg/OvmfPkgX64.dsc >> @@ -748,6 +748,7 @@ [Components] >> <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 34c9de19dfba..442c0730ef32 100644 >> --- a/OvmfPkg/OvmfXen.dsc >> +++ b/OvmfPkg/OvmfXen.dsc >> @@ -544,6 +544,7 @@ [Components] >> <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> > > (5) Please keep the PciHostBridgeUtilityLib class resolution right next > to the PciHostBridgeLib one. > > >> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> index 6ec9ec751478..7d01528c94f1 100644 >> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf >> @@ -41,6 +41,7 @@ [LibraryClasses] >> MemoryAllocationLib >> PciLib >> QemuFwCfgLib >> + PciHostBridgeUtilityLib >> >> [Pcd] >> gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase > > (6) Please keep the entries under [LibraryClasses] alphabetically > sorted. > > >> diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf >> new file mode 100644 >> index 000000000000..c88ab8e4155d >> --- /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. > > (7) I suggest the following description: > > Provide common utility functions to PciHostBridgeLib instances in > ArmVirtPkg and OvmfPkg. > > >> +# >> +# Copyright (C) 2016, Red Hat, Inc. >> +# Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR> > > (8) It makes sense to duplicate the (C) notices from the original file > (namely "OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf"), but > this is still a new file due to your contribution. > > Therefore, please add a proper Huawei copyright notice. > > >> +# >> +# SPDX-License-Identifier: BSD-2-Clause-Patent >> +# >> +# >> +## >> + >> +[Defines] >> + INF_VERSION = 0x00010005 > > (9) The latest specified INF version is 1.29, according to > <https://edk2-docs.gitbook.io/edk-ii-inf-specification/#edk-ii-module-information-inf-file-specification>; > please write > > INF_VERSION = 1.29 > > >> + 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 >> +# >> + > > (10) EBC is gone, plus we'll want to plug this lib into > "ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf", which > itself names AARCH64 and ARM for VALID_ARCHITECTURES. > > Therefore, please state "IA32 X64 AARCH64 ARM" here. > > >> +[Sources] >> + PciHostBridgeUtilityLib.c >> + >> +[Packages] >> + MdePkg/MdePkg.dec >> + OvmfPkg/OvmfPkg.dec >> + >> +[LibraryClasses] >> + BaseMemoryLib >> + DebugLib >> + DevicePathLib >> + MemoryAllocationLib >> + PciLib >> + QemuFwCfgLib > > (11) The list of required (consumed) library classes, along with the > library class header files included in "PciHostBridgeUtilityLib.c", > should be as minimal as possible (and alphabetically sorted). > > Specifically, you only need to keep "DebugLib"; everything else should > be removed, as far as I can tell. > > (We can extend the [LibraryClasses] section in later patches, if > necessary.) > > >> + >> +[Pcd] >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciIoBase >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciIoSize >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Base >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio32Size >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base >> + gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId > > (12) The [Pcd] section should be dropped altogether. > > >> diff --git a/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h b/OvmfPkg/Include/Library/PciHostBridgeUtilityLib.h >> new file mode 100644 >> index 000000000000..d2622fd907e6 >> --- /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. > > (13) This file-top comment should say: > > Provide common utility functions to PciHostBridgeLib instances in > ArmVirtPkg and OvmfPkg. > > >> + >> + Copyright (c) 2016, Intel Corporation. All rights reserved.<BR> > > (14) It's fine to keep the (C) notices from the file that you are using > as template, but you should please add a Huawei (C) notice as well. > > >> + 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 >> + ); >> + > > (15) Please rename this function to > "PciHostBridgeUtilityResourceConflict". > > The idea is that no PciHostBridgeUtilityLib instance should directly > implement a PciHostBridgeLib API. PciHostBridgeUtilityLib can offer APIs > that are as "fat" as necessary, taking over as much work as possible, > but we should keep the namespaces isolated. > > > (16) In our helper library, the "HostBridgeHandle" parameter is not > used; please remove it from both the leading comment on the function, > and also from the function declaration. > > >> +#endif > > (17) Please append a comment to this line: > > // __PCI_HOST_BRIDGE_UTILITY_LIB_H__ > > >> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c >> index e850f7d183ee..1c8f465834f3 100644 >> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c >> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c >> @@ -22,6 +22,7 @@ >> #include <Library/PciHostBridgeLib.h> >> #include <Library/PciLib.h> >> #include <Library/QemuFwCfgLib.h> >> +#include <Library/PciHostBridgeUtilityLib.h> >> #include "PciHostBridge.h" >> >> > > (18) Please keep the #include list of the Library class headers > alphabetically sorted. > > >> @@ -33,12 +34,6 @@ typedef struct { >> #pragma pack () >> >> >> -GLOBAL_REMOVE_IF_UNREFERENCED >> -CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = { >> - L"Mem", L"I/O", L"Bus" >> -}; >> - >> - >> STATIC >> CONST >> OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH mRootBridgeDevicePathTemplate = { >> @@ -384,60 +379,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 >> - ); >> - } >> -} > > (19) Please do not remove the entire definition of this function. > According to my point (15), only replace the body of this function with > the following function call: > > PciHostBridgeUtilityResourceConflict (Configuration); > > >> diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c >> new file mode 100644 >> index 000000000000..7e9512dc08f1 >> --- /dev/null >> +++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c >> @@ -0,0 +1,74 @@ >> +/** @file >> + OVMF's instance of the PCI Host Bridge Library. >> + >> + Copyright (c) 2020, Huawei Corporation. All rights reserved.<BR> > > (20) Please preserve the (C) notices from the original file (from where > the code is coming); namely > "OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c". > > >> + >> + SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +**/ >> +#include <Library/DebugLib.h> >> +#include <Library/PciHostBridgeUtilityLib.h> > > (21) Is this #include list really sufficient to compile the tree when > only this patch is applied? > > I would think the following directive is needed, additionally: > > #include <IndustryStandard/Acpi10.h> > > (I suggest placing it above the Library #includes.) > > >> + >> + >> +GLOBAL_REMOVE_IF_UNREFERENCED >> +CHAR16 *mPciHostBridgeLibAcpiAddressSpaceTypeStr[] = { >> + L"Mem", L"I/O", L"Bus" >> +}; >> + >> + > > (22) Please rename this object to > "mPciHostBridgeUtilityLibAcpiAddressSpaceTypeStr" (you'll have to modify > the reference to it below as well). > > >> +/** >> + 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 ( > > (23) Please rename to "PciHostBridgeUtilityResourceConflict". > > (24) Please drop "HostBridgeHandle", corresponding to point (16). > > >> + 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 >> + ); >> + } >> +} >> + >> > > Thanks! > Laszlo > > . > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-12 3:21 UTC | newest] Thread overview: 8+ 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 1/4] OvmfPkg: Extract functions form PciHostBridgeLib Jiahui Cen 2020-11-11 16:45 ` Laszlo Ersek 2020-11-12 3:21 ` Jiahui Cen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox