[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

[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

[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

[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

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

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.

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.

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.

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.

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

[Kemeng Shi]

在 2026/6/15 16:52:56, 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.
OK, I will only keep fix in this patch.> 
>>
>> 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...
You are right. I'm considering use xarray to track the lpi range or
modify free_lpi_range to try merge first before memory allocation.
What would you recommend?

Thanks,
Kemeng Shi> 
> 	M.
> 

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

[Kemeng Shi]

在 2026/6/15 16:59:14, 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?
> 
The extra helper is added for symmetry with its_alloc_collections(), keeping
allocation/deallocation logic paired. In this way, we need to only modified
allocation/deallocation function if more member is added for collections.
Setting collections to NULL is a personal habit to quickly catch use-after-free
of collections. Could drop this which is not that import.

Thanks,
Kemeng
> 	M.
> 

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

[Kemeng Shi]

在 2026/6/15 17:01:52, 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.
> 
I agree this is only useful in rare cases, but I believe there
is no lose...
But no insistant on this anyway...

Thanks,
Kemeng
> 	M.
> 

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

[Kemeng Shi]

在 2026/6/15 17:03:47, 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.
Will improve with this in next version.

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

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

[Kemeng Shi]

在 2026/6/15 17:14:27, 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.
> 
No insistant on this. I will drop this in next version.

Thanks,
Kemeng> 	M.
>