[PATCH 0/3 - resend] kvmclock reboot

[PATCH 0/3 - resend] kvmclock reboot

[Glauber Costa]

Avi,

Hope this series is okay now.

Thanks for the testing



-------------------------------------------------------------------------
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/

[PATCH 1/3] [PATCH] allow machine_crash_shutdown to be replaced

[Glauber Costa]

This patch a llows machine_crash_shutdown to
be replaced, just like any of the other functions
in machine_ops

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 arch/x86/kernel/crash.c  |    3 ++-
 arch/x86/kernel/reboot.c |   11 ++++++++++-
 include/asm-x86/reboot.h |    1 +
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 9a5fa0a..d262306 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -25,6 +25,7 @@
 #include <asm/hpet.h>
 #include <linux/kdebug.h>
 #include <asm/smp.h>
+#include <asm/reboot.h>
 
 #ifdef CONFIG_X86_32
 #include <mach_ipi.h>
@@ -121,7 +122,7 @@ static void nmi_shootdown_cpus(void)
 }
 #endif
 
-void machine_crash_shutdown(struct pt_regs *regs)
+void native_machine_crash_shutdown(struct pt_regs *regs)
 {
 	/* This function is only called after the system
 	 * has panicked or is otherwise in a critical state.
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 55ceb8c..ea95f11 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -453,7 +453,10 @@ struct machine_ops machine_ops = {
 	.shutdown = native_machine_shutdown,
 	.emergency_restart = native_machine_emergency_restart,
 	.restart = native_machine_restart,
-	.halt = native_machine_halt
+	.halt = native_machine_halt,
+#ifdef CONFIG_KEXEC
+	.crash_shutdown = native_machine_crash_shutdown,
+#endif
 };
 
 void machine_power_off(void)
@@ -481,3 +484,9 @@ void machine_halt(void)
 	machine_ops.halt();
 }
 
+#ifdef CONFIG_KEXEC
+void machine_crash_shutdown(struct pt_regs *regs)
+{
+	machine_ops.crash_shutdown(regs);
+}
+#endif
diff --git a/include/asm-x86/reboot.h b/include/asm-x86/reboot.h
index e9e3ffc..ff9b546 100644
--- a/include/asm-x86/reboot.h
+++ b/include/asm-x86/reboot.h
@@ -16,5 +16,6 @@ struct machine_ops
 extern struct machine_ops machine_ops;
 
 void machine_real_restart(unsigned char *code, int length);
+void native_machine_crash_shutdown(struct pt_regs *regs);
 
 #endif	/* _ASM_REBOOT_H */
-- 
1.5.0.6


-------------------------------------------------------------------------
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/

[PATCH 2/3] [PATCH] make native_machine_shutdown non-static

[Glauber Costa]

it will allow external users to call it. It is mainly
useful for routines that will override its machine_ops
field for its own special purposes, but want to call the
normal shutdown routine after they're done

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 arch/x86/kernel/reboot.c |    2 +-
 include/asm-x86/reboot.h |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index ea95f11..7215de6 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -382,7 +382,7 @@ static void native_machine_emergency_restart(void)
 	}
 }
 
-static void native_machine_shutdown(void)
+void native_machine_shutdown(void)
 {
 	/* Stop the cpus and apics */
 #ifdef CONFIG_SMP
diff --git a/include/asm-x86/reboot.h b/include/asm-x86/reboot.h
index ff9b546..c5e8722 100644
--- a/include/asm-x86/reboot.h
+++ b/include/asm-x86/reboot.h
@@ -17,5 +17,6 @@ extern struct machine_ops machine_ops;
 
 void machine_real_restart(unsigned char *code, int length);
 void native_machine_crash_shutdown(struct pt_regs *regs);
+void native_machine_shutdown(void);
 
 #endif	/* _ASM_REBOOT_H */
-- 
1.5.0.6


-------------------------------------------------------------------------
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/

[PATCH 3/3] [PATCH] disable clock before rebooting.

[Glauber Costa]

This patch writes 0 (actually, what really matters is that the
LSB is cleared) to the system time msr before shutting down
the machine for kexec.

Without it, we can have a random memory location being written
when the guest comes back

It overrides the functions shutdown, used in the path of kernel_kexec() (sys.c)
and crash_shutdown, used in the path of crash_kexec() (kexec.c)

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 arch/x86/kernel/kvmclock.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index b999f5e..ddee040 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -22,6 +22,7 @@
 #include <asm/msr.h>
 #include <asm/apic.h>
 #include <linux/percpu.h>
+#include <asm/reboot.h>
 
 #define KVM_SCALE 22
 
@@ -143,6 +144,28 @@ static void kvm_setup_secondary_clock(void)
 	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, this memory
+ * won't be valid. In cases like kexec, in which you install a new kernel, this
+ * means 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
+ */
+#ifdef CONFIG_KEXEC
+static void kvm_crash_shutdown(struct pt_regs *regs)
+{
+	native_write_msr_safe(MSR_KVM_SYSTEM_TIME, 0, 0);
+	native_machine_crash_shutdown(regs);
+}
+#endif
+
+static void kvm_shutdown(void)
+{
+	native_write_msr_safe(MSR_KVM_SYSTEM_TIME, 0, 0);
+	native_machine_shutdown();
+}
+
 void __init kvmclock_init(void)
 {
 	if (!kvm_para_available())
@@ -155,6 +178,10 @@ 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.shutdown  = kvm_shutdown;
+#ifdef CONFIG_KEXEC
+		machine_ops.crash_shutdown  = kvm_crash_shutdown;
+#endif
 		clocksource_register(&kvm_clock);
 	}
 }
-- 
1.5.0.6

[PATCH 3/3] [PATCH] disable clock before rebooting.

[Glauber Costa]

This patch writes 0 (actually, what really matters is that the
LSB is cleared) to the system time msr before shutting down
the machine for kexec.

Without it, we can have a random memory location being written
when the guest comes back

It overrides the functions shutdown, used in the path of kernel_kexec() (sys.c)
and crash_shutdown, used in the path of crash_kexec() (kexec.c)

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 arch/x86/kernel/kvmclock.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index b999f5e..ddee040 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -22,6 +22,7 @@
 #include <asm/msr.h>
 #include <asm/apic.h>
 #include <linux/percpu.h>
+#include <asm/reboot.h>
 
 #define KVM_SCALE 22
 
@@ -143,6 +144,28 @@ static void kvm_setup_secondary_clock(void)
 	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, this memory
+ * won't be valid. In cases like kexec, in which you install a new kernel, this
+ * means 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
+ */
+#ifdef CONFIG_KEXEC
+static void kvm_crash_shutdown(struct pt_regs *regs)
+{
+	native_write_msr_safe(MSR_KVM_SYSTEM_TIME, 0, 0);
+	native_machine_crash_shutdown(regs);
+}
+#endif
+
+static void kvm_shutdown(void)
+{
+	native_write_msr_safe(MSR_KVM_SYSTEM_TIME, 0, 0);
+	native_machine_shutdown();
+}
+
 void __init kvmclock_init(void)
 {
 	if (!kvm_para_available())
@@ -155,6 +178,10 @@ 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.shutdown  = kvm_shutdown;
+#ifdef CONFIG_KEXEC
+		machine_ops.crash_shutdown  = kvm_crash_shutdown;
+#endif
 		clocksource_register(&kvm_clock);
 	}
 }
-- 
1.5.0.6


-------------------------------------------------------------------------
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/

[PATCH 2/3] [PATCH] make native_machine_shutdown non-static

[Glauber Costa]

it will allow external users to call it. It is mainly
useful for routines that will override its machine_ops
field for its own special purposes, but want to call the
normal shutdown routine after they're done

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 arch/x86/kernel/reboot.c |    2 +-
 include/asm-x86/reboot.h |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index ea95f11..7215de6 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -382,7 +382,7 @@ static void native_machine_emergency_restart(void)
 	}
 }
 
-static void native_machine_shutdown(void)
+void native_machine_shutdown(void)
 {
 	/* Stop the cpus and apics */
 #ifdef CONFIG_SMP
diff --git a/include/asm-x86/reboot.h b/include/asm-x86/reboot.h
index ff9b546..c5e8722 100644
--- a/include/asm-x86/reboot.h
+++ b/include/asm-x86/reboot.h
@@ -17,5 +17,6 @@ extern struct machine_ops machine_ops;
 
 void machine_real_restart(unsigned char *code, int length);
 void native_machine_crash_shutdown(struct pt_regs *regs);
+void native_machine_shutdown(void);
 
 #endif	/* _ASM_REBOOT_H */
-- 
1.5.0.6

[PATCH 1/3] [PATCH] allow machine_crash_shutdown to be replaced

[Glauber Costa]

This patch a llows machine_crash_shutdown to
be replaced, just like any of the other functions
in machine_ops

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 arch/x86/kernel/crash.c  |    3 ++-
 arch/x86/kernel/reboot.c |   11 ++++++++++-
 include/asm-x86/reboot.h |    1 +
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 9a5fa0a..d262306 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -25,6 +25,7 @@
 #include <asm/hpet.h>
 #include <linux/kdebug.h>
 #include <asm/smp.h>
+#include <asm/reboot.h>
 
 #ifdef CONFIG_X86_32
 #include <mach_ipi.h>
@@ -121,7 +122,7 @@ static void nmi_shootdown_cpus(void)
 }
 #endif
 
-void machine_crash_shutdown(struct pt_regs *regs)
+void native_machine_crash_shutdown(struct pt_regs *regs)
 {
 	/* This function is only called after the system
 	 * has panicked or is otherwise in a critical state.
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 55ceb8c..ea95f11 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -453,7 +453,10 @@ struct machine_ops machine_ops = {
 	.shutdown = native_machine_shutdown,
 	.emergency_restart = native_machine_emergency_restart,
 	.restart = native_machine_restart,
-	.halt = native_machine_halt
+	.halt = native_machine_halt,
+#ifdef CONFIG_KEXEC
+	.crash_shutdown = native_machine_crash_shutdown,
+#endif
 };
 
 void machine_power_off(void)
@@ -481,3 +484,9 @@ void machine_halt(void)
 	machine_ops.halt();
 }
 
+#ifdef CONFIG_KEXEC
+void machine_crash_shutdown(struct pt_regs *regs)
+{
+	machine_ops.crash_shutdown(regs);
+}
+#endif
diff --git a/include/asm-x86/reboot.h b/include/asm-x86/reboot.h
index e9e3ffc..ff9b546 100644
--- a/include/asm-x86/reboot.h
+++ b/include/asm-x86/reboot.h
@@ -16,5 +16,6 @@ struct machine_ops
 extern struct machine_ops machine_ops;
 
 void machine_real_restart(unsigned char *code, int length);
+void native_machine_crash_shutdown(struct pt_regs *regs);
 
 #endif	/* _ASM_REBOOT_H */
-- 
1.5.0.6

Re: [PATCH 0/3 - resend] kvmclock reboot

[Avi Kivity]

Glauber Costa wrote:
> Avi,
>
> Hope this series is okay now.
>
> Thanks for the testing
>
>
>   

Applied, thanks.

[Once a patchset has been applied and pushed (as seen on kvm-commits), I 
prefer incremental patches]

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
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/

Re: [PATCH 0/3 - resend] kvmclock reboot

[Glauber Costa]

Avi Kivity wrote:
> Glauber Costa wrote:
>> Avi,
>>
>> Hope this series is okay now.
>>
>> Thanks for the testing
>>
>>
>>   
> 
> Applied, thanks.
> 
> [Once a patchset has been applied and pushed (as seen on kvm-commits), I 
> prefer incremental patches]
> 
But I think this is a very good case of an exception, since the applied 
patches would break bisect. I prefer to hear "damn glommer!" because of 
that than "damn glommer!!" because of a broken bisect.

Re: [PATCH 0/3 - resend] kvmclock reboot

[Glauber Costa]

Avi Kivity wrote:
> Glauber Costa wrote:
>> Avi,
>>
>> Hope this series is okay now.
>>
>> Thanks for the testing
>>
>>
>>   
> 
> Applied, thanks.
> 
> [Once a patchset has been applied and pushed (as seen on kvm-commits), I 
> prefer incremental patches]
> 
But I think this is a very good case of an exception, since the applied 
patches would break bisect. I prefer to hear "damn glommer!" because of 
that than "damn glommer!!" because of a broken bisect.

-------------------------------------------------------------------------
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/

Re: [PATCH 0/3 - resend] kvmclock reboot

[Avi Kivity]

Glauber Costa wrote:
>>
>> [Once a patchset has been applied and pushed (as seen on 
>> kvm-commits), I prefer incremental patches]
>>
> But I think this is a very good case of an exception, since the 
> applied patches would break bisect. I prefer to hear "damn glommer!" 
> because of that than "damn glommer!!" because of a broken bisect.

I fold them up before sending to Linus anyway, so upstream bisect 
continues to work.  For kvm.git, it's a choice between rewinding it a 
bit (which may confuse anyone who pulls) and breaking bisect.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH 0/3 - resend] kvmclock reboot

[Avi Kivity]

Glauber Costa wrote:
>>
>> [Once a patchset has been applied and pushed (as seen on 
>> kvm-commits), I prefer incremental patches]
>>
> But I think this is a very good case of an exception, since the 
> applied patches would break bisect. I prefer to hear "damn glommer!" 
> because of that than "damn glommer!!" because of a broken bisect.

I fold them up before sending to Linus anyway, so upstream bisect 
continues to work.  For kvm.git, it's a choice between rewinding it a 
bit (which may confuse anyone who pulls) and breaking bisect.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
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/

Re: [PATCH 0/3 - resend] kvmclock reboot

[Glauber Costa]

Avi Kivity wrote:
> Glauber Costa wrote:
>>>
>>> [Once a patchset has been applied and pushed (as seen on 
>>> kvm-commits), I prefer incremental patches]
>>>
>> But I think this is a very good case of an exception, since the 
>> applied patches would break bisect. I prefer to hear "damn glommer!" 
>> because of that than "damn glommer!!" because of a broken bisect.
> 
> I fold them up before sending to Linus anyway, so upstream bisect 
> continues to work.  For kvm.git, it's a choice between rewinding it a 
> bit (which may confuse anyone who pulls) and breaking bisect.
> 
> 
ok, this wfm. I'll send incrementally next time.

-------------------------------------------------------------------------
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/

Re: [PATCH 0/3 - resend] kvmclock reboot

[Glauber Costa]

Avi Kivity wrote:
> Glauber Costa wrote:
>>>
>>> [Once a patchset has been applied and pushed (as seen on 
>>> kvm-commits), I prefer incremental patches]
>>>
>> But I think this is a very good case of an exception, since the 
>> applied patches would break bisect. I prefer to hear "damn glommer!" 
>> because of that than "damn glommer!!" because of a broken bisect.
> 
> I fold them up before sending to Linus anyway, so upstream bisect 
> continues to work.  For kvm.git, it's a choice between rewinding it a 
> bit (which may confuse anyone who pulls) and breaking bisect.
> 
> 
ok, this wfm. I'll send incrementally next time.

Re: [PATCH 0/3 - resend] kvmclock reboot

[Avi Kivity]

Glauber Costa wrote:
> Avi,
>
> Hope this series is okay now.
>
> Thanks for the testing
>
>
>   

Applied, thanks.

[Once a patchset has been applied and pushed (as seen on kvm-commits), I 
prefer incremental patches]

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

[PATCH 0/3 - resend] kvmclock reboot

[Glauber Costa]

Avi,

Hope this series is okay now.

Thanks for the testing