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

Prabhakar Kushwaha
2012-03-22T05:52:46Z

Hi Scott,

Please find my reply in-lined

On Thursday 22 March 2012 01:22 AM, Scott Wood wrote:
> 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).

Sure. May be separate patch will be send.

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

Sure, i will update.

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

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

> 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

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

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

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

Ok.

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

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

--Prabhakar