[PATCH 0/6] misc changes for kexec in pv-on-hvm guests

[PATCH 0/6] misc changes for kexec in pv-on-hvm guests

[Olaf Hering]

The following series partly fixes kexec in a pv-on-hvm guest. After a few
iterations of kexec boots the guest will panic with memory corruption.

A fixed kexec-tools-2.0.2 package is required:
http://lists.infradead.org/pipermail/kexec/2011-May/005026.html

Olaf

[PATCH 1/6] xen: use static initializers in xen-balloon.c

[Olaf Hering]

[-- Attachment #1: xen.balloon.static_initializer.patch --]
[-- Type: text/plain, Size: 1619 bytes --]

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 drivers/xen/xen-balloon.c |   18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

Index: linux-3.0/drivers/xen/xen-balloon.c
===================================================================
--- linux-3.0.orig/drivers/xen/xen-balloon.c
+++ linux-3.0/drivers/xen/xen-balloon.c
@@ -50,11 +50,6 @@ static struct sys_device balloon_sysdev;
 
 static int register_balloon(struct sys_device *sysdev);
 
-static struct xenbus_watch target_watch =
-{
-	.node = "memory/target"
-};
-
 /* React to a change in the target key */
 static void watch_target(struct xenbus_watch *watch,
 			 const char **vec, unsigned int len)
@@ -74,6 +69,11 @@ static void watch_target(struct xenbus_w
 	balloon_set_new_target(new_target >> (PAGE_SHIFT - 10));
 }
 
+static struct xenbus_watch target_watch = {
+	.node = "memory/target",
+	.callback = watch_target,
+};
+
 static int balloon_init_watcher(struct notifier_block *notifier,
 				unsigned long event,
 				void *data)
@@ -87,7 +87,9 @@ static int balloon_init_watcher(struct n
 	return NOTIFY_DONE;
 }
 
-static struct notifier_block xenstore_notifier;
+static struct notifier_block xenstore_notifier = {
+	.notifier_call = balloon_init_watcher,
+};
 
 static int __init balloon_init(void)
 {
@@ -97,10 +99,6 @@ static int __init balloon_init(void)
 	pr_info("xen-balloon: Initialising balloon driver.\n");
 
 	register_balloon(&balloon_sysdev);
-
-	target_watch.callback = watch_target;
-	xenstore_notifier.notifier_call = balloon_init_watcher;
-
 	register_xenstore_notifier(&xenstore_notifier);
 
 	return 0;

Re: [PATCH 1/6] xen: use static initializers in xen-balloon.c

[Konrad Rzeszutek Wilk]

On Tue, Jul 26, 2011 at 01:52:10PM +0200, Olaf Hering wrote:

You need a description of why you are doing this.

And also make sure you use git, not patches.

Lastly, please also CC the LKML mailing list and
whoever shows up as you run scripts/get_maintainer.pl
for the patches.

> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> ---
>  drivers/xen/xen-balloon.c |   18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 
> Index: linux-3.0/drivers/xen/xen-balloon.c
> ===================================================================
> --- linux-3.0.orig/drivers/xen/xen-balloon.c
> +++ linux-3.0/drivers/xen/xen-balloon.c
> @@ -50,11 +50,6 @@ static struct sys_device balloon_sysdev;
>  
>  static int register_balloon(struct sys_device *sysdev);
>  
> -static struct xenbus_watch target_watch =
> -{
> -	.node = "memory/target"
> -};
> -
>  /* React to a change in the target key */
>  static void watch_target(struct xenbus_watch *watch,
>  			 const char **vec, unsigned int len)
> @@ -74,6 +69,11 @@ static void watch_target(struct xenbus_w
>  	balloon_set_new_target(new_target >> (PAGE_SHIFT - 10));
>  }
>  
> +static struct xenbus_watch target_watch = {
> +	.node = "memory/target",
> +	.callback = watch_target,
> +};
> +
>  static int balloon_init_watcher(struct notifier_block *notifier,
>  				unsigned long event,
>  				void *data)
> @@ -87,7 +87,9 @@ static int balloon_init_watcher(struct n
>  	return NOTIFY_DONE;
>  }
>  
> -static struct notifier_block xenstore_notifier;
> +static struct notifier_block xenstore_notifier = {
> +	.notifier_call = balloon_init_watcher,
> +};
>  
>  static int __init balloon_init(void)
>  {
> @@ -97,10 +99,6 @@ static int __init balloon_init(void)
>  	pr_info("xen-balloon: Initialising balloon driver.\n");
>  
>  	register_balloon(&balloon_sysdev);
> -
> -	target_watch.callback = watch_target;
> -	xenstore_notifier.notifier_call = balloon_init_watcher;
> -
>  	register_xenstore_notifier(&xenstore_notifier);
>  
>  	return 0;
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot

[Olaf Hering]

[-- Attachment #1: xen.syscore_ops.manage.shutdown_event.patch --]
[-- Type: text/plain, Size: 1156 bytes --]

Unregister the shutdown and sysrq watch during kexec.  The watches can
not be re-registered in the kexec kernel because they are still seen as
busy by xenstore.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 drivers/xen/manage.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

Index: linux-3.0/drivers/xen/manage.c
===================================================================
--- linux-3.0.orig/drivers/xen/manage.c
+++ linux-3.0/drivers/xen/manage.c
@@ -320,6 +320,18 @@ static int shutdown_event(struct notifie
 	return NOTIFY_DONE;
 }
 
+static void xenbus_disable_shutdown_watcher(void)
+{
+	unregister_xenbus_watch(&shutdown_watch);
+#ifdef CONFIG_MAGIC_SYSRQ
+	unregister_xenbus_watch(&sysrq_watch);
+#endif
+}
+
+static struct syscore_ops xenbus_watcher_syscore_ops = {
+	.shutdown = xenbus_disable_shutdown_watcher,
+};
+
 int xen_setup_shutdown_event(void)
 {
 	static struct notifier_block xenstore_notifier = {
@@ -329,6 +341,7 @@ int xen_setup_shutdown_event(void)
 	if (!xen_domain())
 		return -ENODEV;
 	register_xenstore_notifier(&xenstore_notifier);
+	register_syscore_ops(&xenbus_watcher_syscore_ops);
 
 	return 0;
 }

Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot

[Konrad Rzeszutek Wilk]

On Tue, Jul 26, 2011 at 01:52:11PM +0200, Olaf Hering wrote:
> Unregister the shutdown and sysrq watch during kexec.  The watches can
> not be re-registered in the kexec kernel because they are still seen as
> busy by xenstore.

So this is the PV or HVM guest doing the kexec?

> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> ---
>  drivers/xen/manage.c |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> Index: linux-3.0/drivers/xen/manage.c
> ===================================================================
> --- linux-3.0.orig/drivers/xen/manage.c
> +++ linux-3.0/drivers/xen/manage.c
> @@ -320,6 +320,18 @@ static int shutdown_event(struct notifie
>  	return NOTIFY_DONE;
>  }
>  
> +static void xenbus_disable_shutdown_watcher(void)
> +{
> +	unregister_xenbus_watch(&shutdown_watch);
> +#ifdef CONFIG_MAGIC_SYSRQ
> +	unregister_xenbus_watch(&sysrq_watch);
> +#endif
> +}
> +
> +static struct syscore_ops xenbus_watcher_syscore_ops = {
> +	.shutdown = xenbus_disable_shutdown_watcher,
> +};
> +
>  int xen_setup_shutdown_event(void)
>  {
>  	static struct notifier_block xenstore_notifier = {
> @@ -329,6 +341,7 @@ int xen_setup_shutdown_event(void)
>  	if (!xen_domain())
>  		return -ENODEV;
>  	register_xenstore_notifier(&xenstore_notifier);
> +	register_syscore_ops(&xenbus_watcher_syscore_ops);
>  
>  	return 0;
>  }
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 2/6] xen/hvm kexec: unregister shutdown+sysrq watches during reboot

[Olaf Hering]

On Tue, Jul 26, Konrad Rzeszutek Wilk wrote:

> On Tue, Jul 26, 2011 at 01:52:11PM +0200, Olaf Hering wrote:
> > Unregister the shutdown and sysrq watch during kexec.  The watches can
> > not be re-registered in the kexec kernel because they are still seen as
> > busy by xenstore.
> 
> So this is the PV or HVM guest doing the kexec?

Its for HVM guests with PV drivers.

Olaf

[PATCH 3/6] xen/hvm kexec: unregister memory/target watch in xen-balloon.c

[Olaf Hering]

[-- Attachment #1: xen.syscore_ops.xen-balloon.unregister_watch.patch --]
[-- Type: text/plain, Size: 1270 bytes --]

Unregister the memory/target watch during kexec. The watche can not be
re-registered in the kexec kernel because it is still seen as busy by
xenstore.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 drivers/xen/xen-balloon.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

Index: linux-3.0/drivers/xen/xen-balloon.c
===================================================================
--- linux-3.0.orig/drivers/xen/xen-balloon.c
+++ linux-3.0/drivers/xen/xen-balloon.c
@@ -34,6 +34,7 @@
 #include <linux/module.h>
 #include <linux/sysdev.h>
 #include <linux/capability.h>
+#include <linux/syscore_ops.h>
 
 #include <xen/xen.h>
 #include <xen/interface/xen.h>
@@ -91,6 +92,15 @@ static struct notifier_block xenstore_no
 	.notifier_call = balloon_init_watcher,
 };
 
+static void xen_balloon_shutdown_watcher(void)
+{
+	unregister_xenbus_watch(&target_watch);
+}
+
+static struct syscore_ops xen_balloon_watcher_syscore_ops = {
+	.shutdown = xen_balloon_shutdown_watcher,
+};
+
 static int __init balloon_init(void)
 {
 	if (!xen_domain())
@@ -100,6 +110,7 @@ static int __init balloon_init(void)
 
 	register_balloon(&balloon_sysdev);
 	register_xenstore_notifier(&xenstore_notifier);
+	register_syscore_ops(&xen_balloon_watcher_syscore_ops);
 
 	return 0;
 }

Re: [PATCH 3/6] xen/hvm kexec: unregister memory/target watch in xen-balloon.c

[Konrad Rzeszutek Wilk]

On Tue, Jul 26, 2011 at 01:52:12PM +0200, Olaf Hering wrote:
> Unregister the memory/target watch during kexec. The watche can not be

watche? watcher I think?

> re-registered in the kexec kernel because it is still seen as busy by
> xenstore.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> ---
>  drivers/xen/xen-balloon.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Index: linux-3.0/drivers/xen/xen-balloon.c
> ===================================================================
> --- linux-3.0.orig/drivers/xen/xen-balloon.c
> +++ linux-3.0/drivers/xen/xen-balloon.c
> @@ -34,6 +34,7 @@
>  #include <linux/module.h>
>  #include <linux/sysdev.h>
>  #include <linux/capability.h>
> +#include <linux/syscore_ops.h>
>  
>  #include <xen/xen.h>
>  #include <xen/interface/xen.h>
> @@ -91,6 +92,15 @@ static struct notifier_block xenstore_no
>  	.notifier_call = balloon_init_watcher,
>  };
>  
> +static void xen_balloon_shutdown_watcher(void)
> +{
> +	unregister_xenbus_watch(&target_watch);
> +}
> +
> +static struct syscore_ops xen_balloon_watcher_syscore_ops = {
> +	.shutdown = xen_balloon_shutdown_watcher,
> +};
> +
>  static int __init balloon_init(void)
>  {
>  	if (!xen_domain())
> @@ -100,6 +110,7 @@ static int __init balloon_init(void)
>  
>  	register_balloon(&balloon_sysdev);
>  	register_xenstore_notifier(&xenstore_notifier);
> +	register_syscore_ops(&xen_balloon_watcher_syscore_ops);
>  
>  	return 0;
>  }
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[PATCH 4/6] xen/hvm kexec: unbind debugirq during reboot

[Olaf Hering]

[-- Attachment #1: xen.syscore_ops.smp.unbind-xen_debug_irq.patch --]
[-- Type: text/plain, Size: 1408 bytes --]

Unregister the debugirq during kexec, otherwise the kexec kernel can not
bind to the still registered virq.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 arch/x86/xen/smp.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Index: linux-3.0/arch/x86/xen/smp.c
===================================================================
--- linux-3.0.orig/arch/x86/xen/smp.c
+++ linux-3.0/arch/x86/xen/smp.c
@@ -16,6 +16,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include <linux/smp.h>
+#include <linux/syscore_ops.h>
 
 #include <asm/paravirt.h>
 #include <asm/desc.h>
@@ -45,6 +46,21 @@ static DEFINE_PER_CPU(int, xen_debug_irq
 static irqreturn_t xen_call_function_interrupt(int irq, void *dev_id);
 static irqreturn_t xen_call_function_single_interrupt(int irq, void *dev_id);
 
+static void xen_hvn_smp_shutdown(void)
+{
+	int cpu;
+	for_each_online_cpu(cpu) {
+		if (per_cpu(xen_debug_irq, cpu) < 0)
+			continue;
+		unbind_from_irqhandler(per_cpu(xen_debug_irq, cpu), NULL);
+		per_cpu(xen_debug_irq, cpu) = -1;
+	}
+}
+
+static struct syscore_ops xen_hvn_smp_syscore_ops = {
+	.shutdown = xen_hvn_smp_shutdown,
+};
+
 /*
  * Reschedule call back.
  */
@@ -525,6 +541,7 @@ static void __init xen_hvm_smp_prepare_c
 		return;
 	xen_init_lock_cpu(0);
 	xen_init_spinlocks();
+	register_syscore_ops(&xen_hvn_smp_syscore_ops);
 }
 
 static int __cpuinit xen_hvm_cpu_up(unsigned int cpu)

Re: [PATCH 4/6] xen/hvm kexec: unbind debugirq during reboot

[Konrad Rzeszutek Wilk]

On Tue, Jul 26, 2011 at 01:52:13PM +0200, Olaf Hering wrote:
> Unregister the debugirq during kexec, otherwise the kexec kernel can not
> bind to the still registered virq.

What about the other ones? Say spinlock and the timer ones?

> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> ---
>  arch/x86/xen/smp.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> Index: linux-3.0/arch/x86/xen/smp.c
> ===================================================================
> --- linux-3.0.orig/arch/x86/xen/smp.c
> +++ linux-3.0/arch/x86/xen/smp.c
> @@ -16,6 +16,7 @@
>  #include <linux/err.h>
>  #include <linux/slab.h>
>  #include <linux/smp.h>
> +#include <linux/syscore_ops.h>
>  
>  #include <asm/paravirt.h>
>  #include <asm/desc.h>
> @@ -45,6 +46,21 @@ static DEFINE_PER_CPU(int, xen_debug_irq
>  static irqreturn_t xen_call_function_interrupt(int irq, void *dev_id);
>  static irqreturn_t xen_call_function_single_interrupt(int irq, void *dev_id);
>  
> +static void xen_hvn_smp_shutdown(void)
> +{
> +	int cpu;
> +	for_each_online_cpu(cpu) {
> +		if (per_cpu(xen_debug_irq, cpu) < 0)
> +			continue;
> +		unbind_from_irqhandler(per_cpu(xen_debug_irq, cpu), NULL);
> +		per_cpu(xen_debug_irq, cpu) = -1;
> +	}
> +}
> +
> +static struct syscore_ops xen_hvn_smp_syscore_ops = {
> +	.shutdown = xen_hvn_smp_shutdown,
> +};
> +
>  /*
>   * Reschedule call back.
>   */
> @@ -525,6 +541,7 @@ static void __init xen_hvm_smp_prepare_c
>  		return;
>  	xen_init_lock_cpu(0);
>  	xen_init_spinlocks();
> +	register_syscore_ops(&xen_hvn_smp_syscore_ops);
>  }
>  
>  static int __cpuinit xen_hvm_cpu_up(unsigned int cpu)
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[PATCH 5/6] xen/hvm kexec: unregister timer interrupt during reboot

[Olaf Hering]

[-- Attachment #1: xen.syscore_ops.time.unbind_timer-irq.patch --]
[-- Type: text/plain, Size: 1883 bytes --]

The kexec kernel will crash because the timer interrupt is already
registerd with EVTCHNOP_bind_virq.  Unbind the event channel during
shutdown so that the kexec kernel can reregister the interrupt.

Signed-off-by: Olaf Hering <olaf@aepfle.de>

---
 arch/x86/xen/time.c |   27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

Index: linux-3.0/arch/x86/xen/time.c
===================================================================
--- linux-3.0.orig/arch/x86/xen/time.c
+++ linux-3.0/arch/x86/xen/time.c
@@ -14,6 +14,7 @@
 #include <linux/kernel_stat.h>
 #include <linux/math64.h>
 #include <linux/gfp.h>
+#include <linux/syscore_ops.h>
 
 #include <asm/pvclock.h>
 #include <asm/xen/hypervisor.h>
@@ -405,12 +406,20 @@ void xen_setup_timer(int cpu)
 	evt->irq = irq;
 }
 
-void xen_teardown_timer(int cpu)
+static void xen_unbind_timer(int cpu)
 {
 	struct clock_event_device *evt;
-	BUG_ON(cpu == 0);
 	evt = &per_cpu(xen_clock_events, cpu);
-	unbind_from_irqhandler(evt->irq, NULL);
+	if (evt->irq >= 0) {
+		unbind_from_irqhandler(evt->irq, NULL);
+		evt->irq = -1;
+	}
+}
+
+void xen_teardown_timer(int cpu)
+{
+	BUG_ON(cpu == 0);
+	xen_unbind_timer(cpu);
 }
 
 void xen_setup_cpu_clockevents(void)
@@ -478,6 +487,17 @@ void __init xen_init_time_ops(void)
 }
 
 #ifdef CONFIG_XEN_PVHVM
+static void xen_hvmtimer_shutdown(void)
+{
+	int cpu;
+	for_each_online_cpu(cpu)
+		xen_unbind_timer(cpu);
+}
+
+static struct syscore_ops xen_hvmtimer_syscore_ops = {
+	.shutdown = xen_hvmtimer_shutdown,
+};
+
 static void xen_hvm_setup_cpu_clockevents(void)
 {
 	int cpu = smp_processor_id();
@@ -506,5 +526,6 @@ void __init xen_hvm_init_time_ops(void)
 	x86_platform.calibrate_tsc = xen_tsc_khz;
 	x86_platform.get_wallclock = xen_get_wallclock;
 	x86_platform.set_wallclock = xen_set_wallclock;
+	register_syscore_ops(&xen_hvmtimer_syscore_ops);
 }
 #endif

Re: [PATCH 5/6] xen/hvm kexec: unregister timer interrupt during reboot

[Konrad Rzeszutek Wilk]

On Tue, Jul 26, 2011 at 01:52:14PM +0200, Olaf Hering wrote:
> The kexec kernel will crash because the timer interrupt is already
> registerd with EVTCHNOP_bind_virq.  Unbind the event channel during
> shutdown so that the kexec kernel can reregister the interrupt.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> ---
>  arch/x86/xen/time.c |   27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> Index: linux-3.0/arch/x86/xen/time.c
> ===================================================================
> --- linux-3.0.orig/arch/x86/xen/time.c
> +++ linux-3.0/arch/x86/xen/time.c
> @@ -14,6 +14,7 @@
>  #include <linux/kernel_stat.h>
>  #include <linux/math64.h>
>  #include <linux/gfp.h>
> +#include <linux/syscore_ops.h>
>  
>  #include <asm/pvclock.h>
>  #include <asm/xen/hypervisor.h>
> @@ -405,12 +406,20 @@ void xen_setup_timer(int cpu)
>  	evt->irq = irq;
>  }
>  
> -void xen_teardown_timer(int cpu)
> +static void xen_unbind_timer(int cpu)
>  {
>  	struct clock_event_device *evt;
> -	BUG_ON(cpu == 0);
>  	evt = &per_cpu(xen_clock_events, cpu);
> -	unbind_from_irqhandler(evt->irq, NULL);
> +	if (evt->irq >= 0) {
> +		unbind_from_irqhandler(evt->irq, NULL);
> +		evt->irq = -1;
> +	}
> +}
> +
> +void xen_teardown_timer(int cpu)
> +{
> +	BUG_ON(cpu == 0);

Why the BUG? Ah you just copied it from xen_unbind_timer.
Hm, I wonder if we actually need that BUG_ON.

> +	xen_unbind_timer(cpu);
>  }
>  
>  void xen_setup_cpu_clockevents(void)
> @@ -478,6 +487,17 @@ void __init xen_init_time_ops(void)
>  }
>  
>  #ifdef CONFIG_XEN_PVHVM
> +static void xen_hvmtimer_shutdown(void)
> +{
> +	int cpu;
> +	for_each_online_cpu(cpu)
> +		xen_unbind_timer(cpu);
> +}
> +
> +static struct syscore_ops xen_hvmtimer_syscore_ops = {
> +	.shutdown = xen_hvmtimer_shutdown,
> +};
> +
>  static void xen_hvm_setup_cpu_clockevents(void)
>  {
>  	int cpu = smp_processor_id();
> @@ -506,5 +526,6 @@ void __init xen_hvm_init_time_ops(void)
>  	x86_platform.calibrate_tsc = xen_tsc_khz;
>  	x86_platform.get_wallclock = xen_get_wallclock;
>  	x86_platform.set_wallclock = xen_set_wallclock;
> +	register_syscore_ops(&xen_hvmtimer_syscore_ops);
>  }
>  #endif
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 5/6] xen/hvm kexec: unregister timer interrupt during reboot

[Olaf Hering]

On Tue, Jul 26, Konrad Rzeszutek Wilk wrote:

> On Tue, Jul 26, 2011 at 01:52:14PM +0200, Olaf Hering wrote:
> > The kexec kernel will crash because the timer interrupt is already
> > registerd with EVTCHNOP_bind_virq.  Unbind the event channel during
> > shutdown so that the kexec kernel can reregister the interrupt.
> > 
> > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > 
> > ---
> >  arch/x86/xen/time.c |   27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> > 
> > Index: linux-3.0/arch/x86/xen/time.c
> > ===================================================================
> > --- linux-3.0.orig/arch/x86/xen/time.c
> > +++ linux-3.0/arch/x86/xen/time.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/kernel_stat.h>
> >  #include <linux/math64.h>
> >  #include <linux/gfp.h>
> > +#include <linux/syscore_ops.h>
> >  
> >  #include <asm/pvclock.h>
> >  #include <asm/xen/hypervisor.h>
> > @@ -405,12 +406,20 @@ void xen_setup_timer(int cpu)
> >  	evt->irq = irq;
> >  }
> >  
> > -void xen_teardown_timer(int cpu)
> > +static void xen_unbind_timer(int cpu)
> >  {
> >  	struct clock_event_device *evt;
> > -	BUG_ON(cpu == 0);
> >  	evt = &per_cpu(xen_clock_events, cpu);
> > -	unbind_from_irqhandler(evt->irq, NULL);
> > +	if (evt->irq >= 0) {
> > +		unbind_from_irqhandler(evt->irq, NULL);
> > +		evt->irq = -1;
> > +	}
> > +}
> > +
> > +void xen_teardown_timer(int cpu)
> > +{
> > +	BUG_ON(cpu == 0);
> 
> Why the BUG? Ah you just copied it from xen_unbind_timer.
> Hm, I wonder if we actually need that BUG_ON.

A quick grep did not show up the place where
/sys/devices/system/cpu/cpu0/online would have been created. Since that
file is missing in my guest I think cpu0 can not be shutdown, So that
BUG_ON() can probably go.

Olaf

Re: [PATCH 5/6] xen/hvm kexec: unregister timer interrupt during reboot

[Konrad Rzeszutek Wilk]

On Wed, Jul 27, 2011 at 04:05:12PM +0200, Olaf Hering wrote:
> On Tue, Jul 26, Konrad Rzeszutek Wilk wrote:
> 
> > On Tue, Jul 26, 2011 at 01:52:14PM +0200, Olaf Hering wrote:
> > > The kexec kernel will crash because the timer interrupt is already
> > > registerd with EVTCHNOP_bind_virq.  Unbind the event channel during
> > > shutdown so that the kexec kernel can reregister the interrupt.
> > > 
> > > Signed-off-by: Olaf Hering <olaf@aepfle.de>
> > > 
> > > ---
> > >  arch/x86/xen/time.c |   27 ++++++++++++++++++++++++---
> > >  1 file changed, 24 insertions(+), 3 deletions(-)
> > > 
> > > Index: linux-3.0/arch/x86/xen/time.c
> > > ===================================================================
> > > --- linux-3.0.orig/arch/x86/xen/time.c
> > > +++ linux-3.0/arch/x86/xen/time.c
> > > @@ -14,6 +14,7 @@
> > >  #include <linux/kernel_stat.h>
> > >  #include <linux/math64.h>
> > >  #include <linux/gfp.h>
> > > +#include <linux/syscore_ops.h>
> > >  
> > >  #include <asm/pvclock.h>
> > >  #include <asm/xen/hypervisor.h>
> > > @@ -405,12 +406,20 @@ void xen_setup_timer(int cpu)
> > >  	evt->irq = irq;
> > >  }
> > >  
> > > -void xen_teardown_timer(int cpu)
> > > +static void xen_unbind_timer(int cpu)
> > >  {
> > >  	struct clock_event_device *evt;
> > > -	BUG_ON(cpu == 0);
> > >  	evt = &per_cpu(xen_clock_events, cpu);
> > > -	unbind_from_irqhandler(evt->irq, NULL);
> > > +	if (evt->irq >= 0) {
> > > +		unbind_from_irqhandler(evt->irq, NULL);
> > > +		evt->irq = -1;
> > > +	}
> > > +}
> > > +
> > > +void xen_teardown_timer(int cpu)
> > > +{
> > > +	BUG_ON(cpu == 0);
> > 
> > Why the BUG? Ah you just copied it from xen_unbind_timer.
> > Hm, I wonder if we actually need that BUG_ON.
> 
> A quick grep did not show up the place where
> /sys/devices/system/cpu/cpu0/online would have been created. Since that
> file is missing in my guest I think cpu0 can not be shutdown, So that
> BUG_ON() can probably go.

ok, Thanks for looking around for that. If you can just send that
BUG_ON() remove check as a seperate cleanup patch it would be much appreciated.
(either before this patch or after it).

> 
> Olaf
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

[PATCH 6/6] xen kexec: reset device state to Initializing during reboot

[Olaf Hering]

[-- Attachment #1: xen.xenbus_dev_shutdown.XenbusStateInitialising.patch --]
[-- Type: text/plain, Size: 1827 bytes --]

During kexec all devices will be shutdown, the backend drivers enter the
Closed state. But in this state the kexec kernel can not connect to the
backend because it expects the devices in InitWait state.
After triggering the Closing event, trigger also the Initializing event
and wait until the backend has changed its state. Without this waiting
the kexec kernel may find a device where a state change is still in
progress.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 drivers/xen/xenbus/xenbus_probe.c |   23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Index: linux-3.0/drivers/xen/xenbus/xenbus_probe.c
===================================================================
--- linux-3.0.orig/drivers/xen/xenbus/xenbus_probe.c
+++ linux-3.0/drivers/xen/xenbus/xenbus_probe.c
@@ -192,8 +192,19 @@ void xenbus_otherend_changed(struct xenb
 	 * work that can fail e.g., when the rootfs is gone.
 	 */
 	if (system_state > SYSTEM_RUNNING) {
-		if (ignore_on_shutdown && (state == XenbusStateClosing))
-			xenbus_frontend_closed(dev);
+		if (ignore_on_shutdown) {
+			switch (state) {
+			case XenbusStateClosing:
+				xenbus_frontend_closed(dev);
+				break;
+			case XenbusStateInitialising:
+			case XenbusStateInitWait:
+				complete(&dev->down);
+				break;
+			default:
+				break;
+			}
+		}
 		return;
 	}
 
@@ -284,6 +295,14 @@ void xenbus_dev_shutdown(struct device *
 	if (!timeout)
 		printk(KERN_INFO "%s: %s timeout closing device\n",
 		       __func__, dev->nodename);
+
+	if (system_state > SYSTEM_RUNNING) {
+		xenbus_switch_state(dev, XenbusStateInitialising);
+		timeout = wait_for_completion_timeout(&dev->down, timeout);
+		if (!timeout)
+			printk(KERN_INFO "%s: %s timeout initializing device\n",
+			       __func__, dev->nodename);
+	}
  out:
 	put_device(&dev->dev);
 }

Re: [PATCH 6/6] xen kexec: reset device state to Initializing during reboot

[Konrad Rzeszutek Wilk]

On Tue, Jul 26, 2011 at 01:52:15PM +0200, Olaf Hering wrote:
> During kexec all devices will be shutdown, the backend drivers enter the
                                ^^ - get rid of that.

> Closed state. But in this state the kexec kernel can not connect to the
> backend because it expects the devices in InitWait state.
                                        ^- add "to be"
> After triggering the Closing event, trigger also the Initializing event
> and wait until the backend has changed its state. Without this waiting

> the kexec kernel may find a device where a state change is still in
> progress.

Uhh, say that again? Are you saying that after moving to Initializing state
we should allow the allow the state changes to proceed as they would do under
normal circumstances?

> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  drivers/xen/xenbus/xenbus_probe.c |   23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> Index: linux-3.0/drivers/xen/xenbus/xenbus_probe.c
> ===================================================================
> --- linux-3.0.orig/drivers/xen/xenbus/xenbus_probe.c
> +++ linux-3.0/drivers/xen/xenbus/xenbus_probe.c
> @@ -192,8 +192,19 @@ void xenbus_otherend_changed(struct xenb
>  	 * work that can fail e.g., when the rootfs is gone.
>  	 */
>  	if (system_state > SYSTEM_RUNNING) {
> -		if (ignore_on_shutdown && (state == XenbusStateClosing))
> -			xenbus_frontend_closed(dev);
> +		if (ignore_on_shutdown) {
> +			switch (state) {
> +			case XenbusStateClosing:
> +				xenbus_frontend_closed(dev);
> +				break;
> +			case XenbusStateInitialising:
> +			case XenbusStateInitWait:
> +				complete(&dev->down);
> +				break;
> +			default:
> +				break;
> +			}
> +		}
>  		return;
>  	}
>  
> @@ -284,6 +295,14 @@ void xenbus_dev_shutdown(struct device *
>  	if (!timeout)
>  		printk(KERN_INFO "%s: %s timeout closing device\n",
>  		       __func__, dev->nodename);
> +
> +	if (system_state > SYSTEM_RUNNING) {
> +		xenbus_switch_state(dev, XenbusStateInitialising);
> +		timeout = wait_for_completion_timeout(&dev->down, timeout);
> +		if (!timeout)
> +			printk(KERN_INFO "%s: %s timeout initializing device\n",
> +			       __func__, dev->nodename);
> +	}
>   out:
>  	put_device(&dev->dev);
>  }
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 6/6] xen kexec: reset device state to Initializing during reboot

[Stefano Stabellini]

On Tue, 26 Jul 2011, Olaf Hering wrote:
> During kexec all devices will be shutdown, the backend drivers enter the
> Closed state. But in this state the kexec kernel can not connect to the
> backend because it expects the devices in InitWait state.
> After triggering the Closing event, trigger also the Initializing event
> and wait until the backend has changed its state. Without this waiting
> the kexec kernel may find a device where a state change is still in
> progress.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  drivers/xen/xenbus/xenbus_probe.c |   23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> Index: linux-3.0/drivers/xen/xenbus/xenbus_probe.c
> ===================================================================
> --- linux-3.0.orig/drivers/xen/xenbus/xenbus_probe.c
> +++ linux-3.0/drivers/xen/xenbus/xenbus_probe.c
> @@ -192,8 +192,19 @@ void xenbus_otherend_changed(struct xenb
>  	 * work that can fail e.g., when the rootfs is gone.
>  	 */
>  	if (system_state > SYSTEM_RUNNING) {
> -		if (ignore_on_shutdown && (state == XenbusStateClosing))
> -			xenbus_frontend_closed(dev);
> +		if (ignore_on_shutdown) {
> +			switch (state) {
> +			case XenbusStateClosing:
> +				xenbus_frontend_closed(dev);
> +				break;
> +			case XenbusStateInitialising:
> +			case XenbusStateInitWait:
> +				complete(&dev->down);
> +				break;
> +			default:
> +				break;
> +			}
> +		}
>  		return;
>  	}
>  
> @@ -284,6 +295,14 @@ void xenbus_dev_shutdown(struct device *
>  	if (!timeout)
>  		printk(KERN_INFO "%s: %s timeout closing device\n",
>  		       __func__, dev->nodename);
> +
> +	if (system_state > SYSTEM_RUNNING) {
> +		xenbus_switch_state(dev, XenbusStateInitialising);
> +		timeout = wait_for_completion_timeout(&dev->down, timeout);
> +		if (!timeout)
> +			printk(KERN_INFO "%s: %s timeout initializing device\n",
> +			       __func__, dev->nodename);
> +	}
>   out:
>  	put_device(&dev->dev);
>  }

It looks like this code path is going to be triggered every time we shut
down a PV on HVM guest. I think we should avoid going into
XenbusStateInitialising on normal shut down or reboot.

Re: [PATCH 6/6] xen kexec: reset device state to Initializing during reboot

[Olaf Hering]

On Wed, Jul 27, Stefano Stabellini wrote:

> It looks like this code path is going to be triggered every time we shut
> down a PV on HVM guest. I think we should avoid going into
> XenbusStateInitialising on normal shut down or reboot.

There is currently no way to know wether its a reboot via kexec.
That would be a different patch.

Olaf

Re: [PATCH 6/6] xen kexec: reset device state to Initializing during reboot

[Stefano Stabellini]

On Wed, 27 Jul 2011, Olaf Hering wrote:
> On Wed, Jul 27, Stefano Stabellini wrote:
> 
> > It looks like this code path is going to be triggered every time we shut
> > down a PV on HVM guest. I think we should avoid going into
> > XenbusStateInitialising on normal shut down or reboot.
> 
> There is currently no way to know wether its a reboot via kexec.
> That would be a different patch.

Then we need a different patch :)