From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x234.google.com (mail-io0-x234.google.com [IPv6:2607:f8b0:4001:c06::234]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5E43921954063 for ; Mon, 17 Apr 2017 06:42:32 -0700 (PDT) Received: by mail-io0-x234.google.com with SMTP id o22so29851313iod.3 for ; Mon, 17 Apr 2017 06:42:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=2fuUU9nW243uGolbVgXhdP+MUmXh1AeyCaMPejJRkT0=; b=cv7Wf3U2XU28pM6rQLcrp7eEjBzUPhodO8hcWypvDt17jEc63RaTWwEUzz5N+gsJlR p9ap8zjrgKGwATNjrzQgRVFjleGCEtYIt/6mLm8Uy4F3qoqcHAtMK9vLIUsSpPpebPu5 reRBfozggOFMNtCbT/x1wNFlUcVSxLOYZJWU8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=2fuUU9nW243uGolbVgXhdP+MUmXh1AeyCaMPejJRkT0=; b=TaV3o4/ZKBkvSQgDqKsnc7OEV/iP58aKJVZQWNJSNqb93ndwcWBgOuqBrjhtVSBbzR I5m7TZ2RFpphpHMB5uffeoUCMLuh7wiINfiY+dSKCSUHsw8tqH7vc6ryZypE3iiZTe5V dUdyrVjUOxDfMr8Lboyd06/Qk/gKq0g0oA42zP7BDF7zU3jI3peZcbcXBvx8CY9O5ZRR EvswVDgl5cGzKGNZtzIa1v+gbew0+W6hMdhjvEznBJfxitRZ3XvYTkRBxI0F8dxOkkdG 6mx1+3voexoze2NeX7htC7mG9qzJ4P9oqdrdTBLDfO1/Spzok+c/amT8WCMV7HlJVWD5 KRFQ== X-Gm-Message-State: AN3rC/5kk7//KNbtzipW0dWto7kTNe0vnJ6dGDDIKkQNcBTGLpl34n+E wVOwFY3Zp5Pk8nizU9D+dknQPIIMmrKk X-Received: by 10.36.120.82 with SMTP id p79mr9614732itc.59.1492436551509; Mon, 17 Apr 2017 06:42:31 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.201.76 with HTTP; Mon, 17 Apr 2017 06:42:31 -0700 (PDT) In-Reply-To: <1491289579-15888-2-git-send-email-jiewen.yao@intel.com> References: <1491289579-15888-1-git-send-email-jiewen.yao@intel.com> <1491289579-15888-2-git-send-email-jiewen.yao@intel.com> From: Ard Biesheuvel Date: Mon, 17 Apr 2017 14:42:31 +0100 Message-ID: To: Jiewen Yao Cc: "edk2-devel@lists.01.org" , Ruiyu Ni , Leo Duran , Brijesh Singh Subject: Re: [RFC] [PATCH V3 1/3] MdeModulePkg/Include: Add IOMMU protocol definition. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Apr 2017 13:42:32 -0000 Content-Type: text/plain; charset=UTF-8 On 4 April 2017 at 08:06, Jiewen Yao 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. > > 4) AMD "SEV" feature. > We can have an AMD SEV specific IOMMU driver to produce IOMMU protocol, > and manage SEV bit in SetAttribute(), and return UNSUPPORTED > for SetRemapAddress/GetRemapAddress(). > > Cc: Ruiyu Ni > Cc: Leo Duran > Cc: Brijesh Singh > Cc: Ard Biesheuvel > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jiewen Yao > --- > MdeModulePkg/Include/Protocol/IoMmu.h | 196 ++++++++++++++++++++ > MdeModulePkg/MdeModulePkg.dec | 3 + > 2 files changed, 199 insertions(+) > > diff --git a/MdeModulePkg/Include/Protocol/IoMmu.h b/MdeModulePkg/Include/Protocol/IoMmu.h > new file mode 100644 > index 0000000..766b918 > --- /dev/null > +++ b/MdeModulePkg/Include/Protocol/IoMmu.h > @@ -0,0 +1,196 @@ > +/** @file > + EFI IOMMU Protocol. > + > +Copyright (c) 2017, Intel Corporation. All rights reserved.
> +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 attribute. > +// These types can be "ORed" together as needed. > +// Any undefined bits are reserved and must be zero. > +// > +#define EDKII_IOMMU_ATTRIBUTE_READ 0x1 > +#define EDKII_IOMMU_ATTRIBUTE_WRITE 0x2 > + > +/** > + Get IOMMU page size. > + > + This is the minimal supported page size for a IOMMU. > + For example, if an IOMMU supports 4KiB, 2MiB and 1GiB, > + 4KiB should be returned. > + > + @param[in] This Protocol instance pointer. > + @param[out] PageSize The minimal supported page size for a IOMMU. > + > + @retval EFI_SUCCESS The page size is returned. > + @retval EFI_INVALID_PARAMETER PageSize is NULL. > + > +**/ > +typedef > +EFI_STATUS > +(EFIAPI *EDKII_IOMMU_GET_PAGE_SIZE)( > + IN EDKII_IOMMU_PROTOCOL *This, > + OUT UINTN *PageSize > + ); > + When do we expect to need this? Is it common on x86 to have IOMMUs that don't support 4 KB page size? > +/** > + Set IOMMU attribute for a system memory. > + > + If the IOMMU protocol exists, the system memory cannot be used > + for DMA by default. > + I suppose this is required for encryption support, but isn't it overly restrictive in the general case? On ARM systems, many drivers exist that are not PCI based but do perform DMA. (i.e., DmaLib) > + 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_ATTRIBUTE_READ. > + The memory for BusMasterWrite need set EDKII_IOMMU_ATTRIBUTE_WRITE. > + The memory for BusMasterCommonBuffer need set EDKII_IOMMU_ATTRIBUTE_READ|EDKII_IOMMU_ATTRIBUTE_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_ATTRIBUTE_READ and/or EDKII_IOMMU_ATTRIBUTE_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] IoMmuAttribute The IOMMU attribute. > + > + @retval EFI_SUCCESS The IoMmuAttribute 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 IoMmuAttribute specified an illegal combination of attributes. > + @retval EFI_UNSUPPORTED DeviceHandle is unknown by the IOMMU. > + @retval EFI_UNSUPPORTED The bit mask of IoMmuAttribute 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 attribute. > + @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 IoMmuAttribute > + ); > + > +/** > + Remap a device address to host address. > + > + For example, this function may support map an above 4GiB host address > + to a below 4GiB device address. > + > + @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] HostAddress The base of host 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. > + > + @retval EFI_SUCCESS The HostAddress is remmapped for the memory range specified by DeviceAddress and Length. > + @retval EFI_INVALID_PARAMETER DeviceHandle is not NULL, and it is an invalid handle. > + @retval EFI_INVALID_PARAMETER DeviceAddress is not IoMmu Page size aligned. > + @retval EFI_INVALID_PARAMETER HostAddress 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_UNSUPPORTED DeviceHandle is unknown by the IOMMU. > + @retval EFI_UNSUPPORTED The remap function is unsupported 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 attribute. > + @retval EFI_DEVICE_ERROR The IOMMU device reported an error while attempting the operation. > + > +**/ > +typedef > +EFI_STATUS > +(EFIAPI *EDKII_IOMMU_SET_REMAP_ADDRESS)( > + IN EDKII_IOMMU_PROTOCOL *This, > + IN EFI_HANDLE DeviceHandle, OPTIONAL > + IN EFI_PHYSICAL_ADDRESS DeviceAddress, > + IN VOID *HostAddress, > + IN UINT64 Length > + ); > + > +/** > + Return a remapped host address from a device address. > + > + @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[out] HostAddress The base of host memory address to be used as the DMA memory. > + > + @retval EFI_SUCCESS The HostAddress remmapped for the memory range specified by DeviceAddress is returned. > + @retval EFI_INVALID_PARAMETER DeviceHandle is not NULL, and it is an invalid handle. > + @retval EFI_INVALID_PARAMETER DeviceAddress is not IoMmu Page size aligned. > + @retval EFI_INVALID_PARAMETER HostAddress is NULL. > + @retval EFI_UNSUPPORTED DeviceHandle is unknown by the IOMMU. > + @retval EFI_UNSUPPORTED The remap function is unsupported by the IOMMU. > + @retval EFI_NOT_FOUND The HostAddress for DeviceAddress is not found. > + @retval EFI_DEVICE_ERROR The IOMMU device reported an error while attempting the operation. > + > +**/ > +typedef > +EFI_STATUS > +(EFIAPI *EDKII_IOMMU_GET_REMAP_ADDRESS)( > + IN EDKII_IOMMU_PROTOCOL *This, > + IN EFI_HANDLE DeviceHandle, OPTIONAL > + IN EFI_PHYSICAL_ADDRESS DeviceAddress, > + OUT VOID **HostAddress > + ); > + > +/// > +/// IOMMU Protocol structure. > +/// > +struct _EDKII_IOMMU_PROTOCOL { > + UINT64 Revision; > + EDKII_IOMMU_GET_PAGE_SIZE GetPageSize; > + EDKII_IOMMU_SET_ATTRIBUTE SetAttribute; > + EDKII_IOMMU_SET_REMAP_ADDRESS SetRemapAddress; > + EDKII_IOMMU_GET_REMAP_ADDRESS GetRemapAddress; > +}; > + > +/// > +/// 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. I see how this protocol intends to cater for a lot of different use cases, but the one that will be the most common on 64-bit ARM is a simple static translation between the start of DRAM (which may be > 4 GB) and PCI address range [ 0x0, 4 GB ), and a 1:1 translation for all other masters. This may require bounce buffering for non-coherent mappings, but it can be achieved with a very simple driver. I implemented a proof of concept based on Leo's BmDmaLib here https://github.com/ardbiesheuvel/OpenPlatformPkg/commit/dc251f0bf7a85205fb3590ccc9788b072a2bdcb8 Implementing IOMMU protocol as specified here, with remap() entry points, forces me to manage the IOMMU page tables dynamically, which is a level of complexity that is really not needed for my use case, especially given the required TLB maintenance and other concerns that don't really exist for a static mapping. Also, could you explain why we need to change the root bridge *and* the bus driver?