[PATCH v2] hw/virtio/vhost-shadow-virtqueue: range boundary in translation

Connor Kite posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260817233147.2867623-1-connorkite@gmail.com
Maintainers: "Eugenio Pérez" <eperezma@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>
hw/virtio/vhost-shadow-virtqueue.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[PATCH v2] hw/virtio/vhost-shadow-virtqueue: range boundary in translation
Posted by Connor Kite 1 month, 1 week ago
iova-tree expects inclusive range sizing when maps are allocated or searched.
Currently, svqs use exclusive sizing when searching their
vhost-iova-tree for a match to the region to be translated.  This could
lead to errors if the region to be translated is at the edge of an iova
region.

Fix this by reducing `needle.size` by 1 in
vhost_svq_translate_addr to bring it in line with DMAMap and iova-tree
convention.

The only current user of svq->iova_tree is hw/virtio/vhost-vdpa.c, which
also treats size as inclusive when using the tree API.  In progress work
on vhost-user isolation mode in hw/virtio.vhost-vdpa.c also uses
inclusive sizing when interacting with the tree.

This patch is an updated version of one from the patch series "vhost-user:
isolated memory".  The patch has been isolated from the series as it
is not tightly coupled with the rest of the series.

Signed-off-by: Connor Kite <connorkite@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
V2 Differences:
- Context added about users of svq->iova_tree and their adoption of
  inclusive sizing

 hw/virtio/vhost-shadow-virtqueue.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index bcb7f2ffc7..c8831d52be 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -99,19 +99,25 @@ static bool vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
         const DMAMap *map;
         DMAMap needle;
 
+        if (unlikely(iovec[i].iov_len == 0)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                          "Zero-sized buffer made available by guest");
+            return false;
+        }
+
         /* Check if the descriptor is backed by guest memory  */
         if (gpas) {
             /* Search the GPA->IOVA tree */
             needle = (DMAMap) {
                 .translated_addr = gpas[i],
-                .size = iovec[i].iov_len,
+                .size = iovec[i].iov_len - 1,  /* Inclusive */
             };
             map = vhost_iova_tree_find_gpa(svq->iova_tree, &needle);
         } else {
             /* Search the IOVA->HVA tree */
             needle = (DMAMap) {
                 .translated_addr = (hwaddr)(uintptr_t)iovec[i].iov_base,
-                .size = iovec[i].iov_len,
+                .size = iovec[i].iov_len - 1, /* Inclusive */
             };
             map = vhost_iova_tree_find_iova(svq->iova_tree, &needle);
         }
-- 
2.43.0
Re: [PATCH v2] hw/virtio/vhost-shadow-virtqueue: range boundary in translation
Posted by Eugenio Perez Martin 1 month ago
On Tue, Aug 18, 2026 at 1:32 AM Connor Kite <connorkite@gmail.com> wrote:
>
> iova-tree expects inclusive range sizing when maps are allocated or searched.
> Currently, svqs use exclusive sizing when searching their
> vhost-iova-tree for a match to the region to be translated.  This could
> lead to errors if the region to be translated is at the edge of an iova
> region.
>
> Fix this by reducing `needle.size` by 1 in
> vhost_svq_translate_addr to bring it in line with DMAMap and iova-tree
> convention.
>
> The only current user of svq->iova_tree is hw/virtio/vhost-vdpa.c, which
> also treats size as inclusive when using the tree API.  In progress work
> on vhost-user isolation mode in hw/virtio.vhost-vdpa.c also uses
> inclusive sizing when interacting with the tree.
>
> This patch is an updated version of one from the patch series "vhost-user:
> isolated memory".  The patch has been isolated from the series as it
> is not tightly coupled with the rest of the series.
>
> Signed-off-by: Connor Kite <connorkite@gmail.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

I'd like to improve the patch subject by adding a verb and the object.
Something like (*add* range boundary in *vhost svq* translation).

Also, adding the Fixes tag,

Reviewed-by: Eugenio Pérez <eperezma@redhat.com>

Thank you very much for the catch!

> ---
> V2 Differences:
> - Context added about users of svq->iova_tree and their adoption of
>   inclusive sizing
>
>  hw/virtio/vhost-shadow-virtqueue.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index bcb7f2ffc7..c8831d52be 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -99,19 +99,25 @@ static bool vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
>          const DMAMap *map;
>          DMAMap needle;
>
> +        if (unlikely(iovec[i].iov_len == 0)) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "Zero-sized buffer made available by guest");
> +            return false;
> +        }
> +
>          /* Check if the descriptor is backed by guest memory  */
>          if (gpas) {
>              /* Search the GPA->IOVA tree */
>              needle = (DMAMap) {
>                  .translated_addr = gpas[i],
> -                .size = iovec[i].iov_len,
> +                .size = iovec[i].iov_len - 1,  /* Inclusive */
>              };
>              map = vhost_iova_tree_find_gpa(svq->iova_tree, &needle);
>          } else {
>              /* Search the IOVA->HVA tree */
>              needle = (DMAMap) {
>                  .translated_addr = (hwaddr)(uintptr_t)iovec[i].iov_base,
> -                .size = iovec[i].iov_len,
> +                .size = iovec[i].iov_len - 1, /* Inclusive */
>              };
>              map = vhost_iova_tree_find_iova(svq->iova_tree, &needle);
>          }
> --
> 2.43.0
>
Re: [PATCH v2] hw/virtio/vhost-shadow-virtqueue: range boundary in translation
Posted by Stefano Garzarella 1 month, 1 week ago
On Mon, Aug 17, 2026 at 04:31:47PM -0700, Connor Kite wrote:
>iova-tree expects inclusive range sizing when maps are allocated or searched.
>Currently, svqs use exclusive sizing when searching their
>vhost-iova-tree for a match to the region to be translated.  This could
>lead to errors if the region to be translated is at the edge of an iova
>region.
>
>Fix this by reducing `needle.size` by 1 in
>vhost_svq_translate_addr to bring it in line with DMAMap and iova-tree
>convention.
>
>The only current user of svq->iova_tree is hw/virtio/vhost-vdpa.c, which
>also treats size as inclusive when using the tree API.  In progress work
>on vhost-user isolation mode in hw/virtio.vhost-vdpa.c also uses
>inclusive sizing when interacting with the tree.
>
>This patch is an updated version of one from the patch series "vhost-user:
>isolated memory".  The patch has been isolated from the series as it
>is not tightly coupled with the rest of the series.
>
>Signed-off-by: Connor Kite <connorkite@gmail.com>
>Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>---
>V2 Differences:
>- Context added about users of svq->iova_tree and their adoption of
>  inclusive sizing
>
> hw/virtio/vhost-shadow-virtqueue.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
>diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
>index bcb7f2ffc7..c8831d52be 100644
>--- a/hw/virtio/vhost-shadow-virtqueue.c
>+++ b/hw/virtio/vhost-shadow-virtqueue.c
>@@ -99,19 +99,25 @@ static bool vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
>         const DMAMap *map;
>         DMAMap needle;
>
>+        if (unlikely(iovec[i].iov_len == 0)) {
>+            qemu_log_mask(LOG_GUEST_ERROR,
>+                          "Zero-sized buffer made available by guest");
>+            return false;
>+        }
>+
>         /* Check if the descriptor is backed by guest memory  */
>         if (gpas) {
>             /* Search the GPA->IOVA tree */
>             needle = (DMAMap) {
>                 .translated_addr = gpas[i],
>-                .size = iovec[i].iov_len,
>+                .size = iovec[i].iov_len - 1,  /* Inclusive */
                                                ^
nit: double spaces here...

>             };
>             map = vhost_iova_tree_find_gpa(svq->iova_tree, &needle);
>         } else {
>             /* Search the IOVA->HVA tree */
>             needle = (DMAMap) {
>                 .translated_addr = (hwaddr)(uintptr_t)iovec[i].iov_base,
>-                .size = iovec[i].iov_len,
>+                .size = iovec[i].iov_len - 1, /* Inclusive */
                                                ^
... single here.
If you need to respin, maybe we can be consistent and fix it.

Anyway with Akihiko's comments fixed:

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks for the fix!
Stefano
Re: [PATCH v2] hw/virtio/vhost-shadow-virtqueue: range boundary in translation
Posted by Akihiko Odaki 1 month, 1 week ago
On 2026/08/18 8:31, Connor Kite wrote:
> iova-tree expects inclusive range sizing when maps are allocated or searched.
> Currently, svqs use exclusive sizing when searching their
> vhost-iova-tree for a match to the region to be translated.  This could
> lead to errors if the region to be translated is at the edge of an iova
> region.
> 
> Fix this by reducing `needle.size` by 1 in
> vhost_svq_translate_addr to bring it in line with DMAMap and iova-tree
> convention.
> 
> The only current user of svq->iova_tree is hw/virtio/vhost-vdpa.c, which
> also treats size as inclusive when using the tree API.  In progress work
> on vhost-user isolation mode in hw/virtio.vhost-vdpa.c also uses
> inclusive sizing when interacting with the tree.

Please correct hw/virtio.vhost-vdpa.c to hw/virtio/vhost-user.c. The 
former does not exist and misidentifies where the vhost-user isolation 
work lives.

> 
> This patch is an updated version of one from the patch series "vhost-user:
> isolated memory".  The patch has been isolated from the series as it
> is not tightly coupled with the rest of the series.
> 
> Signed-off-by: Connor Kite <connorkite@gmail.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Please add the required trailer:

     Fixes: 34e3c94edaef ("vdpa: Add custom IOTLB translations to SVQ").

That commit introduced the oversized needle, and QEMU policy requests a
Fixes: trailer for in-tree regressions.

Regards,
Akihiko Odaki

> ---
> V2 Differences:
> - Context added about users of svq->iova_tree and their adoption of
>    inclusive sizing
> 
>   hw/virtio/vhost-shadow-virtqueue.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index bcb7f2ffc7..c8831d52be 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -99,19 +99,25 @@ static bool vhost_svq_translate_addr(const VhostShadowVirtqueue *svq,
>           const DMAMap *map;
>           DMAMap needle;
>   
> +        if (unlikely(iovec[i].iov_len == 0)) {
> +            qemu_log_mask(LOG_GUEST_ERROR,
> +                          "Zero-sized buffer made available by guest");
> +            return false;
> +        }
> +
>           /* Check if the descriptor is backed by guest memory  */
>           if (gpas) {
>               /* Search the GPA->IOVA tree */
>               needle = (DMAMap) {
>                   .translated_addr = gpas[i],
> -                .size = iovec[i].iov_len,
> +                .size = iovec[i].iov_len - 1,  /* Inclusive */
>               };
>               map = vhost_iova_tree_find_gpa(svq->iova_tree, &needle);
>           } else {
>               /* Search the IOVA->HVA tree */
>               needle = (DMAMap) {
>                   .translated_addr = (hwaddr)(uintptr_t)iovec[i].iov_base,
> -                .size = iovec[i].iov_len,
> +                .size = iovec[i].iov_len - 1, /* Inclusive */
>               };
>               map = vhost_iova_tree_find_iova(svq->iova_tree, &needle);
>           }