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:
> On 14/11/2018 22:45, Stefano Stabellini wrote:
> > 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.
> 
> 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
suspend/resume.


> Furthermore, we still have to suspend/resume other drivers in Xen. I 
> think this is a bit painful to have to rely on every drivers to deal 
> with their interrupts.

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