diff mbox

[v13,3/6] CPM/QE: use genalloc to manage CPM/QE muram

Message ID 1448851737-33125-3-git-send-email-qiang.zhao@freescale.com (mailing list archive)
State Accepted
Delegated to: Scott Wood
Headers show

Commit Message

Zhao Qiang Nov. 30, 2015, 2:48 a.m. UTC
Use genalloc to manage CPM/QE muram instead of rheap.

Signed-off-by: Zhao Qiang <qiang.zhao@freescale.com>
---
Changes for v9:
	- splitted from patch 3/5, modify cpm muram management functions.
Changes for v10:
	- modify cpm muram first, then move to qe_common
	- modify commit.
Changes for v11:
	- factor out the common alloc code
	- modify min_alloc_order to zero for cpm_muram_alloc_fixed.
Changes for v12:
	- Nil 
Changes for v13:
	- rebase

 arch/powerpc/include/asm/cpm.h   |   3 +
 arch/powerpc/platforms/Kconfig   |   4 +-
 arch/powerpc/sysdev/cpm_common.c | 126 +++++++++++++++++++++++++++------------
 lib/genalloc.c                   |   2 +-
 4 files changed, 94 insertions(+), 41 deletions(-)

Comments

Christophe Leroy Aug. 5, 2016, 4:58 p.m. UTC | #1
Le 30/11/2015 à 03:48, Zhao Qiang a écrit :
> Use genalloc to manage CPM/QE muram instead of rheap.
>
> Signed-off-by: Zhao Qiang <qiang.zhao@freescale.com>
> ---
> Changes for v9:
> 	- splitted from patch 3/5, modify cpm muram management functions.
> Changes for v10:
> 	- modify cpm muram first, then move to qe_common
> 	- modify commit.
> Changes for v11:
> 	- factor out the common alloc code
> 	- modify min_alloc_order to zero for cpm_muram_alloc_fixed.
> Changes for v12:
> 	- Nil
> Changes for v13:
> 	- rebase
>
>  arch/powerpc/include/asm/cpm.h   |   3 +
>  arch/powerpc/platforms/Kconfig   |   4 +-
>  arch/powerpc/sysdev/cpm_common.c | 126 +++++++++++++++++++++++++++------------
>  lib/genalloc.c                   |   2 +-
>  4 files changed, 94 insertions(+), 41 deletions(-)
>

With that patch applied, I get the following Oops on a 8xx (Which has a 
CPM1).

cpm_muram_init() is called from setup_arch()

It seems that gen_pool_add() tries to kmalloc() memory but the SLAB is 
not available yet.

[    0.000000] Unable to handle kernel paging request for data at 
address 0x00000008
[    0.000000] Faulting instruction address: 0xc01acce0
[    0.000000] Oops: Kernel access of bad area, sig: 11 [#1]
[    0.000000] PREEMPT CMPC885
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 
4.4.14-s3k-dev-g0886ed8-svn #5
[    0.000000] task: c05183e0 ti: c0536000 task.ti: c0536000
[    0.000000] NIP: c01acce0 LR: c0011068 CTR: 00000000
[    0.000000] REGS: c0537e50 TRAP: 0300   Not tainted 
(4.4.14-s3k-dev-g0886ed8-svn)
[    0.000000] MSR: 00001032 <ME,IR,DR,RI>  CR: 28044428  XER: 00000000
[    0.000000] DAR: 00000008 DSISR: c0000000
GPR00: c0011068 c0537f00 c05183e0 00000000 00009000 ffffffff 00000bc0 
ffffffff
GPR08: ff003000 ff00b000 ff003bbf 00000000 22044422 100d43a8 00000000 
07ff94e8
GPR16: 00000000 07bb5d70 00000000 07ff81f4 07ff81f4 07ff81f4 00000000 
00000000
GPR24: 07ffb3a0 07fe7628 c0550000 c7ffa190 c0540000 ff003bbf 00000000 
00000001
[    0.000000] NIP [c01acce0] gen_pool_add_virt+0x14/0xdc
[    0.000000] LR [c0011068] cpm_muram_init+0xd4/0x18c
[    0.000000] Call Trace:
[    0.000000] [c0537f00] [00000200] 0x200 (unreliable)
[    0.000000] [c0537f20] [c0011068] cpm_muram_init+0xd4/0x18c
[    0.000000] [c0537f70] [c0494684] cpm_reset+0xb4/0xc8
[    0.000000] [c0537f90] [c0494c64] cmpc885_setup_arch+0x10/0x30
[    0.000000] [c0537fa0] [c0493cd4] setup_arch+0x130/0x168
[    0.000000] [c0537fb0] [c04906bc] start_kernel+0x88/0x380
[    0.000000] [c0537ff0] [c0002224] start_here+0x38/0x98
[    0.000000] Instruction dump:
[    0.000000] 91430010 91430014 80010014 83e1000c 7c0803a6 38210010 
4e800020 7c0802a6
[    0.000000] 9421ffe0 bf61000c 90010024 7c7e1b78 <80630008> 7c9c2378 
7cc31c30 3863001f
[    0.000000] ---[ end trace dc8fa200cb88537f ]---
Qiang Zhao Aug. 8, 2016, 3 a.m. UTC | #2
On 6/8/2016 03:48AM,  Christophe Leroy <christophe.leroy@c-s.fr> wrote :

> -----Original Message-----

> From: Christophe Leroy [mailto:christophe.leroy@c-s.fr]

> Sent: Saturday, August 06, 2016 12:59 AM

> To: Zhao Qiang <qiang.zhao@freescale.com>; lauraa@codeaurora.org

> Cc: catalin.marinas@arm.com; linux-kernel@vger.kernel.org; Scott Wood

> <oss@buserror.net>; olof@lixom.net; akpm@linux-foundation.org; linuxppc-

> dev@lists.ozlabs.org; X.xie@freescale.com

> Subject: Re: [PATCH v13 3/6] CPM/QE: use genalloc to manage CPM/QE muram

> 

> 

> 

> Le 30/11/2015 à 03:48, Zhao Qiang a écrit :

> > Use genalloc to manage CPM/QE muram instead of rheap.

> >

> > Signed-off-by: Zhao Qiang <qiang.zhao@freescale.com>

> > ---

> > Changes for v9:

> > 	- splitted from patch 3/5, modify cpm muram management functions.

> > Changes for v10:

> > 	- modify cpm muram first, then move to qe_common

> > 	- modify commit.

> > Changes for v11:

> > 	- factor out the common alloc code

> > 	- modify min_alloc_order to zero for cpm_muram_alloc_fixed.

> > Changes for v12:

> > 	- Nil

> > Changes for v13:

> > 	- rebase

> >

> >  arch/powerpc/include/asm/cpm.h   |   3 +

> >  arch/powerpc/platforms/Kconfig   |   4 +-

> >  arch/powerpc/sysdev/cpm_common.c | 126

> +++++++++++++++++++++++++++------------

> >  lib/genalloc.c                   |   2 +-

> >  4 files changed, 94 insertions(+), 41 deletions(-)

> >

> 

> With that patch applied, I get the following Oops on a 8xx (Which has a CPM1).

> 

> cpm_muram_init() is called from setup_arch()

> 

> It seems that gen_pool_add() tries to kmalloc() memory but the SLAB is not

> available yet.

> 


Thank you for your comments, I can't find a 8xx board, would you like to test the patch 
Attached on your board? 

> [    0.000000] Unable to handle kernel paging request for data at

> address 0x00000008

> [    0.000000] Faulting instruction address: 0xc01acce0

> [    0.000000] Oops: Kernel access of bad area, sig: 11 [#1]

> [    0.000000] PREEMPT CMPC885

> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted

> 4.4.14-s3k-dev-g0886ed8-svn #5

> [    0.000000] task: c05183e0 ti: c0536000 task.ti: c0536000

> [    0.000000] NIP: c01acce0 LR: c0011068 CTR: 00000000

> [    0.000000] REGS: c0537e50 TRAP: 0300   Not tainted

> (4.4.14-s3k-dev-g0886ed8-svn)

> [    0.000000] MSR: 00001032 <ME,IR,DR,RI>  CR: 28044428  XER: 00000000

> [    0.000000] DAR: 00000008 DSISR: c0000000

> GPR00: c0011068 c0537f00 c05183e0 00000000 00009000 ffffffff 00000bc0 ffffffff

> GPR08: ff003000 ff00b000 ff003bbf 00000000 22044422 100d43a8 00000000

> 07ff94e8

> GPR16: 00000000 07bb5d70 00000000 07ff81f4 07ff81f4 07ff81f4 00000000

> 00000000

> GPR24: 07ffb3a0 07fe7628 c0550000 c7ffa190 c0540000 ff003bbf 00000000

> 00000001

> [    0.000000] NIP [c01acce0] gen_pool_add_virt+0x14/0xdc

> [    0.000000] LR [c0011068] cpm_muram_init+0xd4/0x18c

> [    0.000000] Call Trace:

> [    0.000000] [c0537f00] [00000200] 0x200 (unreliable)

> [    0.000000] [c0537f20] [c0011068] cpm_muram_init+0xd4/0x18c

> [    0.000000] [c0537f70] [c0494684] cpm_reset+0xb4/0xc8

> [    0.000000] [c0537f90] [c0494c64] cmpc885_setup_arch+0x10/0x30

> [    0.000000] [c0537fa0] [c0493cd4] setup_arch+0x130/0x168

> [    0.000000] [c0537fb0] [c04906bc] start_kernel+0x88/0x380

> [    0.000000] [c0537ff0] [c0002224] start_here+0x38/0x98

> [    0.000000] Instruction dump:

> [    0.000000] 91430010 91430014 80010014 83e1000c 7c0803a6 38210010

> 4e800020 7c0802a6

> [    0.000000] 9421ffe0 bf61000c 90010024 7c7e1b78 <80630008> 7c9c2378

> 7cc31c30 3863001f

> [    0.000000] ---[ end trace dc8fa200cb88537f ]---
Christophe Leroy Aug. 8, 2016, 5:49 p.m. UTC | #3
Le 08/08/2016 à 05:00, Qiang Zhao a écrit :
> On 6/8/2016 03:48AM,  Christophe Leroy <christophe.leroy@c-s.fr> wrote :
>
>> -----Original Message-----
>> From: Christophe Leroy [mailto:christophe.leroy@c-s.fr]
>> Sent: Saturday, August 06, 2016 12:59 AM
>> To: Zhao Qiang <qiang.zhao@freescale.com>; lauraa@codeaurora.org
>> Cc: catalin.marinas@arm.com; linux-kernel@vger.kernel.org; Scott Wood
>> <oss@buserror.net>; olof@lixom.net; akpm@linux-foundation.org; linuxppc-
>> dev@lists.ozlabs.org; X.xie@freescale.com
>> Subject: Re: [PATCH v13 3/6] CPM/QE: use genalloc to manage CPM/QE muram
>>
>>
>>
>> Le 30/11/2015 à 03:48, Zhao Qiang a écrit :
>>> Use genalloc to manage CPM/QE muram instead of rheap.
>>>
>>> Signed-off-by: Zhao Qiang <qiang.zhao@freescale.com>
>>> ---
>>> Changes for v9:
>>> 	- splitted from patch 3/5, modify cpm muram management functions.
>>> Changes for v10:
>>> 	- modify cpm muram first, then move to qe_common
>>> 	- modify commit.
>>> Changes for v11:
>>> 	- factor out the common alloc code
>>> 	- modify min_alloc_order to zero for cpm_muram_alloc_fixed.
>>> Changes for v12:
>>> 	- Nil
>>> Changes for v13:
>>> 	- rebase
>>>
>>>  arch/powerpc/include/asm/cpm.h   |   3 +
>>>  arch/powerpc/platforms/Kconfig   |   4 +-
>>>  arch/powerpc/sysdev/cpm_common.c | 126
>> +++++++++++++++++++++++++++------------
>>>  lib/genalloc.c                   |   2 +-
>>>  4 files changed, 94 insertions(+), 41 deletions(-)
>>>
>>
>> With that patch applied, I get the following Oops on a 8xx (Which has a CPM1).
>>
>> cpm_muram_init() is called from setup_arch()
>>
>> It seems that gen_pool_add() tries to kmalloc() memory but the SLAB is not
>> available yet.
>>
>
> Thank you for your comments, I can't find a 8xx board, would you like to test the patch
> Attached on your board?

Thanks for your support (indeed I only received your mail a few minutes, 
that is after I proposed another patch).

Your patch will not work, because initcalls are called too late. The 8xx 
needs the SMCs from the CPM for console, that is long before initcalls 
are called.

I sent a proposed patch approximatly 2 hours ago, it is called "[PATCH] 
soc: fsl/qe: fix Oops on CPM1 (and likely CPM2)"
That one works. Could you have a look ?

Christophe

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
diff mbox

Patch

diff --git a/arch/powerpc/include/asm/cpm.h b/arch/powerpc/include/asm/cpm.h
index 4398a6c..46e86b5 100644
--- a/arch/powerpc/include/asm/cpm.h
+++ b/arch/powerpc/include/asm/cpm.h
@@ -2,6 +2,7 @@ 
 #define __CPM_H
 
 #include <linux/compiler.h>
+#include <linux/genalloc.h>
 #include <linux/types.h>
 #include <linux/errno.h>
 #include <linux/of.h>
@@ -161,6 +162,8 @@  int cpm_muram_init(void);
 unsigned long cpm_muram_alloc(unsigned long size, unsigned long align);
 int cpm_muram_free(unsigned long offset);
 unsigned long cpm_muram_alloc_fixed(unsigned long offset, unsigned long size);
+unsigned long cpm_muram_alloc_common(unsigned long size, genpool_algo_t algo,
+				     void *data);
 void __iomem *cpm_muram_addr(unsigned long offset);
 unsigned long cpm_muram_offset(void __iomem *addr);
 dma_addr_t cpm_muram_dma(void __iomem *addr);
diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index b7f9c40..57069eb 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -275,7 +275,7 @@  config TAU_AVERAGE
 config QUICC_ENGINE
 	bool "Freescale QUICC Engine (QE) Support"
 	depends on FSL_SOC && PPC32
-	select PPC_LIB_RHEAP
+	select GENERIC_ALLOCATOR
 	select CRC32
 	help
 	  The QUICC Engine (QE) is a new generation of communications
@@ -295,7 +295,6 @@  config CPM2
 	bool "Enable support for the CPM2 (Communications Processor Module)"
 	depends on (FSL_SOC_BOOKE && PPC32) || 8260
 	select CPM
-	select PPC_LIB_RHEAP
 	select PPC_PCI_CHOICE
 	select ARCH_REQUIRE_GPIOLIB
 	help
@@ -325,6 +324,7 @@  config FSL_ULI1575
 
 config CPM
 	bool
+	select GENERIC_ALLOCATOR
 
 config OF_RTC
 	bool
diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/cpm_common.c
index e00a5ee..fcc83cd 100644
--- a/arch/powerpc/sysdev/cpm_common.c
+++ b/arch/powerpc/sysdev/cpm_common.c
@@ -17,6 +17,7 @@ 
  * published by the Free Software Foundation.
  */
 
+#include <linux/genalloc.h>
 #include <linux/init.h>
 #include <linux/of_device.h>
 #include <linux/spinlock.h>
@@ -27,7 +28,6 @@ 
 
 #include <asm/udbg.h>
 #include <asm/io.h>
-#include <asm/rheap.h>
 #include <asm/cpm.h>
 
 #include <mm/mmu_decl.h>
@@ -65,14 +65,22 @@  void __init udbg_init_cpm(void)
 }
 #endif
 
+static struct gen_pool *muram_pool;
 static spinlock_t cpm_muram_lock;
-static rh_block_t cpm_boot_muram_rh_block[16];
-static rh_info_t cpm_muram_info;
 static u8 __iomem *muram_vbase;
 static phys_addr_t muram_pbase;
 
-/* Max address size we deal with */
+struct muram_block {
+	struct list_head head;
+	unsigned long start;
+	int size;
+};
+
+static LIST_HEAD(muram_block_list);
+
+/* max address size we deal with */
 #define OF_MAX_ADDR_CELLS	4
+#define GENPOOL_OFFSET		(4096 * 8)
 
 int cpm_muram_init(void)
 {
@@ -87,50 +95,51 @@  int cpm_muram_init(void)
 		return 0;
 
 	spin_lock_init(&cpm_muram_lock);
-	/* initialize the info header */
-	rh_init(&cpm_muram_info, 1,
-	        sizeof(cpm_boot_muram_rh_block) /
-	        sizeof(cpm_boot_muram_rh_block[0]),
-	        cpm_boot_muram_rh_block);
-
 	np = of_find_compatible_node(NULL, NULL, "fsl,cpm-muram-data");
 	if (!np) {
 		/* try legacy bindings */
 		np = of_find_node_by_name(NULL, "data-only");
 		if (!np) {
-			printk(KERN_ERR "Cannot find CPM muram data node");
+			pr_err("Cannot find CPM muram data node");
 			ret = -ENODEV;
-			goto out;
+			goto out_muram;
 		}
 	}
 
+	muram_pool = gen_pool_create(0, -1);
 	muram_pbase = of_translate_address(np, zero);
 	if (muram_pbase == (phys_addr_t)OF_BAD_ADDR) {
-		printk(KERN_ERR "Cannot translate zero through CPM muram node");
+		pr_err("Cannot translate zero through CPM muram node");
 		ret = -ENODEV;
-		goto out;
+		goto out_pool;
 	}
 
 	while (of_address_to_resource(np, i++, &r) == 0) {
 		if (r.end > max)
 			max = r.end;
-
-		rh_attach_region(&cpm_muram_info, r.start - muram_pbase,
-				 resource_size(&r));
+		ret = gen_pool_add(muram_pool, r.start - muram_pbase +
+				   GENPOOL_OFFSET, resource_size(&r), -1);
+		if (ret) {
+			pr_err("QE: couldn't add muram to pool!\n");
+			goto out_pool;
+		}
 	}
 
 	muram_vbase = ioremap(muram_pbase, max - muram_pbase + 1);
 	if (!muram_vbase) {
-		printk(KERN_ERR "Cannot map CPM muram");
+		pr_err("Cannot map QE muram");
 		ret = -ENOMEM;
+		goto out_pool;
 	}
-
-out:
+	goto out_muram;
+out_pool:
+	gen_pool_destroy(muram_pool);
+out_muram:
 	of_node_put(np);
 	return ret;
 }
 
-/**
+/*
  * cpm_muram_alloc - allocate the requested size worth of multi-user ram
  * @size: number of bytes to allocate
  * @align: requested alignment, in bytes
@@ -143,14 +152,13 @@  unsigned long cpm_muram_alloc(unsigned long size, unsigned long align)
 {
 	unsigned long start;
 	unsigned long flags;
+	struct genpool_data_align muram_pool_data;
 
 	spin_lock_irqsave(&cpm_muram_lock, flags);
-	cpm_muram_info.alignment = align;
-	start = rh_alloc(&cpm_muram_info, size, "commproc");
-	if (!IS_ERR_VALUE(start))
-		memset_io(cpm_muram_addr(start), 0, size);
+	muram_pool_data.align = align;
+	start = cpm_muram_alloc_common(size, gen_pool_first_fit_align,
+				       &muram_pool_data);
 	spin_unlock_irqrestore(&cpm_muram_lock, flags);
-
 	return start;
 }
 EXPORT_SYMBOL(cpm_muram_alloc);
@@ -161,23 +169,31 @@  EXPORT_SYMBOL(cpm_muram_alloc);
  */
 int cpm_muram_free(unsigned long offset)
 {
-	int ret;
 	unsigned long flags;
+	int size;
+	struct muram_block *tmp;
 
+	size = 0;
 	spin_lock_irqsave(&cpm_muram_lock, flags);
-	ret = rh_free(&cpm_muram_info, offset);
+	list_for_each_entry(tmp, &muram_block_list, head) {
+		if (tmp->start == offset) {
+			size = tmp->size;
+			list_del(&tmp->head);
+			kfree(tmp);
+			break;
+		}
+	}
+	gen_pool_free(muram_pool, offset + GENPOOL_OFFSET, size);
 	spin_unlock_irqrestore(&cpm_muram_lock, flags);
-
-	return ret;
+	return size;
 }
 EXPORT_SYMBOL(cpm_muram_free);
 
-/**
+/*
  * cpm_muram_alloc_fixed - reserve a specific region of multi-user ram
- * @offset: the offset into the muram area to reserve
- * @size: the number of bytes to reserve
- *
- * This function returns "start" on success, -ENOMEM on failure.
+ * @offset: offset of allocation start address
+ * @size: number of bytes to allocate
+ * This function returns an offset into the muram area
  * Use cpm_dpram_addr() to get the virtual address of the area.
  * Use cpm_muram_free() to free the allocation.
  */
@@ -185,16 +201,50 @@  unsigned long cpm_muram_alloc_fixed(unsigned long offset, unsigned long size)
 {
 	unsigned long start;
 	unsigned long flags;
+	struct genpool_data_fixed muram_pool_data_fixed;
 
 	spin_lock_irqsave(&cpm_muram_lock, flags);
-	cpm_muram_info.alignment = 1;
-	start = rh_alloc_fixed(&cpm_muram_info, offset, size, "commproc");
+	muram_pool_data_fixed.offset = offset + GENPOOL_OFFSET;
+	start = cpm_muram_alloc_common(size, gen_pool_fixed_alloc,
+				       &muram_pool_data_fixed);
 	spin_unlock_irqrestore(&cpm_muram_lock, flags);
-
 	return start;
 }
 EXPORT_SYMBOL(cpm_muram_alloc_fixed);
 
+/*
+ * cpm_muram_alloc_common - cpm_muram_alloc common code
+ * @size: number of bytes to allocate
+ * @algo: algorithm for alloc.
+ * @data: data for genalloc's algorithm.
+ *
+ * This function returns an offset into the muram area.
+ */
+unsigned long cpm_muram_alloc_common(unsigned long size, genpool_algo_t algo,
+				     void *data)
+{
+	struct muram_block *entry;
+	unsigned long start;
+
+	start = gen_pool_alloc_algo(muram_pool, size, algo, data);
+	if (!start)
+		goto out2;
+	start = start - GENPOOL_OFFSET;
+	memset_io(cpm_muram_addr(start), 0, size);
+	entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		goto out1;
+	entry->start = start;
+	entry->size = size;
+	list_add(&entry->head, &muram_block_list);
+
+	return start;
+out1:
+	gen_pool_free(muram_pool, start, size);
+out2:
+	return (unsigned long)-ENOMEM;
+}
+
 /**
  * cpm_muram_addr - turn a muram offset into a virtual address
  * @offset: muram offset to convert
diff --git a/lib/genalloc.c b/lib/genalloc.c
index 5ec83cd..0a11396 100644
--- a/lib/genalloc.c
+++ b/lib/genalloc.c
@@ -576,7 +576,7 @@  unsigned long gen_pool_fixed_alloc(unsigned long *map, unsigned long size,
 	fixed_data = data;
 	order = pool->min_alloc_order;
 	offset_bit = fixed_data->offset >> order;
-	if (WARN_ON(fixed_data->offset & (1UL << order - 1)))
+	if (WARN_ON(fixed_data->offset & ((1UL << order) - 1)))
 		return size;
 
 	start_bit = bitmap_find_next_zero_area(map, size,