[PATCH 1/2] genirq: Record dangling hwirq number into struct irq_data

[PATCH 1/2] genirq: Record dangling hwirq number into struct irq_data

[Liao Chang]

Following interrupt allocation process lead to some interrupts are
mapped in the low-level domain(Arm ITS), but they are never been mapped
at the higher level.

irq_domain_alloc_irqs_hierarchy(.., nr_irqs, ...)
  its_irq_domain_alloc(..., nr_irqs, ...)
    its_alloc_device_irq(..., nr_irqs, ...)
      bitmap_find_free_region(..., get_count_order(nr_irqs))

Since ITS domain find a region of zero bits, the length of which must
aligned to power of two. If nr_irqs is 30, the length of zero bits is
actually 32, but only first 30 bits are really mapped.

On teardown, low-level domain only free these interrupts that actually
mapped, and leave last interrupts dangling in the ITS domain. Thus the
ITS device resources are never freed. On device driver reload, dangling
interrupts prevent ITS domain from allocating enough resource.

irq_domain_free_irqs_hierarchy(..., nr_irqs, ...)
  its_irq_domain_free(..., irq_base + i, 1)
    bitmap_release_region(..., irq_base + i, get_count_order(1))

John reported this problem to LKML and Marc provided a solution and fix
it in the generic code, see the discussion from Link tag. Marc's patch
fix John's problem, but does not take care of some corner case, look one
example below.

Step1: 32 interrupts allocated in LPI domain, but return the first 30 to
higher driver.

   111111111111111111111111111111 11
  |<------------0~29------------>|30,31|

Step2: interrupt #16~28 are released one by one, then #0~15 and #29~31
still be there.

   1111111111111111 0000000000000 1  11
  |<-----0~15----->|<---16~28--->|29|30,31|

Step#: on driver teardown, generic code will invoke ITS domain code
twice. The first time, #0~15 will be released, the second one, only #29
will be released(1 align to power of two).

   0000000000000000 0000000000000 0  11
  |<-----0~15----->|<---16~28--->|29|30,31|

In short summary, the dangling problem stems from the number of released
hwirq is less than the one of allocated hwirq in ITS domain. In order to
fix this problem, make irq_data record the number of allocated but
unmapped hwirq. If hwirq followed by some unmapped bits, ITS domain
record the number of unmapped bits to the last irq_data mapped to higher
level, when the last hwirq followed by unmapped hwirq is released, some
dangling bit will be clear eventualy, look back the trivial example
above.

Step1: record '2' into the irq_data.dangling of #29 hwirq.

           111111111111111111111111111111 11
          |<------------0~29------------>|30,31|
dangling:  000000000000000000000000000002

Step2: no change

          1111111111111111 0000000000000 1  11
         |<-----0~15----->|<---16~28--->|29|30,31|
dangling: 0000000000000000 0000000000000 2

Step3: ITS domain will release #30~31 since the irq_data.dangling of #29
is '2'.

           0000000000000000 0000000000000 0  00
          |<-----0~15----->|<---16~28--->|29|30,31|
dangling:  0000000000000000 0000000000000 2

Fixes: 4615fbc3788dd ("genirq/irqdomain: Don't try to free an interrupt
that has no mapping")
Reported-by: John Garry <john.garry@huawei.com>
Signed-off-by: Liao Chang <liaochang1@huawei.com>
Link: https://lore.kernel.org/lkml/3d3d0155e66429968cb4f6b4feeae4b3@kernel.org/
---
 include/linux/irq.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index c3eb89606c2b..c48f10a0c230 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -167,6 +167,10 @@ struct irq_common_data {
  * @mask:		precomputed bitmask for accessing the chip registers
  * @irq:		interrupt number
  * @hwirq:		hardware interrupt number, local to the interrupt domain
+ * @dangling:		amount of dangling hardware interrupt, Arm ITS allocate
+ *			hardware interrupt more than expected, aligned to power
+ *			of two, so that unsued interrupt number become dangling.
+ *			Use this field to record dangling bits follwoing @hwirq.
  * @common:		point to data shared by all irqchips
  * @chip:		low level interrupt hardware access
  * @domain:		Interrupt translation domain; responsible for mapping
@@ -180,6 +184,7 @@ struct irq_data {
 	u32			mask;
 	unsigned int		irq;
 	unsigned long		hwirq;
+	unsigned long		dangling;
 	struct irq_common_data	*common;
 	struct irq_chip		*chip;
 	struct irq_domain	*domain;
@@ -455,6 +460,11 @@ static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
 	return d->hwirq;
 }
 
+static inline void irqd_set_dangling(struct irq_data *d, unsigned int dangling)
+{
+	d->dangling = dangling;
+}
+
 /**
  * struct irq_chip - hardware interrupt chip descriptor
  *
-- 
2.17.1

[PATCH 2/2] irqchip/gic-v3-its: Release the allocated but unmapped bits in LPI maps

[Liao Chang]

If one hwirq allocated in ITS domain followed by some unmapped bits, the
number of unmapped bits will be recorded, so that this hwirq and
following unmapped bits could be released both.

Reported-by: John Garry <john.garry@huawei.com>
Signed-off-by: Liao Chang <liaochang1@huawei.com>
Link: https://lore.kernel.org/lkml/3d3d0155e66429968cb4f6b4feeae4b3@kernel.org/
---
 drivers/irqchip/irq-gic-v3-its.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 5ff09de6c48f..36a1bc88e832 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3572,11 +3572,19 @@ static int its_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
 		irqd = irq_get_irq_data(virq + i);
 		irqd_set_single_target(irqd);
 		irqd_set_affinity_on_activate(irqd);
+		irqd_set_dangling(irqd, 0);
 		pr_debug("ID:%d pID:%d vID:%d\n",
 			 (int)(hwirq + i - its_dev->event_map.lpi_base),
 			 (int)(hwirq + i), virq + i);
 	}
 
+	/*
+	 * In order to free dangling hwirq bits, kernel uses the irq_data
+	 * of hwirq which is followed by dangling bits to record dangling
+	 * number.
+	 */
+	irqd_set_dangling(irqd, (1 << get_count_order(nr_irqs)) - nr_irqs);
+
 	return 0;
 }
 
@@ -3617,12 +3625,16 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
 	struct irq_data *d = irq_domain_get_irq_data(domain, virq);
 	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
 	struct its_node *its = its_dev->its;
+	unsigned int pos = its_get_event_id(d);
 	int i;
 
-	bitmap_release_region(its_dev->event_map.lpi_map,
-			      its_get_event_id(irq_domain_get_irq_data(domain, virq)),
+	bitmap_release_region(its_dev->event_map.lpi_map, pos,
 			      get_count_order(nr_irqs));
 
+	for (i = 0; i < d->dangling; i++)
+		bitmap_release_region(its_dev->event_map.lpi_map,
+				      pos + nr_irqs + i, get_count_order(1));
+
 	for (i = 0; i < nr_irqs; i++) {
 		struct irq_data *data = irq_domain_get_irq_data(domain,
 								virq + i);
-- 
2.17.1

Re: [PATCH 1/2] genirq: Record dangling hwirq number into struct irq_data

[Andy Shevchenko]

On Thu, Aug 25, 2022 at 9:11 AM Liao Chang <liaochang1@huawei.com> wrote:

Use spell-checker, please. Or ask somebody for proof-reading of your
commit messages and comments in the code.

> Following interrupt allocation process lead to some interrupts are

leads

> mapped in the low-level domain(Arm ITS), but they are never been mapped

never mapped
  ...or...
they have never

> at the higher level.
>
> irq_domain_alloc_irqs_hierarchy(.., nr_irqs, ...)
>   its_irq_domain_alloc(..., nr_irqs, ...)
>     its_alloc_device_irq(..., nr_irqs, ...)
>       bitmap_find_free_region(..., get_count_order(nr_irqs))
>
> Since ITS domain find a region of zero bits, the length of which must

finds

> aligned to power of two. If nr_irqs is 30, the length of zero bits is

the power

> actually 32, but only first 30 bits are really mapped.

the first

> On teardown, low-level domain only free these interrupts that actually

the low-level
   ...or...
domains

> mapped, and leave last interrupts dangling in the ITS domain. Thus the
> ITS device resources are never freed. On device driver reload, dangling
> interrupts prevent ITS domain from allocating enough resource.
>
> irq_domain_free_irqs_hierarchy(..., nr_irqs, ...)
>   its_irq_domain_free(..., irq_base + i, 1)
>     bitmap_release_region(..., irq_base + i, get_count_order(1))
>
> John reported this problem to LKML and Marc provided a solution and fix
> it in the generic code, see the discussion from Link tag. Marc's patch
> fix John's problem, but does not take care of some corner case, look one
> example below.
>
> Step1: 32 interrupts allocated in LPI domain, but return the first 30 to
> higher driver.
>
>    111111111111111111111111111111 11
>   |<------------0~29------------>|30,31|
>
> Step2: interrupt #16~28 are released one by one, then #0~15 and #29~31
> still be there.
>
>    1111111111111111 0000000000000 1  11
>   |<-----0~15----->|<---16~28--->|29|30,31|
>
> Step#: on driver teardown, generic code will invoke ITS domain code
> twice. The first time, #0~15 will be released, the second one, only #29
> will be released(1 align to power of two).
>
>    0000000000000000 0000000000000 0  11
>   |<-----0~15----->|<---16~28--->|29|30,31|
>
> In short summary, the dangling problem stems from the number of released
> hwirq is less than the one of allocated hwirq in ITS domain. In order to

the allocated

> fix this problem, make irq_data record the number of allocated but
> unmapped hwirq. If hwirq followed by some unmapped bits, ITS domain
> record the number of unmapped bits to the last irq_data mapped to higher
> level, when the last hwirq followed by unmapped hwirq is released, some
> dangling bit will be clear eventualy, look back the trivial example

eventually

> above.
>
> Step1: record '2' into the irq_data.dangling of #29 hwirq.
>
>            111111111111111111111111111111 11
>           |<------------0~29------------>|30,31|
> dangling:  000000000000000000000000000002
>
> Step2: no change
>
>           1111111111111111 0000000000000 1  11
>          |<-----0~15----->|<---16~28--->|29|30,31|
> dangling: 0000000000000000 0000000000000 2
>
> Step3: ITS domain will release #30~31 since the irq_data.dangling of #29
> is '2'.
>
>            0000000000000000 0000000000000 0  00
>           |<-----0~15----->|<---16~28--->|29|30,31|
> dangling:  0000000000000000 0000000000000 2

> Fixes: 4615fbc3788dd ("genirq/irqdomain: Don't try to free an interrupt
> that has no mapping")

All tags must be one-liners. I.o.w. do not split a tag to multiple lines.

...

> + * @dangling:          amount of dangling hardware interrupt, Arm ITS allocate
> + *                     hardware interrupt more than expected, aligned to power
> + *                     of two, so that unsued interrupt number become dangling.

unused
becomes

> + *                     Use this field to record dangling bits follwoing @hwirq.

following

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/2] irqchip/gic-v3-its: Release the allocated but unmapped bits in LPI maps

[Andy Shevchenko]

On Thu, Aug 25, 2022 at 9:11 AM Liao Chang <liaochang1@huawei.com> wrote:
>
> If one hwirq allocated in ITS domain followed by some unmapped bits, the
> number of unmapped bits will be recorded, so that this hwirq and
> following unmapped bits could be released both.

...

> +       /*
> +        * In order to free dangling hwirq bits, kernel uses the irq_data

the kernel

> +        * of hwirq which is followed by dangling bits to record dangling
> +        * number.
> +        */


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] genirq: Record dangling hwirq number into struct irq_data

[liaochang (A)]

在 2022/8/25 16:27, Andy Shevchenko 写道:
> On Thu, Aug 25, 2022 at 9:11 AM Liao Chang <liaochang1@huawei.com> wrote:
> 
> Use spell-checker, please. Or ask somebody for proof-reading of your
> commit messages and comments in the code.
> 
>> Following interrupt allocation process lead to some interrupts are
> 
> leads
> 
>> mapped in the low-level domain(Arm ITS), but they are never been mapped
> 
> never mapped
>   ...or...
> they have never
> 
>> at the higher level.
>>
>> irq_domain_alloc_irqs_hierarchy(.., nr_irqs, ...)
>>   its_irq_domain_alloc(..., nr_irqs, ...)
>>     its_alloc_device_irq(..., nr_irqs, ...)
>>       bitmap_find_free_region(..., get_count_order(nr_irqs))
>>
>> Since ITS domain find a region of zero bits, the length of which must
> 
> finds
> 
>> aligned to power of two. If nr_irqs is 30, the length of zero bits is
> 
> the power
> 
>> actually 32, but only first 30 bits are really mapped.
> 
> the first
> 
>> On teardown, low-level domain only free these interrupts that actually
> 
> the low-level
>    ...or...
> domains
> 
>> mapped, and leave last interrupts dangling in the ITS domain. Thus the
>> ITS device resources are never freed. On device driver reload, dangling
>> interrupts prevent ITS domain from allocating enough resource.
>>
>> irq_domain_free_irqs_hierarchy(..., nr_irqs, ...)
>>   its_irq_domain_free(..., irq_base + i, 1)
>>     bitmap_release_region(..., irq_base + i, get_count_order(1))
>>
>> John reported this problem to LKML and Marc provided a solution and fix
>> it in the generic code, see the discussion from Link tag. Marc's patch
>> fix John's problem, but does not take care of some corner case, look one
>> example below.
>>
>> Step1: 32 interrupts allocated in LPI domain, but return the first 30 to
>> higher driver.
>>
>>    111111111111111111111111111111 11
>>   |<------------0~29------------>|30,31|
>>
>> Step2: interrupt #16~28 are released one by one, then #0~15 and #29~31
>> still be there.
>>
>>    1111111111111111 0000000000000 1  11
>>   |<-----0~15----->|<---16~28--->|29|30,31|
>>
>> Step#: on driver teardown, generic code will invoke ITS domain code
>> twice. The first time, #0~15 will be released, the second one, only #29
>> will be released(1 align to power of two).
>>
>>    0000000000000000 0000000000000 0  11
>>   |<-----0~15----->|<---16~28--->|29|30,31|
>>
>> In short summary, the dangling problem stems from the number of released
>> hwirq is less than the one of allocated hwirq in ITS domain. In order to
> 
> the allocated
> 
>> fix this problem, make irq_data record the number of allocated but
>> unmapped hwirq. If hwirq followed by some unmapped bits, ITS domain
>> record the number of unmapped bits to the last irq_data mapped to higher
>> level, when the last hwirq followed by unmapped hwirq is released, some
>> dangling bit will be clear eventualy, look back the trivial example
> 
> eventually
> 
>> above.
>>
>> Step1: record '2' into the irq_data.dangling of #29 hwirq.
>>
>>            111111111111111111111111111111 11
>>           |<------------0~29------------>|30,31|
>> dangling:  000000000000000000000000000002
>>
>> Step2: no change
>>
>>           1111111111111111 0000000000000 1  11
>>          |<-----0~15----->|<---16~28--->|29|30,31|
>> dangling: 0000000000000000 0000000000000 2
>>
>> Step3: ITS domain will release #30~31 since the irq_data.dangling of #29
>> is '2'.
>>
>>            0000000000000000 0000000000000 0  00
>>           |<-----0~15----->|<---16~28--->|29|30,31|
>> dangling:  0000000000000000 0000000000000 2
> 
>> Fixes: 4615fbc3788dd ("genirq/irqdomain: Don't try to free an interrupt
>> that has no mapping")
> 
> All tags must be one-liners. I.o.w. do not split a tag to multiple lines.
> 
> ...
> 
>> + * @dangling:          amount of dangling hardware interrupt, Arm ITS allocate
>> + *                     hardware interrupt more than expected, aligned to power
>> + *                     of two, so that unsued interrupt number become dangling.
> 
> unused
> becomes
> 
>> + *                     Use this field to record dangling bits follwoing @hwirq.
> 
> following
> 
Appreciate for your help, i will correct them.

-- 
BR,
Liao, Chang

Re: [PATCH 1/2] genirq: Record dangling hwirq number into struct irq_data

[Marc Zyngier]

On Thu, 25 Aug 2022 07:08:18 +0100,
Liao Chang <liaochang1@huawei.com> wrote:
> 
> Following interrupt allocation process lead to some interrupts are
> mapped in the low-level domain(Arm ITS), but they are never been mapped
> at the higher level.
> 
> irq_domain_alloc_irqs_hierarchy(.., nr_irqs, ...)
>   its_irq_domain_alloc(..., nr_irqs, ...)
>     its_alloc_device_irq(..., nr_irqs, ...)
>       bitmap_find_free_region(..., get_count_order(nr_irqs))
> 
> Since ITS domain find a region of zero bits, the length of which must
> aligned to power of two. If nr_irqs is 30, the length of zero bits is
> actually 32, but only first 30 bits are really mapped.
> 
> On teardown, low-level domain only free these interrupts that actually
> mapped, and leave last interrupts dangling in the ITS domain. Thus the
> ITS device resources are never freed. On device driver reload, dangling
> interrupts prevent ITS domain from allocating enough resource.
> 
> irq_domain_free_irqs_hierarchy(..., nr_irqs, ...)
>   its_irq_domain_free(..., irq_base + i, 1)
>     bitmap_release_region(..., irq_base + i, get_count_order(1))
> 
> John reported this problem to LKML and Marc provided a solution and fix
> it in the generic code, see the discussion from Link tag. Marc's patch
> fix John's problem, but does not take care of some corner case, look one
> example below.
> 
> Step1: 32 interrupts allocated in LPI domain, but return the first 30 to
> higher driver.
> 
>    111111111111111111111111111111 11
>   |<------------0~29------------>|30,31|
> 
> Step2: interrupt #16~28 are released one by one, then #0~15 and #29~31
> still be there.
> 
>    1111111111111111 0000000000000 1  11
>   |<-----0~15----->|<---16~28--->|29|30,31|
> 
> Step#: on driver teardown, generic code will invoke ITS domain code
> twice. The first time, #0~15 will be released, the second one, only #29
> will be released(1 align to power of two).
> 
>    0000000000000000 0000000000000 0  11
>   |<-----0~15----->|<---16~28--->|29|30,31|
> 
> In short summary, the dangling problem stems from the number of released
> hwirq is less than the one of allocated hwirq in ITS domain. In order to
> fix this problem, make irq_data record the number of allocated but
> unmapped hwirq. If hwirq followed by some unmapped bits, ITS domain
> record the number of unmapped bits to the last irq_data mapped to higher
> level, when the last hwirq followed by unmapped hwirq is released, some
> dangling bit will be clear eventualy, look back the trivial example
> above.
> 
> Step1: record '2' into the irq_data.dangling of #29 hwirq.
> 
>            111111111111111111111111111111 11
>           |<------------0~29------------>|30,31|
> dangling:  000000000000000000000000000002
> 
> Step2: no change
> 
>           1111111111111111 0000000000000 1  11
>          |<-----0~15----->|<---16~28--->|29|30,31|
> dangling: 0000000000000000 0000000000000 2
> 
> Step3: ITS domain will release #30~31 since the irq_data.dangling of #29
> is '2'.
> 
>            0000000000000000 0000000000000 0  00
>           |<-----0~15----->|<---16~28--->|29|30,31|
> dangling:  0000000000000000 0000000000000 2
> 
> Fixes: 4615fbc3788dd ("genirq/irqdomain: Don't try to free an interrupt
> that has no mapping")
> Reported-by: John Garry <john.garry@huawei.com>
> Signed-off-by: Liao Chang <liaochang1@huawei.com>
> Link: https://lore.kernel.org/lkml/3d3d0155e66429968cb4f6b4feeae4b3@kernel.org/
> ---
>  include/linux/irq.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/linux/irq.h b/include/linux/irq.h
> index c3eb89606c2b..c48f10a0c230 100644
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -167,6 +167,10 @@ struct irq_common_data {
>   * @mask:		precomputed bitmask for accessing the chip registers
>   * @irq:		interrupt number
>   * @hwirq:		hardware interrupt number, local to the interrupt domain
> + * @dangling:		amount of dangling hardware interrupt, Arm ITS allocate
> + *			hardware interrupt more than expected, aligned to power
> + *			of two, so that unsued interrupt number become dangling.
> + *			Use this field to record dangling bits follwoing @hwirq.
>   * @common:		point to data shared by all irqchips
>   * @chip:		low level interrupt hardware access
>   * @domain:		Interrupt translation domain; responsible for mapping
> @@ -180,6 +184,7 @@ struct irq_data {
>  	u32			mask;
>  	unsigned int		irq;
>  	unsigned long		hwirq;
> +	unsigned long		dangling;
>  	struct irq_common_data	*common;
>  	struct irq_chip		*chip;
>  	struct irq_domain	*domain;


There is no way I will put ITS-specific hacks in the core, and I
really don't think this is the correct way to address this. Also, why
track this sort of thing on a per-interrupt basis, while this is
obviously a device-level allocation?

The real issue is that there is currently no way for the ITS code to
know when we're done with *all* the interrupts of a device. This is
what needs fixing.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 1/2] genirq: Record dangling hwirq number into struct irq_data

[liaochang (A)]

在 2022/8/28 0:15, Marc Zyngier 写道:
> On Thu, 25 Aug 2022 07:08:18 +0100,
> Liao Chang <liaochang1@huawei.com> wrote:
>>
>> Following interrupt allocation process lead to some interrupts are
>> mapped in the low-level domain(Arm ITS), but they are never been mapped
>> at the higher level.
>>
>> irq_domain_alloc_irqs_hierarchy(.., nr_irqs, ...)
>>   its_irq_domain_alloc(..., nr_irqs, ...)
>>     its_alloc_device_irq(..., nr_irqs, ...)
>>       bitmap_find_free_region(..., get_count_order(nr_irqs))
>>
>> Since ITS domain find a region of zero bits, the length of which must
>> aligned to power of two. If nr_irqs is 30, the length of zero bits is
>> actually 32, but only first 30 bits are really mapped.
>>
>> On teardown, low-level domain only free these interrupts that actually
>> mapped, and leave last interrupts dangling in the ITS domain. Thus the
>> ITS device resources are never freed. On device driver reload, dangling
>> interrupts prevent ITS domain from allocating enough resource.
>>
>> irq_domain_free_irqs_hierarchy(..., nr_irqs, ...)
>>   its_irq_domain_free(..., irq_base + i, 1)
>>     bitmap_release_region(..., irq_base + i, get_count_order(1))
>>
>> John reported this problem to LKML and Marc provided a solution and fix
>> it in the generic code, see the discussion from Link tag. Marc's patch
>> fix John's problem, but does not take care of some corner case, look one
>> example below.
>>
>> Step1: 32 interrupts allocated in LPI domain, but return the first 30 to
>> higher driver.
>>
>>    111111111111111111111111111111 11
>>   |<------------0~29------------>|30,31|
>>
>> Step2: interrupt #16~28 are released one by one, then #0~15 and #29~31
>> still be there.
>>
>>    1111111111111111 0000000000000 1  11
>>   |<-----0~15----->|<---16~28--->|29|30,31|
>>
>> Step#: on driver teardown, generic code will invoke ITS domain code
>> twice. The first time, #0~15 will be released, the second one, only #29
>> will be released(1 align to power of two).
>>
>>    0000000000000000 0000000000000 0  11
>>   |<-----0~15----->|<---16~28--->|29|30,31|
>>
>> In short summary, the dangling problem stems from the number of released
>> hwirq is less than the one of allocated hwirq in ITS domain. In order to
>> fix this problem, make irq_data record the number of allocated but
>> unmapped hwirq. If hwirq followed by some unmapped bits, ITS domain
>> record the number of unmapped bits to the last irq_data mapped to higher
>> level, when the last hwirq followed by unmapped hwirq is released, some
>> dangling bit will be clear eventualy, look back the trivial example
>> above.
>>
>> Step1: record '2' into the irq_data.dangling of #29 hwirq.
>>
>>            111111111111111111111111111111 11
>>           |<------------0~29------------>|30,31|
>> dangling:  000000000000000000000000000002
>>
>> Step2: no change
>>
>>           1111111111111111 0000000000000 1  11
>>          |<-----0~15----->|<---16~28--->|29|30,31|
>> dangling: 0000000000000000 0000000000000 2
>>
>> Step3: ITS domain will release #30~31 since the irq_data.dangling of #29
>> is '2'.
>>
>>            0000000000000000 0000000000000 0  00
>>           |<-----0~15----->|<---16~28--->|29|30,31|
>> dangling:  0000000000000000 0000000000000 2
>>
>> Fixes: 4615fbc3788dd ("genirq/irqdomain: Don't try to free an interrupt
>> that has no mapping")
>> Reported-by: John Garry <john.garry@huawei.com>
>> Signed-off-by: Liao Chang <liaochang1@huawei.com>
>> Link: https://lore.kernel.org/lkml/3d3d0155e66429968cb4f6b4feeae4b3@kernel.org/
>> ---
>>  include/linux/irq.h | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/include/linux/irq.h b/include/linux/irq.h
>> index c3eb89606c2b..c48f10a0c230 100644
>> --- a/include/linux/irq.h
>> +++ b/include/linux/irq.h
>> @@ -167,6 +167,10 @@ struct irq_common_data {
>>   * @mask:		precomputed bitmask for accessing the chip registers
>>   * @irq:		interrupt number
>>   * @hwirq:		hardware interrupt number, local to the interrupt domain
>> + * @dangling:		amount of dangling hardware interrupt, Arm ITS allocate
>> + *			hardware interrupt more than expected, aligned to power
>> + *			of two, so that unsued interrupt number become dangling.
>> + *			Use this field to record dangling bits follwoing @hwirq.
>>   * @common:		point to data shared by all irqchips
>>   * @chip:		low level interrupt hardware access
>>   * @domain:		Interrupt translation domain; responsible for mapping
>> @@ -180,6 +184,7 @@ struct irq_data {
>>  	u32			mask;
>>  	unsigned int		irq;
>>  	unsigned long		hwirq;
>> +	unsigned long		dangling;
>>  	struct irq_common_data	*common;
>>  	struct irq_chip		*chip;
>>  	struct irq_domain	*domain;
> 
> 
> There is no way I will put ITS-specific hacks in the core, and I
> really don't think this is the correct way to address this. Also, why
> track this sort of thing on a per-interrupt basis, while this is
> obviously a device-level allocation?
Thanks for comment, my older version is introduce a dangling bitmap in ITS
irq_domain at the cose of one more bitmap. such as like this:

struct event_lpi_map {
   unsigned long  *lpi_map;
+  unsigned long  *dangling_map;
   ...
}

+dev->event.map.dangling_map = bitmap_alloc(nevc);

Obviously this dangling bitmap is likely to be sparse, which means most of bits
are wasted, hence I decide to record dangling bits into the per-interrupt object.
Well, it sounds a bad idea...
> 
> The real issue is that there is currently no way for the ITS code to
> know when we're done with *all* the interrupts of a device. This is
> what needs fixing.

Let my understand what you mean, you are saying let **device driver** record these mapped
and dangling interrupts, and free these dangling interrupts explicitly when driver is uninstall.

> 
> 	M.
> 

-- 
BR,
Liao, Chang