On Tue, Jun 2, 2015 at 9:33 AM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> From: Eric Auger <eric.au...@linaro.org>
>
> Device tree nodes for the platform bus and its children dynamic sysbus
> devices are added in a machine init done notifier. To load the dtb once,
> after those latter nodes are built and before ROM freeze, the actual
> arm_load_kernel existing code is moved into a notifier notify function,
> arm_load_kernel_notify. arm_load_kernel now only registers the
> corresponding notifier.
>
Does this work? I am experiencing a regression on this patch for
xlnx-ep108 board. I think it is because this is now delaying
arm_load_kernel_notify call until after rom_load_all. From vl.c:
if (rom_load_all() != 0) {
fprintf(stderr, "rom loading failed\n");
exit(1);
}
/* TODO: once all bus devices are qdevified, this should be done
* when bus is created by qdev.c */
qemu_register_reset(qbus_reset_all_fn, sysbus_get_default());
qemu_run_machine_init_done_notifiers();
the machine_init_done_notifiers are called after the rom_load_all()
call which does the image loading. So the image-to-load registration
is too late.
Straight revert of this patch fixes the issue for me.
Regards,
Peter
> Machine files that do not support platform bus stay unchanged. Machine
> files willing to support dynamic sysbus devices must call arm_load_kernel
> before sysbus-fdt arm_register_platform_bus_fdt_creator to make sure
> dynamic sysbus device nodes are integrated in the dtb.
>
> Signed-off-by: Eric Auger <eric.au...@linaro.org>
> Reviewed-by: Shannon Zhao <zhaoshengl...@huawei.com>
> Reviewed-by: Alexander Graf <ag...@suse.de>
> Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
> Message-id: 1433244554-12898-3-git-send-email-eric.au...@linaro.org
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> hw/arm/boot.c | 14 +++++++++++++-
> include/hw/arm/arm.h | 28 ++++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index fa69503..d036624 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -557,7 +557,7 @@ static void load_image_to_fw_cfg(FWCfgState *fw_cfg,
> uint16_t size_key,
> fw_cfg_add_bytes(fw_cfg, data_key, data, size);
> }
>
> -void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> +static void arm_load_kernel_notify(Notifier *notifier, void *data)
> {
> CPUState *cs;
> int kernel_size;
> @@ -568,6 +568,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
> *info)
> hwaddr entry, kernel_load_offset;
> int big_endian;
> static const ARMInsnFixup *primary_loader;
> + ArmLoadKernelNotifier *n = DO_UPCAST(ArmLoadKernelNotifier,
> + notifier, notifier);
> + ARMCPU *cpu = n->cpu;
> + 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
> @@ -775,3 +780,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
> *info)
> ARM_CPU(cs)->env.boot_info = info;
> }
> }
> +
> +void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> +{
> + 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);
> +}
> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
> index 5c940eb..760804c 100644
> --- a/include/hw/arm/arm.h
> +++ b/include/hw/arm/arm.h
> @@ -13,11 +13,21 @@
>
> #include "exec/memory.h"
> #include "hw/irq.h"
> +#include "qemu/notify.h"
>
> /* armv7m.c */
> qemu_irq *armv7m_init(MemoryRegion *system_memory, int mem_size, int num_irq,
> const char *kernel_filename, const char *cpu_model);
>
> +/*
> + * struct used as a parameter of the arm_load_kernel machine init
> + * done notifier
> + */
> +typedef struct {
> + Notifier notifier; /* actual notifier */
> + ARMCPU *cpu; /* handle to the first cpu object */
> +} ArmLoadKernelNotifier;
> +
> /* arm_boot.c */
> struct arm_boot_info {
> uint64_t ram_size;
> @@ -64,6 +74,8 @@ struct arm_boot_info {
> * the user it should implement this hook.
> */
> void (*modify_dtb)(const struct arm_boot_info *info, void *fdt);
> + /* machine init done notifier executing arm_load_dtb */
> + ArmLoadKernelNotifier load_kernel_notifier;
> /* Used internally by arm_boot.c */
> int is_linux;
> hwaddr initrd_start;
> @@ -75,6 +87,22 @@ struct arm_boot_info {
> */
> bool firmware_loaded;
> };
> +
> +/**
> + * arm_load_kernel - Loads memory with everything needed to boot
> + *
> + * @cpu: handle to the first CPU object
> + * @info: handle to the boot info struct
> + * Registers a machine init done notifier that copies to memory
> + * everything needed to boot, depending on machine and user options:
> + * kernel image, boot loaders, initrd, dtb. Also registers the CPU
> + * reset handler.
> + *
> + * In case the machine file supports the platform bus device and its
> + * dynamically instantiable sysbus devices, this function must be called
> + * before sysbus-fdt arm_register_platform_bus_fdt_creator. Indeed the
> + * machine init done notifiers are called in registration reverse order.
> + */
> void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info);
>
> /* Multiplication factor to convert from system clock ticks to qemu timer
> --
> 1.9.1
>
>
