On 28.02.24 16:06, Philippe Mathieu-Daudé wrote:
Hi Heinrich,

On 28/2/24 13:59, Heinrich Schuchardt wrote:
virtqueue_map_desc() is called with values of sz exceeding that may exceed
TARGET_PAGE_SIZE. sz = 0x2800 has been observed.

We only support a single bounce buffer. We have to avoid
virtqueue_map_desc() calling address_space_map() multiple times. Otherwise
we see an error

     qemu: virtio: bogus descriptor or out of resources

Increase the minimum size of the bounce buffer to 0x10000 which matches
the largest value of TARGET_PAGE_SIZE for all architectures.

Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
---
v2:
    remove unrelated change
---
  system/physmem.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index e3ebc19eef..3c82da1c86 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -3151,8 +3151,12 @@ void *address_space_map(AddressSpace *as,
              *plen = 0;
              return NULL;
          }
-        /* Avoid unbounded allocations */
-        l = MIN(l, TARGET_PAGE_SIZE);
+        /*
+         * There is only one bounce buffer. The largest occuring value of
+         * parameter sz of virtqueue_map_desc() must fit into the bounce
+         * buffer.
+         */
+        l = MIN(l, 0x10000);

Please define this magic value. Maybe ANY_TARGET_PAGE_SIZE or
TARGETS_BIGGEST_PAGE_SIZE?

Then along:
   QEMU_BUILD_BUG_ON(TARGET_PAGE_SIZE <= TARGETS_BIGGEST_PAGE_SIZE);

Thank you Philippe for reviewing.

TARGETS_BIGGEST_PAGE_SIZE does not fit as the value is not driven by the page size.
How about MIN_BOUNCE_BUFFER_SIZE?
Is include/exec/memory.h the right include for the constant?

I don't think that TARGET_PAGE_SIZE has any relevance for setting the bounce buffer size. I only mentioned it to say that we are not decreasing the value on any existing architecture.

I don't know why TARGET_PAGE_SIZE ever got into this piece of code. e3127ae0cdcd ("exec: reorganize address_space_map") does not provide a reason for this choice. Maybe Paolo remembers.

Best regards

Heinrich


          bounce.buffer = qemu_memalign(TARGET_PAGE_SIZE, l);
          bounce.addr = addr;
          bounce.len = l;



Reply via email to