subject:"\[edk2\] \[PATCH 3\/3\] MdeModulePkg\/PciBus\: Add IOMMU support."

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

2017-05-02 Thread Yao, Jiewen

Thank you.

My mistake. It seems my test platform does not have such capability.

It is fixed in V5.

Thank you
Yao Jiewen

From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Tuesday, May 2, 2017 6:04 PM
To: Yao, Jiewen 
Cc: edk2-devel@lists.01.org; Ni, Ruiyu ; Leo Duran 
; Brijesh Singh 
Subject: Re: [PATCH 3/3] MdeModulePkg/PciBus: Add IOMMU support.

On 29 April 2017 at 14:48, Jiewen Yao 
> wrote:
> If IOMMU protocol is installed, PciBus need call IOMMU
> to set access attribute for the PCI device in Map/Ummap.
>
> Only after the access attribute is set, the PCI device can
> access the DMA memory.
>
> Cc: Ruiyu Ni >
> Cc: Leo Duran >
> Cc: Brijesh Singh >
> Cc: Ard Biesheuvel 
> >
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao >
> ---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c  |  9 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h  |  1 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf |  1 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c   | 37 
>  4 files changed, 48 insertions(+)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c 
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> index f3be47a..950cacc 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> @@ -42,6 +42,7 @@ UINT64gAllZero  
>= 0;
>
>  EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
>  EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
> +EDKII_IOMMU_PROTOCOL  *mIoMmuProtocol;
>
>
>  GLOBAL_REMOVE_IF_UNREFERENCED EFI_PCI_HOTPLUG_REQUEST_PROTOCOL 
> mPciHotPlugRequest = {
> @@ -284,6 +285,14 @@ PciBusDriverBindingStart (
>);
>}
>
> +  if (mIoMmuProtocol == NULL) {
> +gBS->LocateProtocol (
> +  ,
> +  NULL,
> +  (VOID **) 
> +  );
> +  }
> +
>if (PcdGetBool (PcdPciDisableBusEnumeration)) {
>  gFullEnumeration = FALSE;
>} else {
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h 
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> index 39ba8b9..3bcc134 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> @@ -32,6 +32,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> 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..c0251c0 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> @@ -14,6 +14,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>
>  #include "PciBus.h"
>
> +extern EDKII_IOMMU_PROTOCOL  *mIoMmuProtocol;
> +
>  //
>  // Pci Io Protocol Interface
>  //
> @@ -967,6 +969,7 @@ PciIoMap (
>  {
>EFI_STATUSStatus;
>PCI_IO_DEVICE *PciIoDevice;
> +  UINT64IoMmuAttribute;
>
>PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
>
> @@ -999,6 +1002,31 @@ PciIoMap (
>);
>}
>
> +  if (mIoMmuProtocol != NULL) {
> +if (!EFI_ERROR (Status)) {
> +  switch (Operation) {
> +  case EfiPciIoOperationBusMasterRead:
> +IoMmuAttribute = EDKII_IOMMU_ACCESS_READ;
> +break;
> +  case EfiPciIoOperationBusMasterWrite:
> +IoMmuAttribute = EDKII_IOMMU_ACCESS_WRITE;
> +break;
> +  case EfiPciIoOperationBusMasterCommonBuffer:
> +IoMmuAttribute = EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE;
> +break;
> +  default:
> +ASSERT(FALSE);
> +return EFI_INVALID_PARAMETER;

I am hitting this assert(). The reason is that Operation is modified
if the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute is set, and
so it does not have any of these values anymore.

> +  }
> +

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

2017-05-02 Thread Ard Biesheuvel

On 29 April 2017 at 14:48, Jiewen Yao  wrote:
> If IOMMU protocol is installed, PciBus need call IOMMU
> to set access attribute for the PCI device in Map/Ummap.
>
> Only after the access attribute is set, the PCI device can
> access the DMA memory.
>
> Cc: Ruiyu Ni 
> Cc: Leo Duran 
> Cc: Brijesh Singh 
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao 
> ---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c  |  9 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h  |  1 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf |  1 +
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c   | 37 
>  4 files changed, 48 insertions(+)
>
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c 
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> index f3be47a..950cacc 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
> @@ -42,6 +42,7 @@ UINT64gAllZero  
>= 0;
>
>  EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
>  EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
> +EDKII_IOMMU_PROTOCOL  *mIoMmuProtocol;
>
>
>  GLOBAL_REMOVE_IF_UNREFERENCED EFI_PCI_HOTPLUG_REQUEST_PROTOCOL 
> mPciHotPlugRequest = {
> @@ -284,6 +285,14 @@ PciBusDriverBindingStart (
>);
>}
>
> +  if (mIoMmuProtocol == NULL) {
> +gBS->LocateProtocol (
> +  ,
> +  NULL,
> +  (VOID **) 
> +  );
> +  }
> +
>if (PcdGetBool (PcdPciDisableBusEnumeration)) {
>  gFullEnumeration = FALSE;
>} else {
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h 
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> index 39ba8b9..3bcc134 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
> @@ -32,6 +32,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> 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..c0251c0 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> @@ -14,6 +14,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>
>  #include "PciBus.h"
>
> +extern EDKII_IOMMU_PROTOCOL  *mIoMmuProtocol;
> +
>  //
>  // Pci Io Protocol Interface
>  //
> @@ -967,6 +969,7 @@ PciIoMap (
>  {
>EFI_STATUSStatus;
>PCI_IO_DEVICE *PciIoDevice;
> +  UINT64IoMmuAttribute;
>
>PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
>
> @@ -999,6 +1002,31 @@ PciIoMap (
>);
>}
>
> +  if (mIoMmuProtocol != NULL) {
> +if (!EFI_ERROR (Status)) {
> +  switch (Operation) {
> +  case EfiPciIoOperationBusMasterRead:
> +IoMmuAttribute = EDKII_IOMMU_ACCESS_READ;
> +break;
> +  case EfiPciIoOperationBusMasterWrite:
> +IoMmuAttribute = EDKII_IOMMU_ACCESS_WRITE;
> +break;
> +  case EfiPciIoOperationBusMasterCommonBuffer:
> +IoMmuAttribute = EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE;
> +break;
> +  default:
> +ASSERT(FALSE);
> +return EFI_INVALID_PARAMETER;

I am hitting this assert(). The reason is that Operation is modified
if the EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute is set, and
so it does not have any of these values anymore.

> +  }
> +  mIoMmuProtocol->SetMappingAttribute (
> +mIoMmuProtocol,
> +PciIoDevice->Handle,
> +*Mapping,
> +IoMmuAttribute
> +);
> +}
> +  }
> +
>return Status;
>  }
>
> @@ -1024,6 +1052,15 @@ PciIoUnmap (
>
>PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
>
> +  if (mIoMmuProtocol != NULL) {
> +mIoMmuProtocol->SetMappingAttribute (
> +  mIoMmuProtocol,
> +  PciIoDevice->Handle,
> +  Mapping,
> +  0
> +

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

2017-04-29 Thread Jiewen Yao

If IOMMU protocol is installed, PciBus need call IOMMU
to set access attribute for the PCI device in Map/Ummap.

Only after the access attribute is set, the PCI device can
access the DMA memory.

Cc: Ruiyu Ni 
Cc: Leo Duran 
Cc: Brijesh Singh 
Cc: Ard Biesheuvel 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c  |  9 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h  |  1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf |  1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c   | 37 
 4 files changed, 48 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
index f3be47a..950cacc 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
@@ -42,6 +42,7 @@ UINT64gAllZero
 = 0;
 
 EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
 EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
+EDKII_IOMMU_PROTOCOL  *mIoMmuProtocol;
 
 
 GLOBAL_REMOVE_IF_UNREFERENCED EFI_PCI_HOTPLUG_REQUEST_PROTOCOL 
mPciHotPlugRequest = {
@@ -284,6 +285,14 @@ PciBusDriverBindingStart (
   );
   }  
 
+  if (mIoMmuProtocol == NULL) {
+gBS->LocateProtocol (
+  ,
+  NULL,
+  (VOID **) 
+  );
+  }
+
   if (PcdGetBool (PcdPciDisableBusEnumeration)) {
 gFullEnumeration = FALSE;
   } else {
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 39ba8b9..3bcc134 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -32,6 +32,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
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..c0251c0 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -14,6 +14,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include "PciBus.h"
 
+extern EDKII_IOMMU_PROTOCOL  *mIoMmuProtocol;
+
 //
 // Pci Io Protocol Interface
 //
@@ -967,6 +969,7 @@ PciIoMap (
 {
   EFI_STATUSStatus;
   PCI_IO_DEVICE *PciIoDevice;
+  UINT64IoMmuAttribute;
 
   PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
 
@@ -999,6 +1002,31 @@ PciIoMap (
   );
   }
 
+  if (mIoMmuProtocol != NULL) {
+if (!EFI_ERROR (Status)) {
+  switch (Operation) {
+  case EfiPciIoOperationBusMasterRead:
+IoMmuAttribute = EDKII_IOMMU_ACCESS_READ;
+break;
+  case EfiPciIoOperationBusMasterWrite:
+IoMmuAttribute = EDKII_IOMMU_ACCESS_WRITE;
+break;
+  case EfiPciIoOperationBusMasterCommonBuffer:
+IoMmuAttribute = EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE;
+break;
+  default:
+ASSERT(FALSE);
+return EFI_INVALID_PARAMETER;
+  }
+  mIoMmuProtocol->SetMappingAttribute (
+mIoMmuProtocol,
+PciIoDevice->Handle,
+*Mapping,
+IoMmuAttribute
+);
+}
+  }
+
   return Status;
 }
 
@@ -1024,6 +1052,15 @@ PciIoUnmap (
 
   PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
 
+  if (mIoMmuProtocol != NULL) {
+mIoMmuProtocol->SetMappingAttribute (
+  mIoMmuProtocol,
+  PciIoDevice->Handle,
+  Mapping,
+  0
+  );
+  }
+
   Status = PciIoDevice->PciRootBridgeIo->Unmap (
   PciIoDevice->PciRootBridgeIo,
   Mapping
-- 
2.7.4.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

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

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

3 matches

Site Navigation

Mail list logo

Footer information