Skip to contentSkip to conversation

Xen: hibernation is x86-only at the moment

PATCH In review Arnd Bergmann 4 participants 15 messages Apr 29, 2014 – May 7, 2014

Subsystems (5)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Xen: hibernation is x86-only at the moment

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

---

drivers/xen/manage.c +2 -2
index 32f9236..beba1be 100644
@@ -60,7 +60,7 @@ void xen_resume_notifier_unregister(struct notifier_block *nb)
 }
 EXPORT_SYMBOL_GPL(xen_resume_notifier_unregister);
 
-#ifdef CONFIG_HIBERNATE_CALLBACKS
+#if defined(CONFIG_HIBERNATE_CALLBACKS) && defined(CONFIG_X86)
 static void xen_hvm_post_suspend(int cancelled)
 {
 	xen_arch_hvm_post_suspend(cancelled);
@@ -242,7 +242,7 @@ static void shutdown_handler(struct xenbus_watch *watch,
 		{ "poweroff",	do_poweroff },
 		{ "halt",	do_poweroff },
 		{ "reboot",	do_reboot   },
-#ifdef CONFIG_HIBERNATE_CALLBACKS
+#if defined(CONFIG_HIBERNATE_CALLBACKS) && defined(CONFIG_X86)
 		{ "suspend",	do_suspend  },
 #endif
 		{NULL, NULL},
#1 lore raw

PATCH 1/1 Xen: hibernation is x86-only at the moment

Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

You should better do the dependency in Kconfig.

This way, you'll have to update two places each time new architecture is supported...

Thanks,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#2 lore raw
In reply to Arnd Bergmann
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

You should better do the dependency in Kconfig.

This way, you'll have to update two places each time new architecture is supported...

Thanks,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#3 lore raw

PATCH 1/1 Xen: hibernation is x86-only at the moment

In reply to Arnd Bergmann
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

David

#4 lore raw

PATCH 1/1 Xen: hibernation is x86-only at the moment

Quoted 7 lines · David Vrabel
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines
Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Thanks Arnd.

Quoted 6 lines

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

The sched_on hypercall is already implemented on ARM. However SCHEDOP_shutdown, defined as it is now, is unusable on ARM:

/*
* Halt execution of this domain (all VCPUs) and notify the system controller.
* @arg == pointer to sched_shutdown_t structure.
*
* If the sched_shutdown_t reason is SHUTDOWN_suspend then this
* hypercall takes an additional extra argument which should be the
* MFN of the guest's start_info_t.
*
* In addition, which reason is SHUTDOWN_suspend this hypercall
* returns 1 if suspend was cancelled or the domain was merely
* checkpointed, and 0 if it is resuming in a new domain.
*/
#define SCHEDOP_shutdown 2

We don't have a start_info, and even if we had, we wouldn't know the MFN.
I think we should make it available on ARM only if we change the interface making the third argument x86 only. Of course that would impact the linux side implementation too.

#5 lore raw

PATCH 1/1 Xen: hibernation is x86-only at the moment

Quoted 45 lines · Stefano Stabellini
Quoted 7 lines · David Vrabel
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines
Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Thanks Arnd.

Quoted 6 lines

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

The sched_on hypercall is already implemented on ARM. However SCHEDOP_shutdown, defined as it is now, is unusable on ARM:

/*
* Halt execution of this domain (all VCPUs) and notify the system controller.
* @arg == pointer to sched_shutdown_t structure.
*
* If the sched_shutdown_t reason is SHUTDOWN_suspend then this
* hypercall takes an additional extra argument which should be the
* MFN of the guest's start_info_t.
*
* In addition, which reason is SHUTDOWN_suspend this hypercall
* returns 1 if suspend was cancelled or the domain was merely
* checkpointed, and 0 if it is resuming in a new domain.
*/
#define SCHEDOP_shutdown 2

Quoted 5 lines

We don't have a start_info, and even if we had, we wouldn't know the MFN.
I think we should make it available on ARM only if we change the interface making the third argument x86 only. Of course that would impact the linux side implementation too.

We do not want a different number of arguments to this hypercall. Just pass 0 for the MFN. This hypercall docs should be updated to say this.

This doesn't seem any different to x86 HVM where the start_info_mfn parameter is also not relevant.

	if (xen_hvm_domain()) {
		si.arg = 0UL;
		si.pre = NULL;
		si.post = &xen_hvm_post_suspend;
	} else {
		si.arg = virt_to_mfn(xen_start_info);
		si.pre = &xen_pre_suspend;
		si.post = &xen_post_suspend;
	}

Changing this to be !xen_pv_domain() would then do the right thing for arm.

David

#6 lore raw

PATCH 1/1 Xen: hibernation is x86-only at the moment

Quoted 68 lines · David Vrabel
Quoted 45 lines · Stefano Stabellini
Quoted 7 lines · David Vrabel
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines
Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Thanks Arnd.

Quoted 6 lines

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

The sched_on hypercall is already implemented on ARM. However SCHEDOP_shutdown, defined as it is now, is unusable on ARM:

/*
* Halt execution of this domain (all VCPUs) and notify the system controller.
* @arg == pointer to sched_shutdown_t structure.
*
* If the sched_shutdown_t reason is SHUTDOWN_suspend then this
* hypercall takes an additional extra argument which should be the
* MFN of the guest's start_info_t.
*
* In addition, which reason is SHUTDOWN_suspend this hypercall
* returns 1 if suspend was cancelled or the domain was merely
* checkpointed, and 0 if it is resuming in a new domain.
*/
#define SCHEDOP_shutdown 2

Quoted 5 lines

We don't have a start_info, and even if we had, we wouldn't know the MFN.
I think we should make it available on ARM only if we change the interface making the third argument x86 only. Of course that would impact the linux side implementation too.

We do not want a different number of arguments to this hypercall. Just pass 0 for the MFN. This hypercall docs should be updated to say this.

This doesn't seem any different to x86 HVM where the start_info_mfn parameter is also not relevant.

	if (xen_hvm_domain()) {
		si.arg = 0UL;
		si.pre = NULL;
		si.post = &xen_hvm_post_suspend;
	} else {
		si.arg = virt_to_mfn(xen_start_info);
		si.pre = &xen_pre_suspend;
		si.post = &xen_post_suspend;
	}
Quoted 1 line

Changing this to be !xen_pv_domain() would then do the right thing for arm.

xen_hvm_domain returns true on arm so that's not needed. Unfortunately the patch turns out a bit ugly because of the way hypercalls are implemented in assembly on arm and arm64. Also I needed to provide quite a few empty stubs for other xen arch specific functions.

arch/arm/include/asm/xen/hypercall.h +11 -0
index 7704e28..6d6ddc5 100644
@@ -34,6 +34,7 @@
 #define _ASM_ARM_XEN_HYPERCALL_H
 
 #include <xen/interface/xen.h>
+#include <xen/interface/sched.h>
 
 long privcmd_call(unsigned call, unsigned long a1,
 		unsigned long a2, unsigned long a3,
@@ -48,6 +49,16 @@ int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
 int HYPERVISOR_physdev_op(int cmd, void *arg);
 int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
 int HYPERVISOR_tmem_op(void *arg);
+int HYPERVISOR_sched_op_shutdown(int cmd, void *arg, unsigned long unused);
+
+static inline int
+HYPERVISOR_suspend(unsigned long start_info_mfn)
+{
+	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
+
+	/* start_info_mfn is unused on ARM, pass 0 instead */
+	return HYPERVISOR_sched_op_shutdown(SCHEDOP_shutdown, &r, 0);
+}
 
 static inline void
 MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
arch/arm/xen/enlighten.c +12 -0
index b96723e..6d88c18 100644
@@ -339,6 +339,17 @@ static int __init xen_pm_init(void)
 }
 late_initcall(xen_pm_init);
 
+
+/* empty stubs */
+void xen_arch_pre_suspend(void) { }
+void xen_arch_post_suspend(int suspend_cancelled) { }
+void xen_arch_hvm_post_suspend(int suspend_cancelled) { }
+void xen_mm_pin_all(void) { }
+void xen_mm_unpin_all(void) { }
+void xen_timer_resume(void) { }
+void xen_arch_resume(void) { }
+
+
 /* In the hypervisor.S file. */
 EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op);
@@ -351,3 +362,4 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_physdev_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
 EXPORT_SYMBOL_GPL(privcmd_call);
+EXPORT_SYMBOL_GPL(HYPERVISOR_sched_op_shutdown);
arch/arm/xen/hypercall.S +6 -0
index d1cf7b7..7da8837 100644
@@ -102,3 +102,9 @@ ENTRY(privcmd_call)
 	ldm sp!, {r4}
 	mov pc, lr
 ENDPROC(privcmd_call);
+
+ENTRY(HYPERVISOR_sched_op_shutdown)
+	mov r12, #__HYPERVISOR_sched_op
+	__HVC(XEN_IMM)
+	mov pc, lr
+ENDPROC(HYPERVISOR_sched_op_shutdown)
#7 lore raw

PATCH 1/1 Xen: hibernation is x86-only at the moment

Quoted 69 lines · Stefano Stabellini
Quoted 68 lines · David Vrabel
Quoted 45 lines · Stefano Stabellini
Quoted 7 lines · David Vrabel
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines
Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Thanks Arnd.

Quoted 6 lines

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

The sched_on hypercall is already implemented on ARM. However SCHEDOP_shutdown, defined as it is now, is unusable on ARM:

/*
* Halt execution of this domain (all VCPUs) and notify the system controller.
* @arg == pointer to sched_shutdown_t structure.
*
* If the sched_shutdown_t reason is SHUTDOWN_suspend then this
* hypercall takes an additional extra argument which should be the
* MFN of the guest's start_info_t.
*
* In addition, which reason is SHUTDOWN_suspend this hypercall
* returns 1 if suspend was cancelled or the domain was merely
* checkpointed, and 0 if it is resuming in a new domain.
*/
#define SCHEDOP_shutdown 2

Quoted 5 lines

We don't have a start_info, and even if we had, we wouldn't know the MFN.
I think we should make it available on ARM only if we change the interface making the third argument x86 only. Of course that would impact the linux side implementation too.

We do not want a different number of arguments to this hypercall. Just pass 0 for the MFN. This hypercall docs should be updated to say this.

This doesn't seem any different to x86 HVM where the start_info_mfn parameter is also not relevant.

	if (xen_hvm_domain()) {
		si.arg = 0UL;
		si.pre = NULL;
		si.post = &xen_hvm_post_suspend;
	} else {
		si.arg = virt_to_mfn(xen_start_info);
		si.pre = &xen_pre_suspend;
		si.post = &xen_post_suspend;
	}
Quoted 7 lines
Quoted 1 line

Changing this to be !xen_pv_domain() would then do the right thing for arm.

xen_hvm_domain returns true on arm so that's not needed. Unfortunately the patch turns out a bit ugly because of the way hypercalls are implemented in assembly on arm and arm64. Also I needed to provide quite a few empty stubs for other xen arch specific functions.

Thanks. If you want to add a commit message and apply to devel/for-linus-3.16:

Quoted 32 lines
arch/arm/include/asm/xen/hypercall.h +11 -0
@@ -34,6 +34,7 @@
 #define _ASM_ARM_XEN_HYPERCALL_H
 
 #include <xen/interface/xen.h>
+#include <xen/interface/sched.h>
 
 long privcmd_call(unsigned call, unsigned long a1,
 		unsigned long a2, unsigned long a3,
@@ -48,6 +49,16 @@ int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
 int HYPERVISOR_physdev_op(int cmd, void *arg);
 int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
 int HYPERVISOR_tmem_op(void *arg);
+int HYPERVISOR_sched_op_shutdown(int cmd, void *arg, unsigned long unused);
+
+static inline int
+HYPERVISOR_suspend(unsigned long start_info_mfn)
+{
+	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
+
+	/* start_info_mfn is unused on ARM, pass 0 instead */
+	return HYPERVISOR_sched_op_shutdown(SCHEDOP_shutdown, &r, 0);
+}
 
 static inline void
 MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
arch/arm/xen/enlighten.c +0 -0
index b96723e..6d88c18 100644

arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 348

Quoted 10 lines
arch/arm/xen/enlighten.c +7 -0
@@ -339,6 +339,17 @@ static int __init xen_pm_init(void)
 }
 late_initcall(xen_pm_init);
 
+
+/* empty stubs */
+void xen_arch_pre_suspend(void) { }
+void xen_arch_post_suspend(int suspend_cancelled) { }
+void xen_arch_hvm_post_suspend(int suspend_cancelled) { }
+void xen_mm_pin_all(void) { }
+void xen_mm_unpin_all(void) { }

I wonder if the calls to xen_mm_pin_all() and xen_mm_unpin_all() could be moved into the arch_pre_suspend() and arch_post_suspend() calls instead. So there wouldn't need to be stubs provided for arm.

David

#8 lore raw

PATCH 1/1 Xen: hibernation is x86-only at the moment

Quoted 118 lines · David Vrabel
Quoted 69 lines · Stefano Stabellini
Quoted 68 lines · David Vrabel
Quoted 45 lines · Stefano Stabellini
Quoted 7 lines · David Vrabel
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines
Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Thanks Arnd.

Quoted 6 lines

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

The sched_on hypercall is already implemented on ARM. However SCHEDOP_shutdown, defined as it is now, is unusable on ARM:

/*
* Halt execution of this domain (all VCPUs) and notify the system controller.
* @arg == pointer to sched_shutdown_t structure.
*
* If the sched_shutdown_t reason is SHUTDOWN_suspend then this
* hypercall takes an additional extra argument which should be the
* MFN of the guest's start_info_t.
*
* In addition, which reason is SHUTDOWN_suspend this hypercall
* returns 1 if suspend was cancelled or the domain was merely
* checkpointed, and 0 if it is resuming in a new domain.
*/
#define SCHEDOP_shutdown 2

Quoted 5 lines

We don't have a start_info, and even if we had, we wouldn't know the MFN.
I think we should make it available on ARM only if we change the interface making the third argument x86 only. Of course that would impact the linux side implementation too.

We do not want a different number of arguments to this hypercall. Just pass 0 for the MFN. This hypercall docs should be updated to say this.

This doesn't seem any different to x86 HVM where the start_info_mfn parameter is also not relevant.

	if (xen_hvm_domain()) {
		si.arg = 0UL;
		si.pre = NULL;
		si.post = &xen_hvm_post_suspend;
	} else {
		si.arg = virt_to_mfn(xen_start_info);
		si.pre = &xen_pre_suspend;
		si.post = &xen_post_suspend;
	}
Quoted 7 lines
Quoted 1 line

Changing this to be !xen_pv_domain() would then do the right thing for arm.

xen_hvm_domain returns true on arm so that's not needed. Unfortunately the patch turns out a bit ugly because of the way hypercalls are implemented in assembly on arm and arm64. Also I needed to provide quite a few empty stubs for other xen arch specific functions.

Thanks. If you want to add a commit message and apply to devel/for-linus-3.16:

Quoted 35 lines
arch/arm/include/asm/xen/hypercall.h +11 -0
@@ -34,6 +34,7 @@
 #define _ASM_ARM_XEN_HYPERCALL_H
 
 #include <xen/interface/xen.h>
+#include <xen/interface/sched.h>
 
 long privcmd_call(unsigned call, unsigned long a1,
 		unsigned long a2, unsigned long a3,
@@ -48,6 +49,16 @@ int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
 int HYPERVISOR_physdev_op(int cmd, void *arg);
 int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
 int HYPERVISOR_tmem_op(void *arg);
+int HYPERVISOR_sched_op_shutdown(int cmd, void *arg, unsigned long unused);
+
+static inline int
+HYPERVISOR_suspend(unsigned long start_info_mfn)
+{
+	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
+
+	/* start_info_mfn is unused on ARM, pass 0 instead */
+	return HYPERVISOR_sched_op_shutdown(SCHEDOP_shutdown, &r, 0);
+}
 
 static inline void
 MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
arch/arm/xen/enlighten.c +0 -0
index b96723e..6d88c18 100644
@@ -339,6 +339,17 @@ static int __init xen_pm_init(void)
 }
 late_initcall(xen_pm_init);
 
Quoted 11 lines

arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 348

Quoted 7 lines
arch/arm/xen/enlighten.c +7 -0
+
+/* empty stubs */
+void xen_arch_pre_suspend(void) { }
+void xen_arch_post_suspend(int suspend_cancelled) { }
+void xen_arch_hvm_post_suspend(int suspend_cancelled) { }
+void xen_mm_pin_all(void) { }
+void xen_mm_unpin_all(void) { }

I wonder if the calls to xen_mm_pin_all() and xen_mm_unpin_all() could be moved into the arch_pre_suspend() and arch_post_suspend() calls instead. So there wouldn't need to be stubs provided for arm.

This is how it looks, including the arm64 change:

---

arm,arm64/xen: introduce HYPERVISOR_suspend

Introduce HYPERVISOR_suspend: it is a call to sched_op with an additional argument that is always 0 on arm and arm64.

Introduce a few additional empty stubs for Xen arch specific functions called by drivers/xen/manage.c.

arch/arm/include/asm/xen/hypercall.h +11 -0
index 7658150..b5827e6 100644
@@ -34,6 +34,7 @@
 #define _ASM_ARM_XEN_HYPERCALL_H
 
 #include <xen/interface/xen.h>
+#include <xen/interface/sched.h>
 
 long privcmd_call(unsigned call, unsigned long a1,
 		unsigned long a2, unsigned long a3,
@@ -49,6 +50,16 @@ int HYPERVISOR_physdev_op(int cmd, void *arg);
 int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
 int HYPERVISOR_tmem_op(void *arg);
 int HYPERVISOR_multicall(struct multicall_entry *calls, uint32_t nr);
+int HYPERVISOR_sched_op_shutdown(int cmd, void *arg, unsigned long unused);
+
+static inline int
+HYPERVISOR_suspend(unsigned long start_info_mfn)
+{
+	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
+
+	/* start_info_mfn is unused on ARM, pass 0 instead */
+	return HYPERVISOR_sched_op_shutdown(SCHEDOP_shutdown, &r, 0);
+}
 
 static inline void
 MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
arch/arm/xen/enlighten.c +12 -0
index b96723e..6d88c18 100644
@@ -339,6 +339,17 @@ static int __init xen_pm_init(void)
 }
 late_initcall(xen_pm_init);
 
+
+/* empty stubs */
+void xen_arch_pre_suspend(void) { }
+void xen_arch_post_suspend(int suspend_cancelled) { }
+void xen_arch_hvm_post_suspend(int suspend_cancelled) { }
+void xen_mm_pin_all(void) { }
+void xen_mm_unpin_all(void) { }
+void xen_timer_resume(void) { }
+void xen_arch_resume(void) { }
+
+
 /* In the hypervisor.S file. */
 EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op);
@@ -351,3 +362,4 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_physdev_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
 EXPORT_SYMBOL_GPL(privcmd_call);
+EXPORT_SYMBOL_GPL(HYPERVISOR_sched_op_shutdown);
arch/arm/xen/hypercall.S +6 -0
index 44e3a5f..2bec3fc 100644
@@ -103,3 +103,9 @@ ENTRY(privcmd_call)
 	ldm sp!, {r4}
 	mov pc, lr
 ENDPROC(privcmd_call);
+
+ENTRY(HYPERVISOR_sched_op_shutdown)
+	mov r12, #__HYPERVISOR_sched_op
+	__HVC(XEN_IMM)
+	mov pc, lr
+ENDPROC(HYPERVISOR_sched_op_shutdown)
arch/arm64/xen/hypercall.S +6 -0
index 8bbe940..1fb8cab 100644
@@ -92,3 +92,9 @@ ENTRY(privcmd_call)
 	hvc XEN_IMM
 	ret
 ENDPROC(privcmd_call);
+
+ENTRY(HYPERVISOR_sched_op_shutdown)
+	mov x16, #__HYPERVISOR_sched_op
+	hvc XEN_IMM
+	ret
+ENDPROC(HYPERVISOR_sched_op_shutdown)
#9 lore raw
In reply to David Vrabel
Quoted 118 lines · David Vrabel
Quoted 69 lines · Stefano Stabellini
Quoted 68 lines · David Vrabel
Quoted 45 lines · Stefano Stabellini
Quoted 7 lines · David Vrabel
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines
Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Thanks Arnd.

Quoted 6 lines

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

The sched_on hypercall is already implemented on ARM. However SCHEDOP_shutdown, defined as it is now, is unusable on ARM:

/*
* Halt execution of this domain (all VCPUs) and notify the system controller.
* @arg == pointer to sched_shutdown_t structure.
*
* If the sched_shutdown_t reason is SHUTDOWN_suspend then this
* hypercall takes an additional extra argument which should be the
* MFN of the guest's start_info_t.
*
* In addition, which reason is SHUTDOWN_suspend this hypercall
* returns 1 if suspend was cancelled or the domain was merely
* checkpointed, and 0 if it is resuming in a new domain.
*/
#define SCHEDOP_shutdown 2

Quoted 5 lines

We don't have a start_info, and even if we had, we wouldn't know the MFN.
I think we should make it available on ARM only if we change the interface making the third argument x86 only. Of course that would impact the linux side implementation too.

We do not want a different number of arguments to this hypercall. Just pass 0 for the MFN. This hypercall docs should be updated to say this.

This doesn't seem any different to x86 HVM where the start_info_mfn parameter is also not relevant.

	if (xen_hvm_domain()) {
		si.arg = 0UL;
		si.pre = NULL;
		si.post = &xen_hvm_post_suspend;
	} else {
		si.arg = virt_to_mfn(xen_start_info);
		si.pre = &xen_pre_suspend;
		si.post = &xen_post_suspend;
	}
Quoted 7 lines
Quoted 1 line

Changing this to be !xen_pv_domain() would then do the right thing for arm.

xen_hvm_domain returns true on arm so that's not needed. Unfortunately the patch turns out a bit ugly because of the way hypercalls are implemented in assembly on arm and arm64. Also I needed to provide quite a few empty stubs for other xen arch specific functions.

Thanks. If you want to add a commit message and apply to devel/for-linus-3.16:

Quoted 35 lines
arch/arm/include/asm/xen/hypercall.h +11 -0
@@ -34,6 +34,7 @@
 #define _ASM_ARM_XEN_HYPERCALL_H
 
 #include <xen/interface/xen.h>
+#include <xen/interface/sched.h>
 
 long privcmd_call(unsigned call, unsigned long a1,
 		unsigned long a2, unsigned long a3,
@@ -48,6 +49,16 @@ int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
 int HYPERVISOR_physdev_op(int cmd, void *arg);
 int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
 int HYPERVISOR_tmem_op(void *arg);
+int HYPERVISOR_sched_op_shutdown(int cmd, void *arg, unsigned long unused);
+
+static inline int
+HYPERVISOR_suspend(unsigned long start_info_mfn)
+{
+	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
+
+	/* start_info_mfn is unused on ARM, pass 0 instead */
+	return HYPERVISOR_sched_op_shutdown(SCHEDOP_shutdown, &r, 0);
+}
 
 static inline void
 MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
arch/arm/xen/enlighten.c +0 -0
index b96723e..6d88c18 100644
@@ -339,6 +339,17 @@ static int __init xen_pm_init(void)
 }
 late_initcall(xen_pm_init);
 
Quoted 11 lines

arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 348

Quoted 7 lines
arch/arm/xen/enlighten.c +7 -0
+
+/* empty stubs */
+void xen_arch_pre_suspend(void) { }
+void xen_arch_post_suspend(int suspend_cancelled) { }
+void xen_arch_hvm_post_suspend(int suspend_cancelled) { }
+void xen_mm_pin_all(void) { }
+void xen_mm_unpin_all(void) { }

I wonder if the calls to xen_mm_pin_all() and xen_mm_unpin_all() could be moved into the arch_pre_suspend() and arch_post_suspend() calls instead. So there wouldn't need to be stubs provided for arm.

This is how it looks, including the arm64 change:

---

arm,arm64/xen: introduce HYPERVISOR_suspend

Introduce HYPERVISOR_suspend: it is a call to sched_op with an additional argument that is always 0 on arm and arm64.

Introduce a few additional empty stubs for Xen arch specific functions called by drivers/xen/manage.c.

arch/arm/include/asm/xen/hypercall.h +11 -0
index 7658150..b5827e6 100644
@@ -34,6 +34,7 @@
 #define _ASM_ARM_XEN_HYPERCALL_H
 
 #include <xen/interface/xen.h>
+#include <xen/interface/sched.h>
 
 long privcmd_call(unsigned call, unsigned long a1,
 		unsigned long a2, unsigned long a3,
@@ -49,6 +50,16 @@ int HYPERVISOR_physdev_op(int cmd, void *arg);
 int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
 int HYPERVISOR_tmem_op(void *arg);
 int HYPERVISOR_multicall(struct multicall_entry *calls, uint32_t nr);
+int HYPERVISOR_sched_op_shutdown(int cmd, void *arg, unsigned long unused);
+
+static inline int
+HYPERVISOR_suspend(unsigned long start_info_mfn)
+{
+	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
+
+	/* start_info_mfn is unused on ARM, pass 0 instead */
+	return HYPERVISOR_sched_op_shutdown(SCHEDOP_shutdown, &r, 0);
+}
 
 static inline void
 MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
arch/arm/xen/enlighten.c +12 -0
index b96723e..6d88c18 100644
@@ -339,6 +339,17 @@ static int __init xen_pm_init(void)
 }
 late_initcall(xen_pm_init);
 
+
+/* empty stubs */
+void xen_arch_pre_suspend(void) { }
+void xen_arch_post_suspend(int suspend_cancelled) { }
+void xen_arch_hvm_post_suspend(int suspend_cancelled) { }
+void xen_mm_pin_all(void) { }
+void xen_mm_unpin_all(void) { }
+void xen_timer_resume(void) { }
+void xen_arch_resume(void) { }
+
+
 /* In the hypervisor.S file. */
 EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op);
@@ -351,3 +362,4 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_physdev_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
 EXPORT_SYMBOL_GPL(privcmd_call);
+EXPORT_SYMBOL_GPL(HYPERVISOR_sched_op_shutdown);
arch/arm/xen/hypercall.S +6 -0
index 44e3a5f..2bec3fc 100644
@@ -103,3 +103,9 @@ ENTRY(privcmd_call)
 	ldm sp!, {r4}
 	mov pc, lr
 ENDPROC(privcmd_call);
+
+ENTRY(HYPERVISOR_sched_op_shutdown)
+	mov r12, #__HYPERVISOR_sched_op
+	__HVC(XEN_IMM)
+	mov pc, lr
+ENDPROC(HYPERVISOR_sched_op_shutdown)
arch/arm64/xen/hypercall.S +6 -0
index 8bbe940..1fb8cab 100644
@@ -92,3 +92,9 @@ ENTRY(privcmd_call)
 	hvc XEN_IMM
 	ret
 ENDPROC(privcmd_call);
+
+ENTRY(HYPERVISOR_sched_op_shutdown)
+	mov x16, #__HYPERVISOR_sched_op
+	hvc XEN_IMM
+	ret
+ENDPROC(HYPERVISOR_sched_op_shutdown)
Reviewed-by on v1 1/1
#10 lore raw
In reply to Stefano Stabellini
Quoted 69 lines · Stefano Stabellini
Quoted 68 lines · David Vrabel
Quoted 45 lines · Stefano Stabellini
Quoted 7 lines · David Vrabel
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines
Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Thanks Arnd.

Quoted 6 lines

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

The sched_on hypercall is already implemented on ARM. However SCHEDOP_shutdown, defined as it is now, is unusable on ARM:

/*
* Halt execution of this domain (all VCPUs) and notify the system controller.
* @arg == pointer to sched_shutdown_t structure.
*
* If the sched_shutdown_t reason is SHUTDOWN_suspend then this
* hypercall takes an additional extra argument which should be the
* MFN of the guest's start_info_t.
*
* In addition, which reason is SHUTDOWN_suspend this hypercall
* returns 1 if suspend was cancelled or the domain was merely
* checkpointed, and 0 if it is resuming in a new domain.
*/
#define SCHEDOP_shutdown 2

Quoted 5 lines

We don't have a start_info, and even if we had, we wouldn't know the MFN.
I think we should make it available on ARM only if we change the interface making the third argument x86 only. Of course that would impact the linux side implementation too.

We do not want a different number of arguments to this hypercall. Just pass 0 for the MFN. This hypercall docs should be updated to say this.

This doesn't seem any different to x86 HVM where the start_info_mfn parameter is also not relevant.

	if (xen_hvm_domain()) {
		si.arg = 0UL;
		si.pre = NULL;
		si.post = &xen_hvm_post_suspend;
	} else {
		si.arg = virt_to_mfn(xen_start_info);
		si.pre = &xen_pre_suspend;
		si.post = &xen_post_suspend;
	}
Quoted 7 lines
Quoted 1 line

Changing this to be !xen_pv_domain() would then do the right thing for arm.

xen_hvm_domain returns true on arm so that's not needed. Unfortunately the patch turns out a bit ugly because of the way hypercalls are implemented in assembly on arm and arm64. Also I needed to provide quite a few empty stubs for other xen arch specific functions.

Thanks. If you want to add a commit message and apply to devel/for-linus-3.16:

Quoted 32 lines
arch/arm/include/asm/xen/hypercall.h +11 -0
@@ -34,6 +34,7 @@
 #define _ASM_ARM_XEN_HYPERCALL_H
 
 #include <xen/interface/xen.h>
+#include <xen/interface/sched.h>
 
 long privcmd_call(unsigned call, unsigned long a1,
 		unsigned long a2, unsigned long a3,
@@ -48,6 +49,16 @@ int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
 int HYPERVISOR_physdev_op(int cmd, void *arg);
 int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
 int HYPERVISOR_tmem_op(void *arg);
+int HYPERVISOR_sched_op_shutdown(int cmd, void *arg, unsigned long unused);
+
+static inline int
+HYPERVISOR_suspend(unsigned long start_info_mfn)
+{
+	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
+
+	/* start_info_mfn is unused on ARM, pass 0 instead */
+	return HYPERVISOR_sched_op_shutdown(SCHEDOP_shutdown, &r, 0);
+}
 
 static inline void
 MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
arch/arm/xen/enlighten.c +0 -0
index b96723e..6d88c18 100644

arch/arm/xen/enlighten.c static int __init xen_pm_init(void) · line 348

Quoted 10 lines
arch/arm/xen/enlighten.c +7 -0
@@ -339,6 +339,17 @@ static int __init xen_pm_init(void)
 }
 late_initcall(xen_pm_init);
 
+
+/* empty stubs */
+void xen_arch_pre_suspend(void) { }
+void xen_arch_post_suspend(int suspend_cancelled) { }
+void xen_arch_hvm_post_suspend(int suspend_cancelled) { }
+void xen_mm_pin_all(void) { }
+void xen_mm_unpin_all(void) { }

I wonder if the calls to xen_mm_pin_all() and xen_mm_unpin_all() could be moved into the arch_pre_suspend() and arch_post_suspend() calls instead. So there wouldn't need to be stubs provided for arm.

David

#11 lore raw
In reply to David Vrabel
Quoted 68 lines · David Vrabel
Quoted 45 lines · Stefano Stabellini
Quoted 7 lines · David Vrabel
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines
Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Thanks Arnd.

Quoted 6 lines

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

The sched_on hypercall is already implemented on ARM. However SCHEDOP_shutdown, defined as it is now, is unusable on ARM:

/*
* Halt execution of this domain (all VCPUs) and notify the system controller.
* @arg == pointer to sched_shutdown_t structure.
*
* If the sched_shutdown_t reason is SHUTDOWN_suspend then this
* hypercall takes an additional extra argument which should be the
* MFN of the guest's start_info_t.
*
* In addition, which reason is SHUTDOWN_suspend this hypercall
* returns 1 if suspend was cancelled or the domain was merely
* checkpointed, and 0 if it is resuming in a new domain.
*/
#define SCHEDOP_shutdown 2

Quoted 5 lines

We don't have a start_info, and even if we had, we wouldn't know the MFN.
I think we should make it available on ARM only if we change the interface making the third argument x86 only. Of course that would impact the linux side implementation too.

We do not want a different number of arguments to this hypercall. Just pass 0 for the MFN. This hypercall docs should be updated to say this.

This doesn't seem any different to x86 HVM where the start_info_mfn parameter is also not relevant.

	if (xen_hvm_domain()) {
		si.arg = 0UL;
		si.pre = NULL;
		si.post = &xen_hvm_post_suspend;
	} else {
		si.arg = virt_to_mfn(xen_start_info);
		si.pre = &xen_pre_suspend;
		si.post = &xen_post_suspend;
	}
Quoted 1 line

Changing this to be !xen_pv_domain() would then do the right thing for arm.

xen_hvm_domain returns true on arm so that's not needed. Unfortunately the patch turns out a bit ugly because of the way hypercalls are implemented in assembly on arm and arm64. Also I needed to provide quite a few empty stubs for other xen arch specific functions.

arch/arm/include/asm/xen/hypercall.h +11 -0
index 7704e28..6d6ddc5 100644
@@ -34,6 +34,7 @@
 #define _ASM_ARM_XEN_HYPERCALL_H
 
 #include <xen/interface/xen.h>
+#include <xen/interface/sched.h>
 
 long privcmd_call(unsigned call, unsigned long a1,
 		unsigned long a2, unsigned long a3,
@@ -48,6 +49,16 @@ int HYPERVISOR_memory_op(unsigned int cmd, void *arg);
 int HYPERVISOR_physdev_op(int cmd, void *arg);
 int HYPERVISOR_vcpu_op(int cmd, int vcpuid, void *extra_args);
 int HYPERVISOR_tmem_op(void *arg);
+int HYPERVISOR_sched_op_shutdown(int cmd, void *arg, unsigned long unused);
+
+static inline int
+HYPERVISOR_suspend(unsigned long start_info_mfn)
+{
+	struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
+
+	/* start_info_mfn is unused on ARM, pass 0 instead */
+	return HYPERVISOR_sched_op_shutdown(SCHEDOP_shutdown, &r, 0);
+}
 
 static inline void
 MULTI_update_va_mapping(struct multicall_entry *mcl, unsigned long va,
arch/arm/xen/enlighten.c +12 -0
index b96723e..6d88c18 100644
@@ -339,6 +339,17 @@ static int __init xen_pm_init(void)
 }
 late_initcall(xen_pm_init);
 
+
+/* empty stubs */
+void xen_arch_pre_suspend(void) { }
+void xen_arch_post_suspend(int suspend_cancelled) { }
+void xen_arch_hvm_post_suspend(int suspend_cancelled) { }
+void xen_mm_pin_all(void) { }
+void xen_mm_unpin_all(void) { }
+void xen_timer_resume(void) { }
+void xen_arch_resume(void) { }
+
+
 /* In the hypervisor.S file. */
 EXPORT_SYMBOL_GPL(HYPERVISOR_event_channel_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_grant_table_op);
@@ -351,3 +362,4 @@ EXPORT_SYMBOL_GPL(HYPERVISOR_physdev_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_vcpu_op);
 EXPORT_SYMBOL_GPL(HYPERVISOR_tmem_op);
 EXPORT_SYMBOL_GPL(privcmd_call);
+EXPORT_SYMBOL_GPL(HYPERVISOR_sched_op_shutdown);
arch/arm/xen/hypercall.S +6 -0
index d1cf7b7..7da8837 100644
@@ -102,3 +102,9 @@ ENTRY(privcmd_call)
 	ldm sp!, {r4}
 	mov pc, lr
 ENDPROC(privcmd_call);
+
+ENTRY(HYPERVISOR_sched_op_shutdown)
+	mov r12, #__HYPERVISOR_sched_op
+	__HVC(XEN_IMM)
+	mov pc, lr
+ENDPROC(HYPERVISOR_sched_op_shutdown)
#12 lore raw
In reply to Stefano Stabellini
Quoted 45 lines · Stefano Stabellini
Quoted 7 lines · David Vrabel
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines
Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Thanks Arnd.

Quoted 6 lines

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

The sched_on hypercall is already implemented on ARM. However SCHEDOP_shutdown, defined as it is now, is unusable on ARM:

/*
* Halt execution of this domain (all VCPUs) and notify the system controller.
* @arg == pointer to sched_shutdown_t structure.
*
* If the sched_shutdown_t reason is SHUTDOWN_suspend then this
* hypercall takes an additional extra argument which should be the
* MFN of the guest's start_info_t.
*
* In addition, which reason is SHUTDOWN_suspend this hypercall
* returns 1 if suspend was cancelled or the domain was merely
* checkpointed, and 0 if it is resuming in a new domain.
*/
#define SCHEDOP_shutdown 2

Quoted 5 lines

We don't have a start_info, and even if we had, we wouldn't know the MFN.
I think we should make it available on ARM only if we change the interface making the third argument x86 only. Of course that would impact the linux side implementation too.

We do not want a different number of arguments to this hypercall. Just pass 0 for the MFN. This hypercall docs should be updated to say this.

This doesn't seem any different to x86 HVM where the start_info_mfn parameter is also not relevant.

	if (xen_hvm_domain()) {
		si.arg = 0UL;
		si.pre = NULL;
		si.post = &xen_hvm_post_suspend;
	} else {
		si.arg = virt_to_mfn(xen_start_info);
		si.pre = &xen_pre_suspend;
		si.post = &xen_post_suspend;
	}

Changing this to be !xen_pv_domain() would then do the right thing for arm.

David

#13 lore raw
In reply to David Vrabel
Quoted 7 lines · David Vrabel
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines
Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Thanks Arnd.

Quoted 6 lines

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

The sched_on hypercall is already implemented on ARM. However SCHEDOP_shutdown, defined as it is now, is unusable on ARM:

/*
* Halt execution of this domain (all VCPUs) and notify the system controller.
* @arg == pointer to sched_shutdown_t structure.
*
* If the sched_shutdown_t reason is SHUTDOWN_suspend then this
* hypercall takes an additional extra argument which should be the
* MFN of the guest's start_info_t.
*
* In addition, which reason is SHUTDOWN_suspend this hypercall
* returns 1 if suspend was cancelled or the domain was merely
* checkpointed, and 0 if it is resuming in a new domain.
*/
#define SCHEDOP_shutdown 2

We don't have a start_info, and even if we had, we wouldn't know the MFN.
I think we should make it available on ARM only if we change the interface making the third argument x86 only. Of course that would impact the linux side implementation too.

#14 lore raw
In reply to Arnd Bergmann
Quoted 6 lines · Arnd Bergmann

In commit 603fb42a66499ab "ARM: 8011/1: ARM hibernation / suspend-to-disk", currently in linux-next, the ARM architecture gains support for hibernation (suspend-to-disk). Xen supports this in principle, but only has an architecture specific hypercall defined on x86, which leads to a build error when both hibernation and Xen support are enabled:

Quoted 7 lines

drivers/xen/manage.c:105:2: error: implicit declaration of function 'HYPERVISOR_suspend' [-Werror=implicit-function-declaration]
si->cancelled = HYPERVISOR_suspend(si->arg);

It is probably a good idea to define this hypercall on ARM as well and provide an implementation in the host, but until that is done, this patch helps disable the broken code in the Xen guest by making it depend on CONFIG_X86.

Since this isn't a regression in 3.15-rcX can you provide the appropriate hypercall on ARM. This does not require that there is an implementation on Xen.

When the Xen implementation exists it will be easier to use if the support already exists in Linux.

David