[PATCH] RISC-V: KVM: Fix sign extension for MMIO loads

[Anup Patel]

On Thu, May 14, 2026 at 1:47 PM Jiakai Xu <xujiakai2025 at iscas.ac.cn> wrote:
>
> The kvm_riscv_vcpu_mmio_return() function handles MMIO read results
> by writing the data back to the guest register. For signed load
> instructions (LB, LH, LW on RV64), the value needs sign-extension
> from a smaller integer to unsigned long.
>
> The current code uses:
>     (ulong)data << shift >> shift
> but (ulong) makes the right shift a logical shift (zero-extend)
> rather than an arithmetic shift (sign-extend), causing incorrect
> results when the MMIO device returns a negative value. For example,
> LB reading 0x80 would return 128 instead of -128.
>
> Fix this by casting to (long) after the left shift so that the
> subsequent right shift is arithmetic and correctly propagates
> the sign bit:
>     (long)((ulong)data << shift) >> shift
>
> Additionally, remove the unnecessary shift assignment for LBU
> (unsigned byte load) since it does not need sign extension.
> This makes LBU consistent with LHU and LWU which already keep
> shift = 0.
>
> Fixes: b91f0e4cb8a3 ("RISC-V: KVM: Factor-out instruction emulation into separate sources")
> Signed-off-by: Jiakai Xu <jiakaiPeanut at gmail.com>
> Signed-off-by: Jiakai Xu <xujiakai2025 at iscas.ac.cn>
> Assisted-by: OpenClaw:DeepSeek-V3.2

LGTM.

Reviewed-by: Anup Patel <anup at brainfault.org>

Queued this patch as fix for Linux-7.1-rcX

Thanks,
Anup

> ---
>  arch/riscv/kvm/vcpu_insn.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
> index 4d89b94128aea..f09f9251d1f0a 100644
> --- a/arch/riscv/kvm/vcpu_insn.c
> +++ b/arch/riscv/kvm/vcpu_insn.c
> @@ -415,7 +415,6 @@ int kvm_riscv_vcpu_mmio_load(struct kvm_vcpu *vcpu, struct kvm_run *run,
>                 shift = 8 * (sizeof(ulong) - len);
>         } else if ((insn & INSN_MASK_LBU) == INSN_MATCH_LBU) {
>                 len = 1;
> -               shift = 8 * (sizeof(ulong) - len);
>  #ifdef CONFIG_64BIT
>         } else if ((insn & INSN_MASK_LD) == INSN_MATCH_LD) {
>                 len = 8;
> @@ -649,22 +648,22 @@ int kvm_riscv_vcpu_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
>         case 1:
>                 data8 = *((u8 *)run->mmio.data);
>                 SET_RD(insn, &vcpu->arch.guest_context,
> -                       (ulong)data8 << shift >> shift);
> +                       (long)((ulong)data8 << shift) >> shift);
>                 break;
>         case 2:
>                 data16 = *((u16 *)run->mmio.data);
>                 SET_RD(insn, &vcpu->arch.guest_context,
> -                       (ulong)data16 << shift >> shift);
> +                       (long)((ulong)data16 << shift) >> shift);
>                 break;
>         case 4:
>                 data32 = *((u32 *)run->mmio.data);
>                 SET_RD(insn, &vcpu->arch.guest_context,
> -                       (ulong)data32 << shift >> shift);
> +                       (long)((ulong)data32 << shift) >> shift);
>                 break;
>         case 8:
>                 data64 = *((u64 *)run->mmio.data);
>                 SET_RD(insn, &vcpu->arch.guest_context,
> -                       (ulong)data64 << shift >> shift);
> +                       (long)((ulong)data64 << shift) >> shift);
>                 break;
>         default:
>                 return -EOPNOTSUPP;
> --
> 2.34.1
>