[dpdk-dev] [PATCH v10 7/8] igb_uio: fix unexpected remove issue for hotplug

[Jeff Guo]

When a device is hotplugged out, the PCI resource is released in the
kernel, the UIO file descriptor will disappear and the irq will be
released. After this, a kernel crash will be caused if the igb uio driver
tries to access or release these resources.

And more, uio_remove will be called unexpectedly before uio_release
when device be hotpluggged out, the uio_remove procedure will
free resources that are required by uio_release. This will later affect the
usage of interrupt as there is no way to disable the interrupt which is
defined in uio_release.

To prevent this, the hotplug removal needs to be identified and processed
accordingly in igb uio driver.

This patch proposes the addition of enum rte_udev_state in the
rte_uio_pci_dev struct. This will store the state of the uio device as one
of the following: probed/opened/released/removed.

This patch also checks the kobject's remove_uevent_sent state to detect if
the removal status is hotplug-out. Once a hotplug-out is detected, it will
call uio_release and set the uio status to "removed". After that, uio will
check the status in the uio_release function. If uio has already been
removed, it will only free the dirty uio resource.

Signed-off-by: Jeff Guo <jia.guo at intel.com>
Acked-by: Shaopeng He <shaopeng.he at intel.com>
---
v10->v9:
refine commmit log.
---
 kernel/linux/igb_uio/igb_uio.c | 69 +++++++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 14 deletions(-)

diff --git a/kernel/linux/igb_uio/igb_uio.c b/kernel/linux/igb_uio/igb_uio.c
index 3398eac..d126371 100644
--- a/kernel/linux/igb_uio/igb_uio.c
+++ b/kernel/linux/igb_uio/igb_uio.c
@@ -19,6 +19,14 @@
 
 #include "compat.h"
 
+/* uio pci device state */
+enum rte_udev_state {
+	RTE_UDEV_PROBED,
+	RTE_UDEV_OPENNED,
+	RTE_UDEV_RELEASED,
+	RTE_UDEV_REMOVED,
+};
+
 /**
  * A structure describing the private information for a uio device.
  */
@@ -28,6 +36,7 @@ struct rte_uio_pci_dev {
 	enum rte_intr_mode mode;
 	struct mutex lock;
 	int refcnt;
+	enum rte_udev_state state;
 };
 
 static int wc_activate;
@@ -309,6 +318,17 @@ igbuio_pci_disable_interrupts(struct rte_uio_pci_dev *udev)
 #endif
 }
 
+/* Unmap previously ioremap'd resources */
+static void
+igbuio_pci_release_iomem(struct uio_info *info)
+{
+	int i;
+
+	for (i = 0; i < MAX_UIO_MAPS; i++) {
+		if (info->mem[i].internal_addr)
+			iounmap(info->mem[i].internal_addr);
+	}
+}
 
 /**
  * This gets called while opening uio device file.
@@ -331,20 +351,35 @@ igbuio_pci_open(struct uio_info *info, struct inode *inode)
 
 	/* enable interrupts */
 	err = igbuio_pci_enable_interrupts(udev);
-	mutex_unlock(&udev->lock);
 	if (err) {
 		dev_err(&dev->dev, "Enable interrupt fails\n");
+		pci_clear_master(dev);
+		mutex_unlock(&udev->lock);
 		return err;
 	}
+	udev->state = RTE_UDEV_OPENNED;
+	mutex_unlock(&udev->lock);
 	return 0;
 }
 
+/**
+ * This gets called while closing uio device file.
+ */
 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;
 
+	if (udev->state == RTE_UDEV_REMOVED) {
+		mutex_destroy(&udev->lock);
+		igbuio_pci_release_iomem(&udev->info);
+		pci_disable_device(dev);
+		pci_set_drvdata(dev, NULL);
+		kfree(udev);
+		return 0;
+	}
+
 	mutex_lock(&udev->lock);
 	if (--udev->refcnt > 0) {
 		mutex_unlock(&udev->lock);
@@ -356,7 +391,7 @@ igbuio_pci_release(struct uio_info *info, struct inode *inode)
 
 	/* stop the device from further DMA */
 	pci_clear_master(dev);
-
+	udev->state = RTE_UDEV_RELEASED;
 	mutex_unlock(&udev->lock);
 	return 0;
 }
@@ -414,18 +449,6 @@ igbuio_pci_setup_ioport(struct pci_dev *dev, struct uio_info *info,
 	return 0;
 }
 
-/* Unmap previously ioremap'd resources */
-static void
-igbuio_pci_release_iomem(struct uio_info *info)
-{
-	int i;
-
-	for (i = 0; i < MAX_UIO_MAPS; i++) {
-		if (info->mem[i].internal_addr)
-			iounmap(info->mem[i].internal_addr);
-	}
-}
-
 static int
 igbuio_setup_bars(struct pci_dev *dev, struct uio_info *info)
 {
@@ -562,6 +585,9 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 			 (unsigned long long)map_dma_addr, map_addr);
 	}
 
+	mutex_lock(&udev->lock);
+	udev->state = RTE_UDEV_PROBED;
+	mutex_unlock(&udev->lock);
 	return 0;
 
 fail_remove_group:
@@ -579,6 +605,21 @@ static void
 igbuio_pci_remove(struct pci_dev *dev)
 {
 	struct rte_uio_pci_dev *udev = pci_get_drvdata(dev);
+	struct pci_dev *pdev = udev->pdev;
+	int ret;
+
+	/* handle unexpected removal */
+	if (udev->state == RTE_UDEV_OPENNED ||
+	    (&pdev->dev.kobj)->state_remove_uevent_sent == 1) {
+		dev_notice(&dev->dev, "Unexpected removal!\n");
+		ret = igbuio_pci_release(&udev->info, NULL);
+		if (ret)
+			return;
+		mutex_lock(&udev->lock);
+		udev->state = RTE_UDEV_REMOVED;
+		mutex_unlock(&udev->lock);
+		return;
+	}
 
 	mutex_destroy(&udev->lock);
 	sysfs_remove_group(&dev->dev.kobj, &dev_attr_grp);
-- 
2.7.4