Re: [PATCH] RISC-V: KVM: Avoid re-acquiring memslot in kvm_riscv_gstage_map()

[Sean Christopherson]

On Thu, Jun 12, 2025, Andrew Jones wrote:
> On Wed, Jun 11, 2025 at 09:17:36AM -0700, Sean Christopherson wrote:
> > Looks like y'all also have a bug where an -EEXIST will be returned to userspace,
> > and will generate what's probably a spurious kvm_err() message.
> 
> On 32-bit riscv, due to losing the upper bits of the physical address? Or
> is there yet another thing to fix?

Another bug, I think.  gstage_set_pte() returns -EEXIST if a PTE exists, and I
_assume_ that's supposed to be benign?  But this code returns it blindly:

	if (writable) {
		mark_page_dirty(kvm, gfn);
		ret = gstage_map_page(kvm, pcache, gpa, hfn << PAGE_SHIFT,
				      vma_pagesize, false, true);
	} else {
		ret = gstage_map_page(kvm, pcache, gpa, hfn << PAGE_SHIFT,
				      vma_pagesize, true, true);
	}

	if (ret)
		kvm_err("Failed to map in G-stage\n");

out_unlock:
	kvm_release_faultin_page(kvm, page, ret && ret != -EEXIST, writable);
	spin_unlock(&kvm->mmu_lock);
	return ret;

and gstage_page_fault() forwards negative return codes:

	ret = kvm_riscv_gstage_map(vcpu, memslot, fault_addr, hva,
		(trap->scause == EXC_STORE_GUEST_PAGE_FAULT) ? true : false);
	if (ret < 0)
		return ret;

and so eventually -EEXIST will propagate to userspace.

I haven't looked too closely at the RISC-V MMU, but I would be surprised if
encountering what ends up being a spurious fault is completely impossible.

> The diff looks good to me, should I test and post it for you?

If you test it, I'll happily write changelogs and post patches.