From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id A42F3A0547 for ; Wed, 29 Sep 2021 22:46:19 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 89A9C410EF; Wed, 29 Sep 2021 22:46:19 +0200 (CEST) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id 1C5B9410E5; Wed, 29 Sep 2021 22:46:16 +0200 (CEST) Received: by linux.microsoft.com (Postfix, from userid 1004) id 4D00F20B8008; Wed, 29 Sep 2021 13:46:15 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 4D00F20B8008 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linuxonhyperv.com; s=default; t=1632948375; bh=xrS79hhBtxRv4I2B22qVMs02m/LTnyv8EJ4TlQ34GL0=; h=From:To:Cc:Subject:Date:From; b=NXkcsgL8xkJHrSt9D66J9kbojrMGon2oacubeo/5SkLV0qhqMXyhzrylpbr8CFcJi Ix5d0Vos+KFFR1z4VmyGLel9stp7FbCMg0ggd6HXD+kQeV7x34PIraIYUPmUl5n18H T2VY8x//lyvNLOZvIhB5NPbphYvmao5h7O9cCGNY= From: longli@linuxonhyperv.com To: dev@dpdk.org, Stephen Hemminger , Jonathan Erb Cc: Long Li , stable@dpdk.org Date: Wed, 29 Sep 2021 13:46:10 -0700 Message-Id: <1632948370-25668-1-git-send-email-longli@linuxonhyperv.com> X-Mailer: git-send-email 1.8.3.1 Subject: [dpdk-stable] [Patch v2] bus/vmbus: map to the correct ring buffer addresses for the secondary process X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" From: Long Li 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 Cc: stable@dpdk.org Signed-off-by: Long Li --- Changes in v2: Fixed compiling errors on ARM64. Made log messages single lines to make it easier to locate failures. 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 b52ca5bf1..70b0d098e 100644 --- a/drivers/bus/vmbus/linux/vmbus_uio.c +++ b/drivers/bus/vmbus/linux/vmbus_uio.c @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -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 528d60a42..1bca147e1 100644 --- a/drivers/bus/vmbus/private.h +++ b/drivers/bus/vmbus/private.h @@ -33,6 +33,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. @@ -41,10 +48,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); @@ -105,8 +116,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 f67f1c438..119b9b367 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 8582e32c1..041712fe7 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.25.1