Thanks, Ard. It is a good open that if we really need both SetAttribute and SetMappingAttribute.
Looking at code again, I prefer to remove SetAttribute(). I do not suggest we define MapInfo, because I want to make it DMA engine implementation choice. All we want is just to set an access attribute for this mapping, there is no need to input base/length. Because Map() should always be called by a device driver, no matter it is PCI DMA, ISA DMA, or low power controller DMA, I think SetMappingAttribute is good enough. I will plan to remove SetAttribute() in V5. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard Biesheuvel Sent: Tuesday, May 2, 2017 5:56 PM To: Yao, Jiewen <jiewen....@intel.com> Cc: Ni, Ruiyu <ruiyu...@intel.com>; edk2-devel@lists.01.org; Leo Duran <leo.du...@amd.com> Subject: Re: [edk2] [PATCH 1/3] MdeModulePkg/Include: Add IOMMU protocol definition. Hello Jiewen, On 29 April 2017 at 14:48, Jiewen Yao <jiewen....@intel.com<mailto:jiewen....@intel.com>> wrote: > 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...@intel.com<mailto:ruiyu...@intel.com>> > Cc: Leo Duran <leo.du...@amd.com<mailto:leo.du...@amd.com>> > Cc: Brijesh Singh <brijesh.si...@amd.com<mailto:brijesh.si...@amd.com>> > Cc: Ard Biesheuvel > <ard.biesheu...@linaro.org<mailto:ard.biesheu...@linaro.org>> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao <jiewen....@intel.com<mailto:jiewen....@intel.com>> > --- > MdeModulePkg/Include/Protocol/IoMmu.h | 310 ++++++++++++++++++++ > MdeModulePkg/MdeModulePkg.dec | 3 + > 2 files changed, 313 insertions(+) > > diff --git a/MdeModulePkg/Include/Protocol/IoMmu.h > b/MdeModulePkg/Include/Protocol/IoMmu.h > new file mode 100644 > index 0000000..3f62f46 > --- /dev/null > +++ b/MdeModulePkg/Include/Protocol/IoMmu.h > @@ -0,0 +1,310 @@ > +/** @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] DeviceAddress The base of device memory address to be used > as the DMA memory. > + @param[in] Length The length of device memory address to be > used as the DMA memory. > + @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 DeviceAddress is not IoMmu Page size > aligned. > + @retval EFI_INVALID_PARAMETER Length is not IoMmu Page size aligned. > + @retval EFI_INVALID_PARAMETER Length is 0. > + @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 DeviceAddress and Length. > + @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 EFI_PHYSICAL_ADDRESS DeviceAddress, > + IN UINT64 Length, > + IN UINT64 IoMmuAccess > + ); > + It is a bit unfortunate that we need both SetAttribute and SetMappingAttribute. Could we instead make the MAPPING_INFO struct (or at least, some part of it) part of the protocol? This would allow the PCI bus driver to call SetAttribute() directly, taking the values from the struct. > +/** > + 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 > + ); > + > +/** > + 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_MAPPING_ATTRIBUTE)( > + IN EDKII_IOMMU_PROTOCOL *This, > + IN EFI_HANDLE DeviceHandle, > + IN VOID *Mapping, > + IN UINT64 IoMmuAccess > + ); > + > +/// > +/// 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; > + EDKII_IOMMU_SET_MAPPING_ATTRIBUTE SetMappingAttribute; > +}; > + > +/// > +/// 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 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
