Re: [PATCH v2 04/10] irqchip/gic-v5: Serialise ITS invalidation commands
From: Lorenzo Pieralisi
Date: Thu Nov 05 2026 - 03:33:23 EST
On Wed, Nov 04, 2026 at 05:47:58PM +0000, Sascha Bischoff wrote:
> The ITS uses the shared DIDR and EIDR registers to select the DeviceID
> and EventID whose translation is to be invalidated. Event
> invalidations for different IRQs can overlap, as can an event
> invalidation and a device invalidation. The per-IRQ descriptor lock
> does not serialise these commands, so writes can interleave and cause
> an invalidation to use the DeviceID or EventID of another IRQ being
> invalidated concurrently on a different CPU.
>
> Serialise the command register writes and STATUSR.IDLE polls for both
> invalidation paths with a per-ITS raw spinlock. The KVM
> gicv5_its_set_vcpu_affinity() callback calls the event invalidation
> helper via irq_set_vcpu_affinity(), which holds the IRQ descriptor
> lock. This path cannot sleep, so use atomic polling.
Same comment here as IRS sync. This one though looks like a host
driver bug that can happen regardless of KVM, so I'd just fix it
and remove/rephrase this paragraph.
> Fixes: 57d72196dfc8 ("irqchip/gic-v5: Add GICv5 ITS support")
> Reported-by: Sashiko AI <sashiko-bot@xxxxxxxxxx>
> Link: https://sashiko.dev/#/patchset/20261016155741.979985-1-sascha.bischoff@xxxxxxx?part=11
Again, this is linking to a report on a series that wasn't merged
yet but probably it makes sense to leave it there for context.
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sascha Bischoff <sascha.bischoff@xxxxxxx>
> ---
> drivers/irqchip/irq-gic-v5-its.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
Acked-by: Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx>
> diff --git a/drivers/irqchip/irq-gic-v5-its.c b/drivers/irqchip/irq-gic-v5-its.c
> index 28e39b065de0e..6d98f3dbe34b6 100644
> --- a/drivers/irqchip/irq-gic-v5-its.c
> +++ b/drivers/irqchip/irq-gic-v5-its.c
> @@ -16,6 +16,7 @@
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> #include <linux/slab.h>
> +#include <linux/spinlock.h>
>
> #include <linux/irqchip.h>
> #include <linux/irqchip/arm-gic-v5.h>
> @@ -28,6 +29,8 @@
> struct gicv5_its_chip_data {
> struct xarray its_devices;
> struct mutex dev_alloc_lock;
> + /* Event and device invalidation commands share DIDR. */
> + raw_spinlock_t inv_lock;
> struct fwnode_handle *fwnode;
> struct gicv5_its_devtab_cfg devtab_cfgr;
> void __iomem *its_base;
> @@ -129,6 +132,8 @@ static int gicv5_its_itt_cache_inv(struct gicv5_its_chip_data *its, u32 device_i
> eidr = FIELD_PREP(GICV5_ITS_EIDR_EVENTID, event_id);
> eventr = FIELD_PREP(GICV5_ITS_INV_EVENTR_I, 0x1);
>
> + guard(raw_spinlock_irqsave)(&its->inv_lock);
> +
> its_writeq_relaxed(its, didr, GICV5_ITS_DIDR);
> its_writel_relaxed(its, eidr, GICV5_ITS_EIDR);
> its_writel_relaxed(its, eventr, GICV5_ITS_INV_EVENTR);
> @@ -371,6 +376,9 @@ static int gicv5_its_device_cache_inv(struct gicv5_its_chip_data *its,
> FIELD_PREP(GICV5_ITS_INV_DEVICER_EVENTID_BITS,
> its_dev->itt_cfg.event_id_bits) |
> FIELD_PREP(GICV5_ITS_INV_DEVICER_L1, 0x0);
> +
> + guard(raw_spinlock_irqsave)(&its->inv_lock);
> +
> its_writeq_relaxed(its, didr, GICV5_ITS_DIDR);
> its_writel_relaxed(its, devicer, GICV5_ITS_INV_DEVICER);
>
> @@ -1112,6 +1120,7 @@ static int __init gicv5_its_init_bases(void __iomem *its_base, struct fwnode_han
> return -ENOMEM;
>
> mutex_init(&its_node->dev_alloc_lock);
> + raw_spin_lock_init(&its_node->inv_lock);
> xa_init(&its_node->its_devices);
> its_node->fwnode = handle;
> its_node->its_base = its_base;
>
> --
> 2.34.1
>