There are unpaired calls in the msi:
msi_domain_alloc_irqs() {
...
__irq_domain_alloc_irqs()
...
irq_domain_activate_irq()
}
msi_domain_free_irqs() {
...
irq_domain_free_irqs()
}
When msi_domain_alloc_irqs() and msi_domain_free_irqs() are called in
pairs, the irq_domain_deactivate_irq() is missing calls.
This problem occurs during PCI initialization.After pci_msi_setup_msi_irqs
is executed (invoke irq_domain_activate_irq to initialize the MSI irq),
error_cleanup_irqs() of pcie_port_device_register() is executed.
As follows:
pcie_port_device_register() {
...
error_cleanup_irqs:
pci_free_irq_vectors(dev);
}
Invoking Procedure:
pcie_port_device_register
-> goto error_cleanup_irqs
-> pci_free_irq_vectors(dev);
pci_disable_msi
free_msi_irqs
pci_msi_teardown_msi_irqs
msi_domain_free_irqs // no deactivate before free
irq_domain_free_irqs
Signed-off-by: Bixuan Cui <[email protected]>
---
kernel/irq/msi.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index c41965e348b5..8828d4863c5d 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -476,11 +476,6 @@ int __msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
return 0;
cleanup:
- for_each_msi_vector(desc, i, dev) {
- irq_data = irq_domain_get_irq_data(domain, i);
- if (irqd_is_activated(irq_data))
- irq_domain_deactivate_irq(irq_data);
- }
msi_domain_free_irqs(domain, dev);
return ret;
}
@@ -506,6 +501,14 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
void __msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
{
struct msi_desc *desc;
+ struct irq_data *irq_data;
+ int i;
+
+ for_each_msi_vector(desc, i, dev) {
+ irq_data = irq_domain_get_irq_data(domain, i);
+ if (irqd_is_activated(irq_data))
+ irq_domain_deactivate_irq(irq_data);
+ }
for_each_msi_entry(desc, dev) {
/*
--
2.17.1
A gentle ping :)
Thanks
Bixuan Cui
On 2021/5/18 11:31, Bixuan Cui wrote:
> There are unpaired calls in the msi:
>
> msi_domain_alloc_irqs() {
> ...
> __irq_domain_alloc_irqs()
> ...
> irq_domain_activate_irq()
> }
>
> msi_domain_free_irqs() {
> ...
> irq_domain_free_irqs()
> }
>
> When msi_domain_alloc_irqs() and msi_domain_free_irqs() are called in
> pairs, the irq_domain_deactivate_irq() is missing calls.
>
> This problem occurs during PCI initialization.After pci_msi_setup_msi_irqs
> is executed (invoke irq_domain_activate_irq to initialize the MSI irq),
> error_cleanup_irqs() of pcie_port_device_register() is executed.
> As follows:
>
> pcie_port_device_register() {
> ...
>
> error_cleanup_irqs:
> pci_free_irq_vectors(dev);
> }
>
> Invoking Procedure:
> pcie_port_device_register
> -> goto error_cleanup_irqs
> -> pci_free_irq_vectors(dev);
> pci_disable_msi
> free_msi_irqs
> pci_msi_teardown_msi_irqs
> msi_domain_free_irqs // no deactivate before free
> irq_domain_free_irqs
>
> Signed-off-by: Bixuan Cui <[email protected]>
> ---
> kernel/irq/msi.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index c41965e348b5..8828d4863c5d 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -476,11 +476,6 @@ int __msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
> return 0;
>
> cleanup:
> - for_each_msi_vector(desc, i, dev) {
> - irq_data = irq_domain_get_irq_data(domain, i);
> - if (irqd_is_activated(irq_data))
> - irq_domain_deactivate_irq(irq_data);
> - }
> msi_domain_free_irqs(domain, dev);
> return ret;
> }
> @@ -506,6 +501,14 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
> void __msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
> {
> struct msi_desc *desc;
> + struct irq_data *irq_data;
> + int i;
> +
> + for_each_msi_vector(desc, i, dev) {
> + irq_data = irq_domain_get_irq_data(domain, i);
> + if (irqd_is_activated(irq_data))
> + irq_domain_deactivate_irq(irq_data);
> + }
>
> for_each_msi_entry(desc, dev) {
> /*
>
The following commit has been merged into the irq/urgent branch of tip:
Commit-ID: dbbc93576e03fbe24b365fab0e901eb442237a8a
Gitweb: https://git.kernel.org/tip/dbbc93576e03fbe24b365fab0e901eb442237a8a
Author: Bixuan Cui <[email protected]>
AuthorDate: Tue, 18 May 2021 11:31:17 +08:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Tue, 10 Aug 2021 15:55:19 +02:00
genirq/msi: Ensure deactivation on teardown
msi_domain_alloc_irqs() invokes irq_domain_activate_irq(), but
msi_domain_free_irqs() does not enforce deactivation before tearing down
the interrupts.
This happens when PCI/MSI interrupts are set up and never used before being
torn down again, e.g. in error handling pathes. The only place which cleans
that up is the error handling path in msi_domain_alloc_irqs().
Move the cleanup from msi_domain_alloc_irqs() into msi_domain_free_irqs()
to cure that.
Fixes: f3b0946d629c ("genirq/msi: Make sure PCI MSIs are activated early")
Signed-off-by: Bixuan Cui <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: [email protected]
Link: https://lore.kernel.org/r/[email protected]
---
kernel/irq/msi.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index c41965e..85df3ca 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -476,11 +476,6 @@ skip_activate:
return 0;
cleanup:
- for_each_msi_vector(desc, i, dev) {
- irq_data = irq_domain_get_irq_data(domain, i);
- if (irqd_is_activated(irq_data))
- irq_domain_deactivate_irq(irq_data);
- }
msi_domain_free_irqs(domain, dev);
return ret;
}
@@ -505,7 +500,15 @@ int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
void __msi_domain_free_irqs(struct irq_domain *domain, struct device *dev)
{
+ struct irq_data *irq_data;
struct msi_desc *desc;
+ int i;
+
+ for_each_msi_vector(desc, i, dev) {
+ irq_data = irq_domain_get_irq_data(domain, i);
+ if (irqd_is_activated(irq_data))
+ irq_domain_deactivate_irq(irq_data);
+ }
for_each_msi_entry(desc, dev) {
/*