* Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
From: Zhang Yanfei @ 2014-06-12 8:19 UTC (permalink / raw)
To: Joonsoo Kim
Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
Marek Szyprowski
In-Reply-To: <20140612060211.GC30128@js1304-P5Q-DELUXE>
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.kim@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 <zhangyanfei@cn.fujitsu.com>
--
Thanks.
Zhang Yanfei
^ permalink raw reply
* Re: [PATCH] powerpc, xmon: Enable hardware instruction breakpoint support on POWER8
From: Anshuman Khandual @ 2014-06-12 8:19 UTC (permalink / raw)
To: Anshuman Khandual; +Cc: Michael Ellerman, linuxppc-dev, mikey, Aneesh Kumar K.V
In-Reply-To: <538EDD61.60801@linux.vnet.ibm.com>
On 06/04/2014 02:18 PM, Anshuman Khandual wrote:
> On 06/03/2014 11:33 AM, Anshuman Khandual wrote:
>> On 05/30/2014 07:12 PM, Aneesh Kumar K.V wrote:
>>>> Anshuman Khandual <khandual@linux.vnet.ibm.com> writes:
>>>>
>>>>>> This patch enables support for hardware instruction breakpoints on POWER8 with
>>>>>> the help of a new register called CIABR (Completed Instruction Address Breakpoint
>>>>>> Register). With this patch, single hardware instruction breakpoint can be added
>>>>>> and cleared during any active xmon debug session. This hardware based instruction
>>>>>> breakpoint mechanism works correctly along with the existing TRAP based instruction
>>>>>> breakpoints available on xmon. Example usage as follows.
>>>>>>
>>>>>> (A) Start xmon:
>>>>>> $echo x > /proc/sysrq-trigger
>>>>>> SysRq : Entering xmon
>>>>>> cpu 0x0: Vector: 0 at [c000001f6c67f960]
>>>>>> pc: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>>>>>> lr: c000000000072078: .sysrq_handle_xmon+0x58/0x60
>>>>>> sp: c000001f6c67fac0
>>>>>> msr: 9000000000009032
>>>>>> current = 0xc000001f6e709ac0
>>>>>> paca = 0xc00000000fffa000 softe: 0 irq_happened: 0x00
>>>>>> pid = 3250, comm = bash
>>>>>> enter ? for help
>>>>>> 0:mon> b
>>>>>> type address
>>>>>>
>>>>>> (B) Set the breakpoint:
>>>>>> 0:mon> ls .power_pmu_add
>>>>>> .power_pmu_add: c000000000078f50
>>>>>> 0:mon> bi c000000000078f50
>>>>>> 0:mon> b
>>>>>> type address
>>>>>> 1 inst c000000000078f50 .power_pmu_add+0x0/0x2e0
>>>>>> 0:mon> ls .perf_event_interrupt
>>>>>> .perf_event_interrupt: c00000000007aee0
>>>>>> 0:mon> bi c00000000007aee0
>>>>>> One instruction breakpoint possible with CIABR
>>>>>> 0:mon> x
>>>>>>
>>>>>> (C) Run the workload (with the breakpoint):
>>>>>> $./perf record ls
>>>>>> cpu 0x2: Vector: d00 (Single Step) at [c000001f718133a0]
>>>>>> pc: c000000000078f54: .power_pmu_add+0x4/0x2e0
>>>>>> lr: c000000000155be0: .event_sched_in+0x90/0x1d0
>>>>>> sp: c000001f71813620
>>>>>> msr: 9000000040109032
>>>>>> current = 0xc000001f6ce30000
>>>>>> paca = 0xc00000000fffa600 softe: 0 irq_happened: 0x01
>>>>>> pid = 3270, comm = ls
>>>>>> std r22,-80(r1)
>>>>>> enter ? for help
>>>>>>
>>>>>> (D) Clear the breakpoint:
>>>>>> 2:mon> bc
>>>>>> All breakpoints cleared
>>>>>> 2:mon> x
>>>>>> [ perf record: Woken up 1 times to write data ]
>>>>>> [ perf record: Captured and wrote 0.002 MB perf.data (~66 samples) ]
>>>>>>
>>>>>> (E) Run the workload again (without any breakpoints):
>>>>>> $./perf record ls
>>>>>> [ perf record: Woken up 1 times to write data ]
>>>>>> [ perf record: Captured and wrote 0.001 MB perf.data (~61 samples) ]
>>>>>>
>>>>>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>>>>> ---
>>>>>> arch/powerpc/xmon/xmon.c | 62 ++++++++++++++++++++++++++++++++++++++++++++----
>>>>>> 1 file changed, 58 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>>>>>> index 3fd1d9a..f74ec83 100644
>>>>>> --- a/arch/powerpc/xmon/xmon.c
>>>>>> +++ b/arch/powerpc/xmon/xmon.c
>>>>>> @@ -48,6 +48,7 @@
>>>>>> #ifdef CONFIG_PPC64
>>>>>> #include <asm/hvcall.h>
>>>>>> #include <asm/paca.h>
>>>>>> +#include <asm/plpar_wrappers.h>
>>>>>> #endif
>>>>>>
>>>>>> #include "nonstdio.h"
>>>>>> @@ -89,6 +90,7 @@ struct bpt {
>>>>>> /* Bits in bpt.enabled */
>>>>>> #define BP_IABR_TE 1 /* IABR translation enabled */
>>>>>> #define BP_IABR 2
>>>>>> +#define BP_CIABR 4
>>>>>> #define BP_TRAP 8
>>>>>> #define BP_DABR 0x10
>>>>>>
>>>>>> @@ -97,6 +99,7 @@ static struct bpt bpts[NBPTS];
>>>>>> static struct bpt dabr;
>>>>>> static struct bpt *iabr;
>>>>>> static unsigned bpinstr = 0x7fe00008; /* trap */
>>>>>> +static bool ciabr_used = false; /* CIABR instruction breakpoint */
>>>>>>
>>>>>> #define BP_NUM(bp) ((bp) - bpts + 1)
>>>>>>
>>>>>> @@ -269,6 +272,34 @@ static inline void cinval(void *p)
>>>>>> asm volatile ("dcbi 0,%0; icbi 0,%0" : : "r" (p));
>>>>>> }
>>>>>>
>>>>>> +static void write_ciabr(unsigned long ciabr)
>>>>>> +{
>>>>>> + if (cpu_has_feature(CPU_FTR_HVMODE)) {
>>>>>> + mtspr(SPRN_CIABR, ciabr);
>>>>>> + return;
>>>>>> + }
>>>>>> +
>>>>>> +#ifdef CONFIG_PPC64
>>>>>> + plapr_set_ciabr(ciabr);
>>>>>> +#endif
>>>>>> +}
>>>>>> +
>>>>>> +static void set_ciabr(unsigned long addr)
>>>>>> +{
>>>>>> + addr &= ~CIABR_PRIV;
>>>>>> + if (cpu_has_feature(CPU_FTR_HVMODE))
>>>>>> + addr |= CIABR_PRIV_HYPER;
>>>>>> + else
>>>>>> + addr |= CIABR_PRIV_SUPER;
>>>>>> + write_ciabr(addr);
>>>>>> +}
>>>>>> +
>>>>>> +static void clear_ciabr(void)
>>>>>> +{
>>>>>> + if (cpu_has_feature(CPU_FTR_ARCH_207S))
>>>>>> + write_ciabr(0);
>>>>>> +}
>>>>>> +
>>>>>> /*
>>>>>> * Disable surveillance (the service processor watchdog function)
>>>>>> * while we are in xmon.
>>>>>> @@ -764,6 +795,9 @@ static void insert_cpu_bpts(void)
>>>>>> if (iabr && cpu_has_feature(CPU_FTR_IABR))
>>>>>> mtspr(SPRN_IABR, iabr->address
>>>>>> | (iabr->enabled & (BP_IABR|BP_IABR_TE)));
>>>>>> +
>>>>>> + if (iabr && cpu_has_feature(CPU_FTR_ARCH_207S))
>>>>>> + set_ciabr(iabr->address);
>>>>>> }
>>>>>>
>>>>>> static void remove_bpts(void)
>>>>>> @@ -791,6 +825,7 @@ static void remove_cpu_bpts(void)
>>>>>> hw_breakpoint_disable();
>>>>>> if (cpu_has_feature(CPU_FTR_IABR))
>>>>>> mtspr(SPRN_IABR, 0);
>>>>>> + clear_ciabr();
>>>>>> }
>>>>>>
>>>>>> /* Command interpreting routine */
>>>>>> @@ -1124,7 +1159,7 @@ static char *breakpoint_help_string =
>>>>>> "b <addr> [cnt] set breakpoint at given instr addr\n"
>>>>>> "bc clear all breakpoints\n"
>>>>>> "bc <n/addr> clear breakpoint number n or at addr\n"
>>>>>> - "bi <addr> [cnt] set hardware instr breakpoint (POWER3/RS64 only)\n"
>>>>>> + "bi <addr> [cnt] set hardware instr breakpoint (POWER3/RS64/POWER8 only)\n"
>>>>>> "bd <addr> [cnt] set hardware data breakpoint\n"
>>>>>> "";
>>>>>>
>>>>>> @@ -1163,11 +1198,20 @@ bpt_cmds(void)
>>>>>> break;
>>>>>>
>>>>>> case 'i': /* bi - hardware instr breakpoint */
>>>>>> - if (!cpu_has_feature(CPU_FTR_IABR)) {
>>>>>> + if (!cpu_has_feature(CPU_FTR_IABR) && !cpu_has_feature(CPU_FTR_ARCH_207S)) {
>>>>>> printf("Hardware instruction breakpoint "
>>>>>> "not supported on this cpu\n");
>>>>>> break;
>>>>>> }
>>>>>> +
>>>>>> + if (cpu_has_feature(CPU_FTR_ARCH_207S)) {
>>>>>> + if (ciabr_used) {
>>>>>> + printf("One instruction breakpoint "
>>>>>> + "possible with CIABR\n");
>>>>>> + break;
>>>>>> + }
>>>>
>>>> We don't seem to do that with iabr ? Why keep ciabr different
>
>> Right now with the IABR implementation if we try to set hardware instruction
>> breakpoint while one is already there, it just get overridden with the new
>> address without complaining. I thought with this at least for CIABR cases,
>> it will complain about it and require the user to clear the breakpoint
>> explicitly before allowing a new breakpoint. Okay will remove this.
>>
>
> I tried removing the "ciabr_used" variable and all related checks/assignments on it.
> Then I was able to add all these three address as hardware instruction breakpoint
> and xmon never complained that it might override the existing actual checkpoint
> implemented with CIABR. It accepted all the three addresses which can later be
> listed as below.
>
> 0:mon> b
> type address
> 1 inst c0000000000830d0 .power_pmu_add+0x0/0x2e0
> 2 inst c000000000084690 .power_pmu_del+0x0/0x3a0
> 3 inst c000000000085060 .perf_event_interrupt+0x0/0x480
>
> But in reality, only perf_event_interrupt function's address got written into CIABR
> register and got triggered with the workload. I dont have a system which has IABR
> support to test it's behaviour for this situation. But this does not sound okay,
> we should explicitly inform the user that the hardware instruction breakpoint has
> been overridden with the latest command or reject the attempt. Looking for some
> suggestions in this regard. Thank you.
Hey Ben/MPE,
Any suggestions on this ^^^^^^^^^^^^ ?
^ permalink raw reply
* Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity
From: Zhang Yanfei @ 2014-06-12 8:28 UTC (permalink / raw)
To: Joonsoo Kim
Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
Marek Szyprowski
In-Reply-To: <1402543307-29800-6-git-send-email-iamjoonsoo.kim@lge.com>
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.kim@lge.com>
Acked-by: Zhang Yanfei <zhangyanfei@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 mutex lock;
> };
>
> @@ -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 *page = NULL;
> int ret;
>
> @@ -358,18 +386,19 @@ static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
> if (!count)
> return NULL;
>
> - mask = (1 << align) - 1;
> -
> + mask = cma_bitmap_aligned_mask(cma, align);
> + bitmap_maxno = cma_bitmap_maxno(cma);
> + nr_bits = cma_bitmap_pages_to_bits(cma, count);
>
> for (;;) {
> mutex_lock(&cma->lock);
> - pageno = bitmap_find_next_zero_area(cma->bitmap, cma->count,
> - start, count, mask);
> - if (pageno >= cma->count) {
> + bitmapno = bitmap_find_next_zero_area(cma->bitmap,
> + bitmap_maxno, start, nr_bits, mask);
> + if (bitmapno >= bitmap_maxno) {
> mutex_unlock(&cma->lock);
> break;
> }
> - bitmap_set(cma->bitmap, pageno, count);
> + bitmap_set(cma->bitmap, bitmapno, nr_bits);
> /*
> * It's safe to drop the lock here. We've marked this region for
> * our exclusive use. If the migration fails we will take the
> @@ -377,7 +406,7 @@ static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
> */
> mutex_unlock(&cma->lock);
>
> - pfn = cma->base_pfn + pageno;
> + pfn = cma->base_pfn + (bitmapno << cma->order_per_bit);
> mutex_lock(&cma_mutex);
> ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA);
> mutex_unlock(&cma_mutex);
> @@ -392,7 +421,7 @@ static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
> pr_debug("%s(): memory range at %p is busy, retrying\n",
> __func__, pfn_to_page(pfn));
> /* try again with a bit different memory target */
> - start = pageno + mask + 1;
> + start = bitmapno + mask + 1;
> }
>
> pr_debug("%s(): returned %p\n", __func__, page);
>
--
Thanks.
Zhang Yanfei
^ permalink raw reply
* Re: [PATCH v2 06/10] CMA: generalize CMA reserved area management functionality
From: Zhang Yanfei @ 2014-06-12 8:29 UTC (permalink / raw)
To: Joonsoo Kim
Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
Marek Szyprowski
In-Reply-To: <1402543307-29800-7-git-send-email-iamjoonsoo.kim@lge.com>
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 <mina86@mina86.com>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Acked-by: Zhang Yanfei <zhangyanfei@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 mutex lock;
> -};
> -
> -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 the entire CMA resv range
> - * to be in the same zone.
> - */
> - if (page_zone(pfn_to_page(pfn)) != zone)
> - 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;
> -}
> -
> -static struct cma cma_areas[MAX_CMA_AREAS];
> -static unsigned cma_area_count;
> -
> -static int __init cma_init_reserved_areas(void)
> -{
> - int i;
> -
> - for (i = 0; i < cma_area_count; i++) {
> - int ret = cma_activate_area(&cma_areas[i]);
> - if (ret)
> - return ret;
> - }
> -
> - return 0;
> -}
> -core_initcall(cma_init_reserved_areas);
> -
> -/**
> - * dma_contiguous_reserve_area() - reserve custom contiguous area
> - * @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
> - * @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
> - *
> - * This function reserves memory from early allocator. It should be
> - * called by arch specific code once the early allocator (memblock or bootmem)
> - * has been activated and all other subsystems have already allocated/reserved
> - * memory. This function allows to create custom reserved areas for specific
> - * devices.
> - *
> - * If @fixed is true, reserve contiguous area at exactly @base. If false,
> - * reserve in range from @base to @limit.
> - */
> -static int __init __dma_contiguous_reserve_area(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)
> -{
> - struct cma *cma = &cma_areas[cma_area_count];
> - int ret = 0;
> -
> - pr_debug("%s(size %lx, base %08lx, limit %08lx align_order %08lx)\n",
> - __func__, (unsigned long)size, (unsigned long)base,
> - (unsigned long)limit, (unsigned long)alignment);
> -
> - /* Sanity checks */
> - if (cma_area_count == ARRAY_SIZE(cma_areas)) {
> - pr_err("%s(): Not enough slots for CMA reserved regions!\n",
> - __func__);
> - return -ENOSPC;
> - }
> -
> - if (!size)
> - return -EINVAL;
> -
> - if (alignment && !is_power_of_2(alignment))
> - return -EINVAL;
> -
> - /*
> - * Sanitise input arguments.
> - * CMA area should be at least MAX_ORDER - 1 aligned. Otherwise,
> - * CMA area could be merged into other MIGRATE_TYPE by buddy mechanism
> - * and CMA property will be broken.
> - */
> - alignment = max(alignment,
> - (phys_addr_t)PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order));
> - 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) {
> - if (memblock_is_region_reserved(base, size) ||
> - memblock_reserve(base, size) < 0) {
> - ret = -EBUSY;
> - goto err;
> - }
> - } else {
> - phys_addr_t addr = memblock_alloc_range(size, alignment, base,
> - limit);
> - if (!addr) {
> - ret = -ENOMEM;
> - goto err;
> - } else {
> - base = addr;
> - }
> - }
> -
> - /*
> - * Each reserved area must be initialised later, when more kernel
> - * subsystems (like slab allocator) are available.
> - */
> - cma->base_pfn = PFN_DOWN(base);
> - cma->count = size >> PAGE_SHIFT;
> - cma->order_per_bit = order_per_bit;
> - *res_cma = cma;
> - cma_area_count++;
> -
> - pr_info("%s(): reserved %ld MiB at %08lx\n",
> - __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
> - return 0;
> -
> -err:
> - pr_err("%s(): failed to reserve %ld MiB\n",
> - __func__, (unsigned long)size / SZ_1M);
> - return ret;
> -}
> -
> int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
> phys_addr_t limit, struct cma **res_cma,
> bool fixed)
> {
> int ret;
>
> - ret = __dma_contiguous_reserve_area(size, base, limit, 0, 0,
> - res_cma, fixed);
> + ret = cma_declare_contiguous(size, base, limit, 0, 0, res_cma, fixed);
> if (ret)
> return ret;
>
> @@ -358,124 +159,17 @@ int __init dma_contiguous_reserve_area(phys_addr_t size, phys_addr_t base,
> return 0;
> }
>
> -/**
> - * dma_alloc_from_contiguous() - allocate pages from contiguous area
> - * @dev: Pointer to device for which the allocation is performed.
> - * @count: Requested number of pages.
> - * @align: Requested alignment of pages (in PAGE_SIZE order).
> - *
> - * This function allocates memory buffer for specified device. It uses
> - * device specific contiguous memory area if available or the default
> - * global one. Requires architecture specific dev_get_cma_area() helper
> - * function.
> - */
> -static struct page *__dma_alloc_from_contiguous(struct cma *cma, int count,
> - unsigned int align)
> -{
> - unsigned long mask, pfn, start = 0;
> - unsigned long bitmap_maxno, bitmapno, nr_bits;
> - struct page *page = NULL;
> - int ret;
> -
> - if (!cma || !cma->count)
> - return NULL;
> -
> - pr_debug("%s(cma %p, count %d, align %d)\n", __func__, (void *)cma,
> - count, align);
> -
> - if (!count)
> - return NULL;
> -
> - mask = cma_bitmap_aligned_mask(cma, align);
> - bitmap_maxno = cma_bitmap_maxno(cma);
> - nr_bits = cma_bitmap_pages_to_bits(cma, count);
> -
> - for (;;) {
> - mutex_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);
> - break;
> - }
> - bitmap_set(cma->bitmap, bitmapno, nr_bits);
> - /*
> - * It's safe to drop the lock here. We've marked this region for
> - * our exclusive use. If the migration fails we will take the
> - * lock again and unmark it.
> - */
> - mutex_unlock(&cma->lock);
> -
> - pfn = cma->base_pfn + (bitmapno << cma->order_per_bit);
> - mutex_lock(&cma_mutex);
> - ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA);
> - mutex_unlock(&cma_mutex);
> - 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);
> - pr_debug("%s(): memory range at %p is busy, retrying\n",
> - __func__, pfn_to_page(pfn));
> - /* try again with a bit different memory target */
> - start = bitmapno + mask + 1;
> - }
> -
> - pr_debug("%s(): returned %p\n", __func__, page);
> - return page;
> -}
> -
> struct page *dma_alloc_from_contiguous(struct device *dev, int count,
> unsigned int align)
> {
> - struct cma *cma = dev_get_cma_area(dev);
> -
> if (align > CONFIG_CMA_ALIGNMENT)
> align = CONFIG_CMA_ALIGNMENT;
>
> - return __dma_alloc_from_contiguous(cma, count, align);
> -}
> -
> -/**
> - * dma_release_from_contiguous() - release allocated pages
> - * @dev: Pointer to device for which the pages were allocated.
> - * @pages: Allocated pages.
> - * @count: Number of allocated pages.
> - *
> - * This function releases memory allocated by dma_alloc_from_contiguous().
> - * It returns false when provided pages do not belong to contiguous area and
> - * true otherwise.
> - */
> -static bool __dma_release_from_contiguous(struct cma *cma, struct page *pages,
> - int count)
> -{
> - unsigned long pfn;
> -
> - if (!cma || !pages)
> - return false;
> -
> - pr_debug("%s(page %p)\n", __func__, (void *)pages);
> -
> - pfn = page_to_pfn(pages);
> -
> - if (pfn < cma->base_pfn || pfn >= cma->base_pfn + cma->count)
> - return false;
> -
> - VM_BUG_ON(pfn + count > cma->base_pfn + cma->count);
> -
> - free_contig_range(pfn, count);
> - clear_cma_bitmap(cma, pfn, count);
> -
> - return true;
> + return cma_alloc(dev_get_cma_area(dev), count, align);
> }
>
> bool dma_release_from_contiguous(struct device *dev, struct page *pages,
> int count)
> {
> - struct cma *cma = dev_get_cma_area(dev);
> -
> - return __dma_release_from_contiguous(cma, pages, count);
> + return cma_release(dev_get_cma_area(dev), pages, count);
> }
> diff --git a/include/linux/cma.h b/include/linux/cma.h
> new file mode 100644
> index 0000000..e38efe9
> --- /dev/null
> +++ b/include/linux/cma.h
> @@ -0,0 +1,12 @@
> +#ifndef __CMA_H__
> +#define __CMA_H__
> +
> +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);
> +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/include/linux/dma-contiguous.h b/include/linux/dma-contiguous.h
> index 772eab5..a40c1f3 100644
> --- a/include/linux/dma-contiguous.h
> +++ b/include/linux/dma-contiguous.h
> @@ -53,9 +53,10 @@
>
> #ifdef __KERNEL__
>
> +#include <linux/device.h>
> +
> struct cma;
> struct page;
> -struct device;
>
> #ifdef CONFIG_DMA_CMA
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 3e9977a..f4899ec 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -508,6 +508,17 @@ config CMA_DEBUG
> processing calls such as dma_alloc_from_contiguous().
> This option does not affect warning and error messages.
>
> +config CMA_AREAS
> + int "Maximum count of the CMA areas"
> + depends on CMA
> + default 7
> + help
> + CMA allows to create CMA areas for particular purpose, mainly,
> + used as device private area. This parameter sets the maximum
> + number of CMA area in the system.
> +
> + If unsure, leave the default value "7".
> +
> config ZBUD
> tristate
> default n
> diff --git a/mm/Makefile b/mm/Makefile
> index 1eaa70b..bc0422b 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -62,3 +62,4 @@ obj-$(CONFIG_MEMORY_ISOLATION) += page_isolation.o
> obj-$(CONFIG_ZBUD) += zbud.o
> obj-$(CONFIG_ZSMALLOC) += zsmalloc.o
> obj-$(CONFIG_GENERIC_EARLY_IOREMAP) += early_ioremap.o
> +obj-$(CONFIG_CMA) += cma.o
> diff --git a/mm/cma.c b/mm/cma.c
> new file mode 100644
> index 0000000..1e1b017
> --- /dev/null
> +++ b/mm/cma.c
> @@ -0,0 +1,330 @@
> +/*
> + * Contiguous Memory Allocator
> + *
> + * Copyright (c) 2010-2011 by Samsung Electronics.
> + * Copyright IBM Corporation, 2013
> + * Copyright LG Electronics Inc., 2014
> + * Written by:
> + * Marek Szyprowski <m.szyprowski@samsung.com>
> + * Michal Nazarewicz <mina86@mina86.com>
> + * Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> + * Joonsoo Kim <iamjoonsoo.kim@lge.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) "cma: " fmt
> +
> +#ifdef CONFIG_CMA_DEBUG
> +#ifndef DEBUG
> +# define DEBUG
> +#endif
> +#endif
> +
> +#include <linux/memblock.h>
> +#include <linux/err.h>
> +#include <linux/mm.h>
> +#include <linux/mutex.h>
> +#include <linux/sizes.h>
> +#include <linux/slab.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 mutex lock;
> +};
> +
> +/*
> + * There is always at least global CMA area and a few optional
> + * areas configured in kernel .config.
> + */
> +#define MAX_CMA_AREAS (1 + CONFIG_CMA_AREAS)
> +
> +static struct cma cma_areas[MAX_CMA_AREAS];
> +static unsigned cma_area_count;
> +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 the entire CMA resv range
> + * to be in the same zone.
> + */
> + if (page_zone(pfn_to_page(pfn)) != zone)
> + 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;
> +}
> +
> +static int __init cma_init_reserved_areas(void)
> +{
> + int i;
> +
> + for (i = 0; i < cma_area_count; i++) {
> + int ret = cma_activate_area(&cma_areas[i]);
> +
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +core_initcall(cma_init_reserved_areas);
> +
> +/**
> + * cma_declare_contiguous() - reserve custom contiguous area
> + * @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
> + * @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
> + *
> + * This function reserves memory from early allocator. It should be
> + * called by arch specific code once the early allocator (memblock or bootmem)
> + * has been activated and all other subsystems have already allocated/reserved
> + * memory. This function allows to create custom reserved areas.
> + *
> + * If @fixed is true, reserve contiguous area at exactly @base. If false,
> + * reserve in range from @base to @limit.
> + */
> +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)
> +{
> + struct cma *cma = &cma_areas[cma_area_count];
> + int ret = 0;
> +
> + pr_debug("%s(size %lx, base %08lx, limit %08lx alignment %08lx)\n",
> + __func__, (unsigned long)size, (unsigned long)base,
> + (unsigned long)limit, (unsigned long)alignment);
> +
> + /* Sanity checks */
> + if (cma_area_count == ARRAY_SIZE(cma_areas)) {
> + pr_err("%s(): Not enough slots for CMA reserved regions!\n",
> + __func__);
> + return -ENOSPC;
> + }
> +
> + if (!size)
> + return -EINVAL;
> +
> + if (alignment && !is_power_of_2(alignment))
> + return -EINVAL;
> +
> + /*
> + * Sanitise input arguments.
> + * CMA area should be at least MAX_ORDER - 1 aligned. Otherwise,
> + * CMA area could be merged into other MIGRATE_TYPE by buddy mechanism
> + * and CMA property will be broken.
> + */
> + alignment = max(alignment,
> + (phys_addr_t)PAGE_SIZE << max(MAX_ORDER - 1, pageblock_order));
> + 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) {
> + if (memblock_is_region_reserved(base, size) ||
> + memblock_reserve(base, size) < 0) {
> + ret = -EBUSY;
> + goto err;
> + }
> + } else {
> + phys_addr_t addr = memblock_alloc_range(size, alignment, base,
> + limit);
> + if (!addr) {
> + ret = -ENOMEM;
> + goto err;
> + } else {
> + base = addr;
> + }
> + }
> +
> + /*
> + * Each reserved area must be initialised later, when more kernel
> + * subsystems (like slab allocator) are available.
> + */
> + cma->base_pfn = PFN_DOWN(base);
> + cma->count = size >> PAGE_SHIFT;
> + cma->order_per_bit = order_per_bit;
> + *res_cma = cma;
> + cma_area_count++;
> +
> + pr_info("%s(): reserved %ld MiB at %08lx\n",
> + __func__, (unsigned long)size / SZ_1M, (unsigned long)base);
> + return 0;
> +
> +err:
> + pr_err("%s(): failed to reserve %ld MiB\n",
> + __func__, (unsigned long)size / SZ_1M);
> + return ret;
> +}
> +
> +/**
> + * cma_alloc() - allocate pages from contiguous area
> + * @cma: Contiguous memory region for which the allocation is performed.
> + * @count: Requested number of pages.
> + * @align: Requested alignment of pages (in PAGE_SIZE order).
> + *
> + * This function allocates part of contiguous memory on specific
> + * contiguous memory area.
> + */
> +struct page *cma_alloc(struct cma *cma, int count, unsigned int align)
> +{
> + unsigned long mask, pfn, start = 0;
> + unsigned long bitmap_maxno, bitmapno, nr_bits;
> + struct page *page = NULL;
> + int ret;
> +
> + if (!cma || !cma->count)
> + return NULL;
> +
> + pr_debug("%s(cma %p, count %d, align %d)\n", __func__, (void *)cma,
> + count, align);
> +
> + if (!count)
> + return NULL;
> +
> + mask = cma_bitmap_aligned_mask(cma, align);
> + bitmap_maxno = cma_bitmap_maxno(cma);
> + nr_bits = cma_bitmap_pages_to_bits(cma, count);
> +
> + for (;;) {
> + mutex_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);
> + break;
> + }
> + bitmap_set(cma->bitmap, bitmapno, nr_bits);
> + /*
> + * It's safe to drop the lock here. We've marked this region for
> + * our exclusive use. If the migration fails we will take the
> + * lock again and unmark it.
> + */
> + mutex_unlock(&cma->lock);
> +
> + pfn = cma->base_pfn + (bitmapno << cma->order_per_bit);
> + mutex_lock(&cma_mutex);
> + ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA);
> + mutex_unlock(&cma_mutex);
> + 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);
> + pr_debug("%s(): memory range at %p is busy, retrying\n",
> + __func__, pfn_to_page(pfn));
> + /* try again with a bit different memory target */
> + start = bitmapno + mask + 1;
> + }
> +
> + pr_debug("%s(): returned %p\n", __func__, page);
> + return page;
> +}
> +
> +/**
> + * cma_release() - release allocated pages
> + * @cma: Contiguous memory region for which the allocation is performed.
> + * @pages: Allocated pages.
> + * @count: Number of allocated pages.
> + *
> + * This function releases memory allocated by alloc_cma().
> + * It returns false when provided pages do not belong to contiguous area and
> + * true otherwise.
> + */
> +bool cma_release(struct cma *cma, struct page *pages, int count)
> +{
> + unsigned long pfn;
> +
> + if (!cma || !pages)
> + return false;
> +
> + pr_debug("%s(page %p)\n", __func__, (void *)pages);
> +
> + pfn = page_to_pfn(pages);
> +
> + if (pfn < cma->base_pfn || pfn >= cma->base_pfn + cma->count)
> + return false;
> +
> + VM_BUG_ON(pfn + count > cma->base_pfn + cma->count);
> +
> + free_contig_range(pfn, count);
> + clear_cma_bitmap(cma, pfn, count);
> +
> + return true;
> +}
>
--
Thanks.
Zhang Yanfei
^ permalink raw reply
* Re: [PATCH v2 08/10] mm, cma: clean-up cma allocation error path
From: Zhang Yanfei @ 2014-06-12 8:31 UTC (permalink / raw)
To: Joonsoo Kim
Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
Paolo Bonzini, Andrew Morton, linuxppc-dev, linux-arm-kernel,
Marek Szyprowski
In-Reply-To: <1402543307-29800-9-git-send-email-iamjoonsoo.kim@lge.com>
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.kim@lge.com>
Reviewed-by: Zhang Yanfei <zhangyanfei@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
^ permalink raw reply
* Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
From: Michal Nazarewicz @ 2014-06-12 8:55 UTC (permalink / raw)
To: Joonsoo Kim, Aneesh Kumar K.V
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
Minchan Kim, Paul Mackerras, Paolo Bonzini, Andrew Morton,
linuxppc-dev, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20140612055358.GA30128@js1304-P5Q-DELUXE>
>> Joonsoo Kim <iamjoonsoo.kim@lge.com> writes:
>>=20
>> > 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.kim@lge.com>
>> >
>> > diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguo=
us.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 lim=
it)
>> > }
>> >
>> > 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.kim@lge.com> wrote:
> I also haven't seen this format in other kernel code, but, in cma, they u=
se
> 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 =E2=80=9Cfunction(arg1, arg2, =E2=80=A6)=E2=80=9D 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 =E2=80=9Cfunction:=E2=80=9D (or just no a=
dditional
prefix). IMO that's a reasonable strategy.
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply
* Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
From: Michal Nazarewicz @ 2014-06-12 8:56 UTC (permalink / raw)
To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-2-git-send-email-iamjoonsoo.kim@lge.com>
On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@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 <mina86@mina86.com>
even though I'd go without empty =E2=80=9C()=E2=80=9D.
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@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)
> }
>=20=20
> 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);
>=20=20
> dma_contiguous_reserve_area(selected_size, selected_base,
> @@ -163,8 +163,9 @@ static int __init cma_activate_area(struct cma *cma)
> unsigned i =3D cma->count >> pageblock_order;
> struct zone *zone;
>=20=20
> - cma->bitmap =3D kzalloc(bitmap_size, GFP_KERNEL);
> + pr_debug("%s()\n", __func__);
>=20=20
> + cma->bitmap =3D kzalloc(bitmap_size, GFP_KERNEL);
> if (!cma->bitmap)
> return -ENOMEM;
>=20=20
> @@ -234,7 +235,8 @@ int __init dma_contiguous_reserve_area(phys_addr_t si=
ze, phys_addr_t base,
>=20=20
> /* Sanity checks */
> if (cma_area_count =3D=3D 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;
> }
>=20=20
> @@ -274,14 +276,15 @@ int __init dma_contiguous_reserve_area(phys_addr_t =
size, phys_addr_t base,
> *res_cma =3D cma;
> cma_area_count++;
>=20=20
> - 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);
>=20=20
> /* 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;
> }
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply
* Re: [PATCH V3 0/3] Add new PowerPC specific ELF core notes
From: Anshuman Khandual @ 2014-06-12 9:09 UTC (permalink / raw)
To: Anshuman Khandual, Benjamin Herrenschmidt
Cc: Michael Neuling, james.hogan, avagin, Paul.Clothier, davem,
peterz, palves, linux-kernel, oleg, dhowells, linuxppc-dev, davej,
akpm, tglx
In-Reply-To: <1400858138-3939-1-git-send-email-khandual@linux.vnet.ibm.com>
On 05/23/2014 08:45 PM, Anshuman Khandual wrote:
> This patch series adds five new ELF core note sections which can be
> used with existing ptrace request PTRACE_GETREGSET/SETREGSET for accessing
> various transactional memory and miscellaneous register sets on PowerPC
> platform. Please find a test program exploiting these new ELF core note
> types on a POWER8 system.
>
> RFC: https://lkml.org/lkml/2014/4/1/292
> V1: https://lkml.org/lkml/2014/4/2/43
> V2: https://lkml.org/lkml/2014/5/5/88
>
> Changes in V3
> =============
> (1) Added two new error paths in every TM related get/set functions when regset
> support is not present on the system (ENODEV) or when the process does not
> have any transaction active (ENODATA) in the context
>
> (2) Installed the active hooks for all the newly added regset core note types
>
> Changes in V2
> =============
> (1) Removed all the power specific ptrace requests corresponding to new NT_PPC_*
> elf core note types. Now all the register sets can be accessed from ptrace
> through PTRACE_GETREGSET/PTRACE_SETREGSET using the individual NT_PPC* core
> note type instead
> (2) Fixed couple of attribute values for REGSET_TM_CGPR register set
> (3) Renamed flush_tmreg_to_thread as flush_tmregs_to_thread
> (4) Fixed 32 bit checkpointed GPR support
> (5) Changed commit messages accordingly
>
> Outstanding Issues
> ==================
> (1) Running DSCR register value inside a transaction does not seem to be saved
> at thread.dscr when the process stops for ptrace examination.
Hey Ben,
Any updates on this patch series ?
^ permalink raw reply
* Re: [PATCH v2 02/10] DMA, CMA: fix possible memory leak
From: Michal Nazarewicz @ 2014-06-12 9:47 UTC (permalink / raw)
To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-3-git-send-email-iamjoonsoo.kim@lge.com>
On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@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.kim@lge.com>
Reviewed-by: Michal Nazarewicz <mina86@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 =3D pfn;
> for (j =3D 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)) !=3D zone)
> - return -EINVAL;
> + goto err;
> }
> init_cma_reserved_pageblock(pfn_to_page(base_pfn));
> } while (--i);
>=20=20
> mutex_init(&cma->lock);
> return 0;
> +
> +err:
> + kfree(cma->bitmap);
> + return -EINVAL;
> }
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply
* Re: [PATCH v2 01/10] DMA, CMA: clean-up log message
From: Michal Nazarewicz @ 2014-06-12 9:53 UTC (permalink / raw)
To: Joonsoo Kim, Aneesh Kumar K.V
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
Minchan Kim, Paul Mackerras, Paolo Bonzini, Andrew Morton,
linuxppc-dev, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <xa1toaxyjym3.fsf@mina86.com>
On Thu, Jun 12 2014, Michal Nazarewicz <mina86@mina86.com> wrote:
> I used =E2=80=9Cfunction(arg1, arg2, =E2=80=A6)=E2=80=9D at the *beginnin=
g* of functions when
> the arguments passed to the function were included in the message. In
> all other cases I left it at just =E2=80=9Cfunction:=E2=80=9D (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 =E2=80=9Cfunction(=E2=80=A6)=E2=80=9D only=
at the beginning of
a function. Just my 0.02 CHF.
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply
* Re: [PATCH v2 03/10] DMA, CMA: separate core cma management codes from DMA APIs
From: Michal Nazarewicz @ 2014-06-12 9:55 UTC (permalink / raw)
To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-4-git-send-email-iamjoonsoo.kim@lge.com>
On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@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.kim@lge.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply
* Re: [PATCH v2 04/10] DMA, CMA: support alignment constraint on cma region
From: Michal Nazarewicz @ 2014-06-12 10:02 UTC (permalink / raw)
To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-5-git-send-email-iamjoonsoo.kim@lge.com>
On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> ppc kvm's cma area management needs alignment constraint on
I've noticed it earlier and cannot seem to get to terms with this. It
should IMO be PPC, KVM and CMA since those are acronyms. But if you
have strong feelings, it's not a big issue.
> 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.kim@lge.com>
Acked-by: Michal Nazarewicz <mina86@mina86.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
> @@ -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
=E2=80=9Cmust be power of 2 or zero=E2=80=9D.
> * @res_cma: Pointer to store the created cma region.
> * @fixed: hint about where to place the reserved area
> *
> @@ -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 =3D &cma_areas[cma_area_count];
> - phys_addr_t alignment;
> int ret =3D 0;
>=20=20
> - 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",
> + __func__, (unsigned long)size, (unsigned long)base,
> + (unsigned long)limit, (unsigned long)alignment);
Nit: Align with the rest of the arguments, i.e.:
+ pr_debug("%s(size %lx, base %08lx, limit %08lx align_order %08lx)\n",
+ __func__, (unsigned long)size, (unsigned long)base,
+ (unsigned long)limit, (unsigned long)alignment);
>=20=20
> /* Sanity checks */
> if (cma_area_count =3D=3D ARRAY_SIZE(cma_areas)) {
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply
* Re: [PATCH v2 05/10] DMA, CMA: support arbitrary bitmap granularity
From: Michal Nazarewicz @ 2014-06-12 10:19 UTC (permalink / raw)
To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-6-git-send-email-iamjoonsoo.kim@lge.com>
On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@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.kim@lge.com>
Acked-by: Michal Nazarewicz <mina86@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 mutex lock;
> };
>=20=20
> +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 =3D (pfn - cma->base_pfn) >> cma->order_per_bit;
> + nr_bits =3D 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 =3D BITS_TO_LONGS(cma->count) * sizeof(long);
> + int bitmap_maxno =3D cma_bitmap_maxno(cma);
> + int bitmap_size =3D BITS_TO_LONGS(bitmap_maxno) * sizeof(long);
> unsigned long base_pfn =3D cma->base_pfn, pfn =3D base_pfn;
> unsigned i =3D cma->count >> pageblock_order;
> struct zone *zone;
bitmap_maxno is never used again, perhaps:
+ int bitmap_size =3D BITS_TO_LONGS(cma_bitmap_maxno(cma)) * sizeof(long);
instead? Up to you.
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply
* Re: [PATCH v2 08/10] mm, cma: clean-up cma allocation error path
From: Michal Nazarewicz @ 2014-06-12 11:34 UTC (permalink / raw)
To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-9-git-send-email-iamjoonsoo.kim@lge.com>
On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@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.kim@lge.com>
Reviewed-by: Michal Nazarewicz <mina86@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 =3D=3D 0) {
> page =3D pfn_to_page(pfn);
> break;
> - } else if (ret !=3D -EBUSY) {
> - clear_cma_bitmap(cma, pfn, count);
> - break;
> }
> +
> clear_cma_bitmap(cma, pfn, count);
> + if (ret !=3D -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 */
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply
* Re: [PATCH v2 09/10] mm, cma: move output param to the end of param list
From: Michal Nazarewicz @ 2014-06-12 11:38 UTC (permalink / raw)
To: Joonsoo Kim, Andrew Morton, Aneesh Kumar K.V, Marek Szyprowski
Cc: Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, kvm-ppc, linux-kernel,
Minchan Kim, Paul Mackerras, Paolo Bonzini, Joonsoo Kim,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <1402543307-29800-10-git-send-email-iamjoonsoo.kim@lge.com>
On Thu, Jun 12 2014, Joonsoo Kim <iamjoonsoo.kim@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.kim@lge.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
>
> diff --git a/arch/powerpc/kvm/book3s_hv_builtin.c b/arch/powerpc/kvm/book=
3s_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)
>=20=20
> align_size =3D 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);
> }
> }
>=20=20
> 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 si=
ze, phys_addr_t base,
> {
> int ret;
>=20=20
> - ret =3D cma_declare_contiguous(size, base, limit, 0, 0, res_cma, fixed);
> + ret =3D cma_declare_contiguous(size, base, limit, 0, 0, fixed, res_cma);
> if (ret)
> return ret;
>=20=20
> 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 a=
lign);
> 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 bo=
otmem)
> @@ -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 =3D &cma_areas[cma_area_count];
> + struct cma *cma;
> int ret =3D 0;
>=20=20
> 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 =3D &cma_areas[cma_area_count];
> cma->base_pfn =3D PFN_DOWN(base);
> cma->count =3D size >> PAGE_SHIFT;
> cma->order_per_bit =3D order_per_bit;
> --=20
> 1.7.9.5
>
--=20
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=3D./ `o
..o | Computer Science, Micha=C5=82 =E2=80=9Cmina86=E2=80=9D Nazarewicz =
(o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
^ permalink raw reply
* Re: Kernel build issues after yesterdays merge by Linus
From: Anton Blanchard @ 2014-06-12 12:09 UTC (permalink / raw)
To: Christoph Lameter; +Cc: Rusty Russell, linuxppc-dev
In-Reply-To: <alpine.DEB.2.10.1406111013580.8422@gentwo.org>
Hi Christoph,
> This is under Ubuntu Utopic Unicorn on a Power 8 system while simply
> trying to build with the Ubuntu standard kernel config. It could be
> that these issues come about because we do not have an rc1 yet but I
> wanted to give some early notice. Also this is a new arch to me so I
> may not be aware of how things work.
>
> 1. Bad relocation while building:
>
> root@rd-power8:/rdhome/clameter/linux# make
> CHK include/config/kernel.release
> CHK include/generated/uapi/linux/version.h
> CHK include/generated/utsrelease.h
> CALL scripts/checksyscalls.sh
> CHK include/generated/compile.h
> SKIPPED include/generated/compile.h
> CALL arch/powerpc/kernel/systbl_chk.sh
> CALL arch/powerpc/kernel/prom_init_check.sh
> CHK kernel/config_data.h
> CALL arch/powerpc/relocs_check.pl
> WARNING: 1 bad relocations
> c000000000cc7df0 R_PPC64_ADDR64 __crc_TOC.
The TOC symbol export is special and I think we might need to get our
script to ignore it.
> 2. "make install" fails
>
> root@rd-power8:/rdhome/clameter/linux# make install
> sh -x /rdhome/clameter/linux/arch/powerpc/boot/install.sh "3.15.0+"
> vmlinux System.map "/boot" arch/powerpc/boot/zImage.pseries
> arch/powerpc/boot/zImage.epapr
> + set -e
> + [ -x /home/clameter/bin/installkernel ]
> + [ -x /sbin/installkernel ]
> + exec /sbin/installkernel 3.15.0+ vmlinux System.map /boot
> arch/powerpc/boot/zImage.pseries arch/powerpc/boot/zImage.epapr
> Usage: installkernel <version> <image> <System.map> <directory>
> /rdhome/clameter/linux/arch/powerpc/boot/Makefile:393: recipe for target
> 'install' failed
> make[1]: *** [install] Error 1
> /rdhome/clameter/linux/arch/powerpc/Makefile:294: recipe for target
> 'install' failed
> make: *** [install] Error 2
Shows how much we use make install :) Below is a quick hack to get you going
but we should look at either fixing the Ubuntu installkernel to handle
extra optional args, or stop passing them from the ppc64 kernel
install.sh script.
Anton
--
diff --git a/arch/powerpc/boot/install.sh b/arch/powerpc/boot/install.sh
index b6a256b..e096e5a 100644
--- a/arch/powerpc/boot/install.sh
+++ b/arch/powerpc/boot/install.sh
@@ -23,8 +23,8 @@ set -e
# User may have a custom install script
-if [ -x ~/bin/${INSTALLKERNEL} ]; then exec ~/bin/${INSTALLKERNEL} "$@"; fi
-if [ -x /sbin/${INSTALLKERNEL} ]; then exec /sbin/${INSTALLKERNEL} "$@"; fi
+if [ -x ~/bin/${INSTALLKERNEL} ]; then exec ~/bin/${INSTALLKERNEL} $1 $2 $3 $4; fi
+if [ -x /sbin/${INSTALLKERNEL} ]; then exec /sbin/${INSTALLKERNEL} $1 $2 $3 $4; fi
# Default install
^ permalink raw reply related
* Re: powerpc/ppc64: Allow allmodconfig to build (finally !)
From: Guenter Roeck @ 2014-06-12 12:26 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1399874254.17624.85.camel@pasglop>
On Mon, May 12, 2014 at 03:57:34PM +1000, Benjamin Herrenschmidt wrote:
> This shuffles code around in exceptions-64s.S in order to
> allow an allmodconfig build to succeed.
>
> The main problems were:
>
> - We have a fixed hole from 0x7000 to 0x8000 for use by FW,
> under some circumstances the code before that would grow too
> big and hit the . = 0x7000
>
> - The various attempts at making space in there would trigger
> cases where short conditional branches from assembly would no
> longer be able to reach their target. This is especially nasty
> when these branches reside in alternate feature sections which
> are appended at the end of each .o file
>
> This fixes it by essentially moving all the "second level"
> exception handlers to after the hole and moving a couple of
> functions near the hole itself so they sit at reachable distance
> of both the first level handlers (before the hole) and the alternate
> feature sections (end of file).
>
> In the long run, if we start hitting this again, we'll probably
> have to split the file in two, probably at the hole location,
> to keep the alt sections used by the first level handlers close
> to them, and move everything else further away.
>
> But for now, this will do.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
Hi Ben,
what happened with this patch ?
Thanks,
Guenter
^ permalink raw reply
* Re: power and percpu: Could we move the paca into the percpu area?
From: Segher Boessenkool @ 2014-06-12 12:26 UTC (permalink / raw)
To: Gabriel Paubert; +Cc: Tejun Heo, linuxppc-dev, Christoph Lameter
In-Reply-To: <20140611210351.GA7155@visitor2.iram.es>
> Actually, from gcc/config/rs6000.h:
>
> /* 1 for registers that have pervasive standard uses
> and are not available for the register allocator.
[snip]
> So cr5, which is number 73, is never used by gcc.
Not available for RA is not the same thing at all as not used by GCC.
For example, GPR1 and XER[CA] are also fixed regs.
But, indeed, it does look like GCC doesn't use it. It seems to me that
some ABI forbade userland (or non-libraries or whatever) from using it.
I'll see what I can find out.
Segher
^ permalink raw reply
* RE: Kernel build issues after yesterdays merge by Linus
From: David Laight @ 2014-06-12 12:36 UTC (permalink / raw)
To: 'Anton Blanchard', Christoph Lameter
Cc: Rusty Russell, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20140612220912.3447677f@kryten>
RnJvbTogQW50b24gQmxhbmNoYXJkDQouLi4NCj4gZGlmZiAtLWdpdCBhL2FyY2gvcG93ZXJwYy9i
b290L2luc3RhbGwuc2ggYi9hcmNoL3Bvd2VycGMvYm9vdC9pbnN0YWxsLnNoDQo+IGluZGV4IGI2
YTI1NmIuLmUwOTZlNWEgMTAwNjQ0DQo+IC0tLSBhL2FyY2gvcG93ZXJwYy9ib290L2luc3RhbGwu
c2gNCj4gKysrIGIvYXJjaC9wb3dlcnBjL2Jvb3QvaW5zdGFsbC5zaA0KPiBAQCAtMjMsOCArMjMs
OCBAQCBzZXQgLWUNCj4gDQo+ICAjIFVzZXIgbWF5IGhhdmUgYSBjdXN0b20gaW5zdGFsbCBzY3Jp
cHQNCj4gDQo+IC1pZiBbIC14IH4vYmluLyR7SU5TVEFMTEtFUk5FTH0gXTsgdGhlbiBleGVjIH4v
YmluLyR7SU5TVEFMTEtFUk5FTH0gIiRAIjsgZmkNCj4gLWlmIFsgLXggL3NiaW4vJHtJTlNUQUxM
S0VSTkVMfSBdOyB0aGVuIGV4ZWMgL3NiaW4vJHtJTlNUQUxMS0VSTkVMfSAiJEAiOyBmaQ0KPiAr
aWYgWyAteCB+L2Jpbi8ke0lOU1RBTExLRVJORUx9IF07IHRoZW4gZXhlYyB+L2Jpbi8ke0lOU1RB
TExLRVJORUx9ICQxICQyICQzICQ0OyBmaQ0KPiAraWYgWyAteCAvc2Jpbi8ke0lOU1RBTExLRVJO
RUx9IF07IHRoZW4gZXhlYyAvc2Jpbi8ke0lOU1RBTExLRVJORUx9ICQxICQyICQzICQ0OyBmaQ0K
DQpZb3UgcHJvYmFibHkgd2FudCB0byBlbmNsb3NlIHRoZSAkMSBpbiAiIGFzOg0KDQo+ICtpZiBb
IC14IC9zYmluLyR7SU5TVEFMTEtFUk5FTH0gXTsgdGhlbiBleGVjIC9zYmluLyR7SU5TVEFMTEtF
Uk5FTH0gIiQxIiAiJDIiICIkMyIgIiQ0IjsgZmkNCg0KCURhdmlkDQoNCg==
^ permalink raw reply
* [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule
From: Mihai Caraman @ 2014-06-12 14:00 UTC (permalink / raw)
To: kvm-ppc; +Cc: Mihai Caraman, linuxppc-dev, kvm
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>
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);
}
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH] powerpc: Add AT_HWCAP2 to indicate V.CRYPTO category support
From: Steve Munroe @ 2014-06-12 15:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Adhemerval Zanella, Michael J Wolf,
Hanns-Joachim Uhl
In-Reply-To: <1402376680.14780.12.camel@pasglop>
[-- Attachment #1.1: Type: text/plain, Size: 3660 bytes --]
The precedent we have used is that features that refer to ISA Categories
include _HAS_ following the PPC_FEATURE[2]. So GLIBC specifies in hwcap.h:
/* Feature definitions in AT_HWCAP2. */
#define PPC_FEATURE2_ARCH_2_07 0x80000000 /* ISA 2.07 */
#define PPC_FEATURE2_HAS_HTM 0x40000000 /* Hardware Transactional
Memory */
#define PPC_FEATURE2_HAS_DSCR 0x20000000 /* Data Stream Control
Register */
#define PPC_FEATURE2_HAS_EBB 0x10000000 /* Event Base Branching */
#define PPC_FEATURE2_HAS_ISEL 0x08000000 /* Integer Select */
#define PPC_FEATURE2_HAS_TAR 0x04000000 /* Target Address Register */
This was carried from the original AT_HWCAP defines.
$ grep _HAS_ ./sysdeps/powerpc/bits/hwcap.h
#define PPC_FEATURE_HAS_ALTIVEC 0x10000000 /* SIMD/Vector Unit. */
#define PPC_FEATURE_HAS_FPU 0x08000000 /* Floating Point Unit. */
#define PPC_FEATURE_HAS_MMU 0x04000000 /* Memory Management Unit. */
#define PPC_FEATURE_HAS_4xxMAC 0x02000000 /* 4xx Multiply
Accumulator. */
#define PPC_FEATURE_HAS_SPE 0x00800000 /* Signal Processing ext. */
#define PPC_FEATURE_HAS_EFP_SINGLE 0x00400000 /* SPE Float. */
#define PPC_FEATURE_HAS_EFP_DOUBLE 0x00200000 /* SPE Double. */
#define PPC_FEATURE_HAS_DFP 0x00000400 /* Decimal FP Unit */
#define PPC_FEATURE_HAS_VSX 0x00000080 /* P7 Vector Extension. */
So could we agree on PPC_FEATURE2_HAS_VEC_CRYPTO or the shorter
PPC_FEATURE2_HAS_VCRYPTO?
Steven J. Munroe
Linux on Power Toolchain Architect
IBM Corporation, Linux Technology Center
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: linuxppc-dev@ozlabs.org
Cc: Steve Munroe/Rochester/IBM@IBMUS, Hanns-Joachim Uhl
<hannsj_uhl@de.ibm.com>, Diane Brent/Poughkeepsie/IBM@IBMUS,
Jeffrey Scheel/Rochester/IBM@IBMUS, Adhemerval Zanella
<azanella@linux.vnet.ibm.com>
Date: 06/10/2014 12:05 AM
Subject: [PATCH] powerpc: Add AT_HWCAP2 to indicate V.CRYPTO category
support
The Vector Crypto category instructions are supported by current POWER8
chips, advertise them to userspace using a specific bit to properly
differentiate with chips of the same architecture level that might not
have them.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org> [v3.10+]
--
diff --git a/arch/powerpc/include/uapi/asm/cputable.h
b/arch/powerpc/include/uapi/asm/cputable.h
index 5b76579..de2c0e4 100644
--- a/arch/powerpc/include/uapi/asm/cputable.h
+++ b/arch/powerpc/include/uapi/asm/cputable.h
@@ -41,5 +41,6 @@
#define PPC_FEATURE2_EBB 0x10000000
#define PPC_FEATURE2_ISEL 0x08000000
#define PPC_FEATURE2_TAR 0x04000000
+#define PPC_FEATURE2_VEC_CRYPTO 0x02000000
#endif /* _UAPI__ASM_POWERPC_CPUTABLE_H */
diff --git a/arch/powerpc/kernel/cputable.c
b/arch/powerpc/kernel/cputable.c
index c1faade..11da04a 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -109,7 +109,8 @@ extern void __restore_cpu_e6500(void);
PPC_FEATURE_PSERIES_PERFMON_COMPAT)
#define COMMON_USER2_POWER8 (PPC_FEATURE2_ARCH_2_07 | \
PPC_FEATURE2_HTM_COMP |
PPC_FEATURE2_DSCR | \
- PPC_FEATURE2_ISEL |
PPC_FEATURE2_TAR)
+ PPC_FEATURE2_ISEL |
PPC_FEATURE2_TAR | \
+ PPC_FEATURE2_VEC_CRYPTO)
#define COMMON_USER_PA6T (COMMON_USER_PPC64 | PPC_FEATURE_PA6T
|\
PPC_FEATURE_TRUE_LE | \
PPC_FEATURE_HAS_ALTIVEC_COMP)
[-- Attachment #1.2: Type: text/html, Size: 6435 bytes --]
[-- Attachment #2: graycol.gif --]
[-- Type: image/gif, Size: 105 bytes --]
^ permalink raw reply related
* Re: [PATCH 3/4 v3] KVM: PPC: Alow kvmppc_get_last_inst() to fail
From: Alexander Graf @ 2014-06-12 15:15 UTC (permalink / raw)
To: Mihai Caraman; +Cc: linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1401724216-26486-4-git-send-email-mihai.caraman@freescale.com>
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.caraman@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 path */
> + if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED)
> + ret = kvmppc_load_last_inst(vcpu, prev, &vcpu->arch.last_inst);
> +
> + *inst = kvmppc_need_byteswap(vcpu) ? swab32(vcpu->arch.last_inst) :
> + vcpu->arch.last_inst;
> +
> + return ret;
> +}
> +
> static inline bool is_kvmppc_hv_enabled(struct kvm *kvm)
> {
> return kvm->arch.kvm_ops == kvmppc_hv_ops;
> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> index 8056107..2ffb3dd 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> @@ -530,21 +530,13 @@ static int instruction_is_store(unsigned int instr)
> static int kvmppc_hv_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu,
> unsigned long gpa, gva_t ea, int is_store)
> {
> - int ret;
> u32 last_inst;
> - unsigned long srr0 = kvmppc_get_pc(vcpu);
>
> - /* We try to load the last instruction. We don't let
> - * emulate_instruction do it as it doesn't check what
> - * kvmppc_ld returns.
> + /*
> * If we fail, we just return to the guest and try executing it again.
> */
> - if (vcpu->arch.last_inst == KVM_INST_FETCH_FAILED) {
> - ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
> - if (ret != EMULATE_DONE || last_inst == KVM_INST_FETCH_FAILED)
> - return RESUME_GUEST;
> - vcpu->arch.last_inst = last_inst;
> - }
> + if (kvmppc_get_last_inst(vcpu, false, &last_inst) != EMULATE_DONE)
> + return RESUME_GUEST;
>
> /*
> * WARNING: We do not know for sure whether the instruction we just
> @@ -558,7 +550,7 @@ static int kvmppc_hv_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu,
> * we just return and retry the instruction.
> */
>
> - if (instruction_is_store(kvmppc_get_last_inst(vcpu)) != !!is_store)
> + if (instruction_is_store(last_inst) != !!is_store)
> return RESUME_GUEST;
>
> /*
> diff --git a/arch/powerpc/kvm/book3s_paired_singles.c b/arch/powerpc/kvm/book3s_paired_singles.c
> index 6c8011f..6362ad4 100644
> --- a/arch/powerpc/kvm/book3s_paired_singles.c
> +++ b/arch/powerpc/kvm/book3s_paired_singles.c
> @@ -639,26 +639,36 @@ static int kvmppc_ps_one_in(struct kvm_vcpu *vcpu, bool rc,
>
> int kvmppc_emulate_paired_single(struct kvm_run *run, struct kvm_vcpu *vcpu)
> {
> - u32 inst = kvmppc_get_last_inst(vcpu);
> + u32 inst;
> enum emulation_result emulated = EMULATE_DONE;
> + int ax_rd, ax_ra, ax_rb, ax_rc;
> + short full_d;
> + u64 *fpr_d, *fpr_a, *fpr_b, *fpr_c;
>
> - int ax_rd = inst_get_field(inst, 6, 10);
> - int ax_ra = inst_get_field(inst, 11, 15);
> - int ax_rb = inst_get_field(inst, 16, 20);
> - int ax_rc = inst_get_field(inst, 21, 25);
> - short full_d = inst_get_field(inst, 16, 31);
> -
> - u64 *fpr_d = &VCPU_FPR(vcpu, ax_rd);
> - u64 *fpr_a = &VCPU_FPR(vcpu, ax_ra);
> - u64 *fpr_b = &VCPU_FPR(vcpu, ax_rb);
> - u64 *fpr_c = &VCPU_FPR(vcpu, ax_rc);
> -
> - bool rcomp = (inst & 1) ? true : false;
> - u32 cr = kvmppc_get_cr(vcpu);
> + bool rcomp;
> + u32 cr;
> #ifdef DEBUG
> int i;
> #endif
>
> + emulated = kvmppc_get_last_inst(vcpu, false, &inst);
> + if (emulated != EMULATE_DONE)
> + return emulated;
> +
> + ax_rd = inst_get_field(inst, 6, 10);
> + ax_ra = inst_get_field(inst, 11, 15);
> + ax_rb = inst_get_field(inst, 16, 20);
> + ax_rc = inst_get_field(inst, 21, 25);
> + full_d = inst_get_field(inst, 16, 31);
> +
> + fpr_d = &VCPU_FPR(vcpu, ax_rd);
> + fpr_a = &VCPU_FPR(vcpu, ax_ra);
> + fpr_b = &VCPU_FPR(vcpu, ax_rb);
> + fpr_c = &VCPU_FPR(vcpu, ax_rc);
> +
> + rcomp = (inst & 1) ? true : false;
> + cr = kvmppc_get_cr(vcpu);
> +
> if (!kvmppc_inst_is_paired_single(vcpu, inst))
> return EMULATE_FAIL;
>
> diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
> index 23367a7..48b633c 100644
> --- a/arch/powerpc/kvm/book3s_pr.c
> +++ b/arch/powerpc/kvm/book3s_pr.c
> @@ -637,42 +637,6 @@ static void kvmppc_giveup_fac(struct kvm_vcpu *vcpu, ulong fac)
> #endif
> }
>
> -static int kvmppc_read_inst(struct kvm_vcpu *vcpu)
> -{
> - ulong srr0 = kvmppc_get_pc(vcpu);
> - u32 last_inst = kvmppc_get_last_inst(vcpu);
> - int ret;
> -
> - ret = kvmppc_ld(vcpu, &srr0, sizeof(u32), &last_inst, false);
> - if (ret == -ENOENT) {
> - ulong msr = kvmppc_get_msr(vcpu);
> -
> - msr = kvmppc_set_field(msr, 33, 33, 1);
> - msr = kvmppc_set_field(msr, 34, 36, 0);
> - msr = kvmppc_set_field(msr, 42, 47, 0);
> - kvmppc_set_msr_fast(vcpu, msr);
> - kvmppc_book3s_queue_irqprio(vcpu, BOOK3S_INTERRUPT_INST_STORAGE);
> - return EMULATE_AGAIN;
> - }
> -
> - return EMULATE_DONE;
> -}
> -
> -static int kvmppc_check_ext(struct kvm_vcpu *vcpu, unsigned int exit_nr)
> -{
> -
> - /* Need to do paired single emulation? */
> - if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE))
> - return EMULATE_DONE;
> -
> - /* Read out the instruction */
> - if (kvmppc_read_inst(vcpu) == EMULATE_DONE)
> - /* Need to emulate */
> - return EMULATE_FAIL;
> -
> - return EMULATE_AGAIN;
> -}
> -
> /* Handle external providers (FPU, Altivec, VSX) */
> static int kvmppc_handle_ext(struct kvm_vcpu *vcpu, unsigned int exit_nr,
> ulong msr)
> @@ -977,15 +941,24 @@ int kvmppc_handle_exit_pr(struct kvm_run *run, struct kvm_vcpu *vcpu,
> {
> enum emulation_result er;
> ulong flags;
> + u32 last_inst;
> + int emul;
>
> program_interrupt:
> flags = vcpu->arch.shadow_srr1 & 0x1f0000ull;
>
> + emul = kvmppc_get_last_inst(vcpu, false, &last_inst);
> + if (emul != EMULATE_DONE) {
> + r = RESUME_GUEST;
> + break;
> + }
> +
> if (kvmppc_get_msr(vcpu) & MSR_PR) {
> #ifdef EXIT_DEBUG
> - printk(KERN_INFO "Userspace triggered 0x700 exception at 0x%lx (0x%x)\n", kvmppc_get_pc(vcpu), kvmppc_get_last_inst(vcpu));
> + pr_info("Userspace triggered 0x700 exception at\n"
I don't think we want the \n here :).
> + "0x%lx (0x%x)\n", kvmppc_get_pc(vcpu), last_inst);
> #endif
> - if ((kvmppc_get_last_inst(vcpu) & 0xff0007ff) !=
> + if ((last_inst & 0xff0007ff) !=
> (INS_DCBZ & 0xfffffff7)) {
> kvmppc_core_queue_program(vcpu, flags);
> r = RESUME_GUEST;
> @@ -1004,7 +977,7 @@ program_interrupt:
> break;
> case EMULATE_FAIL:
> printk(KERN_CRIT "%s: emulation at %lx failed (%08x)\n",
> - __func__, kvmppc_get_pc(vcpu), kvmppc_get_last_inst(vcpu));
> + __func__, kvmppc_get_pc(vcpu), last_inst);
> kvmppc_core_queue_program(vcpu, flags);
> r = RESUME_GUEST;
> break;
> @@ -1021,8 +994,25 @@ program_interrupt:
> break;
> }
> case BOOK3S_INTERRUPT_SYSCALL:
> + {
> + u32 last_sc;
> + int emul;
> +
> + /* Get last sc for papr */
> + if (vcpu->arch.papr_enabled) {
> + /*
> + * The sc instuction sets SRR0 to point to the next inst
> + */
> + emul = kvmppc_get_last_inst(vcpu, true, &last_sc);
> + if (emul != EMULATE_DONE) {
> + kvmppc_set_pc(vcpu, kvmppc_get_pc(vcpu) - 4);
> + r = RESUME_GUEST;
> + break;
> + }
> + }
> +
> if (vcpu->arch.papr_enabled &&
> - (kvmppc_get_last_sc(vcpu) == 0x44000022) &&
> + (last_sc == 0x44000022) &&
> !(kvmppc_get_msr(vcpu) & MSR_PR)) {
> /* SC 1 papr hypercalls */
> ulong cmd = kvmppc_get_gpr(vcpu, 3);
> @@ -1067,36 +1057,53 @@ program_interrupt:
> r = RESUME_GUEST;
> }
> break;
> + }
> case BOOK3S_INTERRUPT_FP_UNAVAIL:
> case BOOK3S_INTERRUPT_ALTIVEC:
> case BOOK3S_INTERRUPT_VSX:
> {
> int ext_msr = 0;
> + int emul;
> + u32 last_inst;
>
> - switch (exit_nr) {
> - case BOOK3S_INTERRUPT_FP_UNAVAIL: ext_msr = MSR_FP; break;
> - case BOOK3S_INTERRUPT_ALTIVEC: ext_msr = MSR_VEC; break;
> - case BOOK3S_INTERRUPT_VSX: ext_msr = MSR_VSX; break;
> - }
> + if (!(vcpu->arch.hflags & BOOK3S_HFLAG_PAIRED_SINGLE)) {
> + /* Do paired single emulation */
> +
> + switch (exit_nr) {
> + case BOOK3S_INTERRUPT_FP_UNAVAIL:
> + ext_msr = MSR_FP;
> + break;
> +
> + case BOOK3S_INTERRUPT_ALTIVEC:
> + ext_msr = MSR_VEC;
> + break;
> +
> + case BOOK3S_INTERRUPT_VSX:
> + ext_msr = MSR_VSX;
> + break;
> + }
>
> - switch (kvmppc_check_ext(vcpu, exit_nr)) {
> - case EMULATE_DONE:
> - /* everything ok - let's enable the ext */
> r = kvmppc_handle_ext(vcpu, exit_nr, ext_msr);
> break;
> - case EMULATE_FAIL:
> + }
> +
> + emul = kvmppc_get_last_inst(vcpu, false, &last_inst);
> + if (emul == EMULATE_DONE) {
> /* we need to emulate this instruction */
> goto program_interrupt;
> break;
> - default:
> - /* nothing to worry about - go again */
> - break;
> + } else {
> + r = RESUME_GUEST;
> }
> +
> break;
> }
> case BOOK3S_INTERRUPT_ALIGNMENT:
> - if (kvmppc_read_inst(vcpu) == EMULATE_DONE) {
Phew - this removes the ability to inject an instruction page fault on
this path. It's probably ok to do so, but please eliminate
kvmppc_read_inst() in a separate patch before this one so we can later
bisect it more easily when it fails.
Alex
> - u32 last_inst = kvmppc_get_last_inst(vcpu);
> + {
> + u32 last_inst;
> + int emul = kvmppc_get_last_inst(vcpu, false, &last_inst);
> +
> + if (emul == EMULATE_DONE) {
> u32 dsisr;
> u64 dar;
>
> @@ -1110,6 +1117,7 @@ program_interrupt:
> }
> r = RESUME_GUEST;
> break;
> + }
> #ifdef CONFIG_PPC_BOOK3S_64
> case BOOK3S_INTERRUPT_FAC_UNAVAIL:
> kvmppc_handle_fac(vcpu, vcpu->arch.shadow_fscr >> 56);
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index ab62109..34a42b9 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -752,6 +752,9 @@ static int emulation_exit(struct kvm_run *run, struct kvm_vcpu *vcpu)
> * they were actually modified by emulation. */
> return RESUME_GUEST_NV;
>
> + case EMULATE_AGAIN:
> + return RESUME_GUEST;
> +
> case EMULATE_DO_DCR:
> run->exit_reason = KVM_EXIT_DCR;
> return RESUME_HOST;
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
> index dd2cc03..f692c12 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -606,6 +606,11 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
> }
> }
>
> +int kvmppc_load_last_inst(struct kvm_vcpu *vcpu, bool prev, u32 *instr)
> +{
> + return EMULATE_FAIL;
> +}
> +
> /************* MMU Notifiers *************/
>
> int kvm_unmap_hva(struct kvm *kvm, unsigned long hva)
> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
> index da86d9b..c5c64b6 100644
> --- a/arch/powerpc/kvm/emulate.c
> +++ b/arch/powerpc/kvm/emulate.c
> @@ -224,19 +224,25 @@ static int kvmppc_emulate_mfspr(struct kvm_vcpu *vcpu, int sprn, int rt)
> * from opcode tables in the future. */
> int kvmppc_emulate_instruction(struct kvm_run *run, struct kvm_vcpu *vcpu)
> {
> - u32 inst = kvmppc_get_last_inst(vcpu);
> - int ra = get_ra(inst);
> - int rs = get_rs(inst);
> - int rt = get_rt(inst);
> - int sprn = get_sprn(inst);
> - enum emulation_result emulated = EMULATE_DONE;
> + u32 inst;
> + int ra, rs, rt, sprn;
> + enum emulation_result emulated;
> int advance = 1;
>
> /* this default type might be overwritten by subcategories */
> kvmppc_set_exit_type(vcpu, EMULATED_INST_EXITS);
>
> + emulated = kvmppc_get_last_inst(vcpu, false, &inst);
> + if (emulated != EMULATE_DONE)
> + return emulated;
> +
> pr_debug("Emulating opcode %d / %d\n", get_op(inst), get_xop(inst));
>
> + ra = get_ra(inst);
> + rs = get_rs(inst);
> + rt = get_rt(inst);
> + sprn = get_sprn(inst);
> +
> switch (get_op(inst)) {
> case OP_TRAP:
> #ifdef CONFIG_PPC_BOOK3S
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index bab20f4..1dba84b 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -261,6 +261,9 @@ int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu)
> * actually modified. */
> r = RESUME_GUEST_NV;
> break;
> + case EMULATE_AGAIN:
> + r = RESUME_GUEST;
> + break;
> case EMULATE_DO_MMIO:
> run->exit_reason = KVM_EXIT_MMIO;
> /* We must reload nonvolatiles because "update" load/store
> @@ -270,11 +273,14 @@ int kvmppc_emulate_mmio(struct kvm_run *run, struct kvm_vcpu *vcpu)
> r = RESUME_HOST_NV;
> break;
> case EMULATE_FAIL:
> + {
> + u32 last_inst;
> + kvmppc_get_last_inst(vcpu, false, &last_inst);
> /* XXX Deliver Program interrupt to guest. */
> - printk(KERN_EMERG "%s: emulation failed (%08x)\n", __func__,
> - kvmppc_get_last_inst(vcpu));
> + pr_emerg("%s: emulation failed (%08x)\n", __func__, last_inst);
> r = RESUME_HOST;
> break;
> + }
> default:
> WARN_ON(1);
> r = RESUME_GUEST;
^ permalink raw reply
* Device Tree Binding for DSA on P1023RDB
From: Pannirselvam Kanagaratnam @ 2014-06-12 15:36 UTC (permalink / raw)
To: linuxppc-dev
[-- Attachment #1: Type: text/html, Size: 43231 bytes --]
^ permalink raw reply
* Re: [PATCH 4/4 v3] KVM: PPC: Bookehv: Get vcpu's last instruction for emulation
From: Alexander Graf @ 2014-06-12 16:04 UTC (permalink / raw)
To: Mihai Caraman; +Cc: Scott Wood, linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1401724216-26486-5-git-send-email-mihai.caraman@freescale.com>
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 10000`; do /bin/echo > /dev/null; done
>
> real 0m 8.85s
> user 0m 4.34s
> sys 0m 4.48s
>
> vs
>
> real 0m 8.84s
> user 0m 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.caraman@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 for signals if we
> * aren't already exiting to userspace for some other reason.
> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
> index 6ff4480..e000b39 100644
> --- a/arch/powerpc/kvm/bookehv_interrupts.S
> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
> @@ -121,38 +121,14 @@
> 1:
>
> .if \flags & NEED_EMU
> - /*
> - * This assumes you have external PID support.
> - * To support a bookehv CPU without external PID, you'll
> - * need to look up the TLB entry and create a temporary mapping.
> - *
> - * FIXME: we don't currently handle if the lwepx faults. PR-mode
> - * booke doesn't handle it either. Since Linux doesn't use
> - * broadcast tlbivax anymore, the only way this should happen is
> - * if the guest maps its memory execute-but-not-read, or if we
> - * somehow take a TLB miss in the middle of this entry code and
> - * evict the relevant entry. On e500mc, all kernel lowmem is
> - * bolted into TLB1 large page mappings, and we don't use
> - * broadcast invalidates, so we should not take a TLB miss here.
> - *
> - * Later we'll need to deal with faults here. Disallowing guest
> - * mappings that are execute-but-not-read could be an option on
> - * e500mc, but not on chips with an LRAT if it is used.
> - */
> -
> - mfspr r3, SPRN_EPLC /* will already have correct ELPID and EGS */
> PPC_STL r15, VCPU_GPR(R15)(r4)
> PPC_STL r16, VCPU_GPR(R16)(r4)
> PPC_STL r17, VCPU_GPR(R17)(r4)
> PPC_STL r18, VCPU_GPR(R18)(r4)
> PPC_STL r19, VCPU_GPR(R19)(r4)
> - mr r8, r3
> PPC_STL r20, VCPU_GPR(R20)(r4)
> - rlwimi r8, r6, EPC_EAS_SHIFT - MSR_IR_LG, EPC_EAS
> PPC_STL r21, VCPU_GPR(R21)(r4)
> - rlwimi r8, r6, EPC_EPR_SHIFT - MSR_PR_LG, EPC_EPR
> PPC_STL r22, VCPU_GPR(R22)(r4)
> - rlwimi r8, r10, EPC_EPID_SHIFT, EPC_EPID
> PPC_STL r23, VCPU_GPR(R23)(r4)
> PPC_STL r24, VCPU_GPR(R24)(r4)
> PPC_STL r25, VCPU_GPR(R25)(r4)
> @@ -162,10 +138,15 @@
> PPC_STL r29, VCPU_GPR(R29)(r4)
> PPC_STL r30, VCPU_GPR(R30)(r4)
> PPC_STL r31, VCPU_GPR(R31)(r4)
> - mtspr SPRN_EPLC, r8
> - isync
> - lwepx r9, 0, r5
> - mtspr SPRN_EPLC, r3
> +
> + /*
> + * We don't use external PID support. lwepx faults would need to be
> + * handled by KVM and this implies aditional code in DO_KVM (for
> + * DTB_MISS, DSI and LRAT) to check ESR[EPID] and EPLC[EGS] which
> + * is too intrusive for the host. Get last instuction in
> + * kvmppc_get_last_inst().
> + */
> + li r9, KVM_INST_FETCH_FAILED
> stw r9, VCPU_LAST_INST(r4)
> .endif
>
> diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
> index f692c12..0528fe5 100644
> --- a/arch/powerpc/kvm/e500_mmu_host.c
> +++ b/arch/powerpc/kvm/e500_mmu_host.c
> @@ -606,10 +606,103 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
> }
> }
>
> +#ifdef CONFIG_KVM_BOOKE_HV
> int kvmppc_load_last_inst(struct kvm_vcpu *vcpu, bool prev, u32 *instr)
> {
> + gva_t geaddr;
> + hpa_t addr;
> + hfn_t pfn;
> + hva_t eaddr;
> + u32 mas0, mas1, mas2, mas3;
> + u64 mas7_mas3;
> + struct page *page;
> + unsigned int addr_space, psize_shift;
> + bool pr;
> + unsigned long flags;
> +
> + WARN_ON_ONCE(prev);
> +
> + /* Search TLB for guest pc to get the real address */
> + geaddr = kvmppc_get_pc(vcpu);
> +
> + addr_space = (vcpu->arch.shared->msr & MSR_IS) >> MSR_IR_LG;
> +
> + local_irq_save(flags);
> + mtspr(SPRN_MAS6, (vcpu->arch.pid << MAS6_SPID_SHIFT) | addr_space);
> + mtspr(SPRN_MAS5, MAS5_SGS | vcpu->kvm->arch.lpid);
> + asm volatile("tlbsx 0, %[geaddr]\n" : :
> + [geaddr] "r" (geaddr));
> + mtspr(SPRN_MAS5, 0);
> + mtspr(SPRN_MAS8, 0);
> + mas0 = mfspr(SPRN_MAS0);
> + mas1 = mfspr(SPRN_MAS1);
> + mas2 = mfspr(SPRN_MAS2);
> + mas3 = mfspr(SPRN_MAS3);
> + mas7_mas3 = (((u64) mfspr(SPRN_MAS7)) << 32) | mas3;
> + local_irq_restore(flags);
> +
> + /*
> + * If the TLB entry for guest pc was evicted, return to the guest.
> + * There are high chances to find a valid TLB entry next time.
> + */
> + if (!(mas1 & MAS1_VALID))
> + return EMULATE_AGAIN;
> +
> + /*
> + * Another thread may rewrite the TLB entry in parallel, don't
> + * execute from the address if the execute permission is not set
> + */
> + pr = vcpu->arch.shared->msr & MSR_PR;
> + if ((pr && !(mas3 & MAS3_UX)) || (!pr && !(mas3 & MAS3_SX))) {
> + pr_debug("Instuction emulation from a guest page\n"
> + "withot execute permission\n");
> + return EMULATE_FAIL;
> + }
> +
> + /*
> + * We will map the real address through a cacheable page, so we will
> + * not support cache-inhibited guest pages. Fortunately emulated
> + * instructions should not live there.
> + */
> + if (mas2 & MAS2_I) {
> + pr_debug("Instuction emulation from cache-inhibited\n"
> + "guest pages is not supported\n");
> + return EMULATE_FAIL;
> + }
> +
> + /* Get page size */
> + psize_shift = MAS1_GET_TSIZE(mas1) + 10;
> +
> + /* Map a page and get guest's instruction */
> + addr = (mas7_mas3 & (~0ULL << psize_shift)) |
> + (geaddr & ((1ULL << psize_shift) - 1ULL));
> + pfn = addr >> PAGE_SHIFT;
> +
> + /* Guard us against emulation from devices area */
> + if (unlikely(!page_is_ram(pfn))) {
> + pr_debug("Instruction emulation from non-RAM host\n"
> + "pages is not supported\n");
> + return EMULATE_FAIL;
> + }
> +
> + if (unlikely(!pfn_valid(pfn))) {
> + pr_debug("Invalid frame number\n");
> + return EMULATE_FAIL;
> + }
> +
> + page = pfn_to_page(pfn);
> + eaddr = (unsigned long)kmap_atomic(page);
> + *instr = *(u32 *)(eaddr | (addr & ~PAGE_MASK));
> + kunmap_atomic((u32 *)eaddr);
> +
> + return EMULATE_DONE;
> +}
> +#else
> +int kvmppc_load_last_inst(struct kvm_vcpu *vcpu, u32 *instr)
> +{
> return EMULATE_FAIL;
> }
> +#endif
>
> /************* MMU Notifiers *************/
>
^ permalink raw reply
* Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule
From: Alexander Graf @ 2014-06-12 17:04 UTC (permalink / raw)
To: Mihai Caraman; +Cc: Scott Wood, linuxppc-dev, kvm, kvm-ppc
In-Reply-To: <1402581610-16585-1-git-send-email-mihai.caraman@freescale.com>
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);
> }
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox