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

[Julien Grall <julien.grall@linaro.org>]

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,

-- 
Julien Grall