Messages in this thread | | | Date | Thu, 16 Feb 2023 08:31:06 +0000 | From | Marc Zyngier <> | Subject | Re: [PATCH 3/3] irqchip/gic-v3: Save and restore distributor and re-distributor |
| |
On Wed, 15 Feb 2023 15:10:48 +0000, Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Wed, Feb 15, 2023 at 02:40:04PM +0000, Marc Zyngier wrote: > > On Wed, 15 Feb 2023 12:10:50 +0000, > > Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > On Wed, Feb 15, 2023 at 08:02:20AM +0000, Marc Zyngier wrote: > > > > On Tue, 14 Feb 2023 23:34:26 +0000, > > > > Florian Fainelli <f.fainelli@gmail.com> wrote: > > > > > > > > > > On platforms implementing Suspend to RAM where the GIC loses power, we > > > > > are not properly saving and restoring the GIC distributor and > > > > > re-distributor registers thus leading to the system resuming without any > > > > > functional interrupts. > > > > > > > > The real question is *why* we need any of this. On any decent system, > > > > this is the firmware's job. It was *never* the OS GIC driver's job > > > > the first place. > > > > > > > > > > Completely agreed on the points you have made here, no disagreement. > > > However I would like to iterate some of the arguments/concerns the > > > firmware teams I have interacted in the past have made around this. > > > And this is while ago(couple of years) and they may have different > > > views. I am repeating them as I think it may be still valid on some > > > systems so that we can make some suggestions if we have here. > > > > > > > Importantly, the OS cannot save the full state: a large part of it is > > > > only accessible via secure, and Linux doesn't run in secure mode. How > > > > do you restore the group configuration, for example? Oh wait, you > > > > don't even save it. > > > > > > > > > > Agreed, we can't manage secure side configurations. But one of the concern > > > was about the large memory footprint to save the larger non-secure GIC > > > context in the smaller secure memory. > > > > > > One of the suggestion at the time was to carve out a chunk of non-secure > > > memory and let the secure side use the same for context save and restore. > > > Not sure if this was tried out especially for the GIC. I may need to > > > chase that with the concerned teams. > > > > The main issue is that you still need secure memory to save the secure > > state, as leaving it in NS memory would be an interesting attack > > vector! Other than that, I see no issue with FW carving out the memory > > it needs to save/restore the NS state of the GIC. > > > > Yes I meant NS memory for only NS state of GIC. > > > Note that this isn't only the (re-)distributor(s) PPI/SPI registers. > > The LPI setup must also be saved, and that includes all the ITS > > registers. I'm surprised the FW folks are, all of a sudden, > > discovering this requirements. It isn't like the GIC architecture is a > > novelty, and they have had ample time to review the spec... > > > > I understand your concern about late realisation 😄. > > Another issue in general I see with reference firmware stack(like > Trusted Firmware in this case) is that the requirements are driven from > the reference platforms which may not have this GIC save/restore > requirement as they are in always on domain and it is then made platform > specific problem in that project which may not be ideal and may result > in somewhat misleading indirectly other firmware developers using > it.
Yeah, that's the usual state of affair. Unrealistic platforms, no insight (and more generally no interest) in the actual usage model.
Still, most people got it right, so I guess they must be reading the spec. How comes this was never picked from contributions to TF-A? Surely duplication of platform code should be a massive hint to the firmware maintainers?
Thanks,
M.
-- Without deviation from the norm, progress is not possible.
| |