[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.