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

login
register
mail settings
Submitter Evgeny Voevodin
Date Dec. 23, 2011, 11:40 a.m.
Message ID <1324640414-16000-7-git-send-email-e.voevodin@samsung.com>
Download mbox | patch
Permalink /patch/133016/
State New
Headers show

Comments

Evgeny Voevodin - Dec. 23, 2011, 11:40 a.m.
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(-)
Peter Maydell - Jan. 10, 2012, 3:32 p.m.
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

Patch

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..700a89d 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 held here */
 };
 
 #define WRITE_WORD(p, value) do { \
@@ -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) {
@@ -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;
+            }
+            smpboot[ARRAY_SIZE(smpboot) - 1] = info->smp_bootreg_addr;
+            smpboot[ARRAY_SIZE(smpboot) - 2] = info->smp_priv_base;
+            for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
                 smpboot[n] = tswap32(smpboot[n]);
             }
             rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot),