Re: [RFC v2 1/6] xen/arm: Save and restore support with hvm context hypercalls

[Wei Huang <w1.huang@samsung.com>]

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