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

[Wei Huang <w1.huang@xxxxxxxxxxx>]

On 05/13/2014 10:42 AM, Julien Grall wrote:
> (Adding back Xen devel)
>
> On 05/13/2014 04:31 PM, Wei Huang wrote:
> > On 05/12/2014 07:04 AM, Julien Grall wrote:
> > > On 05/12/2014 10:16 AM, Ian Campbell wrote:
> > > > On Thu, 2014-04-17 at 16:06 +0100, Julien Grall wrote:
> > > > > 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?
> > > > The timer offset is mainly accounting for the fact that the domain is
> > > > not booted when the hardware is started.
> > > >
> > > > However time does continue while a VCPU is not scheduled, this is
> > > > exposed via the PV "stolen time" mechanism.
> > > >
> > > > Now it is in theory possible to virtualise time differently so that
> > > > stolen time is not possible, but unless you want to cope with different
> > > > VCPUs seeing different times (because they have been descheduled for
> > > > differently lengths of times) then you either need to do gang scheduling
> > > > or play other (likely complicated) tricks. With the model we have on ARM
> > > > paravirtualising this is the right thing to do.
> > > >
> > > > Not sure what you mean about CVAL (the timer compare val) not running,
> > > > when we deschedule a VCPU we figure out when CVAL would have caused the
> > > > timer interrupt to fire and setup a Xen timer to make sure we unblock
> > > > the VCPU at that point. When we switch back to the VCPU we of course
> > > > restore the compare value to what the guest wrote, nothing else would
> > > > make sense.
> > > After reading your explanation and the ARM ARM again, I think I mingled
> > > CNT (the counter) and CVAL (the compare val).
> > >
> > > Thank you for the explanation.
> > Other than the code comments (case/switch), are you OK with the design
> > of the latest ARCH_TIMER patch?
> I made some comment on the v3. Once you will address comments from
> Andrew and me, the patch will be in good shape.
Given the comments from you and Andrew, I will revise the context struct 
to the following format. With this, we can get rid of most problems 
(switch/case/...).
struct hvm_arm_timer
{
    /* phys_timer */
    uint64_t phys_vtb_offset;
    uint64_t phys_cval;
    uint32_t phys_ctl;

    /* virt_timer */
    uint64_t virt_vtb_offset;
    uint64_t virt_cval;
    uint32_t virt_ctl;
};
DECLARE_HVM_SAVE_TYPE(TIMER, 4, struct hvm_arm_timer);

Any comments, please let me know.

> Regards,

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