ArmVirtPkg/ArmVirtQemu.dsc | 2 + ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ 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 | 2 + OvmfPkg/OvmfPkgIa32X64.dsc | 2 + OvmfPkg/OvmfPkgX64.dsc | 2 + 14 files changed, 642 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
This series provides an abstraction layer for Bus-master DMA operations as
currently 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).
Please refer to the RFC discussion for SEV here:
http://marc.info/?l=linux-mm&m=147190814023863&w=2
This new BmDmaLib class library is cloned from the existing DmaLib with
an additional DmaAbove4GB (BOOLEAN) parameter for the Map and Allocate
interfaces, so that decisions can be made about the need to allocate DMA
buffers below the 4GB boundary.
NOTE: The abstraction layer is intended for Bus-master (DMA capable)
devices, and not restricted to the PCI Root-Bridge use-case. Thus the
rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION types.
Changes since v3:
- Add copyright notice on changed files.
- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS->FreePages().
Leo Duran (6):
MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library
ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver
CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver
MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver
OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver
MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class
library
ArmVirtPkg/ArmVirtQemu.dsc | 2 +
ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 +
CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 +
CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 +
.../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 +
.../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +-
.../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------
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 | 2 +
OvmfPkg/OvmfPkgIa32X64.dsc | 2 +
OvmfPkg/OvmfPkgX64.dsc | 2 +
14 files changed, 642 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
--
2.7.4
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leo, I talked with Jiewen in office today. He felt that Intel might have a similar need of such layer of abstraction (BmDmaLib). We are investigating it. Can you please wait for several days? We'd like to review the current interfaces of BmDmaLib to make sure it's general enough to meet any potential needs. Sorry about the delay. Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo Duran >Sent: Friday, March 3, 2017 2:47 AM >To: edk2-devel@ml01.01.org >Cc: Leo Duran <leo.duran@amd.com> >Subject: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > >This series provides an abstraction layer for Bus-master DMA operations as >currently 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). > >Please refer to the RFC discussion for SEV here: >http://marc.info/?l=linux-mm&m=147190814023863&w=2 > >This new BmDmaLib class library is cloned from the existing DmaLib with >an additional DmaAbove4GB (BOOLEAN) parameter for the Map and Allocate >interfaces, so that decisions can be made about the need to allocate DMA >buffers below the 4GB boundary. > >NOTE: The abstraction layer is intended for Bus-master (DMA capable) >devices, and not restricted to the PCI Root-Bridge use-case. Thus the >rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION types. > >Changes since v3: >- Add copyright notice on changed files. >- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS->FreePages(). > >Leo Duran (6): > MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library > ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class > library > > ArmVirtPkg/ArmVirtQemu.dsc | 2 + > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ > 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 | 2 + > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgX64.dsc | 2 + > 14 files changed, 642 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 > >-- >2.7.4 > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hi Thanks for your patient. 1) I think it is good to abstract the DMA operation. However, the problem on doing this in PciHostBridge driver is that: We lose the information on which PCI device submits the Map/Ummap() request. For example, we can allocate one DMA memory for an ATA device, and we do not want USB device access this DMA memory. For Intel VTd, we need this information. Is that possible to share your feature code to help me understand how this API is used on your side? Or if it is hard to share the code, is that possible to help us evaluate if we can move this library or hook to the PciBus driver ? 2) Back to the API, I have a little concern on having "DmaAbove4GB" in BmDmaMap() and BmDmaAllocateBuffer(). Per my understanding: - The PciHostBridgeDxe driver is the core module. - The PciHostBridgeLib is the silicon hardware layout abstraction. The producer should be silicon driver. - The BmDmaLib is the PCI feature abstraction. The producer could be a core module or a platform module. I think we had better make the BmDmaLib API be similar to the API defined in UEFI spec. I discussed with Ruiyu, and we have some idea to eliminate "DmaAbove4GB" and make API consistent with UEFI spec. 3) I am not sure if BmDmaLib API need a way to distinguish which PCI_ROOT_BRIDGE_IO it is handling. That is an open question. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Friday, March 3, 2017 10:04 AM To: Leo Duran <leo.duran@amd.com>; edk2-devel@ml01.01.org Cc: Yao, Jiewen <jiewen.yao@intel.com> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Leo, I talked with Jiewen in office today. He felt that Intel might have a similar need of such layer of abstraction (BmDmaLib). We are investigating it. Can you please wait for several days? We'd like to review the current interfaces of BmDmaLib to make sure it's general enough to meet any potential needs. Sorry about the delay. Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo Duran >Sent: Friday, March 3, 2017 2:47 AM >To: edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> >Cc: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>> >Subject: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > >This series provides an abstraction layer for Bus-master DMA operations as >currently 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). > >Please refer to the RFC discussion for SEV here: >http://marc.info/?l=linux-mm&m=147190814023863&w=2 > >This new BmDmaLib class library is cloned from the existing DmaLib with >an additional DmaAbove4GB (BOOLEAN) parameter for the Map and Allocate >interfaces, so that decisions can be made about the need to allocate DMA >buffers below the 4GB boundary. > >NOTE: The abstraction layer is intended for Bus-master (DMA capable) >devices, and not restricted to the PCI Root-Bridge use-case. Thus the >rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION types. > >Changes since v3: >- Add copyright notice on changed files. >- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS->FreePages(). > >Leo Duran (6): > MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library > ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class > library > > ArmVirtPkg/ArmVirtQemu.dsc | 2 + > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ > 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 | 2 + > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgX64.dsc | 2 + > 14 files changed, 642 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 > >-- >2.7.4 > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Hi Yao, Please me replies inline below. Leo From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Friday, March 03, 2017 12:13 AM To: Ni, Ruiyu <ruiyu.ni@intel.com>; Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org Cc: Yao, Jiewen <jiewen.yao@intel.com> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Thanks for your patient. 1) I think it is good to abstract the DMA operation. However, the problem on doing this in PciHostBridge driver is that: We lose the information on which PCI device submits the Map/Ummap() request. For example, we can allocate one DMA memory for an ATA device, and we do not want USB device access this DMA memory. For Intel VTd, we need this information. Is that possible to share your feature code to help me understand how this API is used on your side? Or if it is hard to share the code, is that possible to help us evaluate if we can move this library or hook to the PciBus driver ? [Duran, Leo] The intent is to allow override of BmDmalib, so that we may set or clear the SEV mask as DMA buffers are allocated or free'ed. 2) Back to the API, I have a little concern on having "DmaAbove4GB" in BmDmaMap() and BmDmaAllocateBuffer(). [Duran, Leo] The intent of the flag is to determine the need for a bounce buffer if the bus-master device is not capable of accessing memory above 4GB's. Point being, when a bounce buffer is used for DMA we also need to manage the SEV mask on that buffer. Per my understanding: - The PciHostBridgeDxe driver is the core module. - The PciHostBridgeLib is the silicon hardware layout abstraction. The producer should be silicon driver. - The BmDmaLib is the PCI feature abstraction. The producer could be a core module or a platform module. I think we had better make the BmDmaLib API be similar to the API defined in UEFI spec. [Duran, Leo] Can you please point me to the pertinent reference in the UEFI spec? I discussed with Ruiyu, and we have some idea to eliminate "DmaAbove4GB" and make API consistent with UEFI spec. 3) I am not sure if BmDmaLib API need a way to distinguish which PCI_ROOT_BRIDGE_IO it is handling. That is an open question. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Friday, March 3, 2017 10:04 AM To: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Leo, I talked with Jiewen in office today. He felt that Intel might have a similar need of such layer of abstraction (BmDmaLib). We are investigating it. Can you please wait for several days? We'd like to review the current interfaces of BmDmaLib to make sure it's general enough to meet any potential needs. Sorry about the delay. Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo Duran >Sent: Friday, March 3, 2017 2:47 AM >To: edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> >Cc: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>> >Subject: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > >This series provides an abstraction layer for Bus-master DMA operations as >currently 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). > >Please refer to the RFC discussion for SEV here: >http://marc.info/?l=linux-mm&m=147190814023863&w=2 > >This new BmDmaLib class library is cloned from the existing DmaLib with >an additional DmaAbove4GB (BOOLEAN) parameter for the Map and Allocate >interfaces, so that decisions can be made about the need to allocate DMA >buffers below the 4GB boundary. > >NOTE: The abstraction layer is intended for Bus-master (DMA capable) >devices, and not restricted to the PCI Root-Bridge use-case. Thus the >rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION types. > >Changes since v3: >- Add copyright notice on changed files. >- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS->FreePages(). > >Leo Duran (6): > MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library > ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class > library > > ArmVirtPkg/ArmVirtQemu.dsc | 2 + > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ > 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 | 2 + > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgX64.dsc | 2 + > 14 files changed, 642 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 > >-- >2.7.4 > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Thanks for the info. Comment inline. From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo Sent: Friday, March 3, 2017 11:19 PM To: Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@ml01.01.org Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Yao, Please me replies inline below. Leo From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Friday, March 03, 2017 12:13 AM To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Thanks for your patient. 1) I think it is good to abstract the DMA operation. However, the problem on doing this in PciHostBridge driver is that: We lose the information on which PCI device submits the Map/Ummap() request. For example, we can allocate one DMA memory for an ATA device, and we do not want USB device access this DMA memory. For Intel VTd, we need this information. Is that possible to share your feature code to help me understand how this API is used on your side? Or if it is hard to share the code, is that possible to help us evaluate if we can move this library or hook to the PciBus driver ? [Duran, Leo] The intent is to allow override of BmDmalib, so that we may set or clear the SEV mask as DMA buffers are allocated or free'ed. [Jiewen] OK. It seems your requirement is similar to ours. For us, we need update VTd/IOMMU page table for PCI device, to set/clear DMA buffer when it is allocated/freed. The only difference is that our solution need to know the PCI device (bus/device/function) who submit the request. We did some POC work to protect DMA in UEFI phase. The code is at https://github.com/jyao1/STM/tree/master/Test/DmaPkg/DmaProtection, PciHook.c<https://github.com/jyao1/STM/blob/master/Test/DmaPkg/DmaProtection/PciHook.c> There is a white paper to describe the design in detail - https://firmware.intel.com/sites/default/files/resources/A_Tour_Beyond_BIOS_Using_Intel_VT-d_for_DMA_Protection.pdf This implementation hooks PCI_IO protocol Map/Ummap function. However, the *hook* is not a recommended way. Because the BmDmaLib is to abstract DMA operation, I think we can move BmDmaLib to PciBus driver instead of PciHostBridge driver. I believe it can meet both requirement. 2) Back to the API, I have a little concern on having "DmaAbove4GB" in BmDmaMap() and BmDmaAllocateBuffer(). [Duran, Leo] The intent of the flag is to determine the need for a bounce buffer if the bus-master device is not capable of accessing memory above 4GB's. Point being, when a bounce buffer is used for DMA we also need to manage the SEV mask on that buffer. Per my understanding: - The PciHostBridgeDxe driver is the core module. - The PciHostBridgeLib is the silicon hardware layout abstraction. The producer should be silicon driver. - The BmDmaLib is the PCI feature abstraction. The producer could be a core module or a platform module. I think we had better make the BmDmaLib API be similar to the API defined in UEFI spec. [Duran, Leo] Can you please point me to the pertinent reference in the UEFI spec? [Jiewen] Please refer to 13.2 PCI Root Bridge I/O Protocol, 13.4 EFI PCI I/O Protocol. Assuming we still use it in PciHostBridgeDxe, Ruiyu's and my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); // We can use Operation to know it is Above4GB requirement or not. // EfiPciOperationBusMasterRead64/ EfiPciOperationBusMasterWrite64/ EfiPciOperationBusMasterCommonBuffer64 means YES. // EfiPciOperationBusMasterRead/ EfiPciOperationBusMasterWrite/ EfiPciOperationBusMasterCommonBuffer means NO. EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); // We can use Attributes to know it is Above4GB requirement or not. // EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE means YES. Assuming we move to PciBusDxe. my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_IO *PciIo, IN EFI_PCI_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_PCI_IO *PciIo, IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); I discussed with Ruiyu, and we have some idea to eliminate "DmaAbove4GB" and make API consistent with UEFI spec. 3) I am not sure if BmDmaLib API need a way to distinguish which PCI_ROOT_BRIDGE_IO it is handling. That is an open question. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Friday, March 3, 2017 10:04 AM To: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Leo, I talked with Jiewen in office today. He felt that Intel might have a similar need of such layer of abstraction (BmDmaLib). We are investigating it. Can you please wait for several days? We'd like to review the current interfaces of BmDmaLib to make sure it's general enough to meet any potential needs. Sorry about the delay. Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo Duran >Sent: Friday, March 3, 2017 2:47 AM >To: edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> >Cc: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>> >Subject: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > >This series provides an abstraction layer for Bus-master DMA operations as >currently 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). > >Please refer to the RFC discussion for SEV here: >http://marc.info/?l=linux-mm&m=147190814023863&w=2 > >This new BmDmaLib class library is cloned from the existing DmaLib with >an additional DmaAbove4GB (BOOLEAN) parameter for the Map and Allocate >interfaces, so that decisions can be made about the need to allocate DMA >buffers below the 4GB boundary. > >NOTE: The abstraction layer is intended for Bus-master (DMA capable) >devices, and not restricted to the PCI Root-Bridge use-case. Thus the >rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION types. > >Changes since v3: >- Add copyright notice on changed files. >- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS->FreePages(). > >Leo Duran (6): > MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library > ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class > library > > ArmVirtPkg/ArmVirtQemu.dsc | 2 + > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ > 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 | 2 + > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgX64.dsc | 2 + > 14 files changed, 642 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 > >-- >2.7.4 > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
HI Leo/Ruiyu In order to express my thought clearly, I posted some POC code at https://github.com/jyao1/edk2/tree/master a branch named "dma". The library header file is @ https://github.com/jyao1/edk2/blob/dma/MdeModulePkg/Include/Library/BmDmaLib.h The default instance is @ https://github.com/jyao1/edk2/tree/dma/MdeModulePkg/Library/DxeBmDmaLib I also wrote POC template for VTd protection to show the concept @ https://github.com/jyao1/edk2/tree/dma/MdeModulePkg/Library/DxeBmDmaLibVtdSample All code just passes build, not validated yet. Please let me know your idea. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen Sent: Saturday, March 4, 2017 12:03 AM To: Duran, Leo <leo.duran@amd.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@ml01.01.org Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Thanks for the info. Comment inline. From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo Sent: Friday, March 3, 2017 11:19 PM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Yao, Please me replies inline below. Leo From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Friday, March 03, 2017 12:13 AM To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com%3cmailto:ruiyu.ni@intel.com>>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Thanks for your patient. 1) I think it is good to abstract the DMA operation. However, the problem on doing this in PciHostBridge driver is that: We lose the information on which PCI device submits the Map/Ummap() request. For example, we can allocate one DMA memory for an ATA device, and we do not want USB device access this DMA memory. For Intel VTd, we need this information. Is that possible to share your feature code to help me understand how this API is used on your side? Or if it is hard to share the code, is that possible to help us evaluate if we can move this library or hook to the PciBus driver ? [Duran, Leo] The intent is to allow override of BmDmalib, so that we may set or clear the SEV mask as DMA buffers are allocated or free'ed. [Jiewen] OK. It seems your requirement is similar to ours. For us, we need update VTd/IOMMU page table for PCI device, to set/clear DMA buffer when it is allocated/freed. The only difference is that our solution need to know the PCI device (bus/device/function) who submit the request. We did some POC work to protect DMA in UEFI phase. The code is at https://github.com/jyao1/STM/tree/master/Test/DmaPkg/DmaProtection, PciHook.c<https://github.com/jyao1/STM/blob/master/Test/DmaPkg/DmaProtection/PciHook.c> There is a white paper to describe the design in detail - https://firmware.intel.com/sites/default/files/resources/A_Tour_Beyond_BIOS_Using_Intel_VT-d_for_DMA_Protection.pdf This implementation hooks PCI_IO protocol Map/Ummap function. However, the *hook* is not a recommended way. Because the BmDmaLib is to abstract DMA operation, I think we can move BmDmaLib to PciBus driver instead of PciHostBridge driver. I believe it can meet both requirement. 2) Back to the API, I have a little concern on having "DmaAbove4GB" in BmDmaMap() and BmDmaAllocateBuffer(). [Duran, Leo] The intent of the flag is to determine the need for a bounce buffer if the bus-master device is not capable of accessing memory above 4GB's. Point being, when a bounce buffer is used for DMA we also need to manage the SEV mask on that buffer. Per my understanding: - The PciHostBridgeDxe driver is the core module. - The PciHostBridgeLib is the silicon hardware layout abstraction. The producer should be silicon driver. - The BmDmaLib is the PCI feature abstraction. The producer could be a core module or a platform module. I think we had better make the BmDmaLib API be similar to the API defined in UEFI spec. [Duran, Leo] Can you please point me to the pertinent reference in the UEFI spec? [Jiewen] Please refer to 13.2 PCI Root Bridge I/O Protocol, 13.4 EFI PCI I/O Protocol. Assuming we still use it in PciHostBridgeDxe, Ruiyu's and my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); // We can use Operation to know it is Above4GB requirement or not. // EfiPciOperationBusMasterRead64/ EfiPciOperationBusMasterWrite64/ EfiPciOperationBusMasterCommonBuffer64 means YES. // EfiPciOperationBusMasterRead/ EfiPciOperationBusMasterWrite/ EfiPciOperationBusMasterCommonBuffer means NO. EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); // We can use Attributes to know it is Above4GB requirement or not. // EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE means YES. Assuming we move to PciBusDxe. my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_IO *PciIo, IN EFI_PCI_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_PCI_IO *PciIo, IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); I discussed with Ruiyu, and we have some idea to eliminate "DmaAbove4GB" and make API consistent with UEFI spec. 3) I am not sure if BmDmaLib API need a way to distinguish which PCI_ROOT_BRIDGE_IO it is handling. That is an open question. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Friday, March 3, 2017 10:04 AM To: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Leo, I talked with Jiewen in office today. He felt that Intel might have a similar need of such layer of abstraction (BmDmaLib). We are investigating it. Can you please wait for several days? We'd like to review the current interfaces of BmDmaLib to make sure it's general enough to meet any potential needs. Sorry about the delay. Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo Duran >Sent: Friday, March 3, 2017 2:47 AM >To: edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>> >Cc: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>> >Subject: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > >This series provides an abstraction layer for Bus-master DMA operations as >currently 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). > >Please refer to the RFC discussion for SEV here: >http://marc.info/?l=linux-mm&m=147190814023863&w=2 > >This new BmDmaLib class library is cloned from the existing DmaLib with >an additional DmaAbove4GB (BOOLEAN) parameter for the Map and Allocate >interfaces, so that decisions can be made about the need to allocate DMA >buffers below the 4GB boundary. > >NOTE: The abstraction layer is intended for Bus-master (DMA capable) >devices, and not restricted to the PCI Root-Bridge use-case. Thus the >rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION types. > >Changes since v3: >- Add copyright notice on changed files. >- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS->FreePages(). > >Leo Duran (6): > MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library > ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class > library > > ArmVirtPkg/ArmVirtQemu.dsc | 2 + > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ > 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 | 2 + > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgX64.dsc | 2 + > 14 files changed, 642 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 > >-- >2.7.4 > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>> >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Jiewen, From your POC, I see a need to propose a new DMA notification protocol to PI spec. This protocol gets notified when AllocateBuffer/FreeBuffer/Map/Unmap is called. I think a notification protocol is more proper. Because calling PCI_ROOT_BRIDGE_IO.AllocateBuffer/FreeBuffer/Map/Unmap is required from UEFI/PI spec, but we implement it as it's a choice of BmDmaLib. Regards, Ray From: Yao, Jiewen Sent: Tuesday, March 7, 2017 10:04 PM To: Yao, Jiewen <jiewen.yao@intel.com>; Duran, Leo <leo.duran@amd.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@ml01.01.org Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library HI Leo/Ruiyu In order to express my thought clearly, I posted some POC code at https://github.com/jyao1/edk2/tree/master a branch named "dma". The library header file is @ https://github.com/jyao1/edk2/blob/dma/MdeModulePkg/Include/Library/BmDmaLib.h The default instance is @ https://github.com/jyao1/edk2/tree/dma/MdeModulePkg/Library/DxeBmDmaLib I also wrote POC template for VTd protection to show the concept @ https://github.com/jyao1/edk2/tree/dma/MdeModulePkg/Library/DxeBmDmaLibVtdSample All code just passes build, not validated yet. Please let me know your idea. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen Sent: Saturday, March 4, 2017 12:03 AM To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Thanks for the info. Comment inline. From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo Sent: Friday, March 3, 2017 11:19 PM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Yao, Please me replies inline below. Leo From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Friday, March 03, 2017 12:13 AM To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com%3cmailto:ruiyu.ni@intel.com>>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Thanks for your patient. 1) I think it is good to abstract the DMA operation. However, the problem on doing this in PciHostBridge driver is that: We lose the information on which PCI device submits the Map/Ummap() request. For example, we can allocate one DMA memory for an ATA device, and we do not want USB device access this DMA memory. For Intel VTd, we need this information. Is that possible to share your feature code to help me understand how this API is used on your side? Or if it is hard to share the code, is that possible to help us evaluate if we can move this library or hook to the PciBus driver ? [Duran, Leo] The intent is to allow override of BmDmalib, so that we may set or clear the SEV mask as DMA buffers are allocated or free'ed. [Jiewen] OK. It seems your requirement is similar to ours. For us, we need update VTd/IOMMU page table for PCI device, to set/clear DMA buffer when it is allocated/freed. The only difference is that our solution need to know the PCI device (bus/device/function) who submit the request. We did some POC work to protect DMA in UEFI phase. The code is at https://github.com/jyao1/STM/tree/master/Test/DmaPkg/DmaProtection, PciHook.c<https://github.com/jyao1/STM/blob/master/Test/DmaPkg/DmaProtection/PciHook.c> There is a white paper to describe the design in detail - https://firmware.intel.com/sites/default/files/resources/A_Tour_Beyond_BIOS_Using_Intel_VT-d_for_DMA_Protection.pdf This implementation hooks PCI_IO protocol Map/Ummap function. However, the *hook* is not a recommended way. Because the BmDmaLib is to abstract DMA operation, I think we can move BmDmaLib to PciBus driver instead of PciHostBridge driver. I believe it can meet both requirement. 2) Back to the API, I have a little concern on having "DmaAbove4GB" in BmDmaMap() and BmDmaAllocateBuffer(). [Duran, Leo] The intent of the flag is to determine the need for a bounce buffer if the bus-master device is not capable of accessing memory above 4GB's. Point being, when a bounce buffer is used for DMA we also need to manage the SEV mask on that buffer. Per my understanding: - The PciHostBridgeDxe driver is the core module. - The PciHostBridgeLib is the silicon hardware layout abstraction. The producer should be silicon driver. - The BmDmaLib is the PCI feature abstraction. The producer could be a core module or a platform module. I think we had better make the BmDmaLib API be similar to the API defined in UEFI spec. [Duran, Leo] Can you please point me to the pertinent reference in the UEFI spec? [Jiewen] Please refer to 13.2 PCI Root Bridge I/O Protocol, 13.4 EFI PCI I/O Protocol. Assuming we still use it in PciHostBridgeDxe, Ruiyu's and my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); // We can use Operation to know it is Above4GB requirement or not. // EfiPciOperationBusMasterRead64/ EfiPciOperationBusMasterWrite64/ EfiPciOperationBusMasterCommonBuffer64 means YES. // EfiPciOperationBusMasterRead/ EfiPciOperationBusMasterWrite/ EfiPciOperationBusMasterCommonBuffer means NO. EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); // We can use Attributes to know it is Above4GB requirement or not. // EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE means YES. Assuming we move to PciBusDxe. my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_IO *PciIo, IN EFI_PCI_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_PCI_IO *PciIo, IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); I discussed with Ruiyu, and we have some idea to eliminate "DmaAbove4GB" and make API consistent with UEFI spec. 3) I am not sure if BmDmaLib API need a way to distinguish which PCI_ROOT_BRIDGE_IO it is handling. That is an open question. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Friday, March 3, 2017 10:04 AM To: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Leo, I talked with Jiewen in office today. He felt that Intel might have a similar need of such layer of abstraction (BmDmaLib). We are investigating it. Can you please wait for several days? We'd like to review the current interfaces of BmDmaLib to make sure it's general enough to meet any potential needs. Sorry about the delay. Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo Duran >Sent: Friday, March 3, 2017 2:47 AM >To: edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>> >Cc: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>> >Subject: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > >This series provides an abstraction layer for Bus-master DMA operations as >currently 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). > >Please refer to the RFC discussion for SEV here: >http://marc.info/?l=linux-mm&m=147190814023863&w=2 > >This new BmDmaLib class library is cloned from the existing DmaLib with >an additional DmaAbove4GB (BOOLEAN) parameter for the Map and Allocate >interfaces, so that decisions can be made about the need to allocate DMA >buffers below the 4GB boundary. > >NOTE: The abstraction layer is intended for Bus-master (DMA capable) >devices, and not restricted to the PCI Root-Bridge use-case. Thus the >rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION types. > >Changes since v3: >- Add copyright notice on changed files. >- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS->FreePages(). > >Leo Duran (6): > MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library > ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class > library > > ArmVirtPkg/ArmVirtQemu.dsc | 2 + > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ > 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 | 2 + > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgX64.dsc | 2 + > 14 files changed, 642 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 > >-- >2.7.4 > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>> >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
In order to configure the DMA remapping table, the VTd instance need convert the bytes based allocation to pages based allocation, which is beyond notification. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Wednesday, March 8, 2017 8:57 AM To: Yao, Jiewen <jiewen.yao@intel.com>; Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Jiewen, From your POC, I see a need to propose a new DMA notification protocol to PI spec. This protocol gets notified when AllocateBuffer/FreeBuffer/Map/Unmap is called. I think a notification protocol is more proper. Because calling PCI_ROOT_BRIDGE_IO.AllocateBuffer/FreeBuffer/Map/Unmap is required from UEFI/PI spec, but we implement it as it's a choice of BmDmaLib. Regards, Ray From: Yao, Jiewen Sent: Tuesday, March 7, 2017 10:04 PM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library HI Leo/Ruiyu In order to express my thought clearly, I posted some POC code at https://github.com/jyao1/edk2/tree/master a branch named "dma". The library header file is @ https://github.com/jyao1/edk2/blob/dma/MdeModulePkg/Include/Library/BmDmaLib.h The default instance is @ https://github.com/jyao1/edk2/tree/dma/MdeModulePkg/Library/DxeBmDmaLib I also wrote POC template for VTd protection to show the concept @ https://github.com/jyao1/edk2/tree/dma/MdeModulePkg/Library/DxeBmDmaLibVtdSample All code just passes build, not validated yet. Please let me know your idea. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen Sent: Saturday, March 4, 2017 12:03 AM To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Thanks for the info. Comment inline. From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo Sent: Friday, March 3, 2017 11:19 PM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Yao, Please me replies inline below. Leo From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Friday, March 03, 2017 12:13 AM To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com%3cmailto:ruiyu.ni@intel.com>>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Thanks for your patient. 1) I think it is good to abstract the DMA operation. However, the problem on doing this in PciHostBridge driver is that: We lose the information on which PCI device submits the Map/Ummap() request. For example, we can allocate one DMA memory for an ATA device, and we do not want USB device access this DMA memory. For Intel VTd, we need this information. Is that possible to share your feature code to help me understand how this API is used on your side? Or if it is hard to share the code, is that possible to help us evaluate if we can move this library or hook to the PciBus driver ? [Duran, Leo] The intent is to allow override of BmDmalib, so that we may set or clear the SEV mask as DMA buffers are allocated or free'ed. [Jiewen] OK. It seems your requirement is similar to ours. For us, we need update VTd/IOMMU page table for PCI device, to set/clear DMA buffer when it is allocated/freed. The only difference is that our solution need to know the PCI device (bus/device/function) who submit the request. We did some POC work to protect DMA in UEFI phase. The code is at https://github.com/jyao1/STM/tree/master/Test/DmaPkg/DmaProtection, PciHook.c<https://github.com/jyao1/STM/blob/master/Test/DmaPkg/DmaProtection/PciHook.c> There is a white paper to describe the design in detail - https://firmware.intel.com/sites/default/files/resources/A_Tour_Beyond_BIOS_Using_Intel_VT-d_for_DMA_Protection.pdf This implementation hooks PCI_IO protocol Map/Ummap function. However, the *hook* is not a recommended way. Because the BmDmaLib is to abstract DMA operation, I think we can move BmDmaLib to PciBus driver instead of PciHostBridge driver. I believe it can meet both requirement. 2) Back to the API, I have a little concern on having "DmaAbove4GB" in BmDmaMap() and BmDmaAllocateBuffer(). [Duran, Leo] The intent of the flag is to determine the need for a bounce buffer if the bus-master device is not capable of accessing memory above 4GB's. Point being, when a bounce buffer is used for DMA we also need to manage the SEV mask on that buffer. Per my understanding: - The PciHostBridgeDxe driver is the core module. - The PciHostBridgeLib is the silicon hardware layout abstraction. The producer should be silicon driver. - The BmDmaLib is the PCI feature abstraction. The producer could be a core module or a platform module. I think we had better make the BmDmaLib API be similar to the API defined in UEFI spec. [Duran, Leo] Can you please point me to the pertinent reference in the UEFI spec? [Jiewen] Please refer to 13.2 PCI Root Bridge I/O Protocol, 13.4 EFI PCI I/O Protocol. Assuming we still use it in PciHostBridgeDxe, Ruiyu's and my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); // We can use Operation to know it is Above4GB requirement or not. // EfiPciOperationBusMasterRead64/ EfiPciOperationBusMasterWrite64/ EfiPciOperationBusMasterCommonBuffer64 means YES. // EfiPciOperationBusMasterRead/ EfiPciOperationBusMasterWrite/ EfiPciOperationBusMasterCommonBuffer means NO. EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); // We can use Attributes to know it is Above4GB requirement or not. // EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE means YES. Assuming we move to PciBusDxe. my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_IO *PciIo, IN EFI_PCI_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_PCI_IO *PciIo, IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); I discussed with Ruiyu, and we have some idea to eliminate "DmaAbove4GB" and make API consistent with UEFI spec. 3) I am not sure if BmDmaLib API need a way to distinguish which PCI_ROOT_BRIDGE_IO it is handling. That is an open question. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Friday, March 3, 2017 10:04 AM To: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Leo, I talked with Jiewen in office today. He felt that Intel might have a similar need of such layer of abstraction (BmDmaLib). We are investigating it. Can you please wait for several days? We'd like to review the current interfaces of BmDmaLib to make sure it's general enough to meet any potential needs. Sorry about the delay. Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo Duran >Sent: Friday, March 3, 2017 2:47 AM >To: edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>> >Cc: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>> >Subject: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > >This series provides an abstraction layer for Bus-master DMA operations as >currently 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). > >Please refer to the RFC discussion for SEV here: >http://marc.info/?l=linux-mm&m=147190814023863&w=2 > >This new BmDmaLib class library is cloned from the existing DmaLib with >an additional DmaAbove4GB (BOOLEAN) parameter for the Map and Allocate >interfaces, so that decisions can be made about the need to allocate DMA >buffers below the 4GB boundary. > >NOTE: The abstraction layer is intended for Bus-master (DMA capable) >devices, and not restricted to the PCI Root-Bridge use-case. Thus the >rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION types. > >Changes since v3: >- Add copyright notice on changed files. >- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS->FreePages(). > >Leo Duran (6): > MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library > ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class > library > > ArmVirtPkg/ArmVirtQemu.dsc | 2 + > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ > 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 | 2 + > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgX64.dsc | 2 + > 14 files changed, 642 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 > >-- >2.7.4 > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>> >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
PCI_ROOT_BRIDGE_IO.Allocate() is page-based allocation, not byte-based allocation. Regards, Ray From: Yao, Jiewen Sent: Wednesday, March 8, 2017 9:21 AM To: Ni, Ruiyu <ruiyu.ni@intel.com>; Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library In order to configure the DMA remapping table, the VTd instance need convert the bytes based allocation to pages based allocation, which is beyond notification. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Wednesday, March 8, 2017 8:57 AM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Jiewen, From your POC, I see a need to propose a new DMA notification protocol to PI spec. This protocol gets notified when AllocateBuffer/FreeBuffer/Map/Unmap is called. I think a notification protocol is more proper. Because calling PCI_ROOT_BRIDGE_IO.AllocateBuffer/FreeBuffer/Map/Unmap is required from UEFI/PI spec, but we implement it as it's a choice of BmDmaLib. Regards, Ray From: Yao, Jiewen Sent: Tuesday, March 7, 2017 10:04 PM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library HI Leo/Ruiyu In order to express my thought clearly, I posted some POC code at https://github.com/jyao1/edk2/tree/master a branch named "dma". The library header file is @ https://github.com/jyao1/edk2/blob/dma/MdeModulePkg/Include/Library/BmDmaLib.h The default instance is @ https://github.com/jyao1/edk2/tree/dma/MdeModulePkg/Library/DxeBmDmaLib I also wrote POC template for VTd protection to show the concept @ https://github.com/jyao1/edk2/tree/dma/MdeModulePkg/Library/DxeBmDmaLibVtdSample All code just passes build, not validated yet. Please let me know your idea. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen Sent: Saturday, March 4, 2017 12:03 AM To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Thanks for the info. Comment inline. From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo Sent: Friday, March 3, 2017 11:19 PM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Yao, Please me replies inline below. Leo From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Friday, March 03, 2017 12:13 AM To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com%3cmailto:ruiyu.ni@intel.com>>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Thanks for your patient. 1) I think it is good to abstract the DMA operation. However, the problem on doing this in PciHostBridge driver is that: We lose the information on which PCI device submits the Map/Ummap() request. For example, we can allocate one DMA memory for an ATA device, and we do not want USB device access this DMA memory. For Intel VTd, we need this information. Is that possible to share your feature code to help me understand how this API is used on your side? Or if it is hard to share the code, is that possible to help us evaluate if we can move this library or hook to the PciBus driver ? [Duran, Leo] The intent is to allow override of BmDmalib, so that we may set or clear the SEV mask as DMA buffers are allocated or free'ed. [Jiewen] OK. It seems your requirement is similar to ours. For us, we need update VTd/IOMMU page table for PCI device, to set/clear DMA buffer when it is allocated/freed. The only difference is that our solution need to know the PCI device (bus/device/function) who submit the request. We did some POC work to protect DMA in UEFI phase. The code is at https://github.com/jyao1/STM/tree/master/Test/DmaPkg/DmaProtection, PciHook.c<https://github.com/jyao1/STM/blob/master/Test/DmaPkg/DmaProtection/PciHook.c> There is a white paper to describe the design in detail - https://firmware.intel.com/sites/default/files/resources/A_Tour_Beyond_BIOS_Using_Intel_VT-d_for_DMA_Protection.pdf This implementation hooks PCI_IO protocol Map/Ummap function. However, the *hook* is not a recommended way. Because the BmDmaLib is to abstract DMA operation, I think we can move BmDmaLib to PciBus driver instead of PciHostBridge driver. I believe it can meet both requirement. 2) Back to the API, I have a little concern on having "DmaAbove4GB" in BmDmaMap() and BmDmaAllocateBuffer(). [Duran, Leo] The intent of the flag is to determine the need for a bounce buffer if the bus-master device is not capable of accessing memory above 4GB's. Point being, when a bounce buffer is used for DMA we also need to manage the SEV mask on that buffer. Per my understanding: - The PciHostBridgeDxe driver is the core module. - The PciHostBridgeLib is the silicon hardware layout abstraction. The producer should be silicon driver. - The BmDmaLib is the PCI feature abstraction. The producer could be a core module or a platform module. I think we had better make the BmDmaLib API be similar to the API defined in UEFI spec. [Duran, Leo] Can you please point me to the pertinent reference in the UEFI spec? [Jiewen] Please refer to 13.2 PCI Root Bridge I/O Protocol, 13.4 EFI PCI I/O Protocol. Assuming we still use it in PciHostBridgeDxe, Ruiyu's and my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); // We can use Operation to know it is Above4GB requirement or not. // EfiPciOperationBusMasterRead64/ EfiPciOperationBusMasterWrite64/ EfiPciOperationBusMasterCommonBuffer64 means YES. // EfiPciOperationBusMasterRead/ EfiPciOperationBusMasterWrite/ EfiPciOperationBusMasterCommonBuffer means NO. EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); // We can use Attributes to know it is Above4GB requirement or not. // EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE means YES. Assuming we move to PciBusDxe. my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_IO *PciIo, IN EFI_PCI_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_PCI_IO *PciIo, IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); I discussed with Ruiyu, and we have some idea to eliminate "DmaAbove4GB" and make API consistent with UEFI spec. 3) I am not sure if BmDmaLib API need a way to distinguish which PCI_ROOT_BRIDGE_IO it is handling. That is an open question. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Friday, March 3, 2017 10:04 AM To: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Leo, I talked with Jiewen in office today. He felt that Intel might have a similar need of such layer of abstraction (BmDmaLib). We are investigating it. Can you please wait for several days? We'd like to review the current interfaces of BmDmaLib to make sure it's general enough to meet any potential needs. Sorry about the delay. Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo Duran >Sent: Friday, March 3, 2017 2:47 AM >To: edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>> >Cc: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>> >Subject: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > >This series provides an abstraction layer for Bus-master DMA operations as >currently 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). > >Please refer to the RFC discussion for SEV here: >http://marc.info/?l=linux-mm&m=147190814023863&w=2 > >This new BmDmaLib class library is cloned from the existing DmaLib with >an additional DmaAbove4GB (BOOLEAN) parameter for the Map and Allocate >interfaces, so that decisions can be made about the need to allocate DMA >buffers below the 4GB boundary. > >NOTE: The abstraction layer is intended for Bus-master (DMA capable) >devices, and not restricted to the PCI Root-Bridge use-case. Thus the >rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION types. > >Changes since v3: >- Add copyright notice on changed files. >- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS->FreePages(). > >Leo Duran (6): > MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library > ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class > library > > ArmVirtPkg/ArmVirtQemu.dsc | 2 + > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ > 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 | 2 + > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgX64.dsc | 2 + > 14 files changed, 642 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 > >-- >2.7.4 > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>> >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
I mean Map(). Thank you Yao Jiewen From: Ni, Ruiyu Sent: Wednesday, March 8, 2017 10:06 AM To: Yao, Jiewen <jiewen.yao@intel.com>; Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library PCI_ROOT_BRIDGE_IO.Allocate() is page-based allocation, not byte-based allocation. Regards, Ray From: Yao, Jiewen Sent: Wednesday, March 8, 2017 9:21 AM To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library In order to configure the DMA remapping table, the VTd instance need convert the bytes based allocation to pages based allocation, which is beyond notification. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Wednesday, March 8, 2017 8:57 AM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Jiewen, From your POC, I see a need to propose a new DMA notification protocol to PI spec. This protocol gets notified when AllocateBuffer/FreeBuffer/Map/Unmap is called. I think a notification protocol is more proper. Because calling PCI_ROOT_BRIDGE_IO.AllocateBuffer/FreeBuffer/Map/Unmap is required from UEFI/PI spec, but we implement it as it's a choice of BmDmaLib. Regards, Ray From: Yao, Jiewen Sent: Tuesday, March 7, 2017 10:04 PM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library HI Leo/Ruiyu In order to express my thought clearly, I posted some POC code at https://github.com/jyao1/edk2/tree/master a branch named "dma". The library header file is @ https://github.com/jyao1/edk2/blob/dma/MdeModulePkg/Include/Library/BmDmaLib.h The default instance is @ https://github.com/jyao1/edk2/tree/dma/MdeModulePkg/Library/DxeBmDmaLib I also wrote POC template for VTd protection to show the concept @ https://github.com/jyao1/edk2/tree/dma/MdeModulePkg/Library/DxeBmDmaLibVtdSample All code just passes build, not validated yet. Please let me know your idea. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen Sent: Saturday, March 4, 2017 12:03 AM To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Thanks for the info. Comment inline. From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo Sent: Friday, March 3, 2017 11:19 PM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Subject: Re: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Yao, Please me replies inline below. Leo From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Friday, March 03, 2017 12:13 AM To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com%3cmailto:ruiyu.ni@intel.com>>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Hi Thanks for your patient. 1) I think it is good to abstract the DMA operation. However, the problem on doing this in PciHostBridge driver is that: We lose the information on which PCI device submits the Map/Ummap() request. For example, we can allocate one DMA memory for an ATA device, and we do not want USB device access this DMA memory. For Intel VTd, we need this information. Is that possible to share your feature code to help me understand how this API is used on your side? Or if it is hard to share the code, is that possible to help us evaluate if we can move this library or hook to the PciBus driver ? [Duran, Leo] The intent is to allow override of BmDmalib, so that we may set or clear the SEV mask as DMA buffers are allocated or free'ed. [Jiewen] OK. It seems your requirement is similar to ours. For us, we need update VTd/IOMMU page table for PCI device, to set/clear DMA buffer when it is allocated/freed. The only difference is that our solution need to know the PCI device (bus/device/function) who submit the request. We did some POC work to protect DMA in UEFI phase. The code is at https://github.com/jyao1/STM/tree/master/Test/DmaPkg/DmaProtection, PciHook.c<https://github.com/jyao1/STM/blob/master/Test/DmaPkg/DmaProtection/PciHook.c> There is a white paper to describe the design in detail - https://firmware.intel.com/sites/default/files/resources/A_Tour_Beyond_BIOS_Using_Intel_VT-d_for_DMA_Protection.pdf This implementation hooks PCI_IO protocol Map/Ummap function. However, the *hook* is not a recommended way. Because the BmDmaLib is to abstract DMA operation, I think we can move BmDmaLib to PciBus driver instead of PciHostBridge driver. I believe it can meet both requirement. 2) Back to the API, I have a little concern on having "DmaAbove4GB" in BmDmaMap() and BmDmaAllocateBuffer(). [Duran, Leo] The intent of the flag is to determine the need for a bounce buffer if the bus-master device is not capable of accessing memory above 4GB's. Point being, when a bounce buffer is used for DMA we also need to manage the SEV mask on that buffer. Per my understanding: - The PciHostBridgeDxe driver is the core module. - The PciHostBridgeLib is the silicon hardware layout abstraction. The producer should be silicon driver. - The BmDmaLib is the PCI feature abstraction. The producer could be a core module or a platform module. I think we had better make the BmDmaLib API be similar to the API defined in UEFI spec. [Duran, Leo] Can you please point me to the pertinent reference in the UEFI spec? [Jiewen] Please refer to 13.2 PCI Root Bridge I/O Protocol, 13.4 EFI PCI I/O Protocol. Assuming we still use it in PciHostBridgeDxe, Ruiyu's and my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); // We can use Operation to know it is Above4GB requirement or not. // EfiPciOperationBusMasterRead64/ EfiPciOperationBusMasterWrite64/ EfiPciOperationBusMasterCommonBuffer64 means YES. // EfiPciOperationBusMasterRead/ EfiPciOperationBusMasterWrite/ EfiPciOperationBusMasterCommonBuffer means NO. EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); // We can use Attributes to know it is Above4GB requirement or not. // EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE means YES. Assuming we move to PciBusDxe. my thought is: EFI_STATUS EFIAPI BmDmaMap ( IN EFI_PCI_IO *PciIo, IN EFI_PCI_IO_PROTOCOL_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ); EFI_STATUS EFIAPI BmDmaAllocateBuffer ( IN EFI_PCI_IO *PciIo, IN EFI_ALLOCATE_TYPE Type, IN EFI_MEMORY_TYPE MemoryType, IN UINTN Pages, OUT VOID **HostAddress, IN UINT64 Attributes ); I discussed with Ruiyu, and we have some idea to eliminate "DmaAbove4GB" and make API consistent with UEFI spec. 3) I am not sure if BmDmaLib API need a way to distinguish which PCI_ROOT_BRIDGE_IO it is handling. That is an open question. Thank you Yao Jiewen From: Ni, Ruiyu Sent: Friday, March 3, 2017 10:04 AM To: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>> Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library Leo, I talked with Jiewen in office today. He felt that Intel might have a similar need of such layer of abstraction (BmDmaLib). We are investigating it. Can you please wait for several days? We'd like to review the current interfaces of BmDmaLib to make sure it's general enough to meet any potential needs. Sorry about the delay. Regards, Ray >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leo Duran >Sent: Friday, March 3, 2017 2:47 AM >To: edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>> >Cc: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>> >Subject: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > >This series provides an abstraction layer for Bus-master DMA operations as >currently 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). > >Please refer to the RFC discussion for SEV here: >http://marc.info/?l=linux-mm&m=147190814023863&w=2 > >This new BmDmaLib class library is cloned from the existing DmaLib with >an additional DmaAbove4GB (BOOLEAN) parameter for the Map and Allocate >interfaces, so that decisions can be made about the need to allocate DMA >buffers below the 4GB boundary. > >NOTE: The abstraction layer is intended for Bus-master (DMA capable) >devices, and not restricted to the PCI Root-Bridge use-case. Thus the >rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION types. > >Changes since v3: >- Add copyright notice on changed files. >- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS->FreePages(). > >Leo Duran (6): > MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library > ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class > library > > ArmVirtPkg/ArmVirtQemu.dsc | 2 + > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ > 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 | 2 + > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > OvmfPkg/OvmfPkgX64.dsc | 2 + > 14 files changed, 642 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 > >-- >2.7.4 > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>> >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
Sounds good, thanks. Leo > -----Original Message----- > From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com] > Sent: Thursday, March 02, 2017 8:04 PM > To: Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org > Cc: Yao, Jiewen <jiewen.yao@intel.com> > Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > > Leo, > I talked with Jiewen in office today. He felt that Intel might have a similar > need of such layer of abstraction (BmDmaLib). > We are investigating it. Can you please wait for several days? > We'd like to review the current interfaces of BmDmaLib to make sure it's > general enough to meet any potential needs. > Sorry about the delay. > > Regards, > Ray > > >-----Original Message----- > >From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > >Leo Duran > >Sent: Friday, March 3, 2017 2:47 AM > >To: edk2-devel@ml01.01.org > >Cc: Leo Duran <leo.duran@amd.com> > >Subject: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library > > > >This series provides an abstraction layer for Bus-master DMA operations > >as currently 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). > > > >Please refer to the RFC discussion for SEV here: > >http://marc.info/?l=linux-mm&m=147190814023863&w=2 > > > >This new BmDmaLib class library is cloned from the existing DmaLib with > >an additional DmaAbove4GB (BOOLEAN) parameter for the Map and > Allocate > >interfaces, so that decisions can be made about the need to allocate > >DMA buffers below the 4GB boundary. > > > >NOTE: The abstraction layer is intended for Bus-master (DMA capable) > >devices, and not restricted to the PCI Root-Bridge use-case. Thus the > >rationale for not using EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION > types. > > > >Changes since v3: > >- Add copyright notice on changed files. > >- Re-order parameters on BmDmaFreeBuffer(), consistent with gBS- > >FreePages(). > > > >Leo Duran (6): > > MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library > > ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > > CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe > >driver > > MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > > OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver > > MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class > > library > > > > ArmVirtPkg/ArmVirtQemu.dsc | 2 + > > ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 + > > CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc | 3 + > > CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 + > > .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 + > > .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 15 +- > > .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 274 ++++------------ > > 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 | 2 + > > OvmfPkg/OvmfPkgIa32X64.dsc | 2 + > > OvmfPkg/OvmfPkgX64.dsc | 2 + > > 14 files changed, 642 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 > > > >-- > >2.7.4 > > > >_______________________________________________ > >edk2-devel mailing list > >edk2-devel@lists.01.org > >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.