Re: [RFC PATCH] RISC-V: KVM: Enable KVM_MEM_USERFAULT support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] gstage_page_fault() returns 0 instead of -EFAULT when preparing KVM_EXIT_MEMORY_FAULT, causing ioctl(KVM_RUN) to return 0.
- [High] kvm_prepare_memory_fault_exit() is called with an unaligned guest physical address alongside a PAGE_SIZE length.
- [Low] Mixed declaration and code when declaring the `userfault` variable.
--

--- Patch [1]: [RFC PATCH] RISC-V: KVM: Enable KVM_MEM_USERFAULT support ---
commit 087f35f3e3961e5845e830a76559e809fe2e2d57
Author: Bingyu.Xian <shanbeeyoo@gmail.com>

RISC-V: KVM: Enable KVM_MEM_USERFAULT support

This commit adds support for KVM_MEM_USERFAULT on RISC-V to handle post-copy
live migration via userfaultfd. It forces PAGE_SIZE mappings and checks the
userfault bitmap in gstage_page_fault().

Link: https://lore.kernel.org/kvm/20260618205753.2600064-1-jthoughton@google.com/

> diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c
> index 8a0aa5e0e216e..766f820c27a73 100644
> --- a/arch/riscv/kvm/mmu.c
> +++ b/arch/riscv/kvm/mmu.c
> @@ -583,7 +583,9 @@ int kvm_riscv_mmu_map(struct kvm_vcpu *vcpu, struct kvm_memory_slot *memslot,
>  	else
>  		vma_pageshift = PAGE_SHIFT;
>  	vma_pagesize = 1ULL << vma_pageshift;
> -	if (logging || (vma->vm_flags & VM_PFNMAP))
> +	bool userfault = !!(memslot->flags & KVM_MEM_USERFAULT);
> +

[Severity: Low]
Is there a mixed declaration and code warning introduced here?

In kvm_riscv_mmu_map(), placing the declaration of userfault after the
executable statement vma_pagesize = 1ULL << vma_pageshift; violates the
standard kernel coding style.

Could we move the declaration to the beginning of the block?

>  	if (logging || userfault || (vma->vm_flags & VM_PFNMAP))
>  		vma_pagesize = PAGE_SIZE;

[ ... ]

> diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
> index 6c8530b9f29ed..31fb0b937afcc 100644
> --- a/arch/riscv/kvm/vcpu_exit.c
> +++ b/arch/riscv/kvm/vcpu_exit.c
> @@ -62,6 +62,13 @@ static int gstage_page_fault(struct kvm_vcpu *vcpu, struct kvm_run *run,
>  		};
>  	}
>  
> +	if (kvm_slot_userfault_fault(memslot, gfn)) {
> +		kvm_prepare_memory_fault_exit(vcpu, fault_addr, PAGE_SIZE,

[Severity: High]
Are we passing an unaligned guest physical address to
kvm_prepare_memory_fault_exit() here?

The KVM ABI defines the memory fault payload as a region [gpa, gpa + size)
that requires userspace resolution. The fault_addr variable is derived from
the stval register and may be an unaligned byte-granular address.

If fault_addr is unaligned (e.g., 0x1004) and paired with PAGE_SIZE, does it
cause the reported range [0x1004, 0x2004) to erroneously span across two
separate guest pages?

Could this cause the VMM to attempt out-of-bounds or redundant page
resolutions for the adjacent page, leading to migration failures?

> +					trap->scause == EXC_STORE_GUEST_PAGE_FAULT,
> +					trap->scause == EXC_INST_GUEST_PAGE_FAULT, false);
> +		return 0;

[Severity: High]
Should this path return -EFAULT instead of 0?

KVM API documentation requires that KVM_EXIT_MEMORY_FAULT be accompanied by a
-1 return code from the KVM_RUN ioctl, with errno set to EFAULT (or
EHWPOISON).

By returning 0 here in gstage_page_fault(), does kvm_riscv_vcpu_exit()
propagate 0 (success) back to the ioctl handler, and ultimately to userspace?

If the ioctl returns 0, will userspace VMMs like QEMU fail to recognize the
memory fault exit, fall through to the default unhandled exit handler, and
abort the VM?

> +	}
> +
>  	ret = kvm_riscv_mmu_map(vcpu, memslot, fault_addr, hva,

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