Re: [PATCH v2] KVM: x86: Restrict saved GPA writes to hardware write faults

[Sean Christopherson <seanjc@google.com>]

On Sat, Aug 29, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 79468ddfe4736..aa5deb7c73aa2 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5088,8 +5088,13 @@ static int emulator_read_write_onepage(unsigned long addr, void *val,
> >  	 * Note, this cannot be used on string operations since string
> >  	 * operation using rep will only have the initial GPA from the NPF
> >  	 * occurred.
> > +	 *
> > +	 * The guest may have changed the instruction since the fault.  Reuse
> > +	 * the GPA for writes only if hardware reported a write access;
> > +	 * otherwise, recheck permissions for the decoded access.

Eh, I would just omit this comment, because a RMW operation is far more likely
than the guest playing TLB games.

> >  	 */
> > -	if (ctxt->gpa_available && emulator_can_use_gpa(ctxt) &&
> > +	if ((ctxt->gpa_access & (write ? ACC_WRITE_MASK : ACC_READ_MASK)) &&
> > +	    emulator_can_use_gpa(ctxt) &&
> 
> [Severity: High]
> Will this break the write phase of read-modify-write (RMW) instructions for
> SEV guests?
> 
> If a SEV guest executes an RMW instruction (like OR [mem], reg) on an
> unmapped MMIO page, the hardware triggers a Nested Page Fault (NPF) on the
> initial read access. KVM will set gpa_access to ACC_READ_MASK.
> 
> When the emulator attempts the write phase, it appears this check will fail
> because ACC_WRITE_MASK is not set. This causes emulator_read_write_onepage()
> to fall back to vcpu_mmio_gva_to_gpa().
> 
> Does this fallback attempt a software page table walk that will fail on
> encrypted guest page tables, resulting in a spurious #PF and crashing the
> SEV guest?

Ugh, SEV.  Part of me wants to take an erratum for the whole mess, but that would
be quite gross since the behavior would be conditional for non-SEV guests.  And
it should be easy enough to make this work for just SEV guests.

> 
> >  	    (addr & ~PAGE_MASK) == (ctxt->gpa_val & ~PAGE_MASK)) {
> >  		gpa = ctxt->gpa_val;
> >  		ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write);
> 
> [ ... ]
> 
> > @@ -6454,7 +6459,10 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> >  
> >  		/* With shadow page tables, cr2 contains a GVA or nGPA. */
> >  		if (vcpu->arch.mmu->root_role.direct) {
> > -			ctxt->gpa_available = true;
> > +			/* A write fault can require a read for RMW emulation. */

Heh, my brain fart notwithstanding, just drop this comment.  Because while RMW
operations are why KVM needs to get this correct, it's doesn't explain why it's
correct to allow reads on write faults.  We could write a comment for that, but
I think anyone that cares enough will be able to quickly deduce that reads are
allowed because x86 paging doesn't support write-only mappings.

> > +			ctxt->gpa_access = ACC_READ_MASK;
> > +			if (emulation_type & EMULTYPE_PF_WRITE)
> > +				ctxt->gpa_access |= ACC_WRITE_MASK;
> 
> [Severity: High]
> Does this limit gpa_access too strictly for read faults on RMW instructions?
> 
> As mentioned above, if an instruction naturally faults on its first (read)
> access, it will only get ACC_READ_MASK here. This seems to be the root cause
> that later prevents the write phase from reusing the GPA during MMIO
> emulation.

For !SEV, that's fine.  In practice, I highly doubt RMW operations are used in
fast paths, e.g. Linux-as-a-guest straight up supports only MOV instructions.

For SEV, this as fixup?  And then if someone cares enough (I don't think I care?),
we could add an entry in Documentation/virt/kvm/x86/errata.rst for SEV.

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3db88742ea9a..4f6659aa72ec 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6459,9 +6459,17 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
 
 		/* With shadow page tables, cr2 contains a GVA or nGPA. */
 		if (vcpu->arch.mmu->root_role.direct) {
-			/* A write fault can require a read for RMW emulation. */
 			ctxt->gpa_access = ACC_READ_MASK;
-			if (emulation_type & EMULTYPE_PF_WRITE)
+			/*
+			 * Always allow writes for SEV guests, as the guest's
+			 * page tables are encrypted, i.e. KVM can't walk the
+			 * guest's page tables and so must always use the GPA
+			 * from the initial fault.  Restricting use of the GPA
+			 * to the access type that faulted would prevent KVM
+			 * from emulating RMW operations for SEV guests.
+			 */
+			if ((emulation_type & EMULTYPE_PF_WRITE) ||
+			    is_sev_guest(vcpu))
 				ctxt->gpa_access |= ACC_WRITE_MASK;
 			ctxt->gpa_val = cr2_or_gpa;
 		}