| Message ID | 1412940944-19107-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 10 October 2014 12:35, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > Move the registering of CPU reset handlers to before the point where > we leave the function in the -bios (not -kernel) case, so CPU reset > works correctly with -bios as well. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > hw/arm/boot.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/arm/boot.c b/hw/arm/boot.c > index c8dc34f0865b..fc6a3ebf853d 100644 > --- a/hw/arm/boot.c > +++ b/hw/arm/boot.c > @@ -488,6 +488,12 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) > int big_endian; > static const ARMInsnFixup *primary_loader; > > + for (; cs; cs = CPU_NEXT(cs)) { > + cpu = ARM_CPU(cs); > + cpu->env.boot_info = info; > + qemu_register_reset(do_cpu_reset, cpu); > + } > + > /* Load the kernel. */ > if (!info->kernel_filename) { > > @@ -651,10 +657,4 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) > } > } > info->is_linux = is_linux; > - > - for (; cs; cs = CPU_NEXT(cs)) { > - cpu = ARM_CPU(cs); > - cpu->env.boot_info = info; > - qemu_register_reset(do_cpu_reset, cpu); > - } > } I've just realised this isn't quite right -- we now call the reset hook in the case where there's no kernel_filename, but we pass it a non-NULL info pointer, which means the reset hook will change the PC to info->entry (which might not even be initialized) rather than leaving it at the reset value set by the cpu's own reset function. One way to fix this would be to have the loop at the top of the function which registers the reset fn not touch cpu->env.boot_info (it will default to NULL), and then retain the loop at the bottom of the function just to set it the boot_info pointer, since at that point we know we have a valid kernel image to load. We could probably also use a comment for the loop at the top. Here's one: /* 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. */ thanks -- PMM
diff --git a/hw/arm/boot.c b/hw/arm/boot.c index c8dc34f0865b..fc6a3ebf853d 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -488,6 +488,12 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) int big_endian; static const ARMInsnFixup *primary_loader; + for (; cs; cs = CPU_NEXT(cs)) { + cpu = ARM_CPU(cs); + cpu->env.boot_info = info; + qemu_register_reset(do_cpu_reset, cpu); + } + /* Load the kernel. */ if (!info->kernel_filename) { @@ -651,10 +657,4 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) } } info->is_linux = is_linux; - - for (; cs; cs = CPU_NEXT(cs)) { - cpu = ARM_CPU(cs); - cpu->env.boot_info = info; - qemu_register_reset(do_cpu_reset, cpu); - } }
Move the registering of CPU reset handlers to before the point where we leave the function in the -bios (not -kernel) case, so CPU reset works correctly with -bios as well. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- hw/arm/boot.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)