On 4/4/2018 1:25 PM, Tan, Jianfeng wrote:

-----Original Message-----
From: Guo, Jia
Sent: Wednesday, April 4, 2018 2:17 AM
To: step...@networkplumber.org; Richardson, Bruce; Yigit, Ferruh;
Ananyev, Konstantin; gaetan.ri...@6wind.com; Wu, Jingjing;
tho...@monjalon.net; mo...@mellanox.com; Van Haaren, Harry; Tan,
Jianfeng
Cc: jblu...@infradead.org; shreyansh.j...@nxp.com; dev@dpdk.org; Guo,
Jia; Zhang, Helin
Subject: [PATCH V18 2/5] bus/pci: implement handle hot unplug operation

When handle device hot unplug event, remap a dummy memory to avoid
bus read/write error.

Signed-off-by: Jeff Guo <jia....@intel.com>
---
v16->v15;
split patch, merge some function to be simple
---
  drivers/bus/pci/pci_common.c     | 42
++++++++++++++++++++++++++++++++++++++++
  drivers/bus/pci/pci_common_uio.c | 33
+++++++++++++++++++++++++++++++
  drivers/bus/pci/private.h        | 12 ++++++++++++
  3 files changed, 87 insertions(+)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 2a00f36..fa077ec 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -474,6 +474,47 @@ pci_find_device(const struct rte_device *start,
rte_dev_cmp_t cmp,
  }

  static int
+pci_handle_hot_unplug(struct rte_device *dev)
+{
+       struct rte_pci_device *pdev;
+       int ret;
+
+       if (dev == NULL)
+               return -EINVAL;
+
+       pdev = RTE_DEV_TO_PCI(dev);
+
+       /* remap resources for devices */
+       switch (pdev->kdrv) {
+       case RTE_KDRV_VFIO:
+#ifdef VFIO_PRESENT
+               /* TODO */
+#endif
What's the difference between uio and vfio? We can just fall though?
the VFIO mapping is functional different with uio, and the key is i don't implement vfio hotplug right now, so let it TODO.
+               break;
+       case RTE_KDRV_IGB_UIO:
+       case RTE_KDRV_UIO_GENERIC:
+               if (rte_eal_using_phys_addrs()) {
Why do we care about if we are using physical addresses?
please check with the mapping function.
+                       /* map resources for devices that use uio */
+                       ret = pci_uio_remap_resource(pdev);
+               }
+               break;
+       case RTE_KDRV_NIC_UIO:
+               ret = pci_uio_remap_resource(pdev);
+               break;
+       default:
+               RTE_LOG(DEBUG, EAL,
+                       "  Not managed by a supported kernel driver, 
skipped\n");
+               ret = 1;
-1 for such case?
thanks.
+               break;
+       }
+
+       if (ret != 0)
+               RTE_LOG(ERR, EAL, "failed to handle hot unplug of %s",
+                       pdev->name);
+       return ret;
+}
+
+static int
  pci_plug(struct rte_device *dev)
  {
        return pci_probe_all_drivers(RTE_DEV_TO_PCI(dev));
@@ -503,6 +544,7 @@ struct rte_pci_bus rte_pci_bus = {
                .unplug = pci_unplug,
                .parse = pci_parse,
                .get_iommu_class = rte_pci_get_iommu_class,
+               .handle_hot_unplug = pci_handle_hot_unplug,
        },
        .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list),
        .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
diff --git a/drivers/bus/pci/pci_common_uio.c
b/drivers/bus/pci/pci_common_uio.c
index 54bc20b..468ade4 100644
--- a/drivers/bus/pci/pci_common_uio.c
+++ b/drivers/bus/pci/pci_common_uio.c
@@ -146,6 +146,39 @@ pci_uio_unmap(struct mapped_pci_resource
*uio_res)
        }
  }

+/* remap the PCI resource of a PCI device in private virtual memory */
+int
+pci_uio_remap_resource(struct rte_pci_device *dev)
Why's this function uio specific? I think we can move it to pci_common.c.
not convince because not let vfio in this patch set.
+{
+       int i;
+       uint64_t phaddr;
+       void *map_address;
+
+       if (dev == NULL)
+               return -1;
+
+       /* Map all BARs */
s/Map/Remap
+       for (i = 0; i != PCI_MAX_RESOURCE; i++) {
+               /* skip empty BAR */
+               phaddr = dev->mem_resource[i].phys_addr;
+               if (phaddr == 0)
+                       continue;
How about just simple:

if (dev->mem_resource[i].phys_addr == 0)

+               pci_unmap_resource(dev->mem_resource[i].addr,
+                               (size_t)dev->mem_resource[i].len);
+               map_address = pci_map_resource(
+                               dev->mem_resource[i].addr, -1, 0,
+                               (size_t)dev->mem_resource[i].len,
+                               MAP_ANONYMOUS | MAP_FIXED);
+               if (map_address == MAP_FAILED) {
+                       RTE_LOG(ERR, EAL,
+                               "Cannot remap resource for device %s\n",
dev->name);
+                       return -1;
+               }
+       }
+
+       return 0;
+}
+
  static struct mapped_pci_resource *
  pci_uio_find_resource(struct rte_pci_device *dev)
  {
diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h
index 88fa587..7a862ef 100644
--- a/drivers/bus/pci/private.h
+++ b/drivers/bus/pci/private.h
@@ -173,6 +173,18 @@ void pci_uio_free_resource(struct rte_pci_device
*dev,
                struct mapped_pci_resource *uio_res);

  /**
+ * remap the pci uio resource..
+ *
+ * @param dev
+ *   Point to the struct rte pci device.
+ * @return
+ *   - On success, zero.
+ *   - On failure, a negative value.
+ */
+int
+pci_uio_remap_resource(struct rte_pci_device *dev);
If we define

+
+/**
   * Map device memory to uio resource
   *
   * This function is private to EAL.
--
2.7.4

Reply via email to