[PATCH 0/2] prevent memory corruption across reboots

[PATCH 0/2] prevent memory corruption across reboots

[Glauber Costa]

Avi,

I tracked down the kexec paths that requires overloading of machine_ops to two.
So here's a simpler version, that'll probably be a best fit.

thanks



-------------------------------------------------------------------------
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/2] [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 |    7 ++++++-
 include/asm-x86/reboot.h |    1 +
 3 files changed, 9 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/apic.h>
 #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 8b577f1..fa30d4e 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -447,7 +447,8 @@ 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,
+	.crash_shutdown = native_machine_crash_shutdown,
 };
 
 void machine_power_off(void)
@@ -475,3 +476,7 @@ void machine_halt(void)
 	machine_ops.halt();
 }
 
+void machine_crash_shutdown(struct pt_regs *regs)
+{
+	machine_ops.crash_shutdown(regs);
+}
diff --git a/include/asm-x86/reboot.h b/include/asm-x86/reboot.h
index 2ea857c..53dcc12 100644
--- a/include/asm-x86/reboot.h
+++ b/include/asm-x86/reboot.h
@@ -22,5 +22,6 @@ void native_machine_shutdown(void);
 void native_machine_restart(char *__unused);
 void native_machine_halt(void);
 void native_machine_power_off(void);
+void native_machine_crash_shutdown(struct pt_regs *regs);
 
 #endif	/* _ASM_REBOOT_H */
-- 
1.4.2


-------------------------------------------------------------------------
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/2] [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 |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index f654a12..8b838d9 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,26 @@ 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, 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
+ */
+static void kvm_crash_shutdown(struct pt_regs *regs)
+{
+	native_write_msr_safe(MSR_KVM_SYSTEM_TIME, 0, 0);
+	native_machine_crash_shutdown(regs);
+}
+
+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())
@@ -154,6 +175,8 @@ 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;
+		machine_ops.crash_shutdown  = kvm_crash_shutdown;
 		clocksource_register(&kvm_clock);
 	}
 }
-- 
1.4.2


-------------------------------------------------------------------------
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 1/2] [PATCH] allow machine_crash_shutdown to be replaced

[Avi Kivity]

Glauber Costa wrote:
> This patch a llows machine_crash_shutdown to
> be replaced, just like any of the other functions
> in machine_ops
>
>   
er, against what tree is this?  doesn't apply to kvm.git.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
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 1/2] [PATCH] allow machine_crash_shutdown to be replaced

[Glauber Costa]

Avi Kivity wrote:
> Glauber Costa wrote:
>> This patch a llows machine_crash_shutdown to
>> be replaced, just like any of the other functions
>> in machine_ops
>>
>>   
> er, against what tree is this?  doesn't apply to kvm.git.
> 
It'd be kvm.git with the machine_ops non-static functions patch.
However, as it turned out, we only used one of the functions, instead of 
all of them. If ingo prefers, we can revert that patch, and come up with 
a new one that just exposes the function we're actually using. I can 
then route it through kvm.git entirely, instead of x86.

-------------------------------------------------------------------------
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 1/2] [PATCH] allow machine_crash_shutdown to be replaced

[Ingo Molnar]

* Glauber Costa <gcosta@redhat.com> wrote:

> It'd be kvm.git with the machine_ops non-static functions patch. 
> However, as it turned out, we only used one of the functions, instead 
> of all of them. If ingo prefers, we can revert that patch, and come up 
> with a new one that just exposes the function we're actually using. I 
> can then route it through kvm.git entirely, instead of x86.

yeah, please do it minimally. I've pulled the patch below from x86.git, 
you've got my Acked-by for the smaller one as well:

 Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

----------->
Subject: x86: export native versions of machine_ops functions
From: Glauber Costa <gcosta@redhat.com>
Date: Wed, 5 Mar 2008 14:44:00 -0300

People overriding machine_ops provided functions may want to
call the native version after its pre-processing. It already
happens for the smp_ops functions, so I don't see a reason
for avoiding it here.

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

Index: linux-x86.q/arch/x86/kernel/reboot.c
===================================================================
--- linux-x86.q.orig/arch/x86/kernel/reboot.c
+++ linux-x86.q/arch/x86/kernel/reboot.c
@@ -330,7 +330,7 @@ void __attribute__((weak)) mach_reboot_f
 {
 }
 
-static void native_machine_emergency_restart(void)
+void native_machine_emergency_restart(void)
 {
 	int i;
 
@@ -382,7 +382,7 @@ static void native_machine_emergency_res
 	}
 }
 
-static void native_machine_shutdown(void)
+void native_machine_shutdown(void)
 {
 	/* Stop the cpus and apics */
 #ifdef CONFIG_SMP
@@ -438,7 +438,7 @@ static void native_machine_shutdown(void
 #endif
 }
 
-static void native_machine_restart(char *__unused)
+void native_machine_restart(char *__unused)
 {
 	printk("machine restart\n");
 
@@ -447,11 +447,11 @@ static void native_machine_restart(char 
 	machine_emergency_restart();
 }
 
-static void native_machine_halt(void)
+void native_machine_halt(void)
 {
 }
 
-static void native_machine_power_off(void)
+void native_machine_power_off(void)
 {
 	if (pm_power_off) {
 		if (!reboot_force)
Index: linux-x86.q/include/asm-x86/reboot.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/reboot.h
+++ linux-x86.q/include/asm-x86/reboot.h
@@ -16,5 +16,10 @@ struct machine_ops
 extern struct machine_ops machine_ops;
 
 void machine_real_restart(unsigned char *code, int length);
+void native_machine_emergency_restart(void);
+void native_machine_shutdown(void);
+void native_machine_restart(char *__unused);
+void native_machine_halt(void);
+void native_machine_power_off(void);
 
 #endif	/* _ASM_REBOOT_H */

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