[U-Boot] [PATCH 2/8] xtensa: add support for the xtensa processor architecture [2/2]
Simon Glass
sjg at chromium.org
Fri Jul 15 02:20:52 CEST 2016
Hi Max,
On 14 July 2016 at 16:58, Max Filippov <jcmvbkbc at gmail.com> wrote:
> Hi Simon,
>
> On Tue, Jul 12, 2016 at 03:56:05PM -0600, Simon Glass wrote:
>
> [...]
>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>
>> Some comments below.
>>
>
>> > diff --git a/arch/xtensa/cpu/start.S b/arch/xtensa/cpu/start.S
>> > new file mode 100644
>> > index 0000000..ac32efb
>> > --- /dev/null
>> > +++ b/arch/xtensa/cpu/start.S
>
> [...]
>
>> > +3: /* All code and initalized data segments have been copied. */
>> > +
>> > + /* Setup PS, PS.WOE = 1, PS.EXCM = 0, PS.INTLEVEL = EXCM level. */
>> > +
>> > +#if __XTENSA_CALL0_ABI__
>> > + movi a2, XCHAL_EXCM_LEVEL
>> > +#else
>> > + movi a2, (1<<PS_WOE_BIT) | XCHAL_EXCM_LEVEL
>> > +#endif
>> > + wsr a2, PS
>> > + rsync
>> > +
>> > + /* Clear BSS */
>> > +
>> > + movi a2, _bss_start
>> > + movi a3, _bss_end
>>
>> Can you please use board_init_f_init_reserve(), etc.? You can copy how
>> ARM does things perhaps. This should not be in assembly.
>
> We use them, one screen below.
> What exactly should not be in assembly? BSS zeroing?
Yes.
I misunderstood this sorry, but the idea is to have a little as
possible in assembler. Perhaps you are already doing that, but if you
can avoid clearing BSS here that would be better.
>
>> > +
>> > + __loopt a2, a3, a4, 2
>> > + s32i a0, a2, 0
>> > + __endla a2, a3, 4
>> > +
>> > + /* Writeback */
>> > +
>> > + ___flush_dcache_all a2, a3
>> > +
>> > +#ifdef __XTENSA_WINDOWED_ABI__
>> > + /*
>> > + * In windowed ABI caller and call target need to be within the same
>> > + * gigabyte. Put the rest of the code into the text segment and jump
>> > + * there.
>> > + */
>> > +
>> > + movi a4, .Lboard_init_code
>> > + jx a4
>> > +
>> > + .text
>> > + .align 4
>> > +.Lboard_init_code:
>> > +#endif
>> > +
>> > + movi a0, 0
>> > + movi sp, (CONFIG_SYS_TEXT_ADDR - 16) & 0xfffffff0
>> > +
>> > +#ifdef CONFIG_DEBUG_UART
>> > + movi a4, debug_uart_init
>> > +#ifdef __XTENSA_CALL0_ABI__
>> > + callx0 a4
>> > +#else
>> > + callx4 a4
>> > +#endif
>> > +#endif
>> > +
>> > + movi a4, board_init_f_alloc_reserve
>> > +
>> > +#ifdef __XTENSA_CALL0_ABI__
>> > + mov a2, sp
>> > + callx0 a4
>> > + mov sp, a2
>> > +#else
>> > + mov a6, sp
>> > + callx4 a4
>> > + movsp sp, a6
>> > +#endif
>> > +
>> > + movi a4, board_init_f_init_reserve
>> > +
>> > +#ifdef __XTENSA_CALL0_ABI__
>> > + callx0 a4
>> > +#else
>> > + callx4 a4
>> > +#endif
>> > +
>> > + /*
>> > + * Call board initialization routine (never returns).
>> > + */
>> > +
>> > + movi a4, board_init_f
>>
>> As above. This is using the old approach.
>
> I see the same sequence in the ARM's _main in arch/arm/lib/crt0.S:
>
> mov r0, sp
> bl board_init_f_alloc_reserve
> mov sp, r0
> /* set up gd here, outside any C code */
> mov r9, r0
> bl board_init_f_init_reserve
>
> mov r0, #0
> bl board_init_f
>
> What is 'the new approach' and how does it differ from the old?
The old approach was to have it all in assembler.
>
> [...]
>
>> > + /* Does not return here. */
>>
>> nit: drop periods before */
>
> Ok.
>
> [...]
>
>> > +fast_alloca_exception: /* must be at _WindowUnderflow4 + 16
>>
>> */ ?
>
> Oops, right.
>
> [...]
>
>> > diff --git a/arch/xtensa/cpu/u-boot.lds b/arch/xtensa/cpu/u-boot.lds
>> > new file mode 100644
>> > index 0000000..853ae5a
>> > --- /dev/null
>> > +++ b/arch/xtensa/cpu/u-boot.lds
>> > @@ -0,0 +1,116 @@
>> > +/*
>> > + * (C) Copyright 2008 - 2013 Tensilica, Inc.
>> > + * (C) Copyright 2014 - 2016 Cadence Design Systems Inc.
>> > + *
>> > + * SPDX-License-Identifier: GPL-2.0+
>> > + */
>> > +
>> > +#include <config.h>
>> > +#include <asm/ldscript.h>
>> > +#include <asm/arch/core.h>
>> > +#include <asm/addrspace.h>
>> > +#include <asm-offsets.h>
>> > +
>> > +OUTPUT_ARCH(xtensa)
>> > +ENTRY(_start)
>> > +
>> > +/*
>> > + * U-Boot resets from SYSROM and unpacks itself from a ROM store to RAM.
>> > + * The reset vector is usually near the base of SYSROM and has room
>> > + * above it for the ROM store into which the rest of U-Boot is packed.
>> > + * The ROM store also needs to be above any other vectors that are in ROM.
>> > + * If a core has its vectors near the top of ROM, this must be edited.
>> > + *
>> > + * Note that to run C code out of ROM, the processor would have to support
>> > + * 'relocatable' exception vectors and provide a scratch memory for the
>> > + * initial stack. Not all Xtensa processor configurations support that, so
>> > + * we can simplify the boot process and unpack U-Boot to RAM immediately.
>> > + * This, however, requires that memory have been initialized throug some
>> > + * other means (serial ROM, for example) or are initialized early (requiring
>> > + * an assembler function. See start.S for more details)
>> > + */
>> > +
>> > +SECTIONS
>> > +{
>> > + . = + SIZEOF_HEADERS;
>> > + SECTION_ResetVector(XCHAL_RESET_VECTOR_VADDR, LMA_EQ_VMA)
>> > +
>> > + .reloc_table ALIGN(4) : FOLLOWING(.ResetVector.text)
>> > + {
>> > + __reloc_table_start = ABSOLUTE(.);
>> > +#if XCHAL_HAVE_WINDOWED
>> > + RELOCATE2(WindowVectors,text);
>> > +#endif
>> > + RELOCATE2(KernelExceptionVector,literal);
>> > + RELOCATE2(KernelExceptionVector,text);
>> > + RELOCATE2(UserExceptionVector,literal);
>> > + RELOCATE2(UserExceptionVector,text);
>> > + RELOCATE2(DoubleExceptionVector,literal);
>> > + RELOCATE2(DoubleExceptionVector,text);
>> > + RELOCATE1(text);
>> > + RELOCATE1(rodata);
>> > + RELOCATE1(data);
>> > + RELOCATE1(u_boot_list);
>> > + __reloc_table_end = ABSOLUTE(.);
>> > + }
>> > +
>> > +#if XCHAL_HAVE_WINDOWED
>> > + SECTION_VECTOR(WindowVectors,text,XCHAL_WINDOW_VECTORS_VADDR,
>> > + FOLLOWING(.reloc_table))
>> > + SECTION_VECTOR(KernelExceptionVector,literal,XCHAL_KERNEL_VECTOR_VADDR-8,
>> > + FOLLOWING(.WindowVectors.text))
>> > +#else
>> > + SECTION_VECTOR(KernelExceptionVector,literal,XCHAL_KERNEL_VECTOR_VADDR-8,
>> > + FOLLOWING(.reloc_table))
>> > +#endif
>> > + SECTION_VECTOR(KernelExceptionVector,text,XCHAL_KERNEL_VECTOR_VADDR,
>> > + FOLLOWING(.KernelExceptionVector.literal))
>> > + SECTION_VECTOR(UserExceptionVector,literal,XCHAL_USER_VECTOR_VADDR-8,
>> > + FOLLOWING(.KernelExceptionVector.text))
>> > + SECTION_VECTOR(UserExceptionVector,text,XCHAL_USER_VECTOR_VADDR,
>> > + FOLLOWING(.UserExceptionVector.literal))
>> > + SECTION_VECTOR(DoubleExceptionVector,literal,XCHAL_DOUBLEEXC_VECTOR_VADDR-8,
>> > + FOLLOWING(.UserExceptionVector.text))
>> > + SECTION_VECTOR(DoubleExceptionVector,text,XCHAL_DOUBLEEXC_VECTOR_VADDR,
>> > + FOLLOWING(.DoubleExceptionVector.literal))
>> > +
>> > + __monitor_start = CONFIG_SYS_TEXT_ADDR;
>> > +
>> > + SECTION_text(CONFIG_SYS_TEXT_ADDR, FOLLOWING(.DoubleExceptionVector.text))
>> > + SECTION_rodata(ALIGN(16), FOLLOWING(.text))
>> > + SECTION_u_boot_list(ALIGN(16), FOLLOWING(.rodata))
>> > + SECTION_data(ALIGN(16), FOLLOWING(.u_boot_list))
>> > +
>> > + __reloc_end = .;
>> > + __init_end = .;
>> > +
>> > + SECTION_bss(__init_end (OVERLAY),)
>> > +
>> > + __monitor_end = .;
>> > +
>> > + /*
>> > + * On many Xtensa boards a region of RAM may be mapped to the ROM address
>> > + * space to facilitate on-chip-debug, and U-Boot must fit with that region.
>> > + * The config variables CONFIG_SYS_MONITOR_* define the region.
>> > + * If U-Boot extends beyond this region it will appear discontiguous in the
>> > + * address space and is in danger of overwriting itself during unpacking
>> > + * ("relocation").
>> > + * This causes U-Boot to crash in a way that is difficult to debug. On some
>> > + * boards (such as xtav60) the region is small enough that U-Boot will not
>> > + * fit if compiled entirely with -O0 (a common scenario). To avoid a lengthy
>> > + * debugging session when this happens, ensure a link-time error occurs.
>> > + *
>> > + */
>> > +
>> > + ASSERT(__monitor_end - __monitor_start <= CONFIG_SYS_MONITOR_LEN,
>> > + "U-Boot ROM image is too large. Check optimization level.")
>> > +
>> > + SECTION_xtensa
>> > + SECTION_debug
>> > +
>> > + /DISCARD/ : { *(.dynstr*) }
>> > + /DISCARD/ : { *(.hash*) }
>> > + /DISCARD/ : { *(.interp) }
>> > + /DISCARD/ : { *(.got*) }
>> > + /DISCARD/ : { *(.dynsym) }
>>
>> Are you including the list regions, etc.? u_boot_list is what it is in
>> most .lds files.
>
> Yes, it's in the SECTION_u_boot_list a few lines above.
OK I see.
Regards,
Simon
More information about the U-Boot
mailing list