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

[Wei Huang <w1.huang@xxxxxxxxxxx>]

On 05/14/2014 06:14 AM, Ian Campbell wrote:
> 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.
This is the preferred approach after discussing with Julien.
> Ian.

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