[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/15/18 12:35 AM, Stefano Stabellini wrote:
On Wed, 14 Nov 2018, Julien Grall wrote:
On 14/11/2018 22:45, Stefano Stabellini wrote:
On Wed, 14 Nov 2018, Julien Grall wrote:

On 13/11/2018 20:44, Stefano Stabellini wrote:
On Mon, 12 Nov 2018, Julien Grall wrote:
(+ Andre)

On 11/12/18 5:42 PM, Mirela Simonovic wrote:
Hi Julien,

On Mon, Nov 12, 2018 at 6:00 PM Julien Grall <julien.grall@xxxxxxx>

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>

Hi Mirela,

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

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

This is required for GIC because a guest leaves enabled interrupts
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
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?

After Xen (or Linux in the config without Xen) hands over the control
to EL3 on suspend (calls system suspend PSCI to EL3), it leaves
enabled interrupts that are its wake-up sources. Saving a GIC state
only means that its current configuration will be remembered somewhere
in data structures, but the configuration is not changed on suspend.
Everything that happens with interrupt configuration from this point
on is platform specific. Now there are 2 options: 1) EL3 will never
allow CPU0 to be powered down and the wake-up interrupt will indeed
propagate via GIC;
or 2) CPU0 will be powered down and the GIC may be
powered down as well, so an external help is needed to deal with
wake-up and resume (e.g. in order to react to a wake-up interrupt
while the GIC is down, and power up CPU0). This external help is
provided by some low-power processor outside of the Cortex-A cluster.

So the platform firmware is responsible for properly configuring a
wake-up path if GIC goes down. This is commonly handled by EL3
communicating with low-power processor. When the GIC power comes up,
the interrupt triggered by a peripheral is still active and the
software on Cortex-A cluster should be able to observe it once the GIC
state is restored, i.e. interrupts get enabled at GIC.

Thank you for the explanation.  Now the question is why can't we reset at
least the GIC CPU interface?

AFAIU, the guest should restore them in any case. The only things we need
know is the interrupt was received for a given guest. We can then queue it
wake-up the domain.

This seems to fit with the description on top of gic_dist_save() in Linux
GICv2 driver.

Can we rely on all PSCI compliant OSes to restore their own GIC again at
resume? The PSCI spec is not very clear in that regard (at the the
version I am looking at...) I am just asking so that we don't come up
with a solution that only works with Linux.
See PSCI 1.1 (DEN0022D) section 6.8. Each level should save its own context
because the PSCI implementations is allowed to shutdown the GIC.

Great, in that case we should be able to skip saving some of the GICD
registers too. We do need to save GICD_ISACTIVER, and GICD_ICFGR,
but we should be able to skip the others (GICD_ISENABLER,
GICD_IPRIORITYR, GICD_ITARGETSR). If we do, we still need to
re-initialize them as we do in gicv2_dist_init.

You are assuming a domain will handle properly the suspend/resume. I
don't think we can promise that as we call freeze/thaw.

Dho! That would break every single guest that has been forcefully
suspended :-/

Right, we can't do that (FYI I tested today the series with an unaware
domU and it all resumed correctly.)

But given that we only suspend/resume GICC_CTLR, GICC_PMR, GICC_BPR of
the GICC interface, it should be fine to re-initialize that. We do need
to be careful because the current implementation of gicv2_cpu_init
touches a bunch of GICD registers that we'll have to save separately for

See my review on patch #10. I suggested to move out GICC_CTLR, GICC_PMR, GICC_BPR in a separate helpers that could be called by gicv2_cpu_init and the new function.

A good name for that helper would be gicv2_cpu_if_up().


Julien Grall

Xen-devel mailing list



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