Re: [PATCH] disable clock before rebooting.

[Glauber Costa <gcosta@redhat.com>]

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
>>
>> Signed-off-by: Glauber Costa <gcosta@redhat.com>
>> ---
>>  arch/x86/kernel/kvmclock.c |   32 ++++++++++++++++++++++++++++++++
>>  1 files changed, 32 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
>> index f654a12..5c9ff8d 100644
>> --- a/arch/x86/kernel/kvmclock.c
>> +++ b/arch/x86/kernel/kvmclock.c
>> @@ -21,6 +21,7 @@ #include <linux/kvm_para.h>
>>  #include <asm/arch_hooks.h>
>>  #include <asm/msr.h>
>>  #include <linux/percpu.h>
>> +#include <asm/reboot.h>
>>  
>>  #define KVM_SCALE 22
>>  
>> @@ -142,6 +143,32 @@ static void kvm_setup_secondary_clock(vo
>>      setup_secondary_APIC_clock();
>>  }
>>  
>> +/*
>> + * After the clock is registered, the host will keep writing to the
>> + * registered memory location. If the guest happens to shutdown, or 
>> restart,
>> + * this memory won't be valid. In cases like kexec, in which you 
>> install a new kernel,
>> + * this will mean a random memory location will be kept being 
>> written. So before
>> + * any kind of shutdown from our side, we unregister the clock by 
>> writting anything
>> + * that does not have the 'enable' bit set in the msr
>> + */ +static void kvm_restart(char *unused) {
>>   
> 
> This looks like a struct, with the { sitting there on the end.
my bad.

>> +    native_write_msr_safe(MSR_KVM_SYSTEM_TIME, 0, 0);
>> +    native_machine_restart(unused);
>> +}
>> +
>> +/* Forgive me dear lord, for my laziness */
>> +#define kvm_reboot_fn(x) \
>> +static void kvm_##x(void) { \
>> +    native_write_msr_safe(MSR_KVM_SYSTEM_TIME, 0, 0); \
>> +    native_machine_##x(); \
>> +}
>> +
>> +kvm_reboot_fn(emergency_restart)
>> +kvm_reboot_fn(shutdown)
>> +kvm_reboot_fn(halt)
>> +kvm_reboot_fn(power_off)
>> +#undef kvm_reboot_fn
>> +
>>   
> 
> Why not go all the way and to _restart the same way?
> 
Because it got a parameter, and doing it in the same macro would make
my beautiful macros ugly.
Using another one, to pass the argument, didn't seem justifiable to me, 
since there were just one of its kind.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/