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

From: Sean Christopherson <seanjc@google.com>
To: Andrew Jones <ajones@ventanamicro.com>
Cc: zhouquan@iscas.ac.cn, anup@brainfault.org, atishp@atishpatra.org,
	 paul.walmsley@sifive.com, palmer@dabbelt.com,
	linux-kernel@vger.kernel.org,  linux-riscv@lists.infradead.org,
	kvm@vger.kernel.org,  kvm-riscv@lists.infradead.org
Subject: Re: [PATCH] RISC-V: KVM: Avoid re-acquiring memslot in kvm_riscv_gstage_map()
Date: Fri, 13 Jun 2025 15:29:19 -0700	[thread overview]
Message-ID: <aEymPwNM59fafP04@google.com> (raw)
In-Reply-To: <20250612-70c2e573983d05c4fbc41102@orel>

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.

-- 
kvm-riscv mailing list
kvm-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kvm-riscv