2022-11-11 14:43:28

by Thomas Gleixner

Subject: [patch 06/20] genirq/msi: Check for invalid MSI parent domain usage

In the upcoming per device MSI domain concept the MSI parent domains are
not allowed to be used as regular MSI domains where the MSI allocation/free
operations are applicable.

Add appropriate checks.

Signed-off-by: Thomas Gleixner <[email protected]>
---
kernel/irq/msi.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)

--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -937,13 +937,21 @@ int msi_domain_alloc_irqs_descs_locked(s

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

+ if (WARN_ON_ONCE(irq_domain_is_msi_parent(domain))) {
+ ret = -EINVAL;
+ goto free;
+ }
+
+ /* Frees allocated descriptors in case of failure. */
ret = msi_domain_add_simple_msi_descs(info, dev, nvec);
if (ret)
return ret;

ret = ops->domain_alloc_irqs(domain, dev, nvec);
- if (ret)
- msi_domain_free_irqs_descs_locked(domain, dev);
+ if (!ret)
+ return 0;
+free:
+ msi_domain_free_irqs_descs_locked(domain, dev);
return ret;
}

@@ -1013,6 +1021,9 @@ void msi_domain_free_irqs_descs_locked(s

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

+ if (WARN_ON_ONCE(irq_domain_is_msi_parent(domain)))
+ return;
+
ops->domain_free_irqs(domain, dev);
if (ops->msi_post_free)
ops->msi_post_free(domain, dev);

2022-11-18 08:29:37

by Tian, Kevin

Subject: RE: [patch 06/20] genirq/msi: Check for invalid MSI parent domain usage

> From: Thomas Gleixner <[email protected]>
> Sent: Friday, November 11, 2022 9:57 PM
>
> @@ -937,13 +937,21 @@ int msi_domain_alloc_irqs_descs_locked(s
>
> lockdep_assert_held(&dev->msi.data->mutex);
>
> + if (WARN_ON_ONCE(irq_domain_is_msi_parent(domain))) {
> + ret = -EINVAL;
> + goto free;
> + }
> +
> + /* Frees allocated descriptors in case of failure. */
> ret = msi_domain_add_simple_msi_descs(info, dev, nvec);
> if (ret)
> return ret;

it's unusual to see a direct return when error unwind is already required
at an early failure point. is it something which can be improved here?

2022-11-18 12:53:10

by Thomas Gleixner

Subject: RE: [patch 06/20] genirq/msi: Check for invalid MSI parent domain usage

On Fri, Nov 18 2022 at 07:50, Kevin Tian wrote:
>> From: Thomas Gleixner <[email protected]>
>> Sent: Friday, November 11, 2022 9:57 PM
>>
>> @@ -937,13 +937,21 @@ int msi_domain_alloc_irqs_descs_locked(s
>>
>> lockdep_assert_held(&dev->msi.data->mutex);
>>
>> + if (WARN_ON_ONCE(irq_domain_is_msi_parent(domain))) {
>> + ret = -EINVAL;
>> + goto free;
>> + }
>> +
>> + /* Frees allocated descriptors in case of failure. */
>> ret = msi_domain_add_simple_msi_descs(info, dev, nvec);
>> if (ret)
>> return ret;
>
> it's unusual to see a direct return when error unwind is already required
> at an early failure point. is it something which can be improved here?

It's redundant in this case, but you are right it looks weird.