Re: [PATCH v19 097/130] KVM: x86: Split core of hypercall emulation to helper function

Sean Christopherson
2024-04-03T18:55:31Z

On Wed, Apr 03, 2024, Isaku Yamahata wrote:
> On Fri, Mar 29, 2024 at 11:24:55AM +0800,
> Chao Gao <chao.gao@intel.com> wrote:
>
> > On Mon, Feb 26, 2024 at 12:26:39AM -0800, isaku.yamahata@intel.com wrote:
> > >@@ -10162,18 +10151,49 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> > >
> > > WARN_ON_ONCE(vcpu->run->hypercall.flags & KVM_EXIT_HYPERCALL_MBZ);
> > > vcpu->arch.complete_userspace_io = complete_hypercall_exit;
> > >+ /* stat is incremented on completion. */
> >
> > Perhaps we could use a distinct return value to signal that the request is redirected
> > to userspace. This way, more cases can be supported, e.g., accesses to MTRR
> > MSRs, requests to service TDs, etc. And then ...
>
> The convention here is the one for exit_handler vcpu_enter_guest() already uses.
> If we introduce something like KVM_VCPU_CONTINUE=1, KVM_VCPU_EXIT_TO_USER=0, it
> will touch many places. So if we will (I'm not sure it's worthwhile), the
> cleanup should be done as independently.

Yeah, this is far from the first time that someone has complained about KVM's
awful 1/0 return magic. And every time we've looked at it, we've come to the
conclusion that it's not worth the churn/risk.

And if we really need to further overload the return value, we can, e.g. KVM
already does this for MSR accesses:

/*
* Internal error codes that are used to indicate that MSR emulation encountered
* an error that should result in #GP in the guest, unless userspace
* handles it.
*/
#define KVM_MSR_RET_INVALID 2 /* in-kernel MSR emulation #GP condition */
#define KVM_MSR_RET_FILTERED 3 /* #GP due to userspace MSR filter */