Re: [PATCH RFCv1] virtio: Inherit max bounce buffer size from bus parent if possible

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jun 10, 2026 at 11:54:47PM +1000, Gavin Shan wrote:
> Hi Michael and Peter,
> 
> On 6/10/26 11:00 PM, Gavin Shan wrote:
> > On 6/10/26 10:27 PM, Michael S. Tsirkin wrote:
> > > On Wed, Jun 10, 2026 at 10:19:31PM +1000, Gavin Shan wrote:
> > > > On 6/10/26 10:12 PM, Michael S. Tsirkin wrote:
> > > > > On Wed, Jun 10, 2026 at 08:55:10PM +1000, Gavin Shan wrote:
> > > > > > On 6/10/26 7:54 PM, Pavel Hrdina wrote:
> > > > 
> > > > [...]
> > > > 
> > > > > > > 
> > > > > > > You did not answer the question that Daniel was asking, how will user
> > > > > > > know that max-bounce-buffer-size should be used if it's necessary to fix
> > > > > > > guest system hangs and how will user know what magic value should be set?
> > > > > > > 
> > > > > > 
> > > > > > Sorry that I missed to answer Daniel's questions. For this specific case,
> > > > > > user need to enlarge the bounce buffer size when seeing the following error
> > > > > > message. We can add an explicit one in address_space_map() if the existing
> > > > > > error message isn't obvious.
> > > > > > 
> > > > > >     qemu-system-aarch64: virtio: bogus descriptor or out of resources
> > > > > > 
> > > > > >     void *address_space_map(AddressSpace *as,
> > > > > >                           hwaddr addr,
> > > > > >                           hwaddr *plen,
> > > > > >                           bool is_write,
> > > > > >                           MemTxAttrs attrs)
> > > > > >     {
> > > > > >         if (!memory_access_is_direct(mr, is_write, attrs)) {
> > > > > >             if (l == 0) {
> > > > > >                 error_report("Running out of bounce buffer size , enlarge it with max-bounce-buffer-size");
> > > > > >                 *plen = 0;
> > > > > >                 return NULL;
> > > > > >             }
> > > > > >         }
> > > > > > 
> > > > > > As to the value user should take for max-bounce-buffer-size, it is really case by case
> > > > > > and decided by user. User needs to try 4096, 8192, ..., 0xFFFFFFFF to figure out the
> > > > > > smallest value works for them. The worst case is to set 0xFFFFFFFF.
> > > > > > 
> > > > > 
> > > > > 
> > > > > This is not at all reasonable. All kind of fixes are possible but
> > > > > fundamentally, bounce buffering data path is by itself already a
> > > > > bad idea.
> > > > > 
> > > > > I have no idea what does bounce buffering device ram accomplish.
> > > > > 
> > > > > In the end, qemu still simply reads the memory from/to the buffer.
> > > > > 
> > > > > My suggestion is to first of all look for ways to mark the
> > > > > memory as direct.
> > > > > 
> > > > 
> > > > As I explained to Peter Xu in another reply, we can't simply mark the (RAM
> > > > DEVICE) memory region is directly accessible. The memory region is initialized
> > > > by memory_region_init_ram_device_ptr() in hw/vfio/region.c::vfio_region_mmap().
> > > > 
> > > > The  accesses to the memory region is handled by 'ram_device_mem_ops' where
> > > > {ldn, stn}_he_p() are used in its read/write handler. They're different
> > > > from memcpy() since the data endianness is well handled in {ldn, stn}_he_p().
> > > > 
> > > > Thanks,
> > > > Gavin
> > > > 
> > > 
> > > What is endianness set to, for this region?
> > > 
> > 
> > The endianness of the memory region is set to that for the host.
> > 
> > static const MemoryRegionOps ram_device_mem_ops = {
> >      .read = memory_region_ram_device_read,
> >      .write = memory_region_ram_device_write,
> >      .endianness = HOST_BIG_ENDIAN ? DEVICE_BIG_ENDIAN : DEVICE_LITTLE_ENDIAN,
> > };
> > 

So there is never any endianness translation.
I think the reason qemu does the bounce buffer is more
to prevent things like vector access from MMIO.


> How about to treat the RAM DEVICE memory region directly accessible in
> address_space_map() only when HOST_BIG_ENDIAN is false,
> something like
> below and I don't hit the guest hang issue with the changes.
> 
> diff --git a/include/system/memory.h b/include/system/memory.h
> index 1417132f6d..9daca55251 100644
> --- a/include/system/memory.h
> +++ b/include/system/memory.h
> @@ -2908,7 +2908,8 @@ void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t addr);
>  int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr);
>  bool prepare_mmio_access(MemoryRegion *mr);
> -static inline bool memory_region_supports_direct_access(const MemoryRegion *mr)
> +static inline bool memory_region_supports_direct_access(const MemoryRegion *mr,
> +                                                        bool check_ram_device)
>  {
>      /* ROM DEVICE regions only allow direct access if in ROMD mode. */
>      if (memory_region_is_romd(mr)) {
> @@ -2922,13 +2923,14 @@ static inline bool memory_region_supports_direct_access(const MemoryRegion *mr)
>       * be MMIO and access using mempy can be wrong (e.g., using instructions not
>       * intended for MMIO access). So we treat this as IO.
>       */
> -    return !memory_region_is_ram_device(mr);
> +    return (!check_ram_device || !memory_region_is_ram_device(mr));
>  }
>  static inline bool memory_access_is_direct(const MemoryRegion *mr,
> +                                           bool check_ram_device,
>                                             bool is_write, MemTxAttrs attrs)
>  {
> -    if (!memory_region_supports_direct_access(mr)) {
> +    if (!memory_region_supports_direct_access(mr, check_ram_device)) {
>          return false;
>      }
> diff --git a/system/physmem.c b/system/physmem.c
> index 7bcbf87573..2e6b72b124 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -3724,7 +3724,7 @@ void *address_space_map(AddressSpace *as,
>      fv = address_space_to_flatview(as);
>      mr = flatview_translate(fv, addr, &xlat, &l, is_write, attrs);
> -    if (!memory_access_is_direct(mr, is_write, attrs)) {
> +    if (!memory_access_is_direct(mr, HOST_BIG_ENDIAN, is_write, attrs)) {
>          size_t used = qatomic_read(&as->bounce_buffer_size);
>          for (;;) {
>              hwaddr alloc = MIN(as->max_bounce_buffer_size - used, l);
> 
> Thanks,
> Gavin
> 

I do not think it has anything to do with host endian-ness.


This is the change that broke it I think?


commit 4a2e242bbb306ef5c16ce9e7bb2da3bd8a4eb098
Author: Alex Williamson <alex@shazbot.org>
Date:   Mon Oct 31 09:53:03 2016 -0600

    memory: Don't use memcpy for ram_device regions
    

Maybe Alex has an opinion on what to do.


-- 
MST