Re: [PATCH 1/5] KVM: x86: Get vmcs12 pages before checking pending interrupts

From: Sean Christopherson <seanjc@google.com>
To: Jim Mattson <jmattson@google.com>
Cc: Maxim Levitsky <mlevitsk@redhat.com>,
	Mingwei Zhang <mizhang@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Joerg Roedel <joro@8bytes.org>, kvm <kvm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Oliver Upton <oupton@google.com>
Subject: Re: [PATCH 1/5] KVM: x86: Get vmcs12 pages before checking pending interrupts
Date: Thu, 25 Aug 2022 14:40:58 +0000	[thread overview]
Message-ID: <YweJ+hX8Ayz11jZi@google.com> (raw)
In-Reply-To: <CALMp9eSZ-C4BSSm6c5HBayjEVBdEwTBFcOw37yrd014cRwKPug@mail.gmail.com>

On Wed, Aug 24, 2022, Jim Mattson wrote:
> On Wed, Aug 24, 2022 at 5:11 PM Sean Christopherson <seanjc@google.com> wrote:
> 
> > @google folks, what would it take for us to mark KVM_REQ_GET_NESTED_STATE_PAGES
> > as deprecated in upstream and stop accepting patches/fixes?  IIUC, when we eventually
> > move to userfaultfd, all this goes away, i.e. we do want to ditch this at some point.
> 
> Userfaultfd is a red herring. There were two reasons that we needed
> this when nested live migration was implemented:
> 1) our netlink socket mechanism for funneling remote page requests to
> a userspace listener was broken.
> 2) we were not necessarily prepared to deal with remote page requests
> during VM setup.
> 
> (1) has long since been fixed. Though our preference is to exit from
> KVM_RUN and get the vCPU thread to request the remote page itself, we
> are now capable of queuing a remote page request with a separate
> listener thread and blocking in the kernel until the page is received.
> I believe that mechanism is functionally equivalent to userfaultfd,
> though not as elegant.
> I don't know about (2). I'm not sure when the listener thread is set
> up, relative to all of the other setup steps. Eliminating
> KVM_REQ_GET_NESTED_STATE_PAGES means that userspace must be prepared
> to fetch a remote page by the first call to KVM_SET_NESTED_STATE. The
> same is true when using userfaultfd.
> 
> These new ordering constraints represent a UAPI breakage, but we don't
> seem to be as concerned about that as we once were. Maybe that's a
> good thing. Can we get rid of all of the superseded ioctls, like
> KVM_SET_CPUID, while we're at it?

I view KVM_REQ_GET_NESTED_STATE_PAGES as a special case.  We are likely the only
users, we can (eventually) wean ourselves off the feature, and we can carry
internal patches (which we are obviously already carrying) until we transition
away.  And unlike KVM_SET_CPUID and other ancient ioctls() that are largely
forgotten, this feature is likely to be a maintenance burden as long as it exists.