[PATCH] net/virtio: fix descriptors buffer addresses on 32 bits builds

[PATCH] net/virtio: fix descriptors buffer addresses on 32 bits builds

[Maxime Coquelin]

With Virtio-user, the Virtio descriptor buffer address is the
virtual address of the mbuf's buffer. On 32 bits builds, it is
expected to be 32 bits.

With Virtio-PCI, the Virtio descriptor buffer address is the
physical address of the mbuf's buffer. On 32 bits builds running
on 64 bits kernel, it is expected to be up to 64 bits.

This patch introduces a new mask field in virtqueue's struct to
filter our the upper 4 bytes of the address only when necessary.
An optimization is introduced for 64 bits builds to remove the
masking, as the address is always 64 bits wide.

Fixes: ba55c94a7ebc ("net/virtio: revert forcing IOVA as VA mode for virtio-user")
Cc: stable@dpdk.org

Reported-by: Sampath Peechu <speechu@cisco.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtqueue.c |  2 ++
 drivers/net/virtio/virtqueue.h | 18 ++++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
index 1d836f2530..6f419665f1 100644
--- a/drivers/net/virtio/virtqueue.c
+++ b/drivers/net/virtio/virtqueue.c
@@ -469,9 +469,11 @@ virtqueue_alloc(struct virtio_hw *hw, uint16_t index, uint16_t num, int type,
 	if (hw->use_va) {
 		vq->vq_ring_mem = (uintptr_t)mz->addr;
 		vq->mbuf_addr_offset = offsetof(struct rte_mbuf, buf_addr);
+		vq->mbuf_addr_mask = UINTPTR_MAX;
 	} else {
 		vq->vq_ring_mem = mz->iova;
 		vq->mbuf_addr_offset = offsetof(struct rte_mbuf, buf_iova);
+		vq->mbuf_addr_mask = UINT64_MAX;
 	}
 
 	PMD_INIT_LOG(DEBUG, "vq->vq_ring_mem: 0x%" PRIx64, vq->vq_ring_mem);
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 9d4aba11a3..c1cb941c43 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -114,17 +114,26 @@ virtqueue_store_flags_packed(struct vring_packed_desc *dp,
 
 #define VIRTQUEUE_MAX_NAME_SZ 32
 
+#ifdef RTE_ARCH_32
+#define VIRTIO_MBUF_ADDR_MASK(vq) ((vq)->mbuf_addr_mask)
+#else
+#define VIRTIO_MBUF_ADDR_MASK(vq) UINT64_MAX
+#endif
+
 /**
  * Return the IOVA (or virtual address in case of virtio-user) of mbuf
  * data buffer.
  *
  * The address is firstly casted to the word size (sizeof(uintptr_t))
- * before casting it to uint64_t. This is to make it work with different
- * combination of word size (64 bit and 32 bit) and virtio device
- * (virtio-pci and virtio-user).
+ * before casting it to uint64_t. It is then masked with the expected
+ * address length (64 bits for virtio-pci, word size for virtio-user).
+ *
+ * This is to make it work with different combination of word size (64
+ * bit and 32 bit) and virtio device (virtio-pci and virtio-user).
  */
 #define VIRTIO_MBUF_ADDR(mb, vq) \
-	((uint64_t)(*(uintptr_t *)((uintptr_t)(mb) + (vq)->mbuf_addr_offset)))
+	((*(uint64_t *)((uintptr_t)(mb) + (vq)->mbuf_addr_offset)) & \
+		VIRTIO_MBUF_ADDR_MASK(vq))
 
 /**
  * Return the physical address (or virtual address in case of
@@ -194,6 +203,7 @@ struct virtqueue {
 	void *vq_ring_virt_mem;  /**< linear address of vring*/
 	unsigned int vq_ring_size;
 	uint16_t mbuf_addr_offset;
+	uint64_t mbuf_addr_mask;
 
 	union {
 		struct virtnet_rx rxq;
-- 
2.41.0

Re: [PATCH] net/virtio: fix descriptors buffer addresses on 32 bits builds

[David Marchand]

On Wed, Sep 20, 2023 at 3:02 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> With Virtio-user, the Virtio descriptor buffer address is the
> virtual address of the mbuf's buffer. On 32 bits builds, it is
> expected to be 32 bits.
>
> With Virtio-PCI, the Virtio descriptor buffer address is the
> physical address of the mbuf's buffer. On 32 bits builds running
> on 64 bits kernel, it is expected to be up to 64 bits.
>
> This patch introduces a new mask field in virtqueue's struct to
> filter our the upper 4 bytes of the address only when necessary.
> An optimization is introduced for 64 bits builds to remove the
> masking, as the address is always 64 bits wide.
>
> Fixes: ba55c94a7ebc ("net/virtio: revert forcing IOVA as VA mode for virtio-user")
> Cc: stable@dpdk.org
>
> Reported-by: Sampath Peechu <speechu@cisco.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Reviewed-by: David Marchand <david.marchand@redhat.com>

-- 
David Marchand

Re: [PATCH] net/virtio: fix descriptors buffer addresses on 32 bits builds

[Dave Johnson (davejo)]

[-- Attachment #1: Type: text/plain, Size: 5246 bytes --]

Hi Maxime,
I back ported the patch to v22.11.2 and it worked for us on both the testpmd app and with our 32-bit DPDK (virtio-pci) application.  The change to set the mbuf_addr_mask was moved under virtio_init_queue() in v22.11.2 (see below).

I’m in the process of updating the application to v23.07 and will test there as well.  Thank you for looking into this and providing the patch.
Regards, Dave

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index b72334455e..bd90ba9d49 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -565,10 +565,13 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t queue_idx)
        memset(mz->addr, 0, mz->len);
-       if (hw->use_va)
+       if (hw->use_va) {
                vq->vq_ring_mem = (uintptr_t)mz->addr;
-       else
+               vq->mbuf_addr_mask = UINTPTR_MAX;
+       } else {
                vq->vq_ring_mem = mz->iova;
+               vq->mbuf_addr_mask = UINT64_MAX;
+       }
        vq->vq_ring_virt_mem = mz->addr;
        PMD_INIT_LOG(DEBUG, "vq->vq_ring_mem: 0x%" PRIx64, vq->vq_ring_mem);

From: Maxime Coquelin <maxime.coquelin@redhat.com>
Date: Wednesday, September 20, 2023 at 9:02 AM
To: dev@dpdk.org <dev@dpdk.org>, Roger Melton (rmelton) <rmelton@cisco.com>, Dave Johnson (davejo) <davejo@cisco.com>, Sampath Peechu (speechu) <speechu@cisco.com>, chenbo.xia@outlook.com <chenbo.xia@outlook.com>, Malcolm Bumgardner (mbumgard) <mbumgard@cisco.com>, Chris Brezovec (cbrezove) <cbrezove@cisco.com>, david.marchand@redhat.com <david.marchand@redhat.com>
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>, stable@dpdk.org <stable@dpdk.org>
Subject: [PATCH] net/virtio: fix descriptors buffer addresses on 32 bits builds
With Virtio-user, the Virtio descriptor buffer address is the
virtual address of the mbuf's buffer. On 32 bits builds, it is
expected to be 32 bits.

With Virtio-PCI, the Virtio descriptor buffer address is the
physical address of the mbuf's buffer. On 32 bits builds running
on 64 bits kernel, it is expected to be up to 64 bits.

This patch introduces a new mask field in virtqueue's struct to
filter our the upper 4 bytes of the address only when necessary.
An optimization is introduced for 64 bits builds to remove the
masking, as the address is always 64 bits wide.

Fixes: ba55c94a7ebc ("net/virtio: revert forcing IOVA as VA mode for virtio-user")
Cc: stable@dpdk.org

Reported-by: Sampath Peechu <speechu@cisco.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtqueue.c |  2 ++
 drivers/net/virtio/virtqueue.h | 18 ++++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio/virtqueue.c b/drivers/net/virtio/virtqueue.c
index 1d836f2530..6f419665f1 100644
--- a/drivers/net/virtio/virtqueue.c
+++ b/drivers/net/virtio/virtqueue.c
@@ -469,9 +469,11 @@ virtqueue_alloc(struct virtio_hw *hw, uint16_t index, uint16_t num, int type,
         if (hw->use_va) {
                 vq->vq_ring_mem = (uintptr_t)mz->addr;
                 vq->mbuf_addr_offset = offsetof(struct rte_mbuf, buf_addr);
+               vq->mbuf_addr_mask = UINTPTR_MAX;
         } else {
                 vq->vq_ring_mem = mz->iova;
                 vq->mbuf_addr_offset = offsetof(struct rte_mbuf, buf_iova);
+               vq->mbuf_addr_mask = UINT64_MAX;
         }

         PMD_INIT_LOG(DEBUG, "vq->vq_ring_mem: 0x%" PRIx64, vq->vq_ring_mem);
diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
index 9d4aba11a3..c1cb941c43 100644
--- a/drivers/net/virtio/virtqueue.h
+++ b/drivers/net/virtio/virtqueue.h
@@ -114,17 +114,26 @@ virtqueue_store_flags_packed(struct vring_packed_desc *dp,

 #define VIRTQUEUE_MAX_NAME_SZ 32

+#ifdef RTE_ARCH_32
+#define VIRTIO_MBUF_ADDR_MASK(vq) ((vq)->mbuf_addr_mask)
+#else
+#define VIRTIO_MBUF_ADDR_MASK(vq) UINT64_MAX
+#endif
+
 /**
  * Return the IOVA (or virtual address in case of virtio-user) of mbuf
  * data buffer.
  *
  * The address is firstly casted to the word size (sizeof(uintptr_t))
- * before casting it to uint64_t. This is to make it work with different
- * combination of word size (64 bit and 32 bit) and virtio device
- * (virtio-pci and virtio-user).
+ * before casting it to uint64_t. It is then masked with the expected
+ * address length (64 bits for virtio-pci, word size for virtio-user).
+ *
+ * This is to make it work with different combination of word size (64
+ * bit and 32 bit) and virtio device (virtio-pci and virtio-user).
  */
 #define VIRTIO_MBUF_ADDR(mb, vq) \
-       ((uint64_t)(*(uintptr_t *)((uintptr_t)(mb) + (vq)->mbuf_addr_offset)))
+       ((*(uint64_t *)((uintptr_t)(mb) + (vq)->mbuf_addr_offset)) & \
+               VIRTIO_MBUF_ADDR_MASK(vq))

 /**
  * Return the physical address (or virtual address in case of
@@ -194,6 +203,7 @@ struct virtqueue {
         void *vq_ring_virt_mem;  /**< linear address of vring*/
         unsigned int vq_ring_size;
         uint16_t mbuf_addr_offset;
+       uint64_t mbuf_addr_mask;

         union {
                 struct virtnet_rx rxq;
--
2.41.0

[-- Attachment #2: Type: text/html, Size: 11568 bytes --]

Re: [PATCH] net/virtio: fix descriptors buffer addresses on 32 bits builds

[Maxime Coquelin]

On 9/20/23 15:01, Maxime Coquelin wrote:
> With Virtio-user, the Virtio descriptor buffer address is the
> virtual address of the mbuf's buffer. On 32 bits builds, it is
> expected to be 32 bits.
> 
> With Virtio-PCI, the Virtio descriptor buffer address is the
> physical address of the mbuf's buffer. On 32 bits builds running
> on 64 bits kernel, it is expected to be up to 64 bits.
> 
> This patch introduces a new mask field in virtqueue's struct to
> filter our the upper 4 bytes of the address only when necessary.
> An optimization is introduced for 64 bits builds to remove the
> masking, as the address is always 64 bits wide.
> 
> Fixes: ba55c94a7ebc ("net/virtio: revert forcing IOVA as VA mode for virtio-user")
> Cc: stable@dpdk.org
> 
> Reported-by: Sampath Peechu <speechu@cisco.com>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>   drivers/net/virtio/virtqueue.c |  2 ++
>   drivers/net/virtio/virtqueue.h | 18 ++++++++++++++----
>   2 files changed, 16 insertions(+), 4 deletions(-)
> 

Applied to nex-virtio/for-next-net.

Thanks,
Maxime