1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v3 0/5] Add extra pci roots support for Arm
  4. View patch

[edk2-devel] [PATCH v3 2/5] ArmVirtPkg: Refactor with PciHostBridgeUtilityLib

Jiahui Cen via groups.io posted 5 patches 5 years, 1 month ago
There is a newer version of this series
[edk2-devel] [PATCH v3 2/5] ArmVirtPkg: Refactor with PciHostBridgeUtilityLib
Posted by Jiahui Cen via groups.io 5 years, 1 month ago 
Eliminate currently duplicated code in ArmVirtPkg with the common utility
class PciHostBridgeUtilityLib.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3059

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
---
 ArmVirtPkg/ArmVirt.dsc.inc                                     |  1 +
 ArmVirtPkg/ArmVirtKvmTool.dsc                                  |  1 +
 ArmVirtPkg/ArmVirtQemu.dsc                                     |  1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc                               |  1 +
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf |  2 +
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c   | 44 ++------------------
 6 files changed, 9 insertions(+), 41 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 9ec92930472d..b9a0cd362416 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -145,6 +145,7 @@ [LibraryClasses.common]
   PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
   PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
   PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
+  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
 
   # USB Libraries
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
index bf008be50fcb..f817132ba7b0 100644
--- a/ArmVirtPkg/ArmVirtKvmTool.dsc
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -65,6 +65,7 @@ [LibraryClasses.common]
   PlatformPeiLib|ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf
 
   PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
+  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
   PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
   SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
 
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index ef5d6dbeaddc..a11ffd9ba553 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -78,6 +78,7 @@ [LibraryClasses.common]
   PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
   PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
   PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
 
 !if $(TPM2_ENABLE) == TRUE
   Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index f8f5f7f4b94b..c27752b4d5e5 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -76,6 +76,7 @@ [LibraryClasses.common]
   PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
   PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
   PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
   TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
 
 [LibraryClasses.common.DXE_DRIVER]
diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
index 277ccfd24546..01d39626d14c 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
@@ -31,12 +31,14 @@ [Packages]
   ArmVirtPkg/ArmVirtPkg.dec
   MdeModulePkg/MdeModulePkg.dec
   MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
   DebugLib
   DevicePathLib
   DxeServicesTableLib
   MemoryAllocationLib
+  PciHostBridgeUtilityLib
   PciPcdProducerLib
 
 [FixedPcd]
diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index 496b192d2291..d554479bf0de 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -7,12 +7,13 @@
 
 **/
 #include <PiDxe.h>
-#include <Library/PciHostBridgeLib.h>
 #include <Library/DebugLib.h>
 #include <Library/DevicePathLib.h>
 #include <Library/DxeServicesTableLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
+#include <Library/PciHostBridgeLib.h>
+#include <Library/PciHostBridgeUtilityLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 
 #include <Protocol/FdtClient.h>
@@ -50,11 +51,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
 // records like this.
@@ -435,39 +431,5 @@ PciHostBridgeResourceConflict (
   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
-                   );
-  }
+  PciHostBridgeUtilityResourceConflict (Configuration);
 }
-- 
2.28.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69380): https://edk2.groups.io/g/devel/message/69380
Mute This Topic: https://groups.io/mt/79146007/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 2/5] ArmVirtPkg: Refactor with PciHostBridgeUtilityLib
Posted by Laszlo Ersek 5 years, 1 month ago 
On 12/22/20 10:59, Jiahui Cen via groups.io wrote:
> Eliminate currently duplicated code in ArmVirtPkg with the common utility
> class PciHostBridgeUtilityLib.
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3059
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
> Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc                                     |  1 +
>  ArmVirtPkg/ArmVirtKvmTool.dsc                                  |  1 +
>  ArmVirtPkg/ArmVirtQemu.dsc                                     |  1 +
>  ArmVirtPkg/ArmVirtQemuKernel.dsc                               |  1 +
>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf |  2 +
>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c   | 44 ++------------------
>  6 files changed, 9 insertions(+), 41 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 9ec92930472d..b9a0cd362416 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -145,6 +145,7 @@ [LibraryClasses.common]
>    PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>    PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>    PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
> +  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>  
>    # USB Libraries
>    UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
> index bf008be50fcb..f817132ba7b0 100644
> --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
> @@ -65,6 +65,7 @@ [LibraryClasses.common]
>    PlatformPeiLib|ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf
>  
>    PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
> +  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>    PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
>    SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
>  
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index ef5d6dbeaddc..a11ffd9ba553 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -78,6 +78,7 @@ [LibraryClasses.common]
>    PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
>    PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
>    PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> +  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>  
>  !if $(TPM2_ENABLE) == TRUE
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index f8f5f7f4b94b..c27752b4d5e5 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -76,6 +76,7 @@ [LibraryClasses.common]
>    PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
>    PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
>    PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> +  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>    TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
>  
>  [LibraryClasses.common.DXE_DRIVER]

Please pay more attention to review feedback. In v2, I requested two
things, and those have not been resolved precisely:

(1) The files

- ArmVirtPkg/ArmVirtKvmTool.dsc
- ArmVirtPkg/ArmVirtQemu.dsc
- ArmVirtPkg/ArmVirtQemuKernel.dsc

should be updated *instead of* "ArmVirtPkg/ArmVirt.dsc.inc" -- not "in
addition to" the latter.

So please drop the "ArmVirtPkg/ArmVirt.dsc.inc" hunk.

(2) In "ArmVirtPkg/ArmVirtKvmTool.dsc", you didn't place the
PciHostBridgeUtilityLib class resolution right after the
PciHostBridgeLib one.

So please move the new lib class resolution into said (more intuitive) spot.


With the above updates, the patch is going to be OK.

Thanks,
Laszlo

> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> index 277ccfd24546..01d39626d14c 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> @@ -31,12 +31,14 @@ [Packages]
>    ArmVirtPkg/ArmVirtPkg.dec
>    MdeModulePkg/MdeModulePkg.dec
>    MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
>  
>  [LibraryClasses]
>    DebugLib
>    DevicePathLib
>    DxeServicesTableLib
>    MemoryAllocationLib
> +  PciHostBridgeUtilityLib
>    PciPcdProducerLib
>  
>  [FixedPcd]
> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> index 496b192d2291..d554479bf0de 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> @@ -7,12 +7,13 @@
>  
>  **/
>  #include <PiDxe.h>
> -#include <Library/PciHostBridgeLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/DevicePathLib.h>
>  #include <Library/DxeServicesTableLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/PcdLib.h>
> +#include <Library/PciHostBridgeLib.h>
> +#include <Library/PciHostBridgeUtilityLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  
>  #include <Protocol/FdtClient.h>
> @@ -50,11 +51,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
>  // records like this.
> @@ -435,39 +431,5 @@ PciHostBridgeResourceConflict (
>    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
> -                   );
> -  }
> +  PciHostBridgeUtilityResourceConflict (Configuration);
>  }
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69801): https://edk2.groups.io/g/devel/message/69801
Mute This Topic: https://groups.io/mt/79146007/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3 2/5] ArmVirtPkg: Refactor with PciHostBridgeUtilityLib
Posted by Jiahui Cen via groups.io 5 years, 1 month ago 
Hi Laszlo,

On 2021/1/6 17:12, Laszlo Ersek wrote:
> On 12/22/20 10:59, Jiahui Cen via groups.io wrote:
>> Eliminate currently duplicated code in ArmVirtPkg with the common utility
>> class PciHostBridgeUtilityLib.
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3059
>>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>> Signed-off-by: Jiahui Cen <cenjiahui@huawei.com>
>> Signed-off-by: Yubo Miao <miaoyubo@huawei.com>
>> ---
>>  ArmVirtPkg/ArmVirt.dsc.inc                                     |  1 +
>>  ArmVirtPkg/ArmVirtKvmTool.dsc                                  |  1 +
>>  ArmVirtPkg/ArmVirtQemu.dsc                                     |  1 +
>>  ArmVirtPkg/ArmVirtQemuKernel.dsc                               |  1 +
>>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf |  2 +
>>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c   | 44 ++------------------
>>  6 files changed, 9 insertions(+), 41 deletions(-)
>>
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index 9ec92930472d..b9a0cd362416 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -145,6 +145,7 @@ [LibraryClasses.common]
>>    PciCapLib|OvmfPkg/Library/BasePciCapLib/BasePciCapLib.inf
>>    PciCapPciSegmentLib|OvmfPkg/Library/BasePciCapPciSegmentLib/BasePciCapPciSegmentLib.inf
>>    PciCapPciIoLib|OvmfPkg/Library/UefiPciCapPciIoLib/UefiPciCapPciIoLib.inf
>> +  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>>  
>>    # USB Libraries
>>    UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
>> diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
>> index bf008be50fcb..f817132ba7b0 100644
>> --- a/ArmVirtPkg/ArmVirtKvmTool.dsc
>> +++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
>> @@ -65,6 +65,7 @@ [LibraryClasses.common]
>>    PlatformPeiLib|ArmVirtPkg/Library/KvmtoolPlatformPeiLib/KvmtoolPlatformPeiLib.inf
>>  
>>    PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
>> +  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>>    PlatformHookLib|ArmVirtPkg/Library/Fdt16550SerialPortHookLib/Fdt16550SerialPortHookLib.inf
>>    SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
>>  
>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
>> index ef5d6dbeaddc..a11ffd9ba553 100644
>> --- a/ArmVirtPkg/ArmVirtQemu.dsc
>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
>> @@ -78,6 +78,7 @@ [LibraryClasses.common]
>>    PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
>>    PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
>>    PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
>> +  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>>  
>>  !if $(TPM2_ENABLE) == TRUE
>>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
>> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> index f8f5f7f4b94b..c27752b4d5e5 100644
>> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>> @@ -76,6 +76,7 @@ [LibraryClasses.common]
>>    PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
>>    PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
>>    PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
>> +  PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
>>    TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
>>  
>>  [LibraryClasses.common.DXE_DRIVER]
> 
> Please pay more attention to review feedback. In v2, I requested two
> things, and those have not been resolved precisely:
> 
> (1) The files
> 
> - ArmVirtPkg/ArmVirtKvmTool.dsc
> - ArmVirtPkg/ArmVirtQemu.dsc
> - ArmVirtPkg/ArmVirtQemuKernel.dsc
> 
> should be updated *instead of* "ArmVirtPkg/ArmVirt.dsc.inc" -- not "in
> addition to" the latter.
> 
> So please drop the "ArmVirtPkg/ArmVirt.dsc.inc" hunk.
> 
> (2) In "ArmVirtPkg/ArmVirtKvmTool.dsc", you didn't place the
> PciHostBridgeUtilityLib class resolution right after the
> PciHostBridgeLib one.
> 
> So please move the new lib class resolution into said (more intuitive) spot.
> 

Sorry for missing these review points. Will fix them in v4.

Thanks,
Jiahui

> 
> With the above updates, the patch is going to be OK.
> 
> Thanks,
> Laszlo
> 
>> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
>> index 277ccfd24546..01d39626d14c 100644
>> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
>> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
>> @@ -31,12 +31,14 @@ [Packages]
>>    ArmVirtPkg/ArmVirtPkg.dec
>>    MdeModulePkg/MdeModulePkg.dec
>>    MdePkg/MdePkg.dec
>> +  OvmfPkg/OvmfPkg.dec
>>  
>>  [LibraryClasses]
>>    DebugLib
>>    DevicePathLib
>>    DxeServicesTableLib
>>    MemoryAllocationLib
>> +  PciHostBridgeUtilityLib
>>    PciPcdProducerLib
>>  
>>  [FixedPcd]
>> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>> index 496b192d2291..d554479bf0de 100644
>> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>> @@ -7,12 +7,13 @@
>>  
>>  **/
>>  #include <PiDxe.h>
>> -#include <Library/PciHostBridgeLib.h>
>>  #include <Library/DebugLib.h>
>>  #include <Library/DevicePathLib.h>
>>  #include <Library/DxeServicesTableLib.h>
>>  #include <Library/MemoryAllocationLib.h>
>>  #include <Library/PcdLib.h>
>> +#include <Library/PciHostBridgeLib.h>
>> +#include <Library/PciHostBridgeUtilityLib.h>
>>  #include <Library/UefiBootServicesTableLib.h>
>>  
>>  #include <Protocol/FdtClient.h>
>> @@ -50,11 +51,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
>>  // records like this.
>> @@ -435,39 +431,5 @@ PciHostBridgeResourceConflict (
>>    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
>> -                   );
>> -  }
>> +  PciHostBridgeUtilityResourceConflict (Configuration);
>>  }
>>
> 
> .
> 


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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.