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

[Julien Grall <julien.grall@linaro.org>]

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.

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.

Did I miss something?

-- 
Julien Grall