* [RFC] [PATCH V3 1/3] MdeModulePkg/Include: Add IOMMU protocol definition.
2017-04-04 7:06 [RFC] [PATCH V3 0/3] Add IOMMU support Jiewen Yao
@ 2017-04-04 7:06 ` Jiewen Yao
2017-04-17 13:42 ` Ard Biesheuvel
2017-04-04 7:06 ` [RFC] [PATCH V3 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support Jiewen Yao
2017-04-04 7:06 ` [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: " Jiewen Yao
2 siblings, 1 reply; 10+ messages in thread
From: Jiewen Yao @ 2017-04-04 7:06 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.
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 <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 | 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.<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 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
+ );
+
+/**
+ 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_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.
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC] [PATCH V3 1/3] MdeModulePkg/Include: Add IOMMU protocol definition.
2017-04-04 7:06 ` [RFC] [PATCH V3 1/3] MdeModulePkg/Include: Add IOMMU protocol definition Jiewen Yao
@ 2017-04-17 13:42 ` Ard Biesheuvel
2017-04-17 15:13 ` Yao, Jiewen
0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2017-04-17 13:42 UTC (permalink / raw)
To: Jiewen Yao; +Cc: edk2-devel@lists.01.org, Ruiyu Ni, Leo Duran, Brijesh Singh
On 4 April 2017 at 08:06, Jiewen Yao <jiewen.yao@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.
>
> 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 <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 | 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.<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 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?
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC] [PATCH V3 1/3] MdeModulePkg/Include: Add IOMMU protocol definition.
2017-04-17 13:42 ` Ard Biesheuvel
@ 2017-04-17 15:13 ` Yao, Jiewen
0 siblings, 0 replies; 10+ messages in thread
From: Yao, Jiewen @ 2017-04-17 15:13 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Ni, Ruiyu, Leo Duran, Brijesh Singh
Hi Ard
Thanks for the feedback.
Detailed comment below:
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Monday, April 17, 2017 9:43 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: edk2-devel@lists.01.org; Ni, Ruiyu <ruiyu.ni@intel.com>; Leo Duran <leo.duran@amd.com>; Brijesh Singh <brijesh.singh@amd.com>
Subject: Re: [RFC] [PATCH V3 1/3] MdeModulePkg/Include: Add IOMMU protocol definition.
On 4 April 2017 at 08:06, Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@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.
>
> 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 <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>>
> ---
> 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.<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 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?
[Jiewen] Yes. X86 IOMMU does support 4KB.
I am not knowledgeable enough to know all other architectures. So this is a generic design.
Of course, if we can confirm all arch support 4KB IOMMU, this API can be removed. ☺
Ard, can you help to double confirm for ARM/AArch64?
> +/**
> + 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)
[Jiewen] This is not for encryption support.
This is to prevent DMA attack for security concern. When IOMMU driver launches, it should set all memory to be not DMA capable. Then only requested DMA memory is open for the device.
In your below case, you just want to remap above 4GiB to below 4GiB. It is not for security.
Maybe we can use weak term – the system memory *might not* 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_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.
[Jiewen] I agree. In your case, it should be very simple.
You can configure a static IOMMU table at the driver enterpoint, but there is no need to update it.
Very good feedback. Let me think about it.
Maybe an IOMMU->GetRemapAddress() in the beginning can make it simple.
If I have new version patch, I will provide IOMMU sample code for your 4G-remap and Leo’s SEV case.
Also, could you explain why we need to change the root bridge *and*
the bus driver?
[Jiewen] Very good question.
In my case, I need to know which PCI device submits DMA request, so that I can configure IOMMU for *this PCI device* only. I am not able to do that in PciHostBridge driver, because the PCI device information is lost.
*That is the biggest reason that I am not able to use Leo’s BmDmaLib directly.* I must update PciBus driver.
PciBus driver can do remap action by itself to make requested memory to be IOMMU page aligned. But that is also not enough.
1) PciHostBridge driver may need do remap with IOMMU page alignment due to 4GiB issue.
2) PciHostBridge driver CopyMem is useless, because CopyMem should happen after SetAttribute to clear SEV bit.
If we can assume 4KB IOMMU, if double copy is not a problem, maybe we can eliminate PciHostBridge update.
Thank you
Yao Jiewen
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC] [PATCH V3 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support.
2017-04-04 7:06 [RFC] [PATCH V3 0/3] Add IOMMU support Jiewen Yao
2017-04-04 7:06 ` [RFC] [PATCH V3 1/3] MdeModulePkg/Include: Add IOMMU protocol definition Jiewen Yao
@ 2017-04-04 7:06 ` Jiewen Yao
2017-04-17 19:33 ` Duran, Leo
2017-04-04 7:06 ` [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: " Jiewen Yao
2 siblings, 1 reply; 10+ messages in thread
From: Jiewen Yao @ 2017-04-04 7:06 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Leo Duran, Brijesh Singh, Ard Biesheuvel
The responsibility of PciHostBridge is to allocate IOMMU
page aligned memory for Map and AllocateBuffer,
because PciHostBridge driver already handles Map() request
to allocate another buffer for DMA read/write.
If the max address requirement can not be satisfied,
PciHostBridge may also allocate any IOMMU page aligned memory
and use IOMMU Remapping feature to map to lower address to
satisfy device requirement.
PciHostBridge does not set IOMMU 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 | 3 +
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 +
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 8 +
MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 350 ++++++++++++++++++--
4 files changed, 326 insertions(+), 36 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index 9005dee..35233a7 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -28,6 +28,9 @@ GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mPciResourceTypeStr[] = {
L"I/O", L"Mem", L"PMem", L"Mem64", L"PMem64", L"Bus"
};
+EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
+UINTN mIoMmuPageSize = 1;
+
/**
Ensure the compatibility of an IO space descriptor with the IO aperture.
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
index d8b0439..2d3c8c9 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
@@ -49,6 +49,7 @@
gEfiDevicePathProtocolGuid ## BY_START
gEfiPciRootBridgeIoProtocolGuid ## BY_START
gEfiPciHostBridgeResourceAllocationProtocolGuid ## BY_START
+ gEdkiiIoMmuProtocolGuid ## CONSUMES
[Depex]
gEfiCpuIo2ProtocolGuid AND
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
index 13185b4..77c3490 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>
@@ -50,8 +51,11 @@ typedef struct {
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION Operation;
UINTN NumberOfBytes;
UINTN NumberOfPages;
+ UINTN MappedNumberOfBytes;
+ UINTN MappedNumberOfPages;
EFI_PHYSICAL_ADDRESS HostAddress;
EFI_PHYSICAL_ADDRESS MappedHostAddress;
+ BOOLEAN RemapNonExisting;
} MAP_INFO;
#define MAP_INFO_FROM_LINK(a) CR (a, MAP_INFO, Link, MAP_INFO_SIGNATURE)
@@ -575,4 +579,8 @@ RootBridgeIoConfiguration (
extern EFI_METRONOME_ARCH_PROTOCOL *mMetronome;
extern EFI_CPU_IO2_PROTOCOL *mCpuIo;
+
+extern EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
+extern UINTN mIoMmuPageSize;
+
#endif
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index 8af131b..2a17eb1 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -1022,6 +1022,121 @@ RootBridgeIoPciWrite (
}
/**
+ Allocates one or more 4KB pages of a certain memory type at a specified alignment.
+
+ Allocates the number of 4KB pages specified by Pages of a certain memory type with an alignment
+ specified by Alignment. The allocated buffer is returned. If Pages is 0, then NULL is returned.
+ If there is not enough memory at the specified alignment remaining to satisfy the request, then
+ NULL is returned.
+ If Alignment is not a power of two and Alignment is not zero, then ASSERT().
+ If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
+
+ @param Type The type of allocation to perform.
+ @param MemoryType The type of memory to allocate.
+ @param Pages The number of 4 KB pages to allocate.
+ @param Alignment The requested alignment of the allocation. Must be a power of two.
+ If Alignment is zero, then byte alignment is used.
+ @param Address Pointer to a physical address.
+
+ @return Memory Allocation Status.
+
+**/
+EFI_STATUS
+InternalAllocateAlignedPagesWithAllocateType (
+ IN EFI_ALLOCATE_TYPE Type,
+ IN EFI_MEMORY_TYPE MemoryType,
+ IN UINTN Pages,
+ IN UINTN Alignment,
+ IN OUT EFI_PHYSICAL_ADDRESS *Address
+ )
+{
+ EFI_STATUS Status;
+ EFI_PHYSICAL_ADDRESS Memory;
+ UINTN AlignedMemory;
+ UINTN AlignmentMask;
+ UINTN UnalignedPages;
+ UINTN RealPages;
+
+ //
+ // Alignment must be a power of two or zero.
+ //
+ ASSERT ((Alignment & (Alignment - 1)) == 0);
+
+ if (Pages == 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+ if (Alignment > EFI_PAGE_SIZE) {
+ //
+ // Calculate the total number of pages since alignment is larger than page size.
+ //
+ AlignmentMask = Alignment - 1;
+ RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
+ //
+ // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not overflow.
+ //
+ ASSERT (RealPages > Pages);
+
+ Memory = *Address;
+ Status = gBS->AllocatePages (Type, MemoryType, RealPages, &Memory);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ AlignedMemory = ((UINTN) Memory + AlignmentMask) & ~AlignmentMask;
+ UnalignedPages = EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN) Memory);
+ if (UnalignedPages > 0) {
+ //
+ // Free first unaligned page(s).
+ //
+ Status = gBS->FreePages (Memory, UnalignedPages);
+ ASSERT_EFI_ERROR (Status);
+ }
+ Memory = (EFI_PHYSICAL_ADDRESS) (AlignedMemory + EFI_PAGES_TO_SIZE (Pages));
+ UnalignedPages = RealPages - Pages - UnalignedPages;
+ if (UnalignedPages > 0) {
+ //
+ // Free last unaligned page(s).
+ //
+ Status = gBS->FreePages (Memory, UnalignedPages);
+ ASSERT_EFI_ERROR (Status);
+ }
+ } else {
+ //
+ // Do not over-allocate pages in this case.
+ //
+ Memory = *Address;
+ Status = gBS->AllocatePages (Type, MemoryType, Pages, &Memory);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ AlignedMemory = (UINTN) Memory;
+ }
+ *Address = AlignedMemory;
+ return EFI_SUCCESS;
+}
+
+/**
+ Return if a value is aligned.
+
+ @param Value the value to be checked
+ @param Alignment the alignment to be checked with.
+
+ @retval TRUE The value is aligned
+ @retval FALSE The value is not aligned.
+**/
+BOOLEAN
+InternalIsAlgined (
+ IN UINTN Value,
+ IN UINTN Alignment
+ )
+{
+ if (Value == ALIGN_VALUE(Value, Alignment)) {
+ return TRUE;
+ } else {
+ return FALSE;
+ }
+}
+
+/**
Provides the PCI controller-specific address needed to access
system memory for DMA.
@@ -1057,6 +1172,9 @@ RootBridgeIoMap (
PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
EFI_PHYSICAL_ADDRESS PhysicalAddress;
MAP_INFO *MapInfo;
+ EFI_PHYSICAL_ADDRESS MaxAddress;
+ BOOLEAN NeedMap;
+ BOOLEAN NeedAllocateNonExisting;
if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL ||
Mapping == NULL) {
@@ -1072,13 +1190,41 @@ RootBridgeIoMap (
RootBridge = ROOT_BRIDGE_FROM_THIS (This);
+ if (gIoMmuProtocol == NULL) {
+ gBS->LocateProtocol (
+ &gEdkiiIoMmuProtocolGuid,
+ NULL,
+ (VOID **) &gIoMmuProtocol
+ );
+ if (gIoMmuProtocol != NULL) {
+ gIoMmuProtocol->GetPageSize (gIoMmuProtocol, &mIoMmuPageSize);
+ ASSERT ((mIoMmuPageSize & (mIoMmuPageSize - 1)) == 0);
+ }
+ }
+
PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
+
+ MaxAddress = (EFI_PHYSICAL_ADDRESS)-1;
+ NeedMap = FALSE;
+ NeedAllocateNonExisting = FALSE;
+
if ((!RootBridge->DmaAbove4G ||
(Operation != EfiPciOperationBusMasterRead64 &&
Operation != EfiPciOperationBusMasterWrite64 &&
Operation != EfiPciOperationBusMasterCommonBuffer64)) &&
((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) {
+ NeedMap = TRUE;
+ MaxAddress = SIZE_4GB - 1;
+ }
+ if (gIoMmuProtocol != NULL) {
+ if ((!InternalIsAlgined (*NumberOfBytes, mIoMmuPageSize)) ||
+ (!InternalIsAlgined ((UINTN)HostAddress, mIoMmuPageSize))) {
+ NeedMap = TRUE;
+ }
+ }
+
+ if (NeedMap) {
//
// If the root bridge or the device cannot handle performing DMA above
// 4GB but any part of the DMA transfer being mapped is above 4GB, then
@@ -1090,9 +1236,17 @@ RootBridgeIoMap (
//
// Common Buffer operations can not be remapped. If the common buffer
// if above 4GB, then it is not possible to generate a mapping, so return
- // an error.
+ // an error if there is no IOMMU.
//
- return EFI_UNSUPPORTED;
+ if (gIoMmuProtocol == NULL) {
+ return EFI_UNSUPPORTED;
+ } else {
+ //
+ // We can try to allocate non-existing memory for below 4GiB address
+ // and use IOMMU to remap it.
+ //
+ NeedAllocateNonExisting = TRUE;
+ }
}
//
@@ -1113,21 +1267,81 @@ RootBridgeIoMap (
MapInfo->NumberOfBytes = *NumberOfBytes;
MapInfo->NumberOfPages = EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes);
MapInfo->HostAddress = PhysicalAddress;
- MapInfo->MappedHostAddress = SIZE_4GB - 1;
+ MapInfo->MappedHostAddress = MaxAddress;
+ MapInfo->MappedNumberOfBytes = ALIGN_VALUE (MapInfo->NumberOfBytes, mIoMmuPageSize);
+ MapInfo->MappedNumberOfPages = EFI_SIZE_TO_PAGES (MapInfo->MappedNumberOfBytes);
+ MapInfo->RemapNonExisting = FALSE;
- //
- // Allocate a buffer below 4GB to map the transfer to.
- //
- Status = gBS->AllocatePages (
- AllocateMaxAddress,
- EfiBootServicesData,
- MapInfo->NumberOfPages,
- &MapInfo->MappedHostAddress
- );
- if (EFI_ERROR (Status)) {
- FreePool (MapInfo);
- *NumberOfBytes = 0;
- return Status;
+ if (!((Operation == EfiPciOperationBusMasterCommonBuffer) ||
+ (Operation == EfiPciOperationBusMasterCommonBuffer64))) {
+ //
+ // Allocate a buffer below 4GB to map the transfer to.
+ //
+ Status = InternalAllocateAlignedPagesWithAllocateType (
+ AllocateMaxAddress,
+ EfiBootServicesData,
+ MapInfo->MappedNumberOfPages,
+ mIoMmuPageSize,
+ &MapInfo->MappedHostAddress
+ );
+ if (EFI_ERROR (Status)) {
+ if (gIoMmuProtocol == NULL) {
+ FreePool (MapInfo);
+ *NumberOfBytes = 0;
+ return Status;
+ }
+ //
+ // We can try to allocate non-existing memory for below 4GiB address
+ // and use IOMMU to remap it.
+ //
+ NeedAllocateNonExisting = TRUE;
+ }
+ }
+
+ if (NeedAllocateNonExisting) {
+ if ((!InternalIsAlgined (*NumberOfBytes, mIoMmuPageSize)) ||
+ (!InternalIsAlgined ((UINTN)HostAddress, mIoMmuPageSize))) {
+ //
+ // If original memory is not IOMMU aligned, we cannot remap it.
+ //
+ FreePool (MapInfo);
+ *NumberOfBytes = 0;
+ return EFI_UNSUPPORTED;
+ }
+ //
+ // If the code runs to here, it means:
+ // 1) We need remap a common buffer to below 4G non-existing memory.
+ // 2) We need rempa a read/write buffer to below 4G non-existing memory.
+ //
+ MapInfo->MappedHostAddress = MaxAddress;
+ Status = gDS->AllocateMemorySpace (
+ EfiGcdAllocateMaxAddressSearchTopDown,
+ EfiGcdMemoryTypeNonExistent,
+ mIoMmuPageSize,
+ MapInfo->MappedNumberOfBytes,
+ &MapInfo->MappedHostAddress,
+ gImageHandle,
+ RootBridge->Handle
+ );
+ if (EFI_ERROR(Status)) {
+ FreePool (MapInfo);
+ *NumberOfBytes = 0;
+ return Status;
+ }
+ Status = gIoMmuProtocol->SetRemapAddress (
+ gIoMmuProtocol,
+ NULL,
+ MapInfo->MappedHostAddress,
+ (VOID *)(UINTN)MapInfo->HostAddress,
+ MapInfo->MappedNumberOfBytes
+ );
+ if (EFI_ERROR(Status)) {
+ gDS->FreeMemorySpace (MapInfo->MappedHostAddress, MapInfo->MappedNumberOfBytes);
+ FreePool (MapInfo);
+ *NumberOfBytes = 0;
+ return Status;
+ }
+ MapInfo->RemapNonExisting = TRUE;
}
//
@@ -1135,19 +1349,26 @@ RootBridgeIoMap (
// then copy the contents of the real buffer into the mapped buffer
// so the Bus Master can read the contents of the real buffer.
//
- if (Operation == EfiPciOperationBusMasterRead ||
- Operation == EfiPciOperationBusMasterRead64) {
- CopyMem (
- (VOID *) (UINTN) MapInfo->MappedHostAddress,
- (VOID *) (UINTN) MapInfo->HostAddress,
- MapInfo->NumberOfBytes
- );
+ // This action can be skipped if there is IOMMU, because PciBus does
+ // the action after setting IOMMU attributes.
+ //
+ if (gIoMmuProtocol == NULL) {
+ if (Operation == EfiPciOperationBusMasterRead ||
+ Operation == EfiPciOperationBusMasterRead64) {
+ CopyMem (
+ (VOID *) (UINTN) MapInfo->MappedHostAddress,
+ (VOID *) (UINTN) MapInfo->HostAddress,
+ MapInfo->NumberOfBytes
+ );
+ }
}
InsertTailList (&RootBridge->Maps, &MapInfo->Link);
//
// The DeviceAddress is the address of the maped buffer below 4GB
+ // NOTE: It can be a valid system memory address.
+ // Or a non-existing memory but mapped by IOMMU.
//
*DeviceAddress = MapInfo->MappedHostAddress;
//
@@ -1228,19 +1449,42 @@ RootBridgeIoUnmap (
// then copy the contents of the mapped buffer into the real buffer
// so the processor can read the contents of the real buffer.
//
- if (MapInfo->Operation == EfiPciOperationBusMasterWrite ||
- MapInfo->Operation == EfiPciOperationBusMasterWrite64) {
- CopyMem (
- (VOID *) (UINTN) MapInfo->HostAddress,
- (VOID *) (UINTN) MapInfo->MappedHostAddress,
- MapInfo->NumberOfBytes
- );
+ // This action can be skipped if there is IOMMU, because PciBus does
+ // the action.
+ //
+ if (gIoMmuProtocol == NULL) {
+ if (MapInfo->Operation == EfiPciOperationBusMasterWrite ||
+ MapInfo->Operation == EfiPciOperationBusMasterWrite64) {
+ CopyMem (
+ (VOID *) (UINTN) MapInfo->HostAddress,
+ (VOID *) (UINTN) MapInfo->MappedHostAddress,
+ MapInfo->NumberOfBytes
+ );
+ }
+ }
+
+ if (gIoMmuProtocol != NULL) {
+ //
+ // Free GCD non existing memory.
+ //
+ if (MapInfo->RemapNonExisting) {
+ gIoMmuProtocol->SetRemapAddress (
+ gIoMmuProtocol,
+ NULL,
+ MapInfo->MappedHostAddress,
+ (VOID *)(UINTN)MapInfo->MappedHostAddress,
+ MapInfo->MappedNumberOfBytes
+ );
+ gDS->FreeMemorySpace (MapInfo->MappedHostAddress, MapInfo->MappedNumberOfBytes);
+ FreePool (Mapping);
+ return EFI_SUCCESS;
+ }
}
//
// Free the mapped buffer and the MAP_INFO structure.
//
- gBS->FreePages (MapInfo->MappedHostAddress, MapInfo->NumberOfPages);
+ gBS->FreePages (MapInfo->MappedHostAddress, MapInfo->MappedNumberOfPages);
FreePool (Mapping);
return EFI_SUCCESS;
}
@@ -1285,7 +1529,7 @@ RootBridgeIoAllocateBuffer (
EFI_STATUS Status;
EFI_PHYSICAL_ADDRESS PhysicalAddress;
PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
- EFI_ALLOCATE_TYPE AllocateType;
+ UINTN Size;
//
// Validate Attributes
@@ -1312,25 +1556,52 @@ RootBridgeIoAllocateBuffer (
RootBridge = ROOT_BRIDGE_FROM_THIS (This);
- AllocateType = AllocateAnyPages;
+ PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (-1);
if (!RootBridge->DmaAbove4G ||
(Attributes & EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) {
//
// Limit allocations to memory below 4GB
//
- AllocateType = AllocateMaxAddress;
PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (SIZE_4GB - 1);
}
- Status = gBS->AllocatePages (
- AllocateType,
+ if (gIoMmuProtocol != NULL) {
+ Size = EFI_PAGES_TO_SIZE(Pages);
+ Size = ALIGN_VALUE(EFI_PAGES_TO_SIZE(Pages), mIoMmuPageSize);
+ Pages = EFI_SIZE_TO_PAGES (Size);
+ }
+ Status = InternalAllocateAlignedPagesWithAllocateType (
+ AllocateMaxAddress,
MemoryType,
Pages,
+ mIoMmuPageSize,
&PhysicalAddress
);
if (!EFI_ERROR (Status)) {
*HostAddress = (VOID *) (UINTN) PhysicalAddress;
+ return EFI_SUCCESS;
}
+ if (gIoMmuProtocol != NULL) {
+ //
+ // Try to allocate AnyAddress here.
+ //
+ // We can try to allocate non-existing memory for below 4GiB address
+ // and use IOMMU to remap it later.
+ //
+ PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (-1);
+ Status = InternalAllocateAlignedPagesWithAllocateType (
+ AllocateMaxAddress,
+ MemoryType,
+ Pages,
+ mIoMmuPageSize,
+ &PhysicalAddress
+ );
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }
+ *HostAddress = (VOID *) (UINTN) PhysicalAddress;
+ return EFI_SUCCESS;
+ }
return Status;
}
@@ -1356,6 +1627,13 @@ RootBridgeIoFreeBuffer (
OUT VOID *HostAddress
)
{
+ UINTN Size;
+
+ if (gIoMmuProtocol != NULL) {
+ Size = EFI_PAGES_TO_SIZE(Pages);
+ Size = ALIGN_VALUE(EFI_PAGES_TO_SIZE(Pages), mIoMmuPageSize);
+ Pages = EFI_SIZE_TO_PAGES (Size);
+ }
return gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress, Pages);
}
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC] [PATCH V3 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support.
2017-04-04 7:06 ` [RFC] [PATCH V3 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support Jiewen Yao
@ 2017-04-17 19:33 ` Duran, Leo
2017-04-18 3:31 ` Yao, Jiewen
0 siblings, 1 reply; 10+ messages in thread
From: Duran, Leo @ 2017-04-17 19:33 UTC (permalink / raw)
To: 'Jiewen Yao', edk2-devel@lists.01.org
Cc: Ruiyu Ni, Singh, Brijesh, Ard Biesheuvel
Hi Yao,
Just a couple of quick comments:
1) gIoMmuProtocol is declared (global) by PciHostBridge.c, but it is not initialized to NULL.
2) Would it be OK to do a one-time LocateProtocol() of gIoMmuProtocol in InitializePciHostBridge()?
BTW, besides the global declaration, gIoMmuProtocol is currently not used by PciHostBridge.c
Leo
> -----Original Message-----
> From: Jiewen Yao [mailto:jiewen.yao@intel.com]
> Sent: Tuesday, April 04, 2017 2:06 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: [RFC] [PATCH V3 2/3] MdeModulePkg/PciHostBridge: Add IOMMU
> support.
>
> The responsibility of PciHostBridge is to allocate IOMMU page aligned
> memory for Map and AllocateBuffer, because PciHostBridge driver already
> handles Map() request to allocate another buffer for DMA read/write.
>
> If the max address requirement can not be satisfied, PciHostBridge may also
> allocate any IOMMU page aligned memory and use IOMMU Remapping
> feature to map to lower address to satisfy device requirement.
>
> PciHostBridge does not set IOMMU 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 | 3 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 8 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 350
> ++++++++++++++++++--
> 4 files changed, 326 insertions(+), 36 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> index 9005dee..35233a7 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> @@ -28,6 +28,9 @@ GLOBAL_REMOVE_IF_UNREFERENCED CHAR16
> *mPciResourceTypeStr[] = {
> L"I/O", L"Mem", L"PMem", L"Mem64", L"PMem64", L"Bus"
> };
>
> +EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
> +UINTN mIoMmuPageSize = 1;
> +
> /**
> Ensure the compatibility of an IO space descriptor with the IO aperture.
>
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> index d8b0439..2d3c8c9 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> @@ -49,6 +49,7 @@
> gEfiDevicePathProtocolGuid ## BY_START
> gEfiPciRootBridgeIoProtocolGuid ## BY_START
> gEfiPciHostBridgeResourceAllocationProtocolGuid ## BY_START
> + gEdkiiIoMmuProtocolGuid ## CONSUMES
>
> [Depex]
> gEfiCpuIo2ProtocolGuid AND
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> index 13185b4..77c3490 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>
> @@ -50,8 +51,11 @@ typedef struct {
> EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION Operation;
> UINTN NumberOfBytes;
> UINTN NumberOfPages;
> + UINTN MappedNumberOfBytes;
> + UINTN MappedNumberOfPages;
> EFI_PHYSICAL_ADDRESS HostAddress;
> EFI_PHYSICAL_ADDRESS MappedHostAddress;
> + BOOLEAN RemapNonExisting;
> } MAP_INFO;
> #define MAP_INFO_FROM_LINK(a) CR (a, MAP_INFO, Link,
> MAP_INFO_SIGNATURE)
>
> @@ -575,4 +579,8 @@ RootBridgeIoConfiguration (
>
> extern EFI_METRONOME_ARCH_PROTOCOL *mMetronome;
> extern EFI_CPU_IO2_PROTOCOL *mCpuIo;
> +
> +extern EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
> +extern UINTN mIoMmuPageSize;
> +
> #endif
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> index 8af131b..2a17eb1 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> @@ -1022,6 +1022,121 @@ RootBridgeIoPciWrite ( }
>
> /**
> + Allocates one or more 4KB pages of a certain memory type at a specified
> alignment.
> +
> + Allocates the number of 4KB pages specified by Pages of a certain
> + memory type with an alignment specified by Alignment. The allocated
> buffer is returned. If Pages is 0, then NULL is returned.
> + If there is not enough memory at the specified alignment remaining to
> + satisfy the request, then NULL is returned.
> + If Alignment is not a power of two and Alignment is not zero, then
> ASSERT().
> + If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
> +
> + @param Type The type of allocation to perform.
> + @param MemoryType The type of memory to allocate.
> + @param Pages The number of 4 KB pages to allocate.
> + @param Alignment The requested alignment of the allocation.
> Must be a power of two.
> + If Alignment is zero, then byte alignment is used.
> + @param Address Pointer to a physical address.
> +
> + @return Memory Allocation Status.
> +
> +**/
> +EFI_STATUS
> +InternalAllocateAlignedPagesWithAllocateType (
> + IN EFI_ALLOCATE_TYPE Type,
> + IN EFI_MEMORY_TYPE MemoryType,
> + IN UINTN Pages,
> + IN UINTN Alignment,
> + IN OUT EFI_PHYSICAL_ADDRESS *Address
> + )
> +{
> + EFI_STATUS Status;
> + EFI_PHYSICAL_ADDRESS Memory;
> + UINTN AlignedMemory;
> + UINTN AlignmentMask;
> + UINTN UnalignedPages;
> + UINTN RealPages;
> +
> + //
> + // Alignment must be a power of two or zero.
> + //
> + ASSERT ((Alignment & (Alignment - 1)) == 0);
> +
> + if (Pages == 0) {
> + return EFI_INVALID_PARAMETER;
> + }
> + if (Alignment > EFI_PAGE_SIZE) {
> + //
> + // Calculate the total number of pages since alignment is larger than page
> size.
> + //
> + AlignmentMask = Alignment - 1;
> + RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
> + //
> + // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not
> overflow.
> + //
> + ASSERT (RealPages > Pages);
> +
> + Memory = *Address;
> + Status = gBS->AllocatePages (Type, MemoryType, RealPages,
> &Memory);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + AlignedMemory = ((UINTN) Memory + AlignmentMask) &
> ~AlignmentMask;
> + UnalignedPages = EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN)
> Memory);
> + if (UnalignedPages > 0) {
> + //
> + // Free first unaligned page(s).
> + //
> + Status = gBS->FreePages (Memory, UnalignedPages);
> + ASSERT_EFI_ERROR (Status);
> + }
> + Memory = (EFI_PHYSICAL_ADDRESS) (AlignedMemory +
> EFI_PAGES_TO_SIZE (Pages));
> + UnalignedPages = RealPages - Pages - UnalignedPages;
> + if (UnalignedPages > 0) {
> + //
> + // Free last unaligned page(s).
> + //
> + Status = gBS->FreePages (Memory, UnalignedPages);
> + ASSERT_EFI_ERROR (Status);
> + }
> + } else {
> + //
> + // Do not over-allocate pages in this case.
> + //
> + Memory = *Address;
> + Status = gBS->AllocatePages (Type, MemoryType, Pages, &Memory);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + AlignedMemory = (UINTN) Memory;
> + }
> + *Address = AlignedMemory;
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Return if a value is aligned.
> +
> + @param Value the value to be checked
> + @param Alignment the alignment to be checked with.
> +
> + @retval TRUE The value is aligned
> + @retval FALSE The value is not aligned.
> +**/
> +BOOLEAN
> +InternalIsAlgined (
> + IN UINTN Value,
> + IN UINTN Alignment
> + )
> +{
> + if (Value == ALIGN_VALUE(Value, Alignment)) {
> + return TRUE;
> + } else {
> + return FALSE;
> + }
> +}
> +
> +/**
> Provides the PCI controller-specific address needed to access
> system memory for DMA.
>
> @@ -1057,6 +1172,9 @@ RootBridgeIoMap (
> PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
> EFI_PHYSICAL_ADDRESS PhysicalAddress;
> MAP_INFO *MapInfo;
> + EFI_PHYSICAL_ADDRESS MaxAddress;
> + BOOLEAN NeedMap;
> + BOOLEAN NeedAllocateNonExisting;
>
> if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress ==
> NULL ||
> Mapping == NULL) {
> @@ -1072,13 +1190,41 @@ RootBridgeIoMap (
>
> RootBridge = ROOT_BRIDGE_FROM_THIS (This);
>
> + if (gIoMmuProtocol == NULL) {
> + gBS->LocateProtocol (
> + &gEdkiiIoMmuProtocolGuid,
> + NULL,
> + (VOID **) &gIoMmuProtocol
> + );
> + if (gIoMmuProtocol != NULL) {
> + gIoMmuProtocol->GetPageSize (gIoMmuProtocol, &mIoMmuPageSize);
> + ASSERT ((mIoMmuPageSize & (mIoMmuPageSize - 1)) == 0);
> + }
> + }
> +
> PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
> +
> + MaxAddress = (EFI_PHYSICAL_ADDRESS)-1; NeedMap = FALSE;
> + NeedAllocateNonExisting = FALSE;
> +
> if ((!RootBridge->DmaAbove4G ||
> (Operation != EfiPciOperationBusMasterRead64 &&
> Operation != EfiPciOperationBusMasterWrite64 &&
> Operation != EfiPciOperationBusMasterCommonBuffer64)) &&
> ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) {
> + NeedMap = TRUE;
> + MaxAddress = SIZE_4GB - 1;
> + }
>
> + if (gIoMmuProtocol != NULL) {
> + if ((!InternalIsAlgined (*NumberOfBytes, mIoMmuPageSize)) ||
> + (!InternalIsAlgined ((UINTN)HostAddress, mIoMmuPageSize))) {
> + NeedMap = TRUE;
> + }
> + }
> +
> + if (NeedMap) {
> //
> // If the root bridge or the device cannot handle performing DMA above
> // 4GB but any part of the DMA transfer being mapped is above 4GB, then
> @@ -1090,9 +1236,17 @@ RootBridgeIoMap (
> //
> // Common Buffer operations can not be remapped. If the common
> buffer
> // if above 4GB, then it is not possible to generate a mapping, so return
> - // an error.
> + // an error if there is no IOMMU.
> //
> - return EFI_UNSUPPORTED;
> + if (gIoMmuProtocol == NULL) {
> + return EFI_UNSUPPORTED;
> + } else {
> + //
> + // We can try to allocate non-existing memory for below 4GiB address
> + // and use IOMMU to remap it.
> + //
> + NeedAllocateNonExisting = TRUE;
> + }
> }
>
> //
> @@ -1113,21 +1267,81 @@ RootBridgeIoMap (
> MapInfo->NumberOfBytes = *NumberOfBytes;
> MapInfo->NumberOfPages = EFI_SIZE_TO_PAGES (MapInfo-
> >NumberOfBytes);
> MapInfo->HostAddress = PhysicalAddress;
> - MapInfo->MappedHostAddress = SIZE_4GB - 1;
> + MapInfo->MappedHostAddress = MaxAddress;
> + MapInfo->MappedNumberOfBytes = ALIGN_VALUE (MapInfo-
> >NumberOfBytes, mIoMmuPageSize);
> + MapInfo->MappedNumberOfPages = EFI_SIZE_TO_PAGES (MapInfo-
> >MappedNumberOfBytes);
> + MapInfo->RemapNonExisting = FALSE;
>
> - //
> - // Allocate a buffer below 4GB to map the transfer to.
> - //
> - Status = gBS->AllocatePages (
> - AllocateMaxAddress,
> - EfiBootServicesData,
> - MapInfo->NumberOfPages,
> - &MapInfo->MappedHostAddress
> - );
> - if (EFI_ERROR (Status)) {
> - FreePool (MapInfo);
> - *NumberOfBytes = 0;
> - return Status;
> + if (!((Operation == EfiPciOperationBusMasterCommonBuffer) ||
> + (Operation == EfiPciOperationBusMasterCommonBuffer64))) {
> + //
> + // Allocate a buffer below 4GB to map the transfer to.
> + //
> + Status = InternalAllocateAlignedPagesWithAllocateType (
> + AllocateMaxAddress,
> + EfiBootServicesData,
> + MapInfo->MappedNumberOfPages,
> + mIoMmuPageSize,
> + &MapInfo->MappedHostAddress
> + );
> + if (EFI_ERROR (Status)) {
> + if (gIoMmuProtocol == NULL) {
> + FreePool (MapInfo);
> + *NumberOfBytes = 0;
> + return Status;
> + }
> + //
> + // We can try to allocate non-existing memory for below 4GiB address
> + // and use IOMMU to remap it.
> + //
> + NeedAllocateNonExisting = TRUE;
> + }
> + }
> +
> + if (NeedAllocateNonExisting) {
> + if ((!InternalIsAlgined (*NumberOfBytes, mIoMmuPageSize)) ||
> + (!InternalIsAlgined ((UINTN)HostAddress, mIoMmuPageSize))) {
> + //
> + // If original memory is not IOMMU aligned, we cannot remap it.
> + //
> + FreePool (MapInfo);
> + *NumberOfBytes = 0;
> + return EFI_UNSUPPORTED;
> + }
> + //
> + // If the code runs to here, it means:
> + // 1) We need remap a common buffer to below 4G non-existing
> memory.
> + // 2) We need rempa a read/write buffer to below 4G non-existing
> memory.
> + //
> + MapInfo->MappedHostAddress = MaxAddress;
> + Status = gDS->AllocateMemorySpace (
> + EfiGcdAllocateMaxAddressSearchTopDown,
> + EfiGcdMemoryTypeNonExistent,
> + mIoMmuPageSize,
> + MapInfo->MappedNumberOfBytes,
> + &MapInfo->MappedHostAddress,
> + gImageHandle,
> + RootBridge->Handle
> + );
> + if (EFI_ERROR(Status)) {
> + FreePool (MapInfo);
> + *NumberOfBytes = 0;
> + return Status;
> + }
> + Status = gIoMmuProtocol->SetRemapAddress (
> + gIoMmuProtocol,
> + NULL,
> + MapInfo->MappedHostAddress,
> + (VOID *)(UINTN)MapInfo->HostAddress,
> + MapInfo->MappedNumberOfBytes
> + );
> + if (EFI_ERROR(Status)) {
> + gDS->FreeMemorySpace (MapInfo->MappedHostAddress, MapInfo-
> >MappedNumberOfBytes);
> + FreePool (MapInfo);
> + *NumberOfBytes = 0;
> + return Status;
> + }
> + MapInfo->RemapNonExisting = TRUE;
> }
>
> //
> @@ -1135,19 +1349,26 @@ RootBridgeIoMap (
> // then copy the contents of the real buffer into the mapped buffer
> // so the Bus Master can read the contents of the real buffer.
> //
> - if (Operation == EfiPciOperationBusMasterRead ||
> - Operation == EfiPciOperationBusMasterRead64) {
> - CopyMem (
> - (VOID *) (UINTN) MapInfo->MappedHostAddress,
> - (VOID *) (UINTN) MapInfo->HostAddress,
> - MapInfo->NumberOfBytes
> - );
> + // This action can be skipped if there is IOMMU, because PciBus does
> + // the action after setting IOMMU attributes.
> + //
> + if (gIoMmuProtocol == NULL) {
> + if (Operation == EfiPciOperationBusMasterRead ||
> + Operation == EfiPciOperationBusMasterRead64) {
> + CopyMem (
> + (VOID *) (UINTN) MapInfo->MappedHostAddress,
> + (VOID *) (UINTN) MapInfo->HostAddress,
> + MapInfo->NumberOfBytes
> + );
> + }
> }
>
> InsertTailList (&RootBridge->Maps, &MapInfo->Link);
>
> //
> // The DeviceAddress is the address of the maped buffer below 4GB
> + // NOTE: It can be a valid system memory address.
> + // Or a non-existing memory but mapped by IOMMU.
> //
> *DeviceAddress = MapInfo->MappedHostAddress;
> //
> @@ -1228,19 +1449,42 @@ RootBridgeIoUnmap (
> // then copy the contents of the mapped buffer into the real buffer
> // so the processor can read the contents of the real buffer.
> //
> - if (MapInfo->Operation == EfiPciOperationBusMasterWrite ||
> - MapInfo->Operation == EfiPciOperationBusMasterWrite64) {
> - CopyMem (
> - (VOID *) (UINTN) MapInfo->HostAddress,
> - (VOID *) (UINTN) MapInfo->MappedHostAddress,
> - MapInfo->NumberOfBytes
> - );
> + // This action can be skipped if there is IOMMU, because PciBus does
> + // the action.
> + //
> + if (gIoMmuProtocol == NULL) {
> + if (MapInfo->Operation == EfiPciOperationBusMasterWrite ||
> + MapInfo->Operation == EfiPciOperationBusMasterWrite64) {
> + CopyMem (
> + (VOID *) (UINTN) MapInfo->HostAddress,
> + (VOID *) (UINTN) MapInfo->MappedHostAddress,
> + MapInfo->NumberOfBytes
> + );
> + }
> + }
> +
> + if (gIoMmuProtocol != NULL) {
> + //
> + // Free GCD non existing memory.
> + //
> + if (MapInfo->RemapNonExisting) {
> + gIoMmuProtocol->SetRemapAddress (
> + gIoMmuProtocol,
> + NULL,
> + MapInfo->MappedHostAddress,
> + (VOID *)(UINTN)MapInfo->MappedHostAddress,
> + MapInfo->MappedNumberOfBytes
> + );
> + gDS->FreeMemorySpace (MapInfo->MappedHostAddress, MapInfo-
> >MappedNumberOfBytes);
> + FreePool (Mapping);
> + return EFI_SUCCESS;
> + }
> }
>
> //
> // Free the mapped buffer and the MAP_INFO structure.
> //
> - gBS->FreePages (MapInfo->MappedHostAddress, MapInfo-
> >NumberOfPages);
> + gBS->FreePages (MapInfo->MappedHostAddress,
> + MapInfo->MappedNumberOfPages);
> FreePool (Mapping);
> return EFI_SUCCESS;
> }
> @@ -1285,7 +1529,7 @@ RootBridgeIoAllocateBuffer (
> EFI_STATUS Status;
> EFI_PHYSICAL_ADDRESS PhysicalAddress;
> PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
> - EFI_ALLOCATE_TYPE AllocateType;
> + UINTN Size;
>
> //
> // Validate Attributes
> @@ -1312,25 +1556,52 @@ RootBridgeIoAllocateBuffer (
>
> RootBridge = ROOT_BRIDGE_FROM_THIS (This);
>
> - AllocateType = AllocateAnyPages;
> + PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (-1);
> if (!RootBridge->DmaAbove4G ||
> (Attributes & EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) {
> //
> // Limit allocations to memory below 4GB
> //
> - AllocateType = AllocateMaxAddress;
> PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (SIZE_4GB - 1);
> }
> - Status = gBS->AllocatePages (
> - AllocateType,
> + if (gIoMmuProtocol != NULL) {
> + Size = EFI_PAGES_TO_SIZE(Pages);
> + Size = ALIGN_VALUE(EFI_PAGES_TO_SIZE(Pages), mIoMmuPageSize);
> + Pages = EFI_SIZE_TO_PAGES (Size);
> + }
> + Status = InternalAllocateAlignedPagesWithAllocateType (
> + AllocateMaxAddress,
> MemoryType,
> Pages,
> + mIoMmuPageSize,
> &PhysicalAddress
> );
> if (!EFI_ERROR (Status)) {
> *HostAddress = (VOID *) (UINTN) PhysicalAddress;
> + return EFI_SUCCESS;
> }
>
> + if (gIoMmuProtocol != NULL) {
> + //
> + // Try to allocate AnyAddress here.
> + //
> + // We can try to allocate non-existing memory for below 4GiB address
> + // and use IOMMU to remap it later.
> + //
> + PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (-1);
> + Status = InternalAllocateAlignedPagesWithAllocateType (
> + AllocateMaxAddress,
> + MemoryType,
> + Pages,
> + mIoMmuPageSize,
> + &PhysicalAddress
> + );
> + if (EFI_ERROR(Status)) {
> + return Status;
> + }
> + *HostAddress = (VOID *) (UINTN) PhysicalAddress;
> + return EFI_SUCCESS;
> + }
> return Status;
> }
>
> @@ -1356,6 +1627,13 @@ RootBridgeIoFreeBuffer (
> OUT VOID *HostAddress
> )
> {
> + UINTN Size;
> +
> + if (gIoMmuProtocol != NULL) {
> + Size = EFI_PAGES_TO_SIZE(Pages);
> + Size = ALIGN_VALUE(EFI_PAGES_TO_SIZE(Pages), mIoMmuPageSize);
> + Pages = EFI_SIZE_TO_PAGES (Size);
> + }
> return gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress,
> Pages); }
>
> --
> 2.7.4.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC] [PATCH V3 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support.
2017-04-17 19:33 ` Duran, Leo
@ 2017-04-18 3:31 ` Yao, Jiewen
0 siblings, 0 replies; 10+ messages in thread
From: Yao, Jiewen @ 2017-04-18 3:31 UTC (permalink / raw)
To: Duran, Leo, edk2-devel@lists.01.org
Cc: Ni, Ruiyu, Singh, Brijesh, Ard Biesheuvel
Thanks Leo.
Answer below:
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo
Sent: Tuesday, April 18, 2017 3:33 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Singh, Brijesh <brijesh.singh@amd.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [edk2] [RFC] [PATCH V3 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support.
Hi Yao,
Just a couple of quick comments:
1) gIoMmuProtocol is declared (global) by PciHostBridge.c, but it is not initialized to NULL.
[Jiewen] Our PECOFF loader zero it. I do not believe it will bring any real impact.
But I agree with you that we can explicit assign to NULL to avoid confusing.
2) Would it be OK to do a one-time LocateProtocol() of gIoMmuProtocol in InitializePciHostBridge()?
BTW, besides the global declaration, gIoMmuProtocol is currently not used by PciHostBridge.c
[Jiewen] It is hard, because of dependency.
We are not able to guarantee IOMMU driver runs before PciHostBridge.
We cannot add dependency for PciHostBridge, because IOMMU is an optional feature.
Leo
> -----Original Message-----
> From: Jiewen Yao [mailto:jiewen.yao@intel.com]
> Sent: Tuesday, April 04, 2017 2:06 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: [RFC] [PATCH V3 2/3] MdeModulePkg/PciHostBridge: Add IOMMU
> support.
>
> The responsibility of PciHostBridge is to allocate IOMMU page aligned
> memory for Map and AllocateBuffer, because PciHostBridge driver already
> handles Map() request to allocate another buffer for DMA read/write.
>
> If the max address requirement can not be satisfied, PciHostBridge may also
> allocate any IOMMU page aligned memory and use IOMMU Remapping
> feature to map to lower address to satisfy device requirement.
>
> PciHostBridge does not set IOMMU 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<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>>
> ---
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 3 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 1 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h | 8 +
> MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 350
> ++++++++++++++++++--
> 4 files changed, 326 insertions(+), 36 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> index 9005dee..35233a7 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> @@ -28,6 +28,9 @@ GLOBAL_REMOVE_IF_UNREFERENCED CHAR16
> *mPciResourceTypeStr[] = {
> L"I/O", L"Mem", L"PMem", L"Mem64", L"PMem64", L"Bus"
> };
>
> +EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
> +UINTN mIoMmuPageSize = 1;
> +
> /**
> Ensure the compatibility of an IO space descriptor with the IO aperture.
>
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> index d8b0439..2d3c8c9 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
> @@ -49,6 +49,7 @@
> gEfiDevicePathProtocolGuid ## BY_START
> gEfiPciRootBridgeIoProtocolGuid ## BY_START
> gEfiPciHostBridgeResourceAllocationProtocolGuid ## BY_START
> + gEdkiiIoMmuProtocolGuid ## CONSUMES
>
> [Depex]
> gEfiCpuIo2ProtocolGuid AND
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
> index 13185b4..77c3490 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>
> @@ -50,8 +51,11 @@ typedef struct {
> EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION Operation;
> UINTN NumberOfBytes;
> UINTN NumberOfPages;
> + UINTN MappedNumberOfBytes;
> + UINTN MappedNumberOfPages;
> EFI_PHYSICAL_ADDRESS HostAddress;
> EFI_PHYSICAL_ADDRESS MappedHostAddress;
> + BOOLEAN RemapNonExisting;
> } MAP_INFO;
> #define MAP_INFO_FROM_LINK(a) CR (a, MAP_INFO, Link,
> MAP_INFO_SIGNATURE)
>
> @@ -575,4 +579,8 @@ RootBridgeIoConfiguration (
>
> extern EFI_METRONOME_ARCH_PROTOCOL *mMetronome;
> extern EFI_CPU_IO2_PROTOCOL *mCpuIo;
> +
> +extern EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
> +extern UINTN mIoMmuPageSize;
> +
> #endif
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> index 8af131b..2a17eb1 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
> @@ -1022,6 +1022,121 @@ RootBridgeIoPciWrite ( }
>
> /**
> + Allocates one or more 4KB pages of a certain memory type at a specified
> alignment.
> +
> + Allocates the number of 4KB pages specified by Pages of a certain
> + memory type with an alignment specified by Alignment. The allocated
> buffer is returned. If Pages is 0, then NULL is returned.
> + If there is not enough memory at the specified alignment remaining to
> + satisfy the request, then NULL is returned.
> + If Alignment is not a power of two and Alignment is not zero, then
> ASSERT().
> + If Pages plus EFI_SIZE_TO_PAGES (Alignment) overflows, then ASSERT().
> +
> + @param Type The type of allocation to perform.
> + @param MemoryType The type of memory to allocate.
> + @param Pages The number of 4 KB pages to allocate.
> + @param Alignment The requested alignment of the allocation.
> Must be a power of two.
> + If Alignment is zero, then byte alignment is used.
> + @param Address Pointer to a physical address.
> +
> + @return Memory Allocation Status.
> +
> +**/
> +EFI_STATUS
> +InternalAllocateAlignedPagesWithAllocateType (
> + IN EFI_ALLOCATE_TYPE Type,
> + IN EFI_MEMORY_TYPE MemoryType,
> + IN UINTN Pages,
> + IN UINTN Alignment,
> + IN OUT EFI_PHYSICAL_ADDRESS *Address
> + )
> +{
> + EFI_STATUS Status;
> + EFI_PHYSICAL_ADDRESS Memory;
> + UINTN AlignedMemory;
> + UINTN AlignmentMask;
> + UINTN UnalignedPages;
> + UINTN RealPages;
> +
> + //
> + // Alignment must be a power of two or zero.
> + //
> + ASSERT ((Alignment & (Alignment - 1)) == 0);
> +
> + if (Pages == 0) {
> + return EFI_INVALID_PARAMETER;
> + }
> + if (Alignment > EFI_PAGE_SIZE) {
> + //
> + // Calculate the total number of pages since alignment is larger than page
> size.
> + //
> + AlignmentMask = Alignment - 1;
> + RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
> + //
> + // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not
> overflow.
> + //
> + ASSERT (RealPages > Pages);
> +
> + Memory = *Address;
> + Status = gBS->AllocatePages (Type, MemoryType, RealPages,
> &Memory);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + AlignedMemory = ((UINTN) Memory + AlignmentMask) &
> ~AlignmentMask;
> + UnalignedPages = EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN)
> Memory);
> + if (UnalignedPages > 0) {
> + //
> + // Free first unaligned page(s).
> + //
> + Status = gBS->FreePages (Memory, UnalignedPages);
> + ASSERT_EFI_ERROR (Status);
> + }
> + Memory = (EFI_PHYSICAL_ADDRESS) (AlignedMemory +
> EFI_PAGES_TO_SIZE (Pages));
> + UnalignedPages = RealPages - Pages - UnalignedPages;
> + if (UnalignedPages > 0) {
> + //
> + // Free last unaligned page(s).
> + //
> + Status = gBS->FreePages (Memory, UnalignedPages);
> + ASSERT_EFI_ERROR (Status);
> + }
> + } else {
> + //
> + // Do not over-allocate pages in this case.
> + //
> + Memory = *Address;
> + Status = gBS->AllocatePages (Type, MemoryType, Pages, &Memory);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> + AlignedMemory = (UINTN) Memory;
> + }
> + *Address = AlignedMemory;
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Return if a value is aligned.
> +
> + @param Value the value to be checked
> + @param Alignment the alignment to be checked with.
> +
> + @retval TRUE The value is aligned
> + @retval FALSE The value is not aligned.
> +**/
> +BOOLEAN
> +InternalIsAlgined (
> + IN UINTN Value,
> + IN UINTN Alignment
> + )
> +{
> + if (Value == ALIGN_VALUE(Value, Alignment)) {
> + return TRUE;
> + } else {
> + return FALSE;
> + }
> +}
> +
> +/**
> Provides the PCI controller-specific address needed to access
> system memory for DMA.
>
> @@ -1057,6 +1172,9 @@ RootBridgeIoMap (
> PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
> EFI_PHYSICAL_ADDRESS PhysicalAddress;
> MAP_INFO *MapInfo;
> + EFI_PHYSICAL_ADDRESS MaxAddress;
> + BOOLEAN NeedMap;
> + BOOLEAN NeedAllocateNonExisting;
>
> if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress ==
> NULL ||
> Mapping == NULL) {
> @@ -1072,13 +1190,41 @@ RootBridgeIoMap (
>
> RootBridge = ROOT_BRIDGE_FROM_THIS (This);
>
> + if (gIoMmuProtocol == NULL) {
> + gBS->LocateProtocol (
> + &gEdkiiIoMmuProtocolGuid,
> + NULL,
> + (VOID **) &gIoMmuProtocol
> + );
> + if (gIoMmuProtocol != NULL) {
> + gIoMmuProtocol->GetPageSize (gIoMmuProtocol, &mIoMmuPageSize);
> + ASSERT ((mIoMmuPageSize & (mIoMmuPageSize - 1)) == 0);
> + }
> + }
> +
> PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
> +
> + MaxAddress = (EFI_PHYSICAL_ADDRESS)-1; NeedMap = FALSE;
> + NeedAllocateNonExisting = FALSE;
> +
> if ((!RootBridge->DmaAbove4G ||
> (Operation != EfiPciOperationBusMasterRead64 &&
> Operation != EfiPciOperationBusMasterWrite64 &&
> Operation != EfiPciOperationBusMasterCommonBuffer64)) &&
> ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) {
> + NeedMap = TRUE;
> + MaxAddress = SIZE_4GB - 1;
> + }
>
> + if (gIoMmuProtocol != NULL) {
> + if ((!InternalIsAlgined (*NumberOfBytes, mIoMmuPageSize)) ||
> + (!InternalIsAlgined ((UINTN)HostAddress, mIoMmuPageSize))) {
> + NeedMap = TRUE;
> + }
> + }
> +
> + if (NeedMap) {
> //
> // If the root bridge or the device cannot handle performing DMA above
> // 4GB but any part of the DMA transfer being mapped is above 4GB, then
> @@ -1090,9 +1236,17 @@ RootBridgeIoMap (
> //
> // Common Buffer operations can not be remapped. If the common
> buffer
> // if above 4GB, then it is not possible to generate a mapping, so return
> - // an error.
> + // an error if there is no IOMMU.
> //
> - return EFI_UNSUPPORTED;
> + if (gIoMmuProtocol == NULL) {
> + return EFI_UNSUPPORTED;
> + } else {
> + //
> + // We can try to allocate non-existing memory for below 4GiB address
> + // and use IOMMU to remap it.
> + //
> + NeedAllocateNonExisting = TRUE;
> + }
> }
>
> //
> @@ -1113,21 +1267,81 @@ RootBridgeIoMap (
> MapInfo->NumberOfBytes = *NumberOfBytes;
> MapInfo->NumberOfPages = EFI_SIZE_TO_PAGES (MapInfo-
> >NumberOfBytes);
> MapInfo->HostAddress = PhysicalAddress;
> - MapInfo->MappedHostAddress = SIZE_4GB - 1;
> + MapInfo->MappedHostAddress = MaxAddress;
> + MapInfo->MappedNumberOfBytes = ALIGN_VALUE (MapInfo-
> >NumberOfBytes, mIoMmuPageSize);
> + MapInfo->MappedNumberOfPages = EFI_SIZE_TO_PAGES (MapInfo-
> >MappedNumberOfBytes);
> + MapInfo->RemapNonExisting = FALSE;
>
> - //
> - // Allocate a buffer below 4GB to map the transfer to.
> - //
> - Status = gBS->AllocatePages (
> - AllocateMaxAddress,
> - EfiBootServicesData,
> - MapInfo->NumberOfPages,
> - &MapInfo->MappedHostAddress
> - );
> - if (EFI_ERROR (Status)) {
> - FreePool (MapInfo);
> - *NumberOfBytes = 0;
> - return Status;
> + if (!((Operation == EfiPciOperationBusMasterCommonBuffer) ||
> + (Operation == EfiPciOperationBusMasterCommonBuffer64))) {
> + //
> + // Allocate a buffer below 4GB to map the transfer to.
> + //
> + Status = InternalAllocateAlignedPagesWithAllocateType (
> + AllocateMaxAddress,
> + EfiBootServicesData,
> + MapInfo->MappedNumberOfPages,
> + mIoMmuPageSize,
> + &MapInfo->MappedHostAddress
> + );
> + if (EFI_ERROR (Status)) {
> + if (gIoMmuProtocol == NULL) {
> + FreePool (MapInfo);
> + *NumberOfBytes = 0;
> + return Status;
> + }
> + //
> + // We can try to allocate non-existing memory for below 4GiB address
> + // and use IOMMU to remap it.
> + //
> + NeedAllocateNonExisting = TRUE;
> + }
> + }
> +
> + if (NeedAllocateNonExisting) {
> + if ((!InternalIsAlgined (*NumberOfBytes, mIoMmuPageSize)) ||
> + (!InternalIsAlgined ((UINTN)HostAddress, mIoMmuPageSize))) {
> + //
> + // If original memory is not IOMMU aligned, we cannot remap it.
> + //
> + FreePool (MapInfo);
> + *NumberOfBytes = 0;
> + return EFI_UNSUPPORTED;
> + }
> + //
> + // If the code runs to here, it means:
> + // 1) We need remap a common buffer to below 4G non-existing
> memory.
> + // 2) We need rempa a read/write buffer to below 4G non-existing
> memory.
> + //
> + MapInfo->MappedHostAddress = MaxAddress;
> + Status = gDS->AllocateMemorySpace (
> + EfiGcdAllocateMaxAddressSearchTopDown,
> + EfiGcdMemoryTypeNonExistent,
> + mIoMmuPageSize,
> + MapInfo->MappedNumberOfBytes,
> + &MapInfo->MappedHostAddress,
> + gImageHandle,
> + RootBridge->Handle
> + );
> + if (EFI_ERROR(Status)) {
> + FreePool (MapInfo);
> + *NumberOfBytes = 0;
> + return Status;
> + }
> + Status = gIoMmuProtocol->SetRemapAddress (
> + gIoMmuProtocol,
> + NULL,
> + MapInfo->MappedHostAddress,
> + (VOID *)(UINTN)MapInfo->HostAddress,
> + MapInfo->MappedNumberOfBytes
> + );
> + if (EFI_ERROR(Status)) {
> + gDS->FreeMemorySpace (MapInfo->MappedHostAddress, MapInfo-
> >MappedNumberOfBytes);
> + FreePool (MapInfo);
> + *NumberOfBytes = 0;
> + return Status;
> + }
> + MapInfo->RemapNonExisting = TRUE;
> }
>
> //
> @@ -1135,19 +1349,26 @@ RootBridgeIoMap (
> // then copy the contents of the real buffer into the mapped buffer
> // so the Bus Master can read the contents of the real buffer.
> //
> - if (Operation == EfiPciOperationBusMasterRead ||
> - Operation == EfiPciOperationBusMasterRead64) {
> - CopyMem (
> - (VOID *) (UINTN) MapInfo->MappedHostAddress,
> - (VOID *) (UINTN) MapInfo->HostAddress,
> - MapInfo->NumberOfBytes
> - );
> + // This action can be skipped if there is IOMMU, because PciBus does
> + // the action after setting IOMMU attributes.
> + //
> + if (gIoMmuProtocol == NULL) {
> + if (Operation == EfiPciOperationBusMasterRead ||
> + Operation == EfiPciOperationBusMasterRead64) {
> + CopyMem (
> + (VOID *) (UINTN) MapInfo->MappedHostAddress,
> + (VOID *) (UINTN) MapInfo->HostAddress,
> + MapInfo->NumberOfBytes
> + );
> + }
> }
>
> InsertTailList (&RootBridge->Maps, &MapInfo->Link);
>
> //
> // The DeviceAddress is the address of the maped buffer below 4GB
> + // NOTE: It can be a valid system memory address.
> + // Or a non-existing memory but mapped by IOMMU.
> //
> *DeviceAddress = MapInfo->MappedHostAddress;
> //
> @@ -1228,19 +1449,42 @@ RootBridgeIoUnmap (
> // then copy the contents of the mapped buffer into the real buffer
> // so the processor can read the contents of the real buffer.
> //
> - if (MapInfo->Operation == EfiPciOperationBusMasterWrite ||
> - MapInfo->Operation == EfiPciOperationBusMasterWrite64) {
> - CopyMem (
> - (VOID *) (UINTN) MapInfo->HostAddress,
> - (VOID *) (UINTN) MapInfo->MappedHostAddress,
> - MapInfo->NumberOfBytes
> - );
> + // This action can be skipped if there is IOMMU, because PciBus does
> + // the action.
> + //
> + if (gIoMmuProtocol == NULL) {
> + if (MapInfo->Operation == EfiPciOperationBusMasterWrite ||
> + MapInfo->Operation == EfiPciOperationBusMasterWrite64) {
> + CopyMem (
> + (VOID *) (UINTN) MapInfo->HostAddress,
> + (VOID *) (UINTN) MapInfo->MappedHostAddress,
> + MapInfo->NumberOfBytes
> + );
> + }
> + }
> +
> + if (gIoMmuProtocol != NULL) {
> + //
> + // Free GCD non existing memory.
> + //
> + if (MapInfo->RemapNonExisting) {
> + gIoMmuProtocol->SetRemapAddress (
> + gIoMmuProtocol,
> + NULL,
> + MapInfo->MappedHostAddress,
> + (VOID *)(UINTN)MapInfo->MappedHostAddress,
> + MapInfo->MappedNumberOfBytes
> + );
> + gDS->FreeMemorySpace (MapInfo->MappedHostAddress, MapInfo-
> >MappedNumberOfBytes);
> + FreePool (Mapping);
> + return EFI_SUCCESS;
> + }
> }
>
> //
> // Free the mapped buffer and the MAP_INFO structure.
> //
> - gBS->FreePages (MapInfo->MappedHostAddress, MapInfo-
> >NumberOfPages);
> + gBS->FreePages (MapInfo->MappedHostAddress,
> + MapInfo->MappedNumberOfPages);
> FreePool (Mapping);
> return EFI_SUCCESS;
> }
> @@ -1285,7 +1529,7 @@ RootBridgeIoAllocateBuffer (
> EFI_STATUS Status;
> EFI_PHYSICAL_ADDRESS PhysicalAddress;
> PCI_ROOT_BRIDGE_INSTANCE *RootBridge;
> - EFI_ALLOCATE_TYPE AllocateType;
> + UINTN Size;
>
> //
> // Validate Attributes
> @@ -1312,25 +1556,52 @@ RootBridgeIoAllocateBuffer (
>
> RootBridge = ROOT_BRIDGE_FROM_THIS (This);
>
> - AllocateType = AllocateAnyPages;
> + PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (-1);
> if (!RootBridge->DmaAbove4G ||
> (Attributes & EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) {
> //
> // Limit allocations to memory below 4GB
> //
> - AllocateType = AllocateMaxAddress;
> PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (SIZE_4GB - 1);
> }
> - Status = gBS->AllocatePages (
> - AllocateType,
> + if (gIoMmuProtocol != NULL) {
> + Size = EFI_PAGES_TO_SIZE(Pages);
> + Size = ALIGN_VALUE(EFI_PAGES_TO_SIZE(Pages), mIoMmuPageSize);
> + Pages = EFI_SIZE_TO_PAGES (Size);
> + }
> + Status = InternalAllocateAlignedPagesWithAllocateType (
> + AllocateMaxAddress,
> MemoryType,
> Pages,
> + mIoMmuPageSize,
> &PhysicalAddress
> );
> if (!EFI_ERROR (Status)) {
> *HostAddress = (VOID *) (UINTN) PhysicalAddress;
> + return EFI_SUCCESS;
> }
>
> + if (gIoMmuProtocol != NULL) {
> + //
> + // Try to allocate AnyAddress here.
> + //
> + // We can try to allocate non-existing memory for below 4GiB address
> + // and use IOMMU to remap it later.
> + //
> + PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (-1);
> + Status = InternalAllocateAlignedPagesWithAllocateType (
> + AllocateMaxAddress,
> + MemoryType,
> + Pages,
> + mIoMmuPageSize,
> + &PhysicalAddress
> + );
> + if (EFI_ERROR(Status)) {
> + return Status;
> + }
> + *HostAddress = (VOID *) (UINTN) PhysicalAddress;
> + return EFI_SUCCESS;
> + }
> return Status;
> }
>
> @@ -1356,6 +1627,13 @@ RootBridgeIoFreeBuffer (
> OUT VOID *HostAddress
> )
> {
> + UINTN Size;
> +
> + if (gIoMmuProtocol != NULL) {
> + Size = EFI_PAGES_TO_SIZE(Pages);
> + Size = ALIGN_VALUE(EFI_PAGES_TO_SIZE(Pages), mIoMmuPageSize);
> + Pages = EFI_SIZE_TO_PAGES (Size);
> + }
> return gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress,
> Pages); }
>
> --
> 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
* [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: Add IOMMU support.
2017-04-04 7:06 [RFC] [PATCH V3 0/3] Add IOMMU support Jiewen Yao
2017-04-04 7:06 ` [RFC] [PATCH V3 1/3] MdeModulePkg/Include: Add IOMMU protocol definition Jiewen Yao
2017-04-04 7:06 ` [RFC] [PATCH V3 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support Jiewen Yao
@ 2017-04-04 7:06 ` Jiewen Yao
2017-04-17 19:58 ` Duran, Leo
2 siblings, 1 reply; 10+ messages in thread
From: Jiewen Yao @ 2017-04-04 7:06 UTC (permalink / raw)
To: edk2-devel; +Cc: Ruiyu Ni, Leo Duran, Brijesh Singh, Ard Biesheuvel
The responsibility of PciBus driver is to set IOMMU attribute,
because only PciBus knows which device submits DMA access request.
PciBus driver guarantee that the request to PciHostBridge is
IOMMU page aligned memory, as such PciHostBridge can allocate
non-existent memory for device memory, to satisfy remap requirement.
PciBus driver does not assume device address is same
as the mapped host address, because IOMMU may remap it.
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 | 12 ++
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 19 ++
MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 225 +++++++++++++++++++-
4 files changed, 247 insertions(+), 10 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
index f3be47a..c9ee4de 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
@@ -42,6 +42,8 @@ UINT64 gAllZero = 0;
EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
+EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
+UINTN mIoMmuPageSize = 1;
GLOBAL_REMOVE_IF_UNREFERENCED EFI_PCI_HOTPLUG_REQUEST_PROTOCOL mPciHotPlugRequest = {
@@ -256,6 +258,16 @@ PciBusDriverBindingStart (
}
gBS->LocateProtocol (
+ &gEdkiiIoMmuProtocolGuid,
+ NULL,
+ (VOID **) &gIoMmuProtocol
+ );
+ if (gIoMmuProtocol != NULL) {
+ gIoMmuProtocol->GetPageSize (gIoMmuProtocol, &mIoMmuPageSize);
+ ASSERT ((mIoMmuPageSize & (mIoMmuPageSize - 1)) == 0);
+ }
+
+ gBS->LocateProtocol (
&gEfiIncompatiblePciDeviceSupportProtocolGuid,
NULL,
(VOID **) &gIncompatiblePciDeviceSupport
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 39ba8b9..185d89c 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>
@@ -289,6 +290,8 @@ struct _PCI_IO_DEVICE {
// This field is used to support this case.
//
UINT16 BridgeIoAlignment;
+
+ LIST_ENTRY Maps;
};
#define PCI_IO_DEVICE_FROM_PCI_IO_THIS(a) \
@@ -304,6 +307,20 @@ struct _PCI_IO_DEVICE {
CR (a, PCI_IO_DEVICE, LoadFile2, PCI_IO_DEVICE_SIGNATURE)
+#define PCI_IO_MAP_INFO_SIGNATURE SIGNATURE_32 ('p', 'm', 'a', 'p')
+typedef struct {
+ UINT32 Signature;
+ LIST_ENTRY Link;
+ EFI_PCI_IO_PROTOCOL_OPERATION Operation;
+ VOID *HostAddress;
+ EFI_PHYSICAL_ADDRESS DeviceAddress;
+ UINTN NumberOfBytes;
+ VOID *AlignedHostAddress;
+ UINTN AlignedNumberOfBytes;
+ VOID *MappedHostAddress;
+ VOID *PciRootBridgeIoMapping;
+} PCI_IO_MAP_INFO;
+#define PCI_IO_MAP_INFO_FROM_LINK(a) CR (a, PCI_IO_MAP_INFO, Link, PCI_IO_MAP_INFO_SIGNATURE)
//
// Global Variables
@@ -321,6 +338,8 @@ extern EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
extern EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
extern BOOLEAN mReserveIsaAliases;
extern BOOLEAN mReserveVgaAliases;
+extern EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
+extern UINTN mIoMmuPageSize;
/**
Macro that checks whether device is a GFX device.
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..31b8c32 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -58,6 +58,7 @@ InitializePciIoInstance (
)
{
CopyMem (&PciIoDevice->PciIo, &mPciIoInterface, sizeof (EFI_PCI_IO_PROTOCOL));
+ InitializeListHead (&PciIoDevice->Maps);
}
/**
@@ -936,6 +937,28 @@ PciIoCopyMem (
}
/**
+ Return if a value is aligned.
+
+ @param Value the value to be checked
+ @param Alignment the alignment to be checked with.
+
+ @retval TRUE The value is aligned
+ @retval FALSE The value is not aligned.
+**/
+BOOLEAN
+InternalIsAlgined (
+ IN UINTN Value,
+ IN UINTN Alignment
+ )
+{
+ if (Value == ALIGN_VALUE(Value, Alignment)) {
+ return TRUE;
+ } else {
+ return FALSE;
+ }
+}
+
+/**
Provides the PCI controller-specific addresses needed to access system memory.
@param This A pointer to the EFI_PCI_IO_PROTOCOL instance.
@@ -967,6 +990,9 @@ PciIoMap (
{
EFI_STATUS Status;
PCI_IO_DEVICE *PciIoDevice;
+ PCI_IO_MAP_INFO *PciIoMapInfo;
+ UINT64 IoMmuAttribute;
+ EFI_STATUS RemapStatus;
PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
@@ -982,15 +1008,60 @@ PciIoMap (
Operation = (EFI_PCI_IO_PROTOCOL_OPERATION) (Operation + EfiPciOperationBusMasterRead64);
}
- Status = PciIoDevice->PciRootBridgeIo->Map (
- PciIoDevice->PciRootBridgeIo,
- (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) Operation,
- HostAddress,
- NumberOfBytes,
- DeviceAddress,
- Mapping
- );
-
+ if (gIoMmuProtocol != NULL) {
+ PciIoMapInfo = AllocatePool (sizeof(*PciIoMapInfo));
+ if (PciIoMapInfo == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+ PciIoMapInfo->Signature = PCI_IO_MAP_INFO_SIGNATURE;
+ PciIoMapInfo->Operation = Operation;
+ PciIoMapInfo->NumberOfBytes = *NumberOfBytes;
+ PciIoMapInfo->DeviceAddress = *DeviceAddress;
+ PciIoMapInfo->HostAddress = HostAddress;
+ //
+ // For non common buffer, we always allocate a new memory if IOMMU exists.
+ // because the original memory might not be DMA capable.
+ //
+ // For common buffer, it is not needed, because common buffer allocate via PciIoAllocateBuffer.
+ // We cannot use AllocateAlignedPages here, because there might be more restriction in PciIoAllocateBuffer().
+ //
+ PciIoMapInfo->AlignedNumberOfBytes = ALIGN_VALUE (PciIoMapInfo->NumberOfBytes, mIoMmuPageSize);
+ if (PciIoMapInfo->Operation != EfiPciIoOperationBusMasterCommonBuffer) {
+ PciIoMapInfo->AlignedHostAddress = AllocateAlignedPages (EFI_SIZE_TO_PAGES(PciIoMapInfo->AlignedNumberOfBytes), mIoMmuPageSize);
+ if (PciIoMapInfo->AlignedHostAddress == NULL) {
+ FreePool (PciIoMapInfo);
+ return EFI_OUT_OF_RESOURCES;
+ }
+ } else {
+ //
+ // For common buffer, the HostAddress must be allocated via PciIoAllocateBuffer.
+ //
+ if (!InternalIsAlgined((UINTN)PciIoMapInfo->HostAddress, mIoMmuPageSize)) {
+ FreePool (PciIoMapInfo);
+ DEBUG ((DEBUG_ERROR, "PciIoMap - map unaligned common buffer with IOMMU\n"));
+ return EFI_UNSUPPORTED;
+ }
+ PciIoMapInfo->AlignedHostAddress = PciIoMapInfo->HostAddress;
+ }
+ PciIoMapInfo->PciRootBridgeIoMapping = NULL;
+ Status = PciIoDevice->PciRootBridgeIo->Map (
+ PciIoDevice->PciRootBridgeIo,
+ (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) Operation,
+ PciIoMapInfo->AlignedHostAddress,
+ &PciIoMapInfo->AlignedNumberOfBytes,
+ &PciIoMapInfo->DeviceAddress,
+ &PciIoMapInfo->PciRootBridgeIoMapping
+ );
+ } else {
+ Status = PciIoDevice->PciRootBridgeIo->Map (
+ PciIoDevice->PciRootBridgeIo,
+ (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) Operation,
+ HostAddress,
+ NumberOfBytes,
+ DeviceAddress,
+ Mapping
+ );
+ }
if (EFI_ERROR (Status)) {
REPORT_STATUS_CODE_WITH_DEVICE_PATH (
EFI_ERROR_CODE | EFI_ERROR_MINOR,
@@ -999,6 +1070,63 @@ PciIoMap (
);
}
+ if (gIoMmuProtocol != NULL) {
+ if (EFI_ERROR(Status)) {
+ if (PciIoMapInfo->Operation != EfiPciIoOperationBusMasterCommonBuffer) {
+ FreePages (PciIoMapInfo->AlignedHostAddress, EFI_SIZE_TO_PAGES(PciIoMapInfo->AlignedNumberOfBytes));
+ }
+ FreePool (PciIoMapInfo);
+ } else {
+ *DeviceAddress = PciIoMapInfo->DeviceAddress;
+ *Mapping = PciIoMapInfo;
+
+ switch (Operation) {
+ case EfiPciIoOperationBusMasterRead:
+ IoMmuAttribute = EDKII_IOMMU_ATTRIBUTE_READ;
+ break;
+ case EfiPciIoOperationBusMasterWrite:
+ IoMmuAttribute = EDKII_IOMMU_ATTRIBUTE_WRITE;
+ break;
+ case EfiPciIoOperationBusMasterCommonBuffer:
+ IoMmuAttribute = EDKII_IOMMU_ATTRIBUTE_READ | EDKII_IOMMU_ATTRIBUTE_WRITE;
+ break;
+ default:
+ ASSERT(FALSE);
+ return EFI_INVALID_PARAMETER;
+ }
+ //
+ // PciHostBridge should map IOMMU page aligned HostAddress.
+ //
+ gIoMmuProtocol->SetAttribute (
+ gIoMmuProtocol,
+ PciIoDevice->Handle,
+ PciIoMapInfo->DeviceAddress,
+ PciIoMapInfo->AlignedNumberOfBytes,
+ IoMmuAttribute
+ );
+ //
+ // We need do copy mem after IoMmu->SetAttribute(),
+ // because it might change IOMMU state.
+ //
+ RemapStatus = gIoMmuProtocol->GetRemapAddress (
+ gIoMmuProtocol,
+ PciIoDevice->Handle,
+ PciIoMapInfo->DeviceAddress,
+ &PciIoMapInfo->MappedHostAddress
+ );
+ if (EFI_ERROR(RemapStatus)) {
+ PciIoMapInfo->MappedHostAddress = (VOID *)(UINTN)PciIoMapInfo->DeviceAddress;
+ }
+ if (Operation == EfiPciIoOperationBusMasterRead) {
+ CopyMem (
+ PciIoMapInfo->MappedHostAddress,
+ PciIoMapInfo->HostAddress,
+ PciIoMapInfo->NumberOfBytes
+ );
+ }
+ InsertTailList (&PciIoDevice->Maps, &PciIoMapInfo->Link);
+ }
+ }
return Status;
}
@@ -1021,9 +1149,48 @@ PciIoUnmap (
{
EFI_STATUS Status;
PCI_IO_DEVICE *PciIoDevice;
+ PCI_IO_MAP_INFO *PciIoMapInfo;
+ LIST_ENTRY *Link;
PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
+ PciIoMapInfo = NULL;
+ if (gIoMmuProtocol != NULL) {
+ PciIoMapInfo = NULL;
+ for (Link = GetFirstNode (&PciIoDevice->Maps)
+ ; !IsNull (&PciIoDevice->Maps, Link)
+ ; Link = GetNextNode (&PciIoDevice->Maps, Link)
+ ) {
+ PciIoMapInfo = PCI_IO_MAP_INFO_FROM_LINK (Link);
+ if (PciIoMapInfo == Mapping) {
+ break;
+ }
+ }
+ //
+ // Mapping is not a valid value returned by Map()
+ //
+ if (PciIoMapInfo != Mapping) {
+ DEBUG ((DEBUG_INFO, "PciIoUnmap - PciIoMapInfo not found!\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+ RemoveEntryList (&PciIoMapInfo->Link);
+ Mapping = PciIoMapInfo->PciRootBridgeIoMapping;
+
+ //
+ // PciHostBridge should map IOMMU page aligned HostAddress.
+ //
+ // We need do copy mem before PciRootBridgeIo->Unmap(),
+ // because it might free mapped host address.
+ //
+ if (PciIoMapInfo->Operation == EfiPciIoOperationBusMasterWrite) {
+ CopyMem (
+ PciIoMapInfo->HostAddress,
+ PciIoMapInfo->MappedHostAddress,
+ PciIoMapInfo->NumberOfBytes
+ );
+ }
+ }
+
Status = PciIoDevice->PciRootBridgeIo->Unmap (
PciIoDevice->PciRootBridgeIo,
Mapping
@@ -1037,6 +1204,25 @@ PciIoUnmap (
);
}
+ if (gIoMmuProtocol != NULL) {
+ if (!EFI_ERROR(Status)) {
+ //
+ // PciHostBridge should map IOMMU page aligned HostAddress.
+ //
+ gIoMmuProtocol->SetAttribute (
+ gIoMmuProtocol,
+ PciIoDevice->Handle,
+ PciIoMapInfo->DeviceAddress,
+ PciIoMapInfo->AlignedNumberOfBytes,
+ 0
+ );
+ if (PciIoMapInfo->Operation != EfiPciIoOperationBusMasterCommonBuffer) {
+ FreePages (PciIoMapInfo->AlignedHostAddress, EFI_SIZE_TO_PAGES(PciIoMapInfo->AlignedNumberOfBytes));
+ }
+ FreePool (PciIoMapInfo);
+ }
+ }
+
return Status;
}
@@ -1073,6 +1259,7 @@ PciIoAllocateBuffer (
{
EFI_STATUS Status;
PCI_IO_DEVICE *PciIoDevice;
+ UINTN Size;
if ((Attributes &
(~(EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE | EFI_PCI_ATTRIBUTE_MEMORY_CACHED))) != 0){
@@ -1085,6 +1272,12 @@ PciIoAllocateBuffer (
Attributes |= EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
}
+ if (gIoMmuProtocol != NULL) {
+ Size = EFI_PAGES_TO_SIZE(Pages);
+ Size = ALIGN_VALUE(Size, mIoMmuPageSize);
+ Pages = EFI_SIZE_TO_PAGES (Size);
+ }
+
Status = PciIoDevice->PciRootBridgeIo->AllocateBuffer (
PciIoDevice->PciRootBridgeIo,
Type,
@@ -1101,7 +1294,9 @@ PciIoAllocateBuffer (
PciIoDevice->DevicePath
);
}
-
+ //
+ // No need to set attribute here, it is done in Map.
+ //
return Status;
}
@@ -1127,9 +1322,16 @@ PciIoFreeBuffer (
{
EFI_STATUS Status;
PCI_IO_DEVICE *PciIoDevice;
+ UINTN Size;
PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
+ if (gIoMmuProtocol != NULL) {
+ Size = EFI_PAGES_TO_SIZE(Pages);
+ Size = ALIGN_VALUE(Size, mIoMmuPageSize);
+ Pages = EFI_SIZE_TO_PAGES (Size);
+ }
+
Status = PciIoDevice->PciRootBridgeIo->FreeBuffer (
PciIoDevice->PciRootBridgeIo,
Pages,
@@ -1144,6 +1346,9 @@ PciIoFreeBuffer (
);
}
+ //
+ // No need to set attribute here, it is done in Unmap.
+ //
return Status;
}
--
2.7.4.windows.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: Add IOMMU support.
2017-04-04 7:06 ` [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: " Jiewen Yao
@ 2017-04-17 19:58 ` Duran, Leo
2017-04-18 3:45 ` Yao, Jiewen
0 siblings, 1 reply; 10+ messages in thread
From: Duran, Leo @ 2017-04-17 19:58 UTC (permalink / raw)
To: 'Jiewen Yao', edk2-devel@lists.01.org
Cc: Ruiyu Ni, Singh, Brijesh, Ard Biesheuvel
Hi Yao,
Regarding RootBridgeIoMap()
I'm wondering if may be possible to simplify the logic requiring flags "NeedMap" and "NeedAllocateNonExisting"?
For example, it seems like (NeedAllocateNonExisting==TRUE) implies (gIoMmuProtocol != NULL), but that did not seem obvious at a glance.
Leo.
> -----Original Message-----
> From: Jiewen Yao [mailto:jiewen.yao@intel.com]
> Sent: Tuesday, April 04, 2017 2:06 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: [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: Add IOMMU support.
>
> The responsibility of PciBus driver is to set IOMMU attribute, because only
> PciBus knows which device submits DMA access request.
>
> PciBus driver guarantee that the request to PciHostBridge is IOMMU page
> aligned memory, as such PciHostBridge can allocate non-existent memory for
> device memory, to satisfy remap requirement.
>
> PciBus driver does not assume device address is same as the mapped host
> address, because IOMMU may remap it.
>
> 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 | 12 ++
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 19 ++
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 225
> +++++++++++++++++++-
> 4 files changed, 247 insertions(+), 10 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> index f3be47a..c9ee4de 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> @@ -42,6 +42,8 @@ UINT64 gAllZero = 0;
>
> EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
> EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
> +EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
> +UINTN mIoMmuPageSize = 1;
>
>
> GLOBAL_REMOVE_IF_UNREFERENCED
> EFI_PCI_HOTPLUG_REQUEST_PROTOCOL mPciHotPlugRequest = { @@ -
> 256,6 +258,16 @@ PciBusDriverBindingStart (
> }
>
> gBS->LocateProtocol (
> + &gEdkiiIoMmuProtocolGuid,
> + NULL,
> + (VOID **) &gIoMmuProtocol
> + );
> + if (gIoMmuProtocol != NULL) {
> + gIoMmuProtocol->GetPageSize (gIoMmuProtocol, &mIoMmuPageSize);
> + ASSERT ((mIoMmuPageSize & (mIoMmuPageSize - 1)) == 0); }
> +
> + gBS->LocateProtocol (
> &gEfiIncompatiblePciDeviceSupportProtocolGuid,
> NULL,
> (VOID **) &gIncompatiblePciDeviceSupport diff --git
> a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> index 39ba8b9..185d89c 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> @@ -289,6 +290,8 @@ struct
> _PCI_IO_DEVICE {
> // This field is used to support this case.
> //
> UINT16 BridgeIoAlignment;
> +
> + LIST_ENTRY Maps;
> };
>
> #define PCI_IO_DEVICE_FROM_PCI_IO_THIS(a) \ @@ -304,6 +307,20 @@
> struct _PCI_IO_DEVICE {
> CR (a, PCI_IO_DEVICE, LoadFile2, PCI_IO_DEVICE_SIGNATURE)
>
>
> +#define PCI_IO_MAP_INFO_SIGNATURE SIGNATURE_32 ('p', 'm', 'a', 'p')
> +typedef struct {
> + UINT32 Signature;
> + LIST_ENTRY Link;
> + EFI_PCI_IO_PROTOCOL_OPERATION Operation;
> + VOID *HostAddress;
> + EFI_PHYSICAL_ADDRESS DeviceAddress;
> + UINTN NumberOfBytes;
> + VOID *AlignedHostAddress;
> + UINTN AlignedNumberOfBytes;
> + VOID *MappedHostAddress;
> + VOID *PciRootBridgeIoMapping;
> +} PCI_IO_MAP_INFO;
> +#define PCI_IO_MAP_INFO_FROM_LINK(a) CR (a, PCI_IO_MAP_INFO,
> Link,
> +PCI_IO_MAP_INFO_SIGNATURE)
>
> //
> // Global Variables
> @@ -321,6 +338,8 @@ extern EFI_PCI_PLATFORM_PROTOCOL
> *gPciPlatformProtocol;
> extern EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
> extern BOOLEAN mReserveIsaAliases;
> extern BOOLEAN mReserveVgaAliases;
> +extern EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
> +extern UINTN mIoMmuPageSize;
>
> /**
> Macro that checks whether device is a GFX device.
> 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..31b8c32 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> @@ -58,6 +58,7 @@ InitializePciIoInstance (
> )
> {
> CopyMem (&PciIoDevice->PciIo, &mPciIoInterface, sizeof
> (EFI_PCI_IO_PROTOCOL));
> + InitializeListHead (&PciIoDevice->Maps);
> }
>
> /**
> @@ -936,6 +937,28 @@ PciIoCopyMem (
> }
>
> /**
> + Return if a value is aligned.
> +
> + @param Value the value to be checked
> + @param Alignment the alignment to be checked with.
> +
> + @retval TRUE The value is aligned
> + @retval FALSE The value is not aligned.
> +**/
> +BOOLEAN
> +InternalIsAlgined (
> + IN UINTN Value,
> + IN UINTN Alignment
> + )
> +{
> + if (Value == ALIGN_VALUE(Value, Alignment)) {
> + return TRUE;
> + } else {
> + return FALSE;
> + }
> +}
> +
> +/**
> Provides the PCI controller-specific addresses needed to access system
> memory.
>
> @param This A pointer to the EFI_PCI_IO_PROTOCOL instance.
> @@ -967,6 +990,9 @@ PciIoMap (
> {
> EFI_STATUS Status;
> PCI_IO_DEVICE *PciIoDevice;
> + PCI_IO_MAP_INFO *PciIoMapInfo;
> + UINT64 IoMmuAttribute;
> + EFI_STATUS RemapStatus;
>
> PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
>
> @@ -982,15 +1008,60 @@ PciIoMap (
> Operation = (EFI_PCI_IO_PROTOCOL_OPERATION) (Operation +
> EfiPciOperationBusMasterRead64);
> }
>
> - Status = PciIoDevice->PciRootBridgeIo->Map (
> - PciIoDevice->PciRootBridgeIo,
> - (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION)
> Operation,
> - HostAddress,
> - NumberOfBytes,
> - DeviceAddress,
> - Mapping
> - );
> -
> + if (gIoMmuProtocol != NULL) {
> + PciIoMapInfo = AllocatePool (sizeof(*PciIoMapInfo));
> + if (PciIoMapInfo == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> + PciIoMapInfo->Signature = PCI_IO_MAP_INFO_SIGNATURE;
> + PciIoMapInfo->Operation = Operation;
> + PciIoMapInfo->NumberOfBytes = *NumberOfBytes;
> + PciIoMapInfo->DeviceAddress = *DeviceAddress;
> + PciIoMapInfo->HostAddress = HostAddress;
> + //
> + // For non common buffer, we always allocate a new memory if IOMMU
> exists.
> + // because the original memory might not be DMA capable.
> + //
> + // For common buffer, it is not needed, because common buffer allocate
> via PciIoAllocateBuffer.
> + // We cannot use AllocateAlignedPages here, because there might be
> more restriction in PciIoAllocateBuffer().
> + //
> + PciIoMapInfo->AlignedNumberOfBytes = ALIGN_VALUE (PciIoMapInfo-
> >NumberOfBytes, mIoMmuPageSize);
> + if (PciIoMapInfo->Operation !=
> EfiPciIoOperationBusMasterCommonBuffer) {
> + PciIoMapInfo->AlignedHostAddress = AllocateAlignedPages
> (EFI_SIZE_TO_PAGES(PciIoMapInfo->AlignedNumberOfBytes),
> mIoMmuPageSize);
> + if (PciIoMapInfo->AlignedHostAddress == NULL) {
> + FreePool (PciIoMapInfo);
> + return EFI_OUT_OF_RESOURCES;
> + }
> + } else {
> + //
> + // For common buffer, the HostAddress must be allocated via
> PciIoAllocateBuffer.
> + //
> + if (!InternalIsAlgined((UINTN)PciIoMapInfo->HostAddress,
> mIoMmuPageSize)) {
> + FreePool (PciIoMapInfo);
> + DEBUG ((DEBUG_ERROR, "PciIoMap - map unaligned common buffer
> with IOMMU\n"));
> + return EFI_UNSUPPORTED;
> + }
> + PciIoMapInfo->AlignedHostAddress = PciIoMapInfo->HostAddress;
> + }
> + PciIoMapInfo->PciRootBridgeIoMapping = NULL;
> + Status = PciIoDevice->PciRootBridgeIo->Map (
> + PciIoDevice->PciRootBridgeIo,
> +
> (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) Operation,
> + PciIoMapInfo->AlignedHostAddress,
> + &PciIoMapInfo->AlignedNumberOfBytes,
> + &PciIoMapInfo->DeviceAddress,
> + &PciIoMapInfo->PciRootBridgeIoMapping
> + ); } else {
> + Status = PciIoDevice->PciRootBridgeIo->Map (
> + PciIoDevice->PciRootBridgeIo,
> +
> (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) Operation,
> + HostAddress,
> + NumberOfBytes,
> + DeviceAddress,
> + Mapping
> + ); }
> if (EFI_ERROR (Status)) {
> REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> EFI_ERROR_CODE | EFI_ERROR_MINOR, @@ -999,6 +1070,63 @@
> PciIoMap (
> );
> }
>
> + if (gIoMmuProtocol != NULL) {
> + if (EFI_ERROR(Status)) {
> + if (PciIoMapInfo->Operation !=
> EfiPciIoOperationBusMasterCommonBuffer) {
> + FreePages (PciIoMapInfo->AlignedHostAddress,
> EFI_SIZE_TO_PAGES(PciIoMapInfo->AlignedNumberOfBytes));
> + }
> + FreePool (PciIoMapInfo);
> + } else {
> + *DeviceAddress = PciIoMapInfo->DeviceAddress;
> + *Mapping = PciIoMapInfo;
> +
> + switch (Operation) {
> + case EfiPciIoOperationBusMasterRead:
> + IoMmuAttribute = EDKII_IOMMU_ATTRIBUTE_READ;
> + break;
> + case EfiPciIoOperationBusMasterWrite:
> + IoMmuAttribute = EDKII_IOMMU_ATTRIBUTE_WRITE;
> + break;
> + case EfiPciIoOperationBusMasterCommonBuffer:
> + IoMmuAttribute = EDKII_IOMMU_ATTRIBUTE_READ |
> EDKII_IOMMU_ATTRIBUTE_WRITE;
> + break;
> + default:
> + ASSERT(FALSE);
> + return EFI_INVALID_PARAMETER;
> + }
> + //
> + // PciHostBridge should map IOMMU page aligned HostAddress.
> + //
> + gIoMmuProtocol->SetAttribute (
> + gIoMmuProtocol,
> + PciIoDevice->Handle,
> + PciIoMapInfo->DeviceAddress,
> + PciIoMapInfo->AlignedNumberOfBytes,
> + IoMmuAttribute
> + );
> + //
> + // We need do copy mem after IoMmu->SetAttribute(),
> + // because it might change IOMMU state.
> + //
> + RemapStatus = gIoMmuProtocol->GetRemapAddress (
> + gIoMmuProtocol,
> + PciIoDevice->Handle,
> + PciIoMapInfo->DeviceAddress,
> + &PciIoMapInfo->MappedHostAddress
> + );
> + if (EFI_ERROR(RemapStatus)) {
> + PciIoMapInfo->MappedHostAddress = (VOID *)(UINTN)PciIoMapInfo-
> >DeviceAddress;
> + }
> + if (Operation == EfiPciIoOperationBusMasterRead) {
> + CopyMem (
> + PciIoMapInfo->MappedHostAddress,
> + PciIoMapInfo->HostAddress,
> + PciIoMapInfo->NumberOfBytes
> + );
> + }
> + InsertTailList (&PciIoDevice->Maps, &PciIoMapInfo->Link);
> + }
> + }
> return Status;
> }
>
> @@ -1021,9 +1149,48 @@ PciIoUnmap (
> {
> EFI_STATUS Status;
> PCI_IO_DEVICE *PciIoDevice;
> + PCI_IO_MAP_INFO *PciIoMapInfo;
> + LIST_ENTRY *Link;
>
> PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
>
> + PciIoMapInfo = NULL;
> + if (gIoMmuProtocol != NULL) {
> + PciIoMapInfo = NULL;
> + for (Link = GetFirstNode (&PciIoDevice->Maps)
> + ; !IsNull (&PciIoDevice->Maps, Link)
> + ; Link = GetNextNode (&PciIoDevice->Maps, Link)
> + ) {
> + PciIoMapInfo = PCI_IO_MAP_INFO_FROM_LINK (Link);
> + if (PciIoMapInfo == Mapping) {
> + break;
> + }
> + }
> + //
> + // Mapping is not a valid value returned by Map()
> + //
> + if (PciIoMapInfo != Mapping) {
> + DEBUG ((DEBUG_INFO, "PciIoUnmap - PciIoMapInfo not found!\n"));
> + return EFI_INVALID_PARAMETER;
> + }
> + RemoveEntryList (&PciIoMapInfo->Link);
> + Mapping = PciIoMapInfo->PciRootBridgeIoMapping;
> +
> + //
> + // PciHostBridge should map IOMMU page aligned HostAddress.
> + //
> + // We need do copy mem before PciRootBridgeIo->Unmap(),
> + // because it might free mapped host address.
> + //
> + if (PciIoMapInfo->Operation == EfiPciIoOperationBusMasterWrite) {
> + CopyMem (
> + PciIoMapInfo->HostAddress,
> + PciIoMapInfo->MappedHostAddress,
> + PciIoMapInfo->NumberOfBytes
> + );
> + }
> + }
> +
> Status = PciIoDevice->PciRootBridgeIo->Unmap (
> PciIoDevice->PciRootBridgeIo,
> Mapping @@ -1037,6 +1204,25 @@ PciIoUnmap (
> );
> }
>
> + if (gIoMmuProtocol != NULL) {
> + if (!EFI_ERROR(Status)) {
> + //
> + // PciHostBridge should map IOMMU page aligned HostAddress.
> + //
> + gIoMmuProtocol->SetAttribute (
> + gIoMmuProtocol,
> + PciIoDevice->Handle,
> + PciIoMapInfo->DeviceAddress,
> + PciIoMapInfo->AlignedNumberOfBytes,
> + 0
> + );
> + if (PciIoMapInfo->Operation !=
> EfiPciIoOperationBusMasterCommonBuffer) {
> + FreePages (PciIoMapInfo->AlignedHostAddress,
> EFI_SIZE_TO_PAGES(PciIoMapInfo->AlignedNumberOfBytes));
> + }
> + FreePool (PciIoMapInfo);
> + }
> + }
> +
> return Status;
> }
>
> @@ -1073,6 +1259,7 @@ PciIoAllocateBuffer ( {
> EFI_STATUS Status;
> PCI_IO_DEVICE *PciIoDevice;
> + UINTN Size;
>
> if ((Attributes &
> (~(EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE |
> EFI_PCI_ATTRIBUTE_MEMORY_CACHED))) != 0){ @@ -1085,6 +1272,12 @@
> PciIoAllocateBuffer (
> Attributes |= EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
> }
>
> + if (gIoMmuProtocol != NULL) {
> + Size = EFI_PAGES_TO_SIZE(Pages);
> + Size = ALIGN_VALUE(Size, mIoMmuPageSize);
> + Pages = EFI_SIZE_TO_PAGES (Size);
> + }
> +
> Status = PciIoDevice->PciRootBridgeIo->AllocateBuffer (
> PciIoDevice->PciRootBridgeIo,
> Type, @@ -1101,7 +1294,9 @@ PciIoAllocateBuffer (
> PciIoDevice->DevicePath
> );
> }
> -
> + //
> + // No need to set attribute here, it is done in Map.
> + //
> return Status;
> }
>
> @@ -1127,9 +1322,16 @@ PciIoFreeBuffer ( {
> EFI_STATUS Status;
> PCI_IO_DEVICE *PciIoDevice;
> + UINTN Size;
>
> PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
>
> + if (gIoMmuProtocol != NULL) {
> + Size = EFI_PAGES_TO_SIZE(Pages);
> + Size = ALIGN_VALUE(Size, mIoMmuPageSize);
> + Pages = EFI_SIZE_TO_PAGES (Size);
> + }
> +
> Status = PciIoDevice->PciRootBridgeIo->FreeBuffer (
> PciIoDevice->PciRootBridgeIo,
> Pages, @@ -1144,6 +1346,9 @@ PciIoFreeBuffer (
> );
> }
>
> + //
> + // No need to set attribute here, it is done in Unmap.
> + //
> return Status;
> }
>
> --
> 2.7.4.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: Add IOMMU support.
2017-04-17 19:58 ` Duran, Leo
@ 2017-04-18 3:45 ` Yao, Jiewen
0 siblings, 0 replies; 10+ messages in thread
From: Yao, Jiewen @ 2017-04-18 3:45 UTC (permalink / raw)
To: Duran, Leo, edk2-devel@lists.01.org
Cc: Ni, Ruiyu, Singh, Brijesh, Ard Biesheuvel
Yes, I agree. It is a little complicated.
NeedAllocateNonExisting is TRUE when:
A. IoMMU is present (AND)
B. We need remap a Buffer below 4GiB, but we cannot find enough memory there.
Because CommonBuffer and Read/WriteBuffer is handled in different way, we have to separate #B to be 2 cases.
B.1 We need remap a CommBuffer, because no below 4GiB CommBuffer returned in AllocateBuffer API. (OR)
B.2 We need remap a Read/Write buffer below 4GiB, but we cannot find enough memory there.
As a result: NeedAllocateNonExisting means (A AND (B.1 OR B.2))
Thank you
Yao Jiewen
From: Duran, Leo [mailto:leo.duran@amd.com]
Sent: Tuesday, April 18, 2017 3:58 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Singh, Brijesh <brijesh.singh@amd.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: RE: [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: Add IOMMU support.
Hi Yao,
Regarding RootBridgeIoMap()
I'm wondering if may be possible to simplify the logic requiring flags "NeedMap" and "NeedAllocateNonExisting"?
For example, it seems like (NeedAllocateNonExisting==TRUE) implies (gIoMmuProtocol != NULL), but that did not seem obvious at a glance.
Leo.
> -----Original Message-----
> From: Jiewen Yao [mailto:jiewen.yao@intel.com]
> Sent: Tuesday, April 04, 2017 2:06 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: [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: Add IOMMU support.
>
> The responsibility of PciBus driver is to set IOMMU attribute, because only
> PciBus knows which device submits DMA access request.
>
> PciBus driver guarantee that the request to PciHostBridge is IOMMU page
> aligned memory, as such PciHostBridge can allocate non-existent memory for
> device memory, to satisfy remap requirement.
>
> PciBus driver does not assume device address is same as the mapped host
> address, because IOMMU may remap it.
>
> 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>>
> ---
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c | 12 ++
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 19 ++
> MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf | 1 +
> MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 225
> +++++++++++++++++++-
> 4 files changed, 247 insertions(+), 10 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> index f3be47a..c9ee4de 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> @@ -42,6 +42,8 @@ UINT64 gAllZero = 0;
>
> EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
> EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
> +EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
> +UINTN mIoMmuPageSize = 1;
>
>
> GLOBAL_REMOVE_IF_UNREFERENCED
> EFI_PCI_HOTPLUG_REQUEST_PROTOCOL mPciHotPlugRequest = { @@ -
> 256,6 +258,16 @@ PciBusDriverBindingStart (
> }
>
> gBS->LocateProtocol (
> + &gEdkiiIoMmuProtocolGuid,
> + NULL,
> + (VOID **) &gIoMmuProtocol
> + );
> + if (gIoMmuProtocol != NULL) {
> + gIoMmuProtocol->GetPageSize (gIoMmuProtocol, &mIoMmuPageSize);
> + ASSERT ((mIoMmuPageSize & (mIoMmuPageSize - 1)) == 0); }
> +
> + gBS->LocateProtocol (
> &gEfiIncompatiblePciDeviceSupportProtocolGuid,
> NULL,
> (VOID **) &gIncompatiblePciDeviceSupport diff --git
> a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> index 39ba8b9..185d89c 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> @@ -289,6 +290,8 @@ struct
> _PCI_IO_DEVICE {
> // This field is used to support this case.
> //
> UINT16 BridgeIoAlignment;
> +
> + LIST_ENTRY Maps;
> };
>
> #define PCI_IO_DEVICE_FROM_PCI_IO_THIS(a) \ @@ -304,6 +307,20 @@
> struct _PCI_IO_DEVICE {
> CR (a, PCI_IO_DEVICE, LoadFile2, PCI_IO_DEVICE_SIGNATURE)
>
>
> +#define PCI_IO_MAP_INFO_SIGNATURE SIGNATURE_32 ('p', 'm', 'a', 'p')
> +typedef struct {
> + UINT32 Signature;
> + LIST_ENTRY Link;
> + EFI_PCI_IO_PROTOCOL_OPERATION Operation;
> + VOID *HostAddress;
> + EFI_PHYSICAL_ADDRESS DeviceAddress;
> + UINTN NumberOfBytes;
> + VOID *AlignedHostAddress;
> + UINTN AlignedNumberOfBytes;
> + VOID *MappedHostAddress;
> + VOID *PciRootBridgeIoMapping;
> +} PCI_IO_MAP_INFO;
> +#define PCI_IO_MAP_INFO_FROM_LINK(a) CR (a, PCI_IO_MAP_INFO,
> Link,
> +PCI_IO_MAP_INFO_SIGNATURE)
>
> //
> // Global Variables
> @@ -321,6 +338,8 @@ extern EFI_PCI_PLATFORM_PROTOCOL
> *gPciPlatformProtocol;
> extern EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
> extern BOOLEAN mReserveIsaAliases;
> extern BOOLEAN mReserveVgaAliases;
> +extern EDKII_IOMMU_PROTOCOL *gIoMmuProtocol;
> +extern UINTN mIoMmuPageSize;
>
> /**
> Macro that checks whether device is a GFX device.
> 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..31b8c32 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> @@ -58,6 +58,7 @@ InitializePciIoInstance (
> )
> {
> CopyMem (&PciIoDevice->PciIo, &mPciIoInterface, sizeof
> (EFI_PCI_IO_PROTOCOL));
> + InitializeListHead (&PciIoDevice->Maps);
> }
>
> /**
> @@ -936,6 +937,28 @@ PciIoCopyMem (
> }
>
> /**
> + Return if a value is aligned.
> +
> + @param Value the value to be checked
> + @param Alignment the alignment to be checked with.
> +
> + @retval TRUE The value is aligned
> + @retval FALSE The value is not aligned.
> +**/
> +BOOLEAN
> +InternalIsAlgined (
> + IN UINTN Value,
> + IN UINTN Alignment
> + )
> +{
> + if (Value == ALIGN_VALUE(Value, Alignment)) {
> + return TRUE;
> + } else {
> + return FALSE;
> + }
> +}
> +
> +/**
> Provides the PCI controller-specific addresses needed to access system
> memory.
>
> @param This A pointer to the EFI_PCI_IO_PROTOCOL instance.
> @@ -967,6 +990,9 @@ PciIoMap (
> {
> EFI_STATUS Status;
> PCI_IO_DEVICE *PciIoDevice;
> + PCI_IO_MAP_INFO *PciIoMapInfo;
> + UINT64 IoMmuAttribute;
> + EFI_STATUS RemapStatus;
>
> PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
>
> @@ -982,15 +1008,60 @@ PciIoMap (
> Operation = (EFI_PCI_IO_PROTOCOL_OPERATION) (Operation +
> EfiPciOperationBusMasterRead64);
> }
>
> - Status = PciIoDevice->PciRootBridgeIo->Map (
> - PciIoDevice->PciRootBridgeIo,
> - (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION)
> Operation,
> - HostAddress,
> - NumberOfBytes,
> - DeviceAddress,
> - Mapping
> - );
> -
> + if (gIoMmuProtocol != NULL) {
> + PciIoMapInfo = AllocatePool (sizeof(*PciIoMapInfo));
> + if (PciIoMapInfo == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> + PciIoMapInfo->Signature = PCI_IO_MAP_INFO_SIGNATURE;
> + PciIoMapInfo->Operation = Operation;
> + PciIoMapInfo->NumberOfBytes = *NumberOfBytes;
> + PciIoMapInfo->DeviceAddress = *DeviceAddress;
> + PciIoMapInfo->HostAddress = HostAddress;
> + //
> + // For non common buffer, we always allocate a new memory if IOMMU
> exists.
> + // because the original memory might not be DMA capable.
> + //
> + // For common buffer, it is not needed, because common buffer allocate
> via PciIoAllocateBuffer.
> + // We cannot use AllocateAlignedPages here, because there might be
> more restriction in PciIoAllocateBuffer().
> + //
> + PciIoMapInfo->AlignedNumberOfBytes = ALIGN_VALUE (PciIoMapInfo-
> >NumberOfBytes, mIoMmuPageSize);
> + if (PciIoMapInfo->Operation !=
> EfiPciIoOperationBusMasterCommonBuffer) {
> + PciIoMapInfo->AlignedHostAddress = AllocateAlignedPages
> (EFI_SIZE_TO_PAGES(PciIoMapInfo->AlignedNumberOfBytes),
> mIoMmuPageSize);
> + if (PciIoMapInfo->AlignedHostAddress == NULL) {
> + FreePool (PciIoMapInfo);
> + return EFI_OUT_OF_RESOURCES;
> + }
> + } else {
> + //
> + // For common buffer, the HostAddress must be allocated via
> PciIoAllocateBuffer.
> + //
> + if (!InternalIsAlgined((UINTN)PciIoMapInfo->HostAddress,
> mIoMmuPageSize)) {
> + FreePool (PciIoMapInfo);
> + DEBUG ((DEBUG_ERROR, "PciIoMap - map unaligned common buffer
> with IOMMU\n"));
> + return EFI_UNSUPPORTED;
> + }
> + PciIoMapInfo->AlignedHostAddress = PciIoMapInfo->HostAddress;
> + }
> + PciIoMapInfo->PciRootBridgeIoMapping = NULL;
> + Status = PciIoDevice->PciRootBridgeIo->Map (
> + PciIoDevice->PciRootBridgeIo,
> +
> (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) Operation,
> + PciIoMapInfo->AlignedHostAddress,
> + &PciIoMapInfo->AlignedNumberOfBytes,
> + &PciIoMapInfo->DeviceAddress,
> + &PciIoMapInfo->PciRootBridgeIoMapping
> + ); } else {
> + Status = PciIoDevice->PciRootBridgeIo->Map (
> + PciIoDevice->PciRootBridgeIo,
> +
> (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) Operation,
> + HostAddress,
> + NumberOfBytes,
> + DeviceAddress,
> + Mapping
> + ); }
> if (EFI_ERROR (Status)) {
> REPORT_STATUS_CODE_WITH_DEVICE_PATH (
> EFI_ERROR_CODE | EFI_ERROR_MINOR, @@ -999,6 +1070,63 @@
> PciIoMap (
> );
> }
>
> + if (gIoMmuProtocol != NULL) {
> + if (EFI_ERROR(Status)) {
> + if (PciIoMapInfo->Operation !=
> EfiPciIoOperationBusMasterCommonBuffer) {
> + FreePages (PciIoMapInfo->AlignedHostAddress,
> EFI_SIZE_TO_PAGES(PciIoMapInfo->AlignedNumberOfBytes));
> + }
> + FreePool (PciIoMapInfo);
> + } else {
> + *DeviceAddress = PciIoMapInfo->DeviceAddress;
> + *Mapping = PciIoMapInfo;
> +
> + switch (Operation) {
> + case EfiPciIoOperationBusMasterRead:
> + IoMmuAttribute = EDKII_IOMMU_ATTRIBUTE_READ;
> + break;
> + case EfiPciIoOperationBusMasterWrite:
> + IoMmuAttribute = EDKII_IOMMU_ATTRIBUTE_WRITE;
> + break;
> + case EfiPciIoOperationBusMasterCommonBuffer:
> + IoMmuAttribute = EDKII_IOMMU_ATTRIBUTE_READ |
> EDKII_IOMMU_ATTRIBUTE_WRITE;
> + break;
> + default:
> + ASSERT(FALSE);
> + return EFI_INVALID_PARAMETER;
> + }
> + //
> + // PciHostBridge should map IOMMU page aligned HostAddress.
> + //
> + gIoMmuProtocol->SetAttribute (
> + gIoMmuProtocol,
> + PciIoDevice->Handle,
> + PciIoMapInfo->DeviceAddress,
> + PciIoMapInfo->AlignedNumberOfBytes,
> + IoMmuAttribute
> + );
> + //
> + // We need do copy mem after IoMmu->SetAttribute(),
> + // because it might change IOMMU state.
> + //
> + RemapStatus = gIoMmuProtocol->GetRemapAddress (
> + gIoMmuProtocol,
> + PciIoDevice->Handle,
> + PciIoMapInfo->DeviceAddress,
> + &PciIoMapInfo->MappedHostAddress
> + );
> + if (EFI_ERROR(RemapStatus)) {
> + PciIoMapInfo->MappedHostAddress = (VOID *)(UINTN)PciIoMapInfo-
> >DeviceAddress;
> + }
> + if (Operation == EfiPciIoOperationBusMasterRead) {
> + CopyMem (
> + PciIoMapInfo->MappedHostAddress,
> + PciIoMapInfo->HostAddress,
> + PciIoMapInfo->NumberOfBytes
> + );
> + }
> + InsertTailList (&PciIoDevice->Maps, &PciIoMapInfo->Link);
> + }
> + }
> return Status;
> }
>
> @@ -1021,9 +1149,48 @@ PciIoUnmap (
> {
> EFI_STATUS Status;
> PCI_IO_DEVICE *PciIoDevice;
> + PCI_IO_MAP_INFO *PciIoMapInfo;
> + LIST_ENTRY *Link;
>
> PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
>
> + PciIoMapInfo = NULL;
> + if (gIoMmuProtocol != NULL) {
> + PciIoMapInfo = NULL;
> + for (Link = GetFirstNode (&PciIoDevice->Maps)
> + ; !IsNull (&PciIoDevice->Maps, Link)
> + ; Link = GetNextNode (&PciIoDevice->Maps, Link)
> + ) {
> + PciIoMapInfo = PCI_IO_MAP_INFO_FROM_LINK (Link);
> + if (PciIoMapInfo == Mapping) {
> + break;
> + }
> + }
> + //
> + // Mapping is not a valid value returned by Map()
> + //
> + if (PciIoMapInfo != Mapping) {
> + DEBUG ((DEBUG_INFO, "PciIoUnmap - PciIoMapInfo not found!\n"));
> + return EFI_INVALID_PARAMETER;
> + }
> + RemoveEntryList (&PciIoMapInfo->Link);
> + Mapping = PciIoMapInfo->PciRootBridgeIoMapping;
> +
> + //
> + // PciHostBridge should map IOMMU page aligned HostAddress.
> + //
> + // We need do copy mem before PciRootBridgeIo->Unmap(),
> + // because it might free mapped host address.
> + //
> + if (PciIoMapInfo->Operation == EfiPciIoOperationBusMasterWrite) {
> + CopyMem (
> + PciIoMapInfo->HostAddress,
> + PciIoMapInfo->MappedHostAddress,
> + PciIoMapInfo->NumberOfBytes
> + );
> + }
> + }
> +
> Status = PciIoDevice->PciRootBridgeIo->Unmap (
> PciIoDevice->PciRootBridgeIo,
> Mapping @@ -1037,6 +1204,25 @@ PciIoUnmap (
> );
> }
>
> + if (gIoMmuProtocol != NULL) {
> + if (!EFI_ERROR(Status)) {
> + //
> + // PciHostBridge should map IOMMU page aligned HostAddress.
> + //
> + gIoMmuProtocol->SetAttribute (
> + gIoMmuProtocol,
> + PciIoDevice->Handle,
> + PciIoMapInfo->DeviceAddress,
> + PciIoMapInfo->AlignedNumberOfBytes,
> + 0
> + );
> + if (PciIoMapInfo->Operation !=
> EfiPciIoOperationBusMasterCommonBuffer) {
> + FreePages (PciIoMapInfo->AlignedHostAddress,
> EFI_SIZE_TO_PAGES(PciIoMapInfo->AlignedNumberOfBytes));
> + }
> + FreePool (PciIoMapInfo);
> + }
> + }
> +
> return Status;
> }
>
> @@ -1073,6 +1259,7 @@ PciIoAllocateBuffer ( {
> EFI_STATUS Status;
> PCI_IO_DEVICE *PciIoDevice;
> + UINTN Size;
>
> if ((Attributes &
> (~(EFI_PCI_ATTRIBUTE_MEMORY_WRITE_COMBINE |
> EFI_PCI_ATTRIBUTE_MEMORY_CACHED))) != 0){ @@ -1085,6 +1272,12 @@
> PciIoAllocateBuffer (
> Attributes |= EFI_PCI_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
> }
>
> + if (gIoMmuProtocol != NULL) {
> + Size = EFI_PAGES_TO_SIZE(Pages);
> + Size = ALIGN_VALUE(Size, mIoMmuPageSize);
> + Pages = EFI_SIZE_TO_PAGES (Size);
> + }
> +
> Status = PciIoDevice->PciRootBridgeIo->AllocateBuffer (
> PciIoDevice->PciRootBridgeIo,
> Type, @@ -1101,7 +1294,9 @@ PciIoAllocateBuffer (
> PciIoDevice->DevicePath
> );
> }
> -
> + //
> + // No need to set attribute here, it is done in Map.
> + //
> return Status;
> }
>
> @@ -1127,9 +1322,16 @@ PciIoFreeBuffer ( {
> EFI_STATUS Status;
> PCI_IO_DEVICE *PciIoDevice;
> + UINTN Size;
>
> PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
>
> + if (gIoMmuProtocol != NULL) {
> + Size = EFI_PAGES_TO_SIZE(Pages);
> + Size = ALIGN_VALUE(Size, mIoMmuPageSize);
> + Pages = EFI_SIZE_TO_PAGES (Size);
> + }
> +
> Status = PciIoDevice->PciRootBridgeIo->FreeBuffer (
> PciIoDevice->PciRootBridgeIo,
> Pages, @@ -1144,6 +1346,9 @@ PciIoFreeBuffer (
> );
> }
>
> + //
> + // No need to set attribute here, it is done in Unmap.
> + //
> return Status;
> }
>
> --
> 2.7.4.windows.1
^ permalink raw reply [flat|nested] 10+ messages in thread