* [PATCH V5 1/3] MdeModulePkg/Include: Add IOMMU protocol definition.
2017-05-04 16:32 [PATCH V5 0/3] Add IOMMU support Jiewen Yao
@ 2017-05-04 16:32 ` Jiewen Yao
2017-05-04 16:32 ` [PATCH V5 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support Jiewen Yao
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Jiewen Yao @ 2017-05-04 16:32 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Leo Duran, Brijesh Singh, Ard Biesheuvel
This protocol is to abstract DMA access from IOMMU.
1) Intel "DMAR" ACPI table.
2) AMD "IVRS" ACPI table
3) ARM "IORT" ACPI table.
There might be multiple IOMMU engines on one platform.
For example, one for graphic and one for rest PCI devices
(such as ATA/USB).
All IOMMU engines are reported by one ACPI table.
All IOMMU protocol provider should be based upon ACPI table.
This single IOMMU protocol can handle multiple IOMMU engines on one system.
This IOMMU protocol provider can use UEFI device path to distinguish
if the device is graphic or ATA/USB, and find out corresponding
IOMMU engine.
The IOMMU protocol provides 2 capabilities:
A) Set DMA access attribute - such as write/read control.
B) Remap DMA memory - such as remap above 4GiB system memory address
to below 4GiB device address.
It provides AllocateBuffer/FreeBuffer/Map/Unmap for DMA memory.
The remapping can be static (fixed at build time) or dynamic (allocate
at runtime).
4) AMD "SEV" feature.
We can have an AMD SEV specific IOMMU driver to produce IOMMU protocol,
and manage SEV bit.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Leo Duran <leo.duran@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Include/Protocol/IoMmu.h | 259 ++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 +
2 files changed, 262 insertions(+)
diff --git a/MdeModulePkg/Include/Protocol/IoMmu.h b/MdeModulePkg/Include/Protocol/IoMmu.h
new file mode 100644
index 0000000..9d25c17
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/IoMmu.h
@@ -0,0 +1,259 @@
+/** @file
+ EFI IOMMU Protocol.
+
+Copyright (c) 2017, Intel Corporation. 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
+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.
+
+**/
+
+
+#ifndef __IOMMU_H__
+#define __IOMMU_H__
+
+//
+// IOMMU Protocol GUID value
+//
+#define EDKII_IOMMU_PROTOCOL_GUID \
+ { \
+ 0x4e939de9, 0xd948, 0x4b0f, { 0x88, 0xed, 0xe6, 0xe1, 0xce, 0x51, 0x7c, 0x1e } \
+ }
+
+//
+// Forward reference for pure ANSI compatability
+//
+typedef struct _EDKII_IOMMU_PROTOCOL EDKII_IOMMU_PROTOCOL;
+
+//
+// Revision The revision to which the IOMMU interface adheres.
+// All future revisions must be backwards compatible.
+// If a future version is not back wards compatible it is not the same GUID.
+//
+#define EDKII_IOMMU_PROTOCOL_REVISION 0x00010000
+
+//
+// IOMMU Access for SetAttribute
+//
+// These types can be "ORed" together as needed.
+// Any undefined bits are reserved and must be zero.
+//
+#define EDKII_IOMMU_ACCESS_READ 0x1
+#define EDKII_IOMMU_ACCESS_WRITE 0x2
+
+//
+// IOMMU Operation for Map
+//
+typedef enum {
+ ///
+ /// A read operation from system memory by a bus master that is not capable of producing
+ /// PCI dual address cycles.
+ ///
+ EdkiiIoMmuOperationBusMasterRead,
+ ///
+ /// A write operation from system memory by a bus master that is not capable of producing
+ /// PCI dual address cycles.
+ ///
+ EdkiiIoMmuOperationBusMasterWrite,
+ ///
+ /// Provides both read and write access to system memory by both the processor and a bus
+ /// master that is not capable of producing PCI dual address cycles.
+ ///
+ EdkiiIoMmuOperationBusMasterCommonBuffer,
+ ///
+ /// A read operation from system memory by a bus master that is capable of producing PCI
+ /// dual address cycles.
+ ///
+ EdkiiIoMmuOperationBusMasterRead64,
+ ///
+ /// A write operation to system memory by a bus master that is capable of producing PCI
+ /// dual address cycles.
+ ///
+ EdkiiIoMmuOperationBusMasterWrite64,
+ ///
+ /// Provides both read and write access to system memory by both the processor and a bus
+ /// master that is capable of producing PCI dual address cycles.
+ ///
+ EdkiiIoMmuOperationBusMasterCommonBuffer64,
+ EdkiiIoMmuOperationMaximum
+} EDKII_IOMMU_OPERATION;
+
+//
+// IOMMU attribute for AllocateBuffer
+// Any undefined bits are reserved and must be zero.
+//
+#define EDKII_IOMMU_ATTRIBUTE_MEMORY_WRITE_COMBINE 0x0080
+#define EDKII_IOMMU_ATTRIBUTE_MEMORY_CACHED 0x0800
+#define EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE 0x8000
+
+#define EDKII_IOMMU_ATTRIBUTE_VALID_FOR_ALLOCATE_BUFFER (EDKII_IOMMU_ATTRIBUTE_MEMORY_WRITE_COMBINE | EDKII_IOMMU_ATTRIBUTE_MEMORY_CACHED | EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
+
+#define EDKII_IOMMU_ATTRIBUTE_INVALID_FOR_ALLOCATE_BUFFER (~EDKII_IOMMU_ATTRIBUTE_VALID_FOR_ALLOCATE_BUFFER)
+
+/**
+ Set IOMMU attribute for a system memory.
+
+ If the IOMMU protocol exists, the system memory cannot be used
+ for DMA by default.
+
+ When a device requests a DMA access for a system memory,
+ the device driver need use SetAttribute() to update the IOMMU
+ attribute to request DMA access (read and/or write).
+
+ The DeviceHandle is used to identify which device submits the request.
+ The IOMMU implementation need translate the device path to an IOMMU device ID,
+ and set IOMMU hardware register accordingly.
+ 1) DeviceHandle can be a standard PCI device.
+ The memory for BusMasterRead need set EDKII_IOMMU_ACCESS_READ.
+ The memory for BusMasterWrite need set EDKII_IOMMU_ACCESS_WRITE.
+ The memory for BusMasterCommonBuffer need set EDKII_IOMMU_ACCESS_READ|EDKII_IOMMU_ACCESS_WRITE.
+ After the memory is used, the memory need set 0 to keep it being protected.
+ 2) DeviceHandle can be an ACPI device (ISA, I2C, SPI, etc).
+ The memory for DMA access need set EDKII_IOMMU_ACCESS_READ and/or EDKII_IOMMU_ACCESS_WRITE.
+
+ @param[in] This The protocol instance pointer.
+ @param[in] DeviceHandle The device who initiates the DMA access request.
+ @param[in] Mapping The mapping value returned from Map().
+ @param[in] IoMmuAccess The IOMMU access.
+
+ @retval EFI_SUCCESS The IoMmuAccess is set for the memory range specified by DeviceAddress and Length.
+ @retval EFI_INVALID_PARAMETER DeviceHandle is an invalid handle.
+ @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by Map().
+ @retval EFI_INVALID_PARAMETER IoMmuAccess specified an illegal combination of access.
+ @retval EFI_UNSUPPORTED DeviceHandle is unknown by the IOMMU.
+ @retval EFI_UNSUPPORTED The bit mask of IoMmuAccess is not supported by the IOMMU.
+ @retval EFI_UNSUPPORTED The IOMMU does not support the memory range specified by Mapping.
+ @retval EFI_OUT_OF_RESOURCES There are not enough resources available to modify the IOMMU access.
+ @retval EFI_DEVICE_ERROR The IOMMU device reported an error while attempting the operation.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_IOMMU_SET_ATTRIBUTE)(
+ IN EDKII_IOMMU_PROTOCOL *This,
+ IN EFI_HANDLE DeviceHandle,
+ IN VOID *Mapping,
+ IN UINT64 IoMmuAccess
+ );
+
+/**
+ Provides the controller-specific addresses required to access system memory from a
+ DMA bus master.
+
+ @param This The protocol instance pointer.
+ @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 PCI 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 PCI controller to use to
+ access the hosts HostAddress.
+ @param Mapping A resulting value to pass to Unmap().
+
+ @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.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_IOMMU_MAP)(
+ IN EDKII_IOMMU_PROTOCOL *This,
+ IN EDKII_IOMMU_OPERATION Operation,
+ IN VOID *HostAddress,
+ IN OUT UINTN *NumberOfBytes,
+ OUT EFI_PHYSICAL_ADDRESS *DeviceAddress,
+ OUT VOID **Mapping
+ );
+
+/**
+ Completes the Map() operation and releases any corresponding resources.
+
+ @param This The protocol instance pointer.
+ @param Mapping The mapping value returned from Map().
+
+ @retval EFI_SUCCESS The range was unmapped.
+ @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by Map().
+ @retval EFI_DEVICE_ERROR The data was not committed to the target system memory.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_IOMMU_UNMAP)(
+ IN EDKII_IOMMU_PROTOCOL *This,
+ IN VOID *Mapping
+ );
+
+/**
+ Allocates pages that are suitable for an OperationBusMasterCommonBuffer or
+ OperationBusMasterCommonBuffer64 mapping.
+
+ @param This The protocol instance pointer.
+ @param Type This parameter is not used and must be ignored.
+ @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.
+ @param Attributes The requested bit mask of attributes for 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.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_IOMMU_ALLOCATE_BUFFER)(
+ IN EDKII_IOMMU_PROTOCOL *This,
+ IN EFI_ALLOCATE_TYPE Type,
+ IN EFI_MEMORY_TYPE MemoryType,
+ IN UINTN Pages,
+ IN OUT VOID **HostAddress,
+ IN UINT64 Attributes
+ );
+
+/**
+ Frees memory that was allocated with AllocateBuffer().
+
+ @param This The protocol instance pointer.
+ @param Pages The number of pages to free.
+ @param HostAddress The base system memory address of the allocated range.
+
+ @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 AllocateBuffer().
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_IOMMU_FREE_BUFFER)(
+ IN EDKII_IOMMU_PROTOCOL *This,
+ IN UINTN Pages,
+ IN VOID *HostAddress
+ );
+
+///
+/// IOMMU Protocol structure.
+///
+struct _EDKII_IOMMU_PROTOCOL {
+ UINT64 Revision;
+ EDKII_IOMMU_SET_ATTRIBUTE SetAttribute;
+ EDKII_IOMMU_MAP Map;
+ EDKII_IOMMU_UNMAP Unmap;
+ EDKII_IOMMU_ALLOCATE_BUFFER AllocateBuffer;
+ EDKII_IOMMU_FREE_BUFFER FreeBuffer;
+};
+
+///
+/// IOMMU Protocol GUID variable.
+///
+extern EFI_GUID gEdkiiIoMmuProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 356b3e1..db596b6 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -540,6 +540,9 @@
## Include/Protocol/NonDiscoverableDevice.h
gEdkiiNonDiscoverableDeviceProtocolGuid = { 0x0d51905b, 0xb77e, 0x452a, {0xa2, 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } }
+ ## Include/Protocol/IoMmu.h
+ gEdkiiIoMmuProtocolGuid = { 0x4e939de9, 0xd948, 0x4b0f, { 0x88, 0xed, 0xe6, 0xe1, 0xce, 0x51, 0x7c, 0x1e } }
+
#
# [Error.gEfiMdeModulePkgTokenSpaceGuid]
# 0x80000001 | Invalid value provided.
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH V5 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support.
2017-05-04 16:32 [PATCH V5 0/3] Add IOMMU support Jiewen Yao
2017-05-04 16:32 ` [PATCH V5 1/3] MdeModulePkg/Include: Add IOMMU protocol definition Jiewen Yao
@ 2017-05-04 16:32 ` Jiewen Yao
2017-05-04 16:32 ` [PATCH V5 3/3] MdeModulePkg/PciBus: " Jiewen Yao
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Jiewen Yao @ 2017-05-04 16:32 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Leo Duran, Brijesh Singh, Ard Biesheuvel
If IOMMU protocol is installed, PciHostBridge just calls
IOMMU AllocateBuffer/FreeBuffer/Map/Unmap.
PciHostBridge does not set IOMMU access attribute,
because it does not know which device request the DMA.
This work is done by PciBus driver.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Leo Duran <leo.duran@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 37 ++++++++++++
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 2 +
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 2 +
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 61 ++++++++++++++++++++
4 files changed, 102 insertions(+)
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index 9005dee..70726a6 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -28,6 +28,10 @@ GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mPciResourceTypeStr[] = {
L"I/O", L"Mem", L"PMem", L"Mem64", L"PMem64", L"Bus"
};
+EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
+EFI_EVENT mIoMmuEvent;
+VOID *mIoMmuRegistration;
+
/**
Ensure the compatibility of an IO space descriptor with the IO aperture.
@@ -313,6 +317,28 @@ FreeMemorySpaceMap:
}
/**
+ Event notification that is fired when IOMMU protocol is installed.
+
+ @param Event The Event that is being processed.
+ @param Context Event Context.
+
+**/
+VOID
+EFIAPI
+IoMmuProtocolCallback (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_STATUS Status;
+
+ Status = gBS->LocateProtocol (&gEdkiiIoMmuProtocolGuid, NULL, (VOID **)&mIoMmuProtocol);
+ if (!EFI_ERROR(Status)) {
+ gBS->CloseEvent (mIoMmuEvent);
+ }
+}
+
+/**
Entry point of this driver.
@@ -489,6 +515,17 @@ InitializePciHostBridge (
ASSERT_EFI_ERROR (Status);
}
PciHostBridgeFreeRootBridges (RootBridges, RootBridgeCount);
+
+ if (!EFI_ERROR (Status)) {
+ mIoMmuEvent = EfiCreateProtocolNotifyEvent (
+ &gEdkiiIoMmuProtocolGuid,
+ TPL_CALLBACK,
+ IoMmuProtocolCallback,
+ NULL,
+ &mIoMmuRegistration
+ );
+ }
+
return Status;
}
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
index d8b0439..42bd8a2 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
@@ -41,6 +41,7 @@
BaseMemoryLib
BaseLib
PciSegmentLib
+ UefiLib
PciHostBridgeLib
[Protocols]
@@ -49,6 +50,7 @@
gEfiDevicePathProtocolGuid ## BY_START
gEfiPciRootBridgeIoProtocolGuid ## BY_START
gEfiPciHostBridgeResourceAllocationProtocolGuid ## BY_START
+ gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
[Depex]
gEfiCpuIo2ProtocolGuid AND
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
index 13185b4..1fec88b 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
@@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Protocol/CpuIo2.h>
#include <Protocol/DevicePath.h>
#include <Protocol/PciRootBridgeIo.h>
+#include <Protocol/IoMmu.h>
#include <Library/DebugLib.h>
#include <Library/DevicePathLib.h>
#include <Library/BaseMemoryLib.h>
@@ -34,6 +35,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/UefiLib.h>
#include "PciHostResource.h"
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index 8af131b..068295b 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -17,6 +17,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include "PciRootBridge.h"
#include "PciHostResource.h"
+extern EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
+
#define NO_MAPPING (VOID *) (UINTN) -1
//
@@ -1072,6 +1074,26 @@ RootBridgeIoMap (
RootBridge = ROOT_BRIDGE_FROM_THIS (This);
+ if (mIoMmuProtocol != NULL) {
+ if (!RootBridge->DmaAbove4G) {
+ //
+ // Clear 64bit support
+ //
+ if (Operation > EfiPciOperationBusMasterCommonBuffer) {
+ Operation = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) (Operation - EfiPciOperationBusMasterRead64);
+ }
+ }
+ Status = mIoMmuProtocol->Map (
+ mIoMmuProtocol,
+ Operation,
+ HostAddress,
+ NumberOfBytes,
+ DeviceAddress,
+ Mapping
+ );
+ return Status;
+ }
+
PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
if ((!RootBridge->DmaAbove4G ||
(Operation != EfiPciOperationBusMasterRead64 &&
@@ -1194,8 +1216,18 @@ RootBridgeIoUnmap (
MAP_INFO *MapInfo;
LIST_ENTRY *Link;
PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
+ EFI_STATUS Status;
+
+ if (mIoMmuProtocol != NULL) {
+ Status = mIoMmuProtocol->Unmap (
+ mIoMmuProtocol,
+ Mapping
+ );
+ return Status;
+ }
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
@@ -1312,6 +1344,24 @@ RootBridgeIoAllocateBuffer (
RootBridge = ROOT_BRIDGE_FROM_THIS (This);
+ if (mIoMmuProtocol != NULL) {
+ if (!RootBridge->DmaAbove4G) {
+ //
+ // Clear DUAL_ADDRESS_CYCLE
+ //
+ Attributes &= ~EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
+ }
+ Status = mIoMmuProtocol->AllocateBuffer (
+ mIoMmuProtocol,
+ Type,
+ MemoryType,
+ Pages,
+ HostAddress,
+ Attributes
+ );
+ return Status;
+ }
+
AllocateType = AllocateAnyPages;
if (!RootBridge->DmaAbove4G ||
(Attributes & EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) {
@@ -1356,6 +1406,17 @@ RootBridgeIoFreeBuffer (
OUT VOID *HostAddress
)
{
+ EFI_STATUS Status;
+
+ if (mIoMmuProtocol != NULL) {
+ Status = mIoMmuProtocol->FreeBuffer (
+ mIoMmuProtocol,
+ Pages,
+ HostAddress
+ );
+ return Status;
+ }
+
return gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress, Pages);
}
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH V5 3/3] MdeModulePkg/PciBus: Add IOMMU support.
2017-05-04 16:32 [PATCH V5 0/3] Add IOMMU support Jiewen Yao
2017-05-04 16:32 ` [PATCH V5 1/3] MdeModulePkg/Include: Add IOMMU protocol definition Jiewen Yao
2017-05-04 16:32 ` [PATCH V5 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support Jiewen Yao
@ 2017-05-04 16:32 ` Jiewen Yao
2017-05-05 1:32 ` [PATCH V5 0/3] " Ni, Ruiyu
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Jiewen Yao @ 2017-05-04 16:32 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Leo Duran, Brijesh Singh, Ard Biesheuvel
If IOMMU protocol is installed, PciBus need call IOMMU
to set access attribute for the PCI device in Map/Ummap.
Only after the access attribute is set, the PCI device can
access the DMA memory.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Leo Duran <leo.duran@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
---
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 9 ++++
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 47 ++++++++++++++++++--
4 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
index f3be47a..950cacc 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
@@ -42,6 +42,7 @@ UINT64 gAllZero = 0;
EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
+EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
GLOBAL_REMOVE_IF_UNREFERENCED EFI_PCI_HOTPLUG_REQUEST_PROTOCOL mPciHotPlugRequest = {
@@ -284,6 +285,14 @@ PciBusDriverBindingStart (
);
}
+ if (mIoMmuProtocol == NULL) {
+ gBS->LocateProtocol (
+ &gEdkiiIoMmuProtocolGuid,
+ NULL,
+ (VOID **) &mIoMmuProtocol
+ );
+ }
+
if (PcdGetBool (PcdPciDisableBusEnumeration)) {
gFullEnumeration = FALSE;
} else {
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 39ba8b9..3bcc134 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -32,6 +32,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Protocol/IncompatiblePciDeviceSupport.h>
#include <Protocol/PciOverride.h>
#include <Protocol/PciEnumerationComplete.h>
+#include <Protocol/IoMmu.h>
#include <Library/DebugLib.h>
#include <Library/UefiDriverEntryPoint.h>
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index a3ab11f..5da094f 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -95,6 +95,7 @@
gEfiPciRootBridgeIoProtocolGuid ## TO_START
gEfiIncompatiblePciDeviceSupportProtocolGuid ## SOMETIMES_CONSUMES
gEfiLoadFile2ProtocolGuid ## SOMETIMES_PRODUCES
+ gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport ## CONSUMES
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index f72598d..3b3b53a 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -14,6 +14,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include "PciBus.h"
+extern EDKII_IOMMU_PROTOCOL *mIoMmuProtocol;
+
//
// Pci Io Protocol Interface
//
@@ -965,8 +967,10 @@ PciIoMap (
OUT VOID **Mapping
)
{
- EFI_STATUS Status;
- PCI_IO_DEVICE *PciIoDevice;
+ EFI_STATUS Status;
+ PCI_IO_DEVICE *PciIoDevice;
+ UINT64 IoMmuAttribute;
+ EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION RootBridgeIoOperation;
PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
@@ -978,13 +982,14 @@ PciIoMap (
return EFI_INVALID_PARAMETER;
}
+ RootBridgeIoOperation = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION)Operation;
if ((PciIoDevice->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) != 0) {
- Operation = (EFI_PCI_IO_PROTOCOL_OPERATION) (Operation + EfiPciOperationBusMasterRead64);
+ RootBridgeIoOperation = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION)(Operation + EfiPciOperationBusMasterRead64);
}
Status = PciIoDevice->PciRootBridgeIo->Map (
PciIoDevice->PciRootBridgeIo,
- (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) Operation,
+ RootBridgeIoOperation,
HostAddress,
NumberOfBytes,
DeviceAddress,
@@ -999,6 +1004,31 @@ PciIoMap (
);
}
+ if (mIoMmuProtocol != NULL) {
+ if (!EFI_ERROR (Status)) {
+ switch (Operation) {
+ case EfiPciIoOperationBusMasterRead:
+ IoMmuAttribute = EDKII_IOMMU_ACCESS_READ;
+ break;
+ case EfiPciIoOperationBusMasterWrite:
+ IoMmuAttribute = EDKII_IOMMU_ACCESS_WRITE;
+ break;
+ case EfiPciIoOperationBusMasterCommonBuffer:
+ IoMmuAttribute = EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE;
+ break;
+ default:
+ ASSERT(FALSE);
+ return EFI_INVALID_PARAMETER;
+ }
+ mIoMmuProtocol->SetAttribute (
+ mIoMmuProtocol,
+ PciIoDevice->Handle,
+ *Mapping,
+ IoMmuAttribute
+ );
+ }
+ }
+
return Status;
}
@@ -1024,6 +1054,15 @@ PciIoUnmap (
PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
+ if (mIoMmuProtocol != NULL) {
+ mIoMmuProtocol->SetAttribute (
+ mIoMmuProtocol,
+ PciIoDevice->Handle,
+ Mapping,
+ 0
+ );
+ }
+
Status = PciIoDevice->PciRootBridgeIo->Unmap (
PciIoDevice->PciRootBridgeIo,
Mapping
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH V5 0/3] Add IOMMU support.
2017-05-04 16:32 [PATCH V5 0/3] Add IOMMU support Jiewen Yao
` (2 preceding siblings ...)
2017-05-04 16:32 ` [PATCH V5 3/3] MdeModulePkg/PciBus: " Jiewen Yao
@ 2017-05-05 1:32 ` Ni, Ruiyu
2017-05-11 20:51 ` Jordan Justen
2017-05-13 19:27 ` Duran, Leo
5 siblings, 0 replies; 10+ messages in thread
From: Ni, Ruiyu @ 2017-05-05 1:32 UTC (permalink / raw)
To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Leo Duran, Ard Biesheuvel
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Thanks/Ray
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jiewen Yao
> Sent: Friday, May 5, 2017 12:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Leo Duran <leo.duran@amd.com>; Ard
> Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [edk2] [PATCH V5 0/3] Add IOMMU support.
>
> ================ V5 ==============
> Minor update from V4.
>
> 1) Remove unused SetAttribute() API in IOMMU protocol.
> (Feedback from Ruiyu and Ard)
> 2) Rename SetMappingAttribute() to SetAttribute().
> (Feedback from Ruiyu)
> 3) Fix the bug in PciBus driver for Operation (Thanks to Ard to catch it)
>
> V4:
> Tested-by: Brijesh Singh <brijesh.singh@amd.com> With the issue in 3/3
> addressed:
> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> ================ V4 ==============
> Refine the EDKII_IOMMU_PROTOCOL.
>
> 1) Add AllocateBuffer/FreeBuffer/Map/Unmap() API.
> They are similar to DmaLib in EmbeddedPkg and similar to the previous
> BmDmaLib (by leo.duran@amd.com).
>
> These APIs are invoked by PciHostBridge driver to allocate DMA memory.
>
> The PciHostBridge driver (IOMMU consumer) is simplified:
> It uses IOMMU, if IOMMU protocol is present.
> Else it uses original logic.
>
> 2) Add SetMappingAttribute() API.
> It is similar to SetAttribute() API in V1.
>
> This API is invoked by PciBus driver to set DMA access attribute (read/write) for
> device.
>
> The PciBus driver (IOMMU consumer) is simplified:
> It sets access attribute in Map/Unmap,
> if IOMMU protocol is present.
>
> 3) Remove SetRemapAddress/GetRemapAddress() API.
> Because PciHostBridge/PciBus can call the APIs defined above, there is no need
> to provide remap capability.
>
> -- Sample producer drivers:
> 1) The sample VTd driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/IntelVTdDxe
>
> It is added to show the concept. It is not fully implemented yet.
> It will not be checked in in this patch.
>
> 2) The sample AMD SEV driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleAmdSevDx
> e
> (code is borrowed from leo.duran@amd.com and brijesh.singh@amd.com)
>
> This is not a right place to put this driver.
>
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
>
> 3) The sample STYX driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleStyxDxe
> (code is borrowed from ard.biesheuvel@linaro.org)
>
> This is not a right place to put this driver.
>
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
>
>
> ================ V3 ==============
> 1) Add Remap capability (from Ard Biesheuvel) Add
> EDKII_IOMMU_REMAP_ADDRESS API in IOMMU_PROTOCOL.
>
> NOTE: The code is not fully validated yet.
> The purpose is to collect feedback to decide the next step.
>
> ================ V2 ==============
> 1) Enhance Unmap() in PciIo (From Ruiyu Ni) Maintain a local list of MapInfo and
> match it in Unmap.
>
> 2) CopyMem for ReadOperation in PciIo after SetAttribute (Leo Duran) Fix a bug
> in V1 that copy mem for read happen before SetAttribute, which will break AMD
> SEV solution.
>
> ================ V1 ==============
>
> This patch series adds IOMMU protocol and updates the consumer to support
> IOMMU based DMA access in UEFI.
>
> This patch series can support the BmDmaLib request for AMD SEV.
> submitted by Duran, Leo <leo.duran@amd.com> and Brijesh Singh
> <brijesh.ksingh@gmail.com>.
> https://lists.01.org/pipermail/edk2-devel/2017-March/008109.html, and
> https://lists.01.org/pipermail/edk2-devel/2017-March/008820.html.
> We can have an AMD SEV specific IOMMU driver to produce IOMMU protocol,
> and clear SEV in IOMMU->SetAttribute().
>
> This patch series can also support Intel VTd based DMA protection, requested by
> Jiewen Yao <jiewen.yao@intel.com>, discussed in
> https://lists.01.org/pipermail/edk2-devel/2017-March/008157.html.
> We can have an Intel VTd specific IOMMU driver to produce IOMMU protocol,
> and update VTd engine to grant or deny access in IOMMU->SetAttribute().
>
> This patch series does not provide a full Intel VTd driver, which will be provide in
> other patch in the future.
>
> The purpose of this patch series to review if this IOMMU protocol design can
> meet all DMA access and management requirement.
>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Leo Duran <leo.duran@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>
> Jiewen Yao (3):
> MdeModulePkg/Include: Add IOMMU protocol definition.
> MdeModulePkg/PciHostBridge: Add IOMMU support.
> MdeModulePkg/PciBus: Add IOMMU support.
>
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 9 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 47 +++-
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 37 +++
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 2 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 2 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 61 +++++
> MdeModulePkg/Include/Protocol/IoMmu.h | 259
> ++++++++++++++++++++
> MdeModulePkg/MdeModulePkg.dec | 3 +
> 10 files changed, 418 insertions(+), 4 deletions(-) create mode 100644
> MdeModulePkg/Include/Protocol/IoMmu.h
>
> --
> 2.7.4.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH V5 0/3] Add IOMMU support.
2017-05-04 16:32 [PATCH V5 0/3] Add IOMMU support Jiewen Yao
` (3 preceding siblings ...)
2017-05-05 1:32 ` [PATCH V5 0/3] " Ni, Ruiyu
@ 2017-05-11 20:51 ` Jordan Justen
2017-05-12 1:36 ` Yao, Jiewen
2017-05-13 19:27 ` Duran, Leo
5 siblings, 1 reply; 10+ messages in thread
From: Jordan Justen @ 2017-05-11 20:51 UTC (permalink / raw)
To: Jiewen Yao, edk2-devel
Cc: Ruiyu Ni, Leo Duran, Ard Biesheuvel, Brijesh Singh, Laszlo Ersek
This design seems to force a platform the use APRIORI to provide the
IoMmu protocol. Isn't this something we try to avoid?
One thought is that the PciHostBridge driver could add a depex on
IoMmu protocol conditionally based on a feature PCD, right?
Unfortunately, in this case it seems that the IoMmu protocol is
installed conditionally at runtime.
One thought I had was, could OVMF set the PCD to force an IoMmu
dependency, and then install a no-op IoMmu protocol instance if AMD's
SEV is not detected?
Another concern I have is that the IoMmu protocol causes big chunks of
the PciHostBridge Map/Unmap implementation to be skipped. Is this
potentially bypassing something important?
-Jordan
On 2017-05-04 09:32:38, Jiewen Yao wrote:
> ================ V5 ==============
> Minor update from V4.
>
> 1) Remove unused SetAttribute() API in IOMMU protocol.
> (Feedback from Ruiyu and Ard)
> 2) Rename SetMappingAttribute() to SetAttribute().
> (Feedback from Ruiyu)
> 3) Fix the bug in PciBus driver for Operation
> (Thanks to Ard to catch it)
>
> V4:
> Tested-by: Brijesh Singh <brijesh.singh@amd.com>
> With the issue in 3/3 addressed:
> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> ================ V4 ==============
> Refine the EDKII_IOMMU_PROTOCOL.
>
> 1) Add AllocateBuffer/FreeBuffer/Map/Unmap() API.
> They are similar to DmaLib in EmbeddedPkg and
> similar to the previous BmDmaLib (by leo.duran@amd.com).
>
> These APIs are invoked by PciHostBridge driver
> to allocate DMA memory.
>
> The PciHostBridge driver (IOMMU consumer) is simplified:
> It uses IOMMU, if IOMMU protocol is present.
> Else it uses original logic.
>
> 2) Add SetMappingAttribute() API.
> It is similar to SetAttribute() API in V1.
>
> This API is invoked by PciBus driver to set DMA
> access attribute (read/write) for device.
>
> The PciBus driver (IOMMU consumer) is simplified:
> It sets access attribute in Map/Unmap,
> if IOMMU protocol is present.
>
> 3) Remove SetRemapAddress/GetRemapAddress() API.
> Because PciHostBridge/PciBus can call the APIs defined
> above, there is no need to provide remap capability.
>
> -- Sample producer drivers:
> 1) The sample VTd driver (IOMMU producer)
> is at https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/IntelVTdDxe
>
> It is added to show the concept. It is not fully implemented yet.
> It will not be checked in in this patch.
>
> 2) The sample AMD SEV driver (IOMMU producer)
> is at https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleAmdSevDxe
> (code is borrowed from leo.duran@amd.com and brijesh.singh@amd.com)
>
> This is not a right place to put this driver.
>
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
>
> 3) The sample STYX driver (IOMMU producer)
> is at https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleStyxDxe
> (code is borrowed from ard.biesheuvel@linaro.org)
>
> This is not a right place to put this driver.
>
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
>
>
> ================ V3 ==============
> 1) Add Remap capability (from Ard Biesheuvel)
> Add EDKII_IOMMU_REMAP_ADDRESS API in IOMMU_PROTOCOL.
>
> NOTE: The code is not fully validated yet.
> The purpose is to collect feedback to decide the next step.
>
> ================ V2 ==============
> 1) Enhance Unmap() in PciIo (From Ruiyu Ni)
> Maintain a local list of MapInfo and match it in Unmap.
>
> 2) CopyMem for ReadOperation in PciIo after SetAttribute (Leo Duran)
> Fix a bug in V1 that copy mem for read happen before SetAttribute,
> which will break AMD SEV solution.
>
> ================ V1 ==============
>
> This patch series adds IOMMU protocol and updates the consumer
> to support IOMMU based DMA access in UEFI.
>
> This patch series can support the BmDmaLib request for AMD SEV.
> submitted by Duran, Leo <leo.duran@amd.com> and Brijesh Singh <brijesh.ksingh@gmail.com>.
> https://lists.01.org/pipermail/edk2-devel/2017-March/008109.html, and
> https://lists.01.org/pipermail/edk2-devel/2017-March/008820.html.
> We can have an AMD SEV specific IOMMU driver to produce IOMMU protocol,
> and clear SEV in IOMMU->SetAttribute().
>
> This patch series can also support Intel VTd based DMA protection,
> requested by Jiewen Yao <jiewen.yao@intel.com>, discussed in
> https://lists.01.org/pipermail/edk2-devel/2017-March/008157.html.
> We can have an Intel VTd specific IOMMU driver to produce IOMMU protocol,
> and update VTd engine to grant or deny access in IOMMU->SetAttribute().
>
> This patch series does not provide a full Intel VTd driver, which
> will be provide in other patch in the future.
>
> The purpose of this patch series to review if this IOMMU protocol design
> can meet all DMA access and management requirement.
>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Leo Duran <leo.duran@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>
> Jiewen Yao (3):
> MdeModulePkg/Include: Add IOMMU protocol definition.
> MdeModulePkg/PciHostBridge: Add IOMMU support.
> MdeModulePkg/PciBus: Add IOMMU support.
>
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 9 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 47 +++-
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 37 +++
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 2 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 2 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 61 +++++
> MdeModulePkg/Include/Protocol/IoMmu.h | 259 ++++++++++++++++++++
> MdeModulePkg/MdeModulePkg.dec | 3 +
> 10 files changed, 418 insertions(+), 4 deletions(-)
> create mode 100644 MdeModulePkg/Include/Protocol/IoMmu.h
>
> --
> 2.7.4.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH V5 0/3] Add IOMMU support.
2017-05-11 20:51 ` Jordan Justen
@ 2017-05-12 1:36 ` Yao, Jiewen
0 siblings, 0 replies; 10+ messages in thread
From: Yao, Jiewen @ 2017-05-12 1:36 UTC (permalink / raw)
To: Justen, Jordan L, edk2-devel@lists.01.org
Cc: Ni, Ruiyu, Leo Duran, Ard Biesheuvel, Brijesh Singh, Laszlo Ersek
Good question.
Comments below:
From: Justen, Jordan L
Sent: Friday, May 12, 2017 4:52 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Leo Duran <leo.duran@amd.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Brijesh Singh <brijesh.singh@amd.com>; Laszlo Ersek <lersek@redhat.com>
Subject: Re: [edk2] [PATCH V5 0/3] Add IOMMU support.
This design seems to force a platform the use APRIORI to provide the
IoMmu protocol. Isn't this something we try to avoid?
[Jiewen] I am confused. This design does not force a platform using APRIORI.
I assume you are talking PciHostBridge driver implementation.
We used protocol callback for IOMMU protocol in PCI HostBridge driver.
For PciHostBridge driver, IOMMU can be dispatched before or after PciHostBridge driver.
For PciBus driver, it is a driver model driver, Start() function is called by BDS.
IOMMU is installed in DXE phase. IOMMU is installed before Pci.Start().
For X86 VTd IOMMU platform, we need PCI bus driver or host bridge driver calls IOMMU in BDS.
That is enough. We do not need APRIORI.
If a platform wants to dispatch IOMMU earlier, that is OK.
But that is the platform choice, not enforcement.
On the other hand, enforcing IOMMU dependency means any platform MUST add a new IOMMU driver,
if this platform is using the open source PciHostBridge driver.
That is the main reason, I did not want to force IOMMU dependency.
I do not want to bring an incompatible change for existing platform.
One thought is that the PciHostBridge driver could add a depex on
IoMmu protocol conditionally based on a feature PCD, right?
Unfortunately, in this case it seems that the IoMmu protocol is
installed conditionally at runtime.
One thought I had was, could OVMF set the PCD to force an IoMmu
dependency, and then install a no-op IoMmu protocol instance if AMD's
SEV is not detected?
[Jiewen] APRIORI is defined by PI specification vol 2 DXE CIS - 10.3 The A Priori File
====================
The a priori file provides a deterministic execution order of DXE drivers. DXE drivers that are executed solely based on their dependency expression are weakly ordered. This means that the execution order is not completely deterministic between boots or between platforms. There are cases where a deterministic execution order is required.
……
The main purpose of the a priori file is to provide a greater degree of flexibility in the firmware design of a platform.
====================
This is industry standard. And this is the platform choice.
Looking at EDKII, we are already using that:
C:\home\Edk-II\OvmfPkg\OvmfPkgIa32.fdf(149):APRIORI PEI {
C:\home\Edk-II\OvmfPkg\OvmfPkgIa32.fdf(190):APRIORI DXE {
C:\home\Edk-II\OvmfPkg\OvmfPkgIa32X64.fdf(149):APRIORI PEI {
C:\home\Edk-II\OvmfPkg\OvmfPkgIa32X64.fdf(190):APRIORI DXE {
C:\home\Edk-II\OvmfPkg\OvmfPkgX64.fdf(149):APRIORI PEI {
C:\home\Edk-II\OvmfPkg\OvmfPkgX64.fdf(190):APRIORI DXE {
C:\home\Edk-II\QuarkPlatformPkg\Quark.fdf(306):APRIORI PEI {
C:\home\Edk-II\QuarkPlatformPkg\QuarkMin.fdf(306):APRIORI PEI {
C:\home\Edk-II\Vlv2TbltDevicePkg\PlatformPkg.fdf(441):APRIORI DXE {
C:\home\Edk-II\Vlv2TbltDevicePkg\PlatformPkgGcc.fdf(398):APRIORI DXE {
C:\home\EdkIIGit\edk2\Nt32Pkg\Nt32Pkg.fdf(164):APRIORI PEI {
C:\home\EdkIIGit\edk2\Nt32Pkg\Nt32Pkg.fdf(170):APRIORI DXE {
The APRIORI is added when the platform is created.
APRIORI is much simpler than a no-op IOMMU, from my point of view.
I do not have concern if a platform wants to use dependency. (Again, that is platform choice)
But at same time, may I know what is the concern if a platform wants to use APRIORI?
Another concern I have is that the IoMmu protocol causes big chunks of
the PciHostBridge Map/Unmap implementation to be skipped. Is this
potentially bypassing something important?
[Jiewen] No.
-Jordan
On 2017-05-04 09:32:38, Jiewen Yao wrote:
> ================ V5 ==============
> Minor update from V4.
>
> 1) Remove unused SetAttribute() API in IOMMU protocol.
> (Feedback from Ruiyu and Ard)
> 2) Rename SetMappingAttribute() to SetAttribute().
> (Feedback from Ruiyu)
> 3) Fix the bug in PciBus driver for Operation
> (Thanks to Ard to catch it)
>
> V4:
> Tested-by: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>
> With the issue in 3/3 addressed:
> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
>
> ================ V4 ==============
> Refine the EDKII_IOMMU_PROTOCOL.
>
> 1) Add AllocateBuffer/FreeBuffer/Map/Unmap() API.
> They are similar to DmaLib in EmbeddedPkg and
> similar to the previous BmDmaLib (by leo.duran@amd.com<mailto:leo.duran@amd.com>).
>
> These APIs are invoked by PciHostBridge driver
> to allocate DMA memory.
>
> The PciHostBridge driver (IOMMU consumer) is simplified:
> It uses IOMMU, if IOMMU protocol is present.
> Else it uses original logic.
>
> 2) Add SetMappingAttribute() API.
> It is similar to SetAttribute() API in V1.
>
> This API is invoked by PciBus driver to set DMA
> access attribute (read/write) for device.
>
> The PciBus driver (IOMMU consumer) is simplified:
> It sets access attribute in Map/Unmap,
> if IOMMU protocol is present.
>
> 3) Remove SetRemapAddress/GetRemapAddress() API.
> Because PciHostBridge/PciBus can call the APIs defined
> above, there is no need to provide remap capability.
>
> -- Sample producer drivers:
> 1) The sample VTd driver (IOMMU producer)
> is at https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/IntelVTdDxe
>
> It is added to show the concept. It is not fully implemented yet.
> It will not be checked in in this patch.
>
> 2) The sample AMD SEV driver (IOMMU producer)
> is at https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleAmdSevDxe
> (code is borrowed from leo.duran@amd.com<mailto:leo.duran@amd.com> and brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>)
>
> This is not a right place to put this driver.
>
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
>
> 3) The sample STYX driver (IOMMU producer)
> is at https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleStyxDxe
> (code is borrowed from ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>)
>
> This is not a right place to put this driver.
>
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
>
>
> ================ V3 ==============
> 1) Add Remap capability (from Ard Biesheuvel)
> Add EDKII_IOMMU_REMAP_ADDRESS API in IOMMU_PROTOCOL.
>
> NOTE: The code is not fully validated yet.
> The purpose is to collect feedback to decide the next step.
>
> ================ V2 ==============
> 1) Enhance Unmap() in PciIo (From Ruiyu Ni)
> Maintain a local list of MapInfo and match it in Unmap.
>
> 2) CopyMem for ReadOperation in PciIo after SetAttribute (Leo Duran)
> Fix a bug in V1 that copy mem for read happen before SetAttribute,
> which will break AMD SEV solution.
>
> ================ V1 ==============
>
> This patch series adds IOMMU protocol and updates the consumer
> to support IOMMU based DMA access in UEFI.
>
> This patch series can support the BmDmaLib request for AMD SEV.
> submitted by Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>> and Brijesh Singh <brijesh.ksingh@gmail.com<mailto:brijesh.ksingh@gmail.com>>.
> https://lists.01.org/pipermail/edk2-devel/2017-March/008109.html, and
> https://lists.01.org/pipermail/edk2-devel/2017-March/008820.html.
> We can have an AMD SEV specific IOMMU driver to produce IOMMU protocol,
> and clear SEV in IOMMU->SetAttribute().
>
> This patch series can also support Intel VTd based DMA protection,
> requested by Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>, discussed in
> https://lists.01.org/pipermail/edk2-devel/2017-March/008157.html.
> We can have an Intel VTd specific IOMMU driver to produce IOMMU protocol,
> and update VTd engine to grant or deny access in IOMMU->SetAttribute().
>
> This patch series does not provide a full Intel VTd driver, which
> will be provide in other patch in the future.
>
> The purpose of this patch series to review if this IOMMU protocol design
> can meet all DMA access and management requirement.
>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>
> Cc: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>>
> Cc: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
>
> Jiewen Yao (3):
> MdeModulePkg/Include: Add IOMMU protocol definition.
> MdeModulePkg/PciHostBridge: Add IOMMU support.
> MdeModulePkg/PciBus: Add IOMMU support.
>
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 9 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 47 +++-
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 37 +++
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 2 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 2 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 61 +++++
> MdeModulePkg/Include/Protocol/IoMmu.h | 259 ++++++++++++++++++++
> MdeModulePkg/MdeModulePkg.dec | 3 +
> 10 files changed, 418 insertions(+), 4 deletions(-)
> create mode 100644 MdeModulePkg/Include/Protocol/IoMmu.h
>
> --
> 2.7.4.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V5 0/3] Add IOMMU support.
2017-05-04 16:32 [PATCH V5 0/3] Add IOMMU support Jiewen Yao
` (4 preceding siblings ...)
2017-05-11 20:51 ` Jordan Justen
@ 2017-05-13 19:27 ` Duran, Leo
2017-05-18 8:37 ` Ard Biesheuvel
5 siblings, 1 reply; 10+ messages in thread
From: Duran, Leo @ 2017-05-13 19:27 UTC (permalink / raw)
To: Jiewen Yao, edk2-devel@lists.01.org
Cc: Ruiyu Ni, Singh, Brijesh, Ard Biesheuvel
Reviewed-by: Leo Duran <leo.duran@amd.com>
Thanks,
Leo.
> -----Original Message-----
> From: Jiewen Yao [mailto:jiewen.yao@intel.com]
> Sent: Thursday, May 04, 2017 11:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>; Duran, Leo <leo.duran@amd.com>;
> Singh, Brijesh <brijesh.singh@amd.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: [PATCH V5 0/3] Add IOMMU support.
>
> ================ V5 ==============
> Minor update from V4.
>
> 1) Remove unused SetAttribute() API in IOMMU protocol.
> (Feedback from Ruiyu and Ard)
> 2) Rename SetMappingAttribute() to SetAttribute().
> (Feedback from Ruiyu)
> 3) Fix the bug in PciBus driver for Operation (Thanks to Ard to catch it)
>
> V4:
> Tested-by: Brijesh Singh <brijesh.singh@amd.com> With the issue in 3/3
> addressed:
> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> ================ V4 ==============
> Refine the EDKII_IOMMU_PROTOCOL.
>
> 1) Add AllocateBuffer/FreeBuffer/Map/Unmap() API.
> They are similar to DmaLib in EmbeddedPkg and similar to the previous
> BmDmaLib (by leo.duran@amd.com).
>
> These APIs are invoked by PciHostBridge driver to allocate DMA memory.
>
> The PciHostBridge driver (IOMMU consumer) is simplified:
> It uses IOMMU, if IOMMU protocol is present.
> Else it uses original logic.
>
> 2) Add SetMappingAttribute() API.
> It is similar to SetAttribute() API in V1.
>
> This API is invoked by PciBus driver to set DMA access attribute (read/write)
> for device.
>
> The PciBus driver (IOMMU consumer) is simplified:
> It sets access attribute in Map/Unmap,
> if IOMMU protocol is present.
>
> 3) Remove SetRemapAddress/GetRemapAddress() API.
> Because PciHostBridge/PciBus can call the APIs defined above, there is no
> need to provide remap capability.
>
> -- Sample producer drivers:
> 1) The sample VTd driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/IntelVTdDxe
>
> It is added to show the concept. It is not fully implemented yet.
> It will not be checked in in this patch.
>
> 2) The sample AMD SEV driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleAmdSe
> vDxe
> (code is borrowed from leo.duran@amd.com and brijesh.singh@amd.com)
>
> This is not a right place to put this driver.
>
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
>
> 3) The sample STYX driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleStyxDx
> e
> (code is borrowed from ard.biesheuvel@linaro.org)
>
> This is not a right place to put this driver.
>
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
>
>
> ================ V3 ==============
> 1) Add Remap capability (from Ard Biesheuvel) Add
> EDKII_IOMMU_REMAP_ADDRESS API in IOMMU_PROTOCOL.
>
> NOTE: The code is not fully validated yet.
> The purpose is to collect feedback to decide the next step.
>
> ================ V2 ==============
> 1) Enhance Unmap() in PciIo (From Ruiyu Ni) Maintain a local list of MapInfo
> and match it in Unmap.
>
> 2) CopyMem for ReadOperation in PciIo after SetAttribute (Leo Duran) Fix a
> bug in V1 that copy mem for read happen before SetAttribute, which will
> break AMD SEV solution.
>
> ================ V1 ==============
>
> This patch series adds IOMMU protocol and updates the consumer to
> support IOMMU based DMA access in UEFI.
>
> This patch series can support the BmDmaLib request for AMD SEV.
> submitted by Duran, Leo <leo.duran@amd.com> and Brijesh Singh
> <brijesh.ksingh@gmail.com>.
> https://lists.01.org/pipermail/edk2-devel/2017-March/008109.html, and
> https://lists.01.org/pipermail/edk2-devel/2017-March/008820.html.
> We can have an AMD SEV specific IOMMU driver to produce IOMMU
> protocol, and clear SEV in IOMMU->SetAttribute().
>
> This patch series can also support Intel VTd based DMA protection,
> requested by Jiewen Yao <jiewen.yao@intel.com>, discussed in
> https://lists.01.org/pipermail/edk2-devel/2017-March/008157.html.
> We can have an Intel VTd specific IOMMU driver to produce IOMMU
> protocol, and update VTd engine to grant or deny access in IOMMU-
> >SetAttribute().
>
> This patch series does not provide a full Intel VTd driver, which will be
> provide in other patch in the future.
>
> The purpose of this patch series to review if this IOMMU protocol design can
> meet all DMA access and management requirement.
>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Leo Duran <leo.duran@amd.com>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>
> Jiewen Yao (3):
> MdeModulePkg/Include: Add IOMMU protocol definition.
> MdeModulePkg/PciHostBridge: Add IOMMU support.
> MdeModulePkg/PciBus: Add IOMMU support.
>
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 9 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 47 +++-
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 37 +++
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 2 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 2 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 61 +++++
> MdeModulePkg/Include/Protocol/IoMmu.h | 259
> ++++++++++++++++++++
> MdeModulePkg/MdeModulePkg.dec | 3 +
> 10 files changed, 418 insertions(+), 4 deletions(-) create mode 100644
> MdeModulePkg/Include/Protocol/IoMmu.h
>
> --
> 2.7.4.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH V5 0/3] Add IOMMU support.
2017-05-13 19:27 ` Duran, Leo
@ 2017-05-18 8:37 ` Ard Biesheuvel
2017-05-18 8:40 ` Yao, Jiewen
0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2017-05-18 8:37 UTC (permalink / raw)
To: Duran, Leo, Jiewen Yao; +Cc: edk2-devel@lists.01.org, Ruiyu Ni, Singh, Brijesh
On 13 May 2017 at 20:27, Duran, Leo <leo.duran@amd.com> wrote:
> Reviewed-by: Leo Duran <leo.duran@amd.com>
>
> Thanks,
> Leo.
>
These patches are breaking the build for me under Clang. Apologies for
not spotting that at review time.
<https://ci.linaro.org/job/leg-virt-tianocore-edk2-upstream/ws/edk2/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c>:1088:32:
error: implicit conversion from enumeration type
'EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION' to different enumeration
type 'EDKII_IOMMU_OPERATION' [-Werror,-Wenum-conversion]
Operation,
^~~~~~~~~
1 error generated.
>> -----Original Message-----
>> From: Jiewen Yao [mailto:jiewen.yao@intel.com]
>> Sent: Thursday, May 04, 2017 11:33 AM
>> To: edk2-devel@lists.01.org
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>; Duran, Leo <leo.duran@amd.com>;
>> Singh, Brijesh <brijesh.singh@amd.com>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>
>> Subject: [PATCH V5 0/3] Add IOMMU support.
>>
>> ================ V5 ==============
>> Minor update from V4.
>>
>> 1) Remove unused SetAttribute() API in IOMMU protocol.
>> (Feedback from Ruiyu and Ard)
>> 2) Rename SetMappingAttribute() to SetAttribute().
>> (Feedback from Ruiyu)
>> 3) Fix the bug in PciBus driver for Operation (Thanks to Ard to catch it)
>>
>> V4:
>> Tested-by: Brijesh Singh <brijesh.singh@amd.com> With the issue in 3/3
>> addressed:
>> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> ================ V4 ==============
>> Refine the EDKII_IOMMU_PROTOCOL.
>>
>> 1) Add AllocateBuffer/FreeBuffer/Map/Unmap() API.
>> They are similar to DmaLib in EmbeddedPkg and similar to the previous
>> BmDmaLib (by leo.duran@amd.com).
>>
>> These APIs are invoked by PciHostBridge driver to allocate DMA memory.
>>
>> The PciHostBridge driver (IOMMU consumer) is simplified:
>> It uses IOMMU, if IOMMU protocol is present.
>> Else it uses original logic.
>>
>> 2) Add SetMappingAttribute() API.
>> It is similar to SetAttribute() API in V1.
>>
>> This API is invoked by PciBus driver to set DMA access attribute (read/write)
>> for device.
>>
>> The PciBus driver (IOMMU consumer) is simplified:
>> It sets access attribute in Map/Unmap,
>> if IOMMU protocol is present.
>>
>> 3) Remove SetRemapAddress/GetRemapAddress() API.
>> Because PciHostBridge/PciBus can call the APIs defined above, there is no
>> need to provide remap capability.
>>
>> -- Sample producer drivers:
>> 1) The sample VTd driver (IOMMU producer) is at
>> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/IntelVTdDxe
>>
>> It is added to show the concept. It is not fully implemented yet.
>> It will not be checked in in this patch.
>>
>> 2) The sample AMD SEV driver (IOMMU producer) is at
>> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleAmdSe
>> vDxe
>> (code is borrowed from leo.duran@amd.com and brijesh.singh@amd.com)
>>
>> This is not a right place to put this driver.
>>
>> It is added to show the concept.
>> It is not fully implemented. It will not be checked in.
>> Please do not use it directly.
>>
>> 3) The sample STYX driver (IOMMU producer) is at
>> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleStyxDx
>> e
>> (code is borrowed from ard.biesheuvel@linaro.org)
>>
>> This is not a right place to put this driver.
>>
>> It is added to show the concept.
>> It is not fully implemented. It will not be checked in.
>> Please do not use it directly.
>>
>>
>> ================ V3 ==============
>> 1) Add Remap capability (from Ard Biesheuvel) Add
>> EDKII_IOMMU_REMAP_ADDRESS API in IOMMU_PROTOCOL.
>>
>> NOTE: The code is not fully validated yet.
>> The purpose is to collect feedback to decide the next step.
>>
>> ================ V2 ==============
>> 1) Enhance Unmap() in PciIo (From Ruiyu Ni) Maintain a local list of MapInfo
>> and match it in Unmap.
>>
>> 2) CopyMem for ReadOperation in PciIo after SetAttribute (Leo Duran) Fix a
>> bug in V1 that copy mem for read happen before SetAttribute, which will
>> break AMD SEV solution.
>>
>> ================ V1 ==============
>>
>> This patch series adds IOMMU protocol and updates the consumer to
>> support IOMMU based DMA access in UEFI.
>>
>> This patch series can support the BmDmaLib request for AMD SEV.
>> submitted by Duran, Leo <leo.duran@amd.com> and Brijesh Singh
>> <brijesh.ksingh@gmail.com>.
>> https://lists.01.org/pipermail/edk2-devel/2017-March/008109.html, and
>> https://lists.01.org/pipermail/edk2-devel/2017-March/008820.html.
>> We can have an AMD SEV specific IOMMU driver to produce IOMMU
>> protocol, and clear SEV in IOMMU->SetAttribute().
>>
>> This patch series can also support Intel VTd based DMA protection,
>> requested by Jiewen Yao <jiewen.yao@intel.com>, discussed in
>> https://lists.01.org/pipermail/edk2-devel/2017-March/008157.html.
>> We can have an Intel VTd specific IOMMU driver to produce IOMMU
>> protocol, and update VTd engine to grant or deny access in IOMMU-
>> >SetAttribute().
>>
>> This patch series does not provide a full Intel VTd driver, which will be
>> provide in other patch in the future.
>>
>> The purpose of this patch series to review if this IOMMU protocol design can
>> meet all DMA access and management requirement.
>>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Leo Duran <leo.duran@amd.com>
>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com>
>>
>> Jiewen Yao (3):
>> MdeModulePkg/Include: Add IOMMU protocol definition.
>> MdeModulePkg/PciHostBridge: Add IOMMU support.
>> MdeModulePkg/PciBus: Add IOMMU support.
>>
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 9 +
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 47 +++-
>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 37 +++
>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 2 +
>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 2 +
>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 61 +++++
>> MdeModulePkg/Include/Protocol/IoMmu.h | 259
>> ++++++++++++++++++++
>> MdeModulePkg/MdeModulePkg.dec | 3 +
>> 10 files changed, 418 insertions(+), 4 deletions(-) create mode 100644
>> MdeModulePkg/Include/Protocol/IoMmu.h
>>
>> --
>> 2.7.4.windows.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH V5 0/3] Add IOMMU support.
2017-05-18 8:37 ` Ard Biesheuvel
@ 2017-05-18 8:40 ` Yao, Jiewen
0 siblings, 0 replies; 10+ messages in thread
From: Yao, Jiewen @ 2017-05-18 8:40 UTC (permalink / raw)
To: Ard Biesheuvel, Duran, Leo
Cc: edk2-devel@lists.01.org, Ni, Ruiyu, Singh, Brijesh
Thank you Ard. You are right.
We also notice that and submitted a new patch in the morning – “[patch] MdeModulePkg/PciHostBridgeDxe: Fix EBC build failure”
Thank you
Yao Jiewen
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Thursday, May 18, 2017 4:38 PM
To: Duran, Leo <leo.duran@amd.com>; Yao, Jiewen <jiewen.yao@intel.com>
Cc: edk2-devel@lists.01.org; Ni, Ruiyu <ruiyu.ni@intel.com>; Singh, Brijesh <brijesh.singh@amd.com>
Subject: Re: [PATCH V5 0/3] Add IOMMU support.
On 13 May 2017 at 20:27, Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>> wrote:
> Reviewed-by: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>>
>
> Thanks,
> Leo.
>
These patches are breaking the build for me under Clang. Apologies for
not spotting that at review time.
<https://ci.linaro.org/job/leg-virt-tianocore-edk2-upstream/ws/edk2/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c>:1088:32:
error: implicit conversion from enumeration type
'EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION' to different enumeration
type 'EDKII_IOMMU_OPERATION' [-Werror,-Wenum-conversion]
Operation,
^~~~~~~~~
1 error generated.
>> -----Original Message-----
>> From: Jiewen Yao [mailto:jiewen.yao@intel.com]
>> Sent: Thursday, May 04, 2017 11:33 AM
>> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>;
>> Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
>> Subject: [PATCH V5 0/3] Add IOMMU support.
>>
>> ================ V5 ==============
>> Minor update from V4.
>>
>> 1) Remove unused SetAttribute() API in IOMMU protocol.
>> (Feedback from Ruiyu and Ard)
>> 2) Rename SetMappingAttribute() to SetAttribute().
>> (Feedback from Ruiyu)
>> 3) Fix the bug in PciBus driver for Operation (Thanks to Ard to catch it)
>>
>> V4:
>> Tested-by: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>> With the issue in 3/3
>> addressed:
>> Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
>>
>> ================ V4 ==============
>> Refine the EDKII_IOMMU_PROTOCOL.
>>
>> 1) Add AllocateBuffer/FreeBuffer/Map/Unmap() API.
>> They are similar to DmaLib in EmbeddedPkg and similar to the previous
>> BmDmaLib (by leo.duran@amd.com<mailto:leo.duran@amd.com>).
>>
>> These APIs are invoked by PciHostBridge driver to allocate DMA memory.
>>
>> The PciHostBridge driver (IOMMU consumer) is simplified:
>> It uses IOMMU, if IOMMU protocol is present.
>> Else it uses original logic.
>>
>> 2) Add SetMappingAttribute() API.
>> It is similar to SetAttribute() API in V1.
>>
>> This API is invoked by PciBus driver to set DMA access attribute (read/write)
>> for device.
>>
>> The PciBus driver (IOMMU consumer) is simplified:
>> It sets access attribute in Map/Unmap,
>> if IOMMU protocol is present.
>>
>> 3) Remove SetRemapAddress/GetRemapAddress() API.
>> Because PciHostBridge/PciBus can call the APIs defined above, there is no
>> need to provide remap capability.
>>
>> -- Sample producer drivers:
>> 1) The sample VTd driver (IOMMU producer) is at
>> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/IntelVTdDxe
>>
>> It is added to show the concept. It is not fully implemented yet.
>> It will not be checked in in this patch.
>>
>> 2) The sample AMD SEV driver (IOMMU producer) is at
>> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleAmdSe
>> vDxe
>> (code is borrowed from leo.duran@amd.com<mailto:leo.duran@amd.com> and brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>)
>>
>> This is not a right place to put this driver.
>>
>> It is added to show the concept.
>> It is not fully implemented. It will not be checked in.
>> Please do not use it directly.
>>
>> 3) The sample STYX driver (IOMMU producer) is at
>> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleStyxDx
>> e
>> (code is borrowed from ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>)
>>
>> This is not a right place to put this driver.
>>
>> It is added to show the concept.
>> It is not fully implemented. It will not be checked in.
>> Please do not use it directly.
>>
>>
>> ================ V3 ==============
>> 1) Add Remap capability (from Ard Biesheuvel) Add
>> EDKII_IOMMU_REMAP_ADDRESS API in IOMMU_PROTOCOL.
>>
>> NOTE: The code is not fully validated yet.
>> The purpose is to collect feedback to decide the next step.
>>
>> ================ V2 ==============
>> 1) Enhance Unmap() in PciIo (From Ruiyu Ni) Maintain a local list of MapInfo
>> and match it in Unmap.
>>
>> 2) CopyMem for ReadOperation in PciIo after SetAttribute (Leo Duran) Fix a
>> bug in V1 that copy mem for read happen before SetAttribute, which will
>> break AMD SEV solution.
>>
>> ================ V1 ==============
>>
>> This patch series adds IOMMU protocol and updates the consumer to
>> support IOMMU based DMA access in UEFI.
>>
>> This patch series can support the BmDmaLib request for AMD SEV.
>> submitted by Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>> and Brijesh Singh
>> <brijesh.ksingh@gmail.com<mailto:brijesh.ksingh@gmail.com>>.
>> https://lists.01.org/pipermail/edk2-devel/2017-March/008109.html, and
>> https://lists.01.org/pipermail/edk2-devel/2017-March/008820.html.
>> We can have an AMD SEV specific IOMMU driver to produce IOMMU
>> protocol, and clear SEV in IOMMU->SetAttribute().
>>
>> This patch series can also support Intel VTd based DMA protection,
>> requested by Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>, discussed in
>> https://lists.01.org/pipermail/edk2-devel/2017-March/008157.html.
>> We can have an Intel VTd specific IOMMU driver to produce IOMMU
>> protocol, and update VTd engine to grant or deny access in IOMMU-
>> >SetAttribute().
>>
>> This patch series does not provide a full Intel VTd driver, which will be
>> provide in other patch in the future.
>>
>> The purpose of this patch series to review if this IOMMU protocol design can
>> meet all DMA access and management requirement.
>>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>
>> Cc: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>>
>> Cc: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
>>
>> Jiewen Yao (3):
>> MdeModulePkg/Include: Add IOMMU protocol definition.
>> MdeModulePkg/PciHostBridge: Add IOMMU support.
>> MdeModulePkg/PciBus: Add IOMMU support.
>>
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 9 +
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
>> MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 47 +++-
>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 37 +++
>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 2 +
>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 2 +
>> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 61 +++++
>> MdeModulePkg/Include/Protocol/IoMmu.h | 259
>> ++++++++++++++++++++
>> MdeModulePkg/MdeModulePkg.dec | 3 +
>> 10 files changed, 418 insertions(+), 4 deletions(-) create mode 100644
>> MdeModulePkg/Include/Protocol/IoMmu.h
>>
>> --
>> 2.7.4.windows.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread