[PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Leo Duran]

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

[PATCH v4 1/6] MdeModulePkg: Add DxeBmDmaLib (BmDmaLib class) library

[Leo Duran]

This patch 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).

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.

Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
---
 MdeModulePkg/Include/Library/BmDmaLib.h          | 161 +++++++++++
 MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.c   | 351 +++++++++++++++++++++++
 MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf |  41 +++
 MdeModulePkg/MdeModulePkg.dsc                    |   2 +
 4 files changed, 555 insertions(+)
 create mode 100644 MdeModulePkg/Include/Library/BmDmaLib.h
 create mode 100644 MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.c
 create mode 100644 MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf

diff --git a/MdeModulePkg/Include/Library/BmDmaLib.h b/MdeModulePkg/Include/Library/BmDmaLib.h
new file mode 100644
index 0000000..070340c
--- /dev/null
+++ b/MdeModulePkg/Include/Library/BmDmaLib.h
@@ -0,0 +1,161 @@
+/** @file
+  DMA abstraction library APIs. Based on PCI IO protocol DMA abstractions.
+
+  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
+  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+
+  DMA Bus Master Read Operation:
+    Call BmDmaMap() for DmaOperationBusMasterRead.
+    Program the DMA Bus Master with the DeviceAddress returned by BmDmaMap().
+    Start the DMA Bus Master.
+    Wait for DMA Bus Master to complete the read operation.
+    Call BmDmaUnmap().
+
+  DMA Bus Master Write Operation:
+    Call BmDmaMap() for DmaOperationBusMasterWrite.
+    Program the DMA Bus Master with the DeviceAddress returned by BmDmaMap().
+    Start the DMA Bus Master.
+    Wait for DMA Bus Master to complete the write operation.
+    Call BmDmaUnmap().
+
+  DMA Bus Master Common Buffer Operation:
+    Call BmDmaAllocateBuffer() to allocate a common buffer.
+    Call BmDmaMap() for DmaOperationBusMasterCommonBuffer.
+    Program the DMA Bus Master with the DeviceAddress returned by BmDmaMap().
+    The common buffer can now be accessed equally by the processor and the DMA bus master.
+    Call BmDmaUnmap().
+    Call BmDmaFreeBuffer().
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+ Derived from:
+   EmbeddedPkg/Include/Library/DmaLib.h
+
+**/
+
+#ifndef __BM_DMA_LIB_H__
+#define __BM_DMA_LIB_H__
+
+
+typedef enum {
+  ///
+  /// A read operation from system memory by a bus master.
+  ///
+  DmaOperationBusMasterRead,
+  ///
+  /// A write operation from system memory by a bus master.
+  ///
+  DmaOperationBusMasterWrite,
+  ///
+  /// Provides both read and write access to system memory by both the processor and a
+  /// bus master. The buffer is coherent from both the processor's and the bus master's point of view.
+  ///
+  DmaOperationBusMasterCommonBuffer,
+  DmaOperationBusMasterMaximum
+} BM_DMA_OPERATION;
+
+
+/**
+  Provides the DMA controller-specific addresses needed to access system memory.
+
+  Operation is relative to the DMA bus master.
+
+  @param  DmaAbove4GB           Indicates capability of DMA operations above 4GB.
+  @param  Operation             Indicates if the bus master is going to read or write to system memory.
+  @param  HostAddress           The system memory address to map to the DMA controller.
+  @param  NumberOfBytes         On input the number of bytes to map. On output the number of bytes
+                                that were mapped.
+  @param  DeviceAddress         The resulting map address for the bus master controller to use to
+                                access the hosts HostAddress.
+  @param  Mapping               A resulting value to pass to BmDmaUnmap().
+
+  @retval EFI_SUCCESS           The range was mapped for the returned NumberOfBytes.
+  @retval EFI_UNSUPPORTED       The HostAddress cannot be mapped as a common buffer.
+  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
+  @retval EFI_DEVICE_ERROR      The system hardware could not map the requested address.
+
+**/
+EFI_STATUS
+EFIAPI
+BmDmaMap (
+  IN     BOOLEAN           DmaAbove4GB,
+  IN     BM_DMA_OPERATION  Operation,
+  IN     VOID              *HostAddress,
+  IN OUT UINTN             *NumberOfBytes,
+  OUT    PHYSICAL_ADDRESS  *DeviceAddress,
+  OUT    VOID              **Mapping
+  );
+
+
+/**
+  Completes the DmaOperationBusMasterRead/Write/CommonBuffer operation
+  and releases any corresponding resources.
+
+  @param  Mapping               The mapping value returned from BmDmaMap().
+
+  @retval EFI_SUCCESS           The range was unmapped.
+  @retval EFI_DEVICE_ERROR      The data was not committed to the target system memory.
+
+**/
+EFI_STATUS
+EFIAPI
+BmDmaUnmap (
+  IN  VOID                 *Mapping
+  );
+
+
+/**
+  Allocates pages that are suitable for a BmDmaMap() of type DmaOperationBusMasterCommonBuffer.
+
+  @param  DmaAbove4GB           Indicates capability of DMA operations above 4GB.
+  @param  MemoryType            The type of memory to allocate: EfiBootServicesData or
+                                EfiRuntimeServicesData.
+  @param  Pages                 The number of pages to allocate.
+  @param  HostAddress           A pointer to store the base system memory address of the
+                                allocated range.
+
+  @retval EFI_SUCCESS           The requested memory pages were allocated.
+  @retval EFI_UNSUPPORTED       Attributes is unsupported. The only legal attribute bits are
+                                MEMORY_WRITE_COMBINE and MEMORY_CACHED.
+  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+  @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
+
+**/
+EFI_STATUS
+EFIAPI
+BmDmaAllocateBuffer (
+  IN  BOOLEAN              DmaAbove4GB,
+  IN  EFI_MEMORY_TYPE      MemoryType,
+  IN  UINTN                Pages,
+  OUT VOID                 **HostAddress
+  );
+
+
+/**
+  Frees memory that was allocated with BmDmaAllocateBuffer().
+
+  @param  HostAddress           The base system memory address of the allocated range.
+  @param  Pages                 The number of pages to free.
+
+  @retval EFI_SUCCESS           The requested memory pages were freed.
+  @retval EFI_INVALID_PARAMETER The memory range specified by HostAddress and Pages
+                                was not allocated with BmDmaAllocateBuffer().
+
+**/
+EFI_STATUS
+EFIAPI
+BmDmaFreeBuffer (
+  IN  VOID                 *HostAddress,
+  IN  UINTN                Pages
+  );
+
+
+#endif
+
diff --git a/MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.c b/MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.c
new file mode 100644
index 0000000..a342c9e
--- /dev/null
+++ b/MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.c
@@ -0,0 +1,351 @@
+/** @file
+  DMA abstraction library APIs. Based on PCI IO protocol DMA abstractions.
+
+  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
+  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+  Derived from:
+   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+
+**/
+
+#include <PiDxe.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/BmDmaLib.h>
+
+
+#define FORCE_BELOW_4GB_TRUE   TRUE
+#define FORCE_BELOW_4GB_FALSE  FALSE
+#define NO_MAPPING             (VOID *) (UINTN) -1
+
+
+typedef struct {
+  BM_DMA_OPERATION      Operation;
+  UINTN                 NumberOfBytes;
+  UINTN                 NumberOfPages;
+  EFI_PHYSICAL_ADDRESS  HostAddress;
+  EFI_PHYSICAL_ADDRESS  MappedHostAddress;
+} MAP_INFO;
+
+
+EFI_STATUS
+AllocateBounceBuffer (
+  IN     BOOLEAN               ForceBelow4GB,
+  IN     BM_DMA_OPERATION      Operation,
+  IN     EFI_PHYSICAL_ADDRESS  HostAddress,
+  IN OUT UINTN                 *NumberOfBytes,
+  OUT    PHYSICAL_ADDRESS      *DeviceAddress,
+  OUT    VOID                  **Mapping
+  )
+{
+  EFI_STATUS         Status;
+  MAP_INFO           *MapInfo;
+  EFI_ALLOCATE_TYPE  AllocateType;
+
+  //
+  // Allocate a MAP_INFO structure to remember the mapping when Unmap() is
+  // called later.
+  //
+  MapInfo = AllocatePool (sizeof (MAP_INFO));
+  if (MapInfo == NULL) {
+    *NumberOfBytes = 0;
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  //
+  // Initialize the MAP_INFO structure
+  //
+  MapInfo->Operation     = Operation;
+  MapInfo->NumberOfBytes = *NumberOfBytes;
+  MapInfo->NumberOfPages = EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes);
+  MapInfo->HostAddress   = HostAddress;
+
+  if (ForceBelow4GB) {
+    //
+    // Limit allocations to memory below 4GB
+    //
+    AllocateType = AllocateMaxAddress;
+    MapInfo->MappedHostAddress = SIZE_4GB - 1;
+  } else {
+    AllocateType = AllocateAnyPages;
+  }
+
+  //
+  // Allocate DMA bounce buffer
+  //
+  Status = gBS->AllocatePages (
+                  AllocateType,
+                  EfiBootServicesData,
+                  MapInfo->NumberOfPages,
+                  &MapInfo->MappedHostAddress
+                  );
+
+  if (EFI_ERROR (Status)) {
+    FreePool (MapInfo);
+    *NumberOfBytes = 0;
+    return Status;
+  }
+
+  //
+  // If this is a read operation from the Bus Master's point of view,
+  // then copy the contents of the real buffer into the mapped buffer
+  // so the Bus Master can read the contents of the real buffer.
+  //
+  if (Operation ==  DmaOperationBusMasterRead) {
+    CopyMem (
+      (VOID *) (UINTN) MapInfo->MappedHostAddress,
+      (VOID *) (UINTN) MapInfo->HostAddress,
+      MapInfo->NumberOfBytes
+      );
+  }
+
+  //
+  // The DeviceAddress is the address of the mapped buffer
+  //
+  *DeviceAddress = MapInfo->MappedHostAddress;
+
+  //
+  // Return a pointer to the MAP_INFO structure in Mapping
+  //
+  *Mapping = MapInfo;
+
+  return EFI_SUCCESS;
+}
+
+
+/**
+  Provides the DMA controller-specific addresses needed to access system memory.
+
+  Operation is relative to the DMA bus master.
+
+  @param  DmaAbove4GB           Indicates capability of DMA operations above 4GB.
+  @param  Operation             Indicates if the bus master is going to read or write to system memory.
+  @param  HostAddress           The system memory address to map to the DMA controller.
+  @param  NumberOfBytes         On input the number of bytes to map. On output the number of bytes
+                                that were mapped.
+  @param  DeviceAddress         The resulting map address for the bus master controller to use to
+                                access the hosts HostAddress.
+  @param  Mapping               A resulting value to pass to BmDmaUnmap().
+
+  @retval EFI_SUCCESS           The range was mapped for the returned NumberOfBytes.
+  @retval EFI_UNSUPPORTED       The HostAddress cannot be mapped as a common buffer.
+  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a lack of resources.
+  @retval EFI_DEVICE_ERROR      The system hardware could not map the requested address.
+
+**/
+EFI_STATUS
+EFIAPI
+BmDmaMap (
+  IN     BOOLEAN           DmaAbove4GB,
+  IN     BM_DMA_OPERATION  Operation,
+  IN     VOID              *HostAddress,
+  IN OUT UINTN             *NumberOfBytes,
+  OUT    PHYSICAL_ADDRESS  *DeviceAddress,
+  OUT    VOID              **Mapping
+  )
+{
+  EFI_PHYSICAL_ADDRESS  PhysicalAddress;
+
+  //
+  // Check for invalid inputs
+  //
+  if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL ||
+      Mapping == NULL || (UINT32) Operation >= DmaOperationBusMasterMaximum) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
+  if (DmaAbove4GB || (PhysicalAddress + *NumberOfBytes) <= SIZE_4GB) {
+    //
+    // If we CAN handle DMA above 4GB or the transfer is below 4GB,
+    // the DeviceAddress is simply the HostAddress
+    //
+    *DeviceAddress = PhysicalAddress;
+    *Mapping       = NO_MAPPING;
+
+    return EFI_SUCCESS;
+  } 
+
+  //
+  // If we cannot handle DMA above 4GB and any part of the DMA transfer
+  // being is above 4GB, then map the DMA transfer to a buffer below 4GB.
+  //
+  if (Operation == DmaOperationBusMasterCommonBuffer) {
+    //
+    // Common Buffer operations cannot be remapped, so return an error.
+    //
+    return EFI_UNSUPPORTED;
+  }
+
+  return AllocateBounceBuffer ( 
+           Operation,
+           FORCE_BELOW_4GB_TRUE,   
+           PhysicalAddress,
+           NumberOfBytes,
+           DeviceAddress,
+           Mapping
+           );
+}
+
+
+/**
+  Completes the DmaOperationBusMasterRead/Write/CommonBuffer operation
+  and releases any corresponding resources.
+
+  @param  Mapping               The mapping value returned from BmDmaMap().
+
+  @retval EFI_SUCCESS           The range was unmapped.
+  @retval EFI_DEVICE_ERROR      The data was not committed to the target system memory.
+
+**/
+EFI_STATUS
+EFIAPI
+BmDmaUnmap (
+  IN  VOID                 *Mapping
+  )
+{
+  MAP_INFO  *MapInfo;
+
+  //
+  // Check for invalid inputs
+  //
+  if (Mapping == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // See if the Map() operation associated with this Unmap() required a mapping
+  // buffer. If a mapping buffer was not required, then this function simply
+  // returns EFI_SUCCESS.
+  //
+  if (Mapping == NO_MAPPING) {
+    return EFI_SUCCESS;
+  }
+
+  //
+  // If this is a write operation from the Bus Master's point of view,
+  // then copy the contents of the mapped buffer into the real buffer
+  // so the processor can read the contents of the real buffer.
+  //
+  MapInfo = (MAP_INFO *)Mapping;
+  if (MapInfo->Operation == DmaOperationBusMasterWrite) {
+    CopyMem (
+      (VOID *) (UINTN) MapInfo->HostAddress,
+      (VOID *) (UINTN) MapInfo->MappedHostAddress,
+      MapInfo->NumberOfBytes
+      );
+  }
+
+  //
+  // Free the mapped buffer and the MAP_INFO structure.
+  //
+  gBS->FreePages (MapInfo->MappedHostAddress, MapInfo->NumberOfPages);
+  FreePool (Mapping);
+  return EFI_SUCCESS;
+}
+
+
+/**
+  Allocates pages that are suitable for a BmDmaMap() of type DmaOperationBusMasterCommonBuffer.
+
+  @param  DmaAbove4GB           Indicates capability of DMA operations above 4GB.
+  @param  MemoryType            The type of memory to allocate: EfiBootServicesData or
+                                EfiRuntimeServicesData.
+  @param  Pages                 The number of pages to allocate.
+  @param  HostAddress           A pointer to store the base system memory address of the
+                                allocated range.
+
+  @retval EFI_SUCCESS           The requested memory pages were allocated.
+  @retval EFI_UNSUPPORTED       Attributes is unsupported. The only legal attribute bits are
+                                MEMORY_WRITE_COMBINE and MEMORY_CACHED.
+  @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+  @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
+
+**/
+EFI_STATUS
+EFIAPI
+BmDmaAllocateBuffer (
+  IN  BOOLEAN              DmaAbove4GB,
+  IN  EFI_MEMORY_TYPE      MemoryType,
+  IN  UINTN                Pages,
+  OUT VOID                 **HostAddress
+  )
+{
+  EFI_STATUS           Status;
+  EFI_PHYSICAL_ADDRESS PhysicalAddress;
+  EFI_ALLOCATE_TYPE    AllocateType;
+
+  //
+  // Check for invalid inputs
+  //
+  if (HostAddress == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // The only valid memory types are EfiBootServicesData and
+  // EfiRuntimeServicesData
+  //
+  if (MemoryType != EfiBootServicesData &&
+      MemoryType != EfiRuntimeServicesData) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (DmaAbove4GB) {
+    AllocateType = AllocateAnyPages;
+  } else {
+    //
+    // Limit allocations to memory below 4GB
+    //
+    AllocateType    = AllocateMaxAddress;
+    PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (SIZE_4GB - 1);
+  }
+  Status = gBS->AllocatePages (
+                  AllocateType,
+                  MemoryType,
+                  Pages,
+                  &PhysicalAddress
+                  );
+  if (!EFI_ERROR (Status)) {
+    *HostAddress = (VOID *) (UINTN) PhysicalAddress;
+  }
+
+  return Status;
+}
+
+
+/**
+  Frees memory that was allocated with BmDmaAllocateBuffer().
+
+  @param  HostAddress           The base system memory address of the allocated range.
+  @param  Pages                 The number of pages to free.
+
+  @retval EFI_SUCCESS           The requested memory pages were freed.
+  @retval EFI_INVALID_PARAMETER The memory range specified by HostAddress and Pages
+                                was not allocated with BmDmaAllocateBuffer().
+
+**/
+EFI_STATUS
+EFIAPI
+BmDmaFreeBuffer (
+  IN  VOID                 *HostAddress,
+  IN  UINTN                Pages
+  )
+{
+  return gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress, Pages);
+}
+
diff --git a/MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf b/MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
new file mode 100644
index 0000000..4ddb27d
--- /dev/null
+++ b/MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
@@ -0,0 +1,41 @@
+## @file
+#
+# DMA abstraction library APIs. Based on PCI IO protocol DMA abstractions.
+#
+#  Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
+#  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = DxeBmDmaLib
+  FILE_GUID                      = daa403e0-071d-44ef-95cf-7f2472e4a4d5
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = BmDmaLib|DXE_DRIVER
+
+[Sources.common]
+  DxeBmDmaLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  DxeServicesTableLib
+  MemoryAllocationLib
+  UefiBootServicesTableLib
+
+
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 1bb361a..0c63197 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -3,6 +3,7 @@
 #
 # (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
 # Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
 #
 #    This program and the accompanying materials
 #    are licensed and made available under the terms and conditions of the BSD License
@@ -275,6 +276,7 @@
   MdeModulePkg/Core/Pei/PeiMain.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 
+  MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
   MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   MdeModulePkg/Library/UefiMemoryAllocationProfileLib/UefiMemoryAllocationProfileLib.inf
   MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
-- 
2.7.4

[PATCH v4 2/6] ArmVirtPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver

[Leo Duran]

This patch adds the new DxeBmDmaLib (BmDmaLib class) library, which
provides an abstraction layer for DMA operations implemented by the
PciHostBridgeDxe driver.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmVirtPkg/ArmVirtQemu.dsc       | 2 ++
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 615e1fc..04edf99 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -2,6 +2,7 @@
 #  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
 #  Copyright (c) 2014, Linaro Limited. All rights reserved.
 #  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -59,6 +60,7 @@
   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
   NorFlashPlatformLib|ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
 
+  BmDmaLib|MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
   PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index e490269..bc8701c 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -2,6 +2,7 @@
 #  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
 #  Copyright (c) 2014, Linaro Limited. All rights reserved.
 #  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -58,6 +59,7 @@
   TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
   NorFlashPlatformLib|ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf
 
+  BmDmaLib|MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
   PlatformBootManagerLib|ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
-- 
2.7.4

[PATCH v4 3/6] CorebootPayloadPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver

[Leo Duran]

This patch adds the new DxeBmDmaLib (BmDmaLib class) library, which
provides an abstraction layer for DMA operations implemented by the
PciHostBridgeDxe driver.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Prince Agyeman <prince.agyeman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc    | 3 +++
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index cdfcb75..59a599f 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -4,6 +4,8 @@
 # Provides drivers and definitions to create uefi payload for coreboot.
 #
 # Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+#
 # This program and the accompanying materials are licensed and made available under
 # the terms and conditions of the BSD License that accompanies this distribution.
 # The full text of the license may be found at
@@ -140,6 +142,7 @@
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
   PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
   CacheMaintenanceLib|MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
+  BmDmaLib|MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
 
   #
   # UEFI & PI
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
index 6b16af6..5177811 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
@@ -4,6 +4,8 @@
 # Provides drivers and definitions to create uefi payload for coreboot.
 #
 # Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+#
 # This program and the accompanying materials are licensed and made available under
 # the terms and conditions of the BSD License that accompanies this distribution.
 # The full text of the license may be found at
@@ -140,6 +142,7 @@
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
   PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
   CacheMaintenanceLib|MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
+  BmDmaLib|MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
 
   #
   # UEFI & PI
-- 
2.7.4

[PATCH v4 4/6] MdeModulePkg: Resolve BmDmaLib class for PciHostBridgeDxe driver

[Leo Duran]

This patch adds the new DxeBmDmaLib (BmDmaLib class) library, which
provides an abstraction layer for DMA operations implemented by the
PciHostBridgeDxe driver.

Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 MdeModulePkg/MdeModulePkg.dsc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 0c63197..d8f9a21 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -49,6 +49,7 @@
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
   PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
   SortLib|MdeModulePkg/Library/BaseSortLib/BaseSortLib.inf
+  BmDmaLib|MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
   #
   # UEFI & PI
   #
-- 
2.7.4

[PATCH v4 5/6] OvmfPkg: Resolve BmDmaLib class for PciHostBridgeDxe driver

[Leo Duran]

This patch adds the new DxeBmDmaLib (BmDmaLib class) library, which
provides an abstraction layer for DMA operations implemented by the
PciHostBridgeDxe driver.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran <leo.duran@amd.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 2 ++
 OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
 OvmfPkg/OvmfPkgX64.dsc     | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 0bce56b..345bbc7 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -3,6 +3,7 @@
 #
 #  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
+#  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -90,6 +91,7 @@
   UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
   HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
   SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
+  BmDmaLib|MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
   UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 56f7ff9..d798eb3 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -3,6 +3,7 @@
 #
 #  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
+#  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -95,6 +96,7 @@
   UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
   HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
   SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
+  BmDmaLib|MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
   UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index d0b0b0e..b751974 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -3,6 +3,7 @@
 #
 #  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
+#  Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -95,6 +96,7 @@
   UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
   HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
   SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
+  BmDmaLib|MdeModulePkg/Library/DxeBmDmaLib/DxeBmDmaLib.inf
   UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
   BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
-- 
2.7.4

[PATCH v4 6/6] MdeModulePkg: Modify PciHostBridgeDxe to use new BmDmaLib class library

[Leo Duran]

The BmDmaLib class library provides an abstraction layer for Bus-master DMA
operations as currently implemented by the PciHostBridgeDxe driver. The
intent is to allow override of the library as required by specific hardware
implementations, such as AMD's Secure Encrypted Virtualization (SEV).

Cc: Feng Tian <feng.tian@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
signed-off-by: Leo Duran <leo.duran@amd.com>
---
 .../Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf  |   1 +
 .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h       |  15 +-
 .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c     | 274 +++++----------------
 3 files changed, 70 insertions(+), 220 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
index d8b0439..35bb5c4 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
@@ -42,6 +42,7 @@
   BaseLib
   PciSegmentLib
   PciHostBridgeLib
+  BmDmaLib
 
 [Protocols]
   gEfiMetronomeArchProtocolGuid                   ## CONSUMES
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
index 13185b4..ca68bba 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
@@ -3,6 +3,8 @@
   The PCI Root Bridge header file.
 
 Copyright (c) 1999 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -34,6 +36,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/BaseLib.h>
 #include <Library/PciSegmentLib.h>
+#include <Library/BmDmaLib.h>
 #include "PciHostResource.h"
 
 
@@ -43,17 +46,6 @@ typedef enum {
   PciOperation
 } OPERATION_TYPE;
 
-#define MAP_INFO_SIGNATURE  SIGNATURE_32 ('_', 'm', 'a', 'p')
-typedef struct {
-  UINT32                                    Signature;
-  LIST_ENTRY                                Link;
-  EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION Operation;
-  UINTN                                     NumberOfBytes;
-  UINTN                                     NumberOfPages;
-  EFI_PHYSICAL_ADDRESS                      HostAddress;
-  EFI_PHYSICAL_ADDRESS                      MappedHostAddress;
-} MAP_INFO;
-#define MAP_INFO_FROM_LINK(a) CR (a, MAP_INFO, Link, MAP_INFO_SIGNATURE)
 
 #define PCI_ROOT_BRIDGE_SIGNATURE SIGNATURE_32 ('_', 'p', 'r', 'b')
 
@@ -79,7 +71,6 @@ typedef struct {
   EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL   RootBridgeIo;
 
   BOOLEAN                           ResourceSubmitted;
-  LIST_ENTRY                        Maps;
 } PCI_ROOT_BRIDGE_INSTANCE;
 
 #define ROOT_BRIDGE_FROM_THIS(a) CR (a, PCI_ROOT_BRIDGE_INSTANCE, RootBridgeIo, PCI_ROOT_BRIDGE_SIGNATURE)
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index 8af131b..7b861c7 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -3,6 +3,8 @@
   PCI Root Bridge Io Protocol code.
 
 Copyright (c) 1999 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
+
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include "PciRootBridge.h"
 #include "PciHostResource.h"
 
-#define NO_MAPPING  (VOID *) (UINTN) -1
 
 //
 // Lookup table for increment values based on transfer widths
@@ -55,6 +56,39 @@ UINT8 mOutStride[] = {
   0  // EfiPciWidthFillUint64
 };
 
+
+BM_DMA_OPERATION
+ConvertDmaOperation (
+  IN     EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION  RbOperation
+  )
+{
+  BM_DMA_OPERATION BmOperation;
+
+  switch (RbOperation) {
+  case EfiPciOperationBusMasterRead:
+  case EfiPciOperationBusMasterRead64:
+    BmOperation = DmaOperationBusMasterRead;
+    break;
+
+  case EfiPciOperationBusMasterWrite:
+  case EfiPciOperationBusMasterWrite64:
+    BmOperation = DmaOperationBusMasterWrite;
+    break;
+
+  case EfiPciOperationBusMasterCommonBuffer:
+  case EfiPciOperationBusMasterCommonBuffer64:
+    BmOperation = DmaOperationBusMasterCommonBuffer;
+    break;
+
+  default:
+    BmOperation = DmaOperationBusMasterMaximum;
+    break;
+  }
+
+  return BmOperation;
+}
+
+
 /**
   Construct the Pci Root Bridge instance.
 
@@ -168,7 +202,6 @@ CreateRootBridge (
     TypeMax * sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + sizeof (EFI_ACPI_END_TAG_DESCRIPTOR)
     );
   ASSERT (RootBridge->ConfigBuffer != NULL);
-  InitializeListHead (&RootBridge->Maps);
 
   CopyMem (&RootBridge->Bus, &Bridge->Bus, sizeof (PCI_ROOT_BRIDGE_APERTURE));
   CopyMem (&RootBridge->Io, &Bridge->Io, sizeof (PCI_ROOT_BRIDGE_APERTURE));
@@ -1053,118 +1086,27 @@ RootBridgeIoMap (
   OUT    VOID                                       **Mapping
   )
 {
-  EFI_STATUS                                        Status;
-  PCI_ROOT_BRIDGE_INSTANCE                          *RootBridge;
-  EFI_PHYSICAL_ADDRESS                              PhysicalAddress;
-  MAP_INFO                                          *MapInfo;
-
-  if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL ||
-      Mapping == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  //
-  // Make sure that Operation is valid
-  //
-  if ((UINT32) Operation >= EfiPciOperationMaximum) {
-    return EFI_INVALID_PARAMETER;
-  }
+  PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
+  BOOLEAN                  DmaAbove4GB;
+  BM_DMA_OPERATION         BmOperation;
 
   RootBridge = ROOT_BRIDGE_FROM_THIS (This);
-
-  PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
-  if ((!RootBridge->DmaAbove4G ||
-       (Operation != EfiPciOperationBusMasterRead64 &&
-        Operation != EfiPciOperationBusMasterWrite64 &&
-        Operation != EfiPciOperationBusMasterCommonBuffer64)) &&
-      ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) {
-
-    //
-    // If the root bridge or the device cannot handle performing DMA above
-    // 4GB but any part of the DMA transfer being mapped is above 4GB, then
-    // map the DMA transfer to a buffer below 4GB.
-    //
-
-    if (Operation == EfiPciOperationBusMasterCommonBuffer ||
-        Operation == EfiPciOperationBusMasterCommonBuffer64) {
-      //
-      // Common Buffer operations can not be remapped.  If the common buffer
-      // if above 4GB, then it is not possible to generate a mapping, so return
-      // an error.
-      //
-      return EFI_UNSUPPORTED;
-    }
-
-    //
-    // Allocate a MAP_INFO structure to remember the mapping when Unmap() is
-    // called later.
-    //
-    MapInfo = AllocatePool (sizeof (MAP_INFO));
-    if (MapInfo == NULL) {
-      *NumberOfBytes = 0;
-      return EFI_OUT_OF_RESOURCES;
-    }
-
-    //
-    // Initialize the MAP_INFO structure
-    //
-    MapInfo->Signature         = MAP_INFO_SIGNATURE;
-    MapInfo->Operation         = Operation;
-    MapInfo->NumberOfBytes     = *NumberOfBytes;
-    MapInfo->NumberOfPages     = EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes);
-    MapInfo->HostAddress       = PhysicalAddress;
-    MapInfo->MappedHostAddress = SIZE_4GB - 1;
-
-    //
-    // Allocate a buffer below 4GB to map the transfer to.
-    //
-    Status = gBS->AllocatePages (
-                    AllocateMaxAddress,
-                    EfiBootServicesData,
-                    MapInfo->NumberOfPages,
-                    &MapInfo->MappedHostAddress
-                    );
-    if (EFI_ERROR (Status)) {
-      FreePool (MapInfo);
-      *NumberOfBytes = 0;
-      return Status;
-    }
-
-    //
-    // If this is a read operation from the Bus Master's point of view,
-    // then copy the contents of the real buffer into the mapped buffer
-    // so the Bus Master can read the contents of the real buffer.
-    //
-    if (Operation == EfiPciOperationBusMasterRead ||
-        Operation == EfiPciOperationBusMasterRead64) {
-      CopyMem (
-        (VOID *) (UINTN) MapInfo->MappedHostAddress,
-        (VOID *) (UINTN) MapInfo->HostAddress,
-        MapInfo->NumberOfBytes
-        );
-    }
-
-    InsertTailList (&RootBridge->Maps, &MapInfo->Link);
-
-    //
-    // The DeviceAddress is the address of the maped buffer below 4GB
-    //
-    *DeviceAddress = MapInfo->MappedHostAddress;
-    //
-    // Return a pointer to the MAP_INFO structure in Mapping
-    //
-    *Mapping       = MapInfo;
-  } else {
-    //
-    // If the root bridge CAN handle performing DMA above 4GB or
-    // the transfer is below 4GB, so the DeviceAddress is simply the
-    // HostAddress
-    //
-    *DeviceAddress = PhysicalAddress;
-    *Mapping       = NO_MAPPING;
-  }
-
-  return EFI_SUCCESS;
+  DmaAbove4GB = RootBridge->DmaAbove4G && (
+                  Operation == EfiPciOperationBusMasterRead64 ||
+                  Operation == EfiPciOperationBusMasterWrite64 ||
+                  Operation == EfiPciOperationBusMasterCommonBuffer64
+                  );
+
+  BmOperation = ConvertDmaOperation (Operation);
+
+  return BmDmaMap (
+           DmaAbove4GB,
+           BmOperation,
+           HostAddress,
+           NumberOfBytes,
+           DeviceAddress,
+           Mapping
+           );
 }
 
 /**
@@ -1191,58 +1133,7 @@ RootBridgeIoUnmap (
   IN VOID                             *Mapping
   )
 {
-  MAP_INFO                 *MapInfo;
-  LIST_ENTRY               *Link;
-  PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
-
-  RootBridge = ROOT_BRIDGE_FROM_THIS (This);
-  //
-  // See if the Map() operation associated with this Unmap() required a mapping
-  // buffer. If a mapping buffer was not required, then this function simply
-  // returns EFI_SUCCESS.
-  //
-  if (Mapping == NO_MAPPING) {
-    return EFI_SUCCESS;
-  }
-
-  MapInfo = NO_MAPPING;
-  for (Link = GetFirstNode (&RootBridge->Maps)
-       ; !IsNull (&RootBridge->Maps, Link)
-       ; Link = GetNextNode (&RootBridge->Maps, Link)
-       ) {
-    MapInfo = MAP_INFO_FROM_LINK (Link);
-    if (MapInfo == Mapping) {
-      break;
-    }
-  }
-  //
-  // Mapping is not a valid value returned by Map()
-  //
-  if (MapInfo != Mapping) {
-    return EFI_INVALID_PARAMETER;
-  }
-  RemoveEntryList (&MapInfo->Link);
-
-  //
-  // If this is a write operation from the Bus Master's point of view,
-  // then copy the contents of the mapped buffer into the real buffer
-  // so the processor can read the contents of the real buffer.
-  //
-  if (MapInfo->Operation == EfiPciOperationBusMasterWrite ||
-      MapInfo->Operation == EfiPciOperationBusMasterWrite64) {
-    CopyMem (
-      (VOID *) (UINTN) MapInfo->HostAddress,
-      (VOID *) (UINTN) MapInfo->MappedHostAddress,
-      MapInfo->NumberOfBytes
-      );
-  }
-
-  //
-  // Free the mapped buffer and the MAP_INFO structure.
-  //
-  gBS->FreePages (MapInfo->MappedHostAddress, MapInfo->NumberOfPages);
-  FreePool (Mapping);
-  return EFI_SUCCESS;
+  return BmDmaUnmap (Mapping);
 }
 
 /**
@@ -1282,56 +1173,23 @@ RootBridgeIoAllocateBuffer (
   IN  UINT64                           Attributes
   )
 {
-  EFI_STATUS                Status;
-  EFI_PHYSICAL_ADDRESS      PhysicalAddress;
-  PCI_ROOT_BRIDGE_INSTANCE  *RootBridge;
-  EFI_ALLOCATE_TYPE         AllocateType;
+  PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
+  BOOLEAN                  DmaAbove4GB;
 
-  //
-  // Validate Attributes
-  //
   if ((Attributes & EFI_PCI_ATTRIBUTE_INVALID_FOR_ALLOCATE_BUFFER) != 0) {
     return EFI_UNSUPPORTED;
   }
 
-  //
-  // Check for invalid inputs
-  //
-  if (HostAddress == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  //
-  // The only valid memory types are EfiBootServicesData and
-  // EfiRuntimeServicesData
-  //
-  if (MemoryType != EfiBootServicesData &&
-      MemoryType != EfiRuntimeServicesData) {
-    return EFI_INVALID_PARAMETER;
-  }
-
   RootBridge = ROOT_BRIDGE_FROM_THIS (This);
+  DmaAbove4GB = RootBridge->DmaAbove4G &&
+                  (Attributes & EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE) != 0;
 
-  AllocateType = AllocateAnyPages;
-  if (!RootBridge->DmaAbove4G ||
-      (Attributes & EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) {
-    //
-    // Limit allocations to memory below 4GB
-    //
-    AllocateType    = AllocateMaxAddress;
-    PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (SIZE_4GB - 1);
-  }
-  Status = gBS->AllocatePages (
-                  AllocateType,
-                  MemoryType,
-                  Pages,
-                  &PhysicalAddress
-                  );
-  if (!EFI_ERROR (Status)) {
-    *HostAddress = (VOID *) (UINTN) PhysicalAddress;
-  }
-
-  return Status;
+  return BmDmaAllocateBuffer (
+           DmaAbove4GB,
+           MemoryType,
+           Pages,
+           HostAddress
+           );
 }
 
 /**
@@ -1356,7 +1214,7 @@ RootBridgeIoFreeBuffer (
   OUT VOID                             *HostAddress
   )
 {
-  return gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress, Pages);
+  return BmDmaFreeBuffer (HostAddress, Pages);
 }
 
 /**
-- 
2.7.4

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Ni, Ruiyu]

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Yao, Jiewen]

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Duran, Leo]

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Yao, Jiewen]

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Yao, Jiewen]

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Ni, Ruiyu]

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Yao, Jiewen]

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Ni, Ruiyu]

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Yao, Jiewen]

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Ni, Ruiyu]

Leo,
We haven't seen your DmaLib implementation for SEV feature.
Compare to the default PCI_ROOT_BRIDGE_IO behavior, does SevDmaLib need to return different host address from AllocateBuffer?
Does SevDmaLib need to return different device address from Map?
Or SevDmaLib just needs to know the returned address from PCI_ROOT_BRIDGE_IO?

If it's the latter case, a notification protocol is enough.

Regards,
Ray

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Duran, Leo]

Hi Ray,

Please see my replies below.
BTW, we plan on having the SEV BmDmaLib patches done in the next 2 weeks, so please stay tuned for that.

Thanks,
Leo

From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com]
Sent: Tuesday, March 07, 2017 7:02 PM
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

Leo,
We haven't seen your DmaLib implementation for SEV feature.
Compare to the default PCI_ROOT_BRIDGE_IO behavior, does SevDmaLib need to return different host address from AllocateBuffer?
[Duran, Leo] No, the returned address is the DMA base address just as it is now (i.e., the address returned by gBS->Allocatepages)

Does SevDmaLib need to return different device address from Map?
[Duran, Leo] That just depends on the DmaAbove4GB flag and the HostAddress range, as currently shown in the BmDmaLib patches.
That is, the SEV version of BmDmaLib will not need to alter that behavior as compared of the non-SEV version of BmDamLib.

Or SevDmaLib just needs to know the returned address from PCI_ROOT_BRIDGE_IO?
[Duran, Leo] I'm not sure I understand the question, but I'll try anyway...
SEV BmDmaLib uses the HostAddress to walk the page tables and configure (set/clear) the AddressressEncMask on page-table entries corresponding to the DMA buffer.

If it's the latter case, a notification protocol is enough.

Regards,
Ray

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Duran, Leo]

Ray, et al,

FYI: We found a bug in [PATCH v4 1/6]: BmDmaMap(), in DxeBmDmaLib.c
BUG: I'm calling AllocateBounceBuffer() with the first 2 parameters in incorrect order.

The bug-fix will be send out along with the SEV version of the library that you've requested (in the next week or so).

Thanks,
Leo.

From: Duran, Leo
Sent: Wednesday, March 08, 2017 10:05 AM
To: 'Ni, Ruiyu' <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@ml01.01.org
Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

Hi Ray,

Please see my replies below.
BTW, we plan on having the SEV BmDmaLib patches done in the next 2 weeks, so please stay tuned for that.

Thanks,
Leo

From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com]
Sent: Tuesday, March 07, 2017 7:02 PM
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

Leo,
We haven't seen your DmaLib implementation for SEV feature.
Compare to the default PCI_ROOT_BRIDGE_IO behavior, does SevDmaLib need to return different host address from AllocateBuffer?
[Duran, Leo] No, the returned address is the DMA base address just as it is now (i.e., the address returned by gBS->Allocatepages)

Does SevDmaLib need to return different device address from Map?
[Duran, Leo] That just depends on the DmaAbove4GB flag and the HostAddress range, as currently shown in the BmDmaLib patches.
That is, the SEV version of BmDmaLib will not need to alter that behavior as compared of the non-SEV version of BmDamLib.

Or SevDmaLib just needs to know the returned address from PCI_ROOT_BRIDGE_IO?
[Duran, Leo] I'm not sure I understand the question, but I'll try anyway...
SEV BmDmaLib uses the HostAddress to walk the page tables and configure (set/clear) the AddressressEncMask on page-table entries corresponding to the DMA buffer.

If it's the latter case, a notification protocol is enough.

Regards,
Ray

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Yao, Jiewen]

HI Leo
We just did an internal discussion.
I will send a proposal to define EDKII_IOMMU_PROTOCOL to abstract DMA operation. (Hope in next week)

Thank you
Yao Jiewen

From: Duran, Leo [mailto:leo.duran@amd.com]
Sent: Wednesday, March 15, 2017 5:22 AM
To: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; 'edk2-devel@ml01.01.org' <edk2-devel@ml01.01.org>
Subject: RE: [edk2] [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

Ray, et al,

FYI: We found a bug in [PATCH v4 1/6]: BmDmaMap(), in DxeBmDmaLib.c
BUG: I'm calling AllocateBounceBuffer() with the first 2 parameters in incorrect order.

The bug-fix will be send out along with the SEV version of the library that you've requested (in the next week or so).

Thanks,
Leo.

From: Duran, Leo
Sent: Wednesday, March 08, 2017 10:05 AM
To: 'Ni, Ruiyu' <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@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 Ray,

Please see my replies below.
BTW, we plan on having the SEV BmDmaLib patches done in the next 2 weeks, so please stay tuned for that.

Thanks,
Leo

From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com]
Sent: Tuesday, March 07, 2017 7:02 PM
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

Leo,
We haven't seen your DmaLib implementation for SEV feature.
Compare to the default PCI_ROOT_BRIDGE_IO behavior, does SevDmaLib need to return different host address from AllocateBuffer?
[Duran, Leo] No, the returned address is the DMA base address just as it is now (i.e., the address returned by gBS->Allocatepages)

Does SevDmaLib need to return different device address from Map?
[Duran, Leo] That just depends on the DmaAbove4GB flag and the HostAddress range, as currently shown in the BmDmaLib patches.
That is, the SEV version of BmDmaLib will not need to alter that behavior as compared of the non-SEV version of BmDamLib.

Or SevDmaLib just needs to know the returned address from PCI_ROOT_BRIDGE_IO?
[Duran, Leo] I'm not sure I understand the question, but I'll try anyway...
SEV BmDmaLib uses the HostAddress to walk the page tables and configure (set/clear) the AddressressEncMask on page-table entries corresponding to the DMA buffer.

If it's the latter case, a notification protocol is enough.

Regards,
Ray

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

Re: [PATCH v4 0/6] DxeBmDmaLib (BmDmaLib class) library

[Duran, Leo]

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