|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH RFC] xen/arm: domain kernel: Small fixes for making suspendable for arm
On Wed, Jul 03, 2013 at 06:16:28PM +0900, Jaeyong Yoo wrote:
> Modify makefile to compile driver/xen/manage.c for arm and implement
> resuming the shared page info. This patch is required for domu kernel
> to test the xen-on-arndale migration.
>
> Since there are lot of missing functions for compiling hibernation mode,
> temporarily I put empty functions in xen/dummy.c, but they are originally
> belong to such as arch/arm/power directories (which is not existing).
> I think there would be any better way...
Since they are both shared by arm and arm64 and x86 could some of this be
in the drivers/xen ? This might mean moving some code from arch/x86?
>
> Signed-off-by: Jaeyong Yoo <jaeyong.yoo@xxxxxxxxxxx>
> ---
> arch/arm/Kconfig | 3 ++
> arch/arm/boot/dts/xenvm-4.2.dts | 2 +-
> arch/arm/xen/Makefile | 2 +-
> arch/arm/xen/dummy.c | 30 ++++++++++++++++
> arch/arm/xen/mmu.c | 12 +++++++
> arch/arm/xen/suspend.c | 76
> +++++++++++++++++++++++++++++++++++++++++
> arch/arm/xen/time.c | 7 ++++
> drivers/xen/Makefile | 2 +-
> drivers/xen/manage.c | 8 +++++
> 9 files changed, 139 insertions(+), 3 deletions(-)
> create mode 100644 arch/arm/xen/dummy.c
> create mode 100644 arch/arm/xen/mmu.c
> create mode 100644 arch/arm/xen/suspend.c
> create mode 100644 arch/arm/xen/time.c
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 2c3bdce..77309f7 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1469,6 +1469,9 @@ config ARCH_NO_VIRT_TO_BUS
> config ISA_DMA_API
> bool
>
> +config ARCH_HIBERNATION_POSSIBLE
> + def_bool y
> +
> config PCI
> bool "PCI support" if MIGHT_HAVE_PCI
> help
> diff --git a/arch/arm/boot/dts/xenvm-4.2.dts b/arch/arm/boot/dts/xenvm-4.2.dts
> index 2f4136b..33df5e6 100644
> --- a/arch/arm/boot/dts/xenvm-4.2.dts
> +++ b/arch/arm/boot/dts/xenvm-4.2.dts
> @@ -17,7 +17,7 @@
>
> chosen {
> /* this field is going to be adjusted by the hypervisor */
> - bootargs = "console=hvc0 root=/dev/xvda";
> + bootargs = "console=hvc0 root=/dev/xvda1 rw init";
Stray patch perhaps?
> };
>
> cpus {
> diff --git a/arch/arm/xen/Makefile b/arch/arm/xen/Makefile
> index 4384103..6fdc47a 100644
> --- a/arch/arm/xen/Makefile
> +++ b/arch/arm/xen/Makefile
> @@ -1 +1 @@
> -obj-y := enlighten.o hypercall.o grant-table.o
> +obj-y := enlighten.o hypercall.o grant-table.o suspend.o
> mmu.o time.o dummy.o
> diff --git a/arch/arm/xen/dummy.c b/arch/arm/xen/dummy.c
> new file mode 100644
> index 0000000..daa949c
> --- /dev/null
> +++ b/arch/arm/xen/dummy.c
> @@ -0,0 +1,30 @@
> +#include <linux/kernel.h>
> +#include <linux/printk.h>
> +
> +void save_processor_state(void)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
Joe Perches made a valiant effort at converting all of the 'printk' to
pr_info/pr_err. Please do use that.
But a better option would be to do:
WARN(1);
As that would also produce a stack-trace to help in diagnosing this.
> +}
> +
> +void restore_processor_state(void)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> +}
But these functions are generic I think? Can't they also be used by
'baremetal' ARM in suspending/resuming? As such shouldn't they be in
a more generic layer?
> +
> +int swsusp_arch_suspend(void)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> + return 0;
> +}
> +
> +int swsusp_arch_resume(void)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> + return 0;
> +}
Ditto for that.
> +
> +int pfn_is_nosave(unsigned long pfn)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> + return 0;
> +}
And that.
And I think you would also need some EXPORT_SYMBOL_GPL.
> diff --git a/arch/arm/xen/mmu.c b/arch/arm/xen/mmu.c
> new file mode 100644
> index 0000000..cc0ccc9
> --- /dev/null
> +++ b/arch/arm/xen/mmu.c
> @@ -0,0 +1,12 @@
> +#include <linux/kernel.h>
> +#include <xen/xen.h>
> +
> +void xen_mm_pin_all(void)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> +}
> +
> +void xen_mm_unpin_all(void)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> +}
A better approach is to make the in include/xen/xen-ops.h a bunch of
#ifdef CONFIG_ARM
void static inline xen_mm_pin_all(void) { }
#else
.. original code.
#endif
> diff --git a/arch/arm/xen/suspend.c b/arch/arm/xen/suspend.c
> new file mode 100644
> index 0000000..946a960
> --- /dev/null
> +++ b/arch/arm/xen/suspend.c
> @@ -0,0 +1,76 @@
> +#include <xen/xen.h>
> +#include <xen/events.h>
> +#include <xen/grant_table.h>
> +#include <xen/hvm.h>
> +#include <xen/interface/vcpu.h>
> +#include <xen/interface/xen.h>
> +#include <xen/interface/memory.h>
> +#include <xen/interface/hvm/params.h>
> +#include <xen/features.h>
> +#include <xen/platform_pci.h>
> +#include <xen/xenbus.h>
> +#include <xen/page.h>
> +#include <xen/xen-ops.h>
> +#include <asm/xen/hypervisor.h>
> +#include <asm/xen/hypercall.h>
> +#include <linux/interrupt.h>
> +#include <linux/irqreturn.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +
> +#include <linux/mm.h>
> +
> +void xen_arch_pre_suspend(void)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> +}
> +
Same thing. Move them in xen-ops.h without any implementation.
> +void xen_arch_hvm_post_suspend(int suspend_cancelled)
> +{
> + if( !suspend_cancelled ) {
> + int cpu;
> + struct xen_add_to_physmap xatp;
> + static struct shared_info *shared_info_page = 0;
> +
> + if( !shared_info_page )
> + shared_info_page = (struct shared_info *)
> + get_zeroed_page(GFP_KERNEL);
> + if (!shared_info_page) {
> + pr_err("not enough memory\n");
Good. you are using pr_err here.
> + return;
> + }
> +
> + xatp.domid = DOMID_SELF;
> + xatp.idx = 0;
> + xatp.space = XENMAPSPACE_shared_info;
> + xatp.gpfn = __pa(shared_info_page) >> PAGE_SHIFT;
> + if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))
> + BUG();
> +
> + HYPERVISOR_shared_info = (struct shared_info *)shared_info_page;
> +
> + /* xen_vcpu is a pointer to the vcpu_info struct in the
> shared_info
> + * page, we use it in the event channel upcall */
> + for_each_online_cpu(cpu) {
> + per_cpu(xen_vcpu, cpu) =
> &HYPERVISOR_shared_info->vcpu_info[cpu];
> + }
> + printk(KERN_ERR"%s: remmaping shared info...\n", __func__);
But here you are using KERN_ERR ? Why not 'pr_info'? Also this looks it
has been already implemented in the arch/arm already? Can you use the existing
code in there and just make it exported?
> + }
> +}
> +
> +void xen_arch_post_suspend(int suspend_cancelled)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> +}
Same thing. Move them in xen-ops.h without any implementation.
> +
> +static void xen_vcpu_notify_restore(void *data)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> +}
Same thing. Move them in xen-ops.h without any implementation.
> +
> +void xen_arch_resume(void)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> +}
Same thing. Move them in xen-ops.h without any implementation.
> diff --git a/arch/arm/xen/time.c b/arch/arm/xen/time.c
> new file mode 100644
> index 0000000..af90e53
> --- /dev/null
> +++ b/arch/arm/xen/time.c
> @@ -0,0 +1,7 @@
> +#include <linux/kernel.h>
> +#include <xen/xen.h>
> +
> +void xen_timer_resume(void)
> +{
> + printk(KERN_ERR"%s: function not implemented\n", __func__);
> +}
I think you know what I am going to say here.
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index eabd0ee..3d24a95 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -1,10 +1,10 @@
> ifneq ($(CONFIG_ARM),y)
> -obj-y += manage.o
> obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o
> endif
> obj-$(CONFIG_X86) += fallback.o
> obj-y += grant-table.o features.o events.o balloon.o
> obj-y += xenbus/
> +obj-y += manage.o
>
> nostackp := $(call cc-option, -fno-stack-protector)
> CFLAGS_features.o := $(nostackp)
> diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
> index 412b96c..140c7a9 100644
> --- a/drivers/xen/manage.c
> +++ b/drivers/xen/manage.c
> @@ -17,6 +17,7 @@
> #include <xen/events.h>
> #include <xen/hvc-console.h>
> #include <xen/xen-ops.h>
> +#include <xen/interface/sched.h>
>
> #include <asm/xen/hypercall.h>
> #include <asm/xen/page.h>
> @@ -86,7 +87,14 @@ static int xen_suspend(void *data)
> * or the domain was merely checkpointed, and 0 if it
> * is resuming in a new domain.
> */
> +#ifdef CONFIG_ARM
> + {
> + struct sched_shutdown r = { .reason = SHUTDOWN_suspend };
> + HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
> + }
> +#else
> si->cancelled = HYPERVISOR_suspend(si->arg);
> +#endif
Please add the HYPERVISOR_suspend in arch/arm/include/asm/xen/hypercall.h
and implement it there.
>
> if (si->post)
> si->post(si->cancelled);
> --
> 1.8.1.2
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |