| Message ID | 1399583908-21755-2-git-send-email-w1.huang@samsung.com |
|---|---|
| State | New |
| Headers | show |
On 08/05/2014 22:18, Wei Huang wrote: > diff --git a/xen/include/asm-arm/hvm/support.h b/xen/include/asm-arm/hvm/support.h > new file mode 100644 > index 0000000..fa5fe75 > --- /dev/null > +++ b/xen/include/asm-arm/hvm/support.h > @@ -0,0 +1,29 @@ > +/* > + * HVM support routines > + * > + * Copyright (c) 2014, Samsung Electronics. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License along with > + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple > + * Place - Suite 330, Boston, MA 02111-1307 USA. > + */ > + > +#ifndef __ASM_ARM_HVM_SUPPORT_H__ > +#define __ASM_ARM_HVM_SUPPORT_H__ > + > +#include <xen/types.h> > +#include <public/hvm/ioreq.h> > +#include <xen/sched.h> > +#include <xen/hvm/save.h> > +#include <asm/processor.h> > + > +#endif /* __ASM_ARM_HVM_SUPPORT_H__ */ This header file isn't touched by any subsequent patches, and just having it as a list of includes is overkill. Can it be dropped? 5 extra includes in a few .c files is hardly breaking the bank. > diff --git a/xen/include/public/arch-arm/hvm/save.h b/xen/include/public/arch-arm/hvm/save.h > index 75b8e65..8312e7b 100644 > --- a/xen/include/public/arch-arm/hvm/save.h > +++ b/xen/include/public/arch-arm/hvm/save.h > @@ -3,6 +3,7 @@ > * be saved along with the domain's memory and device-model state. > * > * Copyright (c) 2012 Citrix Systems Ltd. > + * Copyright (c) 2014 Samsung Electronics. > * > * Permission is hereby granted, free of charge, to any person obtaining a copy > * of this software and associated documentation files (the "Software"), to > @@ -26,6 +27,24 @@ > #ifndef __XEN_PUBLIC_HVM_SAVE_ARM_H__ > #define __XEN_PUBLIC_HVM_SAVE_ARM_H__ > > +#define HVM_ARM_FILE_MAGIC 0x92385520 > +#define HVM_ARM_FILE_VERSION 0x00000001 > + > +/* Note: For compilation purpose hvm_save_header name is the same as x86, > + * but layout is different. */ > +struct hvm_save_header > +{ > + uint32_t magic; /* Must be HVM_ARM_FILE_MAGIC */ > + uint32_t version; /* File format version */ > + uint32_t cpuinfo; /* Record MIDR_EL1 info of saving machine */ > +}; > +DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header); > + > +/* > + * Largest type-code in use > + */ > +#define HVM_SAVE_CODE_MAX 1 > + > #endif > > /* Hmm - it is quite poor to have this magically named "hvm_save_header". If you were to redefine arch_hvm{save,load} as: int arch_hvm_save(struct domain *d, struct hvm_domain_context_t *h); int arch_hvm_load(struct domain *d, struct hvm_domain_context_t *h); and pushed the hvm_save_entry(HEADER, 0, h, &hdr) into arch_hvm_save(), you can remove all trace of "struct hvm_save_header" from common code. This then removes any requirements to have an identically named struct. It is probably worth having this all as a separate patch which just cleans up the x86 and common code before introducing the ARM side of things. The rest of the patch is however looking fine. ~Andrew
On 05/08/2014 05:11 PM, Andrew Cooper wrote: > On 08/05/2014 22:18, Wei Huang wrote: > > >> diff --git a/xen/include/asm-arm/hvm/support.h b/xen/include/asm-arm/hvm/support.h >> new file mode 100644 >> index 0000000..fa5fe75 >> --- /dev/null >> +++ b/xen/include/asm-arm/hvm/support.h >> @@ -0,0 +1,29 @@ >> +/* >> + * HVM support routines >> + * >> + * Copyright (c) 2014, Samsung Electronics. >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms and conditions of the GNU General Public License, >> + * version 2, as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope it will be useful, but WITHOUT >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >> + * more details. >> + * >> + * You should have received a copy of the GNU General Public License along with >> + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple >> + * Place - Suite 330, Boston, MA 02111-1307 USA. >> + */ >> + >> +#ifndef __ASM_ARM_HVM_SUPPORT_H__ >> +#define __ASM_ARM_HVM_SUPPORT_H__ >> + >> +#include <xen/types.h> >> +#include <public/hvm/ioreq.h> >> +#include <xen/sched.h> >> +#include <xen/hvm/save.h> >> +#include <asm/processor.h> >> + >> +#endif /* __ASM_ARM_HVM_SUPPORT_H__ */ > > This header file isn't touched by any subsequent patches, and just > having it as a list of includes is overkill. Can it be dropped? 5 > extra includes in a few .c files is hardly breaking the bank. Last time I tried it quickly, the compilation broke. Will try again. > >> diff --git a/xen/include/public/arch-arm/hvm/save.h b/xen/include/public/arch-arm/hvm/save.h >> index 75b8e65..8312e7b 100644 >> --- a/xen/include/public/arch-arm/hvm/save.h >> +++ b/xen/include/public/arch-arm/hvm/save.h >> @@ -3,6 +3,7 @@ >> * be saved along with the domain's memory and device-model state. >> * >> * Copyright (c) 2012 Citrix Systems Ltd. >> + * Copyright (c) 2014 Samsung Electronics. >> * >> * Permission is hereby granted, free of charge, to any person obtaining a copy >> * of this software and associated documentation files (the "Software"), to >> @@ -26,6 +27,24 @@ >> #ifndef __XEN_PUBLIC_HVM_SAVE_ARM_H__ >> #define __XEN_PUBLIC_HVM_SAVE_ARM_H__ >> >> +#define HVM_ARM_FILE_MAGIC 0x92385520 >> +#define HVM_ARM_FILE_VERSION 0x00000001 >> + >> +/* Note: For compilation purpose hvm_save_header name is the same as x86, >> + * but layout is different. */ >> +struct hvm_save_header >> +{ >> + uint32_t magic; /* Must be HVM_ARM_FILE_MAGIC */ >> + uint32_t version; /* File format version */ >> + uint32_t cpuinfo; /* Record MIDR_EL1 info of saving machine */ >> +}; >> +DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header); >> + >> +/* >> + * Largest type-code in use >> + */ >> +#define HVM_SAVE_CODE_MAX 1 >> + >> #endif >> >> /* > > Hmm - it is quite poor to have this magically named "hvm_save_header". > > If you were to redefine arch_hvm{save,load} as: > > int arch_hvm_save(struct domain *d, struct hvm_domain_context_t *h); > int arch_hvm_load(struct domain *d, struct hvm_domain_context_t *h); > > and pushed the hvm_save_entry(HEADER, 0, h, &hdr) into arch_hvm_save(), > you can remove all trace of "struct hvm_save_header" from common code. > This then removes any requirements to have an identically named struct. > > It is probably worth having this all as a separate patch which just > cleans up the x86 and common code before introducing the ARM side of things. Reasonable. I will fix next revision. > > The rest of the patch is however looking fine. > > ~Andrew > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >
Hi Andrew and Wei, On 08/05/14 23:20, Wei Huang wrote: > On 05/08/2014 05:11 PM, Andrew Cooper wrote: >> On 08/05/2014 22:18, Wei Huang wrote: >> >> >>> diff --git a/xen/include/asm-arm/hvm/support.h >>> b/xen/include/asm-arm/hvm/support.h >>> new file mode 100644 >>> index 0000000..fa5fe75 >>> --- /dev/null >>> +++ b/xen/include/asm-arm/hvm/support.h >>> @@ -0,0 +1,29 @@ >>> +/* >>> + * HVM support routines >>> + * >>> + * Copyright (c) 2014, Samsung Electronics. >>> + * >>> + * This program is free software; you can redistribute it and/or >>> modify it >>> + * under the terms and conditions of the GNU General Public License, >>> + * version 2, as published by the Free Software Foundation. >>> + * >>> + * This program is distributed in the hope it will be useful, but >>> WITHOUT >>> + * ANY WARRANTY; without even the implied warranty of >>> MERCHANTABILITY or >>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public >>> License for >>> + * more details. >>> + * >>> + * You should have received a copy of the GNU General Public License >>> along with >>> + * this program; if not, write to the Free Software Foundation, >>> Inc., 59 Temple >>> + * Place - Suite 330, Boston, MA 02111-1307 USA. >>> + */ >>> + >>> +#ifndef __ASM_ARM_HVM_SUPPORT_H__ >>> +#define __ASM_ARM_HVM_SUPPORT_H__ >>> + >>> +#include <xen/types.h> >>> +#include <public/hvm/ioreq.h> >>> +#include <xen/sched.h> >>> +#include <xen/hvm/save.h> >>> +#include <asm/processor.h> >>> + >>> +#endif /* __ASM_ARM_HVM_SUPPORT_H__ */ >> >> This header file isn't touched by any subsequent patches, and just >> having it as a list of includes is overkill. Can it be dropped? 5 >> extra includes in a few .c files is hardly breaking the bank. > Last time I tried it quickly, the compilation broke. Will try again. This header is used in common code (xen/common/hvm/save.c). It might be better on have a dummy header. Some of the includes you've added shouldn't not be used by ARM (i.e public/hvm/ioreq.h). Regards,
Hi Wei, On 08/05/14 22:18, Wei Huang wrote: > +void arch_hvm_save(struct domain *d, struct hvm_save_header *hdr) > +{ > + hdr->magic = HVM_ARM_FILE_MAGIC; > + hdr->version = HVM_ARM_FILE_VERSION; > + hdr->cpuinfo = READ_SYSREG32(MIDR_EL1); You can directly use boot_cpu_data.midr.bits. > +} > + > +int arch_hvm_load(struct domain *d, struct hvm_save_header *hdr) > +{ > + uint32_t cpuinfo; > + > + if ( hdr->magic != HVM_ARM_FILE_MAGIC ) > + { > + printk(XENLOG_G_ERR "HVM%d restore: bad magic number %#"PRIx32"\n", > + d->domain_id, hdr->magic); > + return -EINVAL; > + } > + > + if ( hdr->version != HVM_ARM_FILE_VERSION ) > + { > + printk(XENLOG_G_ERR "HVM%d restore: unsupported version %u\n", > + d->domain_id, hdr->version); > + return -EINVAL; > + } > + > + cpuinfo = READ_SYSREG32(MIDR_EL1); Same here.
>>> On 08.05.14 at 23:18, <w1.huang@samsung.com> wrote: > This patch implements a basic framework for ARM guest > save/restore. It defines a HVM save header for ARM guests > and correponding arch_ save/load functions. These functions > are hooked up with domain control hypercalls (gethvmcontext > and sethvmcontext). The hypercalls become a common code path to > both x86 and ARM. As a result of merging, the x86 specific > header saving code is moved to x86 sub-directory. > > Signed-off-by: Evgeny Fedotov <e.fedotov@samsung.com> > Signed-off-by: Wei Huang <w1.huang@samsung.com> > --- > xen/arch/arm/Makefile | 1 + > xen/arch/arm/save.c | 68 +++++++++++++++++++++++++ > xen/arch/x86/domctl.c | 85 ------------------------------- > xen/arch/x86/hvm/save.c | 12 +++++ > xen/common/Makefile | 2 +- > xen/common/domctl.c | 86 ++++++++++++++++++++++++++++++++ > xen/common/hvm/save.c | 11 ---- > xen/include/asm-arm/hvm/support.h | 29 +++++++++++ > xen/include/public/arch-arm/hvm/save.h | 19 +++++++ > 9 files changed, 216 insertions(+), 97 deletions(-) Provided it is like it looks like - just code movement - for the non-ARM bits: Acked-by: Jan Beulich <jbeulich@suse.com>
On Thu, 2014-05-08 at 23:11 +0100, Andrew Cooper wrote: > > diff --git a/xen/include/public/arch-arm/hvm/save.h b/xen/include/public/arch-arm/hvm/save.h > > index 75b8e65..8312e7b 100644 > > --- a/xen/include/public/arch-arm/hvm/save.h > > +++ b/xen/include/public/arch-arm/hvm/save.h > > @@ -3,6 +3,7 @@ > > * be saved along with the domain's memory and device-model state. > > * > > * Copyright (c) 2012 Citrix Systems Ltd. > > + * Copyright (c) 2014 Samsung Electronics. > > * > > * Permission is hereby granted, free of charge, to any person obtaining a copy > > * of this software and associated documentation files (the "Software"), to > > @@ -26,6 +27,24 @@ > > #ifndef __XEN_PUBLIC_HVM_SAVE_ARM_H__ > > #define __XEN_PUBLIC_HVM_SAVE_ARM_H__ > > > > +#define HVM_ARM_FILE_MAGIC 0x92385520 > > +#define HVM_ARM_FILE_VERSION 0x00000001 > > + > > +/* Note: For compilation purpose hvm_save_header name is the same as x86, > > + * but layout is different. */ > > +struct hvm_save_header > > +{ > > + uint32_t magic; /* Must be HVM_ARM_FILE_MAGIC */ > > + uint32_t version; /* File format version */ > > + uint32_t cpuinfo; /* Record MIDR_EL1 info of saving machine */ > > +}; > > +DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header); > > + > > +/* > > + * Largest type-code in use > > + */ > > +#define HVM_SAVE_CODE_MAX 1 > > + > > #endif > > > > /* > > Hmm - it is quite poor to have this magically named "hvm_save_header". We frequently have arch interfaces where generic code requires arch code to provide particular structs or functions etc. What is poor about this particular instance of that pattern? Ian.
On Fri, 2014-05-09 at 09:56 +0100, Julien Grall wrote: > >> This header file isn't touched by any subsequent patches, and just > >> having it as a list of includes is overkill. Can it be dropped? 5 > >> extra includes in a few .c files is hardly breaking the bank. > > Last time I tried it quickly, the compilation broke. Will try again. > > This header is used in common code (xen/common/hvm/save.c). It might be > better on have a dummy header. It's used exactly twice in common code, xen/common/hvm/save.c and xen/drivers/passthrough/io.c. I'm not sure which functionality from this header these guys are using but it can't be much (or the arm version would be bigger). I think it's probably better to just eliminate this asm include from common code. Ian.
On Thu, 2014-05-08 at 16:18 -0500, Wei Huang wrote: > This patch implements a basic framework for ARM guest > save/restore. It defines a HVM save header for ARM guests > and correponding arch_ save/load functions. These functions "corresponding" > are hooked up with domain control hypercalls (gethvmcontext > and sethvmcontext). The hypercalls become a common code path to > both x86 and ARM. As a result of merging, the x86 specific > header saving code is moved to x86 sub-directory. > > Signed-off-by: Evgeny Fedotov <e.fedotov@samsung.com> > Signed-off-by: Wei Huang <w1.huang@samsung.com> Other than the comments already made by others this is looking good to me. > [...] > +#define HVM_ARM_FILE_MAGIC 0x92385520 OOI where did that number come from? (often these are a few ASCII characters etc, I'm just curious) > +#define HVM_ARM_FILE_VERSION 0x00000001 > + > +/* Note: For compilation purpose hvm_save_header name is the same as x86, > + * but layout is different. */ > +struct hvm_save_header > +{ > + uint32_t magic; /* Must be HVM_ARM_FILE_MAGIC */ > + uint32_t version; /* File format version */ > + uint32_t cpuinfo; /* Record MIDR_EL1 info of saving machine */ Is the size of this struct the same for arm32 and arm64? Ian.
On 14/05/14 11:25, Ian Campbell wrote: > On Thu, 2014-05-08 at 23:11 +0100, Andrew Cooper wrote: >>> diff --git a/xen/include/public/arch-arm/hvm/save.h b/xen/include/public/arch-arm/hvm/save.h >>> index 75b8e65..8312e7b 100644 >>> --- a/xen/include/public/arch-arm/hvm/save.h >>> +++ b/xen/include/public/arch-arm/hvm/save.h >>> @@ -3,6 +3,7 @@ >>> * be saved along with the domain's memory and device-model state. >>> * >>> * Copyright (c) 2012 Citrix Systems Ltd. >>> + * Copyright (c) 2014 Samsung Electronics. >>> * >>> * Permission is hereby granted, free of charge, to any person obtaining a copy >>> * of this software and associated documentation files (the "Software"), to >>> @@ -26,6 +27,24 @@ >>> #ifndef __XEN_PUBLIC_HVM_SAVE_ARM_H__ >>> #define __XEN_PUBLIC_HVM_SAVE_ARM_H__ >>> >>> +#define HVM_ARM_FILE_MAGIC 0x92385520 >>> +#define HVM_ARM_FILE_VERSION 0x00000001 >>> + >>> +/* Note: For compilation purpose hvm_save_header name is the same as x86, >>> + * but layout is different. */ >>> +struct hvm_save_header >>> +{ >>> + uint32_t magic; /* Must be HVM_ARM_FILE_MAGIC */ >>> + uint32_t version; /* File format version */ >>> + uint32_t cpuinfo; /* Record MIDR_EL1 info of saving machine */ >>> +}; >>> +DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header); >>> + >>> +/* >>> + * Largest type-code in use >>> + */ >>> +#define HVM_SAVE_CODE_MAX 1 >>> + >>> #endif >>> >>> /* >> Hmm - it is quite poor to have this magically named "hvm_save_header". > We frequently have arch interfaces where generic code requires arch code > to provide particular structs or functions etc. What is poor about this > particular instance of that pattern? > > Ian. > Save/restore is currently asymmetric in this regard. The save side treats this x86 structure as common, whereas load is entirely arch specific. Fixing the asymmetry sensibly involves pushing the save side into arch code. ~Andrew
On Wed, 2014-05-14 at 11:46 +0100, Andrew Cooper wrote: > On 14/05/14 11:25, Ian Campbell wrote: > > On Thu, 2014-05-08 at 23:11 +0100, Andrew Cooper wrote: > >>> diff --git a/xen/include/public/arch-arm/hvm/save.h b/xen/include/public/arch-arm/hvm/save.h > >>> index 75b8e65..8312e7b 100644 > >>> --- a/xen/include/public/arch-arm/hvm/save.h > >>> +++ b/xen/include/public/arch-arm/hvm/save.h > >>> @@ -3,6 +3,7 @@ > >>> * be saved along with the domain's memory and device-model state. > >>> * > >>> * Copyright (c) 2012 Citrix Systems Ltd. > >>> + * Copyright (c) 2014 Samsung Electronics. > >>> * > >>> * Permission is hereby granted, free of charge, to any person obtaining a copy > >>> * of this software and associated documentation files (the "Software"), to > >>> @@ -26,6 +27,24 @@ > >>> #ifndef __XEN_PUBLIC_HVM_SAVE_ARM_H__ > >>> #define __XEN_PUBLIC_HVM_SAVE_ARM_H__ > >>> > >>> +#define HVM_ARM_FILE_MAGIC 0x92385520 > >>> +#define HVM_ARM_FILE_VERSION 0x00000001 > >>> + > >>> +/* Note: For compilation purpose hvm_save_header name is the same as x86, > >>> + * but layout is different. */ > >>> +struct hvm_save_header > >>> +{ > >>> + uint32_t magic; /* Must be HVM_ARM_FILE_MAGIC */ > >>> + uint32_t version; /* File format version */ > >>> + uint32_t cpuinfo; /* Record MIDR_EL1 info of saving machine */ > >>> +}; > >>> +DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header); > >>> + > >>> +/* > >>> + * Largest type-code in use > >>> + */ > >>> +#define HVM_SAVE_CODE_MAX 1 > >>> + > >>> #endif > >>> > >>> /* > >> Hmm - it is quite poor to have this magically named "hvm_save_header". > > We frequently have arch interfaces where generic code requires arch code > > to provide particular structs or functions etc. What is poor about this > > particular instance of that pattern? > Save/restore is currently asymmetric in this regard. The save side > treats this x86 structure as common, whereas load is entirely arch specific. > > Fixing the asymmetry sensibly involves pushing the save side into arch code. OK, that's an actual reason, thanks. Ian.
On 05/14/2014 05:37 AM, Ian Campbell wrote: > On Thu, 2014-05-08 at 16:18 -0500, Wei Huang wrote: >> This patch implements a basic framework for ARM guest >> save/restore. It defines a HVM save header for ARM guests >> and correponding arch_ save/load functions. These functions > > "corresponding" > >> are hooked up with domain control hypercalls (gethvmcontext >> and sethvmcontext). The hypercalls become a common code path to >> both x86 and ARM. As a result of merging, the x86 specific >> header saving code is moved to x86 sub-directory. >> >> Signed-off-by: Evgeny Fedotov <e.fedotov@samsung.com> >> Signed-off-by: Wei Huang <w1.huang@samsung.com> > > Other than the comments already made by others this is looking good to > me. > >> [...] >> +#define HVM_ARM_FILE_MAGIC 0x92385520 > > OOI where did that number come from? (often these are a few ASCII > characters etc, I'm just curious) No idea. I inherited from old patch. It doesn't seem to be meaningful ASCII string to me. > >> +#define HVM_ARM_FILE_VERSION 0x00000001 >> + >> +/* Note: For compilation purpose hvm_save_header name is the same as x86, >> + * but layout is different. */ >> +struct hvm_save_header >> +{ >> + uint32_t magic; /* Must be HVM_ARM_FILE_MAGIC */ >> + uint32_t version; /* File format version */ >> + uint32_t cpuinfo; /* Record MIDR_EL1 info of saving machine */ > > Is the size of this struct the same for arm32 and arm64? The size is the same on arm32 and arm64. > > Ian. > >
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 63e0460..d9a328c 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -33,6 +33,7 @@ obj-y += hvm.o obj-y += device.o obj-y += decode.o obj-y += processor.o +obj-y += save.o #obj-bin-y += ....o diff --git a/xen/arch/arm/save.c b/xen/arch/arm/save.c new file mode 100644 index 0000000..c4a6215 --- /dev/null +++ b/xen/arch/arm/save.c @@ -0,0 +1,68 @@ +/* + * hvm/save.c: Save and restore HVM guest's emulated hardware state for ARM. + * + * Copyright (c) 2014 Samsung Electronics. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple + * Place - Suite 330, Boston, MA 02111-1307 USA. + */ +#include <xen/config.h> +#include <asm/hvm/support.h> +#include <public/hvm/save.h> + +void arch_hvm_save(struct domain *d, struct hvm_save_header *hdr) +{ + hdr->magic = HVM_ARM_FILE_MAGIC; + hdr->version = HVM_ARM_FILE_VERSION; + hdr->cpuinfo = READ_SYSREG32(MIDR_EL1); +} + +int arch_hvm_load(struct domain *d, struct hvm_save_header *hdr) +{ + uint32_t cpuinfo; + + if ( hdr->magic != HVM_ARM_FILE_MAGIC ) + { + printk(XENLOG_G_ERR "HVM%d restore: bad magic number %#"PRIx32"\n", + d->domain_id, hdr->magic); + return -EINVAL; + } + + if ( hdr->version != HVM_ARM_FILE_VERSION ) + { + printk(XENLOG_G_ERR "HVM%d restore: unsupported version %u\n", + d->domain_id, hdr->version); + return -EINVAL; + } + + cpuinfo = READ_SYSREG32(MIDR_EL1); + if ( hdr->cpuinfo != cpuinfo ) + { + printk(XENLOG_G_ERR "HVM%d restore: VM saved on one CPU " + "(%#"PRIx32") and restored on another (%#"PRIx32").\n", + d->domain_id, hdr->cpuinfo, cpuinfo); + return -EINVAL; + } + + return 0; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index d792e87..06a19b0 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -401,91 +401,6 @@ long arch_do_domctl( } break; - case XEN_DOMCTL_sethvmcontext: - { - struct hvm_domain_context c = { .size = domctl->u.hvmcontext.size }; - - ret = -EINVAL; - if ( !is_hvm_domain(d) ) - goto sethvmcontext_out; - - ret = -ENOMEM; - if ( (c.data = xmalloc_bytes(c.size)) == NULL ) - goto sethvmcontext_out; - - ret = -EFAULT; - if ( copy_from_guest(c.data, domctl->u.hvmcontext.buffer, c.size) != 0) - goto sethvmcontext_out; - - domain_pause(d); - ret = hvm_load(d, &c); - domain_unpause(d); - - sethvmcontext_out: - if ( c.data != NULL ) - xfree(c.data); - } - break; - - case XEN_DOMCTL_gethvmcontext: - { - struct hvm_domain_context c = { 0 }; - - ret = -EINVAL; - if ( !is_hvm_domain(d) ) - goto gethvmcontext_out; - - c.size = hvm_save_size(d); - - if ( guest_handle_is_null(domctl->u.hvmcontext.buffer) ) - { - /* Client is querying for the correct buffer size */ - domctl->u.hvmcontext.size = c.size; - ret = 0; - goto gethvmcontext_out; - } - - /* Check that the client has a big enough buffer */ - ret = -ENOSPC; - if ( domctl->u.hvmcontext.size < c.size ) - goto gethvmcontext_out; - - /* Allocate our own marshalling buffer */ - ret = -ENOMEM; - if ( (c.data = xmalloc_bytes(c.size)) == NULL ) - goto gethvmcontext_out; - - domain_pause(d); - ret = hvm_save(d, &c); - domain_unpause(d); - - domctl->u.hvmcontext.size = c.cur; - if ( copy_to_guest(domctl->u.hvmcontext.buffer, c.data, c.size) != 0 ) - ret = -EFAULT; - - gethvmcontext_out: - copyback = 1; - - if ( c.data != NULL ) - xfree(c.data); - } - break; - - case XEN_DOMCTL_gethvmcontext_partial: - { - ret = -EINVAL; - if ( !is_hvm_domain(d) ) - break; - - domain_pause(d); - ret = hvm_save_one(d, domctl->u.hvmcontext_partial.type, - domctl->u.hvmcontext_partial.instance, - domctl->u.hvmcontext_partial.buffer); - domain_unpause(d); - } - break; - - case XEN_DOMCTL_set_address_size: { switch ( domctl->u.address_size.size ) diff --git a/xen/arch/x86/hvm/save.c b/xen/arch/x86/hvm/save.c index 6af19be..0d6da5c 100644 --- a/xen/arch/x86/hvm/save.c +++ b/xen/arch/x86/hvm/save.c @@ -27,6 +27,18 @@ void arch_hvm_save(struct domain *d, struct hvm_save_header *hdr) { uint32_t eax, ebx, ecx, edx; + char *c; + + /* Save magic and version info */ + hdr->magic = HVM_FILE_MAGIC; + hdr->version = HVM_FILE_VERSION; + + /* Save xen changeset */ + c = strrchr(xen_changeset(), ':'); + if ( c ) + hdr->changeset = simple_strtoll(c, NULL, 16); + else + hdr->changeset = -1ULL; /* Unknown */ /* Save some CPUID bits */ cpuid(1, &eax, &ebx, &ecx, &edx); diff --git a/xen/common/Makefile b/xen/common/Makefile index 3683ae3..13b781f 100644 --- a/xen/common/Makefile +++ b/xen/common/Makefile @@ -62,7 +62,7 @@ obj-$(CONFIG_XENCOMM) += xencomm.o subdir-$(CONFIG_COMPAT) += compat -subdir-$(x86_64) += hvm +subdir-y += hvm subdir-$(coverage) += gcov diff --git a/xen/common/domctl.c b/xen/common/domctl.c index af3614b..66358e4 100644 --- a/xen/common/domctl.c +++ b/xen/common/domctl.c @@ -24,6 +24,8 @@ #include <xen/bitmap.h> #include <xen/paging.h> #include <xen/hypercall.h> +#include <xen/hvm/save.h> +#include <xen/guest_access.h> #include <asm/current.h> #include <asm/irq.h> #include <asm/page.h> @@ -885,6 +887,90 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl) } break; + case XEN_DOMCTL_sethvmcontext: + { + struct hvm_domain_context c = { .size = op->u.hvmcontext.size }; + + ret = -EINVAL; + if ( (d == current->domain) /* no domain_pause() */ + || !is_hvm_domain(d) ) + goto sethvmcontext_out; + + ret = -ENOMEM; + if ( (c.data = xmalloc_bytes(c.size)) == NULL ) + goto sethvmcontext_out; + + ret = -EFAULT; + if ( copy_from_guest(c.data, op->u.hvmcontext.buffer, c.size) ) + goto sethvmcontext_out; + + domain_pause(d); + ret = hvm_load(d, &c); + domain_unpause(d); + + sethvmcontext_out: + xfree(c.data); + } + break; + + case XEN_DOMCTL_gethvmcontext: + { + struct hvm_domain_context c = { 0 }; + + ret = -EINVAL; + if ( (d == current->domain) /* no domain_pause() */ + || !is_hvm_domain(d) ) + goto gethvmcontext_out; + + c.size = hvm_save_size(d); + + if ( guest_handle_is_null(op->u.hvmcontext.buffer) ) + { + /* Client is querying for the correct buffer size */ + op->u.hvmcontext.size = c.size; + ret = 0; + goto gethvmcontext_out; + } + + /* Check that the client has a big enough buffer */ + ret = -ENOSPC; + if ( op->u.hvmcontext.size < c.size ) + goto gethvmcontext_out; + + /* Allocate our own marshalling buffer */ + ret = -ENOMEM; + if ( (c.data = xmalloc_bytes(c.size)) == NULL ) + goto gethvmcontext_out; + + domain_pause(d); + ret = hvm_save(d, &c); + domain_unpause(d); + + op->u.hvmcontext.size = c.cur; + if ( copy_to_guest(op->u.hvmcontext.buffer, c.data, c.size) ) + ret = -EFAULT; + + gethvmcontext_out: + copyback = 1; + xfree(c.data); + } + break; + + case XEN_DOMCTL_gethvmcontext_partial: + { + ret = -EINVAL; + if ( (d == current->domain) /* no domain_pause() */ + || !is_hvm_domain(d) ) + break; + + domain_pause(d); + ret = hvm_save_one(d, op->u.hvmcontext_partial.type, + op->u.hvmcontext_partial.instance, + op->u.hvmcontext_partial.buffer); + domain_unpause(d); + } + break; + default: ret = arch_do_domctl(op, d, u_domctl); break; diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c index 6c16399..0b303ff 100644 --- a/xen/common/hvm/save.c +++ b/xen/common/hvm/save.c @@ -140,7 +140,6 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance, int hvm_save(struct domain *d, hvm_domain_context_t *h) { - char *c; struct hvm_save_header hdr; struct hvm_save_end end; hvm_save_handler handler; @@ -149,16 +148,6 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h) if ( d->is_dying ) return -EINVAL; - hdr.magic = HVM_FILE_MAGIC; - hdr.version = HVM_FILE_VERSION; - - /* Save xen changeset */ - c = strrchr(xen_changeset(), ':'); - if ( c ) - hdr.changeset = simple_strtoll(c, NULL, 16); - else - hdr.changeset = -1ULL; /* Unknown */ - arch_hvm_save(d, &hdr); if ( hvm_save_entry(HEADER, 0, h, &hdr) != 0 ) diff --git a/xen/include/asm-arm/hvm/support.h b/xen/include/asm-arm/hvm/support.h new file mode 100644 index 0000000..fa5fe75 --- /dev/null +++ b/xen/include/asm-arm/hvm/support.h @@ -0,0 +1,29 @@ +/* + * HVM support routines + * + * Copyright (c) 2014, Samsung Electronics. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program; if not, write to the Free Software Foundation, Inc., 59 Temple + * Place - Suite 330, Boston, MA 02111-1307 USA. + */ + +#ifndef __ASM_ARM_HVM_SUPPORT_H__ +#define __ASM_ARM_HVM_SUPPORT_H__ + +#include <xen/types.h> +#include <public/hvm/ioreq.h> +#include <xen/sched.h> +#include <xen/hvm/save.h> +#include <asm/processor.h> + +#endif /* __ASM_ARM_HVM_SUPPORT_H__ */ diff --git a/xen/include/public/arch-arm/hvm/save.h b/xen/include/public/arch-arm/hvm/save.h index 75b8e65..8312e7b 100644 --- a/xen/include/public/arch-arm/hvm/save.h +++ b/xen/include/public/arch-arm/hvm/save.h @@ -3,6 +3,7 @@ * be saved along with the domain's memory and device-model state. * * Copyright (c) 2012 Citrix Systems Ltd. + * Copyright (c) 2014 Samsung Electronics. * * Permission is hereby granted, free of charge, to any person obtaining a copy * of this software and associated documentation files (the "Software"), to @@ -26,6 +27,24 @@ #ifndef __XEN_PUBLIC_HVM_SAVE_ARM_H__ #define __XEN_PUBLIC_HVM_SAVE_ARM_H__ +#define HVM_ARM_FILE_MAGIC 0x92385520 +#define HVM_ARM_FILE_VERSION 0x00000001 + +/* Note: For compilation purpose hvm_save_header name is the same as x86, + * but layout is different. */ +struct hvm_save_header +{ + uint32_t magic; /* Must be HVM_ARM_FILE_MAGIC */ + uint32_t version; /* File format version */ + uint32_t cpuinfo; /* Record MIDR_EL1 info of saving machine */ +}; +DECLARE_HVM_SAVE_TYPE(HEADER, 1, struct hvm_save_header); + +/* + * Largest type-code in use + */ +#define HVM_SAVE_CODE_MAX 1 + #endif /*