DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@redhat.com>
To: Takeshi T Yoshimura <TYOS@jp.ibm.com>
Cc: dev <dev@dpdk.org>,
	"Burakov, Anatoly" <anatoly.burakov@intel.com>,
	 David Christensen <drc@linux.vnet.ibm.com>,
	Jerin Jacob Kollanukkaran <jerinj@marvell.com>
Subject: Re: [dpdk-dev] [PATCH] bus/pci: always check IOMMU capabilities
Date: Mon, 5 Aug 2019 08:16:40 +0200	[thread overview]
Message-ID: <CAJFAV8y7eQcTVjRKHiFg-727+6aBTUuUEN5k7Pk20t4C7ZONgQ@mail.gmail.com> (raw)
In-Reply-To: <OF80536811.04116BA8-ON0025844D.0015C0D8-0025844D.0015C0DD@notes.na.collabserv.com>

On Mon, Aug 5, 2019 at 5:57 AM Takeshi T Yoshimura <TYOS@jp.ibm.com> wrote:
>
> -----David Marchand <david.marchand@redhat.com> wrote: -----
>
> >To: dev@dpdk.org
> >From: David Marchand <david.marchand@redhat.com>
> >Date: 08/02/2019 07:14PM
> >Cc: anatoly.burakov@intel.com, tyos@jp.ibm.com,
> >drc@linux.vnet.ibm.com
> >Subject: [PATCH] bus/pci: always check IOMMU capabilities
> >
> >IOMMU capabilities won't change and must be checked even if no PCI
> >device
> >seem to be supported yet when EAL initialised.
> >
> >This is to accommodate with SPDK that registers its drivers after
> >rte_eal_init(), especially on PPC platform where the IOMMU does not
> >support VA.
> >
> >Fixes: 703458e19c16 ("bus/pci: consider only usable devices for IOVA
> >mode")
> >
> >Signed-off-by: David Marchand <david.marchand@redhat.com>
> >---
> > drivers/bus/pci/bsd/pci.c    |  6 ++++++
> > drivers/bus/pci/linux/pci.c  | 25 ++++++-------------------
> > drivers/bus/pci/pci_common.c | 16 +++++++++++++++-
> > drivers/bus/pci/private.h    |  5 ++++-
> > 4 files changed, 31 insertions(+), 21 deletions(-)
> >
> >diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
> >index a2de709..8f07ed9 100644
> >--- a/drivers/bus/pci/bsd/pci.c
> >+++ b/drivers/bus/pci/bsd/pci.c
> >@@ -376,6 +376,12 @@ error:
> >       return -1;
> > }
> >
> >+bool
> >+pci_device_iommu_support_va(__rte_unused const struct rte_pci_device
> >*dev)
> >+{
> >+      return false;
> >+}
> >+
> > enum rte_iova_mode
> > pci_device_iova_mode(const struct rte_pci_driver *pdrv __rte_unused,
> >                    const struct rte_pci_device *pdev)
> >diff --git a/drivers/bus/pci/linux/pci.c
> >b/drivers/bus/pci/linux/pci.c
> >index f4fb742..43debaa 100644
> >--- a/drivers/bus/pci/linux/pci.c
> >+++ b/drivers/bus/pci/linux/pci.c
> >@@ -498,8 +498,8 @@ error:
> > }
> >
> > #if defined(RTE_ARCH_X86)
> >-static bool
> >-pci_one_device_iommu_support_va(const struct rte_pci_device *dev)
> >+bool
> >+pci_device_iommu_support_va(const struct rte_pci_device *dev)
> > {
> > #define VTD_CAP_MGAW_SHIFT    16
> > #define VTD_CAP_MGAW_MASK     (0x3fULL << VTD_CAP_MGAW_SHIFT)
> >@@ -546,14 +546,14 @@ pci_one_device_iommu_support_va(const struct
> >rte_pci_device *dev)
> >       return true;
> > }
> > #elif defined(RTE_ARCH_PPC_64)
> >-static bool
> >-pci_one_device_iommu_support_va(__rte_unused const struct
> >rte_pci_device *dev)
> >+bool
> >+pci_device_iommu_support_va(__rte_unused const struct rte_pci_device
> >*dev)
> > {
> >       return false;
> > }
> > #else
> >-static bool
> >-pci_one_device_iommu_support_va(__rte_unused const struct
> >rte_pci_device *dev)
> >+bool
> >+pci_device_iommu_support_va(__rte_unused const struct rte_pci_device
> >*dev)
> > {
> >       return true;
> > }
> >@@ -564,7 +564,6 @@ pci_device_iova_mode(const struct rte_pci_driver
> >*pdrv,
> >                    const struct rte_pci_device *pdev)
> > {
> >       enum rte_iova_mode iova_mode = RTE_IOVA_DC;
> >-      static int iommu_no_va = -1;
> >
> >       switch (pdev->kdrv) {
> >       case RTE_KDRV_VFIO: {
> >@@ -595,18 +594,6 @@ pci_device_iova_mode(const struct rte_pci_driver
> >*pdrv,
> >                       iova_mode = RTE_IOVA_VA;
> >               break;
> >       }
> >-
> >-      if (iova_mode != RTE_IOVA_PA) {
> >-              /*
> >-               * We can check this only once, because the IOMMU hardware is
> >-               * the same for all of them.
> >-               */
> >-              if (iommu_no_va == -1)
> >-                      iommu_no_va = pci_one_device_iommu_support_va(pdev)
> >-                                      ? 0 : 1;
> >-              if (iommu_no_va != 0)
> >-                      iova_mode = RTE_IOVA_PA;
> >-      }
> >       return iova_mode;
> > }
> >
> >diff --git a/drivers/bus/pci/pci_common.c
> >b/drivers/bus/pci/pci_common.c
> >index 9794552..8d1d6ab 100644
> >--- a/drivers/bus/pci/pci_common.c
> >+++ b/drivers/bus/pci/pci_common.c
> >@@ -616,8 +616,16 @@ rte_pci_get_iommu_class(void)
> >       const struct rte_pci_driver *drv;
> >       bool devices_want_va = false;
> >       bool devices_want_pa = false;
> >+      static int iommu_no_va = -1;
> >
> >       FOREACH_DEVICE_ON_PCIBUS(dev) {
> >+              /*
> >+               * We can check this only once, because the IOMMU hardware is
> >+               * the same for all of them.
> >+               */
> >+              if (iommu_no_va == -1)
> >+                      iommu_no_va = pci_device_iommu_support_va(dev)
> >+                                      ? 0 : 1;
> >               if (pci_ignore_device(dev))
> >                       continue;
> >               if (dev->kdrv == RTE_KDRV_UNKNOWN ||
> >@@ -643,7 +651,13 @@ rte_pci_get_iommu_class(void)
> >                               devices_want_va = true;
> >               }
> >       }
> >-      if (devices_want_va && !devices_want_pa) {
> >+      if (iommu_no_va == 1) {
> >+              iova_mode = RTE_IOVA_PA;
> >+              if (devices_want_va) {
> >+                      RTE_LOG(WARNING, EAL, "Some devices want 'VA' but because IOMMU
> >does not support 'VA'.\n");
> >+                      RTE_LOG(WARNING, EAL, "The devices that want 'VA' won't
> >initialize.\n");
> >+              }
> >+      } else if (devices_want_va && !devices_want_pa) {
> >               iova_mode = RTE_IOVA_VA;
> >       } else if (devices_want_pa && !devices_want_va) {
> >               iova_mode = RTE_IOVA_PA;
> >diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h
> >index 8a55240..a205d4d 100644
> >--- a/drivers/bus/pci/private.h
> >+++ b/drivers/bus/pci/private.h
> >@@ -173,9 +173,12 @@ rte_pci_match(const struct rte_pci_driver
> >*pci_drv,
> >             const struct rte_pci_device *pci_dev);
> >
> > /**
> >- * OS specific callback for rte_pci_get_iommu_class
> >+ * OS specific callbacks for rte_pci_get_iommu_class
> >  *
> >  */
> >+bool
> >+pci_device_iommu_support_va(const struct rte_pci_device *dev);
> >+
> > enum rte_iova_mode
> > pci_device_iova_mode(const struct rte_pci_driver *pci_drv,
> >                    const struct rte_pci_device *pci_dev);
> >--
> >1.8.3.1
> >
> >
>
> Tested-by: Takeshi Yoshimura <tyos@jp.ibm.com>

Thank you all.
I'll send a v2 with the comment from Jerin and the log message fix.


-- 
David Marchand

  reply	other threads:[~2019-08-05  6:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-02 10:14 David Marchand
2019-08-02 10:20 ` David Marchand
2019-08-02 17:13   ` David Christensen
2019-08-02 17:10 ` David Christensen
2019-08-03 18:22 ` Jerin Jacob Kollanukkaran
2019-08-05  3:57 ` Takeshi T Yoshimura
2019-08-05  6:16   ` David Marchand [this message]
2019-08-05  6:23 ` [dpdk-dev] [PATCH v2] " David Marchand
2019-08-05 10:09   ` Thomas Monjalon

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-all 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=CAJFAV8y7eQcTVjRKHiFg-727+6aBTUuUEN5k7Pk20t4C7ZONgQ@mail.gmail.com \
    --to=david.marchand@redhat.com \
    --cc=TYOS@jp.ibm.com \
    --cc=anatoly.burakov@intel.com \
    --cc=dev@dpdk.org \
    --cc=drc@linux.vnet.ibm.com \
    --cc=jerinj@marvell.com \
    /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;
as well as URLs for NNTP newsgroup(s).