Re: [PATCH v4 2/3] system/memory: Use qemu_ram_move() for directly accessible regions

On Tue, 28 Jul 2026 at 10:22, Philippe Mathieu-Daudé
<[email protected]> wrote:
>
> On 28/7/26 11:01, Peter Maydell wrote:
> > On Tue, 28 Jul 2026 at 04:01, Gavin Shan <[email protected]> wrote:
> >>
> >> On 7/27/26 10:51 PM, Peter Maydell wrote:


> >>>> +    test = (uintptr_t)src | (uintptr_t)dst | n;
> >>>> +    len = test & -test;
> >>>
> >>> What is this doing? I am not a fan of clever bit twiddling
> >>> that isn't commented to explain itself. Readers of the code
> >>> shouldn't have to go off and search for an explanation of what
> >>> is going on.
> >>>
> >>
> >> The following comments will be added for (v5).
> >>
> >>       /*
> >>        * Maximal length of aligned access that are determined by @src,
> >>        * @dst and @n
> >>        */
> >
> > What I mean is more that it ought to say why "x & -x"
> > achieves that goal.
>
> Hmm isn't "x & -x" restricted to power of 2? Here we have no garanty
> @test is, because we have no garanty @n is. Ah, this is catched by
> the 'len != n' check and returns after calling memmove(). Indeed not
> very clear notation. Again:
>
>    ...
>
>    /* Overlapping buffers, unaligned or oversized access */
>    if (!QEMU_IS_ALIGNED((uintptr_t)src | (uintptr_t)dst, n)) {
>        memmove(dst, src, n);
>        return;
>    }
>
>    switch (n) {
>    ...

I'm not inherently against the bit twiddling (QEMU_IS_ALIGNED
on a variable length will do a division, maybe we care?);
I just want that if we do use a bit-twiddling trick that we
explain why it does the thing we want it to do.

-- PMM