[edk2] [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: Add IOMMU support.

Jiewen Yao posted 3 patches 7 years, 7 months ago
Only 2 patches received!
There is a newer version of this series
[edk2] [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: Add IOMMU support.
Posted by Jiewen Yao 7 years, 7 months ago
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: Add IOMMU support.
Posted by Duran, Leo 7 years, 6 months ago
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [RFC] [PATCH V3 3/3] MdeModulePkg/PciBus: Add IOMMU support.
Posted by Yao, Jiewen 7 years, 6 months ago
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel