Re: [PATCH RFC XEN v1 05/14] xen: arm: Implement basic XEN_DOMCTL_{set, get}hvmcontext support

[Ian Campbell <ian.campbell@citrix.com>]

On Tue, 2015-12-15 at 18:00 +0000, Stefano Stabellini wrote:
> On Wed, 9 Dec 2015, Ian Campbell wrote:
> > This is just the minimally required basic infra and header, no actual
> > useful data is saved yet.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> > ---
> >  xen/arch/arm/Makefile                  |  1 +
> >  xen/arch/arm/domctl.c                  | 74
> > ++++++++++++++++++++++++++++++++++
> >  xen/arch/arm/save.c                    | 53 ++++++++++++++++++++++++
> >  xen/common/Makefile                    |  2 +
> >  xen/include/asm-arm/hvm/support.h      | 25 ++++++++++++
> >  xen/include/public/arch-arm/hvm/save.h | 17 ++++++++
> >  6 files changed, 172 insertions(+)
> >  create mode 100644 xen/arch/arm/save.c
> >  create mode 100644 xen/include/asm-arm/hvm/support.h
> > 
> > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> > index 4ac5edd..5a96486 100644
> > --- a/xen/arch/arm/Makefile
> > +++ b/xen/arch/arm/Makefile
> > @@ -40,6 +40,7 @@ obj-y += device.o
> >  obj-y += decode.o
> >  obj-y += processor.o
> >  obj-y += smc.o
> > +obj-y += save.o
> >  
> >  #obj-bin-y += ....o
> >  
> > diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> > index d3459ee..3403e23 100644
> > --- a/xen/arch/arm/domctl.c
> > +++ b/xen/arch/arm/domctl.c
> > @@ -12,6 +12,7 @@
> >  #include <xen/hypercall.h>
> >  #include <xen/iocap.h>
> >  #include <xsm/xsm.h>
> > +#include <xen/hvm/save.h>
> >  #include <xen/guest_access.h>
> >  #include <public/domctl.h>
> >  
> > @@ -134,6 +135,79 @@ long arch_do_domctl(struct xen_domctl *domctl,
> > struct domain *d,
> >          break;
> >      }
> >  
> > +    case XEN_DOMCTL_sethvmcontext:
> > +    {
> > +        struct hvm_domain_context c = { .size = domctl-
> > >u.hvmcontext.size };
> 
> if ( d == currend->domain )
>     return some_kind_of_error;
> ?

I'd be inclined to put that into the common code so either hvm_load, or
with your suggestion later to make this common (which sounds doable) here.

Not sure about the get case, there might be reasons other than migration to
want to get at this state? (Maybe nothing other than debug hacking I guess)

[...]
> Actually now that I have read the patch, I think that rather than
> duplicating the code, we should just move the x86 implementation of
> XEN_DOMCTL_sethvmcontext and XEN_DOMCTL_gethvmcontext to common code.
> The differences are negligible.

Yes, I'll give that a go.

> 
> > +#ifndef __ASM_ARM_HVM_SUPPORT_H__
> > +#define __ASM_ARM_HVM_SUPPORT_H__
> > +
> > +#include <xen/sched.h>
> > +#include <xen/hvm/save.h>
> > +
> > +#endif /* __ASM_ARM_HVM_SUPPORT_H__ */
> 
> Interestingly I can compile Xen on x86_64 just fine removing the
> #include <asm/hvm/support.h> in xen/common/hvm/save.c. I think that is a
> better option than introducing this empty header file.

I'll give that a go, it might be invalidated by the consolidation you
suggest above, but in that case the empty header is the lesser evil.

> > diff --git a/xen/include/public/arch-arm/hvm/save.h
> > b/xen/include/public/arch-arm/hvm/save.h
> > index 75b8e65..5f4de94 100644
> > --- a/xen/include/public/arch-arm/hvm/save.h
> > +++ b/xen/include/public/arch-arm/hvm/save.h
> > @@ -26,6 +26,23 @@
> >  #ifndef __XEN_PUBLIC_HVM_SAVE_ARM_H__
> >  #define __XEN_PUBLIC_HVM_SAVE_ARM_H__
> >  
> > +#define HVM_FILE_MAGIC   0x92385520
> 
> Out of curiousity, how did you come up with the number? Just making sure
> there is no secret message in there.

I appear to have gotten it from the original Samsung patches.

If there is a secret message then I can't spot it, it's nothing in ascii at
least (<INVALID>8U<SPACE>).

Ian.

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