Re: [PATCH v7 26/26] KVM: selftest: Add a selftest for VMRUN/#VMEXIT with unmappable vmcb12

[Sean Christopherson <seanjc@google.com>]

On Thu, Mar 05, 2026, Jim Mattson wrote:
> On Thu, Mar 5, 2026 at 4:40 PM Yosry Ahmed <yosry@kernel.org> wrote:
> >
> > On Thu, Mar 5, 2026 at 4:05 PM Jim Mattson <jmattson@google.com> wrote:
> > >
> > > On Thu, Mar 5, 2026 at 2:52 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > > >
> > > > On Thu, Mar 5, 2026 at 2:30 PM Jim Mattson <jmattson@google.com> wrote:
> > > > >
> > > > > On Mon, Mar 2, 2026 at 4:43 PM Yosry Ahmed <yosry@kernel.org> wrote:
> > > > > >
> > > > > > Add a test that verifies that KVM correctly injects a #GP for nested
> > > > > > VMRUN and a shutdown for nested #VMEXIT, if the GPA of vmcb12 cannot be
> > > > > > mapped.
> > > > > >
> > > > > > Signed-off-by: Yosry Ahmed <yosry@kernel.org>
> > > > > > ...
> > > > > > +       /*
> > > > > > +        * Find the max legal GPA that is not backed by a memslot (i.e. cannot
> > > > > > +        * be mapped by KVM).
> > > > > > +        */
> > > > > > +       maxphyaddr = kvm_cpuid_property(vcpu->cpuid, X86_PROPERTY_MAX_PHY_ADDR);
> > > > > > +       max_legal_gpa = BIT_ULL(maxphyaddr) - PAGE_SIZE;
> > > > > > +       vcpu_alloc_svm(vm, &nested_gva);
> > > > > > +       vcpu_args_set(vcpu, 2, nested_gva, max_legal_gpa);
> > > > > > +
> > > > > > +       /* VMRUN with max_legal_gpa, KVM injects a #GP */
> > > > > > +       vcpu_run(vcpu);
> > > > > > +       TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
> > > > > > +       TEST_ASSERT_EQ(get_ucall(vcpu, &uc), UCALL_SYNC);
> > > > > > +       TEST_ASSERT_EQ(uc.args[1], SYNC_GP);
> > > > >
> > > > > Why would this raise #GP? That isn't architected behavior.
> > > >
> > > > I don't see architected behavior in the APM for what happens if VMRUN
> > > > fails to load the VMCB from memory. I guess it should be the same as
> > > > what would happen if a PTE is pointing to a physical address that
> > > > doesn't exist? Maybe #MC?
> > >
> > > Reads from non-existent memory return all 1's
> >
> > Today I learned :) Do all x86 CPUs do this?
> 
> Yes. If no device claims the address, reads return all 1s. I think you
> can thank pull-up resistors for that.

Ya, it's officially documented PCI behavior.  Writes are dropped, reads return
all 1s.

> > > so I would expect a #VMEXIT with exitcode VMEXIT_INVALID.
> >
> > This would actually simplify the logic, as it would be the same
> > failure mode as failed consistency checks. That being said, KVM has
> > been injecting a #GP when it fails to map vmcb12 since the beginning.
> 
> KVM has never been known for its attention to detail.

LOL, hey, we try.  Sometimes we just forget things though :-)

7a35e515a705 ("KVM: VMX: Properly handle kvm_read/write_guest_virt*() result")

> > It also does the same thing for VMSAVE/VMLOAD, which seems to also not
> > be architectural. This would be more annoying to handle correctly
> > because we'll need to copy all 1's to the relevant fields in vmcb12 or
> > vmcb01.
> 
> Or just exit to userspace with
> KVM_EXIT_INTERNAL_ERROR/KVM_INTERNAL_ERROR_EMULATION. I think on the
> VMX side, this sort of thing goes through kvm_handle_memory_failure().

Yep, I think this is the correct fixup:

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index b191c6cab57d..78a542c6ddf1 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1105,10 +1105,8 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
 
        vmcb12_gpa = svm->vmcb->save.rax;
        err = nested_svm_copy_vmcb12_to_cache(vcpu, vmcb12_gpa);
-       if (err == -EFAULT) {
-               kvm_inject_gp(vcpu, 0);
-               return 1;
-       }
+       if (err == -EFAULT)
+               return kvm_handle_memory_failure(vcpu, X86EMUL_UNHANDLEABLE, NULL);
 
        /*
         * Advance RIP if #GP or #UD are not injected, but otherwise stop if