[U-Boot] [PATCH v15 07/10] arm64: core support

[bhupesh.sharma at freescale.com]

Hi David,

> -----Original Message-----
> From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de]
> On Behalf Of Bhupesh Sharma
> Sent: Tuesday, December 03, 2013 3:33 PM
> To: 'FengHua'
> Cc: 'trini at ti.com'; 'u-boot at lists.denx.de'; Wood Scott-B07421
> Subject: Re: [U-Boot] [PATCH v15 07/10] arm64: core support
> 
> > -----Original Message-----
> > From: FengHua [mailto:fenghua at phytium.com.cn]
> > Sent: Sunday, December 01, 2013 7:48 PM
> > To: Sharma Bhupesh-B45370
> > Cc: 'Bhupesh SHARMA'; 'u-boot at lists.denx.de'; 'trini at ti.com'; Wood
> > Scott-
> > B07421
> > Subject: Re: RE: Re: [U-Boot] [PATCH v15 07/10] arm64: core support
> >
> >
> > > > -----Original Message-----
> > > > From: FengHua [mailto:fenghua at phytium.com.cn]
> > > > Sent: Friday, November 29, 2013 7:05 PM
> > > > To: Bhupesh SHARMA
> > > > Cc: u-boot at lists.denx.de; Sharma Bhupesh-B45370; trini at ti.com;
> > > > Wood
> > > > Scott-B07421
> > > > Subject: Re: Re: [U-Boot] [PATCH v15 07/10] arm64: core support
> > > >
> > > >
> > > > hi Bhupesh,
> > > >     Thank you for reviewing of the patch.
> > > >
> > > > > > +/*
> > > > > > + * Generic timer implementation of timer_read_counter()  */
> > > > > > +unsigned long timer_read_counter(void) {
> > > > > > +	unsigned long cntpct;
> > > > > > +	isb();
> > > > > > +	asm volatile("mrs %0, cntpct_el0" : "=r" (cntpct));
> > > > > > +	return cntpct;
> > > > > > +}
> > > > > > diff --git a/arch/arm/cpu/armv8/gic.S
> > > > > > b/arch/arm/cpu/armv8/gic.S
> > > > >
> > > > > The ARMv8 foundation model has support for GICv2 while GICv3 is
> > > > > actually compatible to ARMv8. So although you mention in the
> > > > > cover letter that this is currently GICv2 support, now while
> > > > > trying to add
> > > > > GICv3 support it will be difficult to envision GICv2 code in
> > > > 'arch/arm/cpu/armv8/'
> > > > > directory.
> > > > >
> > > > > Infact GICv2 is compatible with ARMv7 and as secure and
> > > > > non-secure copies of GIC registers are equally applicable to
> > > > > ARMv7, would it make sense to keep the GICv2 code at a place
> > > > > where both ARMv7 and
> > > > > ARMv8 can use it?
> > > > >
> > > > > Can we reuse something from [1] for GICv2:
> > > > >
> > > > > [1]
> > > > > http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/non
> > > > > se
> > > > > c_vi
> > > > > rt.S;h=24b4c18bd452fa155bcd5ed94c755aa05a33efe7;hb=HEAD#l88
> > > > >
> > > >
> > > > Gicv2 only support maximum 8 cores, but still could be used with
> > > > armv8 processors if the processor contains less than 8 cores.
> > > > AMCC's
> > > > armv8 processor use Gicv2.
> > > > Yes, as you said it would be better to abstract a few common
> > > > routines of
> > > > Gicv2 and Gicv3 code and place them at a common place (such as
> > > > arm/lib) so that both ARMv7 and ARMv8 could use it.
> > > >
> > > > > > +	/* Cache/BPB/TLB Invalidate */
> > > > > > +	bl	__asm_flush_dcache_all		/* dCache
> > clean&invalidate */
> > > > > > +	bl	__asm_invalidate_icache_all	/* iCache invalidate */
> > > > > > +	bl	__asm_invalidate_tlb_all	/* invalidate TLBs */
> > > > > > +
> > > > > > +	/* Processor specific initialization */
> > > > > > +	bl	lowlevel_init
> > > > >
> > > > > Shouldn't this call be protected inside a '#ifndef
> > > > > CONFIG_SKIP_LOWLEVEL_INIT'?
> > > > >
> > > > We could do so when it is actually needed.
> > > >
> > > > > > +WEAK(lowlevel_init)
> > > > >
> > > > > Ok, so this means that a specific SoC lowlevel_init
> > > > > implementation can override this generic implementation. Because
> > > > > I sure other secure/non-secure settings need to be put into
> > > > > place for ARM IPs like SMMU-500.
> > > > >
> > > >
> > > > > > +ENTRY(armv8_switch_to_el2)
> > > > >
> > > > > Do we need a switch to Secure Monitor here? I am not able to
> > > > > relate how this with the present ARMv7 code (see [2]):
> > > > >
> > > > > [2]
> > > > > http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/non
> > > > > se
> > > > > c_vi
> > > > > rt.S;h=24b4c18bd452fa155bcd5ed94c755aa05a33efe7;hb=HEAD#l29
> > > > >
> > > > Armv8 processor reset at el3(if it support security extension). So
> > > > we need to switch the processor to el2 or el1 before u-boot jump
> > > > to linux kernel due to linux-aarch64 only run at el2 or el1.
> > > >
> > >
> > > Hi David,
> > >
> > > As per ARMv8 Arch Reference Manual (ARM), Monitor mode is provided
> > > to support switching between Secure and Non-secure states. For
> > > switching from secure to non secure state the usual mechanism is an
> > > exception return. To return to Non-secure state (EL1/EL2), software
> > > executing in
> > Monitor mode(EL3) sets SCR.NS to 1 and then performs the exception
> > return.
> > >
> > > The implementation already in place for ARMv7 (see:
> > > http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/nonsec_
> > > vi rt.S;h=24b4c18bd452fa155bcd5ed94c755aa05a33efe7;hb=HEAD#l29),
> > > does this (sets up SMC handler and start.S calls smc #0 to switch to
> > non-secure state).
> > >
> > > Something similar must be put into place for ARMv8 as well.
> > >
> > Sorry，I did not get your initially.
> > In my opinion, u-boot runs at monitor mode(EL3), it coulde set SCR.NS
> > to
> > 1 directly.
> > Why we need to setup SMC handler and call SMC #0 to switch to
> > non-secure state?
> > I don't understand why armv7 did so. Did you have any understanding
> > about this?
> 
> Well, all ARM cores which support security extensions must support a SMC
> exception handler (ARMv7/v8). The monitor mode (which is entered by
> executing a SMC
> instruction) allows ARM core to switch from secure to non-secure state
> and vice-versa.
> 
> Note that for supporting ARM TrustZone, ARM specifies a SMC calling
> convention
> spec:
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0028a/inde
> x.html
> 

In reference to my mail above, I see that the transition to EL2 (from EL3) which occurs very early
in start.S needs to be changed on lines of the ARMv7 code, i.e. the EL2 transition should happen just
before Linux is booted up by the u-boot.

The reason for the same is that a no of ARM IPs like GIC, SMMU and TZPC/TZASC need to be configured to
allow non-secure accesses from Linux world (which runs in EL1 mode). Adding the assembly code for all
such IPs in 'setup_el3' function in start.S, will bloat the start.S and also increase the chances of a
bug in the assembly code.

Hence, I would like to propose a strategy to shift from EL3 to EL2 to some point in u-boot code after the
C Run Time has been initialized (similar to present ARMv7 u-boot code). 

If you are ok with the same, I can try to send out some RFC patches rebased against your latest v16 code-base.

Please let me know.
Regards,
Bhupesh