From mboxrd@z Thu Jan 1 00:00:00 1970 From: Prabhakar Kushwaha Date: Sat, 24 Mar 2012 07:54:22 +0530 Subject: [U-Boot] [PATCH 3/4][v2] powerpc/85xx:Make debug exception vector accessible In-Reply-To: <4F6CBD9E.1000603@freescale.com> References: <1332304982-10835-1-git-send-email-prabhakar@freescale.com> <4F6A3170.30907@freescale.com> <4F6ABE2E.5000502@freescale.com> <4F6B80F5.6070700@freescale.com> <4F6C621B.3020600@freescale.com> <4F6CBD9E.1000603@freescale.com> Message-ID: <4F6D3056.7040201@freescale.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Friday 23 March 2012 11:44 PM, Scott Wood wrote: > On 03/23/2012 06:44 AM, Prabhakar Kushwaha wrote: >> Hi Scott, >> >> On Friday 23 March 2012 01:13 AM, Scott Wood wrote: >>> On 03/22/2012 12:52 AM, Prabhakar Kushwaha wrote: >>>> 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 >>>>>> Signed-off-by: Marius Grigoras >>>>>> Signed-off-by: Prabhakar Kushwaha >>>>> 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. >> 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. >>>>> 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. > 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=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.. >>>>> 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. >>> >> 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. >>>>>> + 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. > 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. > 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 :) >> But i am not sure whether e6500 will be part of mpc85xx or not. > It will. Thanks for confirmation. >> 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