Re: [Qemu-devel] [PATCH v3 07/14] hw/arm_boot.c: Add new secondary CPU bootloader.

[Peter Maydell <peter.maydell@linaro.org>]

On 12 December 2011 06:43, 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 adds new bootloader for
> secondary CPU which allows 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

This commit message isn't quite right. We aren't adding a new bootloader,
we're modifying the existing one.

>
> Signed-off-by: Evgeny Voevodin <e.voevodin@samsung.com>
> ---
>  hw/arm-misc.h |    1 +
>  hw/arm_boot.c |   22 +++++++++++++++-------
>  2 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/hw/arm-misc.h b/hw/arm-misc.h
> index af403a1..6e8ae6b 100644
> --- a/hw/arm-misc.h
> +++ b/hw/arm-misc.h
> @@ -31,6 +31,7 @@ struct arm_boot_info {
>     const char *initrd_filename;
>     target_phys_addr_t loader_start;
>     target_phys_addr_t smp_loader_start;
> +    target_phys_addr_t smp_bootreg_addr;
>     target_phys_addr_t smp_priv_base;
>     int nb_cpus;
>     int board_id;
> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> index 215d5de..ecaac22 100644
> --- a/hw/arm_boot.c
> +++ b/hw/arm_boot.c
> @@ -31,17 +31,17 @@ static uint32_t bootloader[] = {
>  /* Entry point for secondary CPUs.  Enable interrupt controller and
>    Issue WFI until start address is written to system controller.  */
>  static uint32_t smpboot[] = {
> -  0xe59f0020, /* ldr     r0, privbase */
> -  0xe3a01001, /* mov     r1, #1 */
> -  0xe5801100, /* str     r1, [r0, #0x100] */
> -  0xe3a00201, /* mov     r0, #0x10000000 */
> -  0xe3800030, /* orr     r0, #0x30 */
> +  0xe59f201c, /* ldr r2, privbase */
> +  0xe59f001c, /* ldr r0, startaddr */
> +  0xe3a01001, /* mov r1, #1 */
> +  0xe5821100, /* str r1, [r2, #256] */
>   0xe320f003, /* wfi */
>   0xe5901000, /* ldr     r1, [r0] */
>   0xe1110001, /* tst     r1, r1 */
>   0x0afffffb, /* beq     <wfi> */
>   0xe12fff11, /* bx      r1 */
> -  0 /* privbase: Private memory region base address.  */
> +  0,          /* privbase: Private memory region base address.  */
> +  0           /* bootreg: Boot register address is hold here */

"held"

>  };
>
>  #define WRITE_WORD(p, value) do { \
> @@ -179,6 +179,7 @@ static void do_cpu_reset(void *opaque)
>  {
>     CPUState *env = opaque;
>     const struct arm_boot_info *info = env->boot_info;
> +    uint8_t smp_bootreg_addr[4] = {0};
>
>     cpu_reset(env);
>     if (info) {
> @@ -197,6 +198,8 @@ static void do_cpu_reset(void *opaque)
>                                     info->loader_start);
>                 }
>             } else {
> +                cpu_physical_memory_rw(info->smp_bootreg_addr, smp_bootreg_addr,
> +                        sizeof(smp_bootreg_addr), 1);

Any reason not to use WRITE_WORD() ?

>                 env->regs[15] = info->smp_loader_start;
>             }
>         }
> @@ -262,6 +265,7 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>         } else {
>             initrd_size = 0;
>         }
> +
>         bootloader[1] |= info->board_id & 0xff;
>         bootloader[2] |= (info->board_id >> 8) & 0xff;
>         bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;

Stray whitespace change, please remove from patch.

> @@ -272,7 +276,11 @@ 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;
> +            if (!info->smp_bootreg_addr) {
> +                info->smp_bootreg_addr = 0x10000030;
> +            }
> +            smpboot[(sizeof(smpboot) - 8)/4] = info->smp_priv_base;
> +            smpboot[(sizeof(smpboot) - 4)/4] = info->smp_bootreg_addr;

smpboot[ARRAY_SIZE(smpboot) - 1] = ...

>             for (n = 0; n < sizeof(smpboot) / 4; n++) {
>                 smpboot[n] = tswap32(smpboot[n]);
>             }
> --
> 1.7.4.1

-- PMM