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

[Bhupesh Sharma]

> -----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/nonse
> > > > 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/nonse
> > > > 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/index.html

Regards,
Bhupesh