Re: [PATCH v2 04/10] DMA, CMA: support alignment constraint on cma region

2014-06-12 Thread Joonsoo Kim
On Thu, Jun 12, 2014 at 02:52:20PM +0900, Minchan Kim wrote:
 On Thu, Jun 12, 2014 at 12:21:41PM +0900, Joonsoo Kim wrote:
  ppc kvm's cma area management needs alignment constraint on
  cma region. So support it to prepare generalization of cma area
  management functionality.
  
  Additionally, add some comments which tell us why alignment
  constraint is needed on cma region.
  
  Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
  
  diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
  index 8a44c82..bc4c171 100644
  --- a/drivers/base/dma-contiguous.c
  +++ b/drivers/base/dma-contiguous.c
  @@ -32,6 +32,7 @@
   #include linux/swap.h
   #include linux/mm_types.h
   #include linux/dma-contiguous.h
  +#include linux/log2.h
   
   struct cma {
  unsigned long   base_pfn;
  @@ -219,6 +220,7 @@ core_initcall(cma_init_reserved_areas);
* @size: Size of the reserved area (in bytes),
* @base: Base address of the reserved area optional, use 0 for any
* @limit: End address of the reserved memory (optional, 0 for any).
  + * @alignment: Alignment for the contiguous memory area, should be power 
  of 2
* @res_cma: Pointer to store the created cma region.
* @fixed: hint about where to place the reserved area
*
 
 Pz, move the all description to new API function rather than internal one.

Reason I leave all description as is is that I will remove it in
following patch. I think that moving these makes patch bigger and hard
to review.

But, if it is necessary, I will do it. :)

 
  @@ -233,15 +235,15 @@ core_initcall(cma_init_reserved_areas);
*/
   static int __init __dma_contiguous_reserve_area(phys_addr_t size,
  phys_addr_t base, phys_addr_t limit,
  +   phys_addr_t alignment,
  struct cma **res_cma, bool fixed)
   {
  struct cma *cma = cma_areas[cma_area_count];
  -   phys_addr_t alignment;
  int ret = 0;
   
  -   pr_debug(%s(size %lx, base %08lx, limit %08lx)\n, __func__,
  -(unsigned long)size, (unsigned long)base,
  -(unsigned long)limit);
  +   pr_debug(%s(size %lx, base %08lx, limit %08lx align_order %08lx)\n,
 
 Why is it called by align_order?

Oops... mistake.
I will fix it.

Thanks.

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity

2014-06-12 Thread Minchan Kim
On Thu, Jun 12, 2014 at 12:21:42PM +0900, Joonsoo Kim wrote:
 ppc kvm's cma region management requires arbitrary bitmap granularity,
 since they want to reserve very large memory and manage this region
 with bitmap that one bit for several pages to reduce management overheads.
 So support arbitrary bitmap granularity for following generalization.
 
 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
 
 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index bc4c171..9bc9340 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -38,6 +38,7 @@ struct cma {
   unsigned long   base_pfn;
   unsigned long   count;
   unsigned long   *bitmap;
 + int order_per_bit; /* Order of pages represented by one bit */

Hmm, I'm not sure it's good as *general* interface even though it covers
existing usecases.

It forces a cma area should be handled by same size unit. Right?
It's really important point for this patchset's motivation so I will stop
review and wait other opinions.

   struct mutexlock;
  };
  
 @@ -157,9 +158,38 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
  
  static DEFINE_MUTEX(cma_mutex);
  
 +static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int 
 align_order)
 +{
 + return (1  (align_order  cma-order_per_bit)) - 1;
 +}
 +
 +static unsigned long cma_bitmap_maxno(struct cma *cma)
 +{
 + return cma-count  cma-order_per_bit;
 +}
 +
 +static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
 + unsigned long pages)
 +{
 + return ALIGN(pages, 1  cma-order_per_bit)  cma-order_per_bit;
 +}
 +
 +static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
 +{
 + unsigned long bitmapno, nr_bits;
 +
 + bitmapno = (pfn - cma-base_pfn)  cma-order_per_bit;
 + nr_bits = cma_bitmap_pages_to_bits(cma, count);
 +
 + mutex_lock(cma-lock);
 + bitmap_clear(cma-bitmap, bitmapno, nr_bits);
 + mutex_unlock(cma-lock);
 +}
 +
  static int __init cma_activate_area(struct cma *cma)
  {
 - int bitmap_size = BITS_TO_LONGS(cma-count) * sizeof(long);
 + int bitmap_maxno = cma_bitmap_maxno(cma);
 + int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
   unsigned long base_pfn = cma-base_pfn, pfn = base_pfn;
   unsigned i = cma-count  pageblock_order;
   struct zone *zone;
 @@ -221,6 +251,7 @@ core_initcall(cma_init_reserved_areas);
   * @base: Base address of the reserved area optional, use 0 for any
   * @limit: End address of the reserved memory (optional, 0 for any).
   * @alignment: Alignment for the contiguous memory area, should be power of 2
 + * @order_per_bit: Order of pages represented by one bit on bitmap.
   * @res_cma: Pointer to store the created cma region.
   * @fixed: hint about where to place the reserved area
   *
 @@ -235,7 +266,7 @@ core_initcall(cma_init_reserved_areas);
   */
  static int __init __dma_contiguous_reserve_area(phys_addr_t size,
   phys_addr_t base, phys_addr_t limit,
 - phys_addr_t alignment,
 + phys_addr_t alignment, int order_per_bit,
   struct cma **res_cma, bool fixed)
  {
   struct cma *cma = cma_areas[cma_area_count];
 @@ -269,6 +300,8 @@ static int __init 
 __dma_contiguous_reserve_area(phys_addr_t size,
   base = ALIGN(base, alignment);
   size = ALIGN(size, alignment);
   limit = ~(alignment - 1);
 + /* size should be aligned with order_per_bit */
 + BUG_ON(!IS_ALIGNED(size  PAGE_SHIFT, 1  order_per_bit));
  
   /* Reserve memory */
   if (base  fixed) {
 @@ -294,6 +327,7 @@ static int __init 
 __dma_contiguous_reserve_area(phys_addr_t size,
*/
   cma-base_pfn = PFN_DOWN(base);
   cma-count = size  PAGE_SHIFT;
 + cma-order_per_bit = order_per_bit;
   *res_cma = cma;
   cma_area_count++;
  
 @@ -313,7 +347,7 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
  {
   int ret;
  
 - ret = __dma_contiguous_reserve_area(size, base, limit, 0,
 + ret = __dma_contiguous_reserve_area(size, base, limit, 0, 0,
   res_cma, fixed);
   if (ret)
   return ret;
 @@ -324,13 +358,6 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
   return 0;
  }
  
 -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
 -{
 - mutex_lock(cma-lock);
 - bitmap_clear(cma-bitmap, pfn - cma-base_pfn, count);
 - mutex_unlock(cma-lock);
 -}
 -
  /**
   * dma_alloc_from_contiguous() - allocate pages from contiguous area
   * @dev:   Pointer to device for which the allocation is performed.
 @@ -345,7 +372,8 @@ static void clear_cma_bitmap(struct cma *cma, unsigned 
 long pfn, int count)
  static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
  

Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity

2014-06-12 Thread Joonsoo Kim
On Thu, Jun 12, 2014 at 03:06:10PM +0900, Minchan Kim wrote:
 On Thu, Jun 12, 2014 at 12:21:42PM +0900, Joonsoo Kim wrote:
  ppc kvm's cma region management requires arbitrary bitmap granularity,
  since they want to reserve very large memory and manage this region
  with bitmap that one bit for several pages to reduce management overheads.
  So support arbitrary bitmap granularity for following generalization.
  
  Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
  
  diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
  index bc4c171..9bc9340 100644
  --- a/drivers/base/dma-contiguous.c
  +++ b/drivers/base/dma-contiguous.c
  @@ -38,6 +38,7 @@ struct cma {
  unsigned long   base_pfn;
  unsigned long   count;
  unsigned long   *bitmap;
  +   int order_per_bit; /* Order of pages represented by one bit */
 
 Hmm, I'm not sure it's good as *general* interface even though it covers
 existing usecases.
 
 It forces a cma area should be handled by same size unit. Right?
 It's really important point for this patchset's motivation so I will stop
 review and wait other opinions.

If you pass 0 to order_per_bit, you can manage cma area in every
size(page unit) you want. If you pass certain number to order_per_bit,
you can allocate and release cma area in multiple of such page order.

I think that this is more general implementation than previous versions.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity

2014-06-12 Thread Minchan Kim
On Thu, Jun 12, 2014 at 03:43:55PM +0900, Joonsoo Kim wrote:
 On Thu, Jun 12, 2014 at 03:06:10PM +0900, Minchan Kim wrote:
  On Thu, Jun 12, 2014 at 12:21:42PM +0900, Joonsoo Kim wrote:
   ppc kvm's cma region management requires arbitrary bitmap granularity,
   since they want to reserve very large memory and manage this region
   with bitmap that one bit for several pages to reduce management overheads.
   So support arbitrary bitmap granularity for following generalization.
   
   Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
   
   diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
   index bc4c171..9bc9340 100644
   --- a/drivers/base/dma-contiguous.c
   +++ b/drivers/base/dma-contiguous.c
   @@ -38,6 +38,7 @@ struct cma {
 unsigned long   base_pfn;
 unsigned long   count;
 unsigned long   *bitmap;
   + int order_per_bit; /* Order of pages represented by one bit */
  
  Hmm, I'm not sure it's good as *general* interface even though it covers
  existing usecases.
  
  It forces a cma area should be handled by same size unit. Right?
  It's really important point for this patchset's motivation so I will stop
  review and wait other opinions.
 
 If you pass 0 to order_per_bit, you can manage cma area in every
 size(page unit) you want. If you pass certain number to order_per_bit,
 you can allocate and release cma area in multiple of such page order.
 
 I think that this is more general implementation than previous versions.

Fair enough.

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity

2014-06-12 Thread Minchan Kim
On Thu, Jun 12, 2014 at 12:21:42PM +0900, Joonsoo Kim wrote:
 ppc kvm's cma region management requires arbitrary bitmap granularity,
 since they want to reserve very large memory and manage this region
 with bitmap that one bit for several pages to reduce management overheads.
 So support arbitrary bitmap granularity for following generalization.
 
 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
Acked-by: Minchan Kim minc...@kernel.org

Just a nit below.

 
 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index bc4c171..9bc9340 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -38,6 +38,7 @@ struct cma {
   unsigned long   base_pfn;
   unsigned long   count;
   unsigned long   *bitmap;
 + int order_per_bit; /* Order of pages represented by one bit */
   struct mutexlock;
  };
  
 @@ -157,9 +158,38 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
  
  static DEFINE_MUTEX(cma_mutex);
  
 +static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int 
 align_order)
 +{
 + return (1  (align_order  cma-order_per_bit)) - 1;
 +}
 +
 +static unsigned long cma_bitmap_maxno(struct cma *cma)
 +{
 + return cma-count  cma-order_per_bit;
 +}
 +
 +static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
 + unsigned long pages)
 +{
 + return ALIGN(pages, 1  cma-order_per_bit)  cma-order_per_bit;
 +}
 +
 +static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
 +{
 + unsigned long bitmapno, nr_bits;
 +
 + bitmapno = (pfn - cma-base_pfn)  cma-order_per_bit;
 + nr_bits = cma_bitmap_pages_to_bits(cma, count);
 +
 + mutex_lock(cma-lock);
 + bitmap_clear(cma-bitmap, bitmapno, nr_bits);
 + mutex_unlock(cma-lock);
 +}
 +
  static int __init cma_activate_area(struct cma *cma)
  {
 - int bitmap_size = BITS_TO_LONGS(cma-count) * sizeof(long);
 + int bitmap_maxno = cma_bitmap_maxno(cma);
 + int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
   unsigned long base_pfn = cma-base_pfn, pfn = base_pfn;
   unsigned i = cma-count  pageblock_order;
   struct zone *zone;
 @@ -221,6 +251,7 @@ core_initcall(cma_init_reserved_areas);
   * @base: Base address of the reserved area optional, use 0 for any
   * @limit: End address of the reserved memory (optional, 0 for any).
   * @alignment: Alignment for the contiguous memory area, should be power of 2
 + * @order_per_bit: Order of pages represented by one bit on bitmap.
   * @res_cma: Pointer to store the created cma region.
   * @fixed: hint about where to place the reserved area
   *
 @@ -235,7 +266,7 @@ core_initcall(cma_init_reserved_areas);
   */
  static int __init __dma_contiguous_reserve_area(phys_addr_t size,
   phys_addr_t base, phys_addr_t limit,
 - phys_addr_t alignment,
 + phys_addr_t alignment, int order_per_bit,
   struct cma **res_cma, bool fixed)
  {
   struct cma *cma = cma_areas[cma_area_count];
 @@ -269,6 +300,8 @@ static int __init 
 __dma_contiguous_reserve_area(phys_addr_t size,
   base = ALIGN(base, alignment);
   size = ALIGN(size, alignment);
   limit = ~(alignment - 1);
 + /* size should be aligned with order_per_bit */
 + BUG_ON(!IS_ALIGNED(size  PAGE_SHIFT, 1  order_per_bit));
  
   /* Reserve memory */
   if (base  fixed) {
 @@ -294,6 +327,7 @@ static int __init 
 __dma_contiguous_reserve_area(phys_addr_t size,
*/
   cma-base_pfn = PFN_DOWN(base);
   cma-count = size  PAGE_SHIFT;
 + cma-order_per_bit = order_per_bit;
   *res_cma = cma;
   cma_area_count++;
  
 @@ -313,7 +347,7 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
  {
   int ret;
  
 - ret = __dma_contiguous_reserve_area(size, base, limit, 0,
 + ret = __dma_contiguous_reserve_area(size, base, limit, 0, 0,
   res_cma, fixed);
   if (ret)
   return ret;
 @@ -324,13 +358,6 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
   return 0;
  }
  
 -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
 -{
 - mutex_lock(cma-lock);
 - bitmap_clear(cma-bitmap, pfn - cma-base_pfn, count);
 - mutex_unlock(cma-lock);
 -}
 -
  /**
   * dma_alloc_from_contiguous() - allocate pages from contiguous area
   * @dev:   Pointer to device for which the allocation is performed.
 @@ -345,7 +372,8 @@ static void clear_cma_bitmap(struct cma *cma, unsigned 
 long pfn, int count)
  static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
  unsigned int align)
  {
 - unsigned long mask, pfn, pageno, start = 0;
 + unsigned long mask, pfn, start = 0;
 + unsigned long bitmap_maxno, bitmapno, 

Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality

2014-06-12 Thread Minchan Kim
On Thu, Jun 12, 2014 at 12:21:43PM +0900, Joonsoo Kim wrote:
 Currently, there are two users on CMA functionality, one is the DMA
 subsystem and the other is the kvm on powerpc. They have their own code
 to manage CMA reserved area even if they looks really similar.
 From my guess, it is caused by some needs on bitmap management. Kvm side
 wants to maintain bitmap not for 1 page, but for more size. Eventually it
 use bitmap where one bit represents 64 pages.
 
 When I implement CMA related patches, I should change those two places
 to apply my change and it seem to be painful to me. I want to change
 this situation and reduce future code management overhead through
 this patch.
 
 This change could also help developer who want to use CMA in their
 new feature development, since they can use CMA easily without
 copying  pasting this reserved area management code.
 
 In previous patches, we have prepared some features to generalize
 CMA reserved area management and now it's time to do it. This patch
 moves core functions to mm/cma.c and change DMA APIs to use
 these functions.
 
 There is no functional change in DMA APIs.
 
 v2: There is no big change from v1 in mm/cma.c. Mostly renaming.
 
 Acked-by: Michal Nazarewicz min...@mina86.com
 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

Acutally, I want to remove bool return of cma_release but it's not
a out of scope in this patchset.

Acked-by: Minchan Kim minc...@kernel.org

 
 diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
 index 00e13ce..4eac559 100644
 --- a/drivers/base/Kconfig
 +++ b/drivers/base/Kconfig
 @@ -283,16 +283,6 @@ config CMA_ALIGNMENT
  
 If unsure, leave the default value 8.
  
 -config CMA_AREAS
 - int Maximum count of the CMA device-private areas
 - default 7
 - help
 -   CMA allows to create CMA areas for particular devices. This parameter
 -   sets the maximum number of such device private CMA areas in the
 -   system.
 -
 -   If unsure, leave the default value 7.
 -
  endif
  
  endmenu
 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index 9bc9340..f177f73 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -24,25 +24,10 @@
  
  #include linux/memblock.h
  #include linux/err.h
 -#include linux/mm.h
 -#include linux/mutex.h
 -#include linux/page-isolation.h
  #include linux/sizes.h
 -#include linux/slab.h
 -#include linux/swap.h
 -#include linux/mm_types.h
  #include linux/dma-contiguous.h
  #include linux/log2.h

Should we remain log2.h in here?

 -
 -struct cma {
 - unsigned long   base_pfn;
 - unsigned long   count;
 - unsigned long   *bitmap;
 - int order_per_bit; /* Order of pages represented by one bit */
 - struct mutexlock;
 -};
 -
 -struct cma *dma_contiguous_default_area;
 +#include linux/cma.h
  
  #ifdef CONFIG_CMA_SIZE_MBYTES
  #define CMA_SIZE_MBYTES CONFIG_CMA_SIZE_MBYTES
 @@ -50,6 +35,8 @@ struct cma *dma_contiguous_default_area;
  #define CMA_SIZE_MBYTES 0
  #endif
  
 +struct cma *dma_contiguous_default_area;
 +
  /*
   * Default global CMA area size can be defined in kernel's .config.
   * This is useful mainly for distro maintainers to create a kernel
 @@ -156,199 +143,13 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
   }
  }
  
 -static DEFINE_MUTEX(cma_mutex);
 -
 -static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int 
 align_order)
 -{
 - return (1  (align_order  cma-order_per_bit)) - 1;
 -}
 -
 -static unsigned long cma_bitmap_maxno(struct cma *cma)
 -{
 - return cma-count  cma-order_per_bit;
 -}
 -
 -static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
 - unsigned long pages)
 -{
 - return ALIGN(pages, 1  cma-order_per_bit)  cma-order_per_bit;
 -}
 -
 -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
 -{
 - unsigned long bitmapno, nr_bits;
 -
 - bitmapno = (pfn - cma-base_pfn)  cma-order_per_bit;
 - nr_bits = cma_bitmap_pages_to_bits(cma, count);
 -
 - mutex_lock(cma-lock);
 - bitmap_clear(cma-bitmap, bitmapno, nr_bits);
 - mutex_unlock(cma-lock);
 -}
 -
 -static int __init cma_activate_area(struct cma *cma)
 -{
 - int bitmap_maxno = cma_bitmap_maxno(cma);
 - int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
 - unsigned long base_pfn = cma-base_pfn, pfn = base_pfn;
 - unsigned i = cma-count  pageblock_order;
 - struct zone *zone;
 -
 - pr_debug(%s()\n, __func__);
 -
 - cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
 - if (!cma-bitmap)
 - return -ENOMEM;
 -
 - WARN_ON_ONCE(!pfn_valid(pfn));
 - zone = page_zone(pfn_to_page(pfn));
 -
 - do {
 - unsigned j;
 - base_pfn = pfn;
 - for (j = pageblock_nr_pages; j; --j, pfn++) {
 - WARN_ON_ONCE(!pfn_valid(pfn));
 - /*
 -  * 

Re: [PATCH v2 08/10] mm, cma: clean-up cma allocation error path

2014-06-12 Thread Minchan Kim
On Thu, Jun 12, 2014 at 12:21:45PM +0900, Joonsoo Kim wrote:
 We can remove one call sites for clear_cma_bitmap() if we first
 call it before checking error number.
 
 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
Acked-by: Minchan Kim minc...@kernel.org

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 09/10] mm, cma: move output param to the end of param list

2014-06-12 Thread Minchan Kim
On Thu, Jun 12, 2014 at 12:21:46PM +0900, Joonsoo Kim wrote:
 Conventionally, we put output param to the end of param list.
 cma_declare_contiguous() doesn't look like that, so change it.

If you says Conventionally, I'd like to suggest one more thing.
Conventionally, we put 'base' ahead 'size' but dma_contiguous_reserve_area
is opposite.

 
 Additionally, move down cma_areas reference code to the position
 where it is really needed.
 
 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
 
 diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c 
 b/arch/powerpc/kvm/book3s_hv_builtin.c
 index 28ec226..97613ea 100644
 --- a/arch/powerpc/kvm/book3s_hv_builtin.c
 +++ b/arch/powerpc/kvm/book3s_hv_builtin.c
 @@ -184,7 +184,7 @@ void __init kvm_cma_reserve(void)
  
   align_size = max(kvm_rma_pages  PAGE_SHIFT, align_size);
   cma_declare_contiguous(selected_size, 0, 0, align_size,
 - KVM_CMA_CHUNK_ORDER - PAGE_SHIFT, kvm_cma, false);
 + KVM_CMA_CHUNK_ORDER - PAGE_SHIFT, false, kvm_cma);
   }
  }
  
 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index f177f73..bfd4553 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -149,7 +149,7 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
  {
   int ret;
  
 - ret = cma_declare_contiguous(size, base, limit, 0, 0, res_cma, fixed);
 + ret = cma_declare_contiguous(size, base, limit, 0, 0, fixed, res_cma);
   if (ret)
   return ret;
  
 diff --git a/include/linux/cma.h b/include/linux/cma.h
 index e38efe9..e53eead 100644
 --- a/include/linux/cma.h
 +++ b/include/linux/cma.h
 @@ -6,7 +6,7 @@ struct cma;
  extern int __init cma_declare_contiguous(phys_addr_t size,
   phys_addr_t base, phys_addr_t limit,
   phys_addr_t alignment, int order_per_bit,
 - struct cma **res_cma, bool fixed);
 + bool fixed, struct cma **res_cma);
  extern struct page *cma_alloc(struct cma *cma, int count, unsigned int 
 align);
  extern bool cma_release(struct cma *cma, struct page *pages, int count);
  #endif
 diff --git a/mm/cma.c b/mm/cma.c
 index 01a0713..22a5b23 100644
 --- a/mm/cma.c
 +++ b/mm/cma.c
 @@ -142,8 +142,8 @@ core_initcall(cma_init_reserved_areas);
   * @limit: End address of the reserved memory (optional, 0 for any).
   * @alignment: Alignment for the contiguous memory area, should be power of 2
   * @order_per_bit: Order of pages represented by one bit on bitmap.
 - * @res_cma: Pointer to store the created cma region.
   * @fixed: hint about where to place the reserved area
 + * @res_cma: Pointer to store the created cma region.
   *
   * This function reserves memory from early allocator. It should be
   * called by arch specific code once the early allocator (memblock or 
 bootmem)
 @@ -156,9 +156,9 @@ core_initcall(cma_init_reserved_areas);
  int __init cma_declare_contiguous(phys_addr_t size,
   phys_addr_t base, phys_addr_t limit,
   phys_addr_t alignment, int order_per_bit,
 - struct cma **res_cma, bool fixed)
 + bool fixed, struct cma **res_cma)
  {
 - struct cma *cma = cma_areas[cma_area_count];
 + struct cma *cma;
   int ret = 0;
  
   pr_debug(%s(size %lx, base %08lx, limit %08lx alignment %08lx)\n,
 @@ -214,6 +214,7 @@ int __init cma_declare_contiguous(phys_addr_t size,
* Each reserved area must be initialised later, when more kernel
* subsystems (like slab allocator) are available.
*/
 + cma = cma_areas[cma_area_count];
   cma-base_pfn = PFN_DOWN(base);
   cma-count = size  PAGE_SHIFT;
   cma-order_per_bit = order_per_bit;
 -- 
 1.7.9.5

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity

2014-06-12 Thread Zhang Yanfei
On 06/12/2014 03:08 PM, Minchan Kim wrote:
 On Thu, Jun 12, 2014 at 12:21:42PM +0900, Joonsoo Kim wrote:
 ppc kvm's cma region management requires arbitrary bitmap granularity,
 since they want to reserve very large memory and manage this region
 with bitmap that one bit for several pages to reduce management overheads.
 So support arbitrary bitmap granularity for following generalization.

 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
 Acked-by: Minchan Kim minc...@kernel.org
 
 Just a nit below.
 

 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index bc4c171..9bc9340 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -38,6 +38,7 @@ struct cma {
  unsigned long   base_pfn;
  unsigned long   count;
  unsigned long   *bitmap;
 +int order_per_bit; /* Order of pages represented by one bit */
  struct mutexlock;
  };
  
 @@ -157,9 +158,38 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
  
  static DEFINE_MUTEX(cma_mutex);
  
 +static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int 
 align_order)
 +{
 +return (1  (align_order  cma-order_per_bit)) - 1;
 +}
 +
 +static unsigned long cma_bitmap_maxno(struct cma *cma)
 +{
 +return cma-count  cma-order_per_bit;
 +}
 +
 +static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
 +unsigned long pages)
 +{
 +return ALIGN(pages, 1  cma-order_per_bit)  cma-order_per_bit;
 +}
 +
 +static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
 +{
 +unsigned long bitmapno, nr_bits;
 +
 +bitmapno = (pfn - cma-base_pfn)  cma-order_per_bit;
 +nr_bits = cma_bitmap_pages_to_bits(cma, count);
 +
 +mutex_lock(cma-lock);
 +bitmap_clear(cma-bitmap, bitmapno, nr_bits);
 +mutex_unlock(cma-lock);
 +}
 +
  static int __init cma_activate_area(struct cma *cma)
  {
 -int bitmap_size = BITS_TO_LONGS(cma-count) * sizeof(long);
 +int bitmap_maxno = cma_bitmap_maxno(cma);
 +int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
  unsigned long base_pfn = cma-base_pfn, pfn = base_pfn;
  unsigned i = cma-count  pageblock_order;
  struct zone *zone;
 @@ -221,6 +251,7 @@ core_initcall(cma_init_reserved_areas);
   * @base: Base address of the reserved area optional, use 0 for any
   * @limit: End address of the reserved memory (optional, 0 for any).
   * @alignment: Alignment for the contiguous memory area, should be power of 
 2
 + * @order_per_bit: Order of pages represented by one bit on bitmap.
   * @res_cma: Pointer to store the created cma region.
   * @fixed: hint about where to place the reserved area
   *
 @@ -235,7 +266,7 @@ core_initcall(cma_init_reserved_areas);
   */
  static int __init __dma_contiguous_reserve_area(phys_addr_t size,
  phys_addr_t base, phys_addr_t limit,
 -phys_addr_t alignment,
 +phys_addr_t alignment, int order_per_bit,
  struct cma **res_cma, bool fixed)
  {
  struct cma *cma = cma_areas[cma_area_count];
 @@ -269,6 +300,8 @@ static int __init 
 __dma_contiguous_reserve_area(phys_addr_t size,
  base = ALIGN(base, alignment);
  size = ALIGN(size, alignment);
  limit = ~(alignment - 1);
 +/* size should be aligned with order_per_bit */
 +BUG_ON(!IS_ALIGNED(size  PAGE_SHIFT, 1  order_per_bit));
  
  /* Reserve memory */
  if (base  fixed) {
 @@ -294,6 +327,7 @@ static int __init 
 __dma_contiguous_reserve_area(phys_addr_t size,
   */
  cma-base_pfn = PFN_DOWN(base);
  cma-count = size  PAGE_SHIFT;
 +cma-order_per_bit = order_per_bit;
  *res_cma = cma;
  cma_area_count++;
  
 @@ -313,7 +347,7 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
  {
  int ret;
  
 -ret = __dma_contiguous_reserve_area(size, base, limit, 0,
 +ret = __dma_contiguous_reserve_area(size, base, limit, 0, 0,
  res_cma, fixed);
  if (ret)
  return ret;
 @@ -324,13 +358,6 @@ int __init dma_contiguous_reserve_area(phys_addr_t 
 size, phys_addr_t base,
  return 0;
  }
  
 -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
 -{
 -mutex_lock(cma-lock);
 -bitmap_clear(cma-bitmap, pfn - cma-base_pfn, count);
 -mutex_unlock(cma-lock);
 -}
 -
  /**
   * dma_alloc_from_contiguous() - allocate pages from contiguous area
   * @dev:   Pointer to device for which the allocation is performed.
 @@ -345,7 +372,8 @@ static void clear_cma_bitmap(struct cma *cma, unsigned 
 long pfn, int count)
  static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
 unsigned int align)
  {
 -unsigned long mask, pfn, pageno, start = 0;
 +unsigned long mask, pfn, start = 0;
 +unsigned long bitmap_maxno, bitmapno, nr_bits;
 

Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity

2014-06-12 Thread Joonsoo Kim
On Thu, Jun 12, 2014 at 04:08:11PM +0900, Minchan Kim wrote:
 On Thu, Jun 12, 2014 at 12:21:42PM +0900, Joonsoo Kim wrote:
  ppc kvm's cma region management requires arbitrary bitmap granularity,
  since they want to reserve very large memory and manage this region
  with bitmap that one bit for several pages to reduce management overheads.
  So support arbitrary bitmap granularity for following generalization.
  
  Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
 Acked-by: Minchan Kim minc...@kernel.org
 

Thanks.

[snip...]
   /**
* dma_alloc_from_contiguous() - allocate pages from contiguous area
* @dev:   Pointer to device for which the allocation is performed.
  @@ -345,7 +372,8 @@ static void clear_cma_bitmap(struct cma *cma, unsigned 
  long pfn, int count)
   static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
 unsigned int align)
   {
  -   unsigned long mask, pfn, pageno, start = 0;
  +   unsigned long mask, pfn, start = 0;
  +   unsigned long bitmap_maxno, bitmapno, nr_bits;
 
 Just Nit: bitmap_maxno, bitmap_no or something consistent.
 I know you love consistent when I read description in first patch
 in this patchset. ;-)

Yeah, I will fix it. :)

Thanks.
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality

2014-06-12 Thread Joonsoo Kim
On Thu, Jun 12, 2014 at 04:13:11PM +0900, Minchan Kim wrote:
 On Thu, Jun 12, 2014 at 12:21:43PM +0900, Joonsoo Kim wrote:
  Currently, there are two users on CMA functionality, one is the DMA
  subsystem and the other is the kvm on powerpc. They have their own code
  to manage CMA reserved area even if they looks really similar.
  From my guess, it is caused by some needs on bitmap management. Kvm side
  wants to maintain bitmap not for 1 page, but for more size. Eventually it
  use bitmap where one bit represents 64 pages.
  
  When I implement CMA related patches, I should change those two places
  to apply my change and it seem to be painful to me. I want to change
  this situation and reduce future code management overhead through
  this patch.
  
  This change could also help developer who want to use CMA in their
  new feature development, since they can use CMA easily without
  copying  pasting this reserved area management code.
  
  In previous patches, we have prepared some features to generalize
  CMA reserved area management and now it's time to do it. This patch
  moves core functions to mm/cma.c and change DMA APIs to use
  these functions.
  
  There is no functional change in DMA APIs.
  
  v2: There is no big change from v1 in mm/cma.c. Mostly renaming.
  
  Acked-by: Michal Nazarewicz min...@mina86.com
  Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
 
 Acutally, I want to remove bool return of cma_release but it's not
 a out of scope in this patchset.
 
 Acked-by: Minchan Kim minc...@kernel.org
 
  
  diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
  index 00e13ce..4eac559 100644
  --- a/drivers/base/Kconfig
  +++ b/drivers/base/Kconfig
  @@ -283,16 +283,6 @@ config CMA_ALIGNMENT
   
If unsure, leave the default value 8.
   
  -config CMA_AREAS
  -   int Maximum count of the CMA device-private areas
  -   default 7
  -   help
  - CMA allows to create CMA areas for particular devices. This parameter
  - sets the maximum number of such device private CMA areas in the
  - system.
  -
  - If unsure, leave the default value 7.
  -
   endif
   
   endmenu
  diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
  index 9bc9340..f177f73 100644
  --- a/drivers/base/dma-contiguous.c
  +++ b/drivers/base/dma-contiguous.c
  @@ -24,25 +24,10 @@
   
   #include linux/memblock.h
   #include linux/err.h
  -#include linux/mm.h
  -#include linux/mutex.h
  -#include linux/page-isolation.h
   #include linux/sizes.h
  -#include linux/slab.h
  -#include linux/swap.h
  -#include linux/mm_types.h
   #include linux/dma-contiguous.h
   #include linux/log2.h
 
 Should we remain log2.h in here?
 

We should remove it. I will fix it.

Thanks.

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 09/10] mm, cma: move output param to the end of param list

2014-06-12 Thread Joonsoo Kim
On Thu, Jun 12, 2014 at 04:19:31PM +0900, Minchan Kim wrote:
 On Thu, Jun 12, 2014 at 12:21:46PM +0900, Joonsoo Kim wrote:
  Conventionally, we put output param to the end of param list.
  cma_declare_contiguous() doesn't look like that, so change it.
 
 If you says Conventionally, I'd like to suggest one more thing.
 Conventionally, we put 'base' ahead 'size' but dma_contiguous_reserve_area
 is opposite.

Okay. I will do it. :)

Thanks.

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 10/10] mm, cma: use spinlock instead of mutex

2014-06-12 Thread Minchan Kim
On Thu, Jun 12, 2014 at 12:21:47PM +0900, Joonsoo Kim wrote:
 Currently, we should take the mutex for manipulating bitmap.
 This job may be really simple and short so we don't need to sleep
 if contended. So I change it to spinlock.

I'm not sure it would be good always.
Maybe you remember we discussed about similar stuff about bitmap
searching in vmap friend internally, which was really painful
when it was fragmented. So, at least we need number if you really want
and I hope the number from ARM machine most popular platform for CMA
at the moment.

 
 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
 
 diff --git a/mm/cma.c b/mm/cma.c
 index 22a5b23..3085e8c 100644
 --- a/mm/cma.c
 +++ b/mm/cma.c
 @@ -27,6 +27,7 @@
  #include linux/memblock.h
  #include linux/err.h
  #include linux/mm.h
 +#include linux/spinlock.h
  #include linux/mutex.h
  #include linux/sizes.h
  #include linux/slab.h
 @@ -36,7 +37,7 @@ struct cma {
   unsigned long   count;
   unsigned long   *bitmap;
   int order_per_bit; /* Order of pages represented by one bit */
 - struct mutexlock;
 + spinlock_t  lock;
  };
  
  /*
 @@ -72,9 +73,9 @@ static void clear_cma_bitmap(struct cma *cma, unsigned long 
 pfn, int count)
   bitmapno = (pfn - cma-base_pfn)  cma-order_per_bit;
   nr_bits = cma_bitmap_pages_to_bits(cma, count);
  
 - mutex_lock(cma-lock);
 + spin_lock(cma-lock);
   bitmap_clear(cma-bitmap, bitmapno, nr_bits);
 - mutex_unlock(cma-lock);
 + spin_unlock(cma-lock);
  }
  
  static int __init cma_activate_area(struct cma *cma)
 @@ -112,7 +113,7 @@ static int __init cma_activate_area(struct cma *cma)
   init_cma_reserved_pageblock(pfn_to_page(base_pfn));
   } while (--i);
  
 - mutex_init(cma-lock);
 + spin_lock_init(cma-lock);
   return 0;
  
  err:
 @@ -261,11 +262,11 @@ struct page *cma_alloc(struct cma *cma, int count, 
 unsigned int align)
   nr_bits = cma_bitmap_pages_to_bits(cma, count);
  
   for (;;) {
 - mutex_lock(cma-lock);
 + spin_lock(cma-lock);
   bitmapno = bitmap_find_next_zero_area(cma-bitmap,
   bitmap_maxno, start, nr_bits, mask);
   if (bitmapno = bitmap_maxno) {
 - mutex_unlock(cma-lock);
 + spin_unlock(cma-lock);
   break;
   }
   bitmap_set(cma-bitmap, bitmapno, nr_bits);
 @@ -274,7 +275,7 @@ struct page *cma_alloc(struct cma *cma, int count, 
 unsigned int align)
* our exclusive use. If the migration fails we will take the
* lock again and unmark it.
*/
 - mutex_unlock(cma-lock);
 + spin_unlock(cma-lock);
  
   pfn = cma-base_pfn + (bitmapno  cma-order_per_bit);
   mutex_lock(cma_mutex);
 -- 
 1.7.9.5
 
 --
 To unsubscribe, send a message with 'unsubscribe linux-mm' in
 the body to majord...@kvack.org.  For more info on Linux MM,
 see: http://www.linux-mm.org/ .
 Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 10/10] mm, cma: use spinlock instead of mutex

2014-06-12 Thread Joonsoo Kim
On Thu, Jun 12, 2014 at 04:40:29PM +0900, Minchan Kim wrote:
 On Thu, Jun 12, 2014 at 12:21:47PM +0900, Joonsoo Kim wrote:
  Currently, we should take the mutex for manipulating bitmap.
  This job may be really simple and short so we don't need to sleep
  if contended. So I change it to spinlock.
 
 I'm not sure it would be good always.
 Maybe you remember we discussed about similar stuff about bitmap
 searching in vmap friend internally, which was really painful
 when it was fragmented. So, at least we need number if you really want
 and I hope the number from ARM machine most popular platform for CMA
 at the moment.

Good Point!! Agreed. I will drop this one in next spin and re-submit
in separate patchset after some testing.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/4] KVM: PPC: Book3S PR: Fix ABIv2 issues

2014-06-12 Thread Anton Blanchard
binutils knows that a branch to a function descriptor is actually
a branch to the function text. By removing the explicit branch to
dot symbols, we maintain both ABIv1 and ABIv2 compatibility.

Signed-off-by: Anton Blanchard an...@samba.org
---

Index: b/arch/powerpc/kvm/book3s_interrupts.S
===
--- a/arch/powerpc/kvm/book3s_interrupts.S
+++ b/arch/powerpc/kvm/book3s_interrupts.S
@@ -25,11 +25,9 @@
 #include asm/exception-64s.h
 
 #if defined(CONFIG_PPC_BOOK3S_64)
-#define FUNC(name) GLUE(.,name)
 #define GET_SHADOW_VCPU(reg)addi   reg, r13, PACA_SVCPU
 
 #elif defined(CONFIG_PPC_BOOK3S_32)
-#define FUNC(name) name
 #define GET_SHADOW_VCPU(reg)   lwz reg, (THREAD + THREAD_KVM_SVCPU)(r2)
 
 #endif /* CONFIG_PPC_BOOK3S_XX */
@@ -93,7 +91,7 @@ kvm_start_entry:
 kvm_start_lightweight:
/* Copy registers into shadow vcpu so we can access them in real mode */
GET_SHADOW_VCPU(r3)
-   bl  FUNC(kvmppc_copy_to_svcpu)
+   bl  kvmppc_copy_to_svcpu
nop
REST_GPR(4, r1)
 
@@ -131,7 +129,7 @@ after_sprg3_load:
PPC_LL  r4, VCPU_SHADOW_MSR(r4) /* get shadow_msr */
 
/* Jump to segment patching handler and into our guest */
-   bl  FUNC(kvmppc_entry_trampoline)
+   bl  kvmppc_entry_trampoline
nop
 
 /*
@@ -164,7 +162,7 @@ after_sprg3_load:
/* On 64-bit, interrupts are still off at this point */
 
GET_SHADOW_VCPU(r4)
-   bl  FUNC(kvmppc_copy_from_svcpu)
+   bl  kvmppc_copy_from_svcpu
nop
 
 #ifdef CONFIG_PPC_BOOK3S_64
@@ -203,7 +201,7 @@ after_sprg3_load:
 
/* Restore r3 (kvm_run) and r4 (vcpu) */
REST_2GPRS(3, r1)
-   bl  FUNC(kvmppc_handle_exit_pr)
+   bl  kvmppc_handle_exit_pr
 
/* If RESUME_GUEST, get back in the loop */
cmpwi   r3, RESUME_GUEST
Index: b/arch/powerpc/kvm/book3s_rmhandlers.S
===
--- a/arch/powerpc/kvm/book3s_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_rmhandlers.S
@@ -34,13 +34,7 @@
  *   *
  /
 
-#if defined(CONFIG_PPC_BOOK3S_64)
-
-#define FUNC(name) GLUE(.,name)
-
-#elif defined(CONFIG_PPC_BOOK3S_32)
-
-#define FUNC(name) name
+#ifdef CONFIG_PPC_BOOK3S_32
 
 .macro INTERRUPT_TRAMPOLINE intno
 
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/10] DMA, CMA: clean-up log message

2014-06-12 Thread Zhang Yanfei
On 06/12/2014 11:21 AM, Joonsoo Kim wrote:
 We don't need explicit 'CMA:' prefix, since we already define prefix
 'cma:' in pr_fmt. So remove it.
 
 And, some logs print function name and others doesn't. This looks
 bad to me, so I unify log format to print function name consistently.
 
 Lastly, I add one more debug log on cma_activate_area().
 
 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

Reviewed-by: Zhang Yanfei zhangyan...@cn.fujitsu.com

 
 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index 83969f8..bd0bb81 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
   }
  
   if (selected_size  !dma_contiguous_default_area) {
 - pr_debug(%s: reserving %ld MiB for global area\n, __func__,
 + pr_debug(%s(): reserving %ld MiB for global area\n, __func__,
(unsigned long)selected_size / SZ_1M);
  
   dma_contiguous_reserve_area(selected_size, selected_base,
 @@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
   unsigned i = cma-count  pageblock_order;
   struct zone *zone;
  
 - cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
 + pr_debug(%s()\n, __func__);
  
 + cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
   if (!cma-bitmap)
   return -ENOMEM;
  
 @@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
  
   /* Sanity checks */
   if (cma_area_count == ARRAY_SIZE(cma_areas)) {
 - pr_err(Not enough slots for CMA reserved regions!\n);
 + pr_err(%s(): Not enough slots for CMA reserved regions!\n,
 + __func__);
   return -ENOSPC;
   }
  
 @@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t 
 size, phys_addr_t base,
   *res_cma = cma;
   cma_area_count++;
  
 - pr_info(CMA: reserved %ld MiB at %08lx\n, (unsigned long)size / SZ_1M,
 - (unsigned long)base);
 + pr_info(%s(): reserved %ld MiB at %08lx\n,
 + __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
  
   /* Architecture specific contiguous memory fixup. */
   dma_contiguous_early_fixup(base, size);
   return 0;
  err:
 - pr_err(CMA: failed to reserve %ld MiB\n, (unsigned long)size / SZ_1M);
 + pr_err(%s(): failed to reserve %ld MiB\n,
 + __func__, (unsigned long)size / SZ_1M);
   return ret;
  }
  
 


-- 
Thanks.
Zhang Yanfei
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] KVM: PPC: Book3S HV: Fix ABIv2 issues

2014-06-12 Thread Anton Blanchard
binutils knows that a branch to a function descriptor is actually
a branch to the function text. By removing the explicit branch to
dot symbols, we maintain both ABIv1 and ABIv2 compatibility.

Signed-off-by: Anton Blanchard an...@samba.org
---

Index: b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
===
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -675,9 +675,9 @@ END_FTR_SECTION_IFCLR(CPU_FTR_TM)
 
mr  r31, r4
addir3, r31, VCPU_FPRS_TM
-   bl  .load_fp_state
+   bl  load_fp_state
addir3, r31, VCPU_VRS_TM
-   bl  .load_vr_state
+   bl  load_vr_state
mr  r4, r31
lwz r7, VCPU_VRSAVE_TM(r4)
mtspr   SPRN_VRSAVE, r7
@@ -1421,9 +1421,9 @@ END_FTR_SECTION_IFCLR(CPU_FTR_TM)
 
/* Save FP/VSX. */
addir3, r9, VCPU_FPRS_TM
-   bl  .store_fp_state
+   bl  store_fp_state
addir3, r9, VCPU_VRS_TM
-   bl  .store_vr_state
+   bl  store_vr_state
mfspr   r6, SPRN_VRSAVE
stw r6, VCPU_VRSAVE_TM(r9)
 1:
@@ -2421,11 +2421,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
mtmsrd  r8
isync
addir3,r3,VCPU_FPRS
-   bl  .store_fp_state
+   bl  store_fp_state
 #ifdef CONFIG_ALTIVEC
 BEGIN_FTR_SECTION
addir3,r31,VCPU_VRS
-   bl  .store_vr_state
+   bl  store_vr_state
 END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 #endif
mfspr   r6,SPRN_VRSAVE
@@ -2457,11 +2457,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
mtmsrd  r8
isync
addir3,r4,VCPU_FPRS
-   bl  .load_fp_state
+   bl  load_fp_state
 #ifdef CONFIG_ALTIVEC
 BEGIN_FTR_SECTION
addir3,r31,VCPU_VRS
-   bl  .load_vr_state
+   bl  load_vr_state
 END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 #endif
lwz r7,VCPU_VRSAVE(r31)
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] KVM: PPC: Book3S HV: Fix ABIv2 indirect branch issue

2014-06-12 Thread Anton Blanchard
To establish addressability quickly, ABIv2 requires the target
address of the function being called to be in r12.

Signed-off-by: Anton Blanchard an...@samba.org
---

Index: b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
===
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -1920,8 +1920,8 @@ hcall_try_real_mode:
lwaxr3,r3,r4
cmpwi   r3,0
beq guest_exit_cont
-   add r3,r3,r4
-   mtctr   r3
+   add r12,r3,r4
+   mtctr   r12
mr  r3,r9   /* get vcpu pointer */
ld  r4,VCPU_GPR(R4)(r9)
bctrl
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/4] KVM: PPC: Assembly functions exported to modules need _GLOBAL_TOC()

2014-06-12 Thread Anton Blanchard
Both kvmppc_hv_entry_trampoline and kvmppc_entry_trampoline are
assembly functions that are exported to modules and also require
a valid r2.

As such we need to use _GLOBAL_TOC so we provide a global entry
point that establishes the TOC (r2).

Signed-off-by: Anton Blanchard an...@samba.org
---

Index: b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
===
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -51,7 +51,7 @@
  *
  * LR = return address to continue at after eventually re-enabling MMU
  */
-_GLOBAL(kvmppc_hv_entry_trampoline)
+_GLOBAL_TOC(kvmppc_hv_entry_trampoline)
mflrr0
std r0, PPC_LR_STKOFF(r1)
stdur1, -112(r1)
Index: b/arch/powerpc/kvm/book3s_rmhandlers.S
===
--- a/arch/powerpc/kvm/book3s_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_rmhandlers.S
@@ -140,7 +140,7 @@ kvmppc_handler_skip_ins:
  * On entry, r4 contains the guest shadow MSR
  * MSR.EE has to be 0 when calling this function
  */
-_GLOBAL(kvmppc_entry_trampoline)
+_GLOBAL_TOC(kvmppc_entry_trampoline)
mfmsr   r5
LOAD_REG_ADDR(r7, kvmppc_handler_trampoline_enter)
toreal(r7)
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak

2014-06-12 Thread Zhang Yanfei
On 06/12/2014 02:02 PM, Joonsoo Kim wrote:
 On Thu, Jun 12, 2014 at 02:25:43PM +0900, Minchan Kim wrote:
 On Thu, Jun 12, 2014 at 12:21:39PM +0900, Joonsoo Kim wrote:
 We should free memory for bitmap when we find zone mis-match,
 otherwise this memory will leak.

 Then, -stable stuff?
 
 I don't think so. This is just possible leak candidate, so we don't
 need to push this to stable tree.
 


 Additionally, I copy code comment from ppc kvm's cma code to notify
 why we need to check zone mis-match.

 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index bd0bb81..fb0cdce 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma)
 base_pfn = pfn;
 for (j = pageblock_nr_pages; j; --j, pfn++) {
 WARN_ON_ONCE(!pfn_valid(pfn));
 +   /*
 +* alloc_contig_range requires the pfn range
 +* specified to be in the same zone. Make this
 +* simple by forcing the entire CMA resv range
 +* to be in the same zone.
 +*/
 if (page_zone(pfn_to_page(pfn)) != zone)
 -   return -EINVAL;
 +   goto err;

 At a first glance, I thought it would be better to handle such error
 before activating.
 So when I see the registration code(ie, dma_contiguous_revere_area),
 I realized it is impossible because we didn't set up zone yet. :(

 If so, when we detect to fail here, it would be better to report more
 meaningful error message like what was successful zone and what is
 new zone and failed pfn number?
 
 What I want to do in early phase of this patchset is to make cma code
 on DMA APIs similar to ppc kvm's cma code. ppc kvm's cma code already
 has this error handling logic, so I make this patch.
 
 If we think that we need more things, we can do that on general cma code
 after merging this patchset.
 

Yeah, I also like the idea. After all, this patchset aims to a general CMA
management, we could improve more after this patchset. So

Acked-by: Zhang Yanfei zhangyan...@cn.fujitsu.com

-- 
Thanks.
Zhang Yanfei
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity

2014-06-12 Thread Zhang Yanfei
On 06/12/2014 11:21 AM, Joonsoo Kim wrote:
 ppc kvm's cma region management requires arbitrary bitmap granularity,
 since they want to reserve very large memory and manage this region
 with bitmap that one bit for several pages to reduce management overheads.
 So support arbitrary bitmap granularity for following generalization.
 
 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

Acked-by: Zhang Yanfei zhangyan...@cn.fujitsu.com

 
 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index bc4c171..9bc9340 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -38,6 +38,7 @@ struct cma {
   unsigned long   base_pfn;
   unsigned long   count;
   unsigned long   *bitmap;
 + int order_per_bit; /* Order of pages represented by one bit */
   struct mutexlock;
  };
  
 @@ -157,9 +158,38 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
  
  static DEFINE_MUTEX(cma_mutex);
  
 +static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int 
 align_order)
 +{
 + return (1  (align_order  cma-order_per_bit)) - 1;
 +}
 +
 +static unsigned long cma_bitmap_maxno(struct cma *cma)
 +{
 + return cma-count  cma-order_per_bit;
 +}
 +
 +static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
 + unsigned long pages)
 +{
 + return ALIGN(pages, 1  cma-order_per_bit)  cma-order_per_bit;
 +}
 +
 +static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
 +{
 + unsigned long bitmapno, nr_bits;
 +
 + bitmapno = (pfn - cma-base_pfn)  cma-order_per_bit;
 + nr_bits = cma_bitmap_pages_to_bits(cma, count);
 +
 + mutex_lock(cma-lock);
 + bitmap_clear(cma-bitmap, bitmapno, nr_bits);
 + mutex_unlock(cma-lock);
 +}
 +
  static int __init cma_activate_area(struct cma *cma)
  {
 - int bitmap_size = BITS_TO_LONGS(cma-count) * sizeof(long);
 + int bitmap_maxno = cma_bitmap_maxno(cma);
 + int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
   unsigned long base_pfn = cma-base_pfn, pfn = base_pfn;
   unsigned i = cma-count  pageblock_order;
   struct zone *zone;
 @@ -221,6 +251,7 @@ core_initcall(cma_init_reserved_areas);
   * @base: Base address of the reserved area optional, use 0 for any
   * @limit: End address of the reserved memory (optional, 0 for any).
   * @alignment: Alignment for the contiguous memory area, should be power of 2
 + * @order_per_bit: Order of pages represented by one bit on bitmap.
   * @res_cma: Pointer to store the created cma region.
   * @fixed: hint about where to place the reserved area
   *
 @@ -235,7 +266,7 @@ core_initcall(cma_init_reserved_areas);
   */
  static int __init __dma_contiguous_reserve_area(phys_addr_t size,
   phys_addr_t base, phys_addr_t limit,
 - phys_addr_t alignment,
 + phys_addr_t alignment, int order_per_bit,
   struct cma **res_cma, bool fixed)
  {
   struct cma *cma = cma_areas[cma_area_count];
 @@ -269,6 +300,8 @@ static int __init 
 __dma_contiguous_reserve_area(phys_addr_t size,
   base = ALIGN(base, alignment);
   size = ALIGN(size, alignment);
   limit = ~(alignment - 1);
 + /* size should be aligned with order_per_bit */
 + BUG_ON(!IS_ALIGNED(size  PAGE_SHIFT, 1  order_per_bit));
  
   /* Reserve memory */
   if (base  fixed) {
 @@ -294,6 +327,7 @@ static int __init 
 __dma_contiguous_reserve_area(phys_addr_t size,
*/
   cma-base_pfn = PFN_DOWN(base);
   cma-count = size  PAGE_SHIFT;
 + cma-order_per_bit = order_per_bit;
   *res_cma = cma;
   cma_area_count++;
  
 @@ -313,7 +347,7 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
  {
   int ret;
  
 - ret = __dma_contiguous_reserve_area(size, base, limit, 0,
 + ret = __dma_contiguous_reserve_area(size, base, limit, 0, 0,
   res_cma, fixed);
   if (ret)
   return ret;
 @@ -324,13 +358,6 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
   return 0;
  }
  
 -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
 -{
 - mutex_lock(cma-lock);
 - bitmap_clear(cma-bitmap, pfn - cma-base_pfn, count);
 - mutex_unlock(cma-lock);
 -}
 -
  /**
   * dma_alloc_from_contiguous() - allocate pages from contiguous area
   * @dev:   Pointer to device for which the allocation is performed.
 @@ -345,7 +372,8 @@ static void clear_cma_bitmap(struct cma *cma, unsigned 
 long pfn, int count)
  static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
  unsigned int align)
  {
 - unsigned long mask, pfn, pageno, start = 0;
 + unsigned long mask, pfn, start = 0;
 + unsigned long bitmap_maxno, bitmapno, nr_bits;
   struct page 

Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality

2014-06-12 Thread Zhang Yanfei
On 06/12/2014 11:21 AM, Joonsoo Kim wrote:
 Currently, there are two users on CMA functionality, one is the DMA
 subsystem and the other is the kvm on powerpc. They have their own code
 to manage CMA reserved area even if they looks really similar.
From my guess, it is caused by some needs on bitmap management. Kvm side
 wants to maintain bitmap not for 1 page, but for more size. Eventually it
 use bitmap where one bit represents 64 pages.
 
 When I implement CMA related patches, I should change those two places
 to apply my change and it seem to be painful to me. I want to change
 this situation and reduce future code management overhead through
 this patch.
 
 This change could also help developer who want to use CMA in their
 new feature development, since they can use CMA easily without
 copying  pasting this reserved area management code.
 
 In previous patches, we have prepared some features to generalize
 CMA reserved area management and now it's time to do it. This patch
 moves core functions to mm/cma.c and change DMA APIs to use
 these functions.
 
 There is no functional change in DMA APIs.
 
 v2: There is no big change from v1 in mm/cma.c. Mostly renaming.
 
 Acked-by: Michal Nazarewicz min...@mina86.com
 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

Acked-by: Zhang Yanfei zhangyan...@cn.fujitsu.com

 
 diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
 index 00e13ce..4eac559 100644
 --- a/drivers/base/Kconfig
 +++ b/drivers/base/Kconfig
 @@ -283,16 +283,6 @@ config CMA_ALIGNMENT
  
 If unsure, leave the default value 8.
  
 -config CMA_AREAS
 - int Maximum count of the CMA device-private areas
 - default 7
 - help
 -   CMA allows to create CMA areas for particular devices. This parameter
 -   sets the maximum number of such device private CMA areas in the
 -   system.
 -
 -   If unsure, leave the default value 7.
 -
  endif
  
  endmenu
 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index 9bc9340..f177f73 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -24,25 +24,10 @@
  
  #include linux/memblock.h
  #include linux/err.h
 -#include linux/mm.h
 -#include linux/mutex.h
 -#include linux/page-isolation.h
  #include linux/sizes.h
 -#include linux/slab.h
 -#include linux/swap.h
 -#include linux/mm_types.h
  #include linux/dma-contiguous.h
  #include linux/log2.h
 -
 -struct cma {
 - unsigned long   base_pfn;
 - unsigned long   count;
 - unsigned long   *bitmap;
 - int order_per_bit; /* Order of pages represented by one bit */
 - struct mutexlock;
 -};
 -
 -struct cma *dma_contiguous_default_area;
 +#include linux/cma.h
  
  #ifdef CONFIG_CMA_SIZE_MBYTES
  #define CMA_SIZE_MBYTES CONFIG_CMA_SIZE_MBYTES
 @@ -50,6 +35,8 @@ struct cma *dma_contiguous_default_area;
  #define CMA_SIZE_MBYTES 0
  #endif
  
 +struct cma *dma_contiguous_default_area;
 +
  /*
   * Default global CMA area size can be defined in kernel's .config.
   * This is useful mainly for distro maintainers to create a kernel
 @@ -156,199 +143,13 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
   }
  }
  
 -static DEFINE_MUTEX(cma_mutex);
 -
 -static unsigned long cma_bitmap_aligned_mask(struct cma *cma, int 
 align_order)
 -{
 - return (1  (align_order  cma-order_per_bit)) - 1;
 -}
 -
 -static unsigned long cma_bitmap_maxno(struct cma *cma)
 -{
 - return cma-count  cma-order_per_bit;
 -}
 -
 -static unsigned long cma_bitmap_pages_to_bits(struct cma *cma,
 - unsigned long pages)
 -{
 - return ALIGN(pages, 1  cma-order_per_bit)  cma-order_per_bit;
 -}
 -
 -static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int count)
 -{
 - unsigned long bitmapno, nr_bits;
 -
 - bitmapno = (pfn - cma-base_pfn)  cma-order_per_bit;
 - nr_bits = cma_bitmap_pages_to_bits(cma, count);
 -
 - mutex_lock(cma-lock);
 - bitmap_clear(cma-bitmap, bitmapno, nr_bits);
 - mutex_unlock(cma-lock);
 -}
 -
 -static int __init cma_activate_area(struct cma *cma)
 -{
 - int bitmap_maxno = cma_bitmap_maxno(cma);
 - int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
 - unsigned long base_pfn = cma-base_pfn, pfn = base_pfn;
 - unsigned i = cma-count  pageblock_order;
 - struct zone *zone;
 -
 - pr_debug(%s()\n, __func__);
 -
 - cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
 - if (!cma-bitmap)
 - return -ENOMEM;
 -
 - WARN_ON_ONCE(!pfn_valid(pfn));
 - zone = page_zone(pfn_to_page(pfn));
 -
 - do {
 - unsigned j;
 - base_pfn = pfn;
 - for (j = pageblock_nr_pages; j; --j, pfn++) {
 - WARN_ON_ONCE(!pfn_valid(pfn));
 - /*
 -  * alloc_contig_range requires the pfn range
 -  * specified to be in the same zone. Make this
 -  * simple by forcing 

Re: [PATCH v2 08/10] mm, cma: clean-up cma allocation error path

2014-06-12 Thread Zhang Yanfei
On 06/12/2014 11:21 AM, Joonsoo Kim wrote:
 We can remove one call sites for clear_cma_bitmap() if we first
 call it before checking error number.
 
 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

Reviewed-by: Zhang Yanfei zhangyan...@cn.fujitsu.com

 
 diff --git a/mm/cma.c b/mm/cma.c
 index 1e1b017..01a0713 100644
 --- a/mm/cma.c
 +++ b/mm/cma.c
 @@ -282,11 +282,12 @@ struct page *cma_alloc(struct cma *cma, int count, 
 unsigned int align)
   if (ret == 0) {
   page = pfn_to_page(pfn);
   break;
 - } else if (ret != -EBUSY) {
 - clear_cma_bitmap(cma, pfn, count);
 - break;
   }
 +
   clear_cma_bitmap(cma, pfn, count);
 + if (ret != -EBUSY)
 + break;
 +
   pr_debug(%s(): memory range at %p is busy, retrying\n,
__func__, pfn_to_page(pfn));
   /* try again with a bit different memory target */
 


-- 
Thanks.
Zhang Yanfei
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/10] DMA, CMA: clean-up log message

2014-06-12 Thread Michal Nazarewicz
 Joonsoo Kim iamjoonsoo@lge.com writes:
 
  We don't need explicit 'CMA:' prefix, since we already define prefix
  'cma:' in pr_fmt. So remove it.
 
  And, some logs print function name and others doesn't. This looks
  bad to me, so I unify log format to print function name consistently.
 
  Lastly, I add one more debug log on cma_activate_area().
 
  Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
 
  diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
  index 83969f8..bd0bb81 100644
  --- a/drivers/base/dma-contiguous.c
  +++ b/drivers/base/dma-contiguous.c
  @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
 }
 
 if (selected_size  !dma_contiguous_default_area) {
  -  pr_debug(%s: reserving %ld MiB for global area\n, __func__,
  +  pr_debug(%s(): reserving %ld MiB for global area\n, __func__,
  (unsigned long)selected_size / SZ_1M);

 On Thu, Jun 12, 2014 at 10:11:19AM +0530, Aneesh Kumar K.V wrote:
 Do we need to do function(), or just function:. I have seen the later
 usage in other parts of the kernel.

On Thu, Jun 12 2014, Joonsoo Kim iamjoonsoo@lge.com wrote:
 I also haven't seen this format in other kernel code, but, in cma, they use
 this format as following.

 function(arg1, arg2, ...): some message

 If we all dislike this format, we can change it after merging this
 patchset. Until then, it seems better to me to leave it as is.

I used “function(arg1, arg2, …)” at the *beginning* of functions when
the arguments passed to the function were included in the message.  In
all other cases I left it at just “function:” (or just no additional
prefix).  IMO that's a reasonable strategy.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/10] DMA, CMA: clean-up log message

2014-06-12 Thread Michal Nazarewicz
On Thu, Jun 12 2014, Joonsoo Kim iamjoonsoo@lge.com wrote:
 We don't need explicit 'CMA:' prefix, since we already define prefix
 'cma:' in pr_fmt. So remove it.

 And, some logs print function name and others doesn't. This looks
 bad to me, so I unify log format to print function name consistently.

 Lastly, I add one more debug log on cma_activate_area().

I don't particularly care what format of logs you choose, so:

Acked-by: Michal Nazarewicz min...@mina86.com

even though I'd go without empty “()”.

 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index 83969f8..bd0bb81 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -144,7 +144,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
   }
  
   if (selected_size  !dma_contiguous_default_area) {
 - pr_debug(%s: reserving %ld MiB for global area\n, __func__,
 + pr_debug(%s(): reserving %ld MiB for global area\n, __func__,
(unsigned long)selected_size / SZ_1M);
  
   dma_contiguous_reserve_area(selected_size, selected_base,
 @@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
   unsigned i = cma-count  pageblock_order;
   struct zone *zone;
  
 - cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
 + pr_debug(%s()\n, __func__);
  
 + cma-bitmap = kzalloc(bitmap_size, GFP_KERNEL);
   if (!cma-bitmap)
   return -ENOMEM;
  
 @@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
  
   /* Sanity checks */
   if (cma_area_count == ARRAY_SIZE(cma_areas)) {
 - pr_err(Not enough slots for CMA reserved regions!\n);
 + pr_err(%s(): Not enough slots for CMA reserved regions!\n,
 + __func__);
   return -ENOSPC;
   }
  
 @@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t 
 size, phys_addr_t base,
   *res_cma = cma;
   cma_area_count++;
  
 - pr_info(CMA: reserved %ld MiB at %08lx\n, (unsigned long)size / SZ_1M,
 - (unsigned long)base);
 + pr_info(%s(): reserved %ld MiB at %08lx\n,
 + __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
  
   /* Architecture specific contiguous memory fixup. */
   dma_contiguous_early_fixup(base, size);
   return 0;
  err:
 - pr_err(CMA: failed to reserve %ld MiB\n, (unsigned long)size / SZ_1M);
 + pr_err(%s(): failed to reserve %ld MiB\n,
 + __func__, (unsigned long)size / SZ_1M);
   return ret;
  }

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak

2014-06-12 Thread Michal Nazarewicz
On Thu, Jun 12 2014, Joonsoo Kim iamjoonsoo@lge.com wrote:
 We should free memory for bitmap when we find zone mis-match,
 otherwise this memory will leak.

 Additionally, I copy code comment from ppc kvm's cma code to notify
 why we need to check zone mis-match.

 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

Reviewed-by: Michal Nazarewicz min...@mina86.com

 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index bd0bb81..fb0cdce 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -177,14 +177,24 @@ static int __init cma_activate_area(struct cma *cma)
   base_pfn = pfn;
   for (j = pageblock_nr_pages; j; --j, pfn++) {
   WARN_ON_ONCE(!pfn_valid(pfn));
 + /*
 +  * alloc_contig_range requires the pfn range
 +  * specified to be in the same zone. Make this
 +  * simple by forcing the entire CMA resv range
 +  * to be in the same zone.
 +  */
   if (page_zone(pfn_to_page(pfn)) != zone)
 - return -EINVAL;
 + goto err;
   }
   init_cma_reserved_pageblock(pfn_to_page(base_pfn));
   } while (--i);
  
   mutex_init(cma-lock);
   return 0;
 +
 +err:
 + kfree(cma-bitmap);
 + return -EINVAL;
  }

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/10] DMA, CMA: clean-up log message

2014-06-12 Thread Michal Nazarewicz
On Thu, Jun 12 2014, Michal Nazarewicz min...@mina86.com wrote:
 I used “function(arg1, arg2, …)” at the *beginning* of functions when
 the arguments passed to the function were included in the message.  In
 all other cases I left it at just “function:” (or just no additional
 prefix).  IMO that's a reasonable strategy.

At closer inspection, I realised drivers/base/dma-contiguous.c is
Marek's code, but the above I think is still reasonable thing to do, so
I'd rather standardise on having “function(…)” only at the beginning of
a function.  Just my 0.02 CHF.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 03/10] DMA, CMA: separate core cma management codes from DMA APIs

2014-06-12 Thread Michal Nazarewicz
On Thu, Jun 12 2014, Joonsoo Kim iamjoonsoo@lge.com wrote:
 To prepare future generalization work on cma area management code,
 we need to separate core cma management codes from DMA APIs.
 We will extend these core functions to cover requirements of
 ppc kvm's cma area management functionality in following patches.
 This separation helps us not to touch DMA APIs while extending
 core functions.

 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

Acked-by: Michal Nazarewicz min...@mina86.com


-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity

2014-06-12 Thread Michal Nazarewicz
On Thu, Jun 12 2014, Joonsoo Kim iamjoonsoo@lge.com wrote:
 ppc kvm's cma region management requires arbitrary bitmap granularity,
 since they want to reserve very large memory and manage this region
 with bitmap that one bit for several pages to reduce management overheads.
 So support arbitrary bitmap granularity for following generalization.

 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

Acked-by: Michal Nazarewicz min...@mina86.com

 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index bc4c171..9bc9340 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -38,6 +38,7 @@ struct cma {
   unsigned long   base_pfn;
   unsigned long   count;

Have you considered replacing count with maxno?

   unsigned long   *bitmap;
 + int order_per_bit; /* Order of pages represented by one bit */

I'd make it unsigned.

   struct mutexlock;
  };
  
 +static void clear_cma_bitmap(struct cma *cma, unsigned long pfn, int
 count)

For consistency cma_clear_bitmap would make more sense I think.  On the
other hand, you're just moving stuff around so perhaps renaming the
function at this point is not worth it any more.

 +{
 + unsigned long bitmapno, nr_bits;
 +
 + bitmapno = (pfn - cma-base_pfn)  cma-order_per_bit;
 + nr_bits = cma_bitmap_pages_to_bits(cma, count);
 +
 + mutex_lock(cma-lock);
 + bitmap_clear(cma-bitmap, bitmapno, nr_bits);
 + mutex_unlock(cma-lock);
 +}
 +
  static int __init cma_activate_area(struct cma *cma)
  {
 - int bitmap_size = BITS_TO_LONGS(cma-count) * sizeof(long);
 + int bitmap_maxno = cma_bitmap_maxno(cma);
 + int bitmap_size = BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
   unsigned long base_pfn = cma-base_pfn, pfn = base_pfn;
   unsigned i = cma-count  pageblock_order;
   struct zone *zone;

bitmap_maxno is never used again, perhaps:

+   int bitmap_size = BITS_TO_LONGS(cma_bitmap_maxno(cma)) * sizeof(long);

instead? Up to you.

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 08/10] mm, cma: clean-up cma allocation error path

2014-06-12 Thread Michal Nazarewicz
On Thu, Jun 12 2014, Joonsoo Kim iamjoonsoo@lge.com wrote:
 We can remove one call sites for clear_cma_bitmap() if we first
 call it before checking error number.

 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

Reviewed-by: Michal Nazarewicz min...@mina86.com

 diff --git a/mm/cma.c b/mm/cma.c
 index 1e1b017..01a0713 100644
 --- a/mm/cma.c
 +++ b/mm/cma.c
 @@ -282,11 +282,12 @@ struct page *cma_alloc(struct cma *cma, int count, 
 unsigned int align)
   if (ret == 0) {
   page = pfn_to_page(pfn);
   break;
 - } else if (ret != -EBUSY) {
 - clear_cma_bitmap(cma, pfn, count);
 - break;
   }
 +
   clear_cma_bitmap(cma, pfn, count);
 + if (ret != -EBUSY)
 + break;
 +
   pr_debug(%s(): memory range at %p is busy, retrying\n,
__func__, pfn_to_page(pfn));
   /* try again with a bit different memory target */

-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 09/10] mm, cma: move output param to the end of param list

2014-06-12 Thread Michal Nazarewicz
On Thu, Jun 12 2014, Joonsoo Kim iamjoonsoo@lge.com wrote:
 Conventionally, we put output param to the end of param list.
 cma_declare_contiguous() doesn't look like that, so change it.

Perhaps the function should be changed to return an error-pointer?

 Additionally, move down cma_areas reference code to the position
 where it is really needed.

 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com

Acked-by: Michal Nazarewicz min...@mina86.com


 diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c 
 b/arch/powerpc/kvm/book3s_hv_builtin.c
 index 28ec226..97613ea 100644
 --- a/arch/powerpc/kvm/book3s_hv_builtin.c
 +++ b/arch/powerpc/kvm/book3s_hv_builtin.c
 @@ -184,7 +184,7 @@ void __init kvm_cma_reserve(void)
  
   align_size = max(kvm_rma_pages  PAGE_SHIFT, align_size);
   cma_declare_contiguous(selected_size, 0, 0, align_size,
 - KVM_CMA_CHUNK_ORDER - PAGE_SHIFT, kvm_cma, false);
 + KVM_CMA_CHUNK_ORDER - PAGE_SHIFT, false, kvm_cma);
   }
  }
  
 diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
 index f177f73..bfd4553 100644
 --- a/drivers/base/dma-contiguous.c
 +++ b/drivers/base/dma-contiguous.c
 @@ -149,7 +149,7 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, 
 phys_addr_t base,
  {
   int ret;
  
 - ret = cma_declare_contiguous(size, base, limit, 0, 0, res_cma, fixed);
 + ret = cma_declare_contiguous(size, base, limit, 0, 0, fixed, res_cma);
   if (ret)
   return ret;
  
 diff --git a/include/linux/cma.h b/include/linux/cma.h
 index e38efe9..e53eead 100644
 --- a/include/linux/cma.h
 +++ b/include/linux/cma.h
 @@ -6,7 +6,7 @@ struct cma;
  extern int __init cma_declare_contiguous(phys_addr_t size,
   phys_addr_t base, phys_addr_t limit,
   phys_addr_t alignment, int order_per_bit,
 - struct cma **res_cma, bool fixed);
 + bool fixed, struct cma **res_cma);
  extern struct page *cma_alloc(struct cma *cma, int count, unsigned int 
 align);
  extern bool cma_release(struct cma *cma, struct page *pages, int count);
  #endif
 diff --git a/mm/cma.c b/mm/cma.c
 index 01a0713..22a5b23 100644
 --- a/mm/cma.c
 +++ b/mm/cma.c
 @@ -142,8 +142,8 @@ core_initcall(cma_init_reserved_areas);
   * @limit: End address of the reserved memory (optional, 0 for any).
   * @alignment: Alignment for the contiguous memory area, should be power of 2
   * @order_per_bit: Order of pages represented by one bit on bitmap.
 - * @res_cma: Pointer to store the created cma region.
   * @fixed: hint about where to place the reserved area
 + * @res_cma: Pointer to store the created cma region.
   *
   * This function reserves memory from early allocator. It should be
   * called by arch specific code once the early allocator (memblock or 
 bootmem)
 @@ -156,9 +156,9 @@ core_initcall(cma_init_reserved_areas);
  int __init cma_declare_contiguous(phys_addr_t size,
   phys_addr_t base, phys_addr_t limit,
   phys_addr_t alignment, int order_per_bit,
 - struct cma **res_cma, bool fixed)
 + bool fixed, struct cma **res_cma)
  {
 - struct cma *cma = cma_areas[cma_area_count];
 + struct cma *cma;
   int ret = 0;
  
   pr_debug(%s(size %lx, base %08lx, limit %08lx alignment %08lx)\n,
 @@ -214,6 +214,7 @@ int __init cma_declare_contiguous(phys_addr_t size,
* Each reserved area must be initialised later, when more kernel
* subsystems (like slab allocator) are available.
*/
 + cma = cma_areas[cma_area_count];
   cma-base_pfn = PFN_DOWN(base);
   cma-count = size  PAGE_SHIFT;
   cma-order_per_bit = order_per_bit;
 -- 
 1.7.9.5


-- 
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of  o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz(o o)
ooo +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4 v3] KVM: PPC: Alow kvmppc_get_last_inst() to fail

2014-06-12 Thread Alexander Graf

On 06/02/2014 05:50 PM, Mihai Caraman wrote:

On book3e, guest last instruction is read on the exit path using load
external pid (lwepx) dedicated instruction. This load operation may fail
due to TLB eviction and execute-but-not-read entries.

This patch lay down the path for an alternative solution to read the guest
last instruction, by allowing kvmppc_get_lat_inst() function to fail.
Architecture specific implmentations of kvmppc_load_last_inst() may read
last guest instruction and instruct the emulation layer to re-execute the
guest in case of failure.

Make kvmppc_get_last_inst() definition common between architectures.

Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
v3:
  - these changes compile on book3s, please validate the functionality and
do the necessary adaptations!
  - rework patch description
  - add common definition for kvmppc_get_last_inst()
  - check return values in book3s code

v2:
  - integrated kvmppc_get_last_inst() in book3s code and checked build
  - addressed cosmetic feedback

  arch/powerpc/include/asm/kvm_book3s.h|  28 ++--
  arch/powerpc/include/asm/kvm_booke.h |   7 +-
  arch/powerpc/include/asm/kvm_ppc.h   |  16 +
  arch/powerpc/kvm/book3s_64_mmu_hv.c  |  16 ++---
  arch/powerpc/kvm/book3s_paired_singles.c |  38 ++
  arch/powerpc/kvm/book3s_pr.c | 116 +--
  arch/powerpc/kvm/booke.c |   3 +
  arch/powerpc/kvm/e500_mmu_host.c |   5 ++
  arch/powerpc/kvm/emulate.c   |  18 +++--
  arch/powerpc/kvm/powerpc.c   |  10 ++-
  10 files changed, 142 insertions(+), 115 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_book3s.h 
b/arch/powerpc/include/asm/kvm_book3s.h
index f52f656..3409572 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -274,30 +274,14 @@ static inline bool kvmppc_need_byteswap(struct kvm_vcpu 
*vcpu)
return (kvmppc_get_msr(vcpu)  MSR_LE) != (MSR_KERNEL  MSR_LE);
  }
  
-static inline u32 kvmppc_get_last_inst_internal(struct kvm_vcpu *vcpu, ulong pc)

+static inline int kvmppc_load_last_inst(struct kvm_vcpu *vcpu, bool prev,


Please make prev an enum :)


+u32 *inst)
  {
-   /* Load the instruction manually if it failed to do so in the
-* exit path */
-   if (vcpu-arch.last_inst == KVM_INST_FETCH_FAILED)
-   kvmppc_ld(vcpu, pc, sizeof(u32), vcpu-arch.last_inst, false);
+   ulong pc = kvmppc_get_pc(vcpu);
  
-	return kvmppc_need_byteswap(vcpu) ? swab32(vcpu-arch.last_inst) :

-   vcpu-arch.last_inst;
-}
-
-static inline u32 kvmppc_get_last_inst(struct kvm_vcpu *vcpu)
-{
-   return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu));
-}
-
-/*
- * Like kvmppc_get_last_inst(), but for fetching a sc instruction.
- * Because the sc instruction sets SRR0 to point to the following
- * instruction, we have to fetch from pc - 4.
- */
-static inline u32 kvmppc_get_last_sc(struct kvm_vcpu *vcpu)
-{
-   return kvmppc_get_last_inst_internal(vcpu, kvmppc_get_pc(vcpu) - 4);
+   if (prev)
+   pc -= 4;
+   return kvmppc_ld(vcpu, pc, sizeof(u32), vcpu-arch.last_inst, false);
  }


In this case we're already in the slow path. Can we move this into a .c 
file instead? That would unify it with booke and ...


  
  static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu)

diff --git a/arch/powerpc/include/asm/kvm_booke.h 
b/arch/powerpc/include/asm/kvm_booke.h
index c7aed61..1e28371 100644
--- a/arch/powerpc/include/asm/kvm_booke.h
+++ b/arch/powerpc/include/asm/kvm_booke.h
@@ -33,6 +33,8 @@
  #define KVMPPC_INST_EHPRIV_DEBUG  (KVMPPC_INST_EHPRIV | \
 (EHPRIV_OC_DEBUG  EHPRIV_OC_SHIFT))
  
+extern int kvmppc_load_last_inst(struct kvm_vcpu *vcpu, bool prev, u32 *inst);


... allow us to move this to a common header file :).


+
  static inline void kvmppc_set_gpr(struct kvm_vcpu *vcpu, int num, ulong val)
  {
vcpu-arch.gpr[num] = val;
@@ -69,11 +71,6 @@ static inline bool kvmppc_need_byteswap(struct kvm_vcpu 
*vcpu)
return false;
  }
  
-static inline u32 kvmppc_get_last_inst(struct kvm_vcpu *vcpu)

-{
-   return vcpu-arch.last_inst;
-}
-
  static inline void kvmppc_set_ctr(struct kvm_vcpu *vcpu, ulong val)
  {
vcpu-arch.ctr = val;
diff --git a/arch/powerpc/include/asm/kvm_ppc.h 
b/arch/powerpc/include/asm/kvm_ppc.h
index 4a7cc45..619be2f 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -234,6 +234,22 @@ struct kvmppc_ops {
  extern struct kvmppc_ops *kvmppc_hv_ops;
  extern struct kvmppc_ops *kvmppc_pr_ops;
  
+static inline int kvmppc_get_last_inst(struct kvm_vcpu *vcpu, bool prev,

+   u32 *inst)
+{
+   int ret = EMULATE_DONE;
+
+   /* Load the instruction manually if it failed to do so in the
+* exit 

Re: [PATCH 4/4 v3] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation

2014-06-12 Thread Alexander Graf

On 06/02/2014 05:50 PM, Mihai Caraman wrote:

On book3e, KVM uses load external pid (lwepx) dedicated instruction to read
guest last instruction on the exit path. lwepx exceptions (DTLB_MISS, DSI
and LRAT), generated by loading a guest address, needs to be handled by KVM.
These exceptions are generated in a substituted guest translation context
(EPLC[EGS] = 1) from host context (MSR[GS] = 0).

Currently, KVM hooks only interrupts generated from guest context (MSR[GS] = 1),
doing minimal checks on the fast path to avoid host performance degradation.
lwepx exceptions originate from host state (MSR[GS] = 0) which implies
additional checks in DO_KVM macro (beside the current MSR[GS] = 1) by looking
at the Exception Syndrome Register (ESR[EPID]) and the External PID Load Context
Register (EPLC[EGS]). Doing this on each Data TLB miss exception is obvious
too intrusive for the host.

Read guest last instruction from kvmppc_load_last_inst() by searching for the
physical address and kmap it. This address the TODO for TLB eviction and
execute-but-not-read entries, and allow us to get rid of lwepx until we are
able to handle failures.

A simple stress benchmark shows a 1% sys performance degradation compared with
previous approach (lwepx without failure handling):

time for i in `seq 1 1`; do /bin/echo  /dev/null; done

real0m 8.85s
user0m 4.34s
sys 0m 4.48s

vs

real0m 8.84s
user0m 4.36s
sys 0m 4.44s

An alternative solution, to handle lwepx exceptions in KVM, is to temporary
highjack the interrupt vector from host. Some cores share host IVOR registers
between hardware threads, which is the case of FSL e6500, which impose 
additional
synchronization logic for this solution to work. This optimized solution can
be developed later on top of this patch.

Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
v3:
  - reworked patch description
  - use unaltered kmap addr for kunmap
  - get last instruction before beeing preempted

v2:
  - reworked patch description
  - used pr_* functions
  - addressed cosmetic feedback

  arch/powerpc/kvm/booke.c  | 32 
  arch/powerpc/kvm/bookehv_interrupts.S | 37 --
  arch/powerpc/kvm/e500_mmu_host.c  | 93 +++
  3 files changed, 134 insertions(+), 28 deletions(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 34a42b9..4ef52a8 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -880,6 +880,8 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu 
*vcpu,
int r = RESUME_HOST;
int s;
int idx;
+   u32 last_inst = KVM_INST_FETCH_FAILED;
+   enum emulation_result emulated = EMULATE_DONE;
  
  	/* update before a new last_exit_type is rewritten */

kvmppc_update_timing_stats(vcpu);
@@ -887,6 +889,15 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
/* restart interrupts if they were meant for the host */
kvmppc_restart_interrupt(vcpu, exit_nr);
  
+	/*

+* get last instruction before beeing preempted
+* TODO: for e6500 check also BOOKE_INTERRUPT_LRAT_ERROR  ESR_DATA
+*/
+   if (exit_nr == BOOKE_INTERRUPT_DATA_STORAGE ||
+   exit_nr == BOOKE_INTERRUPT_DTLB_MISS ||
+   exit_nr == BOOKE_INTERRUPT_HV_PRIV)


Please make this a switch() - that's easier to read.


+   emulated = kvmppc_get_last_inst(vcpu, false, last_inst);
+
local_irq_enable();
  
  	trace_kvm_exit(exit_nr, vcpu);

@@ -895,6 +906,26 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
run-exit_reason = KVM_EXIT_UNKNOWN;
run-ready_for_interrupt_injection = 1;
  
+	switch (emulated) {

+   case EMULATE_AGAIN:
+   r = RESUME_GUEST;
+   goto out;
+
+   case EMULATE_FAIL:
+   pr_debug(%s: emulation at %lx failed (%08x)\n,
+  __func__, vcpu-arch.pc, last_inst);
+   /* For debugging, encode the failing instruction and
+* report it to userspace. */
+   run-hw.hardware_exit_reason = ~0ULL  32;
+   run-hw.hardware_exit_reason |= last_inst;
+   kvmppc_core_queue_program(vcpu, ESR_PIL);
+   r = RESUME_HOST;
+   goto out;
+
+   default:
+   break;
+   }


I think you can just put this into a function.

Scott, I think the patch overall looks quite good. Can you please check 
as well and if you agree give it your reviewed-by? Mike, when Scott 
gives you a reviewed-by, please include it for the next version.



Alex


+
switch (exit_nr) {
case BOOKE_INTERRUPT_MACHINE_CHECK:
printk(MACHINE CHECK: %lx\n, mfspr(SPRN_MCSR));
@@ -1184,6 +1215,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
kvm_vcpu *vcpu,
BUG();
}
  
+out:

/*
 * To avoid clobbering exit_reason, only check 

Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule

2014-06-12 Thread Alexander Graf

On 06/12/2014 04:00 PM, Mihai Caraman wrote:

On vcpu schedule, the condition checked for tlb pollution is too tight.
The tlb entries of one vcpu are polluted when a different vcpu from the
same partition runs in-between. Relax the current tlb invalidation
condition taking into account the lpid.

Signed-off-by: Mihai Caraman mihai.caraman at freescale.com


Your mailer is broken? :)
This really should be an @.

I think this should work. Scott, please ack.


Alex


Cc: Scott Wood scottwood at freescale.com
---
  arch/powerpc/kvm/e500mc.c | 20 +---
  1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index 17e4562..2e0cd69 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -111,10 +111,12 @@ void kvmppc_mmu_msr_notify(struct kvm_vcpu *vcpu, u32 
old_msr)
  }
  
  static DEFINE_PER_CPU(struct kvm_vcpu *, last_vcpu_on_cpu);

+static DEFINE_PER_CPU(int, last_lpid_on_cpu);
  
  static void kvmppc_core_vcpu_load_e500mc(struct kvm_vcpu *vcpu, int cpu)

  {
struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
+   bool update_last = false, inval_tlb = false;
  
  	kvmppc_booke_vcpu_load(vcpu, cpu);
  
@@ -140,12 +142,24 @@ static void kvmppc_core_vcpu_load_e500mc(struct kvm_vcpu *vcpu, int cpu)

mtspr(SPRN_GDEAR, vcpu-arch.shared-dar);
mtspr(SPRN_GESR, vcpu-arch.shared-esr);
  
-	if (vcpu-arch.oldpir != mfspr(SPRN_PIR) ||

-   __get_cpu_var(last_vcpu_on_cpu) != vcpu) {
-   kvmppc_e500_tlbil_all(vcpu_e500);
+   if (vcpu-arch.oldpir != mfspr(SPRN_PIR)) {
+   /* tlb entries deprecated */
+   inval_tlb = update_last = true;
+   } else if (__get_cpu_var(last_vcpu_on_cpu) != vcpu) {
+   update_last = true;
+   /* tlb entries polluted */
+   inval_tlb = __get_cpu_var(last_lpid_on_cpu) ==
+   vcpu-kvm-arch.lpid;
+   }
+
+   if (update_last) {
__get_cpu_var(last_vcpu_on_cpu) = vcpu;
+   __get_cpu_var(last_lpid_on_cpu) = vcpu-kvm-arch.lpid;
}
  
+	if (inval_tlb)

+   kvmppc_e500_tlbil_all(vcpu_e500);
+
kvmppc_load_guest_fp(vcpu);
  }
  


--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html