Re: [RFC 0/6] DxeBmDmaLib

["Duran, Leo" <leo.duran@amd.com>]

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, January 12, 2017 3:51 AM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org
> Cc: feng.tian@intel.com; Singh, Brijesh <brijesh.singh@amd.com>;
> ard.biesheuvel@linaro.org; jordan.l.justen@intel.com;
> prince.agyeman@intel.com; star.zeng@intel.com
> Subject: Re: [edk2] [RFC 0/6] DxeBmDmaLib
> 
> 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.
[Duran, Leo] 
Laszlo, it's more than just code movement.
- Patch#1 clones the existing implementation from PciHostBridgeDxe,
 but the ported code is simplified a bit by the addition of the 'DmaAbove4GB' parameter.
- Patch#6 removes the ported code from PciHostBridgeDxe to leverage the library.

> 
> * 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
[Duran, Leo] 
OK,  sounds good to me. Thanks.

> 
> 
> >
> > 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
> >