[PATCH 0/6] Some random fixes and cleanups to irqchip/gic-v3-its

[PATCH 0/6] Some random fixes and cleanups to irqchip/gic-v3-its

[Kemeng Shi]

There are some random fixes and cleanups to irqchip/gic-v3-its. More
details can be found in respective patches.
Thanks.

Kemeng Shi (6):
  irqchip/gic-v3-its: Fix LPI range leak and refactor error handler in
    its_lpi_alloc()
  irqchip/gic-v3-its: Fix memleak in its_probe_one()
  irqchip/gic-v3-its: Fix its node leak in gic_acpi_parse_madt_its()
  irqchip/gic-v3-its: Add ITS address info in more error logs
  irqchip/gic-v3-its: fix typo in comments
  irqchip/gic-v3-its: some minor cleanups

 drivers/irqchip/irq-gic-v3-its.c | 68 ++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 29 deletions(-)

-- 
2.36.1

[PATCH 1/6] irqchip/gic-v3-its: Fix LPI range leak and refactor error handler in its_lpi_alloc()

[Kemeng Shi]

Fix the LIP range leak when bitmap_zalloc() failed. Besides refactor
error handling code to make it a little simpler.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 291d7668cc8d..2b7b546c43c8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2217,10 +2217,9 @@ static int __init its_lpi_init(u32 id_bits)
 static unsigned long *its_lpi_alloc(int nr_irqs, u32 *base, int *nr_ids)
 {
 	unsigned long *bitmap = NULL;
-	int err = 0;
 
 	do {
-		err = alloc_lpi_range(nr_irqs, base);
+		int err = alloc_lpi_range(nr_irqs, base);
 		if (!err)
 			break;
 
@@ -2228,22 +2227,20 @@ static unsigned long *its_lpi_alloc(int nr_irqs, u32 *base, int *nr_ids)
 	} while (nr_irqs > 0);
 
 	if (!nr_irqs)
-		err = -ENOSPC;
-
-	if (err)
-		goto out;
+		goto err_out;
 
 	bitmap = bitmap_zalloc(nr_irqs, GFP_ATOMIC);
 	if (!bitmap)
-		goto out;
+		goto err_free_lpi;
 
 	*nr_ids = nr_irqs;
-
-out:
-	if (!bitmap)
-		*base = *nr_ids = 0;
-
 	return bitmap;
+
+err_free_lpi:
+	free_lpi_range(*base, nr_irqs);
+err_out:
+	*base = *nr_ids = 0;
+	return NULL;
 }
 
 static void its_lpi_free(unsigned long *bitmap, u32 base, u32 nr_ids)
-- 
2.36.1

Re: [PATCH 1/6] irqchip/gic-v3-its: Fix LPI range leak and refactor error handler in its_lpi_alloc()

[Marc Zyngier]

On Mon, 15 Jun 2026 04:29:05 +0100,
Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> 
> Fix the LIP range leak when bitmap_zalloc() failed. Besides refactor

Typo.

> error handling code to make it a little simpler.

No. Please don't mix fixes and (totally pointless) refactoring.

> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 291d7668cc8d..2b7b546c43c8 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -2217,10 +2217,9 @@ static int __init its_lpi_init(u32 id_bits)
>  static unsigned long *its_lpi_alloc(int nr_irqs, u32 *base, int *nr_ids)
>  {
>  	unsigned long *bitmap = NULL;
> -	int err = 0;
>  
>  	do {
> -		err = alloc_lpi_range(nr_irqs, base);
> +		int err = alloc_lpi_range(nr_irqs, base);
>  		if (!err)
>  			break;
>  
> @@ -2228,22 +2227,20 @@ static unsigned long *its_lpi_alloc(int nr_irqs, u32 *base, int *nr_ids)
>  	} while (nr_irqs > 0);
>  
>  	if (!nr_irqs)
> -		err = -ENOSPC;
> -
> -	if (err)
> -		goto out;
> +		goto err_out;
>  
>  	bitmap = bitmap_zalloc(nr_irqs, GFP_ATOMIC);
>  	if (!bitmap)
> -		goto out;
> +		goto err_free_lpi;
>  
>  	*nr_ids = nr_irqs;
> -
> -out:
> -	if (!bitmap)
> -		*base = *nr_ids = 0;
> -
>  	return bitmap;
> +
> +err_free_lpi:
> +	free_lpi_range(*base, nr_irqs);
> +err_out:
> +	*base = *nr_ids = 0;
> +	return NULL;
>  }
>  
>  static void its_lpi_free(unsigned long *bitmap, u32 base, u32 nr_ids)

Honestly, I question the validity of handling errors this way. You are
already unable to allocate a per-device bitmap. And yet you are
calling free_lpi_range(), which has the interesting property of
*allocating* memory. Which you don't have. Oh wait...

	M.

-- 
Jazz isn't dead. It just smells funny.

[PATCH 2/6] irqchip/gic-v3-its: Fix memleak in its_probe_one()

[Kemeng Shi]

Fix collection leak when its_init_domain() failed in its_probe_one().

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 2b7b546c43c8..df26ddc97ae2 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3032,6 +3032,12 @@ static int its_alloc_collections(struct its_node *its)
 	return 0;
 }
 
+static void its_free_collections(struct its_node *its)
+{
+	kfree(its->collections);
+	its->collections = NULL;
+}
+
 static struct page *its_allocate_pending_table(gfp_t gfp_flags)
 {
 	struct page *pend_page;
@@ -5323,7 +5329,7 @@ static int __init its_probe_one(struct its_node *its)
 
 	err = its_init_domain(its);
 	if (err)
-		goto out_free_tables;
+		goto out_free_col;
 
 	raw_spin_lock(&its_lock);
 	list_add(&its->entry, &its_nodes);
@@ -5331,6 +5337,8 @@ static int __init its_probe_one(struct its_node *its)
 
 	return 0;
 
+out_free_col:
+	its_free_collections(its);
 out_free_tables:
 	its_free_tables(its);
 out_free_cmd:
-- 
2.36.1

Re: [PATCH 2/6] irqchip/gic-v3-its: Fix memleak in its_probe_one()

[Marc Zyngier]

On Mon, 15 Jun 2026 04:29:06 +0100,
Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> 
> Fix collection leak when its_init_domain() failed in its_probe_one().
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 2b7b546c43c8..df26ddc97ae2 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -3032,6 +3032,12 @@ static int its_alloc_collections(struct its_node *its)
>  	return 0;
>  }
>  
> +static void its_free_collections(struct its_node *its)
> +{
> +	kfree(its->collections);
> +	its->collections = NULL;
> +}

Why do we need an extra helper for something that has a single calling
spot? Why is it important to set collections to NULL, given that we're
about to free the structure without even looking further?

	M.

-- 
Jazz isn't dead. It just smells funny.

[PATCH 3/6] irqchip/gic-v3-its: Fix its node leak in gic_acpi_parse_madt_its()

[Kemeng Shi]

Fix its node leak when its_probe_one() failed in
gic_acpi_parse_madt_its().

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index df26ddc97ae2..158937c867fb 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5750,9 +5750,13 @@ static int __init gic_acpi_parse_madt_its(union acpi_subtable_headers *header,
 		its->flags |= ITS_FLAGS_FORCE_NON_SHAREABLE;
 
 	err = its_probe_one(its);
-	if (!err)
-		return 0;
+	if (err)
+		goto probe_err;
+
+	return 0;
 
+probe_err:
+	its_node_destroy(its);
 node_err:
 	iort_deregister_domain_token(its_entry->translation_id);
 dom_err:
-- 
2.36.1

[PATCH 4/6] irqchip/gic-v3-its: Add ITS address info in more error logs

[Kemeng Shi]

We have a lot of logs containing ITS address info which is helpful to
distiguish which ITS occurs error. Just add ITS address info into more
exsiting error logs.

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 158937c867fb..becd8dd51720 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1120,7 +1120,7 @@ static struct its_cmd_block *its_allocate_entry(struct its_node *its)
 	while (its_queue_full(its)) {
 		count--;
 		if (!count) {
-			pr_err_ratelimited("ITS queue not draining\n");
+			pr_err_ratelimited("ITS@%pa queue not draining\n", &its->phys_base);
 			return NULL;
 		}
 		cpu_relax();
@@ -1196,8 +1196,8 @@ static int its_wait_for_range_completion(struct its_node *its,
 
 		count--;
 		if (!count) {
-			pr_err_ratelimited("ITS queue timeout (%llu %llu)\n",
-					   to_idx, linear_idx);
+			pr_err_ratelimited("ITS@%pa queue timeout (%llu %llu)\n",
+					   &its->phys_base, to_idx, linear_idx);
 			return -1;
 		}
 		prev_idx = rd_idx;
@@ -1244,7 +1244,7 @@ post:									\
 	raw_spin_unlock_irqrestore(&its->lock, flags);			\
 									\
 	if (its_wait_for_range_completion(its, rd_idx, next_cmd))	\
-		pr_err_ratelimited("ITS cmd %ps failed\n", builder);	\
+		pr_err_ratelimited("ITS@%pa cmd %ps failed\n", &its->phys_base, &builder);	\
 }
 
 static void its_build_sync_cmd(struct its_node *its,
@@ -2408,7 +2408,8 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 
 		/* 52bit PA is supported only when PageSize=64K */
 		if (psz != SZ_64K) {
-			pr_err("ITS: no 52bit PA support when psz=%d\n", psz);
+			pr_err("ITS@%pa: no 52bit PA support when psz=%d\n",
+				   &its->phys_base, psz);
 			its_free_pages(base, order);
 			return -ENXIO;
 		}
@@ -5184,7 +5185,7 @@ static int its_init_vpe_domain(void)
 	vpe_proxy.dev = its_create_device(its, devid, entries, false);
 	if (!vpe_proxy.dev) {
 		kfree(vpe_proxy.vpes);
-		pr_err("ITS: Can't allocate GICv4 proxy device\n");
+		pr_err("ITS@%pa: Can't allocate GICv4 proxy device\n", &its->phys_base);
 		return -ENOMEM;
 	}
 
-- 
2.36.1

Re: [PATCH 4/6] irqchip/gic-v3-its: Add ITS address info in more error logs

[Marc Zyngier]

On Mon, 15 Jun 2026 04:29:08 +0100,
Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> 
> We have a lot of logs containing ITS address info which is helpful to
> distiguish which ITS occurs error. Just add ITS address info into more
> exsiting error logs.

That's only useful on buggy HW, for people debugging it. I don't think
that's useful outside of these scenarios, and this hack can live out
of tree.

	M.

-- 
Jazz isn't dead. It just smells funny.

[PATCH 5/6] irqchip/gic-v3-its: fix typo in comments

[Kemeng Shi]

1. "If it some" -> "If some"
2. "by table by reading" -> by reading"

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index becd8dd51720..fc32a1709f76 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -163,7 +163,7 @@ struct event_lpi_map {
 
 /*
  * The ITS view of a device - belongs to an ITS, owns an interrupt
- * translation table, and a list of interrupts.  If it some of its
+ * translation table, and a list of interrupts.  If some of its
  * LPIs are injected into a guest (GICv4), the event_map.vm field
  * indicates which one.
  */
@@ -2501,7 +2501,7 @@ static bool its_parse_indirect_baser(struct its_node *its,
 	if ((esz << ids) > (psz * 2)) {
 		/*
 		 * Find out whether hw supports a single or two-level table by
-		 * table by reading bit at offset '62' after writing '1' to it.
+		 * reading bit at offset '62' after writing '1' to it.
 		 */
 		its_write_baser(its, baser, val | GITS_BASER_INDIRECT);
 		indirect = !!(baser->val & GITS_BASER_INDIRECT);
-- 
2.36.1

Re: [PATCH 5/6] irqchip/gic-v3-its: fix typo in comments

[Marc Zyngier]

On Mon, 15 Jun 2026 04:29:09 +0100,
Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> 
> 1. "If it some" -> "If some"
> 2. "by table by reading" -> by reading"
> 
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index becd8dd51720..fc32a1709f76 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -163,7 +163,7 @@ struct event_lpi_map {
>  
>  /*
>   * The ITS view of a device - belongs to an ITS, owns an interrupt
> - * translation table, and a list of interrupts.  If it some of its
> + * translation table, and a list of interrupts.  If some of its
>   * LPIs are injected into a guest (GICv4), the event_map.vm field
>   * indicates which one.
>   */
> @@ -2501,7 +2501,7 @@ static bool its_parse_indirect_baser(struct its_node *its,
>  	if ((esz << ids) > (psz * 2)) {
>  		/*
>  		 * Find out whether hw supports a single or two-level table by
> -		 * table by reading bit at offset '62' after writing '1' to it.
> +		 * reading bit at offset '62' after writing '1' to it.
>  		 */

If you are going to fix that comment, fix it for good by replacing the
reference to a bit number with its actual name, making it valuable for
everyone.

	M.

>  		its_write_baser(its, baser, val | GITS_BASER_INDIRECT);
>  		indirect = !!(baser->val & GITS_BASER_INDIRECT);
> -- 
> 2.36.1
> 
> 

-- 
Jazz isn't dead. It just smells funny.

[PATCH 6/6] irqchip/gic-v3-its: some minor cleanups

[Kemeng Shi]

1. Remove unneeded NULL check in itt_alloc_pool() as addr will always be
NULL when we reach here.
2. Correct indentation in cpumask_pick_least_loaded()
3. Remove unneeded updation of range node when it is to be deleted in
alloc_lpi_range().
4. Remove unneeded assignment to baser->psz which is already used
as input inits_setup_baser()

Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index fc32a1709f76..ef505575ba86 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -273,7 +273,7 @@ static void *itt_alloc_pool(int node, int size)
 			break;
 
 		gen_pool_add(itt_pool, (unsigned long)page_address(page), PAGE_SIZE, node);
-	} while (!addr);
+	} while (true);
 
 	return (void *)addr;
 }
@@ -1681,7 +1681,7 @@ static unsigned int cpumask_pick_least_loaded(struct irq_data *d,
 		int this_count = its_read_lpi_count(d, tmp);
 		if (this_count < count) {
 			cpu = tmp;
-		        count = this_count;
+			count = this_count;
 		}
 	}
 
@@ -2127,12 +2127,13 @@ static int alloc_lpi_range(u32 nr_lpis, u32 *base)
 	list_for_each_entry_safe(range, tmp, &lpi_range_list, entry) {
 		if (range->span >= nr_lpis) {
 			*base = range->base_id;
-			range->base_id += nr_lpis;
-			range->span -= nr_lpis;
 
-			if (range->span == 0) {
+			if (range->span == nr_lpis) {
 				list_del(&range->entry);
 				kfree(range);
+			} else {
+				range->base_id += nr_lpis;
+				range->span -= nr_lpis;
 			}
 
 			err = 0;
@@ -2472,7 +2473,6 @@ static int its_setup_baser(struct its_node *its, struct its_baser *baser,
 
 	baser->order = order;
 	baser->base = base;
-	baser->psz = psz;
 	tmp = indirect ? GITS_LVL1_ENTRY_SIZE : esz;
 
 	pr_info("ITS@%pa: allocated %d %s @%llx (%s, esz %d, psz %dK, shr %d)\n",
-- 
2.36.1

Re: [PATCH 6/6] irqchip/gic-v3-its: some minor cleanups

[Marc Zyngier]

On Mon, 15 Jun 2026 04:29:10 +0100,
Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> 
> 1. Remove unneeded NULL check in itt_alloc_pool() as addr will always be
> NULL when we reach here.
> 2. Correct indentation in cpumask_pick_least_loaded()
> 3. Remove unneeded updation of range node when it is to be deleted in
> alloc_lpi_range().
> 4. Remove unneeded assignment to baser->psz which is already used
> as input inits_setup_baser()

Honestly, these changes, aside from (maybe) the last one, are pretty
pointless. I'm sure there are better things to waste your time on.

	M.

-- 
Jazz isn't dead. It just smells funny.