Re: [Xen-devel] [RFC v3 1/6] xen/arm: Add basic save/restore support for ARM

[Wei Huang <w1.huang@xxxxxxxxxxx>]

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@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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