Re: [RFC v2 1/6] xen/arm: Save and restore support with hvm context hypercalls

[Wei Huang <w1.huang@samsung.com>]

On 04/17/2014 10:06 AM, Julien Grall wrote:
> Hello Wei,
>
> On 04/15/2014 10:05 PM, Wei Huang wrote:
>> +static int hvm_vtimer_load_ctxt(struct domain *d, hvm_domain_context_t *h)
>> +{
>> +    int vcpuid;
>> +    struct hvm_hw_timer ctxt;
>> +    struct vcpu *v;
>> +    struct vtimer *t = NULL;
>> +
>> +    /* Which vcpu is this? */
>> +    vcpuid = hvm_load_instance(h);
>> +
>> +    if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
>> +    {
>> +        dprintk(XENLOG_G_ERR, "HVM restore: dom%u has no vcpu%u\n",
>> +                d->domain_id, vcpuid);
>> +        return -EINVAL;
>> +    }
>> +
>> +    if ( hvm_load_entry(TIMER, h, &ctxt) != 0 )
>> +        return -EINVAL;
>> +
>> +    if ( ctxt.type == TIMER_TYPE_VIRT )
>> +    {
>> +        t = &v->arch.virt_timer;
>> +        d->arch.virt_timer_base.offset = ctxt.vtb_offset;
>> +    }
>> +    else
>> +    {
>> +        t = &v->arch.phys_timer;
>> +        d->arch.phys_timer_base.offset = ctxt.vtb_offset;
>
> I thought a bit more about the {phys,virt}_timer_base.offset.
>
> When you are migrating a guest, this offset will be invalidated. This
> offset is used to get a relative offset from the Xen timer counter.
>
Agreed.
> That also made me think the context switch in Xen for the timer looks
> wrong to me.
>
> When a guest VCPU is context switch, the Xen timer counter continue to
> run. But not CVAL, so the timer_base.offset will drift a bit. It will
> result by setting a wrong timer via set_timer in Xen.
>
I need to examine the code more carefully. But by skimming through 
vtimer.c, it looks like this is the case: the missing ticks were not 
compensated.
> Did I miss something?
>