Re: [PATCH v3 1/2] system/memory: Use qemu_ram_{copy, move}() in ram device region accessors

[Peter Xu <peterx@redhat.com>]

On Thu, Jul 23, 2026 at 05:05:08AM -0400, Michael S. Tsirkin wrote:
> On Thu, Jul 23, 2026 at 10:52:11AM +0200, Philippe Mathieu-Daudé wrote:
> > On 23/7/26 08:04, Michael S. Tsirkin wrote:
> > > On Wed, Jul 22, 2026 at 12:41:34PM -0400, Peter Xu wrote:
> > > > On Wed, Jul 22, 2026 at 01:58:18AM -0400, Michael S. Tsirkin wrote:
> > > > > On Wed, Jul 22, 2026 at 10:53:27AM +1000, Gavin Shan wrote:
> > > > > > On 7/22/26 2:27 AM, Peter Xu wrote:
> > > > > > > On Tue, Jul 21, 2026 at 03:37:53PM +1000, Gavin Shan wrote:
> > > > > > > > If Peter is fine with two variants for x86 and non-x86 architectures.
> > > > > > > > I can post (v4) for further review. That will be something like below
> > > > > > > > and let me know if there are any other improvements are needed.
> > > > > > > 
> > > > > > > I have a generic question on the "unaligned access for x86": I think the
> > > > > > > question is about the one Michael raised here on unaligned access may break
> > > > > > > x86 here:
> > > > > > > 
> > > > > > >     https://lore.kernel.org/qemu-devel/20260617022330-mutt-send-email-mst@kernel.org/
> > > > > > > 
> > > > > > >     3. (theoretical concern) also on x86, unaligned accesses are
> > > > > > >     possible on guest and host, so converting an unaligned access to a
> > > > > > >     series of aligned ones can in theory break devices.
> > > > > > > 
> > > > > > > Is that a real problem we need to consider, or can we start with unified
> > > > > > > approach and leave it for later?
> > > > > > > 
> > > > > > 
> > > > > > I'm leaving this question to Michael.
> > > > > 
> > > > > Knowing what I know about hardware designers, it's something someone
> > > > > somewhere does)
> > > > > It can be made a separate patch, just to show - it should be all of
> > > > > ~10LOC.
> > > > 
> > > > It's only about removal of anything that might be controversial for now,
> > > > thanks.  I also wonder if anything would break, then it's more solid proof
> > > > that per-arch change is required.
> > > 
> > > Repeating:
> > > 	I think there is exactly 1 kinda reasonable case. A 2 byte read/write at
> > > 	offset 0x1 within a dword. This maps nicely to even classical PCI byte
> > > 	enable mechanism and so yes it works if your CPU can initiate these
> > > 	things, and it's atomic.
> > 
> > Isn't this out of the CPU arch, dealt with at the bus level?
> > 
> > It looks we try to be clever with modern PCI code by optimizing this
> > access -- not saying we can change that, I know it is too late after
> > 20+ years -- relying on hw behavior that was done that way to support
> > legacy hw, in particular broken I/O accesses.
> 
> Not sure what the question is.  We were dicussing how to emulate unaligned
> accesses from x86 guests if they happen.
> On an x86 host we can do that easily, and it's just a couple of LOC.
> Though Peter Maydell dislikes host arch specific code. But I hope
> if it's a separate patch on top and it is visible how small it is,
> he will reconsider)

Yes, if we still want x86 specific change, it would be better to be put
separately.

Said so, I don't think the e1000e LEDCTL test illustrated what might
break..  Isn't that only an exmaple showing unaligned access is
"supported", however nothing breaks even if we use 1B*2?

My question was more about a real breakage, hence whenever it happened
"it's more solid proof that per-arch change is required".

> On other hosts we can't emulate them 100%, we either need to split
> to byte accesses or over-access and mask. Byte accesses feel safer.
> What qemu currently does with memmove is clearly not safe in the
> general case.

We should have another option that is not arch-dependent but keep the
unaligned behavior.

For current master, AFAIU we do unaligned access for both ram_device and
rest. Say, even with ram_device_mem_ops, it has both .unaligned=true for
both .valid & .impl.  I think it means indeed we have unaligned behavior
even for ram_device.  It also means what matters in regards to the Realtek
bug was only about aligned access (with subpage presence).

I think it means we can always keep unaligned to stick with
memcpy()/memmove(), but only use atomic ops for the aligned cases of
1/2/4/8.  With that, I think we can also remove ram_device_mem_ops and fix
the bounce buffer issue.

I think it means we'll stick with memcpy()/memmove() for all archs for
unaligned, which is again not safe... but that can be an existing but
separate problem to solve too.

One more thing to mention below...

> 
> 
> 
> As for optimizing - there is space for optimization e.g. vfio could
> report the properties of a BAR to userspace for optimization
> purposes. On x86 you can then get good speed with just memmove.
> Other arches you will likely need to write arch specific code.
> But this has to do with e.g. DMA into device BAR not CPU accesses.
> As an aside, it is a pity qemu uses same thing for DMA and CPU access,
> we know how virtio accesses work and this might allow optimization.
> 
> > 
> > For Alpha / HPPA / MIPS there were ASIC in the PCI I/O path to handle
> > these odd unaligned accesses inherited from x86 world.
> 
> Right.
> 
> > > It's easy to find more examples of such hardware if one looks. For example,
> > > LEDCTL on e1000e:
> > > 
> > > 20260625101817-mutt-send-email-mst@kernel.org
> > > 
> > > 
> > > A claim that no software uses this hardware capability is the strong
> > > claim that needs proof, not the other way around.
> > > 
> > > 
> > > What to do on non-x86? reading a dword might work, or reading
> > > byte by byte might work. Both can cause issues, just different ones,
> > > but given we did byte by byte previously i guess let's keep
> > > doing that.
> > > 
> > > 
> > > > > 
> > > > > > > PS: I apologize if I missed important piece of info along the way; I didn't
> > > > > > > follow closely on the discussion on this topic in the past few weeks.
> > > > > > > 
> > > > > > > One thing to mention is, what we change should only need to affect
> > > > > > > ram_device, AFAIU.. so most memcpy()/memmove() shouldn't be changed for any
> > > > > > > arch when it's pure RAM.
> > > > > > > 
> > > > > > 
> > > > > > It depends. This patch intends to fix issue [1] in the lower layer by using
> > > > > > the newly added accessors (qemu_ram_{copy, move}) on all directly accessible
> > > > > > regions including the regular (pure) RAM region. Otherwise, the newly added
> > > > > > accessors should be limited to ram_device regions only as you said.
> > > > > > 
> > > > > > [1] https://lore.kernel.org/qemu-devel/20260527091711.3901-1-liugang24219@sangfor.com.cn/
> > > > > > 
> > > > > > Thanks,
> > > > > > Gavin
> > > > > 
> > > > > using memcpy()/memmove() to emulate guest's atomics is generally
> > > > > kinda broken.
> > > > > but yes there are architectures where doing it to device ram is
> > > > > more broken than doing it to regular ram.

.. please someone (at least.. Gavin?) still have a look at below.  It could
matter how the next version looks, especially I want to make sure if we
should get rid of __builtin_*() in the new helpers.

Thanks,

> > > > 
> > > > If we keep memcpy()/memmove() for len>8 (aligned or not), I am thinking no
> > > > perf issue will happen, then looks like we can indeed change this even for
> > > > pure RAM operations, which we can't identify in case of e1000e driver use
> > > > case.
> > > > 
> > > > Then does it mean we should not use __builtin_memcpy()/memmove()?  Even if
> > > > we know constants 1/2/4/8 would work there, why not we go ahead and use
> > > > qatomics, which is even more future proof?  I also stumbled on top of
> > > > commit 77b1757090 ("include/qemu/bswap.h: Use __builtin_memcpy() in
> > > > accessor functions"), which seems to say the same thing "for the long
> > > > term".
> > > > 
> > > > For "should we still do unaligned access if the guest did it, so as to keep
> > > > the original behavior of a bare metal" question, are we on the same page
> > > > that we should just break it into aligned accesses for all archs?  I think
> > > > it means, we will not be able to emulate guest faults correctly as what
> > > > will happen on bare metal, but we're missing the fault injection logics
> > > > anyway, so whenever it's implemented we _could_ switch it back to unaligned
> > > > accesses.  Before that, breaking unaligned seems like a better way to go to
> > > > (1) satisfy all legit users, and (2) don't make QEMU crash by guest
> > > > operations.
> > > > 
> > > > Last, one silly question: why do we need any helper named with *memcpy*, if
> > > > memmove is always superior (to consider range overlap)?  Can we stick with
> > > > memmove all over the places?
> > > > 
> > > > Thanks,
> > > > 
> > > > -- 
> > > > Peter Xu
> > > 
> > > 
> 

-- 
Peter Xu