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

Re: [Xen-devel] [PATCH 03/18] xen/arm: Save GIC and virtual timer context when the domain suspends

On 11/12/18 4:52 PM, Mirela Simonovic wrote:
Hi Julien,


Thanks for the feedback.

On Mon, Nov 12, 2018 at 4:36 PM Julien Grall <julien.grall@xxxxxxx> wrote:

Hi Mirela,

On 11/12/18 11:30 AM, Mirela Simonovic wrote:
GIC and virtual timer context must be saved when the domain suspends.

Please provide the rationale for this. Is it required by the spec?

This is required for GIC because a guest leaves enabled interrupts in
the GIC that could wake it up, and on resume it should be able to
detect which interrupt woke it up. Without saving/restoring the state
of GIC this would not be possible.

I am confused. In patch #10, you save the GIC host because the GIC can be powered-down. Linux is also saving the GIC state. So how the interrupt can come up if the GIC is powered down?

For timer, my understanding is that vtimer accounts for how long the
guest was executing and this tracking should not be affected by the
suspend/resume sequence. Without saving/restoring the state of vtimer,
the time tracking would be affected by the suspend/resume.

Currently, vtimer does not take such things into account. It only setup a timer in Xen to wake up the guest later on.

Hope this is ok, I'll add it to the commit message.

This is done by moving the respective code in ctxt_switch_from()
before the return that happens if the domain suspended.

Signed-off-by: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
Signed-off-by: Saeed Nowshadi <saeed.nowshadi@xxxxxxxxxx>
   xen/arch/arm/domain.c | 14 +++++++-------
   1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 7f8105465c..bebe3238e8 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -97,6 +97,13 @@ static void ctxt_switch_from(struct vcpu *p)
       if ( is_idle_vcpu(p) )

+    /* VGIC */
+    gic_save_state(p);
+    /* Arch timer */
+    p->arch.cntkctl = READ_SYSREG32(CNTKCTL_EL1);
+    virt_timer_save(p);
       /* VCPU's context should not be saved if its domain is suspended */

The GIC and timer are part of the vCPU context. So this comment looks a
bit stale.

It's not stale, but incorrect in that perspective. The comment should
be updated to "VCPU architecture specific context should ..."

But the timer is part of the architecture...


Julien Grall

Xen-devel mailing list



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