From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Huang Subject: Re: [RFC v2 1/6] xen/arm: Save and restore support with hvm context hypercalls Date: Wed, 16 Apr 2014 16:50:00 -0500 Message-ID: <534EFB08.1030608@samsung.com> References: <1397595918-30419-1-git-send-email-w1.huang@samsung.com> <1397595918-30419-2-git-send-email-w1.huang@samsung.com> <534DC2D5.4050302@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-reply-to: <534DC2D5.4050302@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper , xen-devel@lists.xen.org Cc: yjhyun.yoo@samsung.com, julien.grall@linaro.org, ian.campbell@citrix.com, jaeyong.yoo@samsung.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 04/15/2014 06:37 PM, Andrew Cooper wrote: >> --- a/xen/include/public/arch-arm/hvm/save.h >> +++ b/xen/include/public/arch-arm/hvm/save.h >> @@ -26,6 +26,136 @@ >> #ifndef __XEN_PUBLIC_HVM_SAVE_ARM_H__ >> #define __XEN_PUBLIC_HVM_SAVE_ARM_H__ >> >> +#define HVM_FILE_MAGIC 0x92385520 >> +#define HVM_FILE_VERSION 0x00000001 >> + >> +struct hvm_save_header >> +{ >> + uint32_t magic; /* Must be HVM_FILE_MAGIC */ >> + uint32_t version; /* File format version */ >> + uint64_t changeset; /* Version of Xen that saved this file */ >> + uint32_t cpuid; /* MIDR_EL1 on the saving machine */ > > This looks needlessly copied from x86, which is far from ideal. > > On x86, Xen tries to parse the mercural revision number from its compile > time information and fails now that the underlying codebase has moved > from hg to git. As a result, the value is now generally -1. > > cpuid is also an x86ism as far as I am aware. > > I also wonder about the wisdom of having identically named structures > like this in arch code without an arch_ prefix? We should make it as > hard as possible for things like this to accidentally get referenced in > common code. > It is tricky for hvm_save_header. This is a struct used in common code (xen/common/hvm/save.c). Instead of making it arch_ specific, I would move it to common code (include/public/hvm/save.h), with the following modification: 1. Re-define "cpuid" of hvm_save_header as cpu_id. hvm_save_header will be shared by both x86 and ARM. 2. Rename HVM_FILE_MAGIC to HVM_ARM_FILE_MAGIC. Still keep it in arch-arm/hvm/save.h file. This is used in arch- specific code, so we won't get confused. The same applies to HVM_FILE_MAGIC in x86. 3. Other struct in arch-arm/hvm/save.h will remain in the same file. Those structs are arch specific anyway. -Wei