Re: [PATCH] disable clock before rebooting.

Glauber Costa
2008-03-07T14:23:34Z

Avi Kivity wrote:
> Glauber Costa wrote:
>> This patch writes 0 (actually, what really matters is that the
>> LSB is cleared) to the system time msr before rebooting/shutting down
>> the machine.
>>
>> Without it, we can have a random memory location being written
>> when the guest comes back
>> if (!kvm_para_available())
>> @@ -154,6 +181,11 @@ void __init kvmclock_init(void)
>> pv_time_ops.set_wallclock = kvm_set_wallclock;
>> pv_time_ops.sched_clock = kvm_clock_read;
>> pv_apic_ops.setup_secondary_clock = kvm_setup_secondary_clock;
>> + machine_ops.emergency_restart = kvm_emergency_restart;
>> + machine_ops.shutdown = kvm_shutdown;
>> + machine_ops.restart = kvm_restart;
>> + machine_ops.halt = kvm_halt;
>> + machine_ops.power_off = kvm_power_off;
>> clocksource_register(&kvm_clock);
>> }
>> }
>>
>
> Oh, I think that these are all unnecessary. You need to stop the clock
> only if the memory it uses will be reused. Halt, shutdown and poweroff
> clearly don't. Resets need to go through the host anyway, since they
> can be invoked without the guest knowing about it.

power off, agreed.
halt, it doesn't really do anything anyway in reboot.c, and is here just
for "future completeness".

> The only case I can think of where we need to stop the clock is kexec.
>
as for kexec, it uses precisely the shutdown function, doesn't it?

Or is it crash_shutdown?
Humm, /me looks, and I think it's the later, right?

If this is true and resets goes through the host, then we don't even
need the header exports. Ingo'll be happy.