[v5,06/11] hw/arm_boot.c: Extend secondary CPU bootloader.

Secondary CPU bootloader enables interrupt and issues wfi until start address
is written to system controller. The position where to find this start
address is hardcoded to 0x10000030. This commit extends bootloader for
secondary CPU to allow a target board to cpecify a position where
to find start address. If target board doesn't specify start address then
default 0x10000030 is used.

Signed-off-by: Evgeny Voevodin <e.voevodin@samsung.com>
---
 hw/arm-misc.h |    1 +
 hw/arm_boot.c |   23 +++++++++++++++--------
 2 files changed, 16 insertions(+), 8 deletions(-)

On 23 December 2011 11:40, Evgeny Voevodin <e.voevodin@samsung.com> wrote:
> Secondary CPU bootloader enables interrupt and issues wfi until start address
> is written to system controller. The position where to find this start
> address is hardcoded to 0x10000030. This commit extends bootloader for
> secondary CPU to allow a target board to cpecify a position where
> to find start address. If target board doesn't specify start address then
> default 0x10000030 is used.

I think this commit message could be slightly clearer: I suggest:

hw/arm_boot.c: Make SMP boards specify address to poll in bootup loop

The secondary CPU bootloader in arm_boot.c holds secondary CPUs in a
pen until the primary CPU releases them. Make boards specify the
address to be polled to determine whether to leave the pen (it was
previously hardcoded to 0x10000030, which is a Versatile Express/
Realview specific system register address).

[see below for why I say "Make" rather than "Allow".]

> @@ -179,6 +179,8 @@ static void do_cpu_reset(void *opaque)
>  {
>     CPUState *env = opaque;
>     const struct arm_boot_info *info = env->boot_info;
> +    uint32_t smp_bootreg_addr = 0;
> +    target_phys_addr_t p = info->smp_bootreg_addr;
>
>     cpu_reset(env);
>     if (info) {

This is wrong because we dereference info before we check whether
it is NULL.

I apologise for not spotting earlier that WRITE_WORD()
modifies its first argument, or I wouldn't have suggested it.

Let's just do
    stl_phys_notdirty(info->smp_bootreg_addr, 0);
in the else clause below; then we can drop both the local vars here.

> @@ -197,6 +199,7 @@ static void do_cpu_reset(void *opaque)
>                                     info->loader_start);
>                 }
>             } else {
> +                WRITE_WORD(p, smp_bootreg_addr);
>                 env->regs[15] = info->smp_loader_start;
>             }
>         }
> @@ -272,8 +275,12 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>         rom_add_blob_fixed("bootloader", bootloader, sizeof(bootloader),
>                            info->loader_start);
>         if (info->nb_cpus > 1) {
> -            smpboot[10] = info->smp_priv_base;
> -            for (n = 0; n < sizeof(smpboot) / 4; n++) {
> +            if (!info->smp_bootreg_addr) {
> +                info->smp_bootreg_addr = 0x10000030;
> +            }

I think we should just mandate that boards which set
info->nb_cpus to something > 1 must also set info->smp_bootreg_addr.
0x10000030 is realview/vexpress/etc specific (it's the address of
one of the flag registers in the sysregs).

There are only two boards which this affects:
 hw/realview.c
 hw/vexpress.c

(all the other ARM boards are single core).

So it's not a huge effort to update those two boards and it means
we don't need this bit of backcompat code.

Otherwise OK.

-- PMM