1. Patchew
  2. QEMU
  3. View series

[PATCH] virtio-iommu: fix OOM due to unbounded call_rcu

Michael S. Tsirkin posted 1 patch 1 day, 18 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/eb46ab360dbe28c29cfa78812a7440dcb7444d59.1784807826.git.mst@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Eric Auger <eric.auger@redhat.com>
include/hw/virtio/virtio-iommu.h |  1 +
hw/virtio/virtio-iommu.c         | 29 +++++++++++++++++++++++++++++
3 files changed, 35 insertions(+), 1 deletion(-)
[PATCH] virtio-iommu: fix OOM due to unbounded call_rcu
Posted by Michael S. Tsirkin 1 day, 18 hours ago 
Currently, within virtio-iommu, handle_command processes the command vq
without any limits on the number of entries processed.
This can easily and repeatedly enable/disable multiple memory regions.
Within the memory code, this causes an accumulation of an
unbounded number of RCU-deferred FlatViews - each of these
is supposed to be freed with call_rcu, but that never happens
because the main thread never returns to the main loop.

Given FlatView is big, it's easy to have this balloon out to multiple
Gigabytes of memory.

Limit the loop defer any remaining work to a timer.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio-iommu.h |  1 +
 hw/virtio/virtio-iommu.c         | 29 +++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index 3b86050f2c..1f265540ad 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -65,6 +65,7 @@ struct VirtIOIOMMU {
     GTree *domains;
     QemuRecMutex mutex;
     GTree *endpoints;
+    QEMUTimer *cmd_timer;
     bool boot_bypass;
     Notifier machine_done;
     bool granule_frozen;
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 08f7e8b783..533bd5073f 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
     return ret ? ret : virtio_iommu_probe(s, &req, buf);
 }
 
+static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq);
+
+static void virtio_iommu_handle_command_timer(void *opaque)
+{
+    VirtIOIOMMU *s = opaque;
+    VirtIODevice *vdev = VIRTIO_DEVICE(s);
+
+    if (virtio_device_started(vdev, vdev->status) && !vdev->broken) {
+        virtio_iommu_handle_command(vdev, s->req_vq);
+    }
+}
+
 static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
@@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
     struct iovec *iov;
     void *buf = NULL;
     size_t sz;
+    unsigned int batch = 0;
 
     for (;;) {
         size_t output_size = sizeof(tail);
 
+        if (++batch > virtio_queue_get_num(vdev, virtio_get_queue_index(vq))) {
+            timer_mod(s->cmd_timer,
+                      qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
+            break;
+        }
+
         elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
         if (!elem) {
             return;
@@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
     s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
                              virtio_iommu_handle_command);
     s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
+    s->cmd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
+                                virtio_iommu_handle_command_timer, s);
 
     /*
      * config.bypass is needed to get initial address space early, such as
@@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
 
     qemu_rec_mutex_destroy(&s->mutex);
 
+    timer_free(s->cmd_timer);
     virtio_delete_queue(s->req_vq);
     virtio_delete_queue(s->event_vq);
     virtio_cleanup(vdev);
@@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)
 
     trace_virtio_iommu_device_reset_exit();
 
+    timer_del(s->cmd_timer);
+
     if (s->domains) {
         g_tree_destroy(s->domains);
     }
@@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)
      * still correct.
      */
     virtio_iommu_switch_address_space_all(s);
+
+    if (virtio_device_started(VIRTIO_DEVICE(s), VIRTIO_DEVICE(s)->status)) {
+        timer_mod(s->cmd_timer,
+                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
+    }
     return 0;
 }
 
-- 
MST
Re: [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu
Posted by Eric Auger 1 day, 13 hours ago 
Hi Michael,

On 7/23/26 1:58 PM, Michael S. Tsirkin wrote:
> Currently, within virtio-iommu, handle_command processes the command vq
> without any limits on the number of entries processed.
> This can easily and repeatedly enable/disable multiple memory regions.
> Within the memory code, this causes an accumulation of an
> unbounded number of RCU-deferred FlatViews - each of these
> is supposed to be freed with call_rcu, but that never happens
> because the main thread never returns to the main loop.
>
> Given FlatView is big, it's easy to have this balloon out to multiple
> Gigabytes of memory.
>
> Limit the loop defer any remaining work to a timer.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>

test migration, reset and obviously normal execution with lowered threshold.

Thanks!

Eric
> ---
>  include/hw/virtio/virtio-iommu.h |  1 +
>  hw/virtio/virtio-iommu.c         | 29 +++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index 3b86050f2c..1f265540ad 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -65,6 +65,7 @@ struct VirtIOIOMMU {
>      GTree *domains;
>      QemuRecMutex mutex;
>      GTree *endpoints;
> +    QEMUTimer *cmd_timer;
>      bool boot_bypass;
>      Notifier machine_done;
>      bool granule_frozen;
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 08f7e8b783..533bd5073f 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
>      return ret ? ret : virtio_iommu_probe(s, &req, buf);
>  }
>  
> +static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq);
> +
> +static void virtio_iommu_handle_command_timer(void *opaque)
> +{
> +    VirtIOIOMMU *s = opaque;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +
> +    if (virtio_device_started(vdev, vdev->status) && !vdev->broken) {
> +        virtio_iommu_handle_command(vdev, s->req_vq);
> +    }
> +}
> +
>  static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>  {
>      VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
> @@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>      struct iovec *iov;
>      void *buf = NULL;
>      size_t sz;
> +    unsigned int batch = 0;
>  
>      for (;;) {
>          size_t output_size = sizeof(tail);
>  
> +        if (++batch > virtio_queue_get_num(vdev, virtio_get_queue_index(vq))) {
> +            timer_mod(s->cmd_timer,
> +                      qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> +            break;
> +        }
> +
>          elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
>          if (!elem) {
>              return;
> @@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>      s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
>                               virtio_iommu_handle_command);
>      s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
> +    s->cmd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
> +                                virtio_iommu_handle_command_timer, s);
>  
>      /*
>       * config.bypass is needed to get initial address space early, such as
> @@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
>  
>      qemu_rec_mutex_destroy(&s->mutex);
>  
> +    timer_free(s->cmd_timer);
>      virtio_delete_queue(s->req_vq);
>      virtio_delete_queue(s->event_vq);
>      virtio_cleanup(vdev);
> @@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)
>  
>      trace_virtio_iommu_device_reset_exit();
>  
> +    timer_del(s->cmd_timer);
> +
>      if (s->domains) {
>          g_tree_destroy(s->domains);
>      }
> @@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)
>       * still correct.
>       */
>      virtio_iommu_switch_address_space_all(s);
> +
> +    if (virtio_device_started(VIRTIO_DEVICE(s), VIRTIO_DEVICE(s)->status)) {
> +        timer_mod(s->cmd_timer,
> +                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> +    }
>      return 0;
>  }
>
Re: [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu
Posted by Eric Auger 1 day, 14 hours ago 
Hi Michael,

On 7/23/26 1:58 PM, Michael S. Tsirkin wrote:
> Currently, within virtio-iommu, handle_command processes the command vq
> without any limits on the number of entries processed.
> This can easily and repeatedly enable/disable multiple memory regions.
> Within the memory code, this causes an accumulation of an
> unbounded number of RCU-deferred FlatViews - each of these
> is supposed to be freed with call_rcu, but that never happens
> because the main thread never returns to the main loop.
>
> Given FlatView is big, it's easy to have this balloon out to multiple
> Gigabytes of memory.
>
> Limit the loop defer any remaining work to a timer.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  include/hw/virtio/virtio-iommu.h |  1 +
>  hw/virtio/virtio-iommu.c         | 29 +++++++++++++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index 3b86050f2c..1f265540ad 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -65,6 +65,7 @@ struct VirtIOIOMMU {
>      GTree *domains;
>      QemuRecMutex mutex;
>      GTree *endpoints;
> +    QEMUTimer *cmd_timer;
>      bool boot_bypass;
>      Notifier machine_done;
>      bool granule_frozen;
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 08f7e8b783..533bd5073f 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
>      return ret ? ret : virtio_iommu_probe(s, &req, buf);
>  }
>  
> +static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq);
> +
> +static void virtio_iommu_handle_command_timer(void *opaque)
> +{
> +    VirtIOIOMMU *s = opaque;
> +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
> +
> +    if (virtio_device_started(vdev, vdev->status) && !vdev->broken) {
> +        virtio_iommu_handle_command(vdev, s->req_vq);
So what does happen in terms of scheduling. Can't this run concurrently
with the usual direct virtio_iommu_handle_command() path? 
> +    }
> +}
> +
>  static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>  {
>      VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
> @@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>      struct iovec *iov;
>      void *buf = NULL;
>      size_t sz;
> +    unsigned int batch = 0;
>  
>      for (;;) {
>          size_t output_size = sizeof(tail);
>  
> +        if (++batch > virtio_queue_get_num(vdev, virtio_get_queue_index(vq))) {
how did you chose that condition? is it always safe wrt oom situation?
> +            timer_mod(s->cmd_timer,
> +                      qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> +            break;
> +        }
> +
>          elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
>          if (!elem) {
>              return;
> @@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>      s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
>                               virtio_iommu_handle_command);
>      s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
> +    s->cmd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
> +                                virtio_iommu_handle_command_timer, s);
>  
>      /*
>       * config.bypass is needed to get initial address space early, such as
> @@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
>  
>      qemu_rec_mutex_destroy(&s->mutex);
>  
> +    timer_free(s->cmd_timer);
>      virtio_delete_queue(s->req_vq);
>      virtio_delete_queue(s->event_vq);
>      virtio_cleanup(vdev);
> @@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)
>  
>      trace_virtio_iommu_device_reset_exit();
>  
> +    timer_del(s->cmd_timer);
> +
>      if (s->domains) {
>          g_tree_destroy(s->domains);
>      }
> @@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)
>       * still correct.
>       */
>      virtio_iommu_switch_address_space_all(s);
> +
> +    if (virtio_device_started(VIRTIO_DEVICE(s), VIRTIO_DEVICE(s)->status)) {
> +        timer_mod(s->cmd_timer,
> +                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> +    }
>      return 0;
>  }
>  
Thanks

Eric
Re: [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu
Posted by Michael S. Tsirkin 1 day, 14 hours ago 
On Thu, Jul 23, 2026 at 05:42:02PM +0200, Eric Auger wrote:
> Hi Michael,
> 
> On 7/23/26 1:58 PM, Michael S. Tsirkin wrote:
> > Currently, within virtio-iommu, handle_command processes the command vq
> > without any limits on the number of entries processed.
> > This can easily and repeatedly enable/disable multiple memory regions.
> > Within the memory code, this causes an accumulation of an
> > unbounded number of RCU-deferred FlatViews - each of these
> > is supposed to be freed with call_rcu, but that never happens
> > because the main thread never returns to the main loop.
> >
> > Given FlatView is big, it's easy to have this balloon out to multiple
> > Gigabytes of memory.
> >
> > Limit the loop defer any remaining work to a timer.
> >
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
> > Cc: Eric Auger <eric.auger@redhat.com>
> > Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  include/hw/virtio/virtio-iommu.h |  1 +
> >  hw/virtio/virtio-iommu.c         | 29 +++++++++++++++++++++++++++++
> >  3 files changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> > index 3b86050f2c..1f265540ad 100644
> > --- a/include/hw/virtio/virtio-iommu.h
> > +++ b/include/hw/virtio/virtio-iommu.h
> > @@ -65,6 +65,7 @@ struct VirtIOIOMMU {
> >      GTree *domains;
> >      QemuRecMutex mutex;
> >      GTree *endpoints;
> > +    QEMUTimer *cmd_timer;
> >      bool boot_bypass;
> >      Notifier machine_done;
> >      bool granule_frozen;
> > diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> > index 08f7e8b783..533bd5073f 100644
> > --- a/hw/virtio/virtio-iommu.c
> > +++ b/hw/virtio/virtio-iommu.c
> > @@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
> >      return ret ? ret : virtio_iommu_probe(s, &req, buf);
> >  }
> >  
> > +static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq);
> > +
> > +static void virtio_iommu_handle_command_timer(void *opaque)
> > +{
> > +    VirtIOIOMMU *s = opaque;
> > +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
> > +
> > +    if (virtio_device_started(vdev, vdev->status) && !vdev->broken) {
> > +        virtio_iommu_handle_command(vdev, s->req_vq);
> So what does happen in terms of scheduling. Can't this run concurrently
> with the usual direct virtio_iommu_handle_command() path? 

BQL prevents that.


> > +    }
> > +}
> > +
> >  static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
> >  {
> >      VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
> > @@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
> >      struct iovec *iov;
> >      void *buf = NULL;
> >      size_t sz;
> > +    unsigned int batch = 0;
> >  
> >      for (;;) {
> >          size_t output_size = sizeof(tail);
> >  
> > +        if (++batch > virtio_queue_get_num(vdev, virtio_get_queue_index(vq))) {
> how did you chose that condition? is it always safe wrt oom situation?

It's kinda random, but the idea is that queue size already gives us
a point on the memory/speed curve.

There's nothing that is "safe wrt oom" in all configs - qemu makes
no promises and even a single allocation can in theory push you
into oom if memory is tight.

But what this prevents is unbounded memory allocations which will trigger
oom at some point for sure.



> > +            timer_mod(s->cmd_timer,
> > +                      qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> > +            break;
> > +        }
> > +
> >          elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> >          if (!elem) {
> >              return;
> > @@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> >      s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
> >                               virtio_iommu_handle_command);
> >      s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
> > +    s->cmd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
> > +                                virtio_iommu_handle_command_timer, s);
> >  
> >      /*
> >       * config.bypass is needed to get initial address space early, such as
> > @@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
> >  
> >      qemu_rec_mutex_destroy(&s->mutex);
> >  
> > +    timer_free(s->cmd_timer);
> >      virtio_delete_queue(s->req_vq);
> >      virtio_delete_queue(s->event_vq);
> >      virtio_cleanup(vdev);
> > @@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)
> >  
> >      trace_virtio_iommu_device_reset_exit();
> >  
> > +    timer_del(s->cmd_timer);
> > +
> >      if (s->domains) {
> >          g_tree_destroy(s->domains);
> >      }
> > @@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)
> >       * still correct.
> >       */
> >      virtio_iommu_switch_address_space_all(s);
> > +
> > +    if (virtio_device_started(VIRTIO_DEVICE(s), VIRTIO_DEVICE(s)->status)) {
> > +        timer_mod(s->cmd_timer,
> > +                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
> > +    }
> >      return 0;
> >  }
> >  
> Thanks
> 
> Eric
Re: [PATCH] virtio-iommu: fix OOM due to unbounded call_rcu
Posted by Eric Auger 1 day, 13 hours ago 

On 7/23/26 6:01 PM, Michael S. Tsirkin wrote:
> On Thu, Jul 23, 2026 at 05:42:02PM +0200, Eric Auger wrote:
>> Hi Michael,
>>
>> On 7/23/26 1:58 PM, Michael S. Tsirkin wrote:
>>> Currently, within virtio-iommu, handle_command processes the command vq
>>> without any limits on the number of entries processed.
>>> This can easily and repeatedly enable/disable multiple memory regions.
>>> Within the memory code, this causes an accumulation of an
>>> unbounded number of RCU-deferred FlatViews - each of these
>>> is supposed to be freed with call_rcu, but that never happens
>>> because the main thread never returns to the main loop.
>>>
>>> Given FlatView is big, it's easy to have this balloon out to multiple
>>> Gigabytes of memory.
>>>
>>> Limit the loop defer any remaining work to a timer.
>>>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3930
>>> Cc: Eric Auger <eric.auger@redhat.com>
>>> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>  include/hw/virtio/virtio-iommu.h |  1 +
>>>  hw/virtio/virtio-iommu.c         | 29 +++++++++++++++++++++++++++++
>>>  3 files changed, 35 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
>>> index 3b86050f2c..1f265540ad 100644
>>> --- a/include/hw/virtio/virtio-iommu.h
>>> +++ b/include/hw/virtio/virtio-iommu.h
>>> @@ -65,6 +65,7 @@ struct VirtIOIOMMU {
>>>      GTree *domains;
>>>      QemuRecMutex mutex;
>>>      GTree *endpoints;
>>> +    QEMUTimer *cmd_timer;
>>>      bool boot_bypass;
>>>      Notifier machine_done;
>>>      bool granule_frozen;
>>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>>> index 08f7e8b783..533bd5073f 100644
>>> --- a/hw/virtio/virtio-iommu.c
>>> +++ b/hw/virtio/virtio-iommu.c
>>> @@ -993,6 +993,18 @@ static int virtio_iommu_handle_probe(VirtIOIOMMU *s,
>>>      return ret ? ret : virtio_iommu_probe(s, &req, buf);
>>>  }
>>>  
>>> +static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq);
>>> +
>>> +static void virtio_iommu_handle_command_timer(void *opaque)
>>> +{
>>> +    VirtIOIOMMU *s = opaque;
>>> +    VirtIODevice *vdev = VIRTIO_DEVICE(s);
>>> +
>>> +    if (virtio_device_started(vdev, vdev->status) && !vdev->broken) {
>>> +        virtio_iommu_handle_command(vdev, s->req_vq);
>> So what does happen in terms of scheduling. Can't this run concurrently
>> with the usual direct virtio_iommu_handle_command() path? 
> BQL prevents that.
OK
>
>
>>> +    }
>>> +}
>>> +
>>>  static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>>>  {
>>>      VirtIOIOMMU *s = VIRTIO_IOMMU(vdev);
>>> @@ -1003,10 +1015,17 @@ static void virtio_iommu_handle_command(VirtIODevice *vdev, VirtQueue *vq)
>>>      struct iovec *iov;
>>>      void *buf = NULL;
>>>      size_t sz;
>>> +    unsigned int batch = 0;
>>>  
>>>      for (;;) {
>>>          size_t output_size = sizeof(tail);
>>>  
>>> +        if (++batch > virtio_queue_get_num(vdev, virtio_get_queue_index(vq))) {
>> how did you chose that condition? is it always safe wrt oom situation?
> It's kinda random, but the idea is that queue size already gives us
> a point on the memory/speed curve.
>
> There's nothing that is "safe wrt oom" in all configs - qemu makes
> no promises and even a single allocation can in theory push you
> into oom if memory is tight.
>
> But what this prevents is unbounded memory allocations which will trigger
> oom at some point for sure.
OK thanks for the explanation

Eric
>
>
>
>>> +            timer_mod(s->cmd_timer,
>>> +                      qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
>>> +            break;
>>> +        }
>>> +
>>>          elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
>>>          if (!elem) {
>>>              return;
>>> @@ -1416,6 +1435,8 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>>>      s->req_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE,
>>>                               virtio_iommu_handle_command);
>>>      s->event_vq = virtio_add_queue(vdev, VIOMMU_DEFAULT_QUEUE_SIZE, NULL);
>>> +    s->cmd_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
>>> +                                virtio_iommu_handle_command_timer, s);
>>>  
>>>      /*
>>>       * config.bypass is needed to get initial address space early, such as
>>> @@ -1498,6 +1519,7 @@ static void virtio_iommu_device_unrealize(DeviceState *dev)
>>>  
>>>      qemu_rec_mutex_destroy(&s->mutex);
>>>  
>>> +    timer_free(s->cmd_timer);
>>>      virtio_delete_queue(s->req_vq);
>>>      virtio_delete_queue(s->event_vq);
>>>      virtio_cleanup(vdev);
>>> @@ -1509,6 +1531,8 @@ static void virtio_iommu_device_reset_exit(Object *obj, ResetType type)
>>>  
>>>      trace_virtio_iommu_device_reset_exit();
>>>  
>>> +    timer_del(s->cmd_timer);
>>> +
>>>      if (s->domains) {
>>>          g_tree_destroy(s->domains);
>>>      }
>>> @@ -1628,6 +1652,11 @@ static int iommu_post_load(void *opaque, int version_id)
>>>       * still correct.
>>>       */
>>>      virtio_iommu_switch_address_space_all(s);
>>> +
>>> +    if (virtio_device_started(VIRTIO_DEVICE(s), VIRTIO_DEVICE(s)->status)) {
>>> +        timer_mod(s->cmd_timer,
>>> +                  qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL_RT) + 1);
>>> +    }
>>>      return 0;
>>>  }
>>>  
>> Thanks
>>
>> Eric

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.