Re: [PATCH v2 04/10] irqchip/gic-v5: Serialise ITS invalidation commands

[Lorenzo Pieralisi <lpieralisi@kernel.org>]

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@kernel.org>
> Link: https://sashiko.dev/#/patchset/20261016155741.979985-1-sascha.bischoff@arm.com?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@vger.kernel.org
> Signed-off-by: Sascha Bischoff <sascha.bischoff@arm.com>
> ---
>  drivers/irqchip/irq-gic-v5-its.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Acked-by: Lorenzo Pieralisi <lpieralisi@kernel.org>

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