Commit c7832534 authored by Jiakai Xu's avatar Jiakai Xu Committed by Anup Patel
Browse files

RISC-V: KVM: Fix sign extension for MMIO loads 



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: b91f0e4c ("RISC-V: KVM: Factor-out instruction emulation into separate sources")
Signed-off-by: default avatarJiakai Xu <jiakaiPeanut@gmail.com>
Signed-off-by: default avatarJiakai Xu <xujiakai2025@iscas.ac.cn>
Assisted-by: OpenClaw:DeepSeek-V3.2
Reviewed-by: default avatarAnup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260514081752.472987-1-xujiakai2025@iscas.ac.cn


Signed-off-by: default avatarAnup Patel <anup@brainfault.org>
parent fdb69d40

arch/riscv/kvm/vcpu_insn.c

+4 −5
Original line number Diff line number Diff line
@@ -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;