On Mon, Aug 30, 2010 at 07:59:22AM -0500, Anthony Liguori wrote:
> 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<yamah...@valinux.co.jp>
>> ---
>> 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.
I'm lost here. Then, what should qdev_reset() do?
So far you have strongly claimed that qdev_reset() should be cold reset.
(the current implementation is imperfect though)
So I had to create the patch that distinguishes warm reset from cold reset.
But here you suggest that qdev_reset() be warm reset and only way to
cold-reset be destroy/re-create.
Can you elaborate on what qdev_reset() API should do?
If qdev_reset() is cold reset, something like qdev_warm_reset()
is necessary.
If qdev_reset() is warm reset and the only way to cold-reset is
destroy/re-create, I'm fine with it.
> 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;
>>
>
>
--
yamahata
