From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua0-f169.google.com (mail-ua0-f169.google.com [209.85.217.169]) by dpdk.org (Postfix) with ESMTP id 62C7247CD for ; Wed, 26 Apr 2017 10:45:06 +0200 (CEST) Received: by mail-ua0-f169.google.com with SMTP id h2so146428977uaa.1 for ; Wed, 26 Apr 2017 01:45:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=V24QVge+jNMUGrAsvg6maXBSVw6lYXDxnx7ofho1KAU=; b=WuOHgbO+K8BhCHTGHO7f1O2cqvUSdQpieI/vgSTwk4ww+Ntp/UjbzB8Ya/1FdwVpIk riC2nOkUUzWVwV4WjatOhTROu3+OzDYlaQ2d1M+k7tZbh8CxDGtP72TIzsT+gpTzVSQ5 PhAyuMx6+mc1hgmL/5ONilLZtdD6Vi/HXoo/zW2tFSkD5+DEVCxCDRa9F+Zjh1FEdiHH 9N29wW68jLBx5RNIPBBvIu6Vr7FXJKMTx2+vKpLMkimmeMS5Q4ztKoNoI82QaEeIOoNW QHMeoznF/ZxhlIRjfsYms5zb8LcqrBN1+Dp70A0rMPSrzcLYaa7t+aeCY7AzlwulnFgX 5Qkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=V24QVge+jNMUGrAsvg6maXBSVw6lYXDxnx7ofho1KAU=; b=RwBbdX0BM/4TpdEv/oaf/AWrR21Dkg/S5A9nv3pWyO20Hs4SrzOI2ZdcVWT8mI128d DXEVmJ/4OL5kLPOURWNHe6r6ROwSV5dxyL9RKfwjhDPJAvjwVTfTV2o2j2ldqo+hCMyT SkVCXbeklfWgoJhkqpl27YWqjbYjlMoIoPPilXVW3UlcECME0zTwYPPA5yptUSsEl02x KBKaQteAin6eSWhyShUAoP39Hl2haShrQ4RplHTKchIuZZDI2bRLVdP7SvUeCWZOC35q fLhaY2RJLmNjQfjdWoNAgIvOEs7eUG4hrb3vFWcAX2QPC9Ca7g01cdquBFuvoQ25tAX9 sB/w== X-Gm-Message-State: AN3rC/52Abna1M+dbi5+yeyrYI2Vt0gp/0vQhuvnaJdKcDs8UUifzJYi c0+tmFEl0IFIQDAUr6RA+vn7+mgCSVZT X-Received: by 10.176.74.11 with SMTP id q11mr8398862uae.121.1493196305667; Wed, 26 Apr 2017 01:45:05 -0700 (PDT) MIME-Version: 1.0 Received: by 10.103.14.69 with HTTP; Wed, 26 Apr 2017 01:45:05 -0700 (PDT) In-Reply-To: References: <20170420072402.38106-1-aik@ozlabs.ru> <20170420072402.38106-5-aik@ozlabs.ru> <5b5fe4e4-af39-4c27-fb3c-aff0c601b938@solarflare.com> <9808fd9e-17e5-b873-592a-2309579097dc@ozlabs.ru> From: Alejandro Lucero Date: Wed, 26 Apr 2017 09:45:05 +0100 Message-ID: To: "Burakov, Anatoly" Cc: Alexey Kardashevskiy , Andrew Rybchenko , "dev@dpdk.org" , "JPF@zurich.ibm.com" , Gowrishankar Muthukrishnan Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH dpdk 4/5] vfio: Do try setting IOMMU type if already set X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Apr 2017 08:45:06 -0000 On Wed, Apr 26, 2017 at 9:27 AM, Burakov, Anatoly wrote: > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Alexey > > Kardashevskiy > > Sent: Wednesday, April 26, 2017 8:51 AM > > To: Andrew Rybchenko ; dev@dpdk.org > > Cc: JPF@zurich.ibm.com; Gowrishankar Muthukrishnan > > > > Subject: Re: [dpdk-dev] [PATCH dpdk 4/5] vfio: Do try setting IOMMU type > if > > already set > > > > On 21/04/17 18:54, Andrew Rybchenko wrote: > > > On 04/20/2017 10:24 AM, Alexey Kardashevskiy wrote: > > >> The existing code correctly checks if a container is set to a group > > >> and does not try attaching a group to a container for a > second/third/... > > >> device from the same IOMMU group. > > >> > > >> However it still tries to set IOMMU type to a container for every > > >> device in a group which produces failure and closes container and > group > > fds. > > >> > > >> This moves vfio_set_iommu_type() and DMA mapping code under: > > >> if (!(group_status.flags & VFIO_GROUP_FLAGS_CONTAINER_SET)) > > >> > > >> Signed-off-by: Alexey Kardashevskiy > > >> --- > > >> lib/librte_eal/linuxapp/eal/eal_vfio.c | 50 > > >> +++++++++++++++++----------------- > > >> 1 file changed, 25 insertions(+), 25 deletions(-) > > >> > > >> diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c > > >> b/lib/librte_eal/linuxapp/eal/eal_vfio.c > > >> index 6e2e84ca7..46f951f4d 100644 > > >> --- a/lib/librte_eal/linuxapp/eal/eal_vfio.c > > >> +++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c > > >> @@ -298,33 +298,33 @@ 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; > > >> + } > > >> } > > >> } > > > > > > It looks like a duplicate of the earlier submitted patch > > > http://dpdk.org/ml/archives/dev/2017-April/063077.html > > > > > > It is a duplicate indeed, what needs to be done to have it accepted in > the > > mainline tree? > > I think Alejandro Lucero (CC'd) mentioned that he has another patch for > similar issue that he's working on (for ports being > assigned to the same IOMMU group). That patch would presumably fix this > issue as well, so I'm inclined to > wait for that one. If that's not happening, then we can accept either this > one or the earlier ones, and fix the > remaining issues (I have a setup where I can attempt to reproduce the > issue, so that should be quick enough). > > I was counting on first submitted patch being accepted. There is another thing to solve which is to unplug ports when there are more than one (already plugged) in an IOMMU group. I have a patch for this but I can not test it for that scenario, just for the "normal" one with one port/device per IOMMU group. I'm working on having a way for testing the first case. Once I have said that, I would like to comment code after the patch fixing the plug issue is fine. What I mean is, unplugging a device when more than one device already plugged from the same IOMMU group do not break things. Of course, I can only say that from a theoretical point of view after studying the kernel VFIO code. The only problem is after that, none else device hotplug can happen. I know that is not a good thing because it requires to restart the app, but better than breaking the app with a crash. I will submit later today the patch for fixing the unplug issue. Maybe someone can test that asap while I get some way for doing it. > Thanks, > Anatoly >