Skip to content

Commit 713335b

Browse files
Marc ZyngierKAGA-KOKO
authored andcommitted
irqchip/gic-v3-its: Implement .msi_teardown() callback
The ITS driver currently nukes the structure representing an endpoint device translating via an ITS on freeing the last LPI allocated for it. That's an unfortunate state of affair, as it is pretty common for a driver to allocate a single MSI, do something clever, teardown this MSI, and reallocate a whole bunch of them. The NVME driver does exactly that, amongst others. What happens in that case is that the core code is accidentaly issuing another .msi_prepare() call, even if it shouldn't. This luckily cancels the above behaviour and hides the problem. In order to fix the core code, start by implementing the new .msi_teardown() callback. Nothing calls it yet, so a side effect is that the its_dev structure will not be freed and that the DID will stay mapped. Not a big deal, and this will be solved in following patches. Signed-off-by: Marc Zyngier <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Link: https://lore.kernel.org/all/[email protected]
1 parent 28026cf commit 713335b

File tree

3 files changed

+36
-23
lines changed

3 files changed

+36
-23
lines changed

drivers/irqchip/irq-gic-v3-its-msi-parent.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,14 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
167167
dev, nvec, info);
168168
}
169169

170+
static void its_msi_teardown(struct irq_domain *domain, msi_alloc_info_t *info)
171+
{
172+
struct msi_domain_info *msi_info;
173+
174+
msi_info = msi_get_domain_info(domain->parent);
175+
msi_info->ops->msi_teardown(domain->parent, info);
176+
}
177+
170178
static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
171179
struct irq_domain *real_parent, struct msi_domain_info *info)
172180
{
@@ -190,6 +198,7 @@ static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
190198
* %MSI_MAX_INDEX.
191199
*/
192200
info->ops->msi_prepare = its_pci_msi_prepare;
201+
info->ops->msi_teardown = its_msi_teardown;
193202
break;
194203
case DOMAIN_BUS_DEVICE_MSI:
195204
case DOMAIN_BUS_WIRED_TO_MSI:
@@ -198,6 +207,7 @@ static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
198207
* size is also known at domain creation time.
199208
*/
200209
info->ops->msi_prepare = its_pmsi_prepare;
210+
info->ops->msi_teardown = its_msi_teardown;
201211
break;
202212
default:
203213
/* Confused. How did the lib return true? */

drivers/irqchip/irq-gic-v3-its.c

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3620,8 +3620,33 @@ static int its_msi_prepare(struct irq_domain *domain, struct device *dev,
36203620
return err;
36213621
}
36223622

3623+
static void its_msi_teardown(struct irq_domain *domain, msi_alloc_info_t *info)
3624+
{
3625+
struct its_device *its_dev = info->scratchpad[0].ptr;
3626+
3627+
guard(mutex)(&its_dev->its->dev_alloc_lock);
3628+
3629+
/* If the device is shared, keep everything around */
3630+
if (its_dev->shared)
3631+
return;
3632+
3633+
/* LPIs should have been already unmapped at this stage */
3634+
if (WARN_ON_ONCE(!bitmap_empty(its_dev->event_map.lpi_map,
3635+
its_dev->event_map.nr_lpis)))
3636+
return;
3637+
3638+
its_lpi_free(its_dev->event_map.lpi_map,
3639+
its_dev->event_map.lpi_base,
3640+
its_dev->event_map.nr_lpis);
3641+
3642+
/* Unmap device/itt, and get rid of the tracking */
3643+
its_send_mapd(its_dev, 0);
3644+
its_free_device(its_dev);
3645+
}
3646+
36233647
static struct msi_domain_ops its_msi_domain_ops = {
36243648
.msi_prepare = its_msi_prepare,
3649+
.msi_teardown = its_msi_teardown,
36253650
};
36263651

36273652
static int its_irq_gic_domain_alloc(struct irq_domain *domain,
@@ -3722,7 +3747,6 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
37223747
{
37233748
struct irq_data *d = irq_domain_get_irq_data(domain, virq);
37243749
struct its_device *its_dev = irq_data_get_irq_chip_data(d);
3725-
struct its_node *its = its_dev->its;
37263750
int i;
37273751

37283752
bitmap_release_region(its_dev->event_map.lpi_map,
@@ -3736,26 +3760,6 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
37363760
irq_domain_reset_irq_data(data);
37373761
}
37383762

3739-
mutex_lock(&its->dev_alloc_lock);
3740-
3741-
/*
3742-
* If all interrupts have been freed, start mopping the
3743-
* floor. This is conditioned on the device not being shared.
3744-
*/
3745-
if (!its_dev->shared &&
3746-
bitmap_empty(its_dev->event_map.lpi_map,
3747-
its_dev->event_map.nr_lpis)) {
3748-
its_lpi_free(its_dev->event_map.lpi_map,
3749-
its_dev->event_map.lpi_base,
3750-
its_dev->event_map.nr_lpis);
3751-
3752-
/* Unmap device/itt */
3753-
its_send_mapd(its_dev, 0);
3754-
its_free_device(its_dev);
3755-
}
3756-
3757-
mutex_unlock(&its->dev_alloc_lock);
3758-
37593763
irq_domain_free_irqs_parent(domain, virq, nr_irqs);
37603764
}
37613765

kernel/irq/msi.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -795,8 +795,7 @@ static int msi_domain_ops_prepare(struct irq_domain *domain, struct device *dev,
795795
return 0;
796796
}
797797

798-
static void msi_domain_ops_teardown(struct irq_domain *domain,
799-
msi_alloc_info_t *arg)
798+
static void msi_domain_ops_teardown(struct irq_domain *domain, msi_alloc_info_t *arg)
800799
{
801800
}
802801

0 commit comments

Comments
 (0)