Re: [PATCH v4 RESEND 3/5] KVM: riscv: Implement KVM_PRE_FAULT_MEMORY

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Modifying the HWPOISON path in `kvm_riscv_mmu_map()` to return `-EFAULT` breaks standard `KVM_RUN` memory error handling.
- [High] Incorrect address and boundary calculation for `out_map.addr` causes KVM to skip pre-faulting requested memory or underflow range calculations.
--

--- Patch [3]: [PATCH v4 RESEND 3/5] KVM: riscv: Implement KVM_PRE_FAULT_MEMORY ---
commit 28eceecf0e650b3722970e6824e30848edb82b44
Author: Jinyu Tang <[email protected]>

KVM: riscv: Implement KVM_PRE_FAULT_MEMORY

The generic KVM_PRE_FAULT_MEMORY ioctl lets userspace populate KVM page
tables before running a vCPU over a GPA range. This commit implements the
arch hook for RISC-V and reuses the G-stage fault mapping path.

> diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
> index 54d45addf18f9..dff315dfd24ed 100644
> --- a/arch/riscv/kvm/gstage.c
> +++ b/arch/riscv/kvm/gstage.c
> @@ -286,6 +286,9 @@ int kvm_riscv_gstage_map_page(struct kvm_gstage *gstage,
>  						    out_map->level, true);
>  		} else if (ALIGN_DOWN(PFN_PHYS(pte_pfn(ptep_get(ptep))), page_size) == hpa) {
>  			kvm_riscv_gstage_update_pte_prot(gstage, ptep_level, gpa, ptep, prot);
> +			out_map->addr = ALIGN_DOWN(gpa, page_size);
> +			out_map->level = ptep_level;

[Severity: High]
When pre-faulting a range backed by THP, could aligning out_map->addr to the
huge page size while setting out_map->level to the existing leaf size (e.g.,
4KB) cause an underflow later?

If out_map->addr is aligned to a 2MB boundary in kvm_riscv_gstage_map_page(),
but the map_size is calculated using the 4KB level in 
kvm_arch_vcpu_pre_fault_memory():

    map_size = PAGE_SIZE << (out_map.level * kvm_riscv_gstage_index_bits);
    end = out_map.addr + map_size;

The calculated end might be less than range->gpa. For example, if range->gpa
is (2MB_aligned_gpa + 8KB), then (end - range->gpa) would underflow to a
massive unsigned value, causing the ioctl to silently return range->size and
terminate early without actually pre-faulting the requested memory.

> +			out_map->pte = ptep_get(ptep);
>  			return 0;
>  		}
>  	}

[ ... ]

> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index 2fabcd4099919..f6ca86da53eb8 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
> @@ -717,7 +717,7 @@ int kvm_riscv_mmu_map(struct kvm_vcpu *vcpu, struct kvm_memory_slot *memslot,
>  	if (hfn == KVM_PFN_ERR_HWPOISON) {
>  		send_sig_mceerr(BUS_MCEERR_AR, (void __user *)hva,
>  				vma_pageshift, current);
> -		return 0;
> +		return -EFAULT;

[Severity: Critical]
Does returning -EFAULT here break the standard KVM_RUN memory error handling?

By returning -EFAULT instead of 0 for KVM_PFN_ERR_HWPOISON, KVM_RUN will exit
to userspace with -EFAULT. Userspace VMMs (like QEMU) typically treat an
-EFAULT from KVM_RUN as an unrecoverable hypervisor error and will abruptly
terminate the VM.

Returning 0 allows the run loop to exit cleanly with -EINTR, ensuring the VMM
can handle the queued SIGBUS (via send_sig_mceerr()), isolate the poisoned
page, and inject a Machine Check Exception into the guest.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3