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

[Julien Grall <Julien.Grall@arm.com>]

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@arm.com>
>>>>> 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@arm.com>
>>>>>>> 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.

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.

Cheers,

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