Re: [PATCH v2] RISC-V: KVM: Fix spurious -EEXIST and clean up gstage fault path types

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The widened 34-bit `fault_addr` on RV32 is silently truncated back to 32 bits when passed to MMIO load/store handlers.
--

--- Patch [1]: [PATCH v2] RISC-V: KVM: Fix spurious -EEXIST and clean up gstage fault path types ---
commit 0f1ae40b2c79a54468668b54d25a4ee2ead28522
Author: Bingyu.Xian <shanbeeyoo@gmail.com>

RISC-V: KVM: Fix spurious -EEXIST and clean up gstage fault path types

This commit treats -EEXIST from kvm_riscv_gstage_map_page() as a quiet
success, aligning RISC-V with x86 and arm64. It also widens fault_addr to
gpa_t to prevent 34-bit guest physical addresses from being truncated by
32-bit arithmetic on RV32.

> diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
> index 6c8530b9f29ed..28cf9b27bb07c 100644
> --- a/arch/riscv/kvm/vcpu_exit.c
> +++ b/arch/riscv/kvm/vcpu_exit.c
> @@ -17,12 +17,13 @@ static int gstage_page_fault(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  {
>  	struct kvm_gstage_mapping host_map;
>  	struct kvm_memory_slot *memslot;
> -	unsigned long hva, fault_addr;
> +	unsigned long hva;
> +	gpa_t fault_addr;
>  	bool writable;
>  	gfn_t gfn;
>  	int ret;
>  
> -	fault_addr = (trap->htval << 2) | (trap->stval & 0x3);
> +	fault_addr = ((gpa_t)trap->htval << 2) | (trap->stval & 0x3);

[Severity: High]
This is a pre-existing issue, but since the patch explicitly widens
fault_addr to gpa_t to hold 34-bit addresses on RV32, will this address
be truncated back to 32 bits when passed to the MMIO handlers later in
this function?

Looking at arch/riscv/kvm/vcpu_insn.c, the handlers still expect an
unsigned long:

int kvm_riscv_vcpu_mmio_load(struct kvm_vcpu *vcpu, struct kvm_run *run,
                             unsigned long fault_addr,
                             unsigned long htinst)

and

int kvm_riscv_vcpu_mmio_store(struct kvm_vcpu *vcpu, struct kvm_run *run,
                              unsigned long fault_addr,
                              unsigned long htinst)

Does this mean MMIO accesses to addresses above 4GB on RV32 will still be
silently aliased into the lower 4GB when gstage_page_fault() calls these
handlers?

>  	gfn = fault_addr >> PAGE_SHIFT;
>  	memslot = gfn_to_memslot(vcpu->kvm, gfn);
>  	hva = gfn_to_hva_memslot_prot(memslot, gfn, &writable);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729075230.743030-1-shanbeeyoo@gmail.com?part=1