idpf_vc_event_task() shuts the transaction manager down before every
IDPF_HR_FUNC_RESET. That is only right when the reset was detected in
HW. Resets requested by idpf_tx_timeout() and idpf_idc_request_reset()
find the device still running, and idpf_init_hard_reset() now closes the
netdevs first, so the DISABLE_VPORT and DISABLE_QUEUES messages sent from
idpf_vport_stop() fail silently and the rings are freed while the queues
may still DMA into them. The device reset only follows later, and on a
VF not at all once IDPF_REMOVE_IN_PROG is set.
- Shut the manager down in idpf_service_task() when it detects a HW
reset, and only while IDPF_HR_FUNC_RESET is pending, as that reset is
what rebuilds it. This releases waiters the CP will never answer,
which may hold RTNL, before idpf_detach_and_close() needs it, and
leaves alone a manager the running reset has already rebuilt.
- Cap all transactions while a driver initiated reset runs at
IDPF_VC_XN_SHUTDOWN_TIMEOUT_MSEC, as idpf_shutdown() does, so an
unresponsive CP cannot hold RTNL for the 60 s default per message.
Restore the default once the new mailbox is up.
- Add IDPF_SHUTDOWN_IN_PROG for the shutdown only policies that were
inferred from the shortened timeout: latching the timeout to 0 after
the first expiry, and giving up on SW markers after the first missing
one. Resets restore the timeout, so a late latch could carry into the
next reset, and they should wait for every marker as a close does.
IDPF_REMOVE_IN_PROG cannot be used as remove may wait on a running
reset.
- Cancel serv_task in idpf_deinit_dflt_mbx() before the manager is
freed. Its libie_ctlq_xn_shutdown() can now overlap the one in
idpf_vc_core_deinit(), and the previous patch requires the manager to
outlive every caller.
- Cancel serv_task in idpf_remove() before vc_event_task, as
idpf_shutdown() does, so a reset cannot be queued after remove stopped
waiting for one.
A CP that answers slower than the cap can still have the disable
messages skipped, as before this change, so this narrows that window
rather than closing it.
Fixes: 2e281e1155fc ("idpf: detach and close netdevs while handling a reset")
Assisted-by: LLM
Signed-off-by: Emil Tantilov <[email protected]>
---
drivers/net/ethernet/intel/idpf/idpf.h | 5 ++-
drivers/net/ethernet/intel/idpf/idpf_lib.c | 42 +++++++++++--------
drivers/net/ethernet/intel/idpf/idpf_main.c | 4 ++
.../net/ethernet/intel/idpf/idpf_virtchnl.c | 15 ++++---
4 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf.h
b/drivers/net/ethernet/intel/idpf/idpf.h
index 98adaed4d3a2e..3d0216e89db80 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -86,6 +86,7 @@ enum idpf_state {
* @IDPF_HR_DRV_LOAD: Set on driver load for a clean HW
* @IDPF_HR_RESET_IN_PROG: Reset in progress
* @IDPF_REMOVE_IN_PROG: Driver remove or shutdown in progress
+ * @IDPF_SHUTDOWN_IN_PROG: Driver shutdown in progress
* @IDPF_MB_INTR_MODE: Mailbox in interrupt mode
* @IDPF_VC_CORE_INIT: virtchnl core has been init
* @IDPF_FLAGS_NBITS: Must be last
@@ -95,6 +96,7 @@ enum idpf_flags {
IDPF_HR_DRV_LOAD,
IDPF_HR_RESET_IN_PROG,
IDPF_REMOVE_IN_PROG,
+ IDPF_SHUTDOWN_IN_PROG,
IDPF_MB_INTR_MODE,
IDPF_VC_CORE_INIT,
IDPF_FLAGS_NBITS,
@@ -643,7 +645,8 @@ struct idpf_edt_caps_ilog2 {
* @arq: Receive control queue info
* @xnm: Xn transaction manager
* @vc_xn_timeout_ms: Upper bound for each Xn transaction timeout, shortened
- * on shutdown and set to 0 once a shortened one timed out
+ * on shutdown and reset teardown, and set to 0 once a
+ * shortened one timed out on shutdown
* @num_avail_msix: Available number of MSIX vectors
* @num_msix_entries: Number of entries in MSIX table
* @msix_entries: MSIX table
diff --git a/drivers/net/ethernet/intel/idpf/idpf_lib.c
b/drivers/net/ethernet/intel/idpf/idpf_lib.c
index 0cdbd81b13711..1f38d3e3a3f63 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_lib.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_lib.c
@@ -1431,13 +1431,22 @@ void idpf_service_task(struct work_struct *work)
adapter = container_of(work, struct idpf_adapter, serv_task.work);
if (idpf_is_reset_detected(adapter) &&
- !idpf_is_reset_in_prog(adapter) &&
!test_bit(IDPF_REMOVE_IN_PROG, adapter->flags)) {
- dev_info(&adapter->pdev->dev, "HW reset detected\n");
- set_bit(IDPF_HR_FUNC_RESET, adapter->flags);
- queue_delayed_work(adapter->vc_event_wq,
- &adapter->vc_event_task,
- msecs_to_jiffies(10));
+ bool new_reset = !idpf_is_reset_in_prog(adapter);
+
+ if (new_reset) {
+ dev_info(&adapter->pdev->dev, "HW reset detected\n");
+ set_bit(IDPF_HR_FUNC_RESET, adapter->flags);
+ }
+
+ /* Release the waiters on a mailbox that a reset will rebuild */
+ if (adapter->xnm && test_bit(IDPF_HR_FUNC_RESET,
adapter->flags))
+ libie_ctlq_xn_shutdown(adapter->xnm);
+
+ if (new_reset)
+ queue_delayed_work(adapter->vc_event_wq,
+ &adapter->vc_event_task,
+ msecs_to_jiffies(10));
}
queue_delayed_work(adapter->serv_wq, &adapter->serv_task,
@@ -1930,6 +1939,11 @@ static void idpf_init_hard_reset(struct idpf_adapter
*adapter)
struct device *dev = &adapter->pdev->dev;
int err;
+ /* As in idpf_shutdown(), bound the teardown if the CP is unresponsive
*/
+ if (test_bit(IDPF_HR_FUNC_RESET, adapter->flags))
+ WRITE_ONCE(adapter->vc_xn_timeout_ms,
+ IDPF_VC_XN_SHUTDOWN_TIMEOUT_MSEC);
+
idpf_detach_and_close(adapter);
mutex_lock(&adapter->vport_ctrl_lock);
@@ -1968,6 +1982,8 @@ static void idpf_init_hard_reset(struct idpf_adapter
*adapter)
goto unlock_mutex;
}
+ WRITE_ONCE(adapter->vc_xn_timeout_ms, IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC);
+
/* Initialize the state machine, also allocate memory and request
* resources
*/
@@ -2009,18 +2025,10 @@ void idpf_vc_event_task(struct work_struct *work)
if (test_bit(IDPF_REMOVE_IN_PROG, adapter->flags))
return;
- if (test_bit(IDPF_HR_FUNC_RESET, adapter->flags))
- goto func_reset;
-
- if (test_bit(IDPF_HR_DRV_LOAD, adapter->flags))
- goto drv_load;
-
- return;
+ if (!test_bit(IDPF_HR_FUNC_RESET, adapter->flags) &&
+ !test_bit(IDPF_HR_DRV_LOAD, adapter->flags))
+ return;
-func_reset:
- if (adapter->xnm)
- libie_ctlq_xn_shutdown(adapter->xnm);
-drv_load:
set_bit(IDPF_HR_RESET_IN_PROG, adapter->flags);
idpf_init_hard_reset(adapter);
}
diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c
b/drivers/net/ethernet/intel/idpf/idpf_main.c
index 21be83dc82b1b..edbf55d4e490e 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_main.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
@@ -125,6 +125,9 @@ static void idpf_remove(struct pci_dev *pdev)
set_bit(IDPF_REMOVE_IN_PROG, adapter->flags);
+ /* serv_task can still queue vc_event_task until it is cancelled */
+ cancel_delayed_work_sync(&adapter->serv_task);
+
/* Wait until vc_event_task is done to consider if any hard reset is
* in progress else we may go ahead and release the resources but the
* thread doing the hard reset might continue the init path and
@@ -198,6 +201,7 @@ static void idpf_shutdown(struct pci_dev *pdev)
* destroy the vports while the mailbox is still up.
*/
set_bit(IDPF_REMOVE_IN_PROG, adapter->flags);
+ set_bit(IDPF_SHUTDOWN_IN_PROG, adapter->flags);
cancel_delayed_work_sync(&adapter->serv_task);
cancel_delayed_work_sync(&adapter->vc_event_task);
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index eca954783454f..f45cdd227d482 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -196,8 +196,8 @@ static void idpf_prepare_ptp_mb_msg(struct idpf_adapter
*adapter, u32 op,
* unmap and release the buffer.
*
* The transaction timeout is capped at adapter->vc_xn_timeout_ms. Once a
- * synchronous transaction with a shortened timeout times out, later ones
- * fail at once instead of waiting again.
+ * synchronous transaction with a shortened timeout times out on shutdown,
+ * later ones fail at once instead of waiting again.
*
* Return: 0 if the request was successful, -%EBUSY if reset is detected
* or Tx control queue is full, -%ETIMEDOUT if a shortened transaction
@@ -236,7 +236,9 @@ int idpf_send_mb_msg(struct idpf_adapter *adapter,
idpf_mb_clean(xn_params->ctlq, false);
err = libie_ctlq_xn_send(xn_params);
- if (err == -ETIMEDOUT && timeout_ms < IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC)
+ /* Resets restore the timeout, so only shutdown can latch it */
+ if (err == -ETIMEDOUT && timeout_ms < IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC &&
+ test_bit(IDPF_SHUTDOWN_IN_PROG, adapter->flags))
WRITE_ONCE(adapter->vc_xn_timeout_ms, 0);
return err;
@@ -392,8 +394,7 @@ static int idpf_wait_for_marker_event_set(const struct
idpf_queue_set *qs)
bool wait_all;
/* On shutdown, once one marker is missing, don't wait for the rest */
- wait_all = READ_ONCE(qs->adapter->vc_xn_timeout_ms) >=
- IDPF_VC_XN_DEFAULT_TIMEOUT_MSEC;
+ wait_all = !test_bit(IDPF_SHUTDOWN_IN_PROG, qs->adapter->flags);
for (u32 i = 0; i < qs->num; i++) {
switch (qs->qs[i].type) {
@@ -3027,6 +3028,8 @@ int idpf_init_dflt_mbx(struct idpf_adapter *adapter)
void idpf_deinit_dflt_mbx(struct idpf_adapter *adapter)
{
idpf_mb_intr_rel_irq(adapter);
+ /* serv_task may be in libie_ctlq_xn_shutdown(), xnm must outlive it */
+ cancel_delayed_work_sync(&adapter->serv_task);
cancel_delayed_work_sync(&adapter->mbx_task);
if (adapter->xnm) {
@@ -3287,7 +3290,7 @@ void idpf_vc_core_deinit(struct idpf_adapter *adapter)
if (!test_bit(IDPF_VC_CORE_INIT, adapter->flags))
return;
- /* Avoid transaction timeouts when called during reset */
+ /* Driver initiated resets may end up here with mailbox still up */
remove_in_prog = test_bit(IDPF_REMOVE_IN_PROG, adapter->flags);
if (!remove_in_prog) {
libie_ctlq_xn_shutdown(adapter->xnm);
--
2.37.3
