From: Brett Creeley <brett.creeley@amd.com>
To: <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
<pabeni@redhat.com>, <netdev@vger.kernel.org>,
<linux-kernel@vger.kernel.org>
Cc: <shannon.nelson@amd.com>, <brett.creeley@amd.com>
Subject: [PATCH v2 net-next 09/10] pds_core: Rework teardown/setup flow to be more common
Date: Fri, 26 Jan 2024 09:42:54 -0800 [thread overview]
Message-ID: <20240126174255.17052-10-brett.creeley@amd.com> (raw)
In-Reply-To: <20240126174255.17052-1-brett.creeley@amd.com>
Currently the teardown/setup flow for driver probe/remove is quite
a bit different from the reset flows in pdsc_fw_down()/pdsc_fw_up().
One key piece that's missing are the calls to pci_alloc_irq_vectors()
and pci_free_irq_vectors(). The pcie reset case is calling
pci_free_irq_vectors() on reset_prepare, but not calling the
corresponding pci_alloc_irq_vectors() on reset_done. This is causing
unexpected/unwanted interrupt behavior due to the adminq interrupt
being accidentally put into legacy interrupt mode. Also, the
pci_alloc_irq_vectors()/pci_free_irq_vectors() functions are being
called directly in probe/remove respectively.
Fix this inconsistency by making the following changes:
1. Always call pdsc_dev_init() in pdsc_setup(), which calls
pci_alloc_irq_vectors() and get rid of the now unused
pds_dev_reinit().
2. Always free/clear the pdsc->intr_info in pdsc_teardown()
since this structure will get re-alloced in pdsc_setup().
3. Move the calls of pci_free_irq_vectors() to pdsc_teardown()
since pci_alloc_irq_vectors() will always be called in
pdsc_setup()->pdsc_dev_init() for both the probe/remove and
reset flows.
4. Make sure to only create the debugfs "identity" entry when it
doesn't already exist, which it will in the reset case because
it's already been created in the initial call to pdsc_dev_init().
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Reviewed-by: Shannon Nelson <shannon.nelson@amd.com>
---
drivers/net/ethernet/amd/pds_core/core.c | 13 +++++--------
drivers/net/ethernet/amd/pds_core/core.h | 1 -
drivers/net/ethernet/amd/pds_core/debugfs.c | 4 ++++
drivers/net/ethernet/amd/pds_core/dev.c | 7 -------
drivers/net/ethernet/amd/pds_core/main.c | 2 --
5 files changed, 9 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/amd/pds_core/core.c b/drivers/net/ethernet/amd/pds_core/core.c
index fe7e1d10224c..41507ade3570 100644
--- a/drivers/net/ethernet/amd/pds_core/core.c
+++ b/drivers/net/ethernet/amd/pds_core/core.c
@@ -404,10 +404,7 @@ int pdsc_setup(struct pdsc *pdsc, bool init)
int numdescs;
int err;
- if (init)
- err = pdsc_dev_init(pdsc);
- else
- err = pdsc_dev_reinit(pdsc);
+ err = pdsc_dev_init(pdsc);
if (err)
return err;
@@ -476,10 +473,9 @@ void pdsc_teardown(struct pdsc *pdsc, bool removing)
for (i = 0; i < pdsc->nintrs; i++)
pdsc_intr_free(pdsc, i);
- if (removing) {
- kfree(pdsc->intr_info);
- pdsc->intr_info = NULL;
- }
+ kfree(pdsc->intr_info);
+ pdsc->intr_info = NULL;
+ pdsc->nintrs = 0;
}
if (pdsc->kern_dbpage) {
@@ -487,6 +483,7 @@ void pdsc_teardown(struct pdsc *pdsc, bool removing)
pdsc->kern_dbpage = NULL;
}
+ pci_free_irq_vectors(pdsc->pdev);
set_bit(PDSC_S_FW_DEAD, &pdsc->state);
}
diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
index cbd5716f46e6..110c4b826b22 100644
--- a/drivers/net/ethernet/amd/pds_core/core.h
+++ b/drivers/net/ethernet/amd/pds_core/core.h
@@ -281,7 +281,6 @@ int pdsc_devcmd_locked(struct pdsc *pdsc, union pds_core_dev_cmd *cmd,
union pds_core_dev_comp *comp, int max_seconds);
int pdsc_devcmd_init(struct pdsc *pdsc);
int pdsc_devcmd_reset(struct pdsc *pdsc);
-int pdsc_dev_reinit(struct pdsc *pdsc);
int pdsc_dev_init(struct pdsc *pdsc);
void pdsc_reset_prepare(struct pci_dev *pdev);
diff --git a/drivers/net/ethernet/amd/pds_core/debugfs.c b/drivers/net/ethernet/amd/pds_core/debugfs.c
index d56fdbb4cdb9..6bdd02b7aa6d 100644
--- a/drivers/net/ethernet/amd/pds_core/debugfs.c
+++ b/drivers/net/ethernet/amd/pds_core/debugfs.c
@@ -64,6 +64,10 @@ DEFINE_SHOW_ATTRIBUTE(identity);
void pdsc_debugfs_add_ident(struct pdsc *pdsc)
{
+ /* This file will already exist in the reset flow */
+ if (debugfs_lookup("identity", pdsc->dentry))
+ return;
+
debugfs_create_file("identity", 0400, pdsc->dentry,
pdsc, &identity_fops);
}
diff --git a/drivers/net/ethernet/amd/pds_core/dev.c b/drivers/net/ethernet/amd/pds_core/dev.c
index b237cea65a5e..7dc102a31185 100644
--- a/drivers/net/ethernet/amd/pds_core/dev.c
+++ b/drivers/net/ethernet/amd/pds_core/dev.c
@@ -316,13 +316,6 @@ static int pdsc_identify(struct pdsc *pdsc)
return 0;
}
-int pdsc_dev_reinit(struct pdsc *pdsc)
-{
- pdsc_init_devinfo(pdsc);
-
- return pdsc_identify(pdsc);
-}
-
int pdsc_dev_init(struct pdsc *pdsc)
{
unsigned int nintrs;
diff --git a/drivers/net/ethernet/amd/pds_core/main.c b/drivers/net/ethernet/amd/pds_core/main.c
index 05fdeb235e5f..cdbf053b5376 100644
--- a/drivers/net/ethernet/amd/pds_core/main.c
+++ b/drivers/net/ethernet/amd/pds_core/main.c
@@ -438,7 +438,6 @@ static void pdsc_remove(struct pci_dev *pdev)
mutex_destroy(&pdsc->config_lock);
mutex_destroy(&pdsc->devcmd_lock);
- pci_free_irq_vectors(pdev);
pdsc_unmap_bars(pdsc);
pci_release_regions(pdev);
}
@@ -470,7 +469,6 @@ void pdsc_reset_prepare(struct pci_dev *pdev)
pdsc_stop_health_thread(pdsc);
pdsc_fw_down(pdsc);
- pci_free_irq_vectors(pdev);
pdsc_unmap_bars(pdsc);
pci_release_regions(pdev);
pci_disable_device(pdev);
--
2.17.1
next prev parent reply other threads:[~2024-01-26 17:43 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-26 17:42 [PATCH v2 net-next 0/10] pds_core: Various improvements and AQ race condition cleanup Brett Creeley
2024-01-26 17:42 ` [PATCH v2 net-next 01/10] pds_core: Prevent health thread from running during reset/remove Brett Creeley
2024-01-26 17:42 ` [PATCH v2 net-next 02/10] pds_core: Cancel AQ work on teardown Brett Creeley
2024-01-26 17:42 ` [PATCH v2 net-next 03/10] pds_core: Use struct pdsc for the pdsc_adminq_isr private data Brett Creeley
2024-01-26 17:42 ` [PATCH v2 net-next 04/10] pds_core: Prevent race issues involving the adminq Brett Creeley
2024-01-26 17:42 ` [PATCH v2 net-next 05/10] pds_core: Clear BARs on reset Brett Creeley
2024-01-26 17:42 ` [PATCH v2 net-next 06/10] pds_core: Don't assign interrupt index/bound_intr to notifyq Brett Creeley
2024-01-26 17:42 ` [PATCH v2 net-next 07/10] pds_core: Unmask adminq interrupt in work thread Brett Creeley
2024-01-26 17:42 ` [PATCH v2 net-next 08/10] pds_core: Fix up some minor issues Brett Creeley
2024-01-26 17:42 ` Brett Creeley [this message]
2024-01-26 17:42 ` [PATCH v2 net-next 10/10] pds_core: Clean up init/uninit flows to be more readable Brett Creeley
2024-01-27 4:44 ` [PATCH v2 net-next 0/10] pds_core: Various improvements and AQ race condition cleanup Jakub Kicinski
2024-01-29 17:27 ` Brett Creeley
2024-01-29 20:05 ` Jakub Kicinski
2024-01-29 21:12 ` Brett Creeley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240126174255.17052-10-brett.creeley@amd.com \
--to=brett.creeley@amd.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=shannon.nelson@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox