On 29.02.24 12:11, Peter Maydell wrote:
On Thu, 29 Feb 2024 at 10:59, Jonathan Cameron
<jonathan.came...@huawei.com> wrote:
On Thu, 29 Feb 2024 09:38:29 +0000
Peter Maydell <peter.mayd...@linaro.org> wrote:
On Wed, 28 Feb 2024 at 19:07, Heinrich Schuchardt
<heinrich.schucha...@canonical.com> wrote:
On 28.02.24 19:39, Peter Maydell wrote:
The limitation to a page dates back to commit 6d16c2f88f2a in 2009,
which was the first implementation of this function. I don't think
there's a particular reason for that value beyond that it was
probably a convenient value that was assumed to be likely "big enough".
I think the idea with this bounce-buffer has always been that this
isn't really a code path we expected to end up in very often --
it's supposed to be for when devices are doing DMA, which they
will typically be doing to memory (backed by host RAM), not
devices (backed by MMIO and needing a bounce buffer). So the
whole mechanism is a bit "last fallback to stop things breaking
entirely".
The address_space_map() API says that it's allowed to return
a subset of the range you ask for, so if the virtio code doesn't
cope with the minimum being set to TARGET_PAGE_SIZE then either
we need to fix that virtio code or we need to change the API
of this function. (But I think you will also get a reduced
range if you try to use it across a boundary between normal
host-memory-backed RAM and a device MemoryRegion.)
If we allow a bounce buffer only to be used once (via the in_use flag),
why do we allow only a single bounce buffer?
Could address_space_map() allocate a new bounce buffer on every call and
address_space_unmap() deallocate it?
Isn't the design with a single bounce buffer bound to fail with a
multi-threaded client as collision can be expected?
Yeah, I don't suppose multi-threaded was particularly expected.
Again, this is really a "handle the case where the guest does
something silly" setup, which is why only one bounce buffer.
Why is your guest ending up in the bounce-buffer path?
Happens for me with emulated CXL memory.
Can we put that in the "something silly" bucket? :-)
But yes, I'm not surprised that CXL runs into this. Heinrich,
are you doing CXL testing, or is this some other workload?
I am running the UEFI Self-Certification Tests (SCT) on EDK 2 using:
qemu-system-riscv64 \
-M virt,acpi=off -accel tcg -m 4096 \
-serial mon:stdio \
-device virtio-gpu-pci \
-device qemu-xhci \
-device usb-kbd \
-drive
if=pflash,format=raw,unit=0,file=RISCV_VIRT_CODE.fd,readonly=on \
-drive if=pflash,format=raw,unit=1,file=RISCV_VIRT_VARS.fd \
-drive file=sct.img,format=raw,if=virtio \
-device virtio-net-device,netdev=net0 \
-netdev user,id=net0
This does not invoke any CXL related stuff.
Best regards
Heinrich
I think the case I saw
was split descriptors in virtio via address space caches
https://elixir.bootlin.com/qemu/latest/source/hw/virtio/virtio.c#L4043
One bounce buffer is in use for the outer loop and another for the descriptors
it is pointing to.
Mmm. The other assumption made in the design of the address_space_map()
API I think was that it was unlikely that a device would be trying
to do two DMA operations simultaneously. This is clearly not
true in practice. We definitely need to fix one end or other of
this API.
(I'm not sure why the bounce-buffer limit ought to be per-AddressSpace:
is that just done in Matthias' series so that we can attach an
x-thingy property to the individual PCI device?)
-- PMM
