From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 755CF7CDF for ; 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 , Ferruh Yigit Cc: Shijith Thotton , Stephen Hemminger , dev@dpdk.org, Qi Zhang , Wenzhuo Lu , Thomas Monjalon References: <1748341.rbpcFmWp0q@polaris> <1496228966-18573-1-git-send-email-shijith.thotton@caviumnetworks.com> <2232995.tDUxtNZcgW@polaris> From: "Tan, Jianfeng" 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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 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 > > > > --- > > > > 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()) > > > > > > > > > > >