From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [RFC v3 3/6] xen/arm: Add save/restore support for ARM arch timer Date: Wed, 14 May 2014 13:13:24 +0100 Message-ID: <53735DE4.1090900@linaro.org> References: <1399583908-21755-1-git-send-email-w1.huang@samsung.com> <1399583908-21755-4-git-send-email-w1.huang@samsung.com> <1400066090.29366.31.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1400066090.29366.31.camel@kazak.uk.xensource.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: Ian Campbell , Wei Huang Cc: keir@xen.org, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, tim@xen.org, jaeyong.yoo@samsung.com, xen-devel@lists.xen.org, jbeulich@suse.com, ian.jackson@eu.citrix.com, yjhyun.yoo@samsung.com List-Id: xen-devel@lists.xenproject.org On 05/14/2014 12:14 PM, 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? CNTFRQ is set by the platform and can't change for any guest. If we migrate to a platform with a different frequency, then the guest should cope with it. IHMO, CNTKCTL should be saved/restored in guest core registers. >> +#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? Careful, phystimer.vtb_offset is in nanosecond and virttimer.vtb_offset is in ticks. Regards, -- Julien Grall