Xen devel - Apr 2010 - [PATCH 6 of 6] PV on HVM suspend\resume

Hi all,
this patch adds suspend\resume support for PV on HVM guests.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---
 arch/x86/xen/enlighten.c                   |    9 ++-
 drivers/xen/features.c                     |    2 +-
 drivers/xen/manage.c                       |   99 ++++++++++++++++++++++++++-
 drivers/xen/platform-pci.c                 |   28 ++++++++-
 drivers/xen/xenbus/xenbus_probe_frontend.c |   29 ++++++++
 include/xen/platform_pci.h                 |    6 ++
 include/xen/xen-ops.h                      |    2 +
 7 files changed, 165 insertions(+), 10 deletions(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 7acb130..98ad573 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1335,12 +1335,13 @@ static int init_hvm_pv_info(int *major, int *minor)
 	return 0;
 }
 
-static void __init init_shared_info(void)
+static void init_shared_info(void)
 {
 	struct xen_add_to_physmap xatp;
-	struct shared_info *shared_info_page;
+	static struct shared_info *shared_info_page = 0;
 
-	shared_info_page = (struct shared_info *) alloc_bootmem_pages(PAGE_SIZE);
+	if (!shared_info_page)
+		shared_info_page = (struct shared_info *) alloc_bootmem_pages(PAGE_SIZE);
 	xatp.domid = DOMID_SELF;
 	xatp.idx = 0;
 	xatp.space = XENMAPSPACE_shared_info;
@@ -1370,7 +1371,7 @@ void do_hvm_pv_evtchn_intr(void)
 	xen_hvm_evtchn_do_upcall(get_irq_regs());
 }
 
-void __init xen_guest_init(void)
+void xen_guest_init(void)
 {
 	int r;
 	int major, minor;
diff --git a/drivers/xen/features.c b/drivers/xen/features.c
index 9e2b64f..99eda16 100644
--- a/drivers/xen/features.c
+++ b/drivers/xen/features.c
@@ -18,7 +18,7 @@
 u8 xen_features[XENFEAT_NR_SUBMAPS * 32] __read_mostly;
 EXPORT_SYMBOL_GPL(xen_features);
 
-void __init xen_setup_features(void)
+void xen_setup_features(void)
 {
 	struct xen_feature_info fi;
 	int i, j;
diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index 5d42d55..756cc10 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -7,12 +7,16 @@
 #include <linux/sysrq.h>
 #include <linux/stop_machine.h>
 #include <linux/freezer.h>
+#include <linux/pci.h>
+#include <linux/cpumask.h>
 
 #include <xen/xenbus.h>
 #include <xen/grant_table.h>
 #include <xen/events.h>
 #include <xen/hvc-console.h>
 #include <xen/xen-ops.h>
+#include <xen/xen.h>
+#include <xen/platform_pci.h>
 
 #include <asm/xen/hypercall.h>
 #include <asm/xen/page.h>
@@ -32,10 +36,30 @@ enum shutdown_state {
 static enum shutdown_state shutting_down = SHUTDOWN_INVALID;
 
 #ifdef CONFIG_PM_SLEEP
-static int xen_suspend(void *data)
+static int xen_hvm_suspend(void *data)
 {
+	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
 	int *cancelled = data;
+
+	BUG_ON(!irqs_disabled());
+
+	*cancelled = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
+
+	xen_guest_init();
+	gnttab_resume();
+
+	if (!*cancelled) {
+		xen_irq_resume();
+		platform_pci_resume();
+	}
+
+	return 0;
+}
+
+static int xen_suspend(void *data)
+{
 	int err;
+	int *cancelled = data;
 
 	BUG_ON(!irqs_disabled());
 
@@ -72,6 +96,58 @@ static int xen_suspend(void *data)
 	return 0;
 }
 
+static void do_hvm_suspend(void)
+{
+	int err;
+	int cancelled = 1;
+
+	shutting_down = SHUTDOWN_SUSPEND;
+
+	err = stop_machine_create();
+	if (err) {
+		printk(KERN_ERR "xen suspend: failed to setup stop_machine %d\n",
err);
+		goto out;
+	}
+
+#ifdef CONFIG_PREEMPT
+	/* If the kernel is preemptible, we need to freeze all the processes
+	   to prevent them from being in the middle of a pagetable update
+	   during suspend. */
+	err = freeze_processes();
+	if (err) {
+		printk(KERN_ERR "xen suspend: freeze failed %d\n", err);
+		goto out_destroy_sm;
+	}
+#endif
+
+	printk(KERN_DEBUG "suspending xenstore... ");
+	xenbus_suspend();
+	printk(KERN_DEBUG "xenstore suspended\n");
+	platform_pci_disable_irq();
+	
+	err = stop_machine(xen_hvm_suspend, &cancelled, cpumask_of(0));
+	if (err) {
+		printk(KERN_ERR "failed to start xen_suspend: %d\n", err);
+		cancelled = 1;
+	}
+
+	platform_pci_enable_irq();
+
+	if (!cancelled) {
+		xen_arch_resume();
+		xenbus_resume();
+	} else
+		xs_suspend_cancel();
+
+	/* Make sure timer events get retriggered on all CPUs */
+	clock_was_set();
+
+	stop_machine_destroy();
+
+out:
+	shutting_down = SHUTDOWN_INVALID;
+}
+
 static void do_suspend(void)
 {
 	int err;
@@ -184,7 +260,10 @@ static void shutdown_handler(struct xenbus_watch *watch,
 		ctrl_alt_del();
 #ifdef CONFIG_PM_SLEEP
 	} else if (strcmp(str, "suspend") == 0) {
-		do_suspend();
+		if (xen_hvm_domain())
+			do_hvm_suspend();
+		else
+			do_suspend();
 #endif
 	} else {
 		printk(KERN_INFO "Ignoring shutdown request: %s\n", str);
@@ -260,7 +339,19 @@ static int shutdown_event(struct notifier_block *notifier,
 	return NOTIFY_DONE;
 }
 
-static int __init setup_shutdown_event(void)
+static int __init __setup_shutdown_event(void)
+{
+	/* Delay initialization in the PV on HVM case */
+	if (xen_hvm_domain())
+		return 0;
+
+	if (!xen_pv_domain())
+		return -ENODEV;
+
+	return xen_setup_shutdown_event();
+}
+
+int xen_setup_shutdown_event(void)
 {
 	static struct notifier_block xenstore_notifier = {
 		.notifier_call = shutdown_event
@@ -270,4 +361,4 @@ static int __init setup_shutdown_event(void)
 	return 0;
 }
 
-subsys_initcall(setup_shutdown_event);
+subsys_initcall(__setup_shutdown_event);
diff --git a/drivers/xen/platform-pci.c b/drivers/xen/platform-pci.c
index cf91d7e..63a2eac 100644
--- a/drivers/xen/platform-pci.c
+++ b/drivers/xen/platform-pci.c
@@ -32,6 +32,7 @@
 #include <xen/xenbus.h>
 #include <xen/events.h>
 #include <xen/hvm.h>
+#include <xen/xen-ops.h>
 
 #define DRV_NAME    "xen-platform-pci"
 
@@ -42,6 +43,8 @@ MODULE_LICENSE("GPL");
 static unsigned long platform_mmio;
 static unsigned long platform_mmio_alloc;
 static unsigned long platform_mmiolen;
+static uint64_t callback_via;
+struct pci_dev *xen_platform_pdev;
 
 unsigned long alloc_xen_mmio(unsigned long len)
 {
@@ -93,13 +96,33 @@ int xen_irq_init(struct pci_dev *pdev)
 			IRQF_TRIGGER_RISING, "xen-platform-pci", pdev);
 }
 
+void platform_pci_disable_irq(void)
+{
+	printk(KERN_DEBUG "platform_pci_disable_irq\n");
+	disable_irq(xen_platform_pdev->irq);
+}
+
+void platform_pci_enable_irq(void)
+{
+	printk(KERN_DEBUG "platform_pci_enable_irq\n");
+	enable_irq(xen_platform_pdev->irq);
+}
+
+void platform_pci_resume(void)
+{
+	if (xen_set_callback_via(callback_via)) {
+		printk("platform_pci_resume failure!\n");
+		return;
+	}
+}
+
 static int __devinit platform_pci_init(struct pci_dev *pdev,
 				       const struct pci_device_id *ent)
 {
 	int i, ret;
 	long ioaddr, iolen;
 	long mmio_addr, mmio_len;
-	uint64_t callback_via;
+	xen_platform_pdev = pdev;
 
 	i = pci_enable_device(pdev);
 	if (i)
@@ -144,6 +167,9 @@ static int __devinit platform_pci_init(struct pci_dev *pdev,
 	if ((ret = xenbus_probe_init()))
 		goto out;
 
+	if ((ret = xen_setup_shutdown_event()))
+		goto out;
+
  out:
 	if (ret) {
 		release_mem_region(mmio_addr, mmio_len);
diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c
b/drivers/xen/xenbus/xenbus_probe_frontend.c
index 77af5c3..0ef64a6 100644
--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -101,6 +101,35 @@ static struct xen_bus_type xenbus_frontend = {
 	},
 };
 
+static int dev_suspend(struct device *dev, void *data)
+{
+	return xenbus_dev_suspend(dev, PMSG_SUSPEND);
+}
+
+void xenbus_suspend(void)
+{
+	DPRINTK("");
+
+	bus_for_each_dev(&xenbus_frontend.bus, NULL, NULL, dev_suspend);
+	xs_suspend();
+}
+EXPORT_SYMBOL_GPL(xenbus_suspend);
+
+static int dev_resume(struct device *dev, void *data)
+{
+	return xenbus_dev_resume(dev);
+}
+
+void xenbus_resume(void)
+{
+	DPRINTK("");
+
+	xs_resume();
+	bus_for_each_dev(&xenbus_frontend.bus, NULL, NULL, dev_resume);
+}
+EXPORT_SYMBOL_GPL(xenbus_resume);
+
+
 static void frontend_changed(struct xenbus_watch *watch,
 			     const char **vec, unsigned int len)
 {
diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h
index 870e7a4..4e2afa3 100644
--- a/include/xen/platform_pci.h
+++ b/include/xen/platform_pci.h
@@ -30,11 +30,17 @@
 
 #ifdef CONFIG_XEN_PLATFORM_PCI
 unsigned long alloc_xen_mmio(unsigned long len);
+void platform_pci_resume(void);
+void platform_pci_disable_irq(void);
+void platform_pci_enable_irq(void);
 #else
 static inline unsigned long alloc_xen_mmio(unsigned long len)
 {
 	return ~0UL;
 }
+static inline void platform_pci_resume(void) {}
+static inline void platform_pci_disable_irq(void) {}
+static inline void platform_pci_enable_irq(void) {}
 #endif
 
 #endif /* _XEN_PLATFORM_PCI_H */
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index c7b3ce9..09ba229 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -25,4 +25,6 @@ int xen_create_contiguous_region(unsigned long vstart,
unsigned int order,
 
 void xen_destroy_contiguous_region(unsigned long vstart, unsigned int order);
 
+int xen_setup_shutdown_event(void);
+
 #endif /* INCLUDE_XEN_OPS_H */
-- 
1.5.4.3


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

On 04/22/2010 08:17 AM, Stefano Stabellini wrote:
> +static void do_hvm_suspend(void)
> +{
> +	int err;
> +	int cancelled = 1;
> +
> +	shutting_down = SHUTDOWN_SUSPEND;
> +
> +	err = stop_machine_create();
> +	if (err) {
> +		printk(KERN_ERR "xen suspend: failed to setup stop_machine
%d\n", err);
> +		goto out;
> +	}
> +
> +#ifdef CONFIG_PREEMPT
> +	/* If the kernel is preemptible, we need to freeze all the processes
> +	   to prevent them from being in the middle of a pagetable update
> +	   during suspend. */
> +	err = freeze_processes();
> +	if (err) {
> +		printk(KERN_ERR "xen suspend: freeze failed %d\n", err);
> +		goto out_destroy_sm;
>   
This won''t compile with CONFIG_PREEMPT enabled, because out_destroy_sm
is missing below.

    J

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

On Sat, 24 Apr 2010, Jeremy Fitzhardinge wrote:
> On 04/22/2010 08:17 AM, Stefano Stabellini wrote:
> > +static void do_hvm_suspend(void)
> > +{
> > +	int err;
> > +	int cancelled = 1;
> > +
> > +	shutting_down = SHUTDOWN_SUSPEND;
> > +
> > +	err = stop_machine_create();
> > +	if (err) {
> > +		printk(KERN_ERR "xen suspend: failed to setup stop_machine
%d\n", err);
> > +		goto out;
> > +	}
> > +
> > +#ifdef CONFIG_PREEMPT
> > +	/* If the kernel is preemptible, we need to freeze all the processes
> > +	   to prevent them from being in the middle of a pagetable update
> > +	   during suspend. */
> > +	err = freeze_processes();
> > +	if (err) {
> > +		printk(KERN_ERR "xen suspend: freeze failed %d\n", err);
> > +		goto out_destroy_sm;
> >   
> 
> This won''t compile with CONFIG_PREEMPT enabled, because
out_destroy_sm
> is missing below.
> 
 
Now it is fixed, however I tried suspend/resume with CONFIG_PREEMPT
enabled and the kernel seems to hang at resume time; I wonder if it
works for pv guests since the suspend/resume code is similiar...


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

On 04/25/2010 04:25 PM, Stefano Stabellini wrote:
>> This won''t compile with CONFIG_PREEMPT enabled, because
out_destroy_sm
>> is missing below.
>>
>>     
>  
> Now it is fixed,
Did you push out the fix (ideally as a new delta changeset on top of the
previous ones, rather than rebaseing them).

>  however I tried suspend/resume with CONFIG_PREEMPT
> enabled and the kernel seems to hang at resume time; I wonder if it
> works for pv guests since the suspend/resume code is similiar...
>   
I think Pasi and IanC tested it quite a bit not too long ago, but its
possible they didn''t test preempt kernels (and it wouldn''t
surprise me
at all if preempt breaks things).

Thanks,
    J


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

On Mon, 26 Apr 2010, Jeremy Fitzhardinge wrote:
> On 04/25/2010 04:25 PM, Stefano Stabellini wrote:
> >> This won''t compile with CONFIG_PREEMPT enabled, because
out_destroy_sm
> >> is missing below.
> >>
> >>     
> >  
> > Now it is fixed,
> 
> Did you push out the fix (ideally as a new delta changeset on top of the
> previous ones, rather than rebaseing them).
> 
Yes but this time I rebased them; next time I''ll keep the fix
separated.
> 
> >  however I tried suspend/resume with CONFIG_PREEMPT
> > enabled and the kernel seems to hang at resume time; I wonder if it
> > works for pv guests since the suspend/resume code is similiar...
> >   
> 
> I think Pasi and IanC tested it quite a bit not too long ago, but its
> possible they didn''t test preempt kernels (and it
wouldn''t surprise me
> at all if preempt breaks things).
> 
all right, more debugging required here...

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

On Tue, Apr 27, 2010 at 06:08:35AM +0100, Stefano Stabellini
wrote:
> On Mon, 26 Apr 2010, Jeremy Fitzhardinge wrote:
> > On 04/25/2010 04:25 PM, Stefano Stabellini wrote:
> > >> This won''t compile with CONFIG_PREEMPT enabled,
because out_destroy_sm
> > >> is missing below.
> > >>
> > >>     
> > >  
> > > Now it is fixed,
> > 
> > Did you push out the fix (ideally as a new delta changeset on top of
the
> > previous ones, rather than rebaseing them).
> > 
> 
> Yes but this time I rebased them; next time I''ll keep the fix
separated.
> 
> > 
> > >  however I tried suspend/resume with CONFIG_PREEMPT
> > > enabled and the kernel seems to hang at resume time; I wonder if
it
> > > works for pv guests since the suspend/resume code is similiar...
> > >   
> > 
> > I think Pasi and IanC tested it quite a bit not too long ago, but its
> > possible they didn''t test preempt kernels (and it
wouldn''t surprise me
> > at all if preempt breaks things).
> > 
> 
> all right, more debugging required here...
>
I was testing a lot with normal PV guests.. kernel .config was based
on standard Fedora kernels:

$ grep -i empt config-2.6.32.11-99.fc12.x86_64
# CONFIG_TREE_PREEMPT_RCU is not set
CONFIG_PREEMPT_NOTIFIERS=y
# CONFIG_PREEMPT_NONE is not set
CONFIG_PREEMPT_VOLUNTARY=y
# CONFIG_PREEMPT is not set


$ grep -i empt config-2.6.32.9-70.fc12.i686.PAE
# CONFIG_TREE_PREEMPT_RCU is not set
CONFIG_PREEMPT_NOTIFIERS=y
# CONFIG_PREEMPT_NONE is not set
CONFIG_PREEMPT_VOLUNTARY=y
# CONFIG_PREEMPT is not set


-- Pasi


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

On Tue, Apr 27, 2010 at 06:08:35AM +0100, Stefano Stabellini
wrote:
> On Mon, 26 Apr 2010, Jeremy Fitzhardinge wrote:
> > On 04/25/2010 04:25 PM, Stefano Stabellini wrote:
> > >> This won''t compile with CONFIG_PREEMPT enabled,
because out_destroy_sm
> > >> is missing below.
> > >>
> > >>     
> > >  
> > > Now it is fixed,
> > 
> > Did you push out the fix (ideally as a new delta changeset on top of
the
> > previous ones, rather than rebaseing them).
> > 
> 
> Yes but this time I rebased them; next time I''ll keep the fix
separated.
> 
> > 
> > >  however I tried suspend/resume with CONFIG_PREEMPT
> > > enabled and the kernel seems to hang at resume time; I wonder if
it
> > > works for pv guests since the suspend/resume code is similiar...
> > >   
> > 
> > I think Pasi and IanC tested it quite a bit not too long ago, but its
> > possible they didn''t test preempt kernels (and it
wouldn''t surprise me
> > at all if preempt breaks things).
> > 
> 
> all right, more debugging required here...
FYI. The Documents/SubmitChecklist mentions:

12: Has been tested with CONFIG_PREEMPT, CONFIG_DEBUG_PREEMPT,
    CONFIG_DEBUG_SLAB, CONFIG_DEBUG_PAGEALLOC, CONFIG_DEBUG_MUTEXES,
    CONFIG_DEBUG_SPINLOCK, CONFIG_DEBUG_SPINLOCK_SLEEP all simultaneously
    enabled.

amongs other things..

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

On 04/26/2010 10:08 PM, Stefano Stabellini wrote:
>> Did you push out the fix (ideally as a new delta changeset on top of
the
>> previous ones, rather than rebaseing them).
>>
>>     
> Yes but this time I rebased them; next time I''ll keep the fix
separated.
>   
That''s awkward because I''ve already merged the original one. 
The merge
was non-trivial because some of the affected code was moved around, so
I''d prefer not to do it again.  Since it looks like the fix is a
one-liner, I''m just going to add it to your original branch.

    J

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

On Tue, 27 Apr 2010, Stefano Stabellini wrote:
> 
> all right, more debugging required here...
> 
I found the issue: I simply forgot to restart the processes at resume
time :)
I pushed the update to

git://xenbits.xen.org/people/sstabellini/linux-pvhvm.git


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