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....@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.thot...@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())

> >

>

>


Reply via email to