[PATCH net-next 0/8] ionic: updates to PCI error handling

[PATCH net-next 0/8] ionic: updates to PCI error handling

[Shannon Nelson]

These are improvements to our PCI error handling, including FLR and
AER events.

Shannon Nelson (8):
  ionic: pass opcode to devcmd_wait
  ionic: keep filters across FLR
  ionic: bypass firmware cmds when stuck in reset
  ionic: prevent pci disable of already disabled device
  ionic: no fw read when PCI reset failed
  ionic: use timer_shutdown_sync
  ionic: lif debugfs refresh on reset
  ionic: fill out pci error handlers

 .../ethernet/pensando/ionic/ionic_bus_pci.c   | 43 ++++++++++++++++++-
 .../ethernet/pensando/ionic/ionic_debugfs.c   |  3 ++
 .../net/ethernet/pensando/ionic/ionic_dev.c   | 24 ++++++++---
 .../net/ethernet/pensando/ionic/ionic_dev.h   |  1 +
 .../net/ethernet/pensando/ionic/ionic_lif.c   |  3 ++
 .../net/ethernet/pensando/ionic/ionic_main.c  | 22 ++++++----
 6 files changed, 81 insertions(+), 15 deletions(-)

-- 
2.17.1

[PATCH net-next 1/8] ionic: pass opcode to devcmd_wait

[Shannon Nelson]

Don't rely on the PCI memory for the devcmd opcode because we
read a 0xff value if the PCI bus is broken, which can cause us
to report a bogus dev_cmd opcode later.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_dev.c  | 1 +
 drivers/net/ethernet/pensando/ionic/ionic_dev.h  | 1 +
 drivers/net/ethernet/pensando/ionic/ionic_main.c | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
index bb9245d933e4..c0b347dd6bae 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
@@ -321,6 +321,7 @@ void ionic_dev_cmd_comp(struct ionic_dev *idev, union ionic_dev_cmd_comp *comp)
 
 void ionic_dev_cmd_go(struct ionic_dev *idev, union ionic_dev_cmd *cmd)
 {
+	idev->opcode = cmd->cmd.opcode;
 	memcpy_toio(&idev->dev_cmd_regs->cmd, cmd, sizeof(*cmd));
 	iowrite32(0, &idev->dev_cmd_regs->done);
 	iowrite32(1, &idev->dev_cmd_regs->doorbell);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index cee4e5c3d09a..c5a3a966aef6 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -153,6 +153,7 @@ struct ionic_dev {
 	bool fw_hb_ready;
 	bool fw_status_ready;
 	u8 fw_generation;
+	u8 opcode;
 
 	u64 __iomem *db_pages;
 	dma_addr_t phy_db_pages;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 8d15f9203bd5..873a86010b27 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -443,7 +443,7 @@ static int __ionic_dev_cmd_wait(struct ionic *ionic, unsigned long max_seconds,
 	 */
 	max_wait = jiffies + (max_seconds * HZ);
 try_again:
-	opcode = readb(&idev->dev_cmd_regs->cmd.cmd.opcode);
+	opcode = idev->opcode;
 	start_time = jiffies;
 	for (fw_up = ionic_is_fw_running(idev);
 	     !done && fw_up && time_before(jiffies, max_wait);
-- 
2.17.1

[PATCH net-next 2/8] ionic: keep filters across FLR

[Shannon Nelson]

Make sure we keep and replay the filters and RSS config across
an FLR by using our FW_RESET flag.  This gets checked on the
way down and on the way back up to help determine how much LIF
state to keep and restore across a reset action.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index d6ce113a4210..43e7967ad1c5 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -416,6 +416,8 @@ static void ionic_reset_prepare(struct pci_dev *pdev)
 
 	dev_dbg(ionic->dev, "%s: device stopping\n", __func__);
 
+	set_bit(IONIC_LIF_F_FW_RESET, lif->state);
+
 	del_timer_sync(&ionic->watchdog_timer);
 	cancel_work_sync(&lif->deferred.work);
 
-- 
2.17.1

[PATCH net-next 3/8] ionic: bypass firmware cmds when stuck in reset

[Shannon Nelson]

If the driver or firmware is stuck in reset state, don't bother
trying to use adminq commands.  This speeds up shutdown and
prevents unnecessary timeouts and error messages.

This includes a bit of rework on ionic_adminq_post_wait()
and ionic_adminq_post_wait_nomsg() to both use
__ionic_adminq_post_wait() which can do the checks needed in
both cases.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 .../ethernet/pensando/ionic/ionic_bus_pci.c   |  4 ++++
 .../net/ethernet/pensando/ionic/ionic_lif.c   |  3 +++
 .../net/ethernet/pensando/ionic/ionic_main.c  | 20 ++++++++++++-------
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 43e7967ad1c5..f69178b9636f 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -392,6 +392,10 @@ static void ionic_remove(struct pci_dev *pdev)
 	del_timer_sync(&ionic->watchdog_timer);
 
 	if (ionic->lif) {
+		/* prevent adminq cmds if already known as down */
+		if (test_and_clear_bit(IONIC_LIF_F_FW_RESET, ionic->lif->state))
+			set_bit(IONIC_LIF_F_FW_STOPPING, ionic->lif->state);
+
 		ionic_lif_unregister(ionic->lif);
 		ionic_devlink_unregister(ionic);
 		ionic_lif_deinit(ionic->lif);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 6842a31fc04b..6669c5e52c71 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -3161,6 +3161,9 @@ static void ionic_lif_reset(struct ionic_lif *lif)
 {
 	struct ionic_dev *idev = &lif->ionic->idev;
 
+	if (!ionic_is_fw_running(idev))
+		return;
+
 	mutex_lock(&lif->ionic->dev_cmd_lock);
 	ionic_dev_cmd_lif_reset(idev, lif->index);
 	ionic_dev_cmd_wait(lif->ionic, DEVCMD_TIMEOUT);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 873a86010b27..165ab08ad2dd 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -388,22 +388,28 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
 				      do_msg);
 }
 
-int ionic_adminq_post_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx)
+static int __ionic_adminq_post_wait(struct ionic_lif *lif,
+				    struct ionic_admin_ctx *ctx,
+				    const bool do_msg)
 {
 	int err;
 
+	if (!ionic_is_fw_running(&lif->ionic->idev))
+		return 0;
+
 	err = ionic_adminq_post(lif, ctx);
 
-	return ionic_adminq_wait(lif, ctx, err, true);
+	return ionic_adminq_wait(lif, ctx, err, do_msg);
 }
 
-int ionic_adminq_post_wait_nomsg(struct ionic_lif *lif, struct ionic_admin_ctx *ctx)
+int ionic_adminq_post_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx)
 {
-	int err;
-
-	err = ionic_adminq_post(lif, ctx);
+	return __ionic_adminq_post_wait(lif, ctx, true);
+}
 
-	return ionic_adminq_wait(lif, ctx, err, false);
+int ionic_adminq_post_wait_nomsg(struct ionic_lif *lif, struct ionic_admin_ctx *ctx)
+{
+	return __ionic_adminq_post_wait(lif, ctx, false);
 }
 
 static void ionic_dev_cmd_clean(struct ionic *ionic)
-- 
2.17.1

[PATCH net-next 4/8] ionic: prevent pci disable of already disabled device

[Shannon Nelson]

If a reset fails, the PCI device is left in a disabled
state, so don't try to disable it again on driver remove.
This prevents a scary looking WARN trace in the kernel log.

    ionic 0000:2b:00.0: disabling already-disabled device

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index f69178b9636f..da951dc7becb 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -217,7 +217,9 @@ static void ionic_clear_pci(struct ionic *ionic)
 {
 	ionic_unmap_bars(ionic);
 	pci_release_regions(ionic->pdev);
-	pci_disable_device(ionic->pdev);
+
+	if (atomic_read(&ionic->pdev->enable_cnt) > 0)
+		pci_disable_device(ionic->pdev);
 }
 
 static int ionic_setup_one(struct ionic *ionic)
-- 
2.17.1

[PATCH net-next 5/8] ionic: no fw read when PCI reset failed

[Shannon Nelson]

If there was a failed attempt to reset the PCI connection,
don't later try to read from PCI as the space is unmapped
and will cause a paging request crash.  When clearing the PCI
setup we can clear the dev_info register pointer, and check
it before using it in the fw_running test.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 .../ethernet/pensando/ionic/ionic_bus_pci.c   |  5 ++++
 .../net/ethernet/pensando/ionic/ionic_dev.c   | 23 +++++++++++++++----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index da951dc7becb..311d9f4ef0e2 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -215,6 +215,11 @@ static int ionic_sriov_configure(struct pci_dev *pdev, int num_vfs)
 
 static void ionic_clear_pci(struct ionic *ionic)
 {
+	ionic->idev.dev_info_regs = NULL;
+	ionic->idev.dev_cmd_regs = NULL;
+	ionic->idev.intr_status = NULL;
+	ionic->idev.intr_ctrl = NULL;
+
 	ionic_unmap_bars(ionic);
 	pci_release_regions(ionic->pdev);
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
index c0b347dd6bae..1e7c71f7f081 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
@@ -165,9 +165,19 @@ void ionic_dev_teardown(struct ionic *ionic)
 }
 
 /* Devcmd Interface */
-bool ionic_is_fw_running(struct ionic_dev *idev)
+static bool __ionic_is_fw_running(struct ionic_dev *idev, u8 *status_ptr)
 {
-	u8 fw_status = ioread8(&idev->dev_info_regs->fw_status);
+	u8 fw_status;
+
+	if (!idev->dev_info_regs) {
+		if (status_ptr)
+			*status_ptr = 0xff;
+		return false;
+	}
+
+	fw_status = ioread8(&idev->dev_info_regs->fw_status);
+	if (status_ptr)
+		*status_ptr = fw_status;
 
 	/* firmware is useful only if the running bit is set and
 	 * fw_status != 0xff (bad PCI read)
@@ -175,6 +185,11 @@ bool ionic_is_fw_running(struct ionic_dev *idev)
 	return (fw_status != 0xff) && (fw_status & IONIC_FW_STS_F_RUNNING);
 }
 
+bool ionic_is_fw_running(struct ionic_dev *idev)
+{
+	return __ionic_is_fw_running(idev, NULL);
+}
+
 int ionic_heartbeat_check(struct ionic *ionic)
 {
 	unsigned long check_time, last_check_time;
@@ -199,10 +214,8 @@ int ionic_heartbeat_check(struct ionic *ionic)
 		goto do_check_time;
 	}
 
-	fw_status = ioread8(&idev->dev_info_regs->fw_status);
-
 	/* If fw_status is not ready don't bother with the generation */
-	if (!ionic_is_fw_running(idev)) {
+	if (!__ionic_is_fw_running(idev, &fw_status)) {
 		fw_status_ready = false;
 	} else {
 		fw_generation = fw_status & IONIC_FW_STS_F_GENERATION;
-- 
2.17.1

[PATCH net-next 6/8] ionic: use timer_shutdown_sync

[Shannon Nelson]

When stopping the watchdog timer at remove time we should
be using the new timer_shutdown_sync to assure the timer
doesn't ever get rearmed.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 311d9f4ef0e2..fd2135c23862 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -396,7 +396,7 @@ static void ionic_remove(struct pci_dev *pdev)
 {
 	struct ionic *ionic = pci_get_drvdata(pdev);
 
-	del_timer_sync(&ionic->watchdog_timer);
+	timer_shutdown_sync(&ionic->watchdog_timer);
 
 	if (ionic->lif) {
 		/* prevent adminq cmds if already known as down */
-- 
2.17.1

[PATCH net-next 7/8] ionic: lif debugfs refresh on reset

[Shannon Nelson]

Remove and restore the lif's debugfs pointers on a reset,
and make sure to check for the dentry before removing it
in case an earlier reset failed to rebuild the lif.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c | 1 +
 drivers/net/ethernet/pensando/ionic/ionic_debugfs.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index fd2135c23862..60e64ef043af 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -437,6 +437,7 @@ static void ionic_reset_prepare(struct pci_dev *pdev)
 	ionic_txrx_free(lif);
 	ionic_lif_deinit(lif);
 	ionic_qcqs_free(lif);
+	ionic_debugfs_del_lif(lif);
 	mutex_unlock(&lif->queue_lock);
 
 	ionic_dev_teardown(ionic);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
index c58217027564..91327ef670c7 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
@@ -287,6 +287,9 @@ void ionic_debugfs_add_lif(struct ionic_lif *lif)
 
 void ionic_debugfs_del_lif(struct ionic_lif *lif)
 {
+	if (!lif->dentry)
+		return;
+
 	debugfs_remove_recursive(lif->dentry);
 	lif->dentry = NULL;
 }
-- 
2.17.1

[PATCH net-next 8/8] ionic: fill out pci error handlers

[Shannon Nelson]

Set up the pci_error_handlers error_detected and resume to be useful in
handling AER events.  If the error detected is pci_channel_io_frozen we
set up to do an FLR at the end of the AER handling - this tends to clear
things up well enough that traffic can continue.  Else, let the AER/PCI
machinery do what is needed for the less serious errors seen.

Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
---
 .../ethernet/pensando/ionic/ionic_bus_pci.c   | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 60e64ef043af..c49aa358e424 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -469,10 +469,35 @@ static void ionic_reset_done(struct pci_dev *pdev)
 		__func__, err ? "failed" : "done");
 }
 
+static pci_ers_result_t ionic_pci_error_detected(struct pci_dev *pdev,
+						 pci_channel_state_t error)
+{
+	if (error == pci_channel_io_frozen) {
+		ionic_reset_prepare(pdev);
+		return PCI_ERS_RESULT_NEED_RESET;
+	}
+
+	return PCI_ERS_RESULT_NONE;
+}
+
+static void ionic_pci_error_resume(struct pci_dev *pdev)
+{
+	struct ionic *ionic = pci_get_drvdata(pdev);
+	struct ionic_lif *lif = ionic->lif;
+
+	if (lif && test_bit(IONIC_LIF_F_FW_RESET, lif->state))
+		pci_reset_function_locked(pdev);
+}
+
 static const struct pci_error_handlers ionic_err_handler = {
 	/* FLR handling */
 	.reset_prepare      = ionic_reset_prepare,
 	.reset_done         = ionic_reset_done,
+
+	/* PCI bus error detected on this device */
+	.error_detected     = ionic_pci_error_detected,
+	.resume		    = ionic_pci_error_resume,
+
 };
 
 static struct pci_driver ionic_driver = {
-- 
2.17.1

Re: [PATCH net-next 0/8] ionic: updates to PCI error handling

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Mon, 11 Dec 2023 10:57:56 -0800 you wrote:
> These are improvements to our PCI error handling, including FLR and
> AER events.
> 
> Shannon Nelson (8):
>   ionic: pass opcode to devcmd_wait
>   ionic: keep filters across FLR
>   ionic: bypass firmware cmds when stuck in reset
>   ionic: prevent pci disable of already disabled device
>   ionic: no fw read when PCI reset failed
>   ionic: use timer_shutdown_sync
>   ionic: lif debugfs refresh on reset
>   ionic: fill out pci error handlers
> 
> [...]

Here is the summary with links:
  - [net-next,1/8] ionic: pass opcode to devcmd_wait
    https://git.kernel.org/netdev/net-next/c/24f110240c03
  - [net-next,2/8] ionic: keep filters across FLR
    https://git.kernel.org/netdev/net-next/c/45b84188a0a4
  - [net-next,3/8] ionic: bypass firmware cmds when stuck in reset
    https://git.kernel.org/netdev/net-next/c/ca5fdf9a7c5b
  - [net-next,4/8] ionic: prevent pci disable of already disabled device
    https://git.kernel.org/netdev/net-next/c/13943d6c8273
  - [net-next,5/8] ionic: no fw read when PCI reset failed
    https://git.kernel.org/netdev/net-next/c/219e183272b4
  - [net-next,6/8] ionic: use timer_shutdown_sync
    https://git.kernel.org/netdev/net-next/c/b0dbe358fbb4
  - [net-next,7/8] ionic: lif debugfs refresh on reset
    https://git.kernel.org/netdev/net-next/c/ce66172d3393
  - [net-next,8/8] ionic: fill out pci error handlers
    https://git.kernel.org/netdev/net-next/c/c3a910e1c47a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html