Re: [PATCH 14/21] KVM: Move x86's version of struct kvm_mmu_memory_cache to common code

From: Sean Christopherson <sean.j.christopherson@intel.com>
To: Ben Gardon <bgardon@google.com>
Cc: Marc Zyngier <maz@kernel.org>, Paul Mackerras <paulus@ozlabs.org>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Janosch Frank <frankja@linux.ibm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	James Morse <james.morse@arm.com>,
	Julien Thierry <julien.thierry.kdev@gmail.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	David Hildenbrand <david@redhat.com>,
	Cornelia Huck <cohuck@redhat.com>,
	Claudio Imbrenda <imbrenda@linux.ibm.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, linux-mips@vger.kernel.org,
	kvm@vger.kernel.org, kvm-ppc@vger.kernel.org,
	linux-kernel@vger.kernel.org, Peter Feiner <pfeiner@google.com>,
	Peter Shier <pshier@google.com>,
	Junaid Shahid <junaids@google.com>,
	Christoffer Dall <christoffer.dall@arm.com>
Subject: Re: [PATCH 14/21] KVM: Move x86's version of struct kvm_mmu_memory_cache to common code
Date: Mon, 22 Jun 2020 09:57:30 -0700	[thread overview]
Message-ID: <20200622165730.GD5150@linux.intel.com> (raw)
In-Reply-To: <CANgfPd-iH8AShSPPJiaDCxV1H76kfpTOQMZSMP_+nP3LoXbYBg@mail.gmail.com>

On Wed, Jun 10, 2020 at 02:58:21PM -0700, Ben Gardon wrote:
> On Wed, Jun 10, 2020 at 12:01 PM Ben Gardon <bgardon@google.com> wrote:
> > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > > index fb99e6776e27..8e8fea13b6c7 100644
> > > --- a/arch/x86/include/asm/kvm_host.h
> > > +++ b/arch/x86/include/asm/kvm_host.h
> > > @@ -193,8 +193,6 @@ struct x86_exception;
> > >  enum x86_intercept;
> > >  enum x86_intercept_stage;
> > >
> > > -#define KVM_NR_MEM_OBJS 40
> > > -
> Oops I didn't catch this on my first read through, but in patch 16 in
> this series I see some references to KVM_NR_MEM_OBJS being removed. As
> a result I would not expect this patch to build. Other references to
> this value should probably replaced with
> KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE as well.

This patch intentionally uses a different name for the #define (see below)
so that the existing arm64 and MIPS declarations don't get picked up by
common KVM code.  This is required so that arm64 and MIPS continue to use
their versions of the cache implementation until they are converted to the
common implementation later in the series, e.g. in patch 16 when the
references to KVM_NR_MEM_OBJS are removed.

I confirmed the above (after sending v1) by compiling all non-x86 changes
on arm64, MIPS, s390 and PPC to verify that this doesn't break bisection.

> > >  #define KVM_NR_DB_REGS 4
> > >
> > >  #define DR6_BD         (1 << 13)
> > > @@ -245,17 +243,6 @@ enum x86_intercept_stage;
> > >
> > >  struct kvm_kernel_irq_routing_entry;

...

> > > +#ifdef KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE
> > > +/*
> > > + * Memory caches are used to preallocate memory ahead of various MMU flows,
> > > + * e.g. page fault handlers.  Gracefully handling allocation failures deep in
> > > + * MMU flows is problematic, as is triggering reclaim, I/O, etc... while
> > > + * holding MMU locks.  Note, these caches act more like prefetch buffers than
> > > + * classical caches, i.e. objects are not returned to the cache on being freed.
> > > + */
> > > +struct kvm_mmu_memory_cache {
> > > +       int nobjs;
> > > +       gfp_t gfp_zero;
> > > +       struct kmem_cache *kmem_cache;
> > > +       void *objects[KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE];
> > > +};
> > > +#endif
> > > +
> > > +
> > >  #endif /* __KVM_TYPES_H__ */
> > > --
> > > 2.26.0
> > >