[PATCH] hw/virtio/vhost: simplify ring mapping verification with loop

Bin Guo posted 1 patch 1 week, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260403091621.88914-1-guobin@linux.alibaba.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>
hw/virtio/vhost.c | 34 ++++++++++++----------------------
1 file changed, 12 insertions(+), 22 deletions(-)
[PATCH] hw/virtio/vhost: simplify ring mapping verification with loop
Posted by Bin Guo 1 week, 1 day ago
Replace the three nearly-identical vhost_verify_ring_part_mapping() calls
with a simple loop over arrays. This eliminates code duplication while
maintaining identical behavior and error reporting.

Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
---
 hw/virtio/vhost.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index b9dc4ed13b..7b5bfc86e6 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -525,31 +525,21 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
             continue;
         }
 
-        j = 0;
-        r = vhost_verify_ring_part_mapping(
-                vq->desc, vq->desc_phys, vq->desc_size,
-                reg_hva, reg_gpa, reg_size);
-        if (r) {
-            break;
-        }
-
-        j++;
-        r = vhost_verify_ring_part_mapping(
-                vq->avail, vq->avail_phys, vq->avail_size,
-                reg_hva, reg_gpa, reg_size);
-        if (r) {
-            break;
-        }
-
-        j++;
-        r = vhost_verify_ring_part_mapping(
-                vq->used, vq->used_phys, vq->used_size,
-                reg_hva, reg_gpa, reg_size);
-        if (r) {
-            break;
+        void *ring_addrs[] = {vq->desc, vq->avail, vq->used};
+        uint64_t ring_phys[] = {vq->desc_phys, vq->avail_phys, vq->used_phys};
+        uint64_t ring_sizes[] = {vq->desc_size, vq->avail_size, vq->used_size};
+
+        for (j = 0; j < 3; j++) {
+            r = vhost_verify_ring_part_mapping(
+                    ring_addrs[j], ring_phys[j], ring_sizes[j],
+                    reg_hva, reg_gpa, reg_size);
+            if (r) {
+                goto out;
+            }
         }
     }
 
+out:
     if (r == -ENOMEM) {
         error_report("Unable to map %s for ring %d", part_name[j], i);
     } else if (r == -EBUSY) {
-- 
2.50.1 (Apple Git-155)
Re: [PATCH] hw/virtio/vhost: simplify ring mapping verification with loop
Posted by Michael S. Tsirkin 1 week, 1 day ago
On Fri, Apr 03, 2026 at 05:16:21PM +0800, Bin Guo wrote:
> Replace the three nearly-identical vhost_verify_ring_part_mapping() calls
> with a simple loop over arrays. This eliminates code duplication while
> maintaining identical behavior and error reporting.
> 
> Signed-off-by: Bin Guo <guobin@linux.alibaba.com>
> ---
>  hw/virtio/vhost.c | 34 ++++++++++++----------------------
>  1 file changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index b9dc4ed13b..7b5bfc86e6 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -525,31 +525,21 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
>              continue;
>          }
>  
> -        j = 0;
> -        r = vhost_verify_ring_part_mapping(
> -                vq->desc, vq->desc_phys, vq->desc_size,
> -                reg_hva, reg_gpa, reg_size);
> -        if (r) {
> -            break;
> -        }
> -
> -        j++;
> -        r = vhost_verify_ring_part_mapping(
> -                vq->avail, vq->avail_phys, vq->avail_size,
> -                reg_hva, reg_gpa, reg_size);
> -        if (r) {
> -            break;
> -        }
> -
> -        j++;
> -        r = vhost_verify_ring_part_mapping(
> -                vq->used, vq->used_phys, vq->used_size,
> -                reg_hva, reg_gpa, reg_size);
> -        if (r) {
> -            break;
> +        void *ring_addrs[] = {vq->desc, vq->avail, vq->used};
> +        uint64_t ring_phys[] = {vq->desc_phys, vq->avail_phys, vq->used_phys};
> +        uint64_t ring_sizes[] = {vq->desc_size, vq->avail_size, vq->used_size};
> +
> +        for (j = 0; j < 3; j++) {
> +            r = vhost_verify_ring_part_mapping(
> +                    ring_addrs[j], ring_phys[j], ring_sizes[j],
> +                    reg_hva, reg_gpa, reg_size);
> +            if (r) {
> +                goto out;
> +            }
>          }
>      }
>  
> +out:
>      if (r == -ENOMEM) {
>          error_report("Unable to map %s for ring %d", part_name[j], i);


I don't see the point, sorry. Maybe if it was a single array,
and we used ARRAY_SIZE ... Even then, I am not sure.

As it is, we are replacing straight forward linear code with
tricky arrays.

>      } else if (r == -EBUSY) {
> -- 
> 2.50.1 (Apple Git-155)