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

[Pavel Pisa]

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.
So I would add it there or if you think that it does not worth
to add these instructions then I would remove that.
In the fact it is possible to not include parameters
passing at all. But initial idea has been to allow record
HYP startup and it cannot be done before memory is initialized
in some cases. HYP presence should be checked for secondary CPUs
boot on SMP and if it does not match CPU0 then throw fatal error
or stop secondary CPU.

Thanks for review,

                Pavel Pisa