[RFC PATCH 0/11] Rework gfn_to_pfn_cache

From: David Woodhouse <dwmw2@infradead.org>
To: Paolo Bonzini <pbonzini@redhat.com>, kvm <kvm@vger.kernel.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Joao Martins <joao.m.martins@oracle.com>,
	"jmattson@google.com" <jmattson@google.com>,
	"wanpengli@tencent.com" <wanpengli@tencent.com>,
	"seanjc@google.com" <seanjc@google.com>,
	"vkuznets@redhat.com" <vkuznets@redhat.com>,
	"mtosatti@redhat.com" <mtosatti@redhat.com>,
	"joro@8bytes.org" <joro@8bytes.org>,
	karahmed@amazon.com
Subject: [RFC PATCH 0/11] Rework gfn_to_pfn_cache
Date: Mon, 15 Nov 2021 16:47:39 +0000	[thread overview]
Message-ID: <95fae9cf56b1a7f0a5f2b9a1934e29e924908ff2.camel@infradead.org> (raw)
In-Reply-To: <7e4b895b-8f36-69cb-10a9-0b4139b9eb79@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 4926 bytes --]

On Fri, 2021-11-12 at 15:56 +0100, Paolo Bonzini wrote:
> > I think we want to kill the struct kvm_host_map completely, merge its
> > extra 'hva' and 'page' fields into the (possibly renamed)
> > gfn_to_pfn_cache along with your 'guest_uses_pa' flag, and take it from
> > there.
> 
> Yes, that makes sense.

OK... here's what I have so far.

I ended up killing the gfn_to_pfn_cache completely in a preliminary
patch, just so I could cleanly untangle it from the implementation of
kvm_vcpu_map()/kvm_vcpu_unmap(). Those want to die too, but they can
die *after* we have converted all their users to something else.

I then reinstate a newly-invented gfn_to_pfn_cache in another commit,
working only for the rwlock case so far because I have questions... :)

So the idea that we discussed is that the user of the gfn_to_pfn_cache
may set the guest_uses_ga flag to indicate that the cached PFN is used
by (e.g.) the VMCS02.

So... a user of this must check the validity after setting its mode to
IN_GUEST_MODE, and the invalidation must make a request and wake any
vCPU(s) which might be using it. As an optimisation, since these are
likely to be single-vCPU only, I added a 'vcpu' field to the cache.

The wakeup (in #if 0 so far) looks a bit like this...

	unsigned int req = KVM_REQ_GPC_INVALIDATE;

	/*
	 * If the OOM reaper is active, then all vCPUs should have been stopped
	 * already, so perform the request without KVM_REQUEST_WAIT and be sad
	 * if anything needed to be woken.
	 */
	if (!may_block)
		req |= ~KVM_REQUEST_WAIT;

	if (wake_all_vcpus) {
		called = kvm_make_all_cpus_request(kvm, req);
	} else if (wake_vcpus) {
		called = kvm_make_vcpus_request_mask(kvm, req, vcpu_bitmap);
	}
	WARN_ON_ONCE(called && !may_block);

I moved the invalidation to the invalidate_range MMU notifier, as
discussed. But that's where the plan falls down a little bit because
IIUC, that one can't sleep at all. I need to move it *back* to
invalidate_range_start() where I had it before, if I want to let it
wait for vCPUs to exit. Which means... that the cache 'refresh' call
must wait until the mmu_notifier_count reaches zero? Am I allowed to do
that, and make the "There can be only one waiter" comment in
kvm_mmu_notifier_invalidate_range_end() no longer true? Or is there a
better way?

I was also pondering whether to introduce a new arch-independent
KVM_REQ_GPC_INVALIDATE, or let it be arch-dependent and make it a field
of the cache, so that users can raise whatever requests they like?

Anyway, this much works for Xen event channel delivery (again) and
looks like it's *most* of the way there for fixing the nested stuff.

The first four or maybe even eight (modulo testing) patches are
probably ready to be merged anyway. The "Maintain valid mapping of Xen
shared_info page" patch is utterly trivial now and eventually I'll
probably fold it into the subsequent patch, but it's left separate for
illustration, for now.

David Woodhouse (11):
      KVM: x86: Fix steal time asm constraints in 32-bit mode
      KVM: x86/xen: Fix get_attr of KVM_XEN_ATTR_TYPE_SHARED_INFO
      KVM: selftests: Add event channel upcall support to xen_shinfo_test
      KVM: x86/xen: Use sizeof_field() instead of open-coding it
      KVM: nVMX: Use kvm_{read,write}_guest_cached() for shadow_vmcs12
      KVM: nVMX: Use kvm_read_guest_offset_cached() for nested VMCS check
      KVM: nVMX: Use a gfn_to_hva_cache for vmptrld
      KVM: Kill kvm_map_gfn() / kvm_unmap_gfn() and gfn_to_pfn_cache
      KVM: Reinstate gfn_to_pfn_cache with invalidation support
      KVM: x86/xen: Maintain valid mapping of Xen shared_info page
      KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event channel delivery

 Documentation/virt/kvm/api.rst                       |  21 ++++++
 arch/x86/include/asm/kvm_host.h                      |   3 +-
 arch/x86/kvm/irq_comm.c                              |  12 ++++
 arch/x86/kvm/vmx/nested.c                            |  76 +++++++++++++---------
 arch/x86/kvm/vmx/vmx.h                               |  10 +++
 arch/x86/kvm/x86.c                                   |   5 +-
 arch/x86/kvm/xen.c                                   | 305 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 arch/x86/kvm/xen.h                                   |   9 +++
 include/linux/kvm_host.h                             |  27 ++++++--
 include/linux/kvm_types.h                            |  13 +++-
 include/uapi/linux/kvm.h                             |  11 ++++
 tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c | 187 +++++++++++++++++++++++++++++++++++++++++++++++++---
 virt/kvm/kvm_main.c                                  | 340 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------
 13 files changed, 862 insertions(+), 157 deletions(-)

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]