[PATCH 0/2] powerpc: enable the relocatable support for fsl booke 32bit kernel

[PATCH 0/2] powerpc: enable the relocatable support for fsl booke 32bit kernel

[Kevin Hao]

Currently the fsl booke 32bit kernel is using the DYNAMIC_MEMSTART relocation
method. But the RELOCATABLE method is more flexible and has less alignment
restriction. So enable this feature on this platform and use it by
default for the kdump kernel.

These patches have passed the kdump boot test on a p2020rdb board.

Kevin Hao (2):
  powerpc: enable the relocatable support for the fsl booke 32bit kernel
  powerpc/fsl_booke: enable the relocatable for the kdump kernel

 arch/powerpc/Kconfig                          |  5 +-
 arch/powerpc/include/asm/mmu-book3e.h         |  5 ++
 arch/powerpc/kernel/fsl_booke_entry_mapping.S |  8 ++-
 arch/powerpc/kernel/head_fsl_booke.S          | 92 +++++++++++++++++++++++++--
 arch/powerpc/mm/fsl_booke_mmu.c               | 30 ++++++++-
 5 files changed, 127 insertions(+), 13 deletions(-)

-- 
1.8.1.4

[PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Kevin Hao]

This is based on the codes in the head_44x.S. Since we always align to
256M before mapping the PAGE_OFFSET for a relocatable kernel, we also
change the init tlb map to 256M size.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/Kconfig                          |  2 +-
 arch/powerpc/kernel/fsl_booke_entry_mapping.S |  8 ++-
 arch/powerpc/kernel/head_fsl_booke.S          | 92 +++++++++++++++++++++++++--
 3 files changed, 94 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index c33e3ad..9eb97ac 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -866,7 +866,7 @@ config DYNAMIC_MEMSTART
 
 config RELOCATABLE
 	bool "Build a relocatable kernel"
-	depends on ADVANCED_OPTIONS && FLATMEM && 44x
+	depends on ADVANCED_OPTIONS && FLATMEM && (44x || FSL_BOOKE)
 	select NONSTATIC_KERNEL
 	help
 	  This builds a kernel image that is capable of running at the
diff --git a/arch/powerpc/kernel/fsl_booke_entry_mapping.S b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
index a92c79b..32a4b38 100644
--- a/arch/powerpc/kernel/fsl_booke_entry_mapping.S
+++ b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
@@ -165,10 +165,10 @@ skpinv:	addi	r6,r6,1				/* Increment */
 	lis	r6,0x1000		/* Set MAS0(TLBSEL) = TLB1(1), ESEL = 0 */
 	mtspr	SPRN_MAS0,r6
 	lis	r6,(MAS1_VALID|MAS1_IPROT)@h
-	ori	r6,r6,(MAS1_TSIZE(BOOK3E_PAGESZ_64M))@l
+	ori	r6,r6,(MAS1_TSIZE(BOOK3E_PAGESZ_256M))@l
 	mtspr	SPRN_MAS1,r6
-	lis	r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_64M, M_IF_SMP)@h
-	ori	r6,r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_64M, M_IF_SMP)@l
+	lis	r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_256M, M_IF_SMP)@h
+	ori	r6,r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_256M, M_IF_SMP)@l
 	mtspr	SPRN_MAS2,r6
 	mtspr	SPRN_MAS3,r8
 	tlbwe
@@ -176,6 +176,8 @@ skpinv:	addi	r6,r6,1				/* Increment */
 /* 7. Jump to KERNELBASE mapping */
 	lis	r6,(KERNELBASE & ~0xfff)@h
 	ori	r6,r6,(KERNELBASE & ~0xfff)@l
+	rlwinm	r7,r25,0,4,31
+	add	r6,r7,r6
 
 #elif defined(ENTRY_MAPPING_KEXEC_SETUP)
 /*
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index d10a7ca..c3b4c8e53 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -83,10 +83,43 @@ _ENTRY(_start);
 	andc	r31,r20,r18		/* r31 = page base */
 	or	r31,r31,r19		/* r31 = devtree phys addr */
 	mfspr	r30,SPRN_MAS7
-
-	li	r25,0			/* phys kernel start (low) */
 	li	r24,0			/* CPU number */
-	li	r23,0			/* phys kernel start (high) */
+
+#ifdef CONFIG_RELOCATABLE
+	bl	0f				/* Get our runtime address */
+0:	mflr	r3				/* Make it accessible */
+	addis	r3,r3,(_stext - 0b)@ha
+	addi	r3,r3,(_stext - 0b)@l 	/* Get our current runtime base */
+
+	/* Translate _stext address to physical, save in r23/r25 */
+	tlbsx	0,r3			/* must succeed */
+
+	mfspr	r16,SPRN_MAS1
+	mfspr	r20,SPRN_MAS3
+	rlwinm	r17,r16,25,0x1f		/* r17 = log2(page size) */
+	li	r18,1024
+	slw	r18,r18,r17		/* r18 = page size */
+	addi	r18,r18,-1
+	and	r19,r3,r18		/* r19 = page offset */
+	andc	r25,r20,r18		/* r25 = page base */
+	or	r25,r25,r19		/* r25 = _stext phys addr */
+	mfspr	r23,SPRN_MAS7
+
+	/*
+	 * We have the runtime (virutal) address of our base.
+	 * We calculate our shift of offset from a 256M page.
+	 * We could map the 256M page we belong to at PAGE_OFFSET and
+	 * get going from there.
+	 */
+	lis	r4,KERNELBASE@h
+	ori	r4,r4,KERNELBASE@l
+	rlwinm	r6,r25,0,4,31			/* r6 = PHYS_START % 256M */
+	rlwinm	r5,r4,0,4,31			/* r5 = KERNELBASE % 256M */
+	subf	r3,r5,r6			/* r3 = r6 - r5 */
+	add	r3,r4,r3			/* Required Virutal Address */
+
+	bl	relocate
+#endif
 
 /* We try to not make any assumptions about how the boot loader
  * setup or used the TLBs.  We invalidate all mappings from the
@@ -197,7 +230,58 @@ _ENTRY(__early_start)
 
 	bl	early_init
 
-#ifdef CONFIG_DYNAMIC_MEMSTART
+#ifdef CONFIG_RELOCATABLE
+	/*
+	 * Relocatable kernel support based on processing of dynamic
+	 * relocation entries.
+	 *
+	 * r25/r23 will contain RPN/ERPN for the start address of memory
+	 */
+	lis	r3,kernstart_addr@ha
+	la	r3,kernstart_addr@l(r3)
+
+#ifdef CONFIG_PHYS_64BIT
+	stw	r23,0(r3)
+	stw	r25,4(r3)
+#else
+	stw	r25,0(r3)
+#endif
+
+	/*
+	 * Compute the virt_phys_offset :
+	 * virt_phys_offset = stext.run - kernstart_addr
+	 *
+	 * stext.run = (KERNELBASE & ~0xfffffff) + (kernstart_addr & 0xfffffff)
+	 * When we relocate, we have :
+	 *
+	 *	(kernstart_addr & 0xfffffff) = (stext.run & 0xfffffff)
+	 *
+	 * hence:
+	 *  virt_phys_offset = (KERNELBASE & ~0xfffffff) -
+	 *                              (kernstart_addr & ~0xfffffff)
+	 *
+	 */
+
+	/* KERNELBASE&~0xfffffff => (r4,r5) */
+	li	r4, 0		/* higer 32bit */
+	lis	r5,KERNELBASE@h
+	rlwinm	r5,r5,0,0,3	/* Align to 256M, lower 32bit */
+
+	rlwinm	r7,r25,0,0,3
+	/*
+	 * 64bit subtraction.
+	 */
+	subfc	r5,r7,r5
+	subfe	r4,r23,r4
+
+	/* Store virt_phys_offset */
+	lis	r3,virt_phys_offset@ha
+	la	r3,virt_phys_offset@l(r3)
+
+	stw	r4,0(r3)
+	stw	r5,4(r3)
+
+#elif defined(CONFIG_DYNAMIC_MEMSTART)
 	lis	r3,kernstart_addr@ha
 	la	r3,kernstart_addr@l(r3)
 #ifdef CONFIG_PHYS_64BIT
-- 
1.8.1.4

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Scott Wood]

On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> This is based on the codes in the head_44x.S. Since we always align to
> 256M before mapping the PAGE_OFFSET for a relocatable kernel, we also
> change the init tlb map to 256M size.

Why 256M?

This tightens the alignment requirement for dynamic memstart.  And what =20
about boards with less than 256 MiB of RAM?

> @@ -176,6 +176,8 @@ skpinv:	addi	r6,r6,1				=20
> /* Increment */
>  /* 7. Jump to KERNELBASE mapping */
>  	lis	r6,(KERNELBASE & ~0xfff)@h
>  	ori	r6,r6,(KERNELBASE & ~0xfff)@l
> +	rlwinm	r7,r25,0,4,31
> +	add	r6,r7,r6

Please consider using the more readable form of rlwinm/rlwimi:

	rlwinm	r7,r25,0,0x0fffffff

>  #elif defined(ENTRY_MAPPING_KEXEC_SETUP)
>  /*
> diff --git a/arch/powerpc/kernel/head_fsl_booke.S =20
> b/arch/powerpc/kernel/head_fsl_booke.S
> index d10a7ca..c3b4c8e53 100644
> --- a/arch/powerpc/kernel/head_fsl_booke.S
> +++ b/arch/powerpc/kernel/head_fsl_booke.S
> @@ -83,10 +83,43 @@ _ENTRY(_start);
>  	andc	r31,r20,r18		/* r31 =3D page base */
>  	or	r31,r31,r19		/* r31 =3D devtree phys addr */
>  	mfspr	r30,SPRN_MAS7
> -
> -	li	r25,0			/* phys kernel start (low) */
>  	li	r24,0			/* CPU number */
> -	li	r23,0			/* phys kernel start (high) */
> +
> +#ifdef CONFIG_RELOCATABLE
> +	bl	0f				/* Get our runtime =20
> address */
> +0:	mflr	r3				/* Make it accessible */
> +	addis	r3,r3,(_stext - 0b)@ha
> +	addi	r3,r3,(_stext - 0b)@l 	/* Get our current runtime base =20
> */
> +
> +	/* Translate _stext address to physical, save in r23/r25 */
> +	tlbsx	0,r3			/* must succeed */
> +
> +	mfspr	r16,SPRN_MAS1
> +	mfspr	r20,SPRN_MAS3
> +	rlwinm	r17,r16,25,0x1f		/* r17 =3D log2(page size) */
> +	li	r18,1024
> +	slw	r18,r18,r17		/* r18 =3D page size */
> +	addi	r18,r18,-1
> +	and	r19,r3,r18		/* r19 =3D page offset */
> +	andc	r25,r20,r18		/* r25 =3D page base */
> +	or	r25,r25,r19		/* r25 =3D _stext phys addr */
> +	mfspr	r23,SPRN_MAS7

This duplicates the code for finding the device tree physical =20
address... maybe factor it out into a function?

> @@ -197,7 +230,58 @@ _ENTRY(__early_start)
>=20
>  	bl	early_init
>=20
> -#ifdef CONFIG_DYNAMIC_MEMSTART
> +#ifdef CONFIG_RELOCATABLE
> +	/*
> +	 * Relocatable kernel support based on processing of dynamic
> +	 * relocation entries.
> +	 *
> +	 * r25/r23 will contain RPN/ERPN for the start address of memory

The start of memory or the start of the kernel?

> +	 */
> +	lis	r3,kernstart_addr@ha
> +	la	r3,kernstart_addr@l(r3)
> +
> +#ifdef CONFIG_PHYS_64BIT
> +	stw	r23,0(r3)
> +	stw	r25,4(r3)
> +#else
> +	stw	r25,0(r3)
> +#endif

This part looks the same for relocatable and dynamic memstart -- can =20
you avoid duplicating?

> +	/*
> +	 * Compute the virt_phys_offset :
> +	 * virt_phys_offset =3D stext.run - kernstart_addr
> +	 *
> +	 * stext.run =3D (KERNELBASE & ~0xfffffff) + (kernstart_addr & =20
> 0xfffffff)
> +	 * When we relocate, we have :
> +	 *
> +	 *	(kernstart_addr & 0xfffffff) =3D (stext.run & 0xfffffff)
> +	 *
> +	 * hence:
> +	 *  virt_phys_offset =3D (KERNELBASE & ~0xfffffff) -
> +	 *                              (kernstart_addr & ~0xfffffff)
> +	 *
> +	 */
> +
> +	/* KERNELBASE&~0xfffffff =3D> (r4,r5) */

> +	li	r4, 0		/* higer 32bit */
> +	lis	r5,KERNELBASE@h

Please be consistent with whitespace.

-Scott=

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 4210 bytes --]

On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> >This is based on the codes in the head_44x.S. Since we always align to
> >256M before mapping the PAGE_OFFSET for a relocatable kernel, we also
> >change the init tlb map to 256M size.
> 
> Why 256M?

For two reasons:
  1. This is the size which both e500v1 and e500v2 support.
  2. Since we always use the PAGE_OFFSET as 0xc0000000, the 256M is
     max alignment value we can use for this virtual address.

> 
> This tightens the alignment requirement for dynamic memstart.

Yes. But since RELOCATABLE is a superset of DYNAMIC_MEMSTART, we can always
use RELOCATABLE instead of DYNAMIC_MEMSTART for fsl booke board in any cases.
So DYNAMIC_MEMSTART will seem not so useful after we enable this feature.

>  And
> what about boards with less than 256 MiB of RAM?

It should be fine. We just create the map in the tlb. The MM still use
the real size of memory.

> 
> >@@ -176,6 +176,8 @@ skpinv:	addi	r6,r6,1				/* Increment */
> > /* 7. Jump to KERNELBASE mapping */
> > 	lis	r6,(KERNELBASE & ~0xfff)@h
> > 	ori	r6,r6,(KERNELBASE & ~0xfff)@l
> >+	rlwinm	r7,r25,0,4,31
> >+	add	r6,r7,r6
> 
> Please consider using the more readable form of rlwinm/rlwimi:
> 
> 	rlwinm	r7,r25,0,0x0fffffff

Sure.

> 
> > #elif defined(ENTRY_MAPPING_KEXEC_SETUP)
> > /*
> >diff --git a/arch/powerpc/kernel/head_fsl_booke.S
> >b/arch/powerpc/kernel/head_fsl_booke.S
> >index d10a7ca..c3b4c8e53 100644
> >--- a/arch/powerpc/kernel/head_fsl_booke.S
> >+++ b/arch/powerpc/kernel/head_fsl_booke.S
> >@@ -83,10 +83,43 @@ _ENTRY(_start);
> > 	andc	r31,r20,r18		/* r31 = page base */
> > 	or	r31,r31,r19		/* r31 = devtree phys addr */
> > 	mfspr	r30,SPRN_MAS7
> >-
> >-	li	r25,0			/* phys kernel start (low) */
> > 	li	r24,0			/* CPU number */
> >-	li	r23,0			/* phys kernel start (high) */
> >+
> >+#ifdef CONFIG_RELOCATABLE
> >+	bl	0f				/* Get our runtime address */
> >+0:	mflr	r3				/* Make it accessible */
> >+	addis	r3,r3,(_stext - 0b)@ha
> >+	addi	r3,r3,(_stext - 0b)@l 	/* Get our current runtime base */
> >+
> >+	/* Translate _stext address to physical, save in r23/r25 */
> >+	tlbsx	0,r3			/* must succeed */
> >+
> >+	mfspr	r16,SPRN_MAS1
> >+	mfspr	r20,SPRN_MAS3
> >+	rlwinm	r17,r16,25,0x1f		/* r17 = log2(page size) */
> >+	li	r18,1024
> >+	slw	r18,r18,r17		/* r18 = page size */
> >+	addi	r18,r18,-1
> >+	and	r19,r3,r18		/* r19 = page offset */
> >+	andc	r25,r20,r18		/* r25 = page base */
> >+	or	r25,r25,r19		/* r25 = _stext phys addr */
> >+	mfspr	r23,SPRN_MAS7
> 
> This duplicates the code for finding the device tree physical
> address... maybe factor it out into a function?

Sure.

> 
> >@@ -197,7 +230,58 @@ _ENTRY(__early_start)
> >
> > 	bl	early_init
> >
> >-#ifdef CONFIG_DYNAMIC_MEMSTART
> >+#ifdef CONFIG_RELOCATABLE
> >+	/*
> >+	 * Relocatable kernel support based on processing of dynamic
> >+	 * relocation entries.
> >+	 *
> >+	 * r25/r23 will contain RPN/ERPN for the start address of memory
> 
> The start of memory or the start of the kernel?

Should be the start of the kernel. Will fix the comment.

> 
> >+	 */
> >+	lis	r3,kernstart_addr@ha
> >+	la	r3,kernstart_addr@l(r3)
> >+
> >+#ifdef CONFIG_PHYS_64BIT
> >+	stw	r23,0(r3)
> >+	stw	r25,4(r3)
> >+#else
> >+	stw	r25,0(r3)
> >+#endif
> 
> This part looks the same for relocatable and dynamic memstart -- can
> you avoid duplicating?

OK.

> 
> >+	/*
> >+	 * Compute the virt_phys_offset :
> >+	 * virt_phys_offset = stext.run - kernstart_addr
> >+	 *
> >+	 * stext.run = (KERNELBASE & ~0xfffffff) + (kernstart_addr &
> >0xfffffff)
> >+	 * When we relocate, we have :
> >+	 *
> >+	 *	(kernstart_addr & 0xfffffff) = (stext.run & 0xfffffff)
> >+	 *
> >+	 * hence:
> >+	 *  virt_phys_offset = (KERNELBASE & ~0xfffffff) -
> >+	 *                              (kernstart_addr & ~0xfffffff)
> >+	 *
> >+	 */
> >+
> >+	/* KERNELBASE&~0xfffffff => (r4,r5) */
> 
> >+	li	r4, 0		/* higer 32bit */
> >+	lis	r5,KERNELBASE@h
> 
> Please be consistent with whitespace.

Fixed.

Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Scott Wood]

On 06/27/2013 08:36:37 PM, Kevin Hao wrote:
> On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> > On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> > >This is based on the codes in the head_44x.S. Since we always =20
> align to
> > >256M before mapping the PAGE_OFFSET for a relocatable kernel, we =20
> also
> > >change the init tlb map to 256M size.
> >
> > Why 256M?
>=20
> For two reasons:
>   1. This is the size which both e500v1 and e500v2 support.
>   2. Since we always use the PAGE_OFFSET as 0xc0000000, the 256M is
>      max alignment value we can use for this virtual address.

Is there any reason why 64M won't continue to work here?

> > This tightens the alignment requirement for dynamic memstart.
>=20
> Yes. But since RELOCATABLE is a superset of DYNAMIC_MEMSTART, we can =20
> always
> use RELOCATABLE instead of DYNAMIC_MEMSTART for fsl booke board in =20
> any cases.

The extra flexibility of RELOCATABLE may help some use cases, but you'd =20
still require the entire 256M naturally aligned region containing the =20
kernel to be present and owned by this instance of Linux.

> So DYNAMIC_MEMSTART will seem not so useful after we enable this =20
> feature.

Then why doesn't this patch remove it?

> >  And
> > what about boards with less than 256 MiB of RAM?
>=20
> It should be fine. We just create the map in the tlb. The MM still use
> the real size of memory.

No, you must not map anything that is not present with a mapping that =20
is executable and/or not guarded, or you could get speculative accesses =20
to who-knows-what.  Even if RAM is present there but owned by some =20
other entity, you could be creating illegal aliases if that other =20
entity mapped it cache-inhibited or similar.

-Scott=

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 2928 bytes --]

On Thu, Jun 27, 2013 at 08:47:27PM -0500, Scott Wood wrote:
> On 06/27/2013 08:36:37 PM, Kevin Hao wrote:
> >On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> >> On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> >> >This is based on the codes in the head_44x.S. Since we always
> >align to
> >> >256M before mapping the PAGE_OFFSET for a relocatable kernel,
> >we also
> >> >change the init tlb map to 256M size.
> >>
> >> Why 256M?
> >
> >For two reasons:
> >  1. This is the size which both e500v1 and e500v2 support.
> >  2. Since we always use the PAGE_OFFSET as 0xc0000000, the 256M is
> >     max alignment value we can use for this virtual address.
> 
> Is there any reason why 64M won't continue to work here?

Yes. In general we would map the 0 ~ 256M memory region in the first
tlb1 entry. If we align to 64M, the relocatable kernel would not work
if loaded above 64M memory. For example, if we load a relocatable kernel
at 64M memory, we will relocate it as:
	__pa(PAGE_OFFSET) = 0x4000000

But in map_mem_in_cams function, it will create a memory map as:
	__pa(PAGE_OFFSET) = 0x0

The kernel will definitely not work in this case.
	
> 
> >> This tightens the alignment requirement for dynamic memstart.
> >
> >Yes. But since RELOCATABLE is a superset of DYNAMIC_MEMSTART, we
> >can always
> >use RELOCATABLE instead of DYNAMIC_MEMSTART for fsl booke board in
> >any cases.
> 
> The extra flexibility of RELOCATABLE may help some use cases, but
> you'd still require the entire 256M naturally aligned region
> containing the kernel to be present and owned by this instance of
> Linux.
> 
> >So DYNAMIC_MEMSTART will seem not so useful after we enable this
> >feature.
> 
> Then why doesn't this patch remove it?

According to the Kconfig it is still used by 44x. And maybe someone
still want to use this relocation method.

> 
> >>  And
> >> what about boards with less than 256 MiB of RAM?
> >
> >It should be fine. We just create the map in the tlb. The MM still use
> >the real size of memory.
> 
> No, you must not map anything that is not present with a mapping
> that is executable and/or not guarded, or you could get speculative
> accesses to who-knows-what.

Yes, there may be speculative access in this case.

>  Even if RAM is present there but owned
> by some other entity, you could be creating illegal aliases if that
> other entity mapped it cache-inhibited or similar.

Fair enough. So it seems error prone if we map this 256M memory region
blindly. But if we don't do this, it seems that we have to do twice relocation.
The first time we just align to a predefined value (64M for example), and
then parse the device tree and get the real memstart_addr. After that we
should relocate the kernel to the real start address. It seems a little
complicated. Do you have any better ideas?

Thanks,
Kevin


> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Scott Wood]

On 06/30/2013 02:33:10 AM, Kevin Hao wrote:
> On Thu, Jun 27, 2013 at 08:47:27PM -0500, Scott Wood wrote:
> > On 06/27/2013 08:36:37 PM, Kevin Hao wrote:
> > >On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> > >> On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> > >> >This is based on the codes in the head_44x.S. Since we always
> > >align to
> > >> >256M before mapping the PAGE_OFFSET for a relocatable kernel,
> > >we also
> > >> >change the init tlb map to 256M size.
> > >>
> > >> Why 256M?
> > >
> > >For two reasons:
> > >  1. This is the size which both e500v1 and e500v2 support.
> > >  2. Since we always use the PAGE_OFFSET as 0xc0000000, the 256M is
> > >     max alignment value we can use for this virtual address.
> >
> > Is there any reason why 64M won't continue to work here?
>=20
> Yes. In general we would map the 0 ~ 256M memory region in the first
> tlb1 entry. If we align to 64M, the relocatable kernel would not work
> if loaded above 64M memory. For example, if we load a relocatable =20
> kernel
> at 64M memory, we will relocate it as:
> 	__pa(PAGE_OFFSET) =3D 0x4000000
>=20
> But in map_mem_in_cams function, it will create a memory map as:
> 	__pa(PAGE_OFFSET) =3D 0x0
>=20
> The kernel will definitely not work in this case.

That's a problem with map_mem_in_cams(), as discussed in the thread on =20
other patch.  Perhaps fully solving those problems is not worthwhile at =20
this time, but we should at least be able to determine the TLB size =20
automatically based on the alignment of the address you're trying to =20
map.  64M would be used unless (address & (256M - 1)) >=3D 64M.  I hope =20
we can continue to assume the kernel won't cross a 64M boundary.

> > >> This tightens the alignment requirement for dynamic memstart.
> > >
> > >Yes. But since RELOCATABLE is a superset of DYNAMIC_MEMSTART, we
> > >can always
> > >use RELOCATABLE instead of DYNAMIC_MEMSTART for fsl booke board in
> > >any cases.
> >
> > The extra flexibility of RELOCATABLE may help some use cases, but
> > you'd still require the entire 256M naturally aligned region
> > containing the kernel to be present and owned by this instance of
> > Linux.
> >
> > >So DYNAMIC_MEMSTART will seem not so useful after we enable this
> > >feature.
> >
> > Then why doesn't this patch remove it?
>=20
> According to the Kconfig it is still used by 44x.

RELOCATABLE appears to be supported on 44x, and is what CRASH_DUMP uses =20
on 44x.

> And maybe someone still want to use this relocation method.

Then you don't get to dismiss claims that you're changing =20
DYNAMIC_MEMSTART alignment requirements by saying that RELOCATABLE is a =20
strict superset. :-)  Given the requirement that the kernel be in the =20
first TLB entry, though, using RELOCATABLE rather than DYNAMIC_MEMSTART =20
doesn't fix the alignment problem.

I don't think it makes sense to keep both mechanisms around unless =20
there's some obvious reason to prefer DYNAMIC_MEMSTART.

> > >>  And
> > >> what about boards with less than 256 MiB of RAM?
> > >
> > >It should be fine. We just create the map in the tlb. The MM still =20
> use
> > >the real size of memory.
> >
> > No, you must not map anything that is not present with a mapping
> > that is executable and/or not guarded, or you could get speculative
> > accesses to who-knows-what.
>=20
> Yes, there may be speculative access in this case.
>=20
> >  Even if RAM is present there but owned
> > by some other entity, you could be creating illegal aliases if that
> > other entity mapped it cache-inhibited or similar.
>=20
> Fair enough. So it seems error prone if we map this 256M memory region
> blindly. But if we don't do this, it seems that we have to do twice =20
> relocation.
> The first time we just align to a predefined value (64M for example), =20
> and
> then parse the device tree and get the real memstart_addr. After that =20
> we
> should relocate the kernel to the real start address. It seems a =20
> little
> complicated. Do you have any better ideas?

This seems like the proper way to address it, assuming we're unwilling =20
to map the kernel image somewhere other than the normal lowmem mapping =20
(and I think we're unwilling, given how tight the address space is on =20
32-bit, and the intrusiveness of the change).  The dynamic =20
determination of 64M versus 256M could be an acceptable alternative =20
though, if we're OK with not supporting arbitrary relocatable =20
scenarios, but just those that are either needed by kdump, or supported =20
by current kernels (with DYNAMIC_MEMSTART, or just starting at zero =20
with less than 256M of RAM).  If we go that route, the limitations =20
should be documented.

-Scott=

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 5845 bytes --]

On Mon, Jul 01, 2013 at 07:30:45PM -0500, Scott Wood wrote:
> On 06/30/2013 02:33:10 AM, Kevin Hao wrote:
> >On Thu, Jun 27, 2013 at 08:47:27PM -0500, Scott Wood wrote:
> >> On 06/27/2013 08:36:37 PM, Kevin Hao wrote:
> >> >On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> >> >> On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> >> >> >This is based on the codes in the head_44x.S. Since we always
> >> >align to
> >> >> >256M before mapping the PAGE_OFFSET for a relocatable kernel,
> >> >we also
> >> >> >change the init tlb map to 256M size.
> >> >>
> >> >> Why 256M?
> >> >
> >> >For two reasons:
> >> >  1. This is the size which both e500v1 and e500v2 support.
> >> >  2. Since we always use the PAGE_OFFSET as 0xc0000000, the 256M is
> >> >     max alignment value we can use for this virtual address.
> >>
> >> Is there any reason why 64M won't continue to work here?
> >
> >Yes. In general we would map the 0 ~ 256M memory region in the first
> >tlb1 entry. If we align to 64M, the relocatable kernel would not work
> >if loaded above 64M memory. For example, if we load a relocatable
> >kernel
> >at 64M memory, we will relocate it as:
> >	__pa(PAGE_OFFSET) = 0x4000000
> >
> >But in map_mem_in_cams function, it will create a memory map as:
> >	__pa(PAGE_OFFSET) = 0x0
> >
> >The kernel will definitely not work in this case.
> 
> That's a problem with map_mem_in_cams(), as discussed in the thread
> on other patch.  Perhaps fully solving those problems is not
> worthwhile at this time, but we should at least be able to determine
> the TLB size automatically based on the alignment of the address
> you're trying to map.  64M would be used unless (address & (256M -
> 1)) >= 64M.  I hope we can continue to assume the kernel won't cross
> a 64M boundary.

No. The problem is we don't know the physical address of the start of
lowmem at booting. So we have to align to physical address (phys1) blindly
and map the PAGE_OFFSET from there. Then once we get the physical address
(phys2) of the start of lowmem from the device tree later, we will map the
PAGE_OFFSET to the start of lowmem. If the phys1 is not equal to phys2,
we get a problem. The reasons that we can't still map the PAGE_OFFSET to
phys1 in the map_mem_in_cams() are:
  * if phys1 > phys2, then this will waste the memory between phys2 ~ phys1.
  * if phys1 < phys2, then we map a memory region which don't belong to
    this kernel.
 
> 
> >> >> This tightens the alignment requirement for dynamic memstart.
> >> >
> >> >Yes. But since RELOCATABLE is a superset of DYNAMIC_MEMSTART, we
> >> >can always
> >> >use RELOCATABLE instead of DYNAMIC_MEMSTART for fsl booke board in
> >> >any cases.
> >>
> >> The extra flexibility of RELOCATABLE may help some use cases, but
> >> you'd still require the entire 256M naturally aligned region
> >> containing the kernel to be present and owned by this instance of
> >> Linux.
> >>
> >> >So DYNAMIC_MEMSTART will seem not so useful after we enable this
> >> >feature.
> >>
> >> Then why doesn't this patch remove it?
> >
> >According to the Kconfig it is still used by 44x.
> 
> RELOCATABLE appears to be supported on 44x, and is what CRASH_DUMP
> uses on 44x.

The kdump kernel on 44x also use the RELOCATABLE method.

> 
> >And maybe someone still want to use this relocation method.
> 
> Then you don't get to dismiss claims that you're changing
> DYNAMIC_MEMSTART alignment requirements by saying that RELOCATABLE
> is a strict superset. :-)  Given the requirement that the kernel be
> in the first TLB entry, though, using RELOCATABLE rather than
> DYNAMIC_MEMSTART doesn't fix the alignment problem.
> 
> I don't think it makes sense to keep both mechanisms around unless
> there's some obvious reason to prefer DYNAMIC_MEMSTART.

The DYNAMIC_MEMSTART still can be used for such as AMP kernel. It does have
a more small footprint than RELOCATABLE and also doesn't have the overhead
of the relocation. So I don't want to drop it in a rush.

Thanks,
Kevin

> 
> >> >>  And
> >> >> what about boards with less than 256 MiB of RAM?
> >> >
> >> >It should be fine. We just create the map in the tlb. The MM
> >still use
> >> >the real size of memory.
> >>
> >> No, you must not map anything that is not present with a mapping
> >> that is executable and/or not guarded, or you could get speculative
> >> accesses to who-knows-what.
> >
> >Yes, there may be speculative access in this case.
> >
> >>  Even if RAM is present there but owned
> >> by some other entity, you could be creating illegal aliases if that
> >> other entity mapped it cache-inhibited or similar.
> >
> >Fair enough. So it seems error prone if we map this 256M memory region
> >blindly. But if we don't do this, it seems that we have to do
> >twice relocation.
> >The first time we just align to a predefined value (64M for
> >example), and
> >then parse the device tree and get the real memstart_addr. After
> >that we
> >should relocate the kernel to the real start address. It seems a
> >little
> >complicated. Do you have any better ideas?
> 
> This seems like the proper way to address it, assuming we're
> unwilling to map the kernel image somewhere other than the normal
> lowmem mapping (and I think we're unwilling, given how tight the
> address space is on 32-bit, and the intrusiveness of the change).
> The dynamic determination of 64M versus 256M could be an acceptable
> alternative though, if we're OK with not supporting arbitrary
> relocatable scenarios, but just those that are either needed by
> kdump, or supported by current kernels (with DYNAMIC_MEMSTART, or
> just starting at zero with less than 256M of RAM).  If we go that
> route, the limitations should be documented.
> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Scott Wood]

On 07/01/2013 10:24:47 PM, Kevin Hao wrote:
> On Mon, Jul 01, 2013 at 07:30:45PM -0500, Scott Wood wrote:
> > On 06/30/2013 02:33:10 AM, Kevin Hao wrote:
> > >On Thu, Jun 27, 2013 at 08:47:27PM -0500, Scott Wood wrote:
> > >> On 06/27/2013 08:36:37 PM, Kevin Hao wrote:
> > >> >On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> > >> >> On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> > >> >> >This is based on the codes in the head_44x.S. Since we always
> > >> >align to
> > >> >> >256M before mapping the PAGE_OFFSET for a relocatable kernel,
> > >> >we also
> > >> >> >change the init tlb map to 256M size.
> > >> >>
> > >> >> Why 256M?
> > >> >
> > >> >For two reasons:
> > >> >  1. This is the size which both e500v1 and e500v2 support.
> > >> >  2. Since we always use the PAGE_OFFSET as 0xc0000000, the =20
> 256M is
> > >> >     max alignment value we can use for this virtual address.
> > >>
> > >> Is there any reason why 64M won't continue to work here?
> > >
> > >Yes. In general we would map the 0 ~ 256M memory region in the =20
> first
> > >tlb1 entry. If we align to 64M, the relocatable kernel would not =20
> work
> > >if loaded above 64M memory. For example, if we load a relocatable
> > >kernel
> > >at 64M memory, we will relocate it as:
> > >	__pa(PAGE_OFFSET) =3D 0x4000000
> > >
> > >But in map_mem_in_cams function, it will create a memory map as:
> > >	__pa(PAGE_OFFSET) =3D 0x0
> > >
> > >The kernel will definitely not work in this case.
> >
> > That's a problem with map_mem_in_cams(), as discussed in the thread
> > on other patch.  Perhaps fully solving those problems is not
> > worthwhile at this time, but we should at least be able to determine
> > the TLB size automatically based on the alignment of the address
> > you're trying to map.  64M would be used unless (address & (256M -
> > 1)) >=3D 64M.  I hope we can continue to assume the kernel won't cross
> > a 64M boundary.
>=20
> No. The problem is we don't know the physical address of the start of
> lowmem at booting. So we have to align to physical address (phys1) =20
> blindly
> and map the PAGE_OFFSET from there. Then once we get the physical =20
> address
> (phys2) of the start of lowmem from the device tree later, we will =20
> map the
> PAGE_OFFSET to the start of lowmem. If the phys1 is not equal to =20
> phys2,
> we get a problem.

How would you get phys1 !=3D phys2, unless the kernel begins in a =20
256M-aligned region other than the first (which you said is already not =20
supported)?

If (phys1 & (256M - 1)) < 64M, then you'd get the same phys2 regardless =20
of whether you align it to 64M or 256M.
Otherwise, we use a 256M page which is what you're already doing.

> > >And maybe someone still want to use this relocation method.
> >
> > Then you don't get to dismiss claims that you're changing
> > DYNAMIC_MEMSTART alignment requirements by saying that RELOCATABLE
> > is a strict superset. :-)  Given the requirement that the kernel be
> > in the first TLB entry, though, using RELOCATABLE rather than
> > DYNAMIC_MEMSTART doesn't fix the alignment problem.
> >
> > I don't think it makes sense to keep both mechanisms around unless
> > there's some obvious reason to prefer DYNAMIC_MEMSTART.
>=20
> The DYNAMIC_MEMSTART still can be used for such as AMP kernel. It =20
> does have
> a more small footprint than RELOCATABLE and also doesn't have the =20
> overhead
> of the relocation. So I don't want to drop it in a rush.

How much overhead (space and time) is this really?

It will keep the code (and especially the diff) simpler to have this =20
replace DYNAMIC_MEMSTART rather than add to it.

-Scott=

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 4562 bytes --]

On Tue, Jul 02, 2013 at 05:39:18PM -0500, Scott Wood wrote:
> On 07/01/2013 10:24:47 PM, Kevin Hao wrote:
> >On Mon, Jul 01, 2013 at 07:30:45PM -0500, Scott Wood wrote:
> >> On 06/30/2013 02:33:10 AM, Kevin Hao wrote:
> >> >On Thu, Jun 27, 2013 at 08:47:27PM -0500, Scott Wood wrote:
> >> >> On 06/27/2013 08:36:37 PM, Kevin Hao wrote:
> >> >> >On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> >> >> >> On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> >> >> >> >This is based on the codes in the head_44x.S. Since we always
> >> >> >align to
> >> >> >> >256M before mapping the PAGE_OFFSET for a relocatable kernel,
> >> >> >we also
> >> >> >> >change the init tlb map to 256M size.
> >> >> >>
> >> >> >> Why 256M?
> >> >> >
> >> >> >For two reasons:
> >> >> >  1. This is the size which both e500v1 and e500v2 support.
> >> >> >  2. Since we always use the PAGE_OFFSET as 0xc0000000, the
> >256M is
> >> >> >     max alignment value we can use for this virtual address.
> >> >>
> >> >> Is there any reason why 64M won't continue to work here?
> >> >
> >> >Yes. In general we would map the 0 ~ 256M memory region in the
> >first
> >> >tlb1 entry. If we align to 64M, the relocatable kernel would
> >not work
> >> >if loaded above 64M memory. For example, if we load a relocatable
> >> >kernel
> >> >at 64M memory, we will relocate it as:
> >> >	__pa(PAGE_OFFSET) = 0x4000000
> >> >
> >> >But in map_mem_in_cams function, it will create a memory map as:
> >> >	__pa(PAGE_OFFSET) = 0x0
> >> >
> >> >The kernel will definitely not work in this case.
> >>
> >> That's a problem with map_mem_in_cams(), as discussed in the thread
> >> on other patch.  Perhaps fully solving those problems is not
> >> worthwhile at this time, but we should at least be able to determine
> >> the TLB size automatically based on the alignment of the address
> >> you're trying to map.  64M would be used unless (address & (256M -
> >> 1)) >= 64M.  I hope we can continue to assume the kernel won't cross
> >> a 64M boundary.
> >
> >No. The problem is we don't know the physical address of the start of
> >lowmem at booting. So we have to align to physical address (phys1)
> >blindly
> >and map the PAGE_OFFSET from there. Then once we get the physical
> >address
> >(phys2) of the start of lowmem from the device tree later, we will
> >map the
> >PAGE_OFFSET to the start of lowmem. If the phys1 is not equal to
> >phys2,
> >we get a problem.
> 
> How would you get phys1 != phys2, unless the kernel begins in a
> 256M-aligned region other than the first (which you said is already
> not supported)?

Yes, this is the only case which phys1 != phys2 if we align to 256M.
I plan to also fix this in the next version.

> 
> If (phys1 & (256M - 1)) < 64M, then you'd get the same phys2
> regardless of whether you align it to 64M or 256M.
> Otherwise, we use a 256M page which is what you're already doing.

Yes, you are right. I am just trying to say we will run into problem
when loading a kernel between 64M ~ 256M if we don't align to 256M.

> 
> >> >And maybe someone still want to use this relocation method.
> >>
> >> Then you don't get to dismiss claims that you're changing
> >> DYNAMIC_MEMSTART alignment requirements by saying that RELOCATABLE
> >> is a strict superset. :-)  Given the requirement that the kernel be
> >> in the first TLB entry, though, using RELOCATABLE rather than
> >> DYNAMIC_MEMSTART doesn't fix the alignment problem.
> >>
> >> I don't think it makes sense to keep both mechanisms around unless
> >> there's some obvious reason to prefer DYNAMIC_MEMSTART.
> >
> >The DYNAMIC_MEMSTART still can be used for such as AMP kernel. It
> >does have
> >a more small footprint than RELOCATABLE and also doesn't have the
> >overhead
> >of the relocation. So I don't want to drop it in a rush.
> 
> How much overhead (space and time) is this really?

The following is the additional sections when relocatable is enabled for
a p2020rdb board.
   section        size
  .dynsym       000007f0
  .dynstr       00000926
  .dynamic      00000080
  .hash         00000388
  .interp       00000011
  .rela.dyn     00215250

The time for the relocation is about 32ms on a p2020rdb board.

> 
> It will keep the code (and especially the diff) simpler to have this
> replace DYNAMIC_MEMSTART rather than add to it.

OK. If you think that the above overhead is acceptable, I can drop the
DYNAMIC_MEMSTART in the next version.

Thanks,
Kevin
> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Scott Wood]

On 07/02/2013 10:00:44 PM, Kevin Hao wrote:
> On Tue, Jul 02, 2013 at 05:39:18PM -0500, Scott Wood wrote:
> > How much overhead (space and time) is this really?
>=20
> The following is the additional sections when relocatable is enabled =20
> for
> a p2020rdb board.
>    section        size
>   .dynsym       000007f0
>   .dynstr       00000926
>   .dynamic      00000080
>   .hash         00000388
>   .interp       00000011
>   .rela.dyn     00215250
>=20
> The time for the relocation is about 32ms on a p2020rdb board.

Hmm... more relocations than I expected.  What percentage is this of =20
the total image size?

-Scott=

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 966 bytes --]

On Wed, Jul 03, 2013 at 03:38:27PM -0500, Scott Wood wrote:
> On 07/02/2013 10:00:44 PM, Kevin Hao wrote:
> >On Tue, Jul 02, 2013 at 05:39:18PM -0500, Scott Wood wrote:
> >> How much overhead (space and time) is this really?
> >
> >The following is the additional sections when relocatable is
> >enabled for
> >a p2020rdb board.
> >   section        size
> >  .dynsym       000007f0
> >  .dynstr       00000926
> >  .dynamic      00000080
> >  .hash         00000388
> >  .interp       00000011
> >  .rela.dyn     00215250
> >
> >The time for the relocation is about 32ms on a p2020rdb board.
> 
> Hmm... more relocations than I expected.  What percentage is this of
> the total image size?

The size of vmlinux.bin is about 10M. The percentage of the relocation
section is about 20%. But look on the bright side of thing, all the relocation
stuff are in init section and should be discarded at runtime. :-)

Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Scott Wood]

On 07/03/2013 08:08:18 PM, Kevin Hao wrote:
> On Wed, Jul 03, 2013 at 03:38:27PM -0500, Scott Wood wrote:
> > On 07/02/2013 10:00:44 PM, Kevin Hao wrote:
> > >On Tue, Jul 02, 2013 at 05:39:18PM -0500, Scott Wood wrote:
> > >> How much overhead (space and time) is this really?
> > >
> > >The following is the additional sections when relocatable is
> > >enabled for
> > >a p2020rdb board.
> > >   section        size
> > >  .dynsym       000007f0
> > >  .dynstr       00000926
> > >  .dynamic      00000080
> > >  .hash         00000388
> > >  .interp       00000011
> > >  .rela.dyn     00215250
> > >
> > >The time for the relocation is about 32ms on a p2020rdb board.
> >
> > Hmm... more relocations than I expected.  What percentage is this of
> > the total image size?
>=20
> The size of vmlinux.bin is about 10M. The percentage of the relocation
> section is about 20%. But look on the bright side of thing, all the =20
> relocation
> stuff are in init section and should be discarded at runtime. :-)

That doesn't reduce the space it takes up in flash...

-Scott=

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

On Mon, Jul 08, 2013 at 11:48:25AM -0500, Scott Wood wrote:
> On 07/03/2013 08:08:18 PM, Kevin Hao wrote:
> >On Wed, Jul 03, 2013 at 03:38:27PM -0500, Scott Wood wrote:
> >> On 07/02/2013 10:00:44 PM, Kevin Hao wrote:
> >> >On Tue, Jul 02, 2013 at 05:39:18PM -0500, Scott Wood wrote:
> >> >> How much overhead (space and time) is this really?
> >> >
> >> >The following is the additional sections when relocatable is
> >> >enabled for
> >> >a p2020rdb board.
> >> >   section        size
> >> >  .dynsym       000007f0
> >> >  .dynstr       00000926
> >> >  .dynamic      00000080
> >> >  .hash         00000388
> >> >  .interp       00000011
> >> >  .rela.dyn     00215250
> >> >
> >> >The time for the relocation is about 32ms on a p2020rdb board.
> >>
> >> Hmm... more relocations than I expected.  What percentage is this of
> >> the total image size?
> >
> >The size of vmlinux.bin is about 10M. The percentage of the relocation
> >section is about 20%. But look on the bright side of thing, all
> >the relocation
> >stuff are in init section and should be discarded at runtime. :-)
> 
> That doesn't reduce the space it takes up in flash...

Yes. But since we always use a compressed uImage kernel in flash,
the increasing size of the image would shrink to about 500K.

Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Scott Wood]

On 06/27/2013 08:36:37 PM, Kevin Hao wrote:
> On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> > On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> > >This is based on the codes in the head_44x.S. Since we always =20
> align to
> > >256M before mapping the PAGE_OFFSET for a relocatable kernel, we =20
> also
> > >change the init tlb map to 256M size.
> >
> > Why 256M?
>=20
> For two reasons:
>   1. This is the size which both e500v1 and e500v2 support.
>   2. Since we always use the PAGE_OFFSET as 0xc0000000, the 256M is
>      max alignment value we can use for this virtual address.

0xc0000000 is 1G-aligned, so I don't see why 256M is the maximum (after =20
verifying that enough memory is present with the right alignment, of =20
course).  The TLB1 savings would probably not be enough to justify =20
figuring that out, though.

-Scott=

Re: [PATCH 1/2] powerpc: enable the relocatable support for the fsl booke 32bit kernel

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 1468 bytes --]

On Thu, Jun 27, 2013 at 08:52:20PM -0500, Scott Wood wrote:
> On 06/27/2013 08:36:37 PM, Kevin Hao wrote:
> >On Thu, Jun 27, 2013 at 02:58:34PM -0500, Scott Wood wrote:
> >> On 06/26/2013 09:00:33 PM, Kevin Hao wrote:
> >> >This is based on the codes in the head_44x.S. Since we always
> >align to
> >> >256M before mapping the PAGE_OFFSET for a relocatable kernel,
> >we also
> >> >change the init tlb map to 256M size.
> >>
> >> Why 256M?
> >
> >For two reasons:
> >  1. This is the size which both e500v1 and e500v2 support.
> >  2. Since we always use the PAGE_OFFSET as 0xc0000000, the 256M is
> >     max alignment value we can use for this virtual address.
> 
> 0xc0000000 is 1G-aligned, so I don't see why 256M is the maximum
> (after verifying that enough memory is present with the right
> alignment, of course).  The TLB1 savings would probably not be
> enough to justify figuring that out, though.

Sorry, I didn't make myself clear. Yes, the 0xc0000000 is 1G-aligned.
For a 32bit kernel we only have a 1G memory region for the kernel space,
but we can't use all of it for direct map. So we always set the
__max_low_memory to 0x30000000. And for e500 core, it doesn't support
512M page size. So the 256M is the max page size we can use for a
32bit kernel. And since we always cover the boot code in the first
tlb entry, we then use the 256M as the alignment value for the
relocatable kernel.

Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

[PATCH 2/2] powerpc/fsl_booke: enable the relocatable for the kdump kernel

[Kevin Hao]

For a relocatable kdump kernel, we may create a tlb map which is
beyond the real memory allocated to the kdump kernel. For example,
when the boot kernel reserve 32M memory for the kdump kernel by
using 'crashkernel=32M@64M', we will have to create a 256M tlb
entry in the kdump kernel. So define PPC_PIN_SIZE for fsl ppc32 bit,
this will make sure that we still get the right VMALLOC_START in
this case.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/Kconfig                  |  3 +--
 arch/powerpc/include/asm/mmu-book3e.h |  5 +++++
 arch/powerpc/mm/fsl_booke_mmu.c       | 30 +++++++++++++++++++++++++++---
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 9eb97ac..ca237f5 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -382,8 +382,7 @@ config KEXEC
 config CRASH_DUMP
 	bool "Build a kdump crash kernel"
 	depends on PPC64 || 6xx || FSL_BOOKE || (44x && !SMP)
-	select RELOCATABLE if PPC64 || 44x
-	select DYNAMIC_MEMSTART if FSL_BOOKE
+	select RELOCATABLE if PPC64 || 44x || FSL_BOOKE
 	help
 	  Build a kernel suitable for use as a kdump capture kernel.
 	  The same kernel binary can be used as production kernel and dump
diff --git a/arch/powerpc/include/asm/mmu-book3e.h b/arch/powerpc/include/asm/mmu-book3e.h
index 936db36..bf422db 100644
--- a/arch/powerpc/include/asm/mmu-book3e.h
+++ b/arch/powerpc/include/asm/mmu-book3e.h
@@ -214,6 +214,11 @@
 #define TLBILX_T_CLASS2			6
 #define TLBILX_T_CLASS3			7
 
+#ifdef CONFIG_PPC32
+/* The max size that one tlb can map in a 32bit kernel. */
+#define PPC_PIN_SIZE	(1 << 28)	/* 256M */
+#endif
+
 #ifndef __ASSEMBLY__
 #include <asm/bug.h>
 
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index 07ba45b..59549b3 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -52,6 +52,7 @@
 #include <asm/smp.h>
 #include <asm/machdep.h>
 #include <asm/setup.h>
+#include <asm/sections.h>
 
 #include "mmu_decl.h"
 
@@ -177,11 +178,34 @@ unsigned long map_mem_in_cams(unsigned long ram, int max_cam_idx)
 	unsigned long virt = PAGE_OFFSET;
 	phys_addr_t phys = memstart_addr;
 	unsigned long amount_mapped = 0;
-
+	unsigned long cam_sz;
+
+#if defined(CONFIG_RELOCATABLE) && defined(CONFIG_PPC32)
+	/*
+	 * For a relocatable kernel, we would not map from memstart_addr.
+	 * We first align to PPC_PIN_SIZE (256M), then map the PAGE_OFFSET
+	 * from there.
+	 */
+	phys &= ~(PPC_PIN_SIZE - 1);
+	ram += memstart_addr & (PPC_PIN_SIZE - 1);
+
+	/*
+	 * For a kdump kernel, we may use a memory area reserved by the boot
+	 * kernel by using a kernel option like this 'crashkernel=32M@64M'.
+	 * In this case, the ram is 96M. The kernel will try to map the first
+	 * 64M in the first tlb entry. The kernel will definitely get stuck,
+	 * since the kernel is running above the 64M. So we have to make sure
+	 * that the first tlb cover the current kernel running address at least.
+	 */
+	while (1) {
+		cam_sz = calc_cam_sz(ram, virt, phys);
+		if (cam_sz + phys > PHYSICAL_START + _end - _stext)
+			break;
+		ram = 1 << (ilog2(ram) + 1);
+	}
+#endif
 	/* Calculate CAM values */
 	for (i = 0; ram && i < max_cam_idx; i++) {
-		unsigned long cam_sz;
-
 		cam_sz = calc_cam_sz(ram, virt, phys);
 		settlbcam(i, virt, phys, cam_sz, PAGE_KERNEL_X, 0);
 
-- 
1.8.1.4

Re: [PATCH 2/2] powerpc/fsl_booke: enable the relocatable for the kdump kernel

[Scott Wood]

On 06/26/2013 09:00:34 PM, Kevin Hao wrote:
> diff --git a/arch/powerpc/include/asm/mmu-book3e.h =20
> b/arch/powerpc/include/asm/mmu-book3e.h
> index 936db36..bf422db 100644
> --- a/arch/powerpc/include/asm/mmu-book3e.h
> +++ b/arch/powerpc/include/asm/mmu-book3e.h
> @@ -214,6 +214,11 @@
>  #define TLBILX_T_CLASS2			6
>  #define TLBILX_T_CLASS3			7
>=20
> +#ifdef CONFIG_PPC32
> +/* The max size that one tlb can map in a 32bit kernel. */
> +#define PPC_PIN_SIZE	(1 << 28)	/* 256M */
> +#endif

That comment is not true for all chips.

> @@ -177,11 +178,34 @@ unsigned long map_mem_in_cams(unsigned long =20
> ram, int max_cam_idx)
>  	unsigned long virt =3D PAGE_OFFSET;
>  	phys_addr_t phys =3D memstart_addr;
>  	unsigned long amount_mapped =3D 0;
> -
> +	unsigned long cam_sz;
> +
> +#if defined(CONFIG_RELOCATABLE) && defined(CONFIG_PPC32)
> +	/*
> +	 * For a relocatable kernel, we would not map from =20
> memstart_addr.
> +	 * We first align to PPC_PIN_SIZE (256M), then map the =20
> PAGE_OFFSET
> +	 * from there.
> +	 */
> +	phys &=3D ~(PPC_PIN_SIZE - 1);
> +	ram +=3D memstart_addr & (PPC_PIN_SIZE - 1);

You should not map anything before memstart_addr.  If memstart_addr =20
isn't 256M-aligned, you'll have to either use smaller pages or consider =20
that region to be "high"mem (assuming Linux supports highmem existing =20
below lowmem -- I'm skeptical).

> +	/*
> +	 * For a kdump kernel, we may use a memory area reserved by the =20
> boot
> +	 * kernel by using a kernel option like this =20
> 'crashkernel=3D32M@64M'.
> +	 * In this case, the ram is 96M. The kernel will try to map the =20
> first
> +	 * 64M in the first tlb entry. The kernel will definitely get =20
> stuck,
> +	 * since the kernel is running above the 64M. So we have to =20
> make sure
> +	 * that the first tlb cover the current kernel running address =20
> at least.
> +	 */

Maybe we should be running from AS1 when we set this up, to avoid =20
problems replacing an entry while it's in use?

Pardon my ignorance about how kdump/kexec works, but I'm a bit confused =20
by exactly what the situation is with crashkernel.  How do we know that =20
we are the crash kernel, and that we should limit our RAM usage to that =20
area?  I'm wondering if this code is assuming that the crashkernel area =20
is from where the kernel starts to the end of RAM.

> +	while (1) {
> +		cam_sz =3D calc_cam_sz(ram, virt, phys);
> +		if (cam_sz + phys > PHYSICAL_START + _end - _stext)
> +			break;
> +		ram =3D 1 << (ilog2(ram) + 1);
> +	}

The ram that was passed in is as much as you have.  Don't map more.

What happens if (e.g.) memstart_addr is 512M, with a size of 512M, and =20
the kernel starts at 768M?  Increasing the size will never get you a =20
mapping that covers kernstart, because calc_cam_sz will never return =20
more than 256M.

When does memory below the rounded-down kernel start get mapped?

-Scott=

Re: [PATCH 2/2] powerpc/fsl_booke: enable the relocatable for the kdump kernel

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 3879 bytes --]

On Thu, Jun 27, 2013 at 09:19:06PM -0500, Scott Wood wrote:
> On 06/26/2013 09:00:34 PM, Kevin Hao wrote:
> >diff --git a/arch/powerpc/include/asm/mmu-book3e.h
> >b/arch/powerpc/include/asm/mmu-book3e.h
> >index 936db36..bf422db 100644
> >--- a/arch/powerpc/include/asm/mmu-book3e.h
> >+++ b/arch/powerpc/include/asm/mmu-book3e.h
> >@@ -214,6 +214,11 @@
> > #define TLBILX_T_CLASS2			6
> > #define TLBILX_T_CLASS3			7
> >
> >+#ifdef CONFIG_PPC32
> >+/* The max size that one tlb can map in a 32bit kernel. */
> >+#define PPC_PIN_SIZE	(1 << 28)	/* 256M */
> >+#endif
> 
> That comment is not true for all chips.

This is not for the hardware limitation.

> 
> >@@ -177,11 +178,34 @@ unsigned long map_mem_in_cams(unsigned long
> >ram, int max_cam_idx)
> > 	unsigned long virt = PAGE_OFFSET;
> > 	phys_addr_t phys = memstart_addr;
> > 	unsigned long amount_mapped = 0;
> >-
> >+	unsigned long cam_sz;
> >+
> >+#if defined(CONFIG_RELOCATABLE) && defined(CONFIG_PPC32)
> >+	/*
> >+	 * For a relocatable kernel, we would not map from
> >memstart_addr.
> >+	 * We first align to PPC_PIN_SIZE (256M), then map the
> >PAGE_OFFSET
> >+	 * from there.
> >+	 */
> >+	phys &= ~(PPC_PIN_SIZE - 1);
> >+	ram += memstart_addr & (PPC_PIN_SIZE - 1);
> 
> You should not map anything before memstart_addr.  If memstart_addr
> isn't 256M-aligned, you'll have to either use smaller pages or
> consider that region to be "high"mem (assuming Linux supports
> highmem existing below lowmem -- I'm skeptical).

OK, I will try to find a another way to resolve this issue.

> 
> >+	/*
> >+	 * For a kdump kernel, we may use a memory area reserved by the
> >boot
> >+	 * kernel by using a kernel option like this
> >'crashkernel=32M@64M'.
> >+	 * In this case, the ram is 96M. The kernel will try to map the
> >first
> >+	 * 64M in the first tlb entry. The kernel will definitely get
> >stuck,
> >+	 * since the kernel is running above the 64M. So we have to make
> >sure
> >+	 * that the first tlb cover the current kernel running address
> >at least.
> >+	 */
> 
> Maybe we should be running from AS1 when we set this up, to avoid
> problems replacing an entry while it's in use?

I thought about this. The reason that I don't use this method is
we also have to do another relocation if we just want to map
the reserved memory for the kernel.

> 
> Pardon my ignorance about how kdump/kexec works, but I'm a bit
> confused by exactly what the situation is with crashkernel.  How do
> we know that we are the crash kernel, and that we should limit our
> RAM usage to that area?

The kexec tool will parse the command line of the boot kernel and get
the reserved memory info (such as start address, size) and then pass
these informations to the kdump kernel via device tree.

>  I'm wondering if this code is assuming that
> the crashkernel area is from where the kernel starts to the end of
> RAM.

No. We get these information from device tree.

> 
> >+	while (1) {
> >+		cam_sz = calc_cam_sz(ram, virt, phys);
> >+		if (cam_sz + phys > PHYSICAL_START + _end - _stext)
> >+			break;
> >+		ram = 1 << (ilog2(ram) + 1);
> >+	}
> 
> The ram that was passed in is as much as you have.  Don't map more.
> 
> What happens if (e.g.) memstart_addr is 512M, with a size of 512M,
> and the kernel starts at 768M?  Increasing the size will never get
> you a mapping that covers kernstart, because calc_cam_sz will never
> return more than 256M.

Yes, the current code still can't handle this case. We always assume
that the kernel is in the memory region which can be covered by the
first tlb entry.

> 
> When does memory below the rounded-down kernel start get mapped?

This only get mapped via __ioremap when we need to read from it such as
creating the vmcore image.

Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 2/2] powerpc/fsl_booke: enable the relocatable for the kdump kernel

[Scott Wood]

On 06/30/2013 02:35:21 AM, Kevin Hao wrote:
> On Thu, Jun 27, 2013 at 09:19:06PM -0500, Scott Wood wrote:
> > On 06/26/2013 09:00:34 PM, Kevin Hao wrote:
> > >diff --git a/arch/powerpc/include/asm/mmu-book3e.h
> > >b/arch/powerpc/include/asm/mmu-book3e.h
> > >index 936db36..bf422db 100644
> > >--- a/arch/powerpc/include/asm/mmu-book3e.h
> > >+++ b/arch/powerpc/include/asm/mmu-book3e.h
> > >@@ -214,6 +214,11 @@
> > > #define TLBILX_T_CLASS2			6
> > > #define TLBILX_T_CLASS3			7
> > >
> > >+#ifdef CONFIG_PPC32
> > >+/* The max size that one tlb can map in a 32bit kernel. */
> > >+#define PPC_PIN_SIZE	(1 << 28)	/* 256M */
> > >+#endif
> >
> > That comment is not true for all chips.
>=20
> This is not for the hardware limitation.

It's not for a general software limitation, either.  We can use 1G =20
mappings for user hugetlb (if the default mmap address is moved out of =20
the way) or for the kernel lowmem mapping (if the address is moved to =20
0x80000000 instead of 0xc0000000).

It's also possible (although unlikely at this point) that someone could =20
make a 32-bit booke chip that cannot handle 256M mappings.

> > Pardon my ignorance about how kdump/kexec works, but I'm a bit
> > confused by exactly what the situation is with crashkernel.  How do
> > we know that we are the crash kernel, and that we should limit our
> > RAM usage to that area?
>=20
> The kexec tool will parse the command line of the boot kernel and get
> the reserved memory info (such as start address, size) and then pass
> these informations to the kdump kernel via device tree.

Does this alter memstart_addr or just mark the region as reserved?

> > >+	while (1) {
> > >+		cam_sz =3D calc_cam_sz(ram, virt, phys);
> > >+		if (cam_sz + phys > PHYSICAL_START + _end - _stext)
> > >+			break;
> > >+		ram =3D 1 << (ilog2(ram) + 1);
> > >+	}
> >
> > The ram that was passed in is as much as you have.  Don't map more.
> >
> > What happens if (e.g.) memstart_addr is 512M, with a size of 512M,
> > and the kernel starts at 768M?  Increasing the size will never get
> > you a mapping that covers kernstart, because calc_cam_sz will never
> > return more than 256M.
>=20
> Yes, the current code still can't handle this case. We always assume
> that the kernel is in the memory region which can be covered by the
> first tlb entry.

Assuming memstart_addr isn't affected by the crashkernel reservation =20
(if it is, could you point out where?), you'd then have a problem with =20
the crashkernel area going anywhere above 256M.

-Scott=

Re: [PATCH 2/2] powerpc/fsl_booke: enable the relocatable for the kdump kernel

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 3397 bytes --]

On Mon, Jul 01, 2013 at 08:00:06PM -0500, Scott Wood wrote:
> On 06/30/2013 02:35:21 AM, Kevin Hao wrote:
> >On Thu, Jun 27, 2013 at 09:19:06PM -0500, Scott Wood wrote:
> >> On 06/26/2013 09:00:34 PM, Kevin Hao wrote:
> >> >diff --git a/arch/powerpc/include/asm/mmu-book3e.h
> >> >b/arch/powerpc/include/asm/mmu-book3e.h
> >> >index 936db36..bf422db 100644
> >> >--- a/arch/powerpc/include/asm/mmu-book3e.h
> >> >+++ b/arch/powerpc/include/asm/mmu-book3e.h
> >> >@@ -214,6 +214,11 @@
> >> > #define TLBILX_T_CLASS2			6
> >> > #define TLBILX_T_CLASS3			7
> >> >
> >> >+#ifdef CONFIG_PPC32
> >> >+/* The max size that one tlb can map in a 32bit kernel. */
> >> >+#define PPC_PIN_SIZE	(1 << 28)	/* 256M */
> >> >+#endif
> >>
> >> That comment is not true for all chips.
> >
> >This is not for the hardware limitation.
> 
> It's not for a general software limitation, either.  We can use 1G
> mappings for user hugetlb (if the default mmap address is moved out
> of the way) or for the kernel lowmem mapping (if the address is
> moved to 0x80000000 instead of 0xc0000000).
> 
> It's also possible (although unlikely at this point) that someone
> could make a 32-bit booke chip that cannot handle 256M mappings.

Then I have to agree with you. :-)
Wish I can find a way to drop it.

> 
> >> Pardon my ignorance about how kdump/kexec works, but I'm a bit
> >> confused by exactly what the situation is with crashkernel.  How do
> >> we know that we are the crash kernel, and that we should limit our
> >> RAM usage to that area?
> >
> >The kexec tool will parse the command line of the boot kernel and get
> >the reserved memory info (such as start address, size) and then pass
> >these informations to the kdump kernel via device tree.
> 
> Does this alter memstart_addr or just mark the region as reserved?

This will alter memstart_addr since we set memstart_addr based on the
memory node in device tree.

> 
> >> >+	while (1) {
> >> >+		cam_sz = calc_cam_sz(ram, virt, phys);
> >> >+		if (cam_sz + phys > PHYSICAL_START + _end - _stext)
> >> >+			break;
> >> >+		ram = 1 << (ilog2(ram) + 1);
> >> >+	}
> >>
> >> The ram that was passed in is as much as you have.  Don't map more.
> >>
> >> What happens if (e.g.) memstart_addr is 512M, with a size of 512M,
> >> and the kernel starts at 768M?  Increasing the size will never get
> >> you a mapping that covers kernstart, because calc_cam_sz will never
> >> return more than 256M.
> >
> >Yes, the current code still can't handle this case. We always assume
> >that the kernel is in the memory region which can be covered by the
> >first tlb entry.
> 
> Assuming memstart_addr isn't affected by the crashkernel reservation
> (if it is, could you point out where?),

No. The memstart_addr does be affected. For example, for a boot kernel
with the command line with "crashkernel=32M@64M", the device tree passed
the kdump kernel will have a memory node like this:
    memory {
            reg = <0x0 0x4000000 0x0 0x2000000>;
            device_type = "memory";
    }

Then the memstart_addr will be set to 0x4000000 in the kdump kernel.

> you'd then have a problem
> with the crashkernel area going anywhere above 256M.

The kdump kernel can be loaded above 256M memory. It only can not be loaded
above memstart_addr + 256M memory.

Thanks,
Kevin

> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH 2/2] powerpc/fsl_booke: enable the relocatable for the kdump kernel

[Scott Wood]

On 07/01/2013 10:45:14 PM, Kevin Hao wrote:
> On Mon, Jul 01, 2013 at 08:00:06PM -0500, Scott Wood wrote:
> > On 06/30/2013 02:35:21 AM, Kevin Hao wrote:
> > >On Thu, Jun 27, 2013 at 09:19:06PM -0500, Scott Wood wrote:
> > >> On 06/26/2013 09:00:34 PM, Kevin Hao wrote:
> > >> >diff --git a/arch/powerpc/include/asm/mmu-book3e.h
> > >> >b/arch/powerpc/include/asm/mmu-book3e.h
> > >> >index 936db36..bf422db 100644
> > >> >--- a/arch/powerpc/include/asm/mmu-book3e.h
> > >> >+++ b/arch/powerpc/include/asm/mmu-book3e.h
> > >> >@@ -214,6 +214,11 @@
> > >> > #define TLBILX_T_CLASS2			6
> > >> > #define TLBILX_T_CLASS3			7
> > >> >
> > >> >+#ifdef CONFIG_PPC32
> > >> >+/* The max size that one tlb can map in a 32bit kernel. */
> > >> >+#define PPC_PIN_SIZE	(1 << 28)	/* 256M */
> > >> >+#endif
> > >>
> > >> That comment is not true for all chips.
> > >
> > >This is not for the hardware limitation.
> >
> > It's not for a general software limitation, either.  We can use 1G
> > mappings for user hugetlb (if the default mmap address is moved out
> > of the way) or for the kernel lowmem mapping (if the address is
> > moved to 0x80000000 instead of 0xc0000000).
> >
> > It's also possible (although unlikely at this point) that someone
> > could make a 32-bit booke chip that cannot handle 256M mappings.
>=20
> Then I have to agree with you. :-)
> Wish I can find a way to drop it.

At least give it a comment that describes how it's actually used.

> > Assuming memstart_addr isn't affected by the crashkernel reservation
> > (if it is, could you point out where?),
>=20
> No. The memstart_addr does be affected. For example, for a boot kernel
> with the command line with "crashkernel=3D32M@64M", the device tree =20
> passed
> the kdump kernel will have a memory node like this:
>     memory {
>             reg =3D <0x0 0x4000000 0x0 0x2000000>;
>             device_type =3D "memory";
>     }
>=20
> Then the memstart_addr will be set to 0x4000000 in the kdump kernel.

OK, so then how does the crash kernel know which regions to dump?  Is =20
that indicated somewhere else in the device tree?

-Scott=

Re: [PATCH 2/2] powerpc/fsl_booke: enable the relocatable for the kdump kernel

[Kevin Hao]

[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]

On Tue, Jul 02, 2013 at 05:41:05PM -0500, Scott Wood wrote:
> On 07/01/2013 10:45:14 PM, Kevin Hao wrote:
> >On Mon, Jul 01, 2013 at 08:00:06PM -0500, Scott Wood wrote:
> >> On 06/30/2013 02:35:21 AM, Kevin Hao wrote:
> >> >On Thu, Jun 27, 2013 at 09:19:06PM -0500, Scott Wood wrote:
> >> >> On 06/26/2013 09:00:34 PM, Kevin Hao wrote:
> >> >> >diff --git a/arch/powerpc/include/asm/mmu-book3e.h
> >> >> >b/arch/powerpc/include/asm/mmu-book3e.h
> >> >> >index 936db36..bf422db 100644
> >> >> >--- a/arch/powerpc/include/asm/mmu-book3e.h
> >> >> >+++ b/arch/powerpc/include/asm/mmu-book3e.h
> >> >> >@@ -214,6 +214,11 @@
> >> >> > #define TLBILX_T_CLASS2			6
> >> >> > #define TLBILX_T_CLASS3			7
> >> >> >
> >> >> >+#ifdef CONFIG_PPC32
> >> >> >+/* The max size that one tlb can map in a 32bit kernel. */
> >> >> >+#define PPC_PIN_SIZE	(1 << 28)	/* 256M */
> >> >> >+#endif
> >> >>
> >> >> That comment is not true for all chips.
> >> >
> >> >This is not for the hardware limitation.
> >>
> >> It's not for a general software limitation, either.  We can use 1G
> >> mappings for user hugetlb (if the default mmap address is moved out
> >> of the way) or for the kernel lowmem mapping (if the address is
> >> moved to 0x80000000 instead of 0xc0000000).
> >>
> >> It's also possible (although unlikely at this point) that someone
> >> could make a 32-bit booke chip that cannot handle 256M mappings.
> >
> >Then I have to agree with you. :-)
> >Wish I can find a way to drop it.
> 
> At least give it a comment that describes how it's actually used.
> 
> >> Assuming memstart_addr isn't affected by the crashkernel reservation
> >> (if it is, could you point out where?),
> >
> >No. The memstart_addr does be affected. For example, for a boot kernel
> >with the command line with "crashkernel=32M@64M", the device tree
> >passed
> >the kdump kernel will have a memory node like this:
> >    memory {
> >            reg = <0x0 0x4000000 0x0 0x2000000>;
> >            device_type = "memory";
> >    }
> >
> >Then the memstart_addr will be set to 0x4000000 in the kdump kernel.
> 
> OK, so then how does the crash kernel know which regions to dump?
> Is that indicated somewhere else in the device tree?

No. We get the dump of the memory via /proc/vmcore in the kdump kernel.

Thanks,
Kevin
> 
> -Scott

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]