David Gibson <da...@gibson.dropbear.id.au> writes:
> At present the qemu_system_reset() function always performs the same basic
> actions on all machines. This includes running all the reset handler
> hooks, however the order in which they run is not controlled by the board
> logic.
Let's be careful here in referring to "board logic".
Reset should propagate--there's no argument there. Having all devices
register in a list with no control over how reset is dispatched is
wrong. It's workable because of qdev imposes per-bus reset functions
and typically we have a small number of busses and ordering doesn't
matter.
> This is incorrect: any modern real hardware will generally have some sort
> of reset controller, sometimes a full microcontroller or even service
> processor which will control the order in which components on the board are
> reset.
I think this is true only with a loose definition of "modern", but okay.
I don't think this is the right argument. The machine should serve as
the entry point for reset. What components get reset in what order will
need to be a machine hook for practical purposes (since not everything
will be fully QOMized all at once).
So I think this is the right approach for now.
But all of the DT initialization stuff that is leading to this
discussion in the first place is a gross hack to make a PV architecture
work that took far too many short cuts.
Building a DT in memory representing hardware instead of making things
discoverable is not how hardware works. This sort of stuff is either
(1) hard coded in a firmware/flashrom or (2) built dynamically in
firmware. Let's not pretend like we're doing this because it's needed
for real hardware.
Regards,
Anthony Liguori
> For one specific example, the machine level reset handlers may need to
> re-establish certain things in memory to meet the emulated platform's
> specified entry conditions, before reexecuting the guest software.
> re-establish the correct specified entry conditions for the emulated
> platform. This must happen _after_ resetting peripheral devices, or they
> could still be in the middle of DMA operations which would clobber the new
> memory content. However with the normal ordering of reset hooks, the
> machine is not able to register a hook which will run after the normal
> qdev reset registered by generic code.
>
> This patch allows the machine to take control of the sequence of operations
> during reset, by adding a new reset hook to the QEMUMachine. This hook is
> optional - without it we just use the same reset sequence as always. That
> logic is available in qemu_default_system_reset() to make it easy to
> implement the case of the machine logic just needing to do some things
> either before or after the normal reset.
>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> hw/boards.h | 3 +++
> sysemu.h | 1 +
> vl.c | 10 +++++++++-
> 3 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/hw/boards.h b/hw/boards.h
> index 59c01d0..f2bfd3e 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -12,11 +12,14 @@ typedef void QEMUMachineInitFunc(ram_addr_t ram_size,
> const char *initrd_filename,
> const char *cpu_model);
>
> +typedef void QEMUMachineResetFunc(bool report);
> +
> typedef struct QEMUMachine {
> const char *name;
> const char *alias;
> const char *desc;
> QEMUMachineInitFunc *init;
> + QEMUMachineResetFunc *reset;
> int use_scsi;
> int max_cpus;
> unsigned int no_serial:1,
> diff --git a/sysemu.h b/sysemu.h
> index 6540c79..f74319b 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -62,6 +62,7 @@ int qemu_powerdown_requested(void);
> void qemu_system_killed(int signal, pid_t pid);
> void qemu_kill_report(void);
> extern qemu_irq qemu_system_powerdown;
> +void qemu_default_system_reset(bool report);
> void qemu_system_reset(bool report);
>
> void qemu_add_exit_notifier(Notifier *notify);
> diff --git a/vl.c b/vl.c
> index 9fea320..ac47a7c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1396,7 +1396,7 @@ void qemu_unregister_reset(QEMUResetHandler *func, void
> *opaque)
> }
> }
>
> -void qemu_system_reset(bool report)
> +void qemu_default_system_reset(bool report)
> {
> QEMUResetEntry *re, *nre;
>
> @@ -1410,6 +1410,14 @@ void qemu_system_reset(bool report)
> cpu_synchronize_all_post_reset();
> }
Not a huge fan of the naming here. How about qemu_devices_reset()?
> +void qemu_system_reset(bool report)
> +{
> + if (current_machine->reset)
> + current_machine->reset(report);
> + else
> + qemu_default_system_reset(report);
> +}
Missing curly braces.
Regards,
Anthony Liguori
> +
> void qemu_system_reset_request(void)
> {
> if (no_reboot) {
> --
> 1.7.10.4
