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

[Yosry Ahmed]

> > > 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:

Looks good, I was going to say ignore the series @
https://lore.kernel.org/kvm/20260305203005.1021335-1-yosry@xxxxxxxxxx/,
because I will incorporate the fix in it after patch 1 (the cleanup),
and patch 2 will need to be redone such that the test checks for
KVM_EXIT_INTERNAL_ERROR/KVM_INTERNAL_ERROR_EMULATION. But then I
stumbled upon the VMSAVE/VMLOAD behavior and the #GP I was observing
with vls=1 (see cover letter).

So I dug a bit deeper. Turns out with vls=1, if the GPA is supported
but not mapped, VMLOAD will generate a #NPF, and because there is no
slot KVM will install an MMIO SPTE and emulate the instruction. The
emulator will end up calling check_svme_pa() -> emulate_gp(). I didn't
catch this initially because I was tracing kvm_queue_exception_e() and
didn't get any hits, but I can see the call to
inject_emulated_exception() with #GP so probably the compiler just
inlines it.

Anyway, we'll also need to update the emulator. Perhaps just return
X86EMUL_UNHANDLEABLE from check_svme_pa() instead of injecting #GP,
although I don't think this will always end up returning to userspace
with
KVM_EXIT_INTERNAL_ERROR/KVM_INTERNAL_ERROR_EMULATION. Looking at
handle_emulation_failure(), we'll only immediately exit to userspace
if KVM_CAP_EXIT_ON_EMULATION_FAILURE is set (because EMULTYPE_SKIP
won't be set). Otherwise we'll inject a #UD, and only exit to
userspace if the VMSAVE/VMLOAD came from L1.

Not sure if that's good enough or if we need to augment the emulator
somehow (e.g. new return value that always exits to userspace? Or
allow EMULTYPE_SKIP x86_emulate_insn() -> check_perm() to change the
emulation type to add EMULTYPE_SKIP?