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

[FengHua]

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

Regards,
David