[U-Boot] [PATCH 3/4][v2] powerpc/85xx:Make debug exception vector accessible

Scott Wood
2012-03-21T19:52:16Z

On 03/20/2012 11:43 PM, Prabhakar Kushwaha wrote:
> 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 <radu.lazarescu@freescale.com>
> Signed-off-by: Marius Grigoras <marius.grigoras@freescale.com>
> Signed-off-by: Prabhakar Kushwaha <prabhakar@freescale.com>

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).

> diff --git a/arch/powerpc/cpu/mpc85xx/start.S b/arch/powerpc/cpu/mpc85xx/start.S
> index 597151b..cef00ba 100644
> --- a/arch/powerpc/cpu/mpc85xx/start.S
> +++ b/arch/powerpc/cpu/mpc85xx/start.S
> @@ -182,6 +182,66 @@ l2_disabled:
> andi. r1,r3,L1CSR0_DCE at l
> 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

> +
> + 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
> + 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.

> + 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.

> +
> + 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).

> + isync
> + msync
> + tlbwe
> +#endif

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

> diff --git a/arch/powerpc/include/asm/config_mpc85xx.h b/arch/powerpc/include/asm/config_mpc85xx.h
> index 8654625..268c56e 100644
> --- a/arch/powerpc/include/asm/config_mpc85xx.h
> +++ b/arch/powerpc/include/asm/config_mpc85xx.h
> @@ -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

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

-Scott