Re: [Xen-devel] [RFC v3 3/6] xen/arm: Add save/restore support for ARM arch timer

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2014-05-08 at 16:18 -0500, Wei Huang wrote:
> This patch implements a save/resore support for ARM architecture

"restore"

> diff --git a/xen/include/public/arch-arm/hvm/save.h 
> b/xen/include/public/arch-arm/hvm/save.h
> index 421a6f6..8679bfd 100644
> --- a/xen/include/public/arch-arm/hvm/save.h
> +++ b/xen/include/public/arch-arm/hvm/save.h
> @@ -72,10 +72,24 @@ struct hvm_arm_gich_v2
>  };
>  DECLARE_HVM_SAVE_TYPE(GICH_V2, 3, struct hvm_arm_gich_v2);
>  
> +/* Two ARM timers (physical and virtual) are saved */

Do you not need to save CNTFRQ and CNTKCTL?

> +#define ARM_TIMER_TYPE_VIRT  0
> +#define ARM_TIMER_TYPE_PHYS  1
> +#define ARM_TIMER_TYPE_COUNT 2       /* total count */
> +
> +struct hvm_arm_timer
> +{
> +    uint64_t vtb_offset;

As discussed elsewhere I don't think the offset is architectural state.
This should be incorporated into the cval. Otherwise how does the
receiver know what this is an offset from?

> +    uint32_t ctl;
> +    uint64_t cval;
> +    uint32_t type;
> +};
> +DECLARE_HVM_SAVE_TYPE(TIMER, 4, struct hvm_arm_timer);

Why not do
DECLARE_HVM_SAVE_TYPE(VTIMER, 4, struct hvm_arm_timer)
DECLARE_HVM_SAVE_TYPE(PTIMER, 5, struct hvm_arm_timer)
and drop the type field?

Or else define hvm_arm_timers with cntfeq, cntkctl and two sets of the
ctl,cval in a single struct.

Ian.


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