powerpc/85xx:Make debug exception vector accessible

PATCH Superseded Prabhakar Kushwaha 3 participants 15 messages Mar 21, 2012 – Mar 26, 2012 PowerPC

Share & tools

Prabhakar Kushwaha

author

2012-03-21 04:43 UTC #0 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Debugging of e500 and e500v1 processer requires debug exception vecter (IVPR + IVOR15) to have valid and fetchable OP code.

While executing in translated space (AS=1), whenever a debug exception is generated, the MSR[DS/IS] gets cleared i.e. AS=0 and the processor tries to fetch an instruction from the debug exception vector (IVPR + IVOR15); since now we are in AS=0, the application needs to ensure the proper TLB configuration to have (IVOR + IVOR15) accessible from AS=0 also.

Create a temporary TLB in AS0 to make sure debug exception verctor is accessible on debug exception.

Signed-off-by Radu Lazarescu <[email protected]>

Signed-off-by Marius Grigoras <[email protected]>

Signed-off-by Prabhakar Kushwaha <[email protected]>

---
Based upon git://git.denx.de/u-boot.git branch master

Changes for v2:
- Put Temporary TLB creation under #define

Tested on
- SoC having E500 Family processor (P1010RDB, BSC9131RDB)
- SoC having E500MC Family processor (P4080DS, P3041DS)

 arch/powerpc/cpu/mpc85xx/cpu_init_early.c |   32 +++++++++++++++-
 arch/powerpc/cpu/mpc85xx/start.S          |   60 +++++++++++++++++++++++++++++
 arch/powerpc/include/asm/config_mpc85xx.h |    3 +-
 3 files changed, 93 insertions(+), 2 deletions(-)

arch/powerpc/cpu/mpc85xx/cpu_init_early.c +31 -1

index 091af7c..d0b15a4 100644
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2011 Freescale Semiconductor, Inc
+ * Copyright 2009-2012 Freescale Semiconductor, Inc
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as

@@ -53,6 +53,36 @@ void setup_ifc(void)
 
 	asm volatile("isync;msync;tlbwe;isync");
 
+#if defined(CONFIG_E500) && defined(CONFIG_SYS_PPC_E500_DEBUG_TLB)
+/*
+ * TLB for debuggging in AS1
+ * Create temporary TLB in AS0 to handle debug exception
+ * As on debug exception MSR is cleared i.e. Address space is changed
+ * to 0. A TLB (in AS0) is required to handle debug exception generated
+ * in AS1.
+ *
+ * TLB is created for IVPR + IVOR15 to map on valid OP code address
+ * bacause flash's physical address is going to change as
+ * CONFIG_SYS_FLASH_BASE_PHYS.
+ */
+	_mas0 = MAS0_TLBSEL(1) |
+			MAS0_ESEL(CONFIG_SYS_PPC_E500_DEBUG_TLB);
+	_mas1 = MAS1_VALID | MAS1_TID(0) | MAS1_IPROT |
+			MAS1_TSIZE(BOOKE_PAGESZ_4M);
+	_mas2 = FSL_BOOKE_MAS2(CONFIG_SYS_TEXT_BASE, MAS2_I|MAS2_G);
+	_mas3 = FSL_BOOKE_MAS3(flash_phys, 0, MAS3_SW|MAS3_SR|MAS3_SX);
+	_mas7 = FSL_BOOKE_MAS7(flash_phys);
+
+	mtspr(MAS0, _mas0);
+	mtspr(MAS1, _mas1);
+	mtspr(MAS2, _mas2);
+	mtspr(MAS3, _mas3);
+	mtspr(MAS7, _mas7);
+
+	asm volatile("isync;msync;tlbwe;isync");
+#endif
+
+	/* Change flash's physical address */
 	out_be32(&(ifc_regs->cspr_cs[0].cspr), CONFIG_SYS_CSPR0);
 	out_be32(&(ifc_regs->csor_cs[0].csor), CONFIG_SYS_CSOR0);
 	out_be32(&(ifc_regs->amask_cs[0].amask), CONFIG_SYS_AMASK0);

arch/powerpc/cpu/mpc85xx/start.S +60 -0

66 lines not on this page · raw message

arch/powerpc/include/asm/config_mpc85xx.h +2 -1

index 8654625..268c56e 100644
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011-2012 Freescale Semiconductor, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as

@@ -107,6 +107,7 @@
 #define CONFIG_MAX_CPUS			1
 #define CONFIG_FSL_SDHC_V2_3
 #define CONFIG_SYS_FSL_NUM_LAWS		12
+#define CONFIG_SYS_PPC_E500_DEBUG_TLB	3
 #define CONFIG_TSECV2
 #define CONFIG_SYS_FSL_SEC_COMPAT	4
 #define CONFIG_FSL_SATA_V2

Signature · 1 line

1.7.5.4

Scott Wood

2012-03-21 16:34 UTC #1 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Quoted 12 lines · Prabhakar Kushwaha

arch/powerpc/cpu/mpc85xx/cpu_init_early.c +1 -1

index 091af7c..d0b15a4 100644
@@ -1,5 +1,5 @@
 /*
- * Copyright 2009-2011 Freescale Semiconductor, Inc
+ * Copyright 2009-2012 Freescale Semiconductor, Inc
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as

arch/powerpc/cpu/mpc85xx/cpu_init_early.c void setup_ifc(void) · line 56

Quoted 4 lines

arch/powerpc/cpu/mpc85xx/cpu_init_early.c +1 -0

@@ -53,6 +53,36 @@ void setup_ifc(void)
 
 	asm volatile("isync;msync;tlbwe;isync");
 
+#if defined(CONFIG_E500) && defined(CONFIG_SYS_PPC_E500_DEBUG_TLB)

There is no need to check for CONFIG_E500 anywhere in arch/powerpc/cpu/mpc85xx. mpc85xx implies CONFIG_E500.

I don't see anywhere in this patchset where you set CONFIG_SYS_PPC_E500_DEBUG_TLB on any actual board. It should be set for all e500v1/v2, not just so that debug works but so that this code remains tested.

-Scott

Prabhakar Kushwaha

author

2012-03-21 17:04 UTC #2 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Hi Scott,

Quoted 14 lines · Scott Wood

Quoted 13 lines · Prabhakar Kushwaha

arch/powerpc/cpu/mpc85xx/cpu_init_early.c +1 -1

index 091af7c..d0b15a4 100644
@@ -1,5 +1,5 @@
  /*
- * Copyright 2009-2011 Freescale Semiconductor, Inc
+ * Copyright 2009-2012 Freescale Semiconductor, Inc
   *
   * This program is free software; you can redistribute it and/or
   * modify it under the terms of the GNU General Public License as

@@ -53,6 +53,36 @@ void setup_ifc(void)

Quoted 5 lines

arch/powerpc/cpu/mpc85xx/cpu_init_early.c void setup_ifc(void)
Quoted 3 lines
arch/powerpc/cpu/mpc85xx/cpu_init_early.c +1 -0
  	asm volatile("isync;msync;tlbwe;isync");
 
+#if defined(CONFIG_E500)&&  defined(CONFIG_SYS_PPC_E500_DEBUG_TLB)
There is no need to check for CONFIG_E500 anywhere in arch/powerpc/cpu/mpc85xx. mpc85xx implies CONFIG_E500.

Ok. I will re-spin all the patches without CONFIG_E500 condition check.

Quoted 2 lines

I don't see anywhere in this patchset where you set CONFIG_SYS_PPC_E500_DEBUG_TLB on any actual board.

This CONFIG_SYS_PPC_E500_DEBUG_TLB is defined in arch/powerpc/include/asm/config_mpc85xx.h.
i defined for P1010 (in this patch). For rest of e500 v1, v2 processor family new patch-set will be sent once this base patch is accepted.

Quoted 3 lines

It should be set for
all e500v1/v2, not just so that debug works but so that this code remains tested.

Yes. I agree with you.
It will be automatically enabled when CONFIG_SYS_PPC_E500_DEBUG_TLB is defined per SoC(e500 v1/v2). As of now only for P1010 it is enabled.

--Prabhakar

Scott Wood

2012-03-21 17:08 UTC #3 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Quoted 10 lines · Prabhakar Kushwaha

Hi Scott,
Quoted 2 lines · Scott Wood
I don't see anywhere in this patchset where you set CONFIG_SYS_PPC_E500_DEBUG_TLB on any actual board.
This CONFIG_SYS_PPC_E500_DEBUG_TLB is defined in arch/powerpc/include/asm/config_mpc85xx.h.
i defined for P1010 (in this patch). For rest of e500 v1, v2 processor family new patch-set will be sent once this base patch is accepted.

Ah sorry, missed that. :-P

-Scott

Scott Wood

2012-03-21 19:52 UTC #4 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

In reply to Prabhakar Kushwaha

Quoted 9 lines · Prabhakar Kushwaha

Debugging of e500 and e500v1 processer requires debug exception vecter (IVPR + IVOR15) to have valid and fetchable OP code.
While executing in translated space (AS=1), whenever a debug exception is generated, the MSR[DS/IS] gets cleared i.e. AS=0 and the processor tries to fetch an instruction from the debug exception vector (IVPR + IVOR15); since now we are in AS=0, the application needs to ensure the proper TLB configuration to have (IVOR + IVOR15) accessible from AS=0 also.

Quoted 6 lines

Create a temporary TLB in AS0 to make sure debug exception verctor is accessible on debug exception.
Signed-off-by Radu Lazarescu <[email protected]>
Signed-off-by Marius Grigoras <[email protected]>
Signed-off-by Prabhakar Kushwaha <[email protected]>

Can you document the flow of exactly what TLB entries are present at various points of the boot flow, for all the various configurations (NOR boot, NAND SPL, RAMBOOT, IFC versus non-IFC, etc).

Quoted 6 lines

arch/powerpc/cpu/mpc85xx/start.S +0 -0

index 597151b..cef00ba 100644
@@ -182,6 +182,66 @@ l2_disabled:
 	andi.	r1,r3,L1CSR0_DCE at l

arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 192

Quoted 10 lines

arch/powerpc/cpu/mpc85xx/start.S +8 -0

 	beq	2b
 
+#if defined(CONFIG_E500) && defined(CONFIG_SYS_PPC_E500_DEBUG_TLB)
+/*
+ * TLB for debuggging in AS1
+ * Create temporary TLB in AS0 to handle debug exception
+ * As on debug exception MSR is cleared i.e. Address space is changed
+ * to 0. A TLB (in AS0) is required to handle debug exception generated
+ * in AS1.
+ */

s/TLB/TLB entry/g

Quoted 29 lines

arch/powerpc/cpu/mpc85xx/start.S +29 -0

+
+	lis     r6,FSL_BOOKE_MAS0(1,
+			CONFIG_SYS_PPC_E500_DEBUG_TLB, 0)@h
+	ori     r6,r6,FSL_BOOKE_MAS0(1,
+			CONFIG_SYS_PPC_E500_DEBUG_TLB, 0)@l
+
+#if !defined(CONFIG_SYS_RAMBOOT)
+/*
+ * TLB is created for IVPR + IVOR15 to map on valid OP code address
+ * bacause flash's virtual address maps to 0xff800000 - 0xffffffff.
+ * and this window is outside of 4K boot window.
+ */
+	lis     r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_4M)@h
+	ori     r7,r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_4M)@l
+
+	lis     r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE & 0xffc00000,
+							(MAS2_I|MAS2_G))@h
+	ori     r8,r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE & 0xffc00000,
+							(MAS2_I|MAS2_G))@l
+
+	/* The 85xx has the default boot window 0xff800000 - 0xffffffff */
+	lis     r9,FSL_BOOKE_MAS3(0xffc00000, 0, (MAS3_SX|MAS3_SW|MAS3_SR))@h
+	ori     r9,r9,FSL_BOOKE_MAS3(0xffc00000, 0, (MAS3_SX|MAS3_SW|MAS3_SR))@l
+#else
+/*
+ * TLB is created for IVPR + IVOR15 to map on valid OP code address
+ * because "nexti" will resize TLB to 4K
+ */
+	lis     r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_256K)@h

arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 231

Quoted 10 lines

arch/powerpc/cpu/mpc85xx/start.S +10 -0

+	ori     r7,r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_256K)@l
+
+	lis     r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE, (MAS2_I|MAS2_G))@h
+	ori     r8,r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE,
+							(MAS2_I|MAS2_G))@l
+	lis     r9,FSL_BOOKE_MAS3(CONFIG_SYS_MONITOR_BASE, 0,
+						(MAS3_SX|MAS3_SW|MAS3_SR))@h
+	ori     r9,r9,FSL_BOOKE_MAS3(CONFIG_SYS_MONITOR_BASE, 0,
+						(MAS3_SX|MAS3_SW|MAS3_SR))@l
+#endif

In the ramboot case is this really supposed to be I+G?

Which path will NAND SPL go through (not the payload, but the SPL itself)? That will be only a 4K window mapped, and guarded doesn't stop speculative instruction fetches, so we don't want to map more than is backed up by something.

arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 233

Quoted 2 lines

arch/powerpc/cpu/mpc85xx/start.S +2 -0
+	lis     r10,0xffc00000 at h
+	ori     r10,r10,0xffc00000 at l

Don't waste instructions -- this could be in an SPL. That ori is a no-op.

arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 239

Quoted 6 lines

arch/powerpc/cpu/mpc85xx/start.S +6 -0

+
+	mtspr   MAS0,r6
+	mtspr   MAS1,r7
+	mtspr   MAS2,r8
+	mtspr   MAS3,r9
+	mtspr   MAS7,r10

Why are you writing 0xffc00000 into MAS7?

Access to MAS7 needs to be conditional on CONFIG_ENABLE_36BIT_PHYS (misnamed, should be something like CONFIG_SYS_PPC_HAS_MAS7).

arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 243

Quoted 4 lines

arch/powerpc/cpu/mpc85xx/start.S +4 -0
+	isync
+	msync
+	tlbwe
+#endif

Need isync after the tlbwe. I don't think we need the msync before it.

Quoted 12 lines

arch/powerpc/include/asm/config_mpc85xx.h +1 -1

index 8654625..268c56e 100644
@@ -1,5 +1,5 @@
 /*
- * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011-2012 Freescale Semiconductor, Inc.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as

arch/powerpc/include/asm/config_mpc85xx.h line 113

Quoted 7 lines

arch/powerpc/include/asm/config_mpc85xx.h +1 -0

@@ -107,6 +107,7 @@
 #define CONFIG_MAX_CPUS			1
 #define CONFIG_FSL_SDHC_V2_3
 #define CONFIG_SYS_FSL_NUM_LAWS		12
+#define CONFIG_SYS_PPC_E500_DEBUG_TLB	3
 #define CONFIG_TSECV2
 #define CONFIG_SYS_FSL_SEC_COMPAT	4
 #define CONFIG_FSL_SATA_V2

It would be nice if we could have one place where all the fixed TLB entries are assigned.

-Scott

Prabhakar Kushwaha

author

2012-03-22 05:52 UTC #5 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Hi Scott,

Please find my reply in-lined

Quoted 13 lines · Scott Wood

Quoted 12 lines · Prabhakar Kushwaha
Debugging of e500 and e500v1 processer requires debug exception vecter (IVPR + IVOR15) to have valid and fetchable OP code.
While executing in translated space (AS=1), whenever a debug exception is generated, the MSR[DS/IS] gets cleared i.e. AS=0 and the processor tries to fetch an instruction from the debug exception vector (IVPR + IVOR15); since now we are in AS=0, the application needs to ensure the proper TLB configuration to have (IVOR + IVOR15) accessible from AS=0 also.
Create a temporary TLB in AS0 to make sure debug exception verctor is accessible on debug exception.

Quoted 6 lines

Quoted 3 lines
Signed-off-by Radu Lazarescu<[email protected]>
Signed-off-by Marius Grigoras<[email protected]>
Signed-off-by Prabhakar Kushwaha<[email protected]>
Can you document the flow of exactly what TLB entries are present at various points of the boot flow, for all the various configurations (NOR boot, NAND SPL, RAMBOOT, IFC versus non-IFC, etc).

Sure. May be separate patch will be send.

Quoted 8 lines

arch/powerpc/cpu/mpc85xx/start.S +0 -0

index 597151b..cef00ba 100644
@@ -182,6 +182,66 @@ l2_disabled:
  	andi.	r1,r3,L1CSR0_DCE at l
  	beq	2b

Quoted 9 lines

arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 192

Quoted 8 lines

arch/powerpc/cpu/mpc85xx/start.S +8 -0

+#if defined(CONFIG_E500)&&  defined(CONFIG_SYS_PPC_E500_DEBUG_TLB)
+/*
+ * TLB for debuggging in AS1
+ * Create temporary TLB in AS0 to handle debug exception
+ * As on debug exception MSR is cleared i.e. Address space is changed
+ * to 0. A TLB (in AS0) is required to handle debug exception generated
+ * in AS1.
+ */

s/TLB/TLB entry/g

Sure, i will update.

Quoted 32 lines

arch/powerpc/cpu/mpc85xx/start.S +32 -0

+
+	lis     r6,FSL_BOOKE_MAS0(1,
+			CONFIG_SYS_PPC_E500_DEBUG_TLB, 0)@h
+	ori     r6,r6,FSL_BOOKE_MAS0(1,
+			CONFIG_SYS_PPC_E500_DEBUG_TLB, 0)@l
+
+#if !defined(CONFIG_SYS_RAMBOOT)
+/*
+ * TLB is created for IVPR + IVOR15 to map on valid OP code address
+ * bacause flash's virtual address maps to 0xff800000 - 0xffffffff.
+ * and this window is outside of 4K boot window.
+ */
+	lis     r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_4M)@h
+	ori     r7,r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_4M)@l
+
+	lis     r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE&  0xffc00000,
+							(MAS2_I|MAS2_G))@h
+	ori     r8,r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE&  0xffc00000,
+							(MAS2_I|MAS2_G))@l
+
+	/* The 85xx has the default boot window 0xff800000 - 0xffffffff */
+	lis     r9,FSL_BOOKE_MAS3(0xffc00000, 0, (MAS3_SX|MAS3_SW|MAS3_SR))@h
+	ori     r9,r9,FSL_BOOKE_MAS3(0xffc00000, 0, (MAS3_SX|MAS3_SW|MAS3_SR))@l
+#else
+/*
+ * TLB is created for IVPR + IVOR15 to map on valid OP code address
+ * because "nexti" will resize TLB to 4K
+ */
+	lis     r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_256K)@h
+	ori     r7,r7,FSL_BOOKE_MAS1(1, 1, 0, 0, BOOKE_PAGESZ_256K)@l
+
+	lis     r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE, (MAS2_I|MAS2_G))@h

Quoted 8 lines

arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 231
Quoted 7 lines
arch/powerpc/cpu/mpc85xx/start.S +7 -0
+	ori     r8,r8,FSL_BOOKE_MAS2(CONFIG_SYS_MONITOR_BASE,
+							(MAS2_I|MAS2_G))@l
+	lis     r9,FSL_BOOKE_MAS3(CONFIG_SYS_MONITOR_BASE, 0,
+						(MAS3_SX|MAS3_SW|MAS3_SR))@h
+	ori     r9,r9,FSL_BOOKE_MAS3(CONFIG_SYS_MONITOR_BASE, 0,
+						(MAS3_SX|MAS3_SW|MAS3_SR))@l
+#endif
In the ramboot case is this really supposed to be I+G?

I am not sure. But same is done under label "create_init_ram_area" for TLB entry 15.
what you suggest.

Quoted 4 lines

Which path will NAND SPL go through (not the payload, but the SPL itself)? That will be only a 4K window mapped, and guarded doesn't stop speculative instruction fetches, so we don't want to map more than is backed up by something.

NAND SPL go via !defined(CONFIG_SYS_RAMBOOT) path.

i think NAND_SPL does not require temporary TLB as NAND SPL even does not have any interrupt vector.

I will check this point

Quoted 3 lines

arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 233
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/start.S +2 -0
+	lis     r10,0xffc00000 at h
+	ori     r10,r10,0xffc00000 at l
Don't waste instructions -- this could be in an SPL. That ori is a no-op.

Please refer above response. May be this piece of code is not required for NAND SPL

Quoted 10 lines

arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 239
Quoted 6 lines
arch/powerpc/cpu/mpc85xx/start.S +6 -0
+
+	mtspr   MAS0,r6
+	mtspr   MAS1,r7
+	mtspr   MAS2,r8
+	mtspr   MAS3,r9
+	mtspr   MAS7,r10
Why are you writing 0xffc00000 into MAS7?
Access to MAS7 needs to be conditional on CONFIG_ENABLE_36BIT_PHYS (misnamed, should be something like CONFIG_SYS_PPC_HAS_MAS7).

i will put this code under #define CONFIG_ENABLE_36BIT_PHYS

For your kind information : in start.S, label label "create_ccsr_new_tlb", "create_ccsr_old_tlb" uses MAS7 without CONFIG_ENABLE_36BIT_PHYS #define.
It should be fixed ??

Quoted 5 lines

arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 243
Quoted 4 lines
arch/powerpc/cpu/mpc85xx/start.S +4 -0
+	isync
+	msync
+	tlbwe
+#endif
Need isync after the tlbwe. I don't think we need the msync before it.

Ok.

Quoted 13 lines

arch/powerpc/include/asm/config_mpc85xx.h +1 -1

index 8654625..268c56e 100644
@@ -1,5 +1,5 @@
  /*
- * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011-2012 Freescale Semiconductor, Inc.
   *
   * This program is free software; you can redistribute it and/or
   * modify it under the terms of the GNU General Public License as

@@ -107,6 +107,7 @@
  #define CONFIG_MAX_CPUS			1

Quoted 9 lines

arch/powerpc/include/asm/config_mpc85xx.h line 113
Quoted 6 lines
arch/powerpc/include/asm/config_mpc85xx.h +1 -0
  #define CONFIG_FSL_SDHC_V2_3
  #define CONFIG_SYS_FSL_NUM_LAWS		12
+#define CONFIG_SYS_PPC_E500_DEBUG_TLB	3
  #define CONFIG_TSECV2
  #define CONFIG_SYS_FSL_SEC_COMPAT	4
  #define CONFIG_FSL_SATA_V2
It would be nice if we could have one place where all the fixed TLB entries are assigned.

Am i supposed to send separate patch having TLB entry defined for all e500 v1/v2 SoC ?

--Prabhakar

Scott Wood

2012-03-22 19:43 UTC #6 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Quoted 23 lines · Prabhakar Kushwaha

Hi Scott,
Please find my reply in-lined
Quoted 18 lines · Scott Wood
Quoted 17 lines · Prabhakar Kushwaha
Debugging of e500 and e500v1 processer requires debug exception vecter (IVPR +
IVOR15) to have valid and fetchable OP code.
While executing in translated space (AS=1), whenever a debug exception is
generated, the MSR[DS/IS] gets cleared i.e. AS=0 and the processor tries to
fetch an instruction from the debug exception vector (IVPR + IVOR15); since now
we are in AS=0, the application needs to ensure the proper TLB configuration to
have (IVOR + IVOR15) accessible from AS=0 also.
Create a temporary TLB in AS0 to make sure debug exception verctor is accessible on debug exception.

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
Signed-off-by Radu Lazarescu<[email protected]>
Signed-off-by Marius Grigoras<[email protected]>
Signed-off-by Prabhakar Kushwaha<[email protected]>
Can you document the flow of exactly what TLB entries are present at various points of the boot flow, for all the various configurations (NOR boot, NAND SPL, RAMBOOT, IFC versus non-IFC, etc).
Sure. May be separate patch will be send.

Let's start with just an e-mail thoroughly describing your understanding of this. It will provide necessary context for review.

We can clean it up for permanent documentation once it's clear to everyone what's going on.

Quoted 5 lines

Quoted 1 line
In the ramboot case is this really supposed to be I+G?
I am not sure. But same is done under label "create_init_ram_area" for TLB entry 15.
what you suggest.

I suggest as part of the request to document all of this, you figure out what should actually be mapped in each configuration. The existing code might be wrong for some of them, but we shouldn't proceed ahead blindly and make an even bigger mess.

Quoted 9 lines

Quoted 4 lines
Which path will NAND SPL go through (not the payload, but the SPL itself)? That will be only a 4K window mapped, and guarded doesn't stop speculative instruction fetches, so we don't want to map more than is backed up by something.
NAND SPL go via !defined(CONFIG_SYS_RAMBOOT) path.
i think NAND_SPL does not require temporary TLB as NAND SPL even does not have any interrupt vector.

So there's no plan to support using breakpoints or single step during the SPL? That's fine with me, but should be documented, and we should make sure this code does not run in that case.

Quoted 7 lines

Quoted 4 lines
arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 233
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/start.S +2 -0
+    lis     r10,0xffc00000 at h
+    ori     r10,r10,0xffc00000 at l
Don't waste instructions -- this could be in an SPL. That ori is a no-op.
Please refer above response. May be this piece of code is not required for NAND SPL

Still, I'd like to know why you're writing 0xffc00000 to MAS7. Only the low 4 bits of MAS7 are valid on current e500.

Quoted 10 lines

Quoted 9 lines
arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 239
Quoted 5 lines
arch/powerpc/cpu/mpc85xx/start.S +5 -0
+    mtspr   MAS0,r6
+    mtspr   MAS1,r7
+    mtspr   MAS2,r8
+    mtspr   MAS3,r9
+    mtspr   MAS7,r10
Why are you writing 0xffc00000 into MAS7?
Access to MAS7 needs to be conditional on CONFIG_ENABLE_36BIT_PHYS (misnamed, should be something like CONFIG_SYS_PPC_HAS_MAS7).

Quoted 6 lines

i will put this code under #define CONFIG_ENABLE_36BIT_PHYS
For your kind information : in start.S, label label "create_ccsr_new_tlb", "create_ccsr_old_tlb" uses MAS7 without CONFIG_ENABLE_36BIT_PHYS #define.
It should be fixed ??

Yes, it should be fixed. That was a fairly recent change and perhaps e500v1 has not been tested since then -- Timur, could you look at this?

Quoted 24 lines

Quoted 14 lines

arch/powerpc/include/asm/config_mpc85xx.h +1 -1

index 8654625..268c56e 100644
@@ -1,5 +1,5 @@
  /*
- * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011-2012 Freescale Semiconductor, Inc.
   *
   * This program is free software; you can redistribute it and/or
   * modify it under the terms of the GNU General Public License as

@@ -107,6 +107,7 @@
  #define CONFIG_MAX_CPUS            1

Quoted 9 lines

arch/powerpc/include/asm/config_mpc85xx.h line 113
Quoted 6 lines
arch/powerpc/include/asm/config_mpc85xx.h +1 -0
  #define CONFIG_FSL_SDHC_V2_3
  #define CONFIG_SYS_FSL_NUM_LAWS        12
+#define CONFIG_SYS_PPC_E500_DEBUG_TLB    3
  #define CONFIG_TSECV2
  #define CONFIG_SYS_FSL_SEC_COMPAT    4
  #define CONFIG_FSL_SATA_V2
It would be nice if we could have one place where all the fixed TLB entries are assigned.

Quoted 2 lines

Am i supposed to send separate patch having TLB entry defined for all e500 v1/v2 SoC ?

That's a different issue, I just mean having one central place where each hardcoded TLB entry gets a symbolic name, as well as a #define for where programmatic TLB entry allocation can begin.

The decision to actually make use of the TLB entry would be a different symbol, if you don't want it to be unconditional (relative to the surrounding code).

-Scott

Timur Tabi

2012-03-22 19:51 UTC #7 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Quoted 7 lines · Scott Wood

Quoted 4 lines
Quoted 4 lines
For your kind information : in start.S, label label "create_ccsr_new_tlb", "create_ccsr_old_tlb" uses MAS7 without CONFIG_ENABLE_36BIT_PHYS #define.
It should be fixed ??
Yes, it should be fixed. That was a fairly recent change and perhaps e500v1 has not been tested since then -- Timur, could you look at this?

On e500v1 parts, CONFIG_SYS_CCSRBAR_PHYS_HIGH will always be 0, so it will only write 0 to MAS7. Is that still considered a bad thing? The tlbwe will ignore MAS7.

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Scott Wood

2012-03-22 19:53 UTC #8 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Quoted 6 lines · Timur Tabi

Quoted 4 lines · Scott Wood
Quoted 4 lines
Quoted 4 lines
For your kind information : in start.S, label label "create_ccsr_new_tlb", "create_ccsr_old_tlb" uses MAS7 without CONFIG_ENABLE_36BIT_PHYS #define.
It should be fixed ??

Quoted 6 lines

Quoted 2 lines
Yes, it should be fixed. That was a fairly recent change and perhaps e500v1 has not been tested since then -- Timur, could you look at this?
On e500v1 parts, CONFIG_SYS_CCSRBAR_PHYS_HIGH will always be 0, so it will only write 0 to MAS7. Is that still considered a bad thing? The tlbwe will ignore MAS7.

Either it's needed, or we should get rid of CONFIG_ENABLE_36BIT_PHYS entirely. Either way, we should test the results on e500v1 hardware.

-Scott

Timur Tabi

2012-03-22 19:56 UTC #9 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Quoted 2 lines · Scott Wood

Either it's needed, or we should get rid of CONFIG_ENABLE_36BIT_PHYS entirely. Either way, we should test the results on e500v1 hardware.

That macro conditionally enables support for MAS7:

#if defined(CONFIG_ENABLE_36BIT_PHYS)
	ori	r0,r0,HID0_ENMAS7 at l	/* Enable MAS7 */

#endif

So I don't think we can get rid of it, otherwise I suspect an e500v1 operating system will not work on an e500v2 part.

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Scott Wood

2012-03-22 19:59 UTC #10 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Quoted 4 lines · Timur Tabi

Quoted 2 lines · Scott Wood
Either it's needed, or we should get rid of CONFIG_ENABLE_36BIT_PHYS entirely. Either way, we should test the results on e500v1 hardware.

Quoted 8 lines

That macro conditionally enables support for MAS7:
#if defined(CONFIG_ENABLE_36BIT_PHYS)
	ori	r0,r0,HID0_ENMAS7 at l	/* Enable MAS7 */
#endif
So I don't think we can get rid of it, otherwise I suspect an e500v1 operating system will not work on an e500v2 part.

Fine, keep it for that (though I'd argue that if that were the concern, we should have been clearing it before we enter the OS, and the Os can set it if it wants). But what about in write_tlb, or tlb.c?

There's obviously an existing attempt to avoid touching MAS7 if CONFIG_ENABLE_36BIT_PHYS is not set. Either that's unnecessary and we should remove it, or we should continue adhering to it.

-Scott

Prabhakar Kushwaha

author

2012-03-23 11:44 UTC #11 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

In reply to Scott Wood

Hi Scott,

Quoted 30 lines · Scott Wood

Quoted 29 lines · Prabhakar Kushwaha
Hi Scott,
Please find my reply in-lined
Quoted 24 lines · Scott Wood
Quoted 14 lines · Prabhakar Kushwaha
Debugging of e500 and e500v1 processer requires debug exception vecter (IVPR +
IVOR15) to have valid and fetchable OP code.
While executing in translated space (AS=1), whenever a debug exception is
generated, the MSR[DS/IS] gets cleared i.e. AS=0 and the processor tries to
fetch an instruction from the debug exception vector (IVPR + IVOR15); since now
we are in AS=0, the application needs to ensure the proper TLB configuration to
have (IVOR + IVOR15) accessible from AS=0 also.
Quoted 6 lines
Create a temporary TLB in AS0 to make sure debug exception verctor is accessible on debug exception.
Signed-off-by Radu Lazarescu<[email protected]>
Signed-off-by Marius Grigoras<[email protected]>
Signed-off-by Prabhakar Kushwaha<[email protected]>
Can you document the flow of exactly what TLB entries are present at various points of the boot flow, for all the various configurations (NOR boot, NAND SPL, RAMBOOT, IFC versus non-IFC, etc).

Quoted 6 lines

Quoted 1 line
Sure. May be separate patch will be send.
Let's start with just an e-mail thoroughly describing your understanding of this. It will provide necessary context for review.
We can clean it up for permanent documentation once it's clear to everyone what's going on.

Sure. I will start this activity from now.
But i will suggest not to combine both patches. let debugger patch to go ahead , if required i will send required patch later-on.

Quoted 9 lines

Quoted 4 lines
Quoted 1 line
In the ramboot case is this really supposed to be I+G?
I am not sure. But same is done under label "create_init_ram_area" for TLB entry 15.
what you suggest.
I suggest as part of the request to document all of this, you figure out what should actually be mapped in each configuration. The existing code might be wrong for some of them, but we shouldn't proceed ahead blindly and make an even bigger mess.

After internal discussion we can have following settings for non-RAMBOOT(NOR boot) ==> I + G
for RAMBOOT ==> I, cache inhibited is required as L1 cache is used as stack.

I=0 it means the memory range is cacheable, so an access to an address
from that range could get the line in cache. If you are using the cache as a memory zone(L1 as stack), it may overwrite some data in cache and replace it with the last access.

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
Which path will NAND SPL go through (not the payload, but the SPL itself)? That will be only a 4K window mapped, and guarded doesn't stop speculative instruction fetches, so we don't want to map more than is backed up by something.

Quoted 8 lines

Quoted 4 lines
NAND SPL go via !defined(CONFIG_SYS_RAMBOOT) path.
i think NAND_SPL does not require temporary TLB as NAND SPL even does not have any interrupt vector.
So there's no plan to support using breakpoints or single step during the SPL? That's fine with me, but should be documented, and we should make sure this code does not run in that case.

Breakpoints will work as it is. No impact will be on debugging for NAND SPL.
Generally any debugger use some initial configuration file. Only problem occurs
when application modifies those configuration.

Quoted 8 lines

Quoted 6 lines
Quoted 4 lines
arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 233
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/start.S +2 -0
+    lis     r10,0xffc00000 at h
+    ori     r10,r10,0xffc00000 at l
Don't waste instructions -- this could be in an SPL. That ori is a no-op.
Please refer above response. May be this piece of code is not required for NAND SPL
Still, I'd like to know why you're writing 0xffc00000 to MAS7. Only the low 4 bits of MAS7 are valid on current e500.

The reason for using 0xffc00000 to support e6500 - since it supports 40-bit physical addresses, the last 8 bits of MAS7 are defined. But i am not sure whether e6500 will be part of mpc85xx or not.

So, I will use as

#ifdef CONFIG_ENABLE_36BIT_PHYS
     lis     r10,0x0000

#endif

Quoted 9 lines

Quoted 9 lines
Quoted 9 lines
arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 239
Quoted 5 lines
arch/powerpc/cpu/mpc85xx/start.S +5 -0
+    mtspr   MAS0,r6
+    mtspr   MAS1,r7
+    mtspr   MAS2,r8
+    mtspr   MAS3,r9
+    mtspr   MAS7,r10
Why are you writing 0xffc00000 into MAS7?
Access to MAS7 needs to be conditional on CONFIG_ENABLE_36BIT_PHYS (misnamed, should be something like CONFIG_SYS_PPC_HAS_MAS7).

Quoted 8 lines

Quoted 6 lines
i will put this code under #define CONFIG_ENABLE_36BIT_PHYS
For your kind information : in start.S, label label "create_ccsr_new_tlb", "create_ccsr_old_tlb" uses MAS7 without CONFIG_ENABLE_36BIT_PHYS #define.
It should be fixed ??
Yes, it should be fixed. That was a fairly recent change and perhaps e500v1 has not been tested since then -- Timur, could you look at this?

i will use CONFIG_ENABLE_36BIT_PHYS defines.

Quoted 23 lines

Quoted 13 lines

arch/powerpc/include/asm/config_mpc85xx.h +1 -1

index 8654625..268c56e 100644
@@ -1,5 +1,5 @@
   /*
- * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011-2012 Freescale Semiconductor, Inc.
    *
    * This program is free software; you can redistribute it and/or
    * modify it under the terms of the GNU General Public License as

arch/powerpc/include/asm/config_mpc85xx.h line 113

Quoted 7 lines

arch/powerpc/include/asm/config_mpc85xx.h +1 -0

@@ -107,6 +107,7 @@
   #define CONFIG_MAX_CPUS            1
   #define CONFIG_FSL_SDHC_V2_3
   #define CONFIG_SYS_FSL_NUM_LAWS        12
+#define CONFIG_SYS_PPC_E500_DEBUG_TLB    3
   #define CONFIG_TSECV2
   #define CONFIG_SYS_FSL_SEC_COMPAT    4
   #define CONFIG_FSL_SATA_V2

It would be nice if we could have one place where all the fixed TLB entries are assigned.

Quoted 5 lines

Quoted 2 lines
Am i supposed to send separate patch having TLB entry defined for all e500 v1/v2 SoC ?
That's a different issue, I just mean having one central place where each hardcoded TLB entry gets a symbolic name, as well as a #define for where programmatic TLB entry allocation can begin.

Yes. I agree with you. May be i can take this activity also.

--Prabhakar

Scott Wood

2012-03-23 18:14 UTC #12 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Quoted 38 lines · Prabhakar Kushwaha

Hi Scott,
Quoted 35 lines · Scott Wood
Quoted 23 lines · Prabhakar Kushwaha
Hi Scott,
Please find my reply in-lined
Quoted 18 lines · Scott Wood
Quoted 17 lines · Prabhakar Kushwaha
Debugging of e500 and e500v1 processer requires debug exception vecter (IVPR +
IVOR15) to have valid and fetchable OP code.
While executing in translated space (AS=1), whenever a debug exception is
generated, the MSR[DS/IS] gets cleared i.e. AS=0 and the processor tries to
fetch an instruction from the debug exception vector (IVPR + IVOR15); since now
we are in AS=0, the application needs to ensure the proper TLB configuration to
have (IVOR + IVOR15) accessible from AS=0 also.
Create a temporary TLB in AS0 to make sure debug exception verctor is accessible on debug exception.
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
Signed-off-by Radu Lazarescu<[email protected]>
Signed-off-by Marius Grigoras<[email protected]>
Signed-off-by Prabhakar Kushwaha<[email protected]>
Can you document the flow of exactly what TLB entries are present at various points of the boot flow, for all the various configurations (NOR
boot, NAND SPL, RAMBOOT, IFC versus non-IFC, etc).
Sure. May be separate patch will be send.
Let's start with just an e-mail thoroughly describing your understanding of this. It will provide necessary context for review.

Quoted 6 lines

Quoted 2 lines
We can clean it up for permanent documentation once it's clear to everyone what's going on.
Sure. I will start this activity from now.
But i will suggest not to combine both patches. let debugger patch to go ahead , if required i will send required patch later-on.

My point is that I cannot fully follow what's going on here without spending a bunch of time looking at it, and I don't see anyone else stepping up to review this, so I'd like to see a write-up of what's going on so that I can properly review these patches.

Quoted 10 lines

Quoted 9 lines
Quoted 4 lines
Quoted 1 line
In the ramboot case is this really supposed to be I+G?
I am not sure. But same is done under label "create_init_ram_area" for TLB entry 15.
what you suggest.
I suggest as part of the request to document all of this, you figure out what should actually be mapped in each configuration. The existing code might be wrong for some of them, but we shouldn't proceed ahead blindly and make an even bigger mess.

Quoted 4 lines

After internal discussion we can have following settings for non-RAMBOOT(NOR boot) ==> I + G
for RAMBOOT ==> I, cache inhibited is required as L1 cache is used as stack.

Why does using L1 for a stack mean that the mapping must be cache inhibited? Why do we even need to use L1 for a stack with ramboot?

Quoted 4 lines

I=0 it means the memory range is cacheable, so an access to an address
from that range could get the line in cache. If you are using the cache as a memory zone(L1 as stack), it may overwrite some data in cache and replace it with the last access.

It will not do that -- when we use L1 (or part of it) for an early stack, we lock the cache lines.

Quoted 14 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Which path will NAND SPL go through (not the payload, but the SPL itself)? That will be only a 4K window mapped, and guarded doesn't stop
speculative instruction fetches, so we don't want to map more than is backed up by something.
Quoted 8 lines
Quoted 4 lines
NAND SPL go via !defined(CONFIG_SYS_RAMBOOT) path.
i think NAND_SPL does not require temporary TLB as NAND SPL even does not have any interrupt vector.
So there's no plan to support using breakpoints or single step during the SPL? That's fine with me, but should be documented, and we should make sure this code does not run in that case.

Quoted 6 lines

Breakpoints will work as it is. No impact will be on debugging for NAND SPL.
Generally any debugger use some initial configuration file. Only problem occurs
when application modifies those configuration.

Then why do we need to set MSR[DE] on entry, if the debugger can take care of it?

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 233
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/start.S +2 -0
+    lis     r10,0xffc00000 at h
+    ori     r10,r10,0xffc00000 at l
Don't waste instructions -- this could be in an SPL. That ori is a no-op.

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
Please refer above response. May be this piece of code is not required for NAND SPL
Still, I'd like to know why you're writing 0xffc00000 to MAS7. Only the low 4 bits of MAS7 are valid on current e500.
The reason for using 0xffc00000 to support e6500 - since it supports 40-bit physical addresses, the last 8 bits of MAS7 are defined.

Regardless, you're setting the wrong end of MAS7. It's the *lower* bits, not the upper bits, that are used.

And we should not be doing anything special for e6500 here. e6500 does not need this, and e6500 platforms should not set CONFIG_SYS_PPC_E500_DEBUG_TLB.

Quoted 1 line

But i am not sure whether e6500 will be part of mpc85xx or not.

It will.

Quoted 4 lines

So, I will use as

#ifdef CONFIG_ENABLE_36BIT_PHYS
    lis     r10,0x0000

#endif

Why?

-Scott

Prabhakar Kushwaha

author

2012-03-24 02:24 UTC #13 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Quoted 41 lines · Scott Wood

Quoted 40 lines · Prabhakar Kushwaha
Hi Scott,
Quoted 37 lines · Scott Wood
Quoted 23 lines · Prabhakar Kushwaha
Hi Scott,
Please find my reply in-lined
Quoted 18 lines · Scott Wood
Quoted 17 lines · Prabhakar Kushwaha
Debugging of e500 and e500v1 processer requires debug exception vecter (IVPR +
IVOR15) to have valid and fetchable OP code.
While executing in translated space (AS=1), whenever a debug exception is
generated, the MSR[DS/IS] gets cleared i.e. AS=0 and the processor tries to
fetch an instruction from the debug exception vector (IVPR + IVOR15); since now
we are in AS=0, the application needs to ensure the proper TLB configuration to
have (IVOR + IVOR15) accessible from AS=0 also.
Create a temporary TLB in AS0 to make sure debug exception verctor is accessible on debug exception.
Quoted 8 lines
Quoted 7 lines
Quoted 3 lines
Signed-off-by Radu Lazarescu<[email protected]>
Signed-off-by Marius Grigoras<[email protected]>
Signed-off-by Prabhakar Kushwaha<[email protected]>
Can you document the flow of exactly what TLB entries are present at various points of the boot flow, for all the various configurations (NOR
boot, NAND SPL, RAMBOOT, IFC versus non-IFC, etc).
Sure. May be separate patch will be send.
Let's start with just an e-mail thoroughly describing your understanding of this. It will provide necessary context for review.
We can clean it up for permanent documentation once it's clear to everyone what's going on.

Quoted 7 lines

Quoted 3 lines
Sure. I will start this activity from now.
But i will suggest not to combine both patches. let debugger patch to go ahead , if required i will send required patch later-on.
My point is that I cannot fully follow what's going on here without spending a bunch of time looking at it, and I don't see anyone else stepping up to review this, so I'd like to see a write-up of what's going on so that I can properly review these patches.

Means i have to make the doc first :(
OK. Its my pleasure.

Quoted 9 lines

Quoted 9 lines
Quoted 9 lines
Quoted 4 lines
Quoted 1 line
In the ramboot case is this really supposed to be I+G?
I am not sure. But same is done under label "create_init_ram_area" for TLB entry 15.
what you suggest.
I suggest as part of the request to document all of this, you figure out what should actually be mapped in each configuration. The existing code might be wrong for some of them, but we shouldn't proceed ahead blindly and make an even bigger mess.

Quoted 6 lines

Quoted 4 lines
After internal discussion we can have following settings for non-RAMBOOT(NOR boot) ==> I + G
for RAMBOOT ==> I, cache inhibited is required as L1 cache is used as stack.
Why does using L1 for a stack mean that the mapping must be cache inhibited? Why do we even need to use L1 for a stack with ramboot?

it is done at label "_start_cont". L1 is set for Stack address as "switch_as" label.
am i wrong??

Quoted 6 lines

Quoted 4 lines
I=0 it means the memory range is cacheable, so an access to an address
from that range could get the line in cache. If you are using the cache as a memory zone(L1 as stack), it may overwrite some data in cache and replace it with the last access.
It will not do that -- when we use L1 (or part of it) for an early stack, we lock the cache lines.

Agree..

Quoted 13 lines

Quoted 13 lines
Quoted 13 lines
Quoted 9 lines
Quoted 5 lines
Which path will NAND SPL go through (not the payload, but the SPL itself)? That will be only a 4K window mapped, and guarded doesn't stop
speculative instruction fetches, so we don't want to map more than is backed up by something.
NAND SPL go via !defined(CONFIG_SYS_RAMBOOT) path.
i think NAND_SPL does not require temporary TLB as NAND SPL even does not have any interrupt vector.
So there's no plan to support using breakpoints or single step during the SPL? That's fine with me, but should be documented, and we should make sure this code does not run in that case.

Quoted 8 lines

Quoted 6 lines
Breakpoints will work as it is. No impact will be on debugging for NAND SPL.
Generally any debugger use some initial configuration file. Only problem occurs
when application modifies those configuration.
Then why do we need to set MSR[DE] on entry, if the debugger can take care of it?

Yes. You are right. It is required only for SD/SPI boot not for NAND_SPL/NOR boot.
But to make MSR[DE] bit set general way (out of #define) throughout u-boot code. I did.

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 4 lines
arch/powerpc/cpu/mpc85xx/start.S l2_disabled: · line 233
Quoted 2 lines
arch/powerpc/cpu/mpc85xx/start.S +2 -0
+    lis     r10,0xffc00000 at h
+    ori     r10,r10,0xffc00000 at l
Don't waste instructions -- this could be in an SPL. That ori is a no-op.
Please refer above response. May be this piece of code is not required for NAND SPL

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
Still, I'd like to know why you're writing 0xffc00000 to MAS7. Only the low 4 bits of MAS7 are valid on current e500.
The reason for using 0xffc00000 to support e6500 - since it supports 40-bit physical addresses, the last 8 bits of MAS7 are defined.
Regardless, you're setting the wrong end of MAS7. It's the *lower* bits, not the upper bits, that are used.

oh.. got your point.

Quoted 3 lines

And we should not be doing anything special for e6500 here. e6500 does not need this, and e6500 platforms should not set CONFIG_SYS_PPC_E500_DEBUG_TLB.

Got it :)

Quoted 2 lines

Quoted 1 line
But i am not sure whether e6500 will be part of mpc85xx or not.
It will.

Thanks for confirmation.

Quoted 4 lines

Quoted 4 lines
So, I will use as
#ifdef CONFIG_ENABLE_36BIT_PHYS
     lis     r10,0x0000
#endif

for 36 bit physical address,
address should be 0x0_1107_f000 or 0x0_ffff_fffc for destination

mas7 should be programmed. To make sure mas7 is 0. I am setting it.
if not required i can remove.

--Prabhakar

Scott Wood

2012-03-26 18:14 UTC #14 lore raw

PATCH v2 3/4 powerpc/85xx:Make debug exception vector accessible

Quoted 6 lines · Prabhakar Kushwaha

Quoted 5 lines · Scott Wood
Quoted 4 lines · Prabhakar Kushwaha
After internal discussion we can have following settings for non-RAMBOOT(NOR boot) ==> I + G
for RAMBOOT ==> I, cache inhibited is required as L1 cache is used as stack.

Quoted 6 lines

Quoted 2 lines
Why does using L1 for a stack mean that the mapping must be cache inhibited? Why do we even need to use L1 for a stack with ramboot?
it is done at label "_start_cont". L1 is set for Stack address as "switch_as" label.
am i wrong??

I don't doubt that we do use the L1 for stack unconditionally -- just that in the ramboot case we could probably get away without it if it were a problem. I don't think it's a problem, though, because it doesn't conflict with creating cacheable mappings.

Quoted 9 lines

Quoted 5 lines
Quoted 3 lines
Generally any debugger use some initial configuration file. Only problem occurs
when application modifies those configuration.
Then why do we need to set MSR[DE] on entry, if the debugger can take care of it?
Yes. You are right. It is required only for SD/SPI boot not for NAND_SPL/NOR boot.
But to make MSR[DE] bit set general way (out of #define) throughout u-boot code. I did.

I'm not saying it should be conditional -- just trying to understand when it's needed at all, and what we can expect the debugger to have already done.

Quoted 9 lines

Quoted 4 lines
Quoted 4 lines
So, I will use as
#ifdef CONFIG_ENABLE_36BIT_PHYS
     lis     r10,0x0000
#endif
for 36 bit physical address,
address should be 0x0_1107_f000 or 0x0_ffff_fffc for destination
mas7 should be programmed. To make sure mas7 is 0. I am setting it.
if not required i can remove.

CONFIG_ENABLE_36BIT_PHYS should be protecting the MAS7 mtspr, not the lis.

-Scott