Mailing List Archive

[patch 03/10] genirq/msi: Make MSI descriptor alloc/free ready for range allocations
Convert the MSI descriptor related functions to ranges and fixup the call
sites.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
drivers/base/platform-msi.c | 3 ++-
include/linux/msi.h | 7 ++++---
kernel/irq/msi.c | 38 +++++++++++++++++++-------------------
3 files changed, 25 insertions(+), 23 deletions(-)

--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -320,11 +320,12 @@ struct irq_domain *
void platform_msi_device_domain_free(struct irq_domain *domain, unsigned int virq,
unsigned int nr_irqs)
{
+ struct msi_range range = { .first = virq, .last = virq + nr_irqs - 1, };
struct platform_msi_priv_data *data = domain->host_data;

msi_lock_descs(data->dev);
irq_domain_free_irqs_common(domain, virq, nr_irqs);
- msi_free_msi_descs_range(data->dev, MSI_DESC_ALL, virq, nr_irqs);
+ msi_free_msi_descs_range(data->dev, MSI_DESC_ALL, &range);
msi_unlock_descs(data->dev);
}

--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -321,8 +321,7 @@ static inline void pci_write_msi_msg(uns
#endif /* CONFIG_PCI_MSI */

int msi_add_msi_desc(struct device *dev, struct msi_desc *init_desc);
-void msi_free_msi_descs_range(struct device *dev, enum msi_desc_filter filter,
- unsigned int base_index, unsigned int ndesc);
+void msi_free_msi_descs_range(struct device *dev, enum msi_desc_filter filter, struct msi_range *range);

/**
* msi_free_msi_descs - Free MSI descriptors of a device
@@ -330,7 +329,9 @@ void msi_free_msi_descs_range(struct dev
*/
static inline void msi_free_msi_descs(struct device *dev)
{
- msi_free_msi_descs_range(dev, MSI_DESC_ALL, 0, UINT_MAX);
+ struct msi_range range = { .first = 0, .last = UINT_MAX, };
+
+ msi_free_msi_descs_range(dev, MSI_DESC_ALL, &range);
}

void __pci_read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -101,19 +101,19 @@ int msi_add_msi_desc(struct device *dev,
*
* Return: 0 on success or an appropriate failure code.
*/
-static int msi_add_simple_msi_descs(struct device *dev, unsigned int index, unsigned int ndesc)
+static int msi_add_simple_msi_descs(struct device *dev, struct msi_range *range)
{
struct msi_desc *desc;
- unsigned long i;
+ unsigned long idx;
int ret;

lockdep_assert_held(&dev->msi.data->mutex);

- for (i = 0; i < ndesc; i++) {
+ for (idx = range->first; idx <= range->last; idx++) {
desc = msi_alloc_desc(dev, 1, NULL);
if (!desc)
goto fail_mem;
- ret = msi_insert_desc(dev->msi.data, desc, index + i);
+ ret = msi_insert_desc(dev->msi.data, desc, idx);
if (ret)
goto fail;
}
@@ -122,7 +122,7 @@ static int msi_add_simple_msi_descs(stru
fail_mem:
ret = -ENOMEM;
fail:
- msi_free_msi_descs_range(dev, MSI_DESC_NOTASSOCIATED, index, ndesc);
+ msi_free_msi_descs_range(dev, MSI_DESC_NOTASSOCIATED, range);
return ret;
}

@@ -148,14 +148,14 @@ static bool msi_desc_match(struct msi_de
* @ndesc: Number of descriptors to free
*/
void msi_free_msi_descs_range(struct device *dev, enum msi_desc_filter filter,
- unsigned int base_index, unsigned int ndesc)
+ struct msi_range *range)
{
struct msi_desc *desc;
unsigned long idx;

lockdep_assert_held(&dev->msi.data->mutex);

- xa_for_each_range(&dev->msi.data->store, idx, desc, base_index, base_index + ndesc - 1) {
+ xa_for_each_range(&dev->msi.data->store, idx, desc, range->first, range->last) {
if (msi_desc_match(desc, filter)) {
xa_erase(&dev->msi.data->store, idx);
msi_free_desc(desc);
@@ -746,17 +746,18 @@ int msi_domain_prepare_irqs(struct irq_d
int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev,
int virq_base, int nvec, msi_alloc_info_t *arg)
{
+ struct msi_range range = { .first = virq_base, .last = virq_base + nvec - 1 };
struct msi_domain_info *info = domain->host_data;
struct msi_domain_ops *ops = info->ops;
struct msi_desc *desc;
int ret, virq;

msi_lock_descs(dev);
- ret = msi_add_simple_msi_descs(dev, virq_base, nvec);
+ ret = msi_add_simple_msi_descs(dev, &range);
if (ret)
goto unlock;

- for (virq = virq_base; virq < virq_base + nvec; virq++) {
+ for (virq = range.first; virq <= range.last; virq++) {
desc = xa_load(&dev->msi.data->store, virq);
desc->irq = virq;

@@ -773,7 +774,7 @@ int msi_domain_populate_irqs(struct irq_
fail:
for (--virq; virq >= virq_base; virq--)
irq_domain_free_irqs_common(domain, virq, 1);
- msi_free_msi_descs_range(dev, MSI_DESC_ALL, virq_base, nvec);
+ msi_free_msi_descs_range(dev, MSI_DESC_ALL, &range);
unlock:
msi_unlock_descs(dev);
return ret;
@@ -932,14 +933,13 @@ int __msi_domain_alloc_irqs(struct irq_d
return 0;
}

-static int msi_domain_add_simple_msi_descs(struct msi_domain_info *info,
- struct device *dev,
- unsigned int num_descs)
+static int msi_domain_add_simple_msi_descs(struct msi_domain_info *info, struct device *dev,
+ struct msi_range *range)
{
if (!(info->flags & MSI_FLAG_ALLOC_SIMPLE_MSI_DESCS))
return 0;

- return msi_add_simple_msi_descs(dev, 0, num_descs);
+ return msi_add_simple_msi_descs(dev, range);
}

/**
@@ -964,7 +964,7 @@ int msi_domain_alloc_irqs_descs_locked(s

lockdep_assert_held(&dev->msi.data->mutex);

- ret = msi_domain_add_simple_msi_descs(info, dev, range->ndesc);
+ ret = msi_domain_add_simple_msi_descs(info, dev, range);
if (ret)
return ret;

@@ -1017,11 +1017,11 @@ void __msi_domain_free_irqs(struct irq_d
}
}

-static void msi_domain_free_msi_descs(struct msi_domain_info *info,
- struct device *dev)
+static void msi_domain_free_msi_descs(struct msi_domain_info *info, struct device *dev,
+ struct msi_range *range)
{
if (info->flags & MSI_FLAG_FREE_MSI_DESCS)
- msi_free_msi_descs(dev);
+ msi_free_msi_descs_range(dev, MSI_DESC_ALL, range);
}

/**
@@ -1043,7 +1043,7 @@ void msi_domain_free_irqs_descs_locked(s
lockdep_assert_held(&dev->msi.data->mutex);

ops->domain_free_irqs(domain, dev, range);
- msi_domain_free_msi_descs(info, dev);
+ msi_domain_free_msi_descs(info, dev, range);
}

/**
[patch 03/10] genirq/msi: Make MSI descriptor alloc/free ready for range allocations [ In reply to ]
Convert the MSI descriptor related functions to ranges and fixup the call
sites.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
drivers/base/platform-msi.c | 3 ++-
include/linux/msi.h | 7 ++++---
kernel/irq/msi.c | 38 +++++++++++++++++++-------------------
3 files changed, 25 insertions(+), 23 deletions(-)

--- a/drivers/base/platform-msi.c
+++ b/drivers/base/platform-msi.c
@@ -320,11 +320,12 @@ struct irq_domain *
void platform_msi_device_domain_free(struct irq_domain *domain, unsigned int virq,
unsigned int nr_irqs)
{
+ struct msi_range range = { .first = virq, .last = virq + nr_irqs - 1, };
struct platform_msi_priv_data *data = domain->host_data;

msi_lock_descs(data->dev);
irq_domain_free_irqs_common(domain, virq, nr_irqs);
- msi_free_msi_descs_range(data->dev, MSI_DESC_ALL, virq, nr_irqs);
+ msi_free_msi_descs_range(data->dev, MSI_DESC_ALL, &range);
msi_unlock_descs(data->dev);
}

--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -321,8 +321,7 @@ static inline void pci_write_msi_msg(uns
#endif /* CONFIG_PCI_MSI */

int msi_add_msi_desc(struct device *dev, struct msi_desc *init_desc);
-void msi_free_msi_descs_range(struct device *dev, enum msi_desc_filter filter,
- unsigned int base_index, unsigned int ndesc);
+void msi_free_msi_descs_range(struct device *dev, enum msi_desc_filter filter, struct msi_range *range);

/**
* msi_free_msi_descs - Free MSI descriptors of a device
@@ -330,7 +329,9 @@ void msi_free_msi_descs_range(struct dev
*/
static inline void msi_free_msi_descs(struct device *dev)
{
- msi_free_msi_descs_range(dev, MSI_DESC_ALL, 0, UINT_MAX);
+ struct msi_range range = { .first = 0, .last = UINT_MAX, };
+
+ msi_free_msi_descs_range(dev, MSI_DESC_ALL, &range);
}

void __pci_read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -101,19 +101,19 @@ int msi_add_msi_desc(struct device *dev,
*
* Return: 0 on success or an appropriate failure code.
*/
-static int msi_add_simple_msi_descs(struct device *dev, unsigned int index, unsigned int ndesc)
+static int msi_add_simple_msi_descs(struct device *dev, struct msi_range *range)
{
struct msi_desc *desc;
- unsigned long i;
+ unsigned long idx;
int ret;

lockdep_assert_held(&dev->msi.data->mutex);

- for (i = 0; i < ndesc; i++) {
+ for (idx = range->first; idx <= range->last; idx++) {
desc = msi_alloc_desc(dev, 1, NULL);
if (!desc)
goto fail_mem;
- ret = msi_insert_desc(dev->msi.data, desc, index + i);
+ ret = msi_insert_desc(dev->msi.data, desc, idx);
if (ret)
goto fail;
}
@@ -122,7 +122,7 @@ static int msi_add_simple_msi_descs(stru
fail_mem:
ret = -ENOMEM;
fail:
- msi_free_msi_descs_range(dev, MSI_DESC_NOTASSOCIATED, index, ndesc);
+ msi_free_msi_descs_range(dev, MSI_DESC_NOTASSOCIATED, range);
return ret;
}

@@ -148,14 +148,14 @@ static bool msi_desc_match(struct msi_de
* @ndesc: Number of descriptors to free
*/
void msi_free_msi_descs_range(struct device *dev, enum msi_desc_filter filter,
- unsigned int base_index, unsigned int ndesc)
+ struct msi_range *range)
{
struct msi_desc *desc;
unsigned long idx;

lockdep_assert_held(&dev->msi.data->mutex);

- xa_for_each_range(&dev->msi.data->store, idx, desc, base_index, base_index + ndesc - 1) {
+ xa_for_each_range(&dev->msi.data->store, idx, desc, range->first, range->last) {
if (msi_desc_match(desc, filter)) {
xa_erase(&dev->msi.data->store, idx);
msi_free_desc(desc);
@@ -746,17 +746,18 @@ int msi_domain_prepare_irqs(struct irq_d
int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev,
int virq_base, int nvec, msi_alloc_info_t *arg)
{
+ struct msi_range range = { .first = virq_base, .last = virq_base + nvec - 1 };
struct msi_domain_info *info = domain->host_data;
struct msi_domain_ops *ops = info->ops;
struct msi_desc *desc;
int ret, virq;

msi_lock_descs(dev);
- ret = msi_add_simple_msi_descs(dev, virq_base, nvec);
+ ret = msi_add_simple_msi_descs(dev, &range);
if (ret)
goto unlock;

- for (virq = virq_base; virq < virq_base + nvec; virq++) {
+ for (virq = range.first; virq <= range.last; virq++) {
desc = xa_load(&dev->msi.data->store, virq);
desc->irq = virq;

@@ -773,7 +774,7 @@ int msi_domain_populate_irqs(struct irq_
fail:
for (--virq; virq >= virq_base; virq--)
irq_domain_free_irqs_common(domain, virq, 1);
- msi_free_msi_descs_range(dev, MSI_DESC_ALL, virq_base, nvec);
+ msi_free_msi_descs_range(dev, MSI_DESC_ALL, &range);
unlock:
msi_unlock_descs(dev);
return ret;
@@ -932,14 +933,13 @@ int __msi_domain_alloc_irqs(struct irq_d
return 0;
}

-static int msi_domain_add_simple_msi_descs(struct msi_domain_info *info,
- struct device *dev,
- unsigned int num_descs)
+static int msi_domain_add_simple_msi_descs(struct msi_domain_info *info, struct device *dev,
+ struct msi_range *range)
{
if (!(info->flags & MSI_FLAG_ALLOC_SIMPLE_MSI_DESCS))
return 0;

- return msi_add_simple_msi_descs(dev, 0, num_descs);
+ return msi_add_simple_msi_descs(dev, range);
}

/**
@@ -964,7 +964,7 @@ int msi_domain_alloc_irqs_descs_locked(s

lockdep_assert_held(&dev->msi.data->mutex);

- ret = msi_domain_add_simple_msi_descs(info, dev, range->ndesc);
+ ret = msi_domain_add_simple_msi_descs(info, dev, range);
if (ret)
return ret;

@@ -1017,11 +1017,11 @@ void __msi_domain_free_irqs(struct irq_d
}
}

-static void msi_domain_free_msi_descs(struct msi_domain_info *info,
- struct device *dev)
+static void msi_domain_free_msi_descs(struct msi_domain_info *info, struct device *dev,
+ struct msi_range *range)
{
if (info->flags & MSI_FLAG_FREE_MSI_DESCS)
- msi_free_msi_descs(dev);
+ msi_free_msi_descs_range(dev, MSI_DESC_ALL, range);
}

/**
@@ -1043,7 +1043,7 @@ void msi_domain_free_irqs_descs_locked(s
lockdep_assert_held(&dev->msi.data->mutex);

ops->domain_free_irqs(domain, dev, range);
- msi_domain_free_msi_descs(info, dev);
+ msi_domain_free_msi_descs(info, dev, range);
}

/**
Re: [patch 03/10] genirq/msi: Make MSI descriptor alloc/free ready for range allocations [ In reply to ]
On Sat, 27 Nov 2021 01:24:34 +0000,
Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Convert the MSI descriptor related functions to ranges and fixup the call
> sites.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> drivers/base/platform-msi.c | 3 ++-
> include/linux/msi.h | 7 ++++---
> kernel/irq/msi.c | 38 +++++++++++++++++++-------------------
> 3 files changed, 25 insertions(+), 23 deletions(-)

This particular patch breaks one of my test boxes when allocating the
MSIs for the first SMMUv3 it encounters:

[ 14.700206] arm-smmu-v3 arm-smmu-v3.0.auto: option mask 0x0
[ 14.705848] arm-smmu-v3 arm-smmu-v3.0.auto: ias 48-bit, oas 48-bit (features 0x00041fff)
[ 14.716184] arm-smmu-v3 arm-smmu-v3.0.auto: allocated 65536 entries for cmdq
[ 14.723285] arm-smmu-v3 arm-smmu-v3.0.auto: allocated 128 entries for evtq
[ 14.730170] arm-smmu-v3 arm-smmu-v3.0.auto: allocated 256 entries for priq
[ 41.282305] watchdog: BUG: soft lockup - CPU#43 stuck for 26s! [swapper/0:1]
[ 41.289383] Modules linked in:
[ 41.292430] irq event stamp: 3906684
[ 41.295994] hardirqs last enabled at (3906683): [<ffffb73f677c54d8>] ___slab_alloc+0x7c8/0x8c0
[ 41.304698] hardirqs last disabled at (3906684): [<ffffb73f6806d478>] el1_interrupt+0x38/0xb0
[ 41.313220] softirqs last enabled at (3798058): [<ffffb73f6746099c>] __do_softirq+0x40c/0x58c
[ 41.321825] softirqs last disabled at (3798053): [<ffffb73f674ee610>] __irq_exit_rcu+0x120/0x160
[ 41.330607] CPU: 43 PID: 1 Comm: swapper/0 Not tainted 5.16.0-rc2-00078-g76af42494903 #41
[ 41.338775] Hardware name: WIWYNN Mt.Jade Server System B81.030Z1.0007/Mt.Jade Motherboard, BIOS 1.3.20210110 2021/01/10
[ 41.349634] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 41.356585] pc : lock_is_held_type+0x124/0x20c
[ 41.361019] lr : lock_is_held_type+0xe8/0x20c
[ 41.365365] sp : ffff80001051b840
[ 41.368669] pmr_save: 000000e0
[ 41.371712] x29: ffff80001051b840 x28: ffff000034d80000 x27: ffff07ff87606600
[ 41.378838] x26: 00000000000000e0 x25: 00000000ffffffff x24: ffffb73f68733d00
[ 41.385964] x23: 0000000000000028 x22: ffff07ff87606fb8 x21: ffffb73f68bffc38
[ 41.393090] x20: ffff07ff87606fe0 x19: 0000000000000002 x18: 0000000000000014
[ 41.400217] x17: 0000000076a13aac x16: 00000000486de301 x15: 00000000ed5ff5e1
[ 41.407342] x14: 00000000fda7b077 x13: 0000000000000006 x12: 00000000b82b73dd
[ 41.414468] x11: ffff07ff87606fb8 x10: ffffb73f6952d000 x9 : ffffb73f675a3750
[ 41.421594] x8 : 0000000000000000 x7 : ffffb73f68e16000 x6 : ffffb73f677c6d64
[ 41.428720] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 000000000000ffff
[ 41.435846] x2 : ffff50ff17454000 x1 : 0000000000000000 x0 : 0000000000000000
[ 41.442972] Call trace:
[ 41.445407] lock_is_held_type+0x124/0x20c
[ 41.449494] rcu_read_lock_sched_held+0x68/0xac
[ 41.454018] trace_lock_acquire+0x78/0x1c0
[ 41.458107] lock_acquire+0x40/0x90
[ 41.461585] fs_reclaim_acquire+0x90/0x114
[ 41.465676] kmem_cache_alloc_trace+0x80/0x300
[ 41.470110] msi_add_simple_msi_descs+0x70/0x150
[ 41.474718] msi_domain_alloc_irqs_descs_locked+0x90/0xfc
[ 41.480106] msi_domain_alloc_irqs+0x58/0xa0
[ 41.484364] platform_msi_domain_alloc_irqs+0x5c/0xa0
[ 41.489410] arm_smmu_device_probe+0xfc0/0x1230
[ 41.493936] platform_probe+0x74/0xe4
[ 41.497590] really_probe+0xc4/0x470
[ 41.501156] __driver_probe_device+0x11c/0x190
[ 41.505589] driver_probe_device+0x48/0x110
[ 41.509761] __driver_attach+0xe0/0x200
[ 41.513585] bus_for_each_dev+0x7c/0xe0
[ 41.517412] driver_attach+0x30/0x3c
[ 41.520976] bus_add_driver+0x150/0x230
[ 41.524801] driver_register+0x84/0x140
[ 41.528626] __platform_driver_register+0x34/0x40
[ 41.533319] arm_smmu_driver_init+0x2c/0x38
[ 41.537496] do_one_initcall+0x80/0x3d0
[ 41.541321] kernel_init_freeable+0x318/0x3a0
[ 41.545672] kernel_init+0x30/0x14c
[ 41.549151] ret_from_fork+0x10/0x20

The issue seems to be originating in the previous patch, where the
following line was added:

+ struct msi_range range = { .first = 0, .last = UINT_MAX, .ndesc = nvec, };

In that context, only 'ndesc' was used, and that was fine.

However, in the current patch, ndesc use is removed, only first/last
are considered, and UINT_MAX is... a lot of MSIs.

This fixes it:

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index bef5b74a7268..a520bfd94a56 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -975,7 +975,7 @@ int msi_domain_alloc_irqs_descs_locked(struct irq_domain *domain, struct device
*/
int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, int nvec)
{
- struct msi_range range = { .first = 0, .last = UINT_MAX, .ndesc = nvec, };
+ struct msi_range range = { .first = 0, .last = nvec - 1, .ndesc = nvec, };
int ret;

msi_lock_descs(dev);

However, it'd be good to clarify the use of range->ndesc.

[...]

> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -101,19 +101,19 @@ int msi_add_msi_desc(struct device *dev,
> *
> * Return: 0 on success or an appropriate failure code.
> */
> -static int msi_add_simple_msi_descs(struct device *dev, unsigned int index, unsigned int ndesc)
> +static int msi_add_simple_msi_descs(struct device *dev, struct msi_range *range)

nit: most of the functions changed in this patch need to have their
documentation tidied up.

Thanks,

M.

--
Without deviation from the norm, progress is not possible.
Re: [patch 03/10] genirq/msi: Make MSI descriptor alloc/free ready for range allocations [ In reply to ]
On Sun, Nov 28 2021 at 15:57, Marc Zyngier wrote:
> On Sat, 27 Nov 2021 01:24:34 +0000,
> Thomas Gleixner <tglx@linutronix.de> wrote:
>
> The issue seems to be originating in the previous patch, where the
> following line was added:
>
> + struct msi_range range = { .first = 0, .last = UINT_MAX, .ndesc = nvec, };
>
> In that context, only 'ndesc' was used, and that was fine.
>
> However, in the current patch, ndesc use is removed, only first/last
> are considered, and UINT_MAX is... a lot of MSIs.
>
> This fixes it:
>
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index bef5b74a7268..a520bfd94a56 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -975,7 +975,7 @@ int msi_domain_alloc_irqs_descs_locked(struct irq_domain *domain, struct device
> */
> int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, int nvec)
> {
> - struct msi_range range = { .first = 0, .last = UINT_MAX, .ndesc = nvec, };
> + struct msi_range range = { .first = 0, .last = nvec - 1, .ndesc = nvec, };
> int ret;
>
> msi_lock_descs(dev);
>
> However, it'd be good to clarify the use of range->ndesc.

Hrm. The stupid search should terminated nevertheless. Let me stare at
it again.

>> -static int msi_add_simple_msi_descs(struct device *dev, unsigned int index, unsigned int ndesc)
>> +static int msi_add_simple_msi_descs(struct device *dev, struct msi_range *range)
>
> nit: most of the functions changed in this patch need to have their
> documentation tidied up.

Duh, yes.

Thanks,

tglx
Re: [patch 03/10] genirq/msi: Make MSI descriptor alloc/free ready for range allocations [ In reply to ]
On Sun, Nov 28 2021 at 20:17, Thomas Gleixner wrote:
> On Sun, Nov 28 2021 at 15:57, Marc Zyngier wrote:
> Hrm. The stupid search should terminated nevertheless. Let me stare at
> it again.

Found it. Just my inability to read xarray documentation.

Thanks,

tglx