[Qemu-devel] Re: [PATCH 5/5] RFC: distinguish warm reset from cold reset.

[Anthony Liguori]

On 08/30/2010 02:49 AM, Isaku Yamahata wrote:
> Distinguish warm reset from cold reset by introducing
> cold/warm reset helper function instead of single reset routines.
>
> Signed-off-by: Isaku Yamahata<address@hidden>
> ---
>   hw/hw.h  |    7 +++++
>   sysemu.h |    4 +++
>   vl.c     |   89 +++++++++++++++++++++++++++++++++++++++++++++++++++----------
>   3 files changed, 85 insertions(+), 15 deletions(-)
>
> diff --git a/hw/hw.h b/hw/hw.h
> index 4405092..6fb844e 100644
> --- a/hw/hw.h
> +++ b/hw/hw.h
> @@ -269,6 +269,13 @@ void register_device_unmigratable(DeviceState *dev, const 
> char *idstr,
>
>   typedef void QEMUResetHandler(void *opaque);
>
> +
> +void qemu_register_cold_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_unregister_cold_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_register_warm_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_unregister_warm_reset(QEMUResetHandler *func, void *opaque);
>    

I was thinking that we should stick entirely within the qdev abstraction.

The patchset I sent out introduced a cold reset as a qdev property on 
the devices.

For warm reset, if I understand correctly, we need two things.  We need 
to 1) control propagation order and we need to 2) differentiate 
per-device between cold reset and warm reset.

For (2), I don't know that we truly do need it.  For something like PCI 
AER, wouldn't we just move the AER initialization to the qdev init 
function and then never change the AER registers during reset?

IOW, the only way to do a cold reset would be to destroy and recreate 
the device.

For (1), I still don't fully understand what's needed here.  Do we just 
care about doing a certain transversal order (like depth-first) or do we 
actually care about withholding the reset signal for devices if as Avi 
said, we SCSI devices don't get reset.

Changing transversal order is trivial.  I think we should be very clear 
though if we're going to introduce bus-specific mechanisms to modify 
transversal though.

Regards,

Anthony Liguori

> +/* those two functions are obsoleted by cold/warm reset API. */
>   void qemu_register_reset(QEMUResetHandler *func, void *opaque);
>   void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
>
> diff --git a/sysemu.h b/sysemu.h
> index 7fc4e20..927892a 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -48,7 +48,11 @@ int64_t cpu_get_ticks(void);
>   void cpu_enable_ticks(void);
>   void cpu_disable_ticks(void);
>
> +/* transitional compat = qemu_system_warm_reset_request() */
>   void qemu_system_reset_request(void);
> +
> +void qemu_system_cold_reset_request(void);
> +void qemu_system_warm_reset_request(void);
>   void qemu_system_shutdown_request(void);
>   void qemu_system_powerdown_request(void);
>   extern qemu_irq qemu_system_powerdown;
> diff --git a/vl.c b/vl.c
> index a919a32..609d74c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1122,9 +1122,13 @@ typedef struct QEMUResetEntry {
>   } QEMUResetEntry;
>
>   QTAILQ_HEAD(reset_handlers, QEMUResetEntry);
> -static struct reset_handlers reset_handlers =
> -    QTAILQ_HEAD_INITIALIZER(reset_handlers);
> -static int reset_requested;
> +
> +static struct reset_handlers cold_reset_handlers =
> +    QTAILQ_HEAD_INITIALIZER(cold_reset_handlers);
> +static struct reset_handlers warm_reset_handlers =
> +    QTAILQ_HEAD_INITIALIZER(warm_reset_handlers);
> +static int cold_reset_requested;
> +static int warm_reset_requested;
>   static int shutdown_requested;
>   static int powerdown_requested;
>   int debug_requested;
> @@ -1142,9 +1146,14 @@ static int qemu_shutdown_requested(void)
>       return qemu_requested(&shutdown_requested);
>   }
>
> -static int qemu_reset_requested(void)
> +static int qemu_cold_reset_requested(void)
> +{
> +    return qemu_requested(&cold_reset_requested);
> +}
> +
> +static int qemu_warm_reset_requested(void)
>   {
> -    return qemu_requested(&reset_requested);
> +    return qemu_requested(&warm_reset_requested);
>   }
>
>   static int qemu_powerdown_requested(void)
> @@ -1196,20 +1205,51 @@ static void qemu_system_reset_handler(struct 
> reset_handlers *handlers)
>       }
>   }
>
> +/* obsolated by qemu_register_cold/warm_reset() */
>   void qemu_register_reset(QEMUResetHandler *func, void *opaque)
>   {
> -    qemu_register_reset_handler(func, opaque,&reset_handlers);
> +    qemu_register_cold_reset(func, opaque);
> +    qemu_register_warm_reset(func, opaque);
>   }
>
> +/* obsolated by qemu_unregister_cold/warm_reset() */
>   void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
>   {
> -    qemu_unregister_reset_handler(func, opaque,&reset_handlers);
> +    qemu_unregister_cold_reset(func, opaque);
> +    qemu_unregister_warm_reset(func, opaque);
> +}
> +
> +void qemu_register_cold_reset(QEMUResetHandler *func, void *opaque)
> +{
> +    qemu_register_reset_handler(func, opaque,&cold_reset_handlers);
> +}
> +
> +void qemu_unregister_cold_reset(QEMUResetHandler *func, void *opaque)
> +{
> +    qemu_unregister_reset_handler(func, opaque,&cold_reset_handlers);
> +}
> +
> +static void qemu_system_cold_reset(void)
> +{
> +    qemu_system_reset_handler(&cold_reset_handlers);
> +    monitor_protocol_event(QEVENT_RESET, NULL); /* QEVENT_RESET_COLD? */
> +    cpu_synchronize_all_post_reset();
> +}
> +
> +void qemu_register_warm_reset(QEMUResetHandler *func, void *opaque)
> +{
> +    qemu_register_reset_handler(func, opaque,&warm_reset_handlers);
> +}
> +
> +void qemu_unregister_warm_reset(QEMUResetHandler *func, void *opaque)
> +{
> +    qemu_unregister_reset_handler(func, opaque,&warm_reset_handlers);
>   }
>
> -static void qemu_system_reset(void)
> +static void qemu_system_warm_reset(void)
>   {
> -    qemu_system_reset_handler(&reset_handlers);
> -    monitor_protocol_event(QEVENT_RESET, NULL);
> +    qemu_system_reset_handler(&warm_reset_handlers);
> +    monitor_protocol_event(QEVENT_RESET, NULL); /* QEVENT_RESET_WARM? */
>       cpu_synchronize_all_post_reset();
>   }
>
> @@ -1227,9 +1267,20 @@ static void qemu_system_request_reboot_check(int 
> *requested)
>       qemu_system_request(requested);
>   }
>
> +void qemu_system_cold_reset_request(void)
> +{
> +    qemu_system_request_reboot_check(&cold_reset_requested);
> +}
> +
> +void qemu_system_warm_reset_request(void)
> +{
> +    qemu_system_request_reboot_check(&warm_reset_requested);
> +}
> +
> +/* trantitional compat */
>   void qemu_system_reset_request(void)
>   {
> -    qemu_system_request_reboot_check(&reset_requested);
> +    qemu_system_request_reboot_check(&warm_reset_requested);
>   }
>
>   void qemu_system_shutdown_request(void)
> @@ -1322,7 +1373,9 @@ static int vm_can_run(void)
>   {
>       if (powerdown_requested)
>           return 0;
> -    if (reset_requested)
> +    if (cold_reset_requested)
> +        return 0;
> +    if (warm_reset_requested)
>           return 0;
>       if (shutdown_requested)
>           return 0;
> @@ -1368,9 +1421,15 @@ static void main_loop(void)
>               } else
>                   break;
>           }
> -        if (qemu_reset_requested()) {
> +        if (qemu_warm_reset_requested()) {
> +            pause_all_vcpus();
> +            qemu_system_warm_reset();
> +            resume_all_vcpus();
> +        }
> +        if (qemu_cold_reset_requested()) {
> +            /* power cycle */
>               pause_all_vcpus();
> -            qemu_system_reset();
> +            qemu_system_cold_reset();
>               resume_all_vcpus();
>           }
>           if (qemu_powerdown_requested()) {
> @@ -2992,7 +3051,7 @@ int main(int argc, char **argv, char **envp)
>           exit(1);
>       }
>
> -    qemu_system_reset();
> +    qemu_system_cold_reset();
>       if (loadvm) {
>           if (load_vmstate(loadvm)<  0) {
>               autostart = 0;
>