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

[Scott Wood <scottwood@freescale.com>]

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