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:

quoted

On 5/31/2017 12:09 PM, Shijith Thotton wrote:

quoted

quoted

Set UIO info device file operations open and release. Call pci reset

quoted

quoted

function inside open and release to clear device state at start and

quoted

quoted

end. Copied this behaviour from vfio_pci kernel module code. With this

quoted

quoted

change, it is not mandatory to issue FLR by PMD's during init and 

close.

quoted

quoted

Cc: Jianfeng Tan <redacted>

quoted

quoted

Jianfeng also implemented following patch:

quoted

http://dpdk.org/dev/patchwork/patch/17495/

quoted

quoted

Which also implements release and open ops, for slightly different

quoted

reason (prevent DMA access after app exit), but mainly both are to

quoted

gracefully handle application exit status.

quoted

quoted

btw, for Jianfeng's case, can adding pci_clear_master() in release and

quoted

moving pci_set_master() to open help preventing unwanted DMA?

quoted

quoted

quoted

Gregory,

quoted

quoted

Can you please check if this patch fixes your issue?

quoted

quoted

Thanks,

quoted

ferruh

quoted

quoted

quoted

quoted

quoted

Signed-off-by: Shijith Thotton <redacted>

quoted

quoted

---

quoted

quoted

lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 30 

++++++++++++++++++++++++++++++

quoted

quoted

1 file changed, 30 insertions(+)

quoted

quoted

quoted

quoted

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c 

b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c

quoted

quoted

index b9d427c..5bc58d2 100644

quoted

quoted

--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c

quoted

quoted

+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c

quoted

quoted

@@ -170,6 +170,34 @@ struct rte_uio_pci_dev {

quoted

quoted

return IRQ_HANDLED;

quoted

quoted

}

quoted

quoted

quoted

quoted

+/**

quoted

quoted

+ * This gets called while opening uio device file. It clears any 

previous state

quoted

quoted

+ * associated with the pci device.

quoted

quoted

+ */

quoted

quoted

+static int

quoted

quoted

+igbuio_pci_open(struct uio_info *info, struct inode *inode)

quoted

quoted

+{

quoted

quoted

+ struct rte_uio_pci_dev *udev = info->priv;

quoted

quoted

+ struct pci_dev *dev = udev->pdev;

quoted

quoted

+

quoted

quoted

+ /* reset the pci device */

quoted

quoted

+ pci_reset_function(dev);

quoted

quoted

+

quoted

quoted

+ return 0;

quoted

quoted

+}

quoted

quoted

+

quoted

quoted

+static int

quoted

quoted

+igbuio_pci_release(struct uio_info *info, struct inode *inode)

quoted

quoted

+{

quoted

quoted

+ struct rte_uio_pci_dev *udev = info->priv;

quoted

quoted

+ struct pci_dev *dev = udev->pdev;

quoted

quoted

+

quoted

quoted

+ /* try to reset the pci device */

quoted

quoted

+ pci_try_reset_function(dev);

quoted

quoted

+

quoted

quoted

+ return 0;

quoted

quoted

+}

quoted

quoted

+

quoted

quoted

#ifdef CONFIG_XEN_DOM0

quoted

quoted

static int

quoted

quoted

igbuio_dom0_mmap_phys(struct uio_info *info, struct vm_area_struct 

*vma)

quoted

quoted

@@ -372,6 +400,8 @@ struct rte_uio_pci_dev {

quoted

quoted

udev->info.version = "0.1";

quoted

quoted

udev->info.handler = igbuio_pci_irqhandler;

quoted

quoted

udev->info.irqcontrol = igbuio_pci_irqcontrol;

quoted

quoted

+ udev->info.open = igbuio_pci_open;

quoted

quoted

+ udev->info.release = igbuio_pci_release;

quoted

quoted

#ifdef CONFIG_XEN_DOM0

quoted

quoted

/* check if the driver run on Xen Dom0 */

quoted

quoted

if (xen_initial_domain())

quoted

quoted

quoted

quoted