From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <jianfeng.tan@intel.com>
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
by dpdk.org (Postfix) with ESMTP id 755CF7CDF
for <dev@dpdk.org>; Mon, 5 Jun 2017 04:29:44 +0200 (CEST)
Received: from fmsmga004.fm.intel.com ([10.253.24.48])
by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
04 Jun 2017 19:29:43 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.39,298,1493708400";
d="scan'208,217";a="270167986"
Received: from shwdeisgchi083.ccr.corp.intel.com (HELO [10.239.67.144])
([10.239.67.144])
by fmsmga004.fm.intel.com with ESMTP; 04 Jun 2017 19:29:41 -0700
To: Gregory Etelson <gregory@weka.io>, Ferruh Yigit <ferruh.yigit@intel.com>
Cc: Shijith Thotton <shijith.thotton@caviumnetworks.com>,
Stephen Hemminger <stephen@networkplumber.org>, dev@dpdk.org,
Qi Zhang <qi.z.zhang@intel.com>, Wenzhuo Lu <wenzhuo.lu@intel.com>,
Thomas Monjalon <thomas@monjalon.net>
References: <1748341.rbpcFmWp0q@polaris>
<1496228966-18573-1-git-send-email-shijith.thotton@caviumnetworks.com>
<f55be7d7-24af-4ea8-2808-c2e50731dce1@intel.com> <2232995.tDUxtNZcgW@polaris>
From: "Tan, Jianfeng" <jianfeng.tan@intel.com>
Message-ID: <4e3b8469-b9be-1963-1ca8-7dd000fe7415@intel.com>
Date: Mon, 5 Jun 2017 10:29:40 +0800
User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101
Thunderbird/52.1.1
MIME-Version: 1.0
In-Reply-To: <2232995.tDUxtNZcgW@polaris>
Content-Language: en-US
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
X-Content-Filtered-By: Mailman/MimeDel 2.1.15
Subject: Re: [dpdk-dev] [RFC PATCH] igb_uio: issue FLR during open and
release of device file
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 05 Jun 2017 02:29:45 -0000
Hi Gregory,
On 6/4/2017 3:22 PM, Gregory Etelson wrote:
>
> In my environment I could reproduce server crash
>
> after applying the patch
>
Different from your patch, my patch calls
pci_enable_device()/pci_disable_device() instead of
pci_reset_function()/pci_try_reset_function() separately in open() and
release(). Could you check if that's the reason?
vfio_pci actually calls both pci_try_reset_function() and
pci_disable_device() in release().
Thanks,
Jianfeng
> Regards,
>
> Gregory
>
> On Wednesday, 31 May 2017 15:20:08 IDT Ferruh Yigit wrote:
>
> > On 5/31/2017 12:09 PM, Shijith Thotton wrote:
>
> > > Set UIO info device file operations open and release. Call pci reset
>
> > > function inside open and release to clear device state at start and
>
> > > end. Copied this behaviour from vfio_pci kernel module code. With this
>
> > > change, it is not mandatory to issue FLR by PMD's during init and
> close.
>
> >
>
> > Cc: Jianfeng Tan <jianfeng.tan@intel.com>
>
> >
>
> > Jianfeng also implemented following patch:
>
> > http://dpdk.org/dev/patchwork/patch/17495/
>
> >
>
> > Which also implements release and open ops, for slightly different
>
> > reason (prevent DMA access after app exit), but mainly both are to
>
> > gracefully handle application exit status.
>
> >
>
> > btw, for Jianfeng's case, can adding pci_clear_master() in release and
>
> > moving pci_set_master() to open help preventing unwanted DMA?
>
> >
>
> >
>
> > Gregory,
>
> >
>
> > Can you please check if this patch fixes your issue?
>
> >
>
> > Thanks,
>
> > ferruh
>
> >
>
> > >
>
> > > Signed-off-by: Shijith Thotton <shijith.thotton@caviumnetworks.com>
>
> > > ---
>
> > > lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 30
> ++++++++++++++++++++++++++++++
>
> > > 1 file changed, 30 insertions(+)
>
> > >
>
> > > diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>
> > > index b9d427c..5bc58d2 100644
>
> > > --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>
> > > +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
>
> > > @@ -170,6 +170,34 @@ struct rte_uio_pci_dev {
>
> > > return IRQ_HANDLED;
>
> > > }
>
> > >
>
> > > +/**
>
> > > + * This gets called while opening uio device file. It clears any
> previous state
>
> > > + * associated with the pci device.
>
> > > + */
>
> > > +static int
>
> > > +igbuio_pci_open(struct uio_info *info, struct inode *inode)
>
> > > +{
>
> > > + struct rte_uio_pci_dev *udev = info->priv;
>
> > > + struct pci_dev *dev = udev->pdev;
>
> > > +
>
> > > + /* reset the pci device */
>
> > > + pci_reset_function(dev);
>
> > > +
>
> > > + return 0;
>
> > > +}
>
> > > +
>
> > > +static int
>
> > > +igbuio_pci_release(struct uio_info *info, struct inode *inode)
>
> > > +{
>
> > > + struct rte_uio_pci_dev *udev = info->priv;
>
> > > + struct pci_dev *dev = udev->pdev;
>
> > > +
>
> > > + /* try to reset the pci device */
>
> > > + pci_try_reset_function(dev);
>
> > > +
>
> > > + return 0;
>
> > > +}
>
> > > +
>
> > > #ifdef CONFIG_XEN_DOM0
>
> > > static int
>
> > > igbuio_dom0_mmap_phys(struct uio_info *info, struct vm_area_struct
> *vma)
>
> > > @@ -372,6 +400,8 @@ struct rte_uio_pci_dev {
>
> > > udev->info.version = "0.1";
>
> > > udev->info.handler = igbuio_pci_irqhandler;
>
> > > udev->info.irqcontrol = igbuio_pci_irqcontrol;
>
> > > + udev->info.open = igbuio_pci_open;
>
> > > + udev->info.release = igbuio_pci_release;
>
> > > #ifdef CONFIG_XEN_DOM0
>
> > > /* check if the driver run on Xen Dom0 */
>
> > > if (xen_initial_domain())
>
> > >
>
> >
>
> >
>