[U-Boot] [PATCH 1/7] armv8:New MMU setup code allowing to set up 2-level page tables

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hello Sergey,

On Thu, 16 Apr 2015 16:15:05 -0700, Sergey Temerkhanov
<s.temerkhanov@gmail.com> wrote:
> This patch adds code which sets up 2-level page tables on ARM64 thus
> extending available VA space. CPUs implementing 64k translation
> granule are able to use direct PA-VA mapping of the whole 48 bit
> address space.
> It also adds the ability to reset the SCTRL register at the very beginning
> of execution to avoid interference from stale mappings set up by early
> firmware/loaders/etc.
> 
> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
> Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> ---
>  arch/arm/cpu/armv8/cache_v8.c      | 95 ++++++++++++++++++++++++++++++++++++++
>  arch/arm/cpu/armv8/start.S         | 36 +++++++++++++++
>  arch/arm/include/asm/armv8/mmu.h   | 79 ++++++++++++++++++++++++++++---
>  arch/arm/include/asm/global_data.h |  1 +
>  arch/arm/include/asm/system.h      |  6 +++
>  arch/arm/lib/board.c               |  6 ++-
>  6 files changed, 215 insertions(+), 8 deletions(-)

This is actually a v2 if the 3-patch series which you posted prevously,
right? If so, then next time please tag the series accordingly ("v2",
"V3" etc).

> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
> index c5ec529..1264caa 100644
> --- a/arch/arm/cpu/armv8/cache_v8.c
> +++ b/arch/arm/cpu/armv8/cache_v8.c
> @@ -12,6 +12,8 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  #ifndef CONFIG_SYS_DCACHE_OFF
> +
> +#ifndef CONFIG_SYS_FULL_VA

CONFIG_SYS_FULL_VA does not exist yet, so it is introduced in this patch.
Therefore it should be described somewhere, e.g. in doc/README.arm64,
so that readers of the code can refer to the description if they need
to.
 
> +	/* Setup the PMD pointers */
> +	for (l1_e = 0; l1_e < CONFIG_SYS_MEM_MAP_SIZE; l1_e++) {

> +/* Setup the page tables */
> +	for (l1_e = 0; l1_e < PTL1_ENTRIES; l1_e++) {

Real nitpicking here, but why is the comment not aligned@the same
column as the code after it?
>  .globl	_start
>  _start:
> +#ifdef CONFIG_SYS_RESET_SCTRL
> +	bl	reset_sctrl
> +#endif
>  	b	reset

Er, no. The _start symbol branches to the reset label. If you really
need to call something early after start of execution, you do it a bit
later, but not here.

Amicalement,
-- 
Albert.