[PATCH 02/10] bsps/arm: Support recent bootloaders starting kernel in HYP mode

[Pavel Pisa]

Hello Sebastian,

On Monday 04 of July 2016 10:18:13 Pavel Pisa wrote:
> Hello Sebastian,
>
> On Monday 04 of July 2016 07:40:30 Sebastian Huber wrote:
> > On 04/07/16 01:52, ppisa at cmp.felk.cvut.cz wrote:
> > > --- a/c/src/lib/libbsp/arm/shared/include/start.h
> > > +++ b/c/src/lib/libbsp/arm/shared/include/start.h
> > > @@ -45,6 +45,15 @@ extern "C" {
> > >
> > >   #define BSP_START_DATA_SECTION
> > > __attribute__((section(".bsp_start_data")))
> > >
> > > +/*
> > > +* Many ARM boot loaders pass arguments to loaded OS kernel
> > > +*/
> > > +#ifdef BSP_START_HOOKS_WITH_LOADER_ARGS
> > > +#define BSP_START_HOOKS_LOADER_ARGS int saved_psr, int saved_machid,
> > > int saved_dtb_adr +#else
> > > +#define BSP_START_HOOKS_LOADER_ARGS void
> > > +#endif
> > > +
> > >   /**
> > >   * @brief System start entry.
> > >   */
> > > @@ -57,7 +66,7 @@ void _start(void);
> > >   * stack pointers are initialized but before the copying of the
> > > exception * vectors.
> > >   */
> > > -void bsp_start_hook_0(void);
> > > +void bsp_start_hook_0(BSP_START_HOOKS_LOADER_ARGS);
> >
> > Do we really need the saved_psr and saved_machid? I think it is
> > sufficient to pass the DTB address or NULL to bsp_start_hook_0() for all
> > BSPs. We should avoid this conditional parameter list.
> >
> > >   /**
> > >   * @brief Start entry hook 1.
> > > @@ -65,7 +74,7 @@ void bsp_start_hook_0(void);
> > >   * This hook will be called from the start entry code after copying of
> > > the * exception vectors but before the call to boot_card().
> > >   */
> > > -void bsp_start_hook_1(void);
> > > +void bsp_start_hook_1(BSP_START_HOOKS_LOADER_ARGS);
> >
> > These parameters are not set up in start.S in this patch.
>
> I agree that it is not so necessary but I thought about next
> scenario, we have system where RAM memory is not available
> (for example DRAM needs configuration) when start label
> is reached. The initial part in start.S does not require
> memory and should be kept that way even when HYP is included.
> bsp_start_hook_0 could be implemented in assembly for such
> cache and bring up RAM (only) then more complex parameters
> parsing can and should be done after BSS is cleared and
> memory is ready, so this means to do that in bsp_start_hook_1.
> If we consider that bsp_start_hook_0 is implemented conforming
> ARM EABI calling convention then registers r4, r5 and most
> likely r6 are preserved so calling of bsp_start_hook_1 with
> original parameters is not a problem. But I have forgot
> to repeat sequence
>
> 	mov	r0, r4		/* original cpsr value */
> 	mov	r1, r5		/* machine type number or ~0 for DT boot */
> 	mov	r2, r6		/* physical address of ATAGs or DTB */
>
> before call to bsp_start_hook_1. And I liked the idea that
> both (all) calls have same prototype.

The approach is not to be done easily. There is

bsp_vector_table_copy

	ldmia	r1!, {r2-r9}
	stmia	r0!, {r2-r9}
	ldmia	r1!, {r2-r9}
	stmia	r0!, {r2-r9}

before

	bl	bsp_start_hook_1

so that would require to add

diff --git a/c/src/lib/libbsp/arm/shared/start/start.S b/c/src/lib/libbsp/arm/shared/start/start.S
index 0848fff..8b9ec6b 100644
--- a/c/src/lib/libbsp/arm/shared/start/start.S
+++ b/c/src/lib/libbsp/arm/shared/start/start.S
@@ -325,6 +325,8 @@ bsp_start_hook_0_done:
         * vectors and the pointers to the default exception handlers.
         */

+       stmdb   sp!, {r4, r5, r6}
+
        ldr     r0, =bsp_vector_table_begin
        adr     r1, bsp_start_vector_table_begin
        cmp     r0, r1
@@ -336,6 +338,8 @@ bsp_start_hook_0_done:

 bsp_vector_table_copy_done:

+       ldmia   sp!, {r0, r1, r2}
+
        SWITCH_FROM_ARM_TO_THUMB        r0

        /* Branch to start hook 1 */

So I am not sure if gain worth this price. The complete
updated patch on on GitHub

  https://github.com/ppisa/rtems/commit/2db24a3a6dd4523d0d1ab018d5ffb4a7dfb8a245

Thanks for your opinion in advance,

              Pavel