From: Mykola Kvach <mykola_kvach@epam.com>
Handle system suspend/resume for GICv3 with an ITS present so LPIs keep
working after firmware powers the GIC down. Snapshot the CPU interface,
distributor and last-CPU redistributor state, disable the ITS to cache its
CTLR/CBASER/BASER registers, then restore everything and re-arm the
collection on resume.
Add list_for_each_entry_continue_reverse() in list.h for the ITS suspend
error path that needs to roll back partially saved state.
Based on Linux commit: dba0bc7b76dc ("irqchip/gic-v3-its: Add ability to save/restore ITS state")
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
xen/arch/arm/gic-v3-its.c | 91 +++++++++++++++++++++++++++
xen/arch/arm/gic-v3.c | 15 ++++-
xen/arch/arm/include/asm/gic_v3_its.h | 23 +++++++
xen/include/xen/list.h | 14 +++++
4 files changed, 140 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
index 34833166ad..08a3d8d1ef 100644
--- a/xen/arch/arm/gic-v3-its.c
+++ b/xen/arch/arm/gic-v3-its.c
@@ -1209,6 +1209,97 @@ int gicv3_its_init(void)
return 0;
}
+#ifdef CONFIG_SYSTEM_SUSPEND
+int gicv3_its_suspend(void)
+{
+ struct host_its *its;
+ int ret;
+
+ list_for_each_entry(its, &host_its_list, entry)
+ {
+ unsigned int i;
+ void __iomem *base = its->its_base;
+
+ its->suspend_ctx.ctlr = readl_relaxed(base + GITS_CTLR);
+ ret = gicv3_disable_its(its);
+ if ( ret )
+ {
+ writel_relaxed(its->suspend_ctx.ctlr, base + GITS_CTLR);
+ goto err;
+ }
+
+ its->suspend_ctx.cbaser = readq_relaxed(base + GITS_CBASER);
+
+ for (i = 0; i < GITS_BASER_NR_REGS; i++) {
+ uint64_t baser = readq_relaxed(base + GITS_BASER0 + i * 8);
+
+ if ( !(baser & GITS_VALID_BIT) )
+ continue;
+
+ its->suspend_ctx.baser[i] = baser;
+ }
+ }
+
+ return 0;
+
+ err:
+ list_for_each_entry_continue_reverse(its, &host_its_list, entry)
+ writel_relaxed(its->suspend_ctx.ctlr, its->its_base + GITS_CTLR);
+
+ return ret;
+}
+
+void gicv3_its_resume(void)
+{
+ struct host_its *its;
+ int ret;
+
+ list_for_each_entry(its, &host_its_list, entry)
+ {
+ void __iomem *base;
+ unsigned int i;
+
+ base = its->its_base;
+
+ /*
+ * Make sure that the ITS is disabled. If it fails to quiesce,
+ * don't restore it since writing to CBASER or BASER<n>
+ * registers is undefined according to the GIC v3 ITS
+ * Specification.
+ *
+ * Firmware resuming with the ITS enabled is terminally broken.
+ */
+ WARN_ON(readl_relaxed(base + GITS_CTLR) & GITS_CTLR_ENABLE);
+ ret = gicv3_disable_its(its);
+ if ( ret )
+ continue;
+
+ writeq_relaxed(its->suspend_ctx.cbaser, base + GITS_CBASER);
+
+ /*
+ * Writing CBASER resets CREADR to 0, so make CWRITER and
+ * cmd_write line up with it.
+ */
+ writeq_relaxed(0, base + GITS_CWRITER);
+
+ /* Restore GITS_BASER from the value cache. */
+ for (i = 0; i < GITS_BASER_NR_REGS; i++) {
+ uint64_t baser = its->suspend_ctx.baser[i];
+
+ if ( !(baser & GITS_VALID_BIT) )
+ continue;
+
+ writeq_relaxed(baser, base + GITS_BASER0 + i * 8);
+ }
+ writel_relaxed(its->suspend_ctx.ctlr, base + GITS_CTLR);
+ }
+
+ ret = gicv3_its_setup_collection(smp_processor_id());
+ if ( ret )
+ panic("GICv3: ITS: resume setup collection failed: %d\n", ret);
+}
+
+#endif /* CONFIG_SYSTEM_SUSPEND */
/*
* Local variables:
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index dc5e58066d..cde76b182f 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -857,7 +857,7 @@ static bool gicv3_enable_lpis(void)
return true;
}
-static int __init gicv3_populate_rdist(void)
+static int gicv3_populate_rdist(void)
{
int i;
uint32_t aff;
@@ -927,7 +927,7 @@ static int __init gicv3_populate_rdist(void)
ret = gicv3_lpi_init_rdist(ptr);
if ( ret && ret != -ENODEV )
{
- printk("GICv3: CPU%d: Cannot initialize LPIs: %u\n",
+ printk("GICv3: CPU%d: Cannot initialize LPIs: %d\n",
smp_processor_id(), ret);
break;
}
@@ -2096,9 +2096,13 @@ static int gicv3_suspend(void)
gicv3_disable_interface();
+ ret = gicv3_its_suspend();
+ if ( ret )
+ goto out_enable_iface;
+
ret = gicv3_disable_redist();
if ( ret )
- return out_enable_iface;
+ goto out_its_resume;
/* Save GICR configuration */
gicv3_redist_wait_for_rwp();
@@ -2133,6 +2137,9 @@ static int gicv3_suspend(void)
return 0;
+ out_its_resume:
+ gicv3_its_resume();
+
out_enable_iface:
gicv3_hyp_enable(true);
WRITE_SYSREG(gicv3_ctx.cpu.ctlr, ICC_CTLR_EL1);
@@ -2205,6 +2212,8 @@ static void gicv3_resume(void)
gicv3_redist_wait_for_rwp();
+ gicv3_its_resume();
+
WRITE_SYSREG(gicv3_ctx.cpu.sre_el2, ICC_SRE_EL2);
isb();
diff --git a/xen/arch/arm/include/asm/gic_v3_its.h b/xen/arch/arm/include/asm/gic_v3_its.h
index fc5a84892c..492c468ae0 100644
--- a/xen/arch/arm/include/asm/gic_v3_its.h
+++ b/xen/arch/arm/include/asm/gic_v3_its.h
@@ -129,6 +129,13 @@ struct host_its {
spinlock_t cmd_lock;
void *cmd_buf;
unsigned int flags;
+#ifdef CONFIG_SYSTEM_SUSPEND
+ struct suspend_ctx {
+ uint32_t ctlr;
+ uint64_t cbaser;
+ uint64_t baser[GITS_BASER_NR_REGS];
+ } suspend_ctx;
+#endif
};
/* Map a collection for this host CPU to each host ITS. */
@@ -204,6 +211,11 @@ uint64_t gicv3_its_get_cacheability(void);
uint64_t gicv3_its_get_shareability(void);
unsigned int gicv3_its_get_memflags(void);
+#ifdef CONFIG_SYSTEM_SUSPEND
+int gicv3_its_suspend(void);
+void gicv3_its_resume(void);
+#endif
+
#else
#ifdef CONFIG_ACPI
@@ -271,6 +283,17 @@ static inline int gicv3_its_make_hwdom_dt_nodes(const struct domain *d,
return 0;
}
+#ifdef CONFIG_SYSTEM_SUSPEND
+static inline int gicv3_its_suspend(void)
+{
+ return 0;
+}
+
+static inline void gicv3_its_resume(void)
+{
+}
+#endif
+
#endif /* CONFIG_HAS_ITS */
#endif
diff --git a/xen/include/xen/list.h b/xen/include/xen/list.h
index 98d8482dab..87c940ac3b 100644
--- a/xen/include/xen/list.h
+++ b/xen/include/xen/list.h
@@ -535,6 +535,20 @@ static inline void list_splice_init(struct list_head *list,
&(pos)->member != (head); \
(pos) = list_entry((pos)->member.next, typeof(*(pos)), member))
+/**
+ * list_for_each_entry_continue_reverse - iterate backwards from the given point
+ * @pos: the type * to use as a loop cursor.
+ * @head: the head for your list.
+ * @member: the name of the list_head within the struct.
+ *
+ * Start to iterate over list of given type backwards, continuing after
+ * the current position.
+ */
+#define list_for_each_entry_continue_reverse(pos, head, member) \
+ for ((pos) = list_entry((pos)->member.prev, typeof(*(pos)), member); \
+ &(pos)->member != (head); \
+ (pos) = list_entry((pos)->member.prev, typeof(*(pos)), member))
+
/**
* list_for_each_entry_from - iterate over list of given type from the
* current point
--
2.43.0
On 11.12.25 20:43, Mykola Kvach wrote:
Hello Mykola
> From: Mykola Kvach <mykola_kvach@epam.com>
>
> Handle system suspend/resume for GICv3 with an ITS present so LPIs keep
> working after firmware powers the GIC down. Snapshot the CPU interface,
> distributor and last-CPU redistributor state, disable the ITS to cache its
> CTLR/CBASER/BASER registers, then restore everything and re-arm the
> collection on resume.
>
> Add list_for_each_entry_continue_reverse() in list.h for the ITS suspend
> error path that needs to roll back partially saved state.
>
> Based on Linux commit: dba0bc7b76dc ("irqchip/gic-v3-its: Add ability to save/restore ITS state")
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> xen/arch/arm/gic-v3-its.c | 91 +++++++++++++++++++++++++++
> xen/arch/arm/gic-v3.c | 15 ++++-
> xen/arch/arm/include/asm/gic_v3_its.h | 23 +++++++
> xen/include/xen/list.h | 14 +++++
> 4 files changed, 140 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index 34833166ad..08a3d8d1ef 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -1209,6 +1209,97 @@ int gicv3_its_init(void)
> return 0;
> }
>
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +int gicv3_its_suspend(void)
> +{
> + struct host_its *its;
> + int ret;
> +
> + list_for_each_entry(its, &host_its_list, entry)
> + {
> + unsigned int i;
> + void __iomem *base = its->its_base;
> +
> + its->suspend_ctx.ctlr = readl_relaxed(base + GITS_CTLR);
> + ret = gicv3_disable_its(its);
> + if ( ret )
> + {
> + writel_relaxed(its->suspend_ctx.ctlr, base + GITS_CTLR);
> + goto err;
> + }
> +
> + its->suspend_ctx.cbaser = readq_relaxed(base + GITS_CBASER);
> +
> + for (i = 0; i < GITS_BASER_NR_REGS; i++) {
> + uint64_t baser = readq_relaxed(base + GITS_BASER0 + i * 8);
> +
> + if ( !(baser & GITS_VALID_BIT) )
> + continue;
> +
> + its->suspend_ctx.baser[i] = baser;
> + }
> + }
> +
> + return 0;
> +
> + err:
> + list_for_each_entry_continue_reverse(its, &host_its_list, entry)
> + writel_relaxed(its->suspend_ctx.ctlr, its->its_base + GITS_CTLR);
> +
> + return ret;
> +}
> +
> +void gicv3_its_resume(void)
> +{
> + struct host_its *its;
> + int ret;
> +
> + list_for_each_entry(its, &host_its_list, entry)
> + {
> + void __iomem *base;
> + unsigned int i;
> +
> + base = its->its_base;
> +
> + /*
> + * Make sure that the ITS is disabled. If it fails to quiesce,
> + * don't restore it since writing to CBASER or BASER<n>
> + * registers is undefined according to the GIC v3 ITS
> + * Specification.
> + *
> + * Firmware resuming with the ITS enabled is terminally broken.
> + */
> + WARN_ON(readl_relaxed(base + GITS_CTLR) & GITS_CTLR_ENABLE);
> + ret = gicv3_disable_its(its);
> + if ( ret )
> + continue;
If ITS fails to disable (quiesce), the code skips restoration and ITS
remains in an unconfigured state. However, immediately after the loop ...
> +
> + writeq_relaxed(its->suspend_ctx.cbaser, base + GITS_CBASER);
> +
> + /*
> + * Writing CBASER resets CREADR to 0, so make CWRITER and
> + * cmd_write line up with it.
NIT: The variable "cmd_write" does not exist in the Xen driver. As I
understand, this comment was ported from the Linux kernel driver as is,
which maintains a software shadow copy of the write pointer named
"cmd_write".
> + */
> + writeq_relaxed(0, base + GITS_CWRITER);
> +
> + /* Restore GITS_BASER from the value cache. */
> + for (i = 0; i < GITS_BASER_NR_REGS; i++) {
> + uint64_t baser = its->suspend_ctx.baser[i];
> +
> + if ( !(baser & GITS_VALID_BIT) )
> + continue;
> +
> + writeq_relaxed(baser, base + GITS_BASER0 + i * 8);
> + }
> + writel_relaxed(its->suspend_ctx.ctlr, base + GITS_CTLR);
> + }
> +
> + ret = gicv3_its_setup_collection(smp_processor_id());
... this function iterates over all host ITS instances (including the
one we skipped) and attempts to send MAPC commands. I am afraid, that
attempting to write to the command queue of an uninitialized/unrestored
ITS might have bad consequences.
> + if ( ret )
> + panic("GICv3: ITS: resume setup collection failed: %d\n", ret);
> +}
> +
> +#endif /* CONFIG_SYSTEM_SUSPEND */
>
[snip]
Hi Oleksandr,
Thanks for the review.
On Sat, Jan 31, 2026 at 7:00 PM Oleksandr Tyshchenko
<olekstysh@gmail.com> wrote:
>
>
>
> On 11.12.25 20:43, Mykola Kvach wrote:
>
> Hello Mykola
>
> > From: Mykola Kvach <mykola_kvach@epam.com>
> >
> > Handle system suspend/resume for GICv3 with an ITS present so LPIs keep
> > working after firmware powers the GIC down. Snapshot the CPU interface,
> > distributor and last-CPU redistributor state, disable the ITS to cache its
> > CTLR/CBASER/BASER registers, then restore everything and re-arm the
> > collection on resume.
> >
> > Add list_for_each_entry_continue_reverse() in list.h for the ITS suspend
> > error path that needs to roll back partially saved state.
> >
> > Based on Linux commit: dba0bc7b76dc ("irqchip/gic-v3-its: Add ability to save/restore ITS state")
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> > xen/arch/arm/gic-v3-its.c | 91 +++++++++++++++++++++++++++
> > xen/arch/arm/gic-v3.c | 15 ++++-
> > xen/arch/arm/include/asm/gic_v3_its.h | 23 +++++++
> > xen/include/xen/list.h | 14 +++++
> > 4 files changed, 140 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> > index 34833166ad..08a3d8d1ef 100644
> > --- a/xen/arch/arm/gic-v3-its.c
> > +++ b/xen/arch/arm/gic-v3-its.c
> > @@ -1209,6 +1209,97 @@ int gicv3_its_init(void)
> > return 0;
> > }
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +int gicv3_its_suspend(void)
> > +{
> > + struct host_its *its;
> > + int ret;
> > +
> > + list_for_each_entry(its, &host_its_list, entry)
> > + {
> > + unsigned int i;
> > + void __iomem *base = its->its_base;
> > +
> > + its->suspend_ctx.ctlr = readl_relaxed(base + GITS_CTLR);
> > + ret = gicv3_disable_its(its);
> > + if ( ret )
> > + {
> > + writel_relaxed(its->suspend_ctx.ctlr, base + GITS_CTLR);
> > + goto err;
> > + }
> > +
> > + its->suspend_ctx.cbaser = readq_relaxed(base + GITS_CBASER);
> > +
> > + for (i = 0; i < GITS_BASER_NR_REGS; i++) {
> > + uint64_t baser = readq_relaxed(base + GITS_BASER0 + i * 8);
> > +
> > + if ( !(baser & GITS_VALID_BIT) )
> > + continue;
> > +
> > + its->suspend_ctx.baser[i] = baser;
> > + }
> > + }
> > +
> > + return 0;
> > +
> > + err:
> > + list_for_each_entry_continue_reverse(its, &host_its_list, entry)
> > + writel_relaxed(its->suspend_ctx.ctlr, its->its_base + GITS_CTLR);
> > +
> > + return ret;
> > +}
> > +
> > +void gicv3_its_resume(void)
> > +{
> > + struct host_its *its;
> > + int ret;
> > +
> > + list_for_each_entry(its, &host_its_list, entry)
> > + {
> > + void __iomem *base;
> > + unsigned int i;
> > +
> > + base = its->its_base;
> > +
> > + /*
> > + * Make sure that the ITS is disabled. If it fails to quiesce,
> > + * don't restore it since writing to CBASER or BASER<n>
> > + * registers is undefined according to the GIC v3 ITS
> > + * Specification.
> > + *
> > + * Firmware resuming with the ITS enabled is terminally broken.
> > + */
> > + WARN_ON(readl_relaxed(base + GITS_CTLR) & GITS_CTLR_ENABLE);
> > + ret = gicv3_disable_its(its);
> > + if ( ret )
> > + continue;
>
> If ITS fails to disable (quiesce), the code skips restoration and ITS
> remains in an unconfigured state. However, immediately after the loop ...
>
>
> > +
> > + writeq_relaxed(its->suspend_ctx.cbaser, base + GITS_CBASER);
> > +
> > + /*
> > + * Writing CBASER resets CREADR to 0, so make CWRITER and
> > + * cmd_write line up with it.
>
> NIT: The variable "cmd_write" does not exist in the Xen driver. As I
> understand, this comment was ported from the Linux kernel driver as is,
> which maintains a software shadow copy of the write pointer named
> "cmd_write".
Good catch, thanks.
You’re right - cmd_write doesn’t exist in the Xen driver; this comment
was ported from Linux. I’ll drop the cmd_write mention and keep the
comment focused on aligning CWRITER with the reset CREADR.
>
>
> > + */
> > + writeq_relaxed(0, base + GITS_CWRITER);
> > +
> > + /* Restore GITS_BASER from the value cache. */
> > + for (i = 0; i < GITS_BASER_NR_REGS; i++) {
> > + uint64_t baser = its->suspend_ctx.baser[i];
> > +
> > + if ( !(baser & GITS_VALID_BIT) )
> > + continue;
> > +
> > + writeq_relaxed(baser, base + GITS_BASER0 + i * 8);
> > + }
> > + writel_relaxed(its->suspend_ctx.ctlr, base + GITS_CTLR);
> > + }
> > +
> > + ret = gicv3_its_setup_collection(smp_processor_id());
>
> ... this function iterates over all host ITS instances (including the
> one we skipped) and attempts to send MAPC commands. I am afraid, that
> attempting to write to the command queue of an uninitialized/unrestored
> ITS might have bad consequences.
Good catch, thanks.
Since gicv3_its_setup_collection() iterates over all host ITS instances,
it may send MAPC commands through an ITS that we intentionally skipped or
failed to restore. That’s unsafe. I’ll fix this by invoking collection
setup only for ITS instances successfully restored in this loop.
Best regards,
Mykola
>
> > + if ( ret )
> > + panic("GICv3: ITS: resume setup collection failed: %d\n", ret);
> > +}
> > +
> > +#endif /* CONFIG_SYSTEM_SUSPEND */
> >
>
> [snip]
>
On 11.12.2025 19:43, Mykola Kvach wrote: > --- a/xen/include/xen/list.h > +++ b/xen/include/xen/list.h > @@ -535,6 +535,20 @@ static inline void list_splice_init(struct list_head *list, > &(pos)->member != (head); \ > (pos) = list_entry((pos)->member.next, typeof(*(pos)), member)) > > +/** > + * list_for_each_entry_continue_reverse - iterate backwards from the given point > + * @pos: the type * to use as a loop cursor. > + * @head: the head for your list. > + * @member: the name of the list_head within the struct. > + * > + * Start to iterate over list of given type backwards, continuing after > + * the current position. > + */ > +#define list_for_each_entry_continue_reverse(pos, head, member) \ > + for ((pos) = list_entry((pos)->member.prev, typeof(*(pos)), member); \ > + &(pos)->member != (head); \ > + (pos) = list_entry((pos)->member.prev, typeof(*(pos)), member)) > + > /** > * list_for_each_entry_from - iterate over list of given type from the > * current point While not said so anywhere, I understand this is taken from Linux. Maybe we should indeed take it verbatim (as far as possible, i.e. without the use of list_entry_is_head() which we don't have yet), but I'd like to point out that in the comment "continuing after the current position" is ambiguous. In list order, what is meant is "before the current position"; "after" is correct only when considering iteration direction. Personally I would much prefer if this was disambiguated. Jan
Hi Jan, Thank you for the review. On Mon, Dec 15, 2025 at 1:39 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 11.12.2025 19:43, Mykola Kvach wrote: > > --- a/xen/include/xen/list.h > > +++ b/xen/include/xen/list.h > > @@ -535,6 +535,20 @@ static inline void list_splice_init(struct list_head *list, > > &(pos)->member != (head); \ > > (pos) = list_entry((pos)->member.next, typeof(*(pos)), member)) > > > > +/** > > + * list_for_each_entry_continue_reverse - iterate backwards from the given point > > + * @pos: the type * to use as a loop cursor. > > + * @head: the head for your list. > > + * @member: the name of the list_head within the struct. > > + * > > + * Start to iterate over list of given type backwards, continuing after > > + * the current position. > > + */ > > +#define list_for_each_entry_continue_reverse(pos, head, member) \ > > + for ((pos) = list_entry((pos)->member.prev, typeof(*(pos)), member); \ > > + &(pos)->member != (head); \ > > + (pos) = list_entry((pos)->member.prev, typeof(*(pos)), member)) > > + > > /** > > * list_for_each_entry_from - iterate over list of given type from the > > * current point > > While not said so anywhere, I understand this is taken from Linux. Maybe we > should indeed take it verbatim (as far as possible, i.e. without the use of > list_entry_is_head() which we don't have yet), but I'd like to point out > that in the comment "continuing after the current position" is ambiguous. > In list order, what is meant is "before the current position"; "after" is > correct only when considering iteration direction. Personally I would much > prefer if this was disambiguated. Got it. I’ll disambiguate the wording while keeping it close to the Linux text, e.g.: “Start to iterate over list of given type backwards, continuing with the entry preceding the current position (i.e. starting from pos->member.prev).” Best regards, Mykola > > Jan
© 2016 - 2026 Red Hat, Inc.