Re: [Qemu-devel] [PATCH 1/3] target-arm: Setup smpboot code in all setups

[Vincent Palatin <vincent.palatin_qemu@polytechnique.org>]

Hi Adam,

>> Moving in the right direction, but it would be cleaner if the secondary
>> CPU reset was handled inside arm_boot.c, I think (there is a TODO
>> in that file to that effect). Then we could get rid of the cpu reset
>> hook from realview.c.
>
> Like the following?

This assumes that all the ARM SMP platforms are booting their
secondary CPU the same way as the emulated Realview.
For example, I'm currently writing a Tegra2 (dual A9) SoC emulation
and the second CPU is halted when the platform starts and I cannot
re-use the current smpboot firmware chunk. My current workaround is to
use "info->nb_cpus = 1" and do the init in the board code. Forcing the
reset function will probably not help.

>  Subject: [PATCH] target-arm: Integrate secondary CPU reset in arm_boot
>
> Integrate secondary CPU reset into arm_boot, removing it from realview.c.
> On non-Linux systems secondary CPUs start with the same entry as the boot
> CPU.
>
> Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
> ---
>  hw/arm_boot.c |   23 +++++++++++++++--------
>  hw/realview.c |   14 --------------
>  2 files changed, 15 insertions(+), 22 deletions(-)
>
> diff --git a/hw/arm_boot.c b/hw/arm_boot.c
> index 620550b..41e99d1 100644
> --- a/hw/arm_boot.c
> +++ b/hw/arm_boot.c
> @@ -175,7 +175,7 @@ static void set_kernel_args_old(struct arm_boot_info *info,
>     }
>  }
>
> -static void main_cpu_reset(void *opaque)
> +static void do_cpu_reset(void *opaque)
>  {
>     CPUState *env = opaque;
>     struct arm_boot_info *info = env->boot_info;
> @@ -187,16 +187,20 @@ static void main_cpu_reset(void *opaque)
>             env->regs[15] = info->entry & 0xfffffffe;
>             env->thumb = info->entry & 1;
>         } else {
> -            env->regs[15] = info->loader_start;
> -            if (old_param) {
> -                set_kernel_args_old(info, info->initrd_size,
> +            if (env == first_cpu) {
> +                env->regs[15] = info->loader_start;
> +                if (old_param) {
> +                    set_kernel_args_old(info, info->initrd_size,
> +                                        info->loader_start);
> +                } else {
> +                    set_kernel_args(info, info->initrd_size,
>                                     info->loader_start);
> +                }
>             } else {
> -                set_kernel_args(info, info->initrd_size, info->loader_start);
> +                env->regs[15] = info->smp_loader_start;
>             }
>         }
>     }
> -    /* TODO:  Reset secondary CPUs.  */
>  }
>
>  void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
> @@ -217,7 +221,6 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>
>     if (info->nb_cpus == 0)
>         info->nb_cpus = 1;
> -    env->boot_info = info;
>
>  #ifdef TARGET_WORDS_BIGENDIAN
>     big_endian = 1;
> @@ -279,5 +282,9 @@ void arm_load_kernel(CPUState *env, struct arm_boot_info *info)
>         info->initrd_size = initrd_size;
>     }
>     info->is_linux = is_linux;
> -    qemu_register_reset(main_cpu_reset, env);
> +
> +    for (; env; env = env->next_cpu) {
> +        env->boot_info = info;
> +        qemu_register_reset(do_cpu_reset, env);
> +    }
>  }
> diff --git a/hw/realview.c b/hw/realview.c
> index 6eb6c6a..fae444a 100644
> --- a/hw/realview.c
> +++ b/hw/realview.c
> @@ -104,17 +104,6 @@ static struct arm_boot_info realview_binfo = {
>     .smp_loader_start = SMP_BOOT_ADDR,
>  };
>
> -static void secondary_cpu_reset(void *opaque)
> -{
> -  CPUState *env = opaque;
> -
> -  cpu_reset(env);
> -  /* Set entry point for secondary CPUs.  This assumes we're using
> -     the init code from arm_boot.c.  Real hardware resets all CPUs
> -     the same.  */
> -  env->regs[15] = SMP_BOOT_ADDR;
> -}
> -
>  /* The following two lists must be consistent.  */
>  enum realview_board_type {
>     BOARD_EB,
> @@ -176,9 +165,6 @@ static void realview_init(ram_addr_t ram_size,
>         }
>         irqp = arm_pic_init_cpu(env);
>         cpu_irq[n] = irqp[ARM_PIC_CPU_IRQ];
> -        if (n > 0) {
> -            qemu_register_reset(secondary_cpu_reset, env);
> -        }
>     }
>     if (arm_feature(env, ARM_FEATURE_V7)) {
>         if (is_mpcore) {
> --
> 1.7.2.3


-- 
Vincent