diff mbox series

[v10,03/19] bus/pci: enable vfio unmap resource for secondary

Message ID 20180709033706.27858-4-qi.z.zhang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series enable hotplug on multi-process |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Qi Zhang July 9, 2018, 3:36 a.m. UTC 
  Subroutine to unmap VFIO resource is shared by secondary and
primary, and it does not work on the secondary process.
The patch adds a dedicate function to handle the situation
when a device is unmapped on a secondary process.

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 drivers/bus/pci/linux/pci_vfio.c | 75 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 2 deletions(-)

Comments

Burakov, Anatoly July 9, 2018, 2:37 p.m. UTC | #1 
On 09-Jul-18 4:36 AM, Qi Zhang wrote:
> Subroutine to unmap VFIO resource is shared by secondary and
> primary, and it does not work on the secondary process.
> The patch adds a dedicate function to handle the situation
> when a device is unmapped on a secondary process.
> 
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---

Hi Qi,

Please correct me if i'm wrong here, but it seems like the unmapping 
code is shared between primary and secondary, and the difference comes 
from interrupts, bus mastering, and removing the device from tailq. Can 
we separate out the common code somehow?
Qi Zhang July 10, 2018, 12:30 a.m. UTC | #2 
> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Monday, July 9, 2018 10:38 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; thomas@monjalon.net
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Shelton, Benjamin H
> <benjamin.h.shelton@intel.com>; Vangati, Narender
> <narender.vangati@intel.com>
> Subject: Re: [PATCH v10 03/19] bus/pci: enable vfio unmap resource for
> secondary
> 
> On 09-Jul-18 4:36 AM, Qi Zhang wrote:
> > Subroutine to unmap VFIO resource is shared by secondary and primary,
> > and it does not work on the secondary process.
> > The patch adds a dedicate function to handle the situation when a
> > device is unmapped on a secondary process.
> >
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> 
> Hi Qi,
> 
> Please correct me if i'm wrong here, but it seems like the unmapping code is
> shared between primary and secondary, and the difference comes from
> interrupts, bus mastering, and removing the device from tailq. Can we
> separate out the common code somehow?

Yes, you are right and I will separate out the common code.

Thanks
Qi
> 
> --
> Thanks,
> Anatoly
diff mbox series

Patch

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index dd25c3542..72481ac45 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -595,6 +595,9 @@  pci_vfio_map_resource_secondary(struct rte_pci_device *dev)
 		dev->mem_resource[i].addr = maps[i].addr;
 	}
 
+	/* we need save vfio_dev_fd, so it can be used during release */
+	dev->intr_handle.vfio_dev_fd = vfio_dev_fd;
+
 	return 0;
 err_vfio_dev_fd:
 	close(vfio_dev_fd);
@@ -614,8 +617,8 @@  pci_vfio_map_resource(struct rte_pci_device *dev)
 		return pci_vfio_map_resource_secondary(dev);
 }
 
-int
-pci_vfio_unmap_resource(struct rte_pci_device *dev)
+static int
+pci_vfio_unmap_resource_primary(struct rte_pci_device *dev)
 {
 	char pci_addr[PATH_MAX] = {0};
 	struct rte_pci_addr *loc = &dev->addr;
@@ -687,6 +690,74 @@  pci_vfio_unmap_resource(struct rte_pci_device *dev)
 	return 0;
 }
 
+static int
+pci_vfio_unmap_resource_secondary(struct rte_pci_device *dev)
+{
+	char pci_addr[PATH_MAX] = {0};
+	struct rte_pci_addr *loc = &dev->addr;
+	int i, ret;
+	struct mapped_pci_resource *vfio_res = NULL;
+	struct mapped_pci_res_list *vfio_res_list;
+
+	struct pci_map *maps;
+
+	/* store PCI address string */
+	snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT,
+			loc->domain, loc->bus, loc->devid, loc->function);
+
+	ret = rte_vfio_release_device(rte_pci_get_sysfs_path(), pci_addr,
+				  dev->intr_handle.vfio_dev_fd);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL,
+			"%s(): cannot release device\n", __func__);
+		return ret;
+	}
+
+	vfio_res_list =
+		RTE_TAILQ_CAST(rte_vfio_tailq.head, mapped_pci_res_list);
+	/* Get vfio_res */
+	TAILQ_FOREACH(vfio_res, vfio_res_list, next) {
+		if (pci_addr_cmp(&vfio_res->pci_addr, &dev->addr))
+			continue;
+		break;
+	}
+	/* if we haven't found our tailq entry, something's wrong */
+	if (vfio_res == NULL) {
+		RTE_LOG(ERR, EAL, "  %s cannot find TAILQ entry for PCI device!\n",
+				pci_addr);
+		return -1;
+	}
+
+	/* unmap BARs */
+	maps = vfio_res->maps;
+
+	RTE_LOG(INFO, EAL, "Releasing pci mapped resource for %s\n",
+		pci_addr);
+	for (i = 0; i < (int) vfio_res->nb_maps; i++) {
+
+		/*
+		 * We do not need to be aware of MSI-X table BAR mappings as
+		 * when mapping. Just using current maps array is enough
+		 */
+		if (maps[i].addr) {
+			RTE_LOG(INFO, EAL, "Calling pci_unmap_resource for %s at %p\n",
+				pci_addr, maps[i].addr);
+			pci_unmap_resource(maps[i].addr, maps[i].size);
+		}
+	}
+
+	return 0;
+}
+
+int
+pci_vfio_unmap_resource(struct rte_pci_device *dev)
+{
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
+		return pci_vfio_unmap_resource_primary(dev);
+	else
+		return pci_vfio_unmap_resource_secondary(dev);
+}
+
 int
 pci_vfio_ioport_map(struct rte_pci_device *dev, int bar,
 		    struct rte_pci_ioport *p)