On Mon, May 16, 2022 at 4:24 PM David Matlack <dmatlack@xxxxxxxxxx> wrote: > > Allow the capacity of the kvm_mmu_memory_cache struct to be chosen at > declaration time rather than being fixed for all declarations. This will > be used in a follow-up commit to declare an cache in x86 with a capacity > of 512+ objects without having to increase the capacity of all caches in > KVM. > > This change requires each cache now specify its capacity at runtime, > since the cache struct itself no longer has a fixed capacity known at > compile time. To protect against someone accidentally defining a > kvm_mmu_memory_cache struct directly (without the extra storage), this > commit includes a WARN_ON() in kvm_mmu_topup_memory_cache(). > > In order to support different capacities, this commit changes the > objects pointer array to be dynamically allocated the first time the > cache is topped-up. > > While here, opportunistically clean up the stack-allocated > kvm_mmu_memory_cache structs in riscv and arm64 to use designated > initializers. > > No functional change intended. > > Reviewed-by: Marc Zyngier <maz@xxxxxxxxxx> > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx> > --- > arch/arm64/kvm/mmu.c | 2 +- > arch/riscv/kvm/mmu.c | 5 +---- > include/linux/kvm_types.h | 6 +++++- > virt/kvm/kvm_main.c | 33 ++++++++++++++++++++++++++++++--- > 4 files changed, 37 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c > index 53ae2c0640bc..f443ed845f85 100644 > --- a/arch/arm64/kvm/mmu.c > +++ b/arch/arm64/kvm/mmu.c > @@ -764,7 +764,7 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa, > { > phys_addr_t addr; > int ret = 0; > - struct kvm_mmu_memory_cache cache = { 0, __GFP_ZERO, NULL, }; > + struct kvm_mmu_memory_cache cache = { .gfp_zero = __GFP_ZERO }; > struct kvm_pgtable *pgt = kvm->arch.mmu.pgt; > enum kvm_pgtable_prot prot = KVM_PGTABLE_PROT_DEVICE | > KVM_PGTABLE_PROT_R | > diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c > index f80a34fbf102..4d95ebe4114f 100644 > --- a/arch/riscv/kvm/mmu.c > +++ b/arch/riscv/kvm/mmu.c > @@ -347,10 +347,7 @@ static int stage2_ioremap(struct kvm *kvm, gpa_t gpa, phys_addr_t hpa, > int ret = 0; > unsigned long pfn; > phys_addr_t addr, end; > - struct kvm_mmu_memory_cache pcache; > - > - memset(&pcache, 0, sizeof(pcache)); > - pcache.gfp_zero = __GFP_ZERO; > + struct kvm_mmu_memory_cache pcache = { .gfp_zero = __GFP_ZERO }; > > end = (gpa + size + PAGE_SIZE - 1) & PAGE_MASK; > pfn = __phys_to_pfn(hpa); > diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h > index ac1ebb37a0ff..68529884eaf8 100644 > --- a/include/linux/kvm_types.h > +++ b/include/linux/kvm_types.h > @@ -83,12 +83,16 @@ struct gfn_to_pfn_cache { > * 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. > + * > + * The @capacity field and @objects array are lazily initialized when the cache > + * is topped up (__kvm_mmu_topup_memory_cache()). > */ > 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]; > + int capacity; > + void **objects; > }; > #endif > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index e089db822c12..5e2e75014256 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -369,14 +369,31 @@ static inline void *mmu_memory_cache_alloc_obj(struct kvm_mmu_memory_cache *mc, > return (void *)__get_free_page(gfp_flags); > } > > -int kvm_mmu_topup_memory_cache(struct kvm_mmu_memory_cache *mc, int min) > +static int __kvm_mmu_topup_memory_cache(struct kvm_mmu_memory_cache *mc, int capacity, int min) > { > + gfp_t gfp = GFP_KERNEL_ACCOUNT; > void *obj; > > if (mc->nobjs >= min) > return 0; > - while (mc->nobjs < ARRAY_SIZE(mc->objects)) { > - obj = mmu_memory_cache_alloc_obj(mc, GFP_KERNEL_ACCOUNT); > + > + if (unlikely(!mc->objects)) { > + if (WARN_ON_ONCE(!capacity)) > + return -EIO; > + > + mc->objects = kvmalloc_array(sizeof(void *), capacity, gfp); > + if (!mc->objects) > + return -ENOMEM; > + > + mc->capacity = capacity; Do we want to ensure the minimum value of the capacity? I think otherwise, we may more likely start using memory from GFP_ATOMIC if the capacity is less than, say 5? But the minimum value seems related to each cache type. > + } > + > + /* It is illegal to request a different capacity across topups. */ > + if (WARN_ON_ONCE(mc->capacity != capacity)) > + return -EIO; > + > + while (mc->nobjs < mc->capacity) { > + obj = mmu_memory_cache_alloc_obj(mc, gfp); > if (!obj) > return mc->nobjs >= min ? 0 : -ENOMEM; > mc->objects[mc->nobjs++] = obj; > @@ -384,6 +401,11 @@ int kvm_mmu_topup_memory_cache(struct kvm_mmu_memory_cache *mc, int min) > return 0; > } > > +int kvm_mmu_topup_memory_cache(struct kvm_mmu_memory_cache *mc, int min) > +{ > + return __kvm_mmu_topup_memory_cache(mc, KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE, min); > +} > + > int kvm_mmu_memory_cache_nr_free_objects(struct kvm_mmu_memory_cache *mc) > { > return mc->nobjs; > @@ -397,6 +419,11 @@ void kvm_mmu_free_memory_cache(struct kvm_mmu_memory_cache *mc) > else > free_page((unsigned long)mc->objects[--mc->nobjs]); > } > + > + kvfree(mc->objects); > + > + mc->objects = NULL; > + mc->capacity = 0; > } > > void *kvm_mmu_memory_cache_alloc(struct kvm_mmu_memory_cache *mc) > -- > 2.36.0.550.gb090851708-goog >