DPDK patches and discussions
 help / color / mirror / Atom feed
From: gowrishankar muthukrishnan <gowrishankar.m@linux.vnet.ibm.com>
To: Alejandro Lucero <alejandro.lucero@netronome.com>
Cc: dev <dev@dpdk.org>, Anatoly Burakov <anatoly.burakov@intel.com>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	Alexey Kardashevskiy <aik@ozlabs.ru>,
	Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [PATCH] vfio: set IOMMU type for the container once
Date: Wed, 26 Apr 2017 16:06:16 +0530	[thread overview]
Message-ID: <0ceec7d0-0ac1-305c-5e20-da0a0ac7d727@linux.vnet.ibm.com> (raw)
In-Reply-To: <CAD+H990YT867MoOTDAuS1Nq6TrkfXETJed+OH34DQoMOPwn2ZQ@mail.gmail.com>

Thanks Alajandro. Could this patch be merged before you make further 
cases considered ?.
Reason is that, we need this fix in 17.05 for ppc64le as I pointed 
earlier (and of course any
changes that would make the fix better, if possible before RCs closed).

Thanks,
Gowrishankar

On Friday 21 April 2017 02:53 PM, Alejandro Lucero wrote:
> The patch is OK for me. I have run tests with that change with no 
> problems. But I can just run tests with devices where each one got its 
> own IOMMU group, so my ack is just for that case (although 
> theoretically this patch fixes the other case). I'm looking at doing 
> some changes to the kernel for being able to have an scenario with 
> several devices sharing same IOMMU group just for the shake of testing.
>
> By other hand, vfio hotplug has a problem when there are several 
> devices in same IOMMU group being plugged and unplugged. I have a 
> patch ready for fixing this issue which I will send soon.
>
> On Fri, Apr 21, 2017 at 11:12 AM, gowrishankar muthukrishnan 
> <gowrishankar.m@linux.vnet.ibm.com 
> <mailto:gowrishankar.m@linux.vnet.ibm.com>> wrote:
>
>     Could this patch be reviewed and merged for 17.05 rc3 ?. It solved
>     regression with i40e pmd
>     bring up in one of our ppc64le server models through vfio-pci pci
>     module.
>
>     Thanks Alexey pointing out this in one of your patches.
>
>     Regards,
>     Gowrishankar
>
>
>     On Tuesday 04 April 2017 09:36 PM, Andrew Rybchenko wrote:
>
>         If more than one used PCI device belongs to one IOMMU group,
>         it is still one IOMMU group and the container IOMMU type
>         should be set only once.
>
>         Fixes: 94c0776b1bad ("vfio: support hotplug")
>
>         Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com
>         <mailto:arybchenko@solarflare.com>>
>         ---
>         My testing of the patch is limitted to my configuration with 2 PCI
>         functions which belong to one IOMMU group.
>
>           lib/librte_eal/linuxapp/eal/eal_vfio.c | 54
>         ++++++++++++++++++----------------
>           1 file changed, 29 insertions(+), 25 deletions(-)
>
>         diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c
>         b/lib/librte_eal/linuxapp/eal/eal_vfio.c
>         index 6e2e84c..dd59c1c 100644
>         --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
>         +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
>         @@ -298,33 +298,37 @@ vfio_setup_device(const char
>         *sysfs_base, const char *dev_addr,
>                                 clear_group(vfio_group_fd);
>                                 return -1;
>                         }
>         -       }
>
>         -       /*
>         -        * pick an IOMMU type and set up DMA mappings for
>         container
>         -        *
>         -        * needs to be done only once, only when first group
>         is assigned to
>         -        * a container and only in primary process. Note this
>         can happen several
>         -        * times with the hotplug functionality.
>         -        */
>         -       if (internal_config.process_type == RTE_PROC_PRIMARY &&
>         -                       vfio_cfg.vfio_active_groups == 1) {
>         -               /* select an IOMMU type which we will be using */
>         -               const struct vfio_iommu_type *t =
>         +               /*
>         +                * pick an IOMMU type and set up DMA mappings
>         for container
>         +                *
>         +                * needs to be done only once, only when first
>         group is
>         +                * assigned to a container and only in primary
>         process.
>         +                * Note this can happen several times with the
>         hotplug
>         +                * functionality.
>         +                */
>         +               if (internal_config.process_type ==
>         RTE_PROC_PRIMARY &&
>         +  vfio_cfg.vfio_active_groups == 1) {
>         +                       /* select an IOMMU type which we will
>         be using */
>         +                       const struct vfio_iommu_type *t =
>         vfio_set_iommu_type(vfio_cfg.vfio_container_fd);
>         -               if (!t) {
>         -                       RTE_LOG(ERR, EAL, "  %s failed to
>         select IOMMU type\n", dev_addr);
>         -                       close(vfio_group_fd);
>         -                       clear_group(vfio_group_fd);
>         -                       return -1;
>         -               }
>         -               ret = t->dma_map_func(vfio_cfg.vfio_container_fd);
>         -               if (ret) {
>         -                       RTE_LOG(ERR, EAL, "  %s DMA remapping
>         failed, "
>         -                                       "error %i (%s)\n",
>         dev_addr, errno, strerror(errno));
>         -                       close(vfio_group_fd);
>         -                       clear_group(vfio_group_fd);
>         -                       return -1;
>         +                       if (!t) {
>         +                               RTE_LOG(ERR, EAL,
>         +                                       "  %s failed to select
>         IOMMU type\n",
>         +                                       dev_addr);
>         +                               close(vfio_group_fd);
>         +  clear_group(vfio_group_fd);
>         +                               return -1;
>         +                       }
>         +                       ret =
>         t->dma_map_func(vfio_cfg.vfio_container_fd);
>         +                       if (ret) {
>         +                               RTE_LOG(ERR, EAL,
>         +                                       "  %s DMA remapping
>         failed, error %i (%s)\n",
>         +                                       dev_addr, errno,
>         strerror(errno));
>         +                               close(vfio_group_fd);
>         +  clear_group(vfio_group_fd);
>         +                               return -1;
>         +                       }
>                         }
>                 }
>
>
>

  reply	other threads:[~2017-04-26 10:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-04 16:06 Andrew Rybchenko
2017-04-21  9:12 ` gowrishankar muthukrishnan
2017-04-21  9:23   ` Alejandro Lucero
2017-04-26 10:36     ` gowrishankar muthukrishnan [this message]
2017-04-26 10:52       ` Alejandro Lucero
2017-04-28 13:24 ` Burakov, Anatoly
2017-04-30 17:25   ` 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=0ceec7d0-0ac1-305c-5e20-da0a0ac7d727@linux.vnet.ibm.com \
    --to=gowrishankar.m@linux.vnet.ibm.com \
    --cc=aik@ozlabs.ru \
    --cc=alejandro.lucero@netronome.com \
    --cc=anatoly.burakov@intel.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=thomas@monjalon.net \
    /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).