[v2] bus/pci: fix legacy device IO port map in secondary process

Message ID 20230821012707.315173-1-wenwux.ma@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] bus/pci: fix legacy device IO port map in secondary process |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/github-robot: build success github build: passed
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/intel-Functional success Functional PASS

Commit Message

Ma, WenwuX Aug. 21, 2023, 1:27 a.m. UTC
  When doing IO port mapping for legacy device
in secondary process, the region information
is missing, so, we need to refill it.

Fixes: 4b741542ecde ("bus/pci: avoid depending on private kernel value")
Cc: stable@dpdk.org

Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
---
 v2: add release of device in pci_vfio_ioport_unmap

---
 drivers/bus/pci/linux/pci_vfio.c | 43 ++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)
  

Comments

Stephen Hemminger Aug. 21, 2023, 2:53 a.m. UTC | #1
On Mon, 21 Aug 2023 09:27:07 +0800
Wenwu Ma <wenwux.ma@intel.com> wrote:

> +	struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
> +	char pci_addr[PATH_MAX] = {0}

Not sure if some tools will complain about initializing chars as zero.
Anyway, why bother since you are using it with snprintf.

Also, the new variables that are only used in the secondary case
should be declared in that if() not for whole function.
  
Ma, WenwuX Aug. 22, 2023, 2:13 a.m. UTC | #2
Hi,

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: 2023年8月21日 10:53
> To: Ma, WenwuX <wenwux.ma@intel.com>
> Cc: nipun.gupta@amd.com; dev@dpdk.org; david.marchand@redhat.com;
> maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>; Li,
> Miao <miao.li@intel.com>; Ling, WeiX <weix.ling@intel.com>;
> stable@dpdk.org
> Subject: Re: [PATCH v2] bus/pci: fix legacy device IO port map in secondary
> process
> 
> On Mon, 21 Aug 2023 09:27:07 +0800
> Wenwu Ma <wenwux.ma@intel.com> wrote:
> 
> > +	struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
> > +	char pci_addr[PATH_MAX] = {0}
> 
> Not sure if some tools will complain about initializing chars as zero.
> Anyway, why bother since you are using it with snprintf.
> 
> Also, the new variables that are only used in the secondary case should be
> declared in that if() not for whole function.
> 
> 
Ok, thanks
  

Patch

diff --git a/drivers/bus/pci/linux/pci_vfio.c b/drivers/bus/pci/linux/pci_vfio.c
index e634de8322..f2cc55c431 100644
--- a/drivers/bus/pci/linux/pci_vfio.c
+++ b/drivers/bus/pci/linux/pci_vfio.c
@@ -1306,6 +1306,11 @@  int
 pci_vfio_ioport_map(struct rte_pci_device *dev, int bar,
 		    struct rte_pci_ioport *p)
 {
+	struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
+	char pci_addr[PATH_MAX] = {0};
+	int vfio_dev_fd;
+	struct rte_pci_addr *loc = &dev->addr;
+	int ret;
 	uint64_t size, offset;
 
 	if (bar < VFIO_PCI_BAR0_REGION_INDEX ||
@@ -1314,6 +1319,22 @@  pci_vfio_ioport_map(struct rte_pci_device *dev, int bar,
 		return -1;
 	}
 
+	if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+		/* store PCI address string */
+		snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT,
+				loc->domain, loc->bus, loc->devid, loc->function);
+
+		ret = rte_vfio_setup_device(rte_pci_get_sysfs_path(), pci_addr,
+						&vfio_dev_fd, &device_info);
+		if (ret)
+			return -1;
+
+		ret = pci_vfio_fill_regions(dev, vfio_dev_fd, &device_info);
+		if (ret)
+			return -1;
+
+	}
+
 	if (pci_vfio_get_region(dev, bar, &size, &offset) != 0) {
 		RTE_LOG(ERR, EAL, "Cannot get offset of region %d.\n", bar);
 		return -1;
@@ -1361,8 +1382,26 @@  pci_vfio_ioport_write(struct rte_pci_ioport *p,
 int
 pci_vfio_ioport_unmap(struct rte_pci_ioport *p)
 {
-	RTE_SET_USED(p);
-	return -1;
+	char pci_addr[PATH_MAX] = {0};
+	struct rte_pci_addr *loc = &p->dev->addr;
+	int ret, vfio_dev_fd;
+
+	/* store PCI address string */
+	snprintf(pci_addr, sizeof(pci_addr), PCI_PRI_FMT,
+			loc->domain, loc->bus, loc->devid, loc->function);
+
+	vfio_dev_fd = rte_intr_dev_fd_get(p->dev->intr_handle);
+	if (vfio_dev_fd < 0)
+		return -1;
+
+	ret = rte_vfio_release_device(rte_pci_get_sysfs_path(), pci_addr,
+				      vfio_dev_fd);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL, "Cannot release VFIO device\n");
+		return ret;
+	}
+
+	return 0;
 }
 
 int