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

[Bhupesh Sharma]

> -----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/nonsec_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/nonsec_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_virt.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.

Regards,
Bhupesh