[dpdk-stable] patch 'bus/vmbus: fix ring buffer mapping in secondary process' has been queued to stable release 20.11.4

[Xueming Li]

Hi,

FYI, your patch has been queued to stable release 20.11.4

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 11/12/21. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://github.com/steevenlee/dpdk

This queued commit can be viewed at:
https://github.com/steevenlee/dpdk/commit/be9717a4ccb476be8188ec94dd010aeb86d53a75

Thanks.

Xueming Li <xuemingl at nvidia.com>

---
>From be9717a4ccb476be8188ec94dd010aeb86d53a75 Mon Sep 17 00:00:00 2001
From: Long Li <longli at microsoft.com>
Date: Wed, 29 Sep 2021 13:46:10 -0700
Subject: [PATCH] bus/vmbus: fix ring buffer mapping in secondary process
Cc: Xueming Li <xuemingl at nvidia.com>

[ upstream commit 70cdd92e041acd7a0aad295f639f435fbe688190 ]

The driver code had wrong assumption that all the addresses to ring buffers
in the secondary process are the same as those in the primary process. This
is not always correct as the channels could be mapped to different
addresses in the secondary process.

Fix this by keeping track of all the mapped addresses from the primary
process in the shared uio_res, and have second process map to the same
addresses.

Fixes: 831dba47bd36 ("bus/vmbus: add Hyper-V virtual bus support")

Reported-by: Jonathan Erb <jonathan.erb at banduracyber.com>
Signed-off-by: Long Li <longli at microsoft.com>
Acked-by: Stephen Hemminger <sthemmin at microsoft.com>
---
 drivers/bus/vmbus/linux/vmbus_uio.c  | 99 +++++++++++++++-------------
 drivers/bus/vmbus/private.h          | 15 ++++-
 drivers/bus/vmbus/vmbus_channel.c    |  4 +-
 drivers/bus/vmbus/vmbus_common_uio.c | 52 +++++++++++----
 4 files changed, 107 insertions(+), 63 deletions(-)

diff --git a/drivers/bus/vmbus/linux/vmbus_uio.c b/drivers/bus/vmbus/linux/vmbus_uio.c
index 5dc0c47de6..fd64be93b0 100644
--- a/drivers/bus/vmbus/linux/vmbus_uio.c
+++ b/drivers/bus/vmbus/linux/vmbus_uio.c
@@ -11,6 +11,7 @@
 #include <sys/stat.h>
 #include <sys/mman.h>
 
+#include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_bus.h>
 #include <rte_memory.h>
@@ -203,6 +204,37 @@ static int vmbus_uio_map_subchan(const struct rte_vmbus_device *dev,
 	struct stat sb;
 	void *mapaddr;
 	int fd;
+	struct mapped_vmbus_resource *uio_res;
+	int channel_idx;
+
+	uio_res = vmbus_uio_find_resource(dev);
+	if (!uio_res) {
+		VMBUS_LOG(ERR, "can not find resources for mapping subchan");
+		return -ENOMEM;
+	}
+
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		if (uio_res->nb_subchannels >= UIO_MAX_SUBCHANNEL) {
+			VMBUS_LOG(ERR,
+				"exceeding max subchannels UIO_MAX_SUBCHANNEL(%d)",
+				UIO_MAX_SUBCHANNEL);
+			VMBUS_LOG(ERR, "Change UIO_MAX_SUBCHANNEL and recompile");
+			return -ENOMEM;
+		}
+	} else {
+		for (channel_idx = 0; channel_idx < uio_res->nb_subchannels;
+		     channel_idx++)
+			if (uio_res->subchannel_maps[channel_idx].relid ==
+					chan->relid)
+				break;
+		if (channel_idx == uio_res->nb_subchannels) {
+			VMBUS_LOG(ERR,
+				"couldn't find sub channel %d from shared mapping in primary",
+				chan->relid);
+			return -ENOMEM;
+		}
+		vmbus_map_addr = uio_res->subchannel_maps[channel_idx].addr;
+	}
 
 	snprintf(ring_path, sizeof(ring_path),
 		 "%s/%s/channels/%u/ring",
@@ -239,58 +271,33 @@ static int vmbus_uio_map_subchan(const struct rte_vmbus_device *dev,
 	if (mapaddr == MAP_FAILED)
 		return -EIO;
 
+	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+
+		/* Add this mapping to uio_res for use by secondary */
+		uio_res->subchannel_maps[uio_res->nb_subchannels].relid =
+			chan->relid;
+		uio_res->subchannel_maps[uio_res->nb_subchannels].addr =
+			mapaddr;
+		uio_res->subchannel_maps[uio_res->nb_subchannels].size =
+			file_size;
+		uio_res->nb_subchannels++;
+
+		vmbus_map_addr = RTE_PTR_ADD(mapaddr, file_size);
+	} else {
+		if (mapaddr != vmbus_map_addr) {
+			VMBUS_LOG(ERR, "failed to map channel %d to addr %p",
+					chan->relid, mapaddr);
+			vmbus_unmap_resource(mapaddr, file_size);
+			return -EIO;
+		}
+	}
+
 	*ring_size = file_size / 2;
 	*ring_buf = mapaddr;
 
-	vmbus_map_addr = RTE_PTR_ADD(mapaddr, file_size);
 	return 0;
 }
 
-int
-vmbus_uio_map_secondary_subchan(const struct rte_vmbus_device *dev,
-				const struct vmbus_channel *chan)
-{
-	const struct vmbus_br *br = &chan->txbr;
-	char ring_path[PATH_MAX];
-	void *mapaddr, *ring_buf;
-	uint32_t ring_size;
-	int fd;
-
-	snprintf(ring_path, sizeof(ring_path),
-		 "%s/%s/channels/%u/ring",
-		 SYSFS_VMBUS_DEVICES, dev->device.name,
-		 chan->relid);
-
-	ring_buf = br->vbr;
-	ring_size = br->dsize + sizeof(struct vmbus_bufring);
-	VMBUS_LOG(INFO, "secondary ring_buf %p size %u",
-		  ring_buf, ring_size);
-
-	fd = open(ring_path, O_RDWR);
-	if (fd < 0) {
-		VMBUS_LOG(ERR, "Cannot open %s: %s",
-			  ring_path, strerror(errno));
-		return -errno;
-	}
-
-	mapaddr = vmbus_map_resource(ring_buf, fd, 0, 2 * ring_size, 0);
-	close(fd);
-
-	if (mapaddr == ring_buf)
-		return 0;
-
-	if (mapaddr == MAP_FAILED)
-		VMBUS_LOG(ERR,
-			  "mmap subchan %u in secondary failed", chan->relid);
-	else {
-		VMBUS_LOG(ERR,
-			  "mmap subchan %u in secondary address mismatch",
-			  chan->relid);
-		vmbus_unmap_resource(mapaddr, 2 * ring_size);
-	}
-	return -1;
-}
-
 int vmbus_uio_map_rings(struct vmbus_channel *chan)
 {
 	const struct rte_vmbus_device *dev = chan->device;
diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h
index f19b14e4a6..74ef948282 100644
--- a/drivers/bus/vmbus/private.h
+++ b/drivers/bus/vmbus/private.h
@@ -36,6 +36,13 @@ struct vmbus_map {
 	uint64_t size;	/* length */
 };
 
+#define UIO_MAX_SUBCHANNEL 128
+struct subchannel_map {
+	uint16_t relid;
+	void *addr;
+	uint64_t size;
+};
+
 /*
  * For multi-process we need to reproduce all vmbus mappings in secondary
  * processes, so save them in a tailq.
@@ -44,10 +51,14 @@ struct mapped_vmbus_resource {
 	TAILQ_ENTRY(mapped_vmbus_resource) next;
 
 	rte_uuid_t id;
+
 	int nb_maps;
-	struct vmbus_channel *primary;
 	struct vmbus_map maps[VMBUS_MAX_RESOURCE];
+
 	char path[PATH_MAX];
+
+	int nb_subchannels;
+	struct subchannel_map subchannel_maps[UIO_MAX_SUBCHANNEL];
 };
 
 TAILQ_HEAD(mapped_vmbus_res_list, mapped_vmbus_resource);
@@ -108,8 +119,6 @@ bool vmbus_uio_subchannels_supported(const struct rte_vmbus_device *dev,
 int vmbus_uio_get_subchan(struct vmbus_channel *primary,
 			  struct vmbus_channel **subchan);
 int vmbus_uio_map_rings(struct vmbus_channel *chan);
-int vmbus_uio_map_secondary_subchan(const struct rte_vmbus_device *dev,
-				    const struct vmbus_channel *chan);
 
 void vmbus_br_setup(struct vmbus_br *br, void *buf, unsigned int blen);
 
diff --git a/drivers/bus/vmbus/vmbus_channel.c b/drivers/bus/vmbus/vmbus_channel.c
index f67f1c438a..119b9b367e 100644
--- a/drivers/bus/vmbus/vmbus_channel.c
+++ b/drivers/bus/vmbus/vmbus_channel.c
@@ -351,10 +351,8 @@ int rte_vmbus_chan_open(struct rte_vmbus_device *device,
 
 	err = vmbus_chan_create(device, device->relid, 0,
 				device->monitor_id, new_chan);
-	if (!err) {
+	if (!err)
 		device->primary = *new_chan;
-		uio_res->primary = *new_chan;
-	}
 
 	return err;
 }
diff --git a/drivers/bus/vmbus/vmbus_common_uio.c b/drivers/bus/vmbus/vmbus_common_uio.c
index a689bf11b3..158e05c889 100644
--- a/drivers/bus/vmbus/vmbus_common_uio.c
+++ b/drivers/bus/vmbus/vmbus_common_uio.c
@@ -69,8 +69,10 @@ vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
 					     fd, offset,
 					     uio_res->maps[i].size, 0);
 
-		if (mapaddr == uio_res->maps[i].addr)
+		if (mapaddr == uio_res->maps[i].addr) {
+			dev->resource[i].addr = mapaddr;
 			continue;	/* successful map */
+		}
 
 		if (mapaddr == MAP_FAILED)
 			VMBUS_LOG(ERR,
@@ -88,19 +90,39 @@ vmbus_uio_map_secondary(struct rte_vmbus_device *dev)
 	/* fd is not needed in secondary process, close it */
 	close(fd);
 
-	dev->primary = uio_res->primary;
-	if (!dev->primary) {
-		VMBUS_LOG(ERR, "missing primary channel");
-		return -1;
+	/* Create and map primary channel */
+	if (vmbus_chan_create(dev, dev->relid, 0,
+					dev->monitor_id, &dev->primary)) {
+		VMBUS_LOG(ERR, "cannot create primary channel");
+		goto failed_primary;
 	}
 
-	STAILQ_FOREACH(chan, &dev->primary->subchannel_list, next) {
-		if (vmbus_uio_map_secondary_subchan(dev, chan) != 0) {
-			VMBUS_LOG(ERR, "cannot map secondary subchan");
-			return -1;
+	/* Create and map sub channels */
+	for (i = 0; i < uio_res->nb_subchannels; i++) {
+		if (rte_vmbus_subchan_open(dev->primary, &chan)) {
+			VMBUS_LOG(ERR,
+				"failed to create subchannel at index %d", i);
+			goto failed_secondary;
 		}
 	}
+
 	return 0;
+
+failed_secondary:
+	while (!STAILQ_EMPTY(&dev->primary->subchannel_list)) {
+		chan = STAILQ_FIRST(&dev->primary->subchannel_list);
+		vmbus_unmap_resource(chan->txbr.vbr, chan->txbr.dsize * 2);
+		rte_vmbus_chan_close(chan);
+	}
+	rte_vmbus_chan_close(dev->primary);
+
+failed_primary:
+	for (i = 0; i != uio_res->nb_maps; i++) {
+		vmbus_unmap_resource(
+				uio_res->maps[i].addr, uio_res->maps[i].size);
+	}
+
+	return -1;
 }
 
 static int
@@ -188,6 +210,11 @@ vmbus_uio_unmap(struct mapped_vmbus_resource *uio_res)
 	if (uio_res == NULL)
 		return;
 
+	for (i = 0; i < uio_res->nb_subchannels; i++) {
+		vmbus_unmap_resource(uio_res->subchannel_maps[i].addr,
+				uio_res->subchannel_maps[i].size);
+	}
+
 	for (i = 0; i != uio_res->nb_maps; i++) {
 		vmbus_unmap_resource(uio_res->maps[i].addr,
 				     (size_t)uio_res->maps[i].size);
@@ -211,8 +238,11 @@ vmbus_uio_unmap_resource(struct rte_vmbus_device *dev)
 		return;
 
 	/* secondary processes - just free maps */
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-		return vmbus_uio_unmap(uio_res);
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+		vmbus_uio_unmap(uio_res);
+		rte_free(dev->primary);
+		return;
+	}
 
 	TAILQ_REMOVE(uio_res_list, uio_res, next);
 
-- 
2.33.0

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2021-11-10 14:17:07.352933455 +0800
+++ 0121-bus-vmbus-fix-ring-buffer-mapping-in-secondary-proce.patch	2021-11-10 14:17:01.884079315 +0800
@@ -1 +1 @@
-From 70cdd92e041acd7a0aad295f639f435fbe688190 Mon Sep 17 00:00:00 2001
+From be9717a4ccb476be8188ec94dd010aeb86d53a75 Mon Sep 17 00:00:00 2001
@@ -4,0 +5,3 @@
+Cc: Xueming Li <xuemingl at nvidia.com>
+
+[ upstream commit 70cdd92e041acd7a0aad295f639f435fbe688190 ]
@@ -16 +18,0 @@
-Cc: stable at dpdk.org
@@ -29 +31 @@
-index b52ca5bf1d..70b0d098e0 100644
+index 5dc0c47de6..fd64be93b0 100644
@@ -159 +161 @@
-index 528d60a42f..1bca147e12 100644
+index f19b14e4a6..74ef948282 100644
@@ -162 +164 @@
-@@ -33,6 +33,13 @@ struct vmbus_map {
+@@ -36,6 +36,13 @@ struct vmbus_map {
@@ -176 +178 @@
-@@ -41,10 +48,14 @@ struct mapped_vmbus_resource {
+@@ -44,10 +51,14 @@ struct mapped_vmbus_resource {
@@ -192 +194 @@
-@@ -105,8 +116,6 @@ bool vmbus_uio_subchannels_supported(const struct rte_vmbus_device *dev,
+@@ -108,8 +119,6 @@ bool vmbus_uio_subchannels_supported(const struct rte_vmbus_device *dev,
@@ -218 +220 @@
-index 8582e32c1d..041712fe75 100644
+index a689bf11b3..158e05c889 100644