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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 14 Nov 2018, Julien Grall wrote:
> Hi,
> 
> 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>
> > > > wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On 11/12/18 4:52 PM, Mirela Simonovic wrote:
> > > > > > Hi Julien,
> > > > > 
> > > > > Hi,
> > > > > 
> > > > > > 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?
> > > > > 
> > > > 
> > > > 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
> > > to
> > > know is the interrupt was received for a given guest. We can then queue it
> > > and
> > > 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.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel