Re: [PATCH 1/2] KVM: arm64: Fix sign-extension of MMIO loads

[Fuad Tabba]

On Thu, 25 Jun 2026 at 11:10, Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> On Tue, 23 Jun 2026 14:51:49 +0100,
> Fuad Tabba <fuad.tabba@xxxxxxxxx> wrote:
> >
> > My reading of the ARM ARM is that the byte reversal is keyed on the access
> > size there too. It lives in Mem{size}, with the register width handled
> > separately by SignExtend(regsize):
> >
> >     data = Mem[address, 2];        // byte-reversed by the access size, BE
> >     X[t] = SignExtend(data, regsize);
> >
> > So vcpu_data_host_to_guest(..., len) swapping by len matches the Mem-side
> > reversal. Swapping by the register width would reorder bytes that were never
> > loaded. An LDRSH into Wt reads 2 bytes but would bswap 4: the halfword
> > reaches the helper as 0x0180 host-native, cpu_to_be32 turns it into
> > 0x80010000 instead of the 0x8001 cpu_to_be16 gives, and it never sign-extends
> > to 0xffff8001.
> >
> > If that reading holds, none of the helper's ops are individually wrong, and
> > the only bug was the order, with the sign-extend running before the swap and
> > the width mask then dropping it. But I've gone round in circles on endianness
> > before (to say the least), so please say if I've done it again.
>
> That's quite convincing.
>
> And the quoted pseudocode is much easier to reason about than the
> current blurb in the commit message. For reference, J1.2.3.111 Mem{}()
> is the relevant bit of the M.b spec and clearly shows that the access
> is done LE, and only then byteswapped. Can you please repaint the
> commit log to describe things in those terms?

Will do.

>
> Also, can you augment your test to cover for BE accesses from the
> guest if the HW supports it?

I'll see what I can come up with...

Cheers,
/fuad



>
> Thanks,
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.