Re: [RFC 0/6] DxeBmDmaLib

[Laszlo Ersek <lersek@redhat.com>]

On 01/10/17 01:16, Leo Duran wrote:
> This patch-set provides an abstraction layer for DMA operations implemented
> by the PciHostBridgeDxe driver. The intent is to then allow override of this
> library as may be required by specific hardware implementations, such as 
> AMD's Secure Encrypted Virtualization (SEV).
> 
> This new (BmDmaLib class) library is modeled after the existing DmaLib
> plus an extra DmaAbove4GB (BOOLEAN) parameter in the Map and Allocate
> interfaces, so that decisions can be made about the need to allocate DMA
> buffers below the 4GB boundary.

* Ard, I remember you worked on >=4GB DMA stuff before, can you please
look over this series?

* Leo, can you confirm that patches #1 and #6 implement nothing more
than code movement? If they implement more than that (judging from the
new DmaAbove4GB parameter above), can you specify how exactly?

The source code changes in patches #2 and #5 look okay, but before I'm
comfortable acking them, I'd like to understand the scope of the code
movement between #1 and #6.

* Also, I suggest the following subject line template for patches #2
through #5:

  XxxPkg: resolve BmDmaLib class for PciHostBridgeDxe driver

* For patch #1, I suggest restricting the client module types to
DXE_DRIVER in the INF file:

  LIBRARY_CLASS                  = BmDmaLib|DXE_DRIVER

Thanks
Laszlo


> 
> Leo Duran (6):
>   MdeModulePkg: Add DxeBmDmaLib library
>   ArmVirtPkg: Modify .DSC files that include PciHostBridgeDxe driver
>   CorebootPayloadPkg: Modify .DSC files that include PciHostBridgeDxe
>     driver
>   MdeModulePkg: Modify .DSC files that include PciHostBridgeDxe driver
>   OvmfPkg: Modify .DSC files that include PciHostBridgeDxe driver
>   MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib library.
> 
>  ArmVirtPkg/ArmVirtQemu.dsc                         |   1 +
>  ArmVirtPkg/ArmVirtQemuKernel.dsc                   |   1 +
>  CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc      |   1 +
>  CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc   |   1 +
>  .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf  |   1 +
>  .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h       |  13 +-
>  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c     | 272 ++++------------
>  MdeModulePkg/Include/Library/BmDmaLib.h            | 161 ++++++++++
>  MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.c     | 351 +++++++++++++++++++++
>  MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf   |  41 +++
>  MdeModulePkg/MdeModulePkg.dsc                      |   3 +
>  OvmfPkg/OvmfPkgIa32.dsc                            |   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                         |   1 +
>  OvmfPkg/OvmfPkgX64.dsc                             |   1 +
>  14 files changed, 629 insertions(+), 220 deletions(-)
>  create mode 100644 MdeModulePkg/Include/Library/BmDmaLib.h
>  create mode 100644 MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.c
>  create mode 100644 MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
>