Re: [Qemu-devel] [PATCH] hw/arm/boot: fix rom_reset notifier registration order

[Peter Crosthwaite]

On Fri, Jun 12, 2015 at 5:19 AM, Eric Auger <address@hidden> wrote:
> commit ac9d32e39664e060cd1b538ff190980d57ad69e4 had the consequence to
> register the do_cpu_reset after the rom_reset one. Hence they get
> executed in the wrong order. This commit restores the registration of
> do_cpu_reset in arm_load_kernel.
>
> Signed-off-by: Eric Auger <address@hidden>

Reported-by: Peter Crosthwaite <address@hidden>
Tested-by: Peter Crosthwaite <address@hidden>
Reviewed-by: Peter Crosthwaite <address@hidden>

I don't think this is the ideal solution but it does fix my bug and it
is straight revert of the critical part of 9d32.., so it is OK as the
near term fix. I'll make some comments on the other thread.

Regards,
Peter

> ---
>  hw/arm/boot.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index d036624..1e7fd28 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -574,15 +574,6 @@ static void arm_load_kernel_notify(Notifier *notifier, 
> void *data)
>      struct arm_boot_info *info =
>          container_of(n, struct arm_boot_info, load_kernel_notifier);
>
> -    /* CPU objects (unlike devices) are not automatically reset on system
> -     * reset, so we must always register a handler to do so. If we're
> -     * actually loading a kernel, the handler is also responsible for
> -     * arranging that we start it correctly.
> -     */
> -    for (cs = CPU(cpu); cs; cs = CPU_NEXT(cs)) {
> -        qemu_register_reset(do_cpu_reset, ARM_CPU(cs));
> -    }
> -
>      /* Load the kernel.  */
>      if (!info->kernel_filename || info->firmware_loaded) {
>
> @@ -783,7 +774,18 @@ static void arm_load_kernel_notify(Notifier *notifier, 
> void *data)
>
>  void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>  {
> +    CPUState *cs;
> +
>      info->load_kernel_notifier.cpu = cpu;
>      info->load_kernel_notifier.notifier.notify = arm_load_kernel_notify;
>      
> qemu_add_machine_init_done_notifier(&info->load_kernel_notifier.notifier);
> +
> +    /* CPU objects (unlike devices) are not automatically reset on system
> +     * reset, so we must always register a handler to do so. If we're
> +     * actually loading a kernel, the handler is also responsible for
> +     * arranging that we start it correctly.
> +     */
> +    for (cs = CPU(cpu); cs; cs = CPU_NEXT(cs)) {
> +        qemu_register_reset(do_cpu_reset, ARM_CPU(cs));
> +    }
>  }
> --
> 1.8.3.2
>
>