On 7/25/26 2:31 AM, Michael S. Tsirkin wrote:
On Fri, Jul 24, 2026 at 09:56:48AM -0400, Peter Xu wrote:
On Fri, Jul 24, 2026 at 11:42:09AM +1000, Gavin Shan wrote:
On 7/23/26 11:46 PM, Peter Xu wrote:
On Thu, Jul 23, 2026 at 05:05:08AM -0400, Michael S. Tsirkin wrote:
[...]
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.
Lets see if Peter Maydell and Michael are happy with this option. At least,
we will have unified qemu_ram_move() for all architectures with this option.
Note that qemu_ram_copy() won't be needed.
I'm putting note on what's to be done in (v4) if this option is to be picked
up. Let me know if there are missed points. It's basically combing what's done
by ram_device_mem_ops to upper layer (e.g. in qemu_ram_move()).
address_space_write
address_space_to_flatview
flatview_write
flatview_translate
flatview_write_continue
flatview_write_continue_step
memmove // (A) to replace it with qemu_ram_move()
/**
* qemu_ram_move: move data to ramblock
*
* @dst: destination where the data is moved to
* @src: source where the data is moved from
* @n: length of data to be moved
*
* Move @n bytes from @src to @dst with the assumption that @src and @dst
* can overlap. The access is atomic if the source and destination buffer
* aren't overlapped for a well aligned and small-sized access. Otherwise,
* fall back to the standard memmove().
*/
static void qemu_ram_move(void *dst, const void *src, size_t n)
{
uintptr_t test, len;
if (src == dst || n == 0) {
return;
}
/* Overlapped buffers */
overlapping
Corrected in v4.
https://lore.kernel.org/qemu-arm/[email protected]/
if (src < (dst + n) && dst < (src + n)) {
s/&&/||/?
It's a bit weird to request memmove() for overlapped, e.g. I don't know if
P2P can overlap too when some fuzzer fills in some MMIO address shifted for
src/dst.. but I think I get what you want to simplify and it looks fine.
Otherwise it looks good.
Why do we bother special casing overlapping buffers though?
I do not get it, looks like rest of logic works exactly
the same for overlapping and non.
Indeed. The explicit check if src/dst are overlapping has been dropped in v4.
memmove(dst, src, n);
return;
}
test = (uintptr_t)src | (uintptr_t)dst | n;
len = test & -test;
/* Unaligned or oversized access */
if (n > 8 || len != n) {
memmove(dst, src, n);
return;
}
switch (len) {
case 1:
qatomic_set((uint8_t *)dst, qatomic_read((uint8_t *)src));
break;
case 2:
qatomic_set((uint16_t *)dst, qatomic_read((uint16_t *)src));
break;
case 4:
qatomic_set((uint32_t *)dst, qatomic_read((uint32_t *)src));
break;
case 8:
qatomic_set((uint64_t *)dst, qatomic_read((uint64_t *)src));
break;
default:
g_assert_not_reached();
}
}
[...]
Thanks,
Gavin
