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

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

On Thu, Jun 11, 2026 at 05:16:09PM +0100, Peter Maydell wrote:
> On Thu, 11 Jun 2026 at 16:57, Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Thu, Jun 11, 2026 at 04:29:49PM +0100, Peter Maydell wrote:
> > > On Thu, 11 Jun 2026 at 16:05, Michael S. Tsirkin <mst@redhat.com> wrote:
> > > > What is "OK"? If the BAR is RAM and I write into it, I will overwrite
> > > > data guest stored there. Is that "OK"?
> > >
> > > By "OK" I mean it behaves like RAM: any kind of load or store
> > > will work, at any width and any alignment.
> > > That is what makes
> > > it OK to return the pointer from address_space_map(), where
> > > the caller will treat it as any other C host pointer (e.g.
> > > assigning it to a C struct pointer and then writing to fields
> > > in that struct), like virtio does:
> > >
> > >         iov[num_sg].iov_base = dma_memory_map(vdev->dma_as, pa, &len,
> > >                                               is_write ?
> > >                                               DMA_DIRECTION_FROM_DEVICE :
> > >                                               DMA_DIRECTION_TO_DEVICE,
> > >                                               MEMTXATTRS_UNSPECIFIED);
> > >         if (!iov[num_sg].iov_base) {
> > >             virtio_error(vdev, "virtio: bogus descriptor or out of resources");
> > >             goto out;
> > >         }
> > > [etc]
> > >
> > > This is only safe when the pointer is something we can
> > > handle as if it were a pointer to normal RAM (because the
> > > C compiler makes no promises about only doing correctly
> > > aligned and sized accesses: to it, memory is memory).
> >
> >
> >
> > Safe as in what? Work meaning what?
> >
> > mmap of a device register is a standard API in Linux, it gives you a
> > pointer and no, it does not imply that "any access" will not make your
> > box go up in smoke, and never did.
> 
> Yes. So you need to be careful about what you do with the
> pointers you get into something you mmap()ed. In particular,
> if it's not actually RAM then you can't just hand it to the
> C compiler and say "this is a pointer to some data structure",
> because the C compiler is not going to restrict itself to
> only doing the things that the hardware says are in spec.
> That's why we don't want to mark these memory regions as
> "direct access OK".
> 
> > > > If the BAR is RAM and I write into it, I will overwrite
> > > > data guest stored there. Is that "OK"?
> > >
> > > If the guest said "please DMA to this address" then they're
> > > expecting you to overwrite that data, yes.
> >
> > The "If the guest said" is critical here. It's not OK
> > at a random time and place of your choosing, and never will be.
> >
> > It's exactly what I said: if we are doing what guest does,
> > like fixed length memcpy does,
> > then we are good. If instead we are replacing guest accesses
> > with different ones like variable length memcpy does,
> > we'll get issues.
> 
> But these accesses are not guest accesses. They're device
> accesses (by a device model doing DMA, including the virtio
> device). So "what access might we do" depends on the device spec.
> There's no "guest software did a 4 byte write" that we're
> converting into something else.


Device does it because guest told it to do it.
It is really the same.

> > > Merging in your other reply:
> > >
> > > >> The
> > > >> problem is that for some PCI devices (like the network card
> > > >> mentioned in 4a2e242bbb30's commit message) the BAR is *not*
> > > >> safe for arbitrary access (because the actual real host hardware
> > > >> inside it is not RAM).
> > > >
> > > > But we don't do arbitrary access. Why would we?
> > >
> > > If we mark the MR as "direct access OK", then we do, or might do.
> >
> > what does this might mean?
> 
> It means that by marking the MR as "direct access" we say
> "these are permitted". Whether they happen or not depends
> on what the code in QEMU does and what the C compiler does
> with that. If you don't want them, don't say they're OK.
> 
> > > The bug the commit is fixing is exactly that there is a codepath
> > > that uses the latitude that marking the MR as direct access permits.
> >
> >
> > No, the bug is that a 4 byte fixed length access was converted
> > to a variable length memcpy function call which started
> > doing single byte accesses.
> >
> >
> > >
> > > > There's no such thing as "not safe for direct access" in PCI.
> > > > All operations are memory operations.
> > > > What can be unsafe is accesses of specific width and length.
> > >
> > > I think we're talking at cross purposes here. By "safe for
> > > direct access" I mean exactly that arbitrary width and
> > > length and alignment are permitted.
> >
> > This is a strong demands that QEMU simply never needs.
> 
> It does for address_space_map(). That's what that gives you.

yes no part of qem use that.

> > > This is what
> > > memory_region_supports_direct_access() is testing.
> > > If the PCI BAR can't handle arbitrary widths etc then that
> > > function mustn't return true for its MR.
> > >
> > > (Though if we ever get into trying to address_space_map()
> > > a BAR like that something is going to go wrong anyway, because
> > > the "not OK for direct access" path fills and empties the bounce
> > > buffer by calling flatview_read() and address_space_write(),
> > > which aren't going to honour any theoretical alignment requirements.
> > > Perhaps that kind of BAR should just flat out not be one we handle
> > > with a ram_device MR?)
> >
> >
> > mmap of a device is a standard thing that everyone did on unix, for
> > decades. just don't do things, with a device, that a driver does not do.
> >
> > > > > (I think there are places where we do need to be more careful about
> > > > > what we do with accesses to real RAM, for where we're emulating a
> > > > > device write to RAM that's updating a data structure shared with the
> > > > > guest, and things like writing multiple times can cause problems.
> > > > > https://lore.kernel.org/qemu-devel/CAFEAcA8dwHV8F48kb-013rxkG9kKcZhym9_qarKmoeUfeh0YWw@mail.gmail.com/
> > > > > is an unrelated example of that, which I haven't done detailed
> > > > > analysis of yet.)
> > >
> > > > If it does not work, then QEMU is broken:
> > > >
> > > >
> > > > /*
> > > >  * Any compiler worth its salt will turn these memcpy into native unaligned
> > > >  * operations.  Thus we don't need to play games with packed attributes, or
> > > >  * inline byte-by-byte stores.
> > > >  * Some compilation environments (eg some fortify-source implementations)
> > > >  * may intercept memcpy() in a way that defeats the compiler optimization,
> > > >  * though, so we use __builtin_memcpy() to give ourselves the best chance
> > > >  * of good performance.
> > > >  */
> > >
> > > Good point. (Though if I were an optimizing compiler I might
> > > be tempted to optimize a switch() on the length where each
> > > case called memmove with a fixed length argument back into a
> > > single call to memmove...)
> > >
> > > I suspect your suggested flatview changes would fix that e1000
> > > bug. But I don't think they're the right thing for this problem.
> 
> > Then I'd like to see an example where we have an actual good reason
> > to do execute arbitrary width accesses on device RAM when the
> > driver does not execute them, please.
> 
> That's the case we started with, with this GPU where the
> virtio data structures are in this PCI BAR. We want the
> virtio backend to have the freedom to say "I'm just going
> to assume the ring buffer etc is in RAM, and I don't need to
> care about carefully ensuring that I only do word accesses".

virtio backend does not need to assume anything. any spec
compliant guest guarantees it. any address given to
a virtio device is ram.


> That's why it uses address_space_map()/dma_memory_map().

Nope.  It could use QEMUSGList just as well. map is just easier.

> 
> -- PMM