Re: [PATCH v2 14/16] riscv: kvm: Use generated instruction headers for csr emulation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] KVM's MMIO emulation unconditionally overwrites the hardwired `x0` register within the guest's context, causing subsequent KVM-emulated instructions to use non-zero garbage values for `x0`.
--

commit 7feea165a4c11a607fc4f7aca428cb92288c4147
Author: Charlie Jenkins <thecharlesjenkins@gmail.com>

riscv: kvm: Use generated instruction headers for csr emulation
    
Migrate the csr emulation code to use the generated instruction headers
instead of the hand-written instruction composition functions.

> diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
> index c5a70de4a5790..d666cd24f8c0e 100644
> --- a/arch/riscv/kvm/vcpu_insn.c
> +++ b/arch/riscv/kvm/vcpu_insn.c
[ ... ]
> @@ -154,7 +150,7 @@ static int csr_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, ulong insn)
>  	#define GET_REG(_rd) (*((unsigned long *)(&vcpu->arch.guest_context) + _rd))
>  
>  	int i, rc = KVM_INSN_ILLEGAL_TRAP;
> -	unsigned int csr_num;
> +	unsigned int csr_num, rd;
>  	const struct csr_func *tcfn, *cfn = NULL;
>  	ulong val = 0, wr_mask = 0, new_val = 0;
>  
> @@ -163,26 +159,32 @@ static int csr_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, ulong insn)
>  		wr_mask = -1UL;
>  		new_val = GET_REG(riscv_insn_csrrw_extract_xs1(insn));

[Severity: High]
This isn't a bug introduced by this patch, but does the newly introduced
GET_REG macro expose a pre-existing corruption of guest_context.zero?

In RISC-V, the x0 register is hardwired to 0. If a guest executes an MMIO
load with destination register x0, KVM unconditionally writes the loaded data
to guest_context.zero:

arch/riscv/kvm/vcpu_insn.c:kvm_riscv_vcpu_mmio_return() {
    ...
    *((ulong *)(&vcpu->arch.guest_context) + vcpu->arch.mmio_decode.rd) =
        (long)data << shift >> shift;
    ...
}

This patch changes csr_insn() to unconditionally read the source register
using GET_REG() without verifying if the register is x0. 

If a guest triggers an MMIO load to x0, and then executes a CSR instruction
such as csrrw x0, sscratch, x0, will KVM read the corrupted non-zero value
from guest_context.zero and write it into the guest's CSR?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260621-riscv_insn_table-v2-0-7f0810ee2274@gmail.com?part=14