diff mbox series

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

Message ID 20221111132706.388892782@linutronix.de
State New
Headers
Series [01/20] genirq/msi: Move IRQ_DOMAIN_MSI_NOMASK_QUIRK to MSI flags |

Commit Message

Thomas Gleixner Nov. 11, 2022, 1:56 p.m. UTC 
  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 <tglx@linutronix.de>
---
 kernel/irq/msi.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Tian, Kevin Nov. 18, 2022, 7:50 a.m. UTC | #1 
> From: Thomas Gleixner <tglx@linutronix.de>
> 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?
Thomas Gleixner Nov. 18, 2022, 12:15 p.m. UTC | #2 
On Fri, Nov 18 2022 at 07:50, Kevin Tian wrote:
>> From: Thomas Gleixner <tglx@linutronix.de>
>> 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.
diff mbox series

Patch

--- 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);