[3/3] irqchip/gic-v3: Save and restore distributor and re-distributor

Message ID 20230214233426.2994501-4-f.fainelli@gmail.com
State New
Headers
Series Save/restore for GICv3 |

Commit Message

Florian Fainelli Feb. 14, 2023, 11:34 p.m. UTC
  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.

Add support for saving and restoring the GIC distributor and
re-distributor in order to properly suspend and resume with a functional
system.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/irqchip/irq-gic-v3.c       | 258 +++++++++++++++++++++++++++++
 include/linux/irqchip/arm-gic-v3.h |   4 +
 2 files changed, 262 insertions(+)
  

Comments

Marc Zyngier Feb. 15, 2023, 8:02 a.m. UTC | #1
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.

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.

So unless you have a single security state system, this cannot
work. And apart from VMs (which by the way do not need any of this),
there is no GICv3-based system without EL3. If you know of one, please
let me know. And if it existed, then all the save/restore should
happen only when GICD_CTLR.DS==1.

To conclude, this patch doesn't do what it advertises, because it
*cannot* do it, by definition. The secure firmware is the only place
where this can be done.

	M.
  
Sudeep Holla Feb. 15, 2023, 12:10 p.m. UTC | #2
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.

Thanks Florian for starting this thread and sorry that I couldn't recollect
lots of the information when we chatted in the private about this. Marc
response triggered all the memory back.

> So unless you have a single security state system, this cannot
> work. And apart from VMs (which by the way do not need any of this),
> there is no GICv3-based system without EL3. If you know of one, please
> let me know. And if it existed, then all the save/restore should
> happen only when GICD_CTLR.DS==1.
>

Yes, now I remember the discussion we had probably almost 9-10 years
back when I first added the CPU PM notifiers for GICv3. I am sure we
would have discussed this at-least couple of times after that. Yet I
just got carried away by the fact that GICv2 does the save/restore and
this should also be possible. Sorry for that.

--
Regards,
Sudeep
  
Marc Zyngier Feb. 15, 2023, 2:40 p.m. UTC | #3
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.

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...

> 
> Thanks Florian for starting this thread and sorry that I couldn't recollect
> lots of the information when we chatted in the private about this. Marc
> response triggered all the memory back.
> 
> > So unless you have a single security state system, this cannot
> > work. And apart from VMs (which by the way do not need any of this),
> > there is no GICv3-based system without EL3. If you know of one, please
> > let me know. And if it existed, then all the save/restore should
> > happen only when GICD_CTLR.DS==1.
> >
> 
> Yes, now I remember the discussion we had probably almost 9-10 years
> back when I first added the CPU PM notifiers for GICv3. I am sure we
> would have discussed this at-least couple of times after that. Yet I
> just got carried away by the fact that GICv2 does the save/restore and
> this should also be possible. Sorry for that.

GICv2 is just as fsck'd. It is just that we pretend it works for the
sake of 32bit that may run in secure mode. On a 64bit machine, or in a
NS setup, it is doomed for the same reasons. There really isn't any
substitute for secure firmware here.

	M.
  
Sudeep Holla Feb. 15, 2023, 3:10 p.m. UTC | #4
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.

Also remember some firmware folks asking about LPI context, I am not sure
if there was any work done in that area.

> >
> > Thanks Florian for starting this thread and sorry that I couldn't recollect
> > lots of the information when we chatted in the private about this. Marc
> > response triggered all the memory back.
> > 
> > > So unless you have a single security state system, this cannot
> > > work. And apart from VMs (which by the way do not need any of this),
> > > there is no GICv3-based system without EL3. If you know of one, please
> > > let me know. And if it existed, then all the save/restore should
> > > happen only when GICD_CTLR.DS==1.
> > >
> > 
> > Yes, now I remember the discussion we had probably almost 9-10 years
> > back when I first added the CPU PM notifiers for GICv3. I am sure we
> > would have discussed this at-least couple of times after that. Yet I
> > just got carried away by the fact that GICv2 does the save/restore and
> > this should also be possible. Sorry for that.
> 
> GICv2 is just as fsck'd. It is just that we pretend it works for the
> sake of 32bit that may run in secure mode. On a 64bit machine, or in a
> NS setup, it is doomed for the same reasons. There really isn't any
> substitute for secure firmware here.

Fair enough and thanks for refreshing my memory on this.

Hi Florian,

I did little bit digging in the TF-A and found this.
plat_arm_gic_{save,resume}()in plat/arm/common/arm_gicv3.c which I assume
makes it platform specific code and hence not used on any other platform.
I also missed to see this earlier as I explicitly ignored the plat/ directory
assuming it is all platform specific code not shared across.

Not sure if the firmware on your platform is not using that or is it
different firmware altogether or may be TF-A forked before this change.
If it is missing anything, it would be good to get that fixed and look
at ways to generalise it.

--
Regards,
Sudeep
  
Florian Fainelli Feb. 15, 2023, 6:09 p.m. UTC | #5
On 2/15/23 07:10, Sudeep Holla 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.

The secure state of the GIC is being re-initialized coming out of 
suspend to DRAM since the chip lost its state, in fact we configure it 
the same as we did during cold boot and then the firmware goes on 
re-initializing the various secure interrupts it uses.

The non-secure state was not dealt with by the firmware, which prompted 
me to mimicking what the GICv2 driver does since there is an expectation 
from Linux that interrupts will be saved/restored.

> 
>> 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.

I suppose it is so obvious that saving/restoring must happen by trusted 
firmware that there is no point in putting that in BIG CAPITAL WORDS for 
people to know about it.

> 
> Also remember some firmware folks asking about LPI context, I am not sure
> if there was any work done in that area.
> 
>>>
>>> Thanks Florian for starting this thread and sorry that I couldn't recollect
>>> lots of the information when we chatted in the private about this. Marc
>>> response triggered all the memory back.
>>>
>>>> So unless you have a single security state system, this cannot
>>>> work. And apart from VMs (which by the way do not need any of this),
>>>> there is no GICv3-based system without EL3. If you know of one, please
>>>> let me know. And if it existed, then all the save/restore should
>>>> happen only when GICD_CTLR.DS==1.
>>>>
>>>
>>> Yes, now I remember the discussion we had probably almost 9-10 years
>>> back when I first added the CPU PM notifiers for GICv3. I am sure we
>>> would have discussed this at-least couple of times after that. Yet I
>>> just got carried away by the fact that GICv2 does the save/restore and
>>> this should also be possible. Sorry for that.
>>
>> GICv2 is just as fsck'd. It is just that we pretend it works for the
>> sake of 32bit that may run in secure mode. On a 64bit machine, or in a
>> NS setup, it is doomed for the same reasons. There really isn't any
>> substitute for secure firmware here.
> 
> Fair enough and thanks for refreshing my memory on this.
> 
> Hi Florian,
> 
> I did little bit digging in the TF-A and found this.
> plat_arm_gic_{save,resume}()in plat/arm/common/arm_gicv3.c which I assume
> makes it platform specific code and hence not used on any other platform.
> I also missed to see this earlier as I explicitly ignored the plat/ directory
> assuming it is all platform specific code not shared across.
> 
> Not sure if the firmware on your platform is not using that or is it
> different firmware altogether or may be TF-A forked before this change.
> If it is missing anything, it would be good to get that fixed and look
> at ways to generalise it.

As you may or may not remember we have our own ARM Trusted Firmware for 
better of for worse, I will put the code to save and restore the 
registers there. Thanks!
  
Marc Zyngier Feb. 16, 2023, 8:31 a.m. UTC | #6
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.
  

Patch

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 48b0e9aba27c..4caab61268d0 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -11,6 +11,7 @@ 
 #include <linux/cpu_pm.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/irqdomain.h>
 #include <linux/kstrtox.h>
 #include <linux/of.h>
@@ -57,6 +58,25 @@  struct gic_chip_data {
 	bool			has_rss;
 	unsigned int		ppi_nr;
 	struct partition_desc	**ppi_descs;
+#ifdef CONFIG_CPU_PM
+	u32			*saved_spi_conf;
+	u64			*saved_spi_target;
+	u32			*saved_spi_enable;
+	u32			*saved_spi_active;
+
+	u32			*saved_espi_conf;
+	u64			*saved_espi_target;
+	u32			*saved_espi_enable;
+	u32			*saved_espi_active;
+
+	u32			saved_ppi_conf;
+	u32			saved_ppi_enable;
+	u32			saved_ppi_active;
+
+	u32			*saved_eppi_conf;
+	u32			*saved_eppi_enable;
+	u32			*saved_eppi_active;
+#endif
 };
 
 static struct gic_chip_data gic_data __read_mostly;
@@ -1371,6 +1391,143 @@  static int gic_retrigger(struct irq_data *data)
 }
 
 #ifdef CONFIG_CPU_PM
+static void gic_rdist_save(void)
+{
+	struct gic_chip_data *gic = &gic_data;
+	void __iomem *rbase = gic_data_rdist_sgi_base();
+	unsigned int i;
+
+	gic->saved_ppi_conf = readl_relaxed(rbase + GICR_ICFGR0 + 4);
+	gic->saved_ppi_enable = readl_relaxed(rbase + GICR_ISENABLER0);
+	gic->saved_ppi_active = readl_relaxed(rbase + GICR_ICACTIVER0);
+
+	for (i = 0; i < DIV_ROUND_UP(gic->ppi_nr - 16, 32); i++) {
+		gic->saved_eppi_conf[i] =
+			readl_relaxed(rbase + GICR_ICFGRnE + i * 4);
+		gic->saved_eppi_enable[i] =
+			readl_relaxed(rbase + GICR_ISENABLERnE + i * 4);
+		gic->saved_eppi_active[i] =
+			readl_relaxed(rbase + GICR_ICACTIVERnE + i * 4);
+	}
+}
+
+static void gic_dist_save(void)
+{
+	struct gic_chip_data *gic = &gic_data;
+	void __iomem *base = gic_data.dist_base;
+	unsigned int i;
+
+	/* Save the SPIs first */
+	for (i = 2; i < DIV_ROUND_UP(GIC_LINE_NR, 16); i++)
+		gic->saved_spi_conf[i] =
+			readl_relaxed(base + GICD_ICFGR + i * 4);
+
+	for (i = 32; i < GIC_LINE_NR; i++)
+		gic->saved_spi_target[i] =
+			readq_relaxed(base + GICD_IROUTER + i * 8);
+
+	for (i = 1; i < DIV_ROUND_UP(GIC_LINE_NR, 32); i++) {
+		gic->saved_spi_enable[i] =
+			readl_relaxed(base + GICD_ISENABLER + i * 4);
+		gic->saved_spi_active[i] =
+			readl_relaxed(base + GICD_ISACTIVER + i * 4);
+	}
+
+	/* Save the EPIs next */
+	for (i = 0; i < DIV_ROUND_UP(GIC_ESPI_NR, 16); i++)
+		gic->saved_espi_conf[i] =
+			readl_relaxed(base + GICD_ICFGRnE + i * 4);
+
+	for (i = 0; i < GIC_ESPI_NR; i++)
+		gic->saved_espi_target[i] =
+			readq_relaxed(base + GICD_IROUTERnE + i * 8);
+
+	for (i = 0; i < DIV_ROUND_UP(GIC_ESPI_NR, 32); i++) {
+		gic->saved_espi_enable[i] =
+			readl_relaxed(base + GICD_ISENABLERnE + i * 4);
+		gic->saved_espi_active[i] =
+			readl_relaxed(base + GICD_ISACTIVERnE + i * 4);
+	}
+}
+
+static void gic_rdist_restore(void)
+{
+	struct gic_chip_data *gic = &gic_data;
+	void __iomem *rbase = gic_data_rdist_sgi_base();
+	unsigned int i;
+
+	writel_relaxed(gic->saved_ppi_conf, rbase + GICR_ICFGR0 + 4);
+	writel_relaxed(gic->saved_ppi_enable, rbase + GICR_ISENABLER0);
+	writel_relaxed(gic->saved_ppi_active, rbase + GICR_ICACTIVER0);
+
+	for (i = 0; i < DIV_ROUND_UP(gic->ppi_nr - 16, 32); i++) {
+		writel_relaxed(gic->saved_eppi_conf[i],
+				rbase + GICR_ICFGRnE + i * 4);
+		writel_relaxed(gic->saved_eppi_enable[i],
+				rbase + GICR_ISENABLERnE + i * 4);
+		writel_relaxed(gic->saved_eppi_active[i],
+				rbase + GICR_ICACTIVERnE + i * 4);
+	}
+}
+
+static void gic_dist_restore(void)
+{
+	struct gic_chip_data *gic = &gic_data;
+	void __iomem *base = gic_data.dist_base;
+	unsigned int i;
+
+	/* Ensure distributor is disabled */
+	writel_relaxed(0, base + GICD_CTLR);
+	gic_dist_wait_for_rwp();
+
+	/* Configure SPIs as non-secure Group-1. */
+	for (i = 32; i < GIC_LINE_NR; i += 32)
+		writel_relaxed(~0, base + GICD_IGROUPR + i / 8);
+
+	/* Restore the SPIs */
+	for (i = 2; i < DIV_ROUND_UP(GIC_LINE_NR, 16); i++)
+		writel_relaxed(gic->saved_spi_conf[i],
+			       base + GICD_ICFGR + i * 4);
+
+	for (i = 32; i < GIC_LINE_NR; i++)
+		writel_relaxed(gic->saved_spi_target[i],
+				base + GICD_IROUTER + i * 8);
+
+	for (i = 1; i < DIV_ROUND_UP(GIC_LINE_NR, 32); i++) {
+		writel_relaxed(gic->saved_spi_enable[i],
+				base + GICD_ISENABLER + i * 4);
+		writel_relaxed(gic->saved_spi_active[i],
+				base + GICD_ISACTIVER + i * 4);
+	}
+
+	/* Configure ESPIs as non-secure Group-1. */
+	for (i = 0; i < GIC_ESPI_NR; i += 32)
+		writel_relaxed(~0U, base + GICD_IGROUPRnE + i / 8);
+
+	/* Restore the ESPIs */
+	for (i = 0; i < DIV_ROUND_UP(GIC_ESPI_NR, 16); i++)
+		writel_relaxed(gic->saved_espi_conf[i],
+				base + GICD_ICFGRnE + i * 4);
+
+	for (i = 0; i < GIC_ESPI_NR; i++)
+		writeq_relaxed(gic->saved_espi_target[i],
+				base + GICD_IROUTERnE + i * 8);
+
+	for (i = 0; i < DIV_ROUND_UP(GIC_ESPI_NR, 32); i++) {
+		writel_relaxed(gic->saved_espi_enable[i],
+				base + GICD_ISENABLERnE + i * 4);
+		writel_relaxed(gic->saved_espi_active[i],
+				base + GICD_ISACTIVERnE + i * 4);
+	}
+
+	for (i = 0; i < GIC_ESPI_NR; i += 4)
+		writel_relaxed(GICD_INT_DEF_PRI_X4, base + GICD_IPRIORITYRnE + i);
+
+	/* Enable distributor with ARE, Group1 */
+	writel_relaxed(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1,
+		       base + GICD_CTLR);
+}
+
 static int gic_cpu_pm_notifier(struct notifier_block *self,
 			       unsigned long cmd, void *v)
 {
@@ -1380,12 +1537,20 @@  static int gic_cpu_pm_notifier(struct notifier_block *self,
 			gic_write_grpen1(0);
 			gic_enable_redist(false);
 		}
+		gic_rdist_save();
+		break;
+	case CPU_CLUSTER_PM_ENTER:
+		gic_dist_save();
 		break;
 	case CPU_PM_EXIT:
+		gic_rdist_restore();
 		if (gic_dist_security_disabled())
 			gic_enable_redist(true);
 		gic_cpu_sys_reg_init();
 		break;
+	case CPU_CLUSTER_PM_EXIT:
+		gic_dist_restore();
+		break;
 	}
 
 	return NOTIFY_OK;
@@ -1397,9 +1562,102 @@  static struct notifier_block gic_cpu_pm_notifier_block = {
 
 static int gic_cpu_pm_init(void)
 {
+	struct gic_chip_data *gic = &gic_data;
+	unsigned int spi_size = DIV_ROUND_UP(GIC_LINE_NR, 32);
+	unsigned int espi_size = DIV_ROUND_UP(GIC_ESPI_NR, 32);
+	unsigned int eppi_size = DIV_ROUND_UP(gic->ppi_nr - 16, 32);
+
+	gic->saved_spi_conf = kcalloc(DIV_ROUND_UP(GIC_LINE_NR, 16),
+				      sizeof(*gic->saved_spi_conf),
+				      GFP_KERNEL);
+	if (WARN_ON(!gic->saved_spi_conf))
+		return -ENOMEM;
+
+	gic->saved_spi_target = kcalloc(GIC_LINE_NR,
+					sizeof(*gic->saved_spi_target),
+					GFP_KERNEL);
+	if (WARN_ON(!gic->saved_spi_target))
+		goto out_free_spi_conf;
+
+	gic->saved_spi_enable = kcalloc(spi_size,
+					sizeof(*gic->saved_spi_enable),
+					GFP_KERNEL);
+	if (WARN_ON(!gic->saved_spi_enable))
+		goto out_free_spi_target;
+
+	gic->saved_spi_active = kcalloc(spi_size,
+					sizeof(*gic->saved_spi_active),
+					GFP_KERNEL);
+	if (WARN_ON(!gic->saved_spi_active))
+		goto out_free_spi_enable;
+
+	gic->saved_espi_conf = kcalloc(DIV_ROUND_UP(GIC_ESPI_NR, 16),
+				       sizeof(*gic->saved_espi_conf),
+				       GFP_KERNEL);
+	if (WARN_ON(!gic->saved_espi_conf))
+		goto out_free_spi_active;
+
+	gic->saved_espi_target = kcalloc(GIC_ESPI_NR,
+					 sizeof(*gic->saved_espi_target),
+					 GFP_KERNEL);
+	if (WARN_ON(!gic->saved_espi_target))
+		goto out_free_espi_conf;
+
+	gic->saved_espi_enable = kcalloc(espi_size,
+					 sizeof(*gic->saved_espi_enable),
+					 GFP_KERNEL);
+	if (WARN_ON(!gic->saved_espi_enable))
+		goto out_free_espi_target;
+
+	gic->saved_espi_active = kcalloc(espi_size,
+					 sizeof(*gic->saved_espi_active),
+					 GFP_KERNEL);
+	if (WARN_ON(!gic->saved_espi_active))
+		goto out_free_espi_enable;
+
+	gic->saved_eppi_conf = kcalloc(DIV_ROUND_UP(gic->ppi_nr - 16, 16),
+				       sizeof(*gic->saved_eppi_conf),
+				       GFP_KERNEL);
+	if (WARN_ON(!gic->saved_eppi_conf))
+		goto out_free_espi_active;
+
+	gic->saved_eppi_enable = kcalloc(eppi_size,
+					 sizeof(*gic->saved_eppi_enable),
+					 GFP_KERNEL);
+	if (WARN_ON(!gic->saved_eppi_enable))
+		goto out_free_eppi_conf;
+
+	gic->saved_eppi_active = kcalloc(eppi_size,
+					 sizeof(*gic->saved_eppi_active),
+					 GFP_KERNEL);
+	if (WARN_ON(!gic->saved_eppi_active))
+		goto out_free_eppi_enable;
+
 	cpu_pm_register_notifier(&gic_cpu_pm_notifier_block);
 
 	return 0;
+
+out_free_eppi_enable:
+	kfree(gic->saved_eppi_enable);
+out_free_eppi_conf:
+	kfree(gic->saved_eppi_conf);
+out_free_espi_active:
+	kfree(gic->saved_espi_active);
+out_free_espi_enable:
+	kfree(gic->saved_espi_enable);
+out_free_espi_target:
+	kfree(gic->saved_espi_target);
+out_free_espi_conf:
+	kfree(gic->saved_espi_conf);
+out_free_spi_active:
+	kfree(gic->saved_spi_active);
+out_free_spi_enable:
+	kfree(gic->saved_spi_enable);
+out_free_spi_target:
+	kfree(gic->saved_spi_target);
+out_free_spi_conf:
+	kfree(gic->saved_spi_conf);
+	return -ENOMEM;
 }
 
 #else
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 728691365464..40483530cadd 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -229,13 +229,17 @@ 
  */
 #define GICR_IGROUPR0			GICD_IGROUPR
 #define GICR_ISENABLER0			GICD_ISENABLER
+#define GICR_ISENABLERnE		GICD_ISENABLERnE
 #define GICR_ICENABLER0			GICD_ICENABLER
 #define GICR_ISPENDR0			GICD_ISPENDR
 #define GICR_ICPENDR0			GICD_ICPENDR
 #define GICR_ISACTIVER0			GICD_ISACTIVER
+#define GICR_ISACTIVERnE		GICD_ISACTIVERnE
 #define GICR_ICACTIVER0			GICD_ICACTIVER
+#define GICR_ICACTIVERnE		GICD_ICACTIVERnE
 #define GICR_IPRIORITYR0		GICD_IPRIORITYR
 #define GICR_ICFGR0			GICD_ICFGR
+#define GICR_ICFGRnE			GICD_ICFGRnE
 #define GICR_IGRPMODR0			GICD_IGRPMODR
 #define GICR_NSACR			GICD_NSACR