public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Jiewen Yao <jiewen.yao@intel.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	Ruiyu Ni <ruiyu.ni@intel.com>,  Leo Duran <leo.duran@amd.com>,
	Brijesh Singh <brijesh.singh@amd.com>
Subject: Re: [PATCH 3/3] MdeModulePkg/PciBus: Add IOMMU support.
Date: Tue, 2 May 2017 11:04:08 +0100	[thread overview]
Message-ID: <CAKv+Gu9k+u3ppcDS5WY6-f8v1LY9d27jrhDF139ZXwcW_+KiuA@mail.gmail.com> (raw)
In-Reply-To: <1493473694-5064-4-git-send-email-jiewen.yao@intel.com>

On 29 April 2017 at 14:48, Jiewen Yao <jiewen.yao@intel.com> 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 <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      |  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 @@ UINT64                                        gAllZero             = 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 (
> +          &gEdkiiIoMmuProtocolGuid,
> +          NULL,
> +          (VOID **) &mIoMmuProtocol
> +          );
> +  }
> +
>    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 <Protocol/IncompatiblePciDeviceSupport.h>
>  #include <Protocol/PciOverride.h>
>  #include <Protocol/PciEnumerationComplete.h>
> +#include <Protocol/IoMmu.h>
>
>  #include <Library/DebugLib.h>
>  #include <Library/UefiDriverEntryPoint.h>
> 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_STATUS    Status;
>    PCI_IO_DEVICE *PciIoDevice;
> +  UINT64        IoMmuAttribute;
>
>    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
> +                      );
> +  }
> +
>    Status = PciIoDevice->PciRootBridgeIo->Unmap (
>                                            PciIoDevice->PciRootBridgeIo,
>                                            Mapping
> --
> 2.7.4.windows.1
>


  reply	other threads:[~2017-05-02 10:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-29 13:48 [RFC] [PATCH V4 0/3] Add IOMMU support Jiewen Yao
2017-04-29 13:48 ` [PATCH 1/3] MdeModulePkg/Include: Add IOMMU protocol definition Jiewen Yao
2017-05-02  9:56   ` Ard Biesheuvel
2017-05-02 12:54     ` Yao, Jiewen
2017-04-29 13:48 ` [PATCH 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support Jiewen Yao
2017-04-29 13:48 ` [PATCH 3/3] MdeModulePkg/PciBus: " Jiewen Yao
2017-05-02 10:04   ` Ard Biesheuvel [this message]
2017-05-02 12:43     ` Yao, Jiewen
2017-05-02 10:14 ` [RFC] [PATCH V4 0/3] " Ard Biesheuvel
2017-05-02 12:59   ` Yao, Jiewen
2017-05-04  7:02 ` Ni, Ruiyu
2017-05-04 13:26   ` Yao, Jiewen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKv+Gu9k+u3ppcDS5WY6-f8v1LY9d27jrhDF139ZXwcW_+KiuA@mail.gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox