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

[Julien Grall <julien.grall@linaro.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