Re: [PATCH v2 07/10] PPC, KVM, CMA: use general CMA reserved area management framework

Joonsoo Kim
2014-06-16T05:29:09Z

On Sat, Jun 14, 2014 at 03:35:33PM +0530, Aneesh Kumar K.V wrote:
> Joonsoo Kim <iamjoonsoo.kim@lge.com> writes:
>
> > Now, we have general CMA reserved area management framework,
> > so use it for future maintainabilty. There is no functional change.
> >
> > Acked-by: Michal Nazarewicz <mina86@mina86.com>
> > Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >
> > diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book3s_hv_builtin.c
> > index 7cde8a6..28ec226 100644
> > --- a/arch/powerpc/kvm/book3s_hv_builtin.c
> > +++ b/arch/powerpc/kvm/book3s_hv_builtin.c
> > @@ -16,12 +16,14 @@
> > #include <linux/init.h>
> > #include <linux/memblock.h>
> > #include <linux/sizes.h>
> > +#include <linux/cma.h>
> >
> > #include <asm/cputable.h>
> > #include <asm/kvm_ppc.h>
> > #include <asm/kvm_book3s.h>
> >
> > -#include "book3s_hv_cma.h"
> > +#define KVM_CMA_CHUNK_ORDER 18
> > +
> > /*
> > * Hash page table alignment on newer cpus(CPU_FTR_ARCH_206)
> > * should be power of 2.
> > @@ -43,6 +45,8 @@ static unsigned long kvm_cma_resv_ratio = 5;
> > unsigned long kvm_rma_pages = (1 << 27) >> PAGE_SHIFT; /* 128MB */
> > EXPORT_SYMBOL_GPL(kvm_rma_pages);
> >
> > +static struct cma *kvm_cma;
> > +
> > /* Work out RMLS (real mode limit selector) field value for a given RMA size.
> > Assumes POWER7 or PPC970. */
> > static inline int lpcr_rmls(unsigned long rma_size)
> > @@ -97,7 +101,7 @@ struct kvm_rma_info *kvm_alloc_rma()
> > ri = kmalloc(sizeof(struct kvm_rma_info), GFP_KERNEL);
> > if (!ri)
> > return NULL;
> > - page = kvm_alloc_cma(kvm_rma_pages, kvm_rma_pages);
> > + page = cma_alloc(kvm_cma, kvm_rma_pages, get_order(kvm_rma_pages));
> > if (!page)
> > goto err_out;
> > atomic_set(&ri->use_count, 1);
> > @@ -112,7 +116,7 @@ EXPORT_SYMBOL_GPL(kvm_alloc_rma);
> > void kvm_release_rma(struct kvm_rma_info *ri)
> > {
> > if (atomic_dec_and_test(&ri->use_count)) {
> > - kvm_release_cma(pfn_to_page(ri->base_pfn), kvm_rma_pages);
> > + cma_release(kvm_cma, pfn_to_page(ri->base_pfn), kvm_rma_pages);
> > kfree(ri);
> > }
> > }
> > @@ -134,13 +138,13 @@ struct page *kvm_alloc_hpt(unsigned long nr_pages)
> > /* Old CPUs require HPT aligned on a multiple of its size */
> > if (!cpu_has_feature(CPU_FTR_ARCH_206))
> > align_pages = nr_pages;
> > - return kvm_alloc_cma(nr_pages, align_pages);
> > + return cma_alloc(kvm_cma, nr_pages, get_order(align_pages));
> > }
> > EXPORT_SYMBOL_GPL(kvm_alloc_hpt);
> >
> > void kvm_release_hpt(struct page *page, unsigned long nr_pages)
> > {
> > - kvm_release_cma(page, nr_pages);
> > + cma_release(kvm_cma, page, nr_pages);
> > }
> > EXPORT_SYMBOL_GPL(kvm_release_hpt);
> >
> > @@ -179,7 +183,8 @@ void __init kvm_cma_reserve(void)
> > align_size = HPT_ALIGN_PAGES << PAGE_SHIFT;
> >
> > align_size = max(kvm_rma_pages << PAGE_SHIFT, align_size);
> > - kvm_cma_declare_contiguous(selected_size, align_size);
> > + cma_declare_contiguous(selected_size, 0, 0, align_size,
> > + KVM_CMA_CHUNK_ORDER - PAGE_SHIFT, &kvm_cma, false);
> > }
> > }
> >
> > diff --git a/arch/powerpc/kvm/book3s_hv_cma.c b/arch/powerpc/kvm/book3s_hv_cma.c
> > deleted file mode 100644
> > index d9d3d85..0000000
> > --- a/arch/powerpc/kvm/book3s_hv_cma.c
> > +++ /dev/null
> > @@ -1,240 +0,0 @@
> > -/*
> > - * Contiguous Memory Allocator for ppc KVM hash pagetable based on CMA
> > - * for DMA mapping framework
> > - *
> > - * Copyright IBM Corporation, 2013
> > - * Author Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > - *
> > - * This program is free software; you can redistribute it and/or
> > - * modify it under the terms of the GNU General Public License as
> > - * published by the Free Software Foundation; either version 2 of the
> > - * License or (at your optional) any later version of the license.
> > - *
> > - */
> > -#define pr_fmt(fmt) "kvm_cma: " fmt
> > -
> > -#ifdef CONFIG_CMA_DEBUG
> > -#ifndef DEBUG
> > -# define DEBUG
> > -#endif
> > -#endif
> > -
> > -#include <linux/memblock.h>
> > -#include <linux/mutex.h>
> > -#include <linux/sizes.h>
> > -#include <linux/slab.h>
> > -
> > -#include "book3s_hv_cma.h"
> > -
> > -struct kvm_cma {
> > - unsigned long base_pfn;
> > - unsigned long count;
> > - unsigned long *bitmap;
> > -};
> > -
> > -static DEFINE_MUTEX(kvm_cma_mutex);
> > -static struct kvm_cma kvm_cma_area;
> > -
> > -/**
> > - * kvm_cma_declare_contiguous() - reserve area for contiguous memory handling
> > - * for kvm hash pagetable
> > - * @size: Size of the reserved memory.
> > - * @alignment: Alignment for the contiguous memory area
> > - *
> > - * This function reserves memory for kvm cma area. It should be
> > - * called by arch code when early allocator (memblock or bootmem)
> > - * is still activate.
> > - */
> > -long __init kvm_cma_declare_contiguous(phys_addr_t size, phys_addr_t alignment)
> > -{
> > - long base_pfn;
> > - phys_addr_t addr;
> > - struct kvm_cma *cma = &kvm_cma_area;
> > -
> > - pr_debug("%s(size %lx)\n", __func__, (unsigned long)size);
> > -
> > - if (!size)
> > - return -EINVAL;
> > - /*
> > - * Sanitise input arguments.
> > - * We should be pageblock aligned for CMA.
> > - */
> > - alignment = max(alignment, (phys_addr_t)(PAGE_SIZE << pageblock_order));
> > - size = ALIGN(size, alignment);
> > - /*
> > - * Reserve memory
> > - * Use __memblock_alloc_base() since
> > - * memblock_alloc_base() panic()s.
> > - */
> > - addr = __memblock_alloc_base(size, alignment, 0);
> > - if (!addr) {
> > - base_pfn = -ENOMEM;
> > - goto err;
> > - } else
> > - base_pfn = PFN_DOWN(addr);
> > -
> > - /*
> > - * Each reserved area must be initialised later, when more kernel
> > - * subsystems (like slab allocator) are available.
> > - */
> > - cma->base_pfn = base_pfn;
> > - cma->count = size >> PAGE_SHIFT;
> > - pr_info("CMA: reserved %ld MiB\n", (unsigned long)size / SZ_1M);
> > - return 0;
> > -err:
> > - pr_err("CMA: failed to reserve %ld MiB\n", (unsigned long)size / SZ_1M);
> > - return base_pfn;
> > -}
> > -
> > -/**
> > - * kvm_alloc_cma() - allocate pages from contiguous area
> > - * @nr_pages: Requested number of pages.
> > - * @align_pages: Requested alignment in number of pages
> > - *
> > - * This function allocates memory buffer for hash pagetable.
> > - */
> > -struct page *kvm_alloc_cma(unsigned long nr_pages, unsigned long align_pages)
> > -{
> > - int ret;
> > - struct page *page = NULL;
> > - struct kvm_cma *cma = &kvm_cma_area;
> > - unsigned long chunk_count, nr_chunk;
> > - unsigned long mask, pfn, pageno, start = 0;
> > -
> > -
> > - if (!cma || !cma->count)
> > - return NULL;
> > -
> > - pr_debug("%s(cma %p, count %lu, align pages %lu)\n", __func__,
> > - (void *)cma, nr_pages, align_pages);
> > -
> > - if (!nr_pages)
> > - return NULL;
> > - /*
> > - * align mask with chunk size. The bit tracks pages in chunk size
> > - */
> > - VM_BUG_ON(!is_power_of_2(align_pages));
> > - mask = (align_pages >> (KVM_CMA_CHUNK_ORDER - PAGE_SHIFT)) - 1;
> > - BUILD_BUG_ON(PAGE_SHIFT > KVM_CMA_CHUNK_ORDER);
> > -
> > - chunk_count = cma->count >> (KVM_CMA_CHUNK_ORDER - PAGE_SHIFT);
> > - nr_chunk = nr_pages >> (KVM_CMA_CHUNK_ORDER - PAGE_SHIFT);
> > -
> > - mutex_lock(&kvm_cma_mutex);
> > - for (;;) {
> > - pageno = bitmap_find_next_zero_area(cma->bitmap, chunk_count,
> > - start, nr_chunk, mask);
> > - if (pageno >= chunk_count)
> > - break;
> > -
> > - pfn = cma->base_pfn + (pageno << (KVM_CMA_CHUNK_ORDER - PAGE_SHIFT));
> > - ret = alloc_contig_range(pfn, pfn + nr_pages, MIGRATE_CMA);
> > - if (ret == 0) {
> > - bitmap_set(cma->bitmap, pageno, nr_chunk);
> > - page = pfn_to_page(pfn);
> > - memset(pfn_to_kaddr(pfn), 0, nr_pages <<
> > PAGE_SHIFT);
>
>
> The above memset is missing in the generic code. May be
>
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index 6f13ee6..8740b4c 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -75,6 +75,7 @@ long kvmppc_alloc_hpt(struct kvm *kvm, u32 *htab_orderp)
> page = kvm_alloc_hpt(1 << (order - PAGE_SHIFT));
> if (page) {
> hpt = (unsigned long)pfn_to_kaddr(page_to_pfn(page));
> + memset((void *)hpt, 0, (1 << order));
> kvm->arch.hpt_cma_alloc = 1;
> } else
> --order;
>
>
>
> With that
>
> Tested-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Okay. Will do.

Really thanks for testing!!

Thanks.