[PATCH net-next 3/3] ionic: add devlink param for command timeout

["Nikhil P. Rao" <nikhil.rao@amd.com>]

From: Arun Kumar Prakash <ArunKumar.Prakash@amd.com>

Some device and admin commands can take firmware longer than the
driver allows to complete, for example while a PFC configuration change
flaps the links. The firmware is alive but slow, yet the timed-out
command and everything queued behind it fail.

Make the timeout configurable so it can be tuned to the firmware
behaviour seen in production.

Add a permanent devlink parameter, cmd_timeout (u8, seconds), stored in
device NVM via a new device attribute, on firmware that advertises the
new IONIC_DEV_CAP_CMD_TIMEOUT capability. A non-zero value replaces the
device command and admin queue timeouts, and the firmware heartbeat
check interval derived from them; 0 restores the default. A new value
takes effect on the next driver probe or device reset.

Back-date the initial heartbeat check time by the largest possible
interval rather than the watchdog period, so that a larger value does
not delay the first heartbeat sample after probe or reset.

Accept 0, or 5 to 255 seconds (the u8 range). Values below the 5 second
default are rejected, as the parameter is only meant to lengthen the
timeouts. On unresponsive firmware, a larger value also lets callers
block for longer: each device or admin queue command can wait up to this
value, other commands wait behind it, and RDMA verbs such as QP and CQ
create can block for several times this value. A large setting may need
a larger kernel.hung_task_timeout_secs.

Signed-off-by: Arun Kumar Prakash <ArunKumar.Prakash@amd.com>
Reviewed-by: Brett Creeley <brett.creeley@amd.com>
Co-developed-by: Nikhil P. Rao <nikhil.rao@amd.com>
Signed-off-by: Nikhil P. Rao <nikhil.rao@amd.com>
---
 Documentation/networking/devlink/ionic.rst         | 29 +++++++
 drivers/net/ethernet/pensando/ionic/ionic.h        | 12 +++
 drivers/net/ethernet/pensando/ionic/ionic_aux.c    |  2 +-
 .../net/ethernet/pensando/ionic/ionic_bus_pci.c    |  3 +
 drivers/net/ethernet/pensando/ionic/ionic_dev.c    | 81 +++++++++++++++++++-
 drivers/net/ethernet/pensando/ionic/ionic_dev.h    |  3 +
 .../net/ethernet/pensando/ionic/ionic_devlink.c    | 89 ++++++++++++++++++++++
 .../net/ethernet/pensando/ionic/ionic_devlink.h    |  2 +
 drivers/net/ethernet/pensando/ionic/ionic_if.h     |  8 ++
 drivers/net/ethernet/pensando/ionic/ionic_lif.c    |  3 +
 drivers/net/ethernet/pensando/ionic/ionic_main.c   |  3 +-
 11 files changed, 231 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/devlink/ionic.rst b/Documentation/networking/devlink/ionic.rst
index 48da9c92d5841..40b280bf15721 100644
--- a/Documentation/networking/devlink/ionic.rst
+++ b/Documentation/networking/devlink/ionic.rst
@@ -7,6 +7,35 @@ ionic devlink support
 This document describes the devlink features implemented by the ``ionic``
 device driver.
 
+Parameters
+==========
+
+The ``ionic`` driver implements the following driver-specific parameters.
+
+.. list-table:: Driver-specific parameters implemented
+   :widths: 5 5 5 85
+
+   * - Name
+     - Type
+     - Mode
+     - Description
+   * - ``cmd_timeout``
+     - u8
+     - permanent
+     - Command timeout in seconds, 0, or 5 (the default) to 255; values below
+       the default are rejected, as the parameter is for cases where the
+       default does not suffice. When set to a non-zero value, replaces the
+       default timeout for device and admin queue commands, and the RDMA admin
+       queue watchdog. A value of 0 restores the default. A larger value delays
+       the firmware heartbeat stall check, which runs at twice this interval.
+       On unresponsive firmware, a larger value also lets callers block for
+       longer: each device or admin queue command can wait up to this value,
+       other commands wait behind it, and RDMA verbs such as QP and CQ create
+       can block for several times this value. A large setting may need a
+       larger kernel.hung_task_timeout_secs. The value is stored in device NVM
+       and takes effect on the next driver probe or device reset. Only
+       available when the device firmware supports it.
+
 Info versions
 =============
 
diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
index 5c54a0ea10a55..db9800147b081 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic.h
@@ -64,8 +64,20 @@ struct ionic {
 	struct timer_list watchdog_timer;
 	int watchdog_period;
 	u8 cmd_timeout;
+	bool dl_params_registered;
 };
 
+static inline bool ionic_cmd_timeout_valid(u8 timeout)
+{
+	return !timeout || timeout >= IONIC_CMD_TIMEOUT_DEF;
+}
+
+static inline bool ionic_cmd_timeout_supported(struct ionic *ionic)
+{
+	return ionic->ident.dev.capabilities &
+	       cpu_to_le64(IONIC_DEV_CAP_CMD_TIMEOUT);
+}
+
 int ionic_adminq_post(struct ionic_lif *lif, struct ionic_admin_ctx *ctx);
 int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
 		      const int err, const bool do_msg);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_aux.c b/drivers/net/ethernet/pensando/ionic/ionic_aux.c
index 3b7cecc7f6dde..50312c1ae9eac 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_aux.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_aux.c
@@ -103,6 +103,6 @@ EXPORT_SYMBOL_NS(ionic_request_rdma_reset, "NET_IONIC");
 
 u8 ionic_cmd_timeout(struct ionic_lif *lif)
 {
-	return lif->ionic->cmd_timeout;
+	return READ_ONCE(lif->ionic->cmd_timeout);
 }
 EXPORT_SYMBOL_NS(ionic_cmd_timeout, "NET_IONIC");
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 74ba923a3f1b6..1ed7e6eb77652 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -281,6 +281,8 @@ static int ionic_setup_one(struct ionic *ionic)
 		goto err_out_teardown;
 	}
 
+	ionic_update_cmd_timeout(ionic);
+
 	/* Configure the port */
 	err = ionic_port_identify(ionic);
 	if (err) {
@@ -455,6 +457,7 @@ static void ionic_reset_prepare(struct pci_dev *pdev)
 	cancel_work_sync(&lif->deferred.work);
 
 	ionic_auxbus_unregister(ionic->lif);
+	ionic_devlink_params_unregister(ionic);
 	mutex_lock(&lif->queue_lock);
 	ionic_stop_queues_reconfig(lif);
 	ionic_txrx_free(lif);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
index f14fff4a74278..64f62d412ee2f 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
@@ -144,7 +144,7 @@ static int ionic_watchdog_init(struct ionic *ionic)
 
 	/* set times to ensure the first check will proceed */
 	atomic_long_set(&idev->last_check_time, jiffies - 2 * HZ);
-	idev->last_hb_time = jiffies - 2 * ionic->watchdog_period;
+	idev->last_hb_time = jiffies - 2 * U8_MAX * HZ;
 	/* init as ready, so no transition if the first check succeeds */
 	idev->last_fw_hb = 0;
 	idev->fw_hb_ready = true;
@@ -594,7 +594,8 @@ int ionic_heartbeat_check(struct ionic *ionic)
 	 */
 	last_check_time = idev->last_hb_time;
 	if (time_before(check_time,
-			last_check_time + ionic->cmd_timeout * 2 * HZ))
+			last_check_time +
+			READ_ONCE(ionic->cmd_timeout) * 2 * HZ))
 		return 0;
 
 	fw_hb = ioread32(&idev->dev_info_regs->fw_heartbeat);
@@ -689,6 +690,82 @@ void ionic_dev_cmd_reset(struct ionic_dev *idev)
 	ionic_dev_cmd_go(idev, &cmd);
 }
 
+static void ionic_dev_cmd_getattr(struct ionic_dev *idev, u8 attr)
+{
+	union ionic_dev_cmd cmd = {
+		.getattr.opcode = IONIC_CMD_GETATTR,
+		.getattr.attr = attr,
+	};
+
+	ionic_dev_cmd_go(idev, &cmd);
+}
+
+int ionic_dev_getattr_cmd_timeout(struct ionic *ionic, u8 *timeout)
+{
+	struct ionic_dev_getattr_comp comp = {};
+	int err;
+
+	mutex_lock(&ionic->dev_cmd_lock);
+	ionic_dev_cmd_getattr(&ionic->idev, IONIC_DEV_ATTR_CMD_TIMEOUT);
+
+	err = ionic_dev_cmd_wait_nomsg(ionic, ionic->cmd_timeout);
+	if (err) {
+		mutex_unlock(&ionic->dev_cmd_lock);
+		return err;
+	}
+
+	ionic_dev_cmd_comp(&ionic->idev, (union ionic_dev_cmd_comp *)&comp);
+	*timeout = comp.cmd_timeout;
+	mutex_unlock(&ionic->dev_cmd_lock);
+
+	return 0;
+}
+
+int ionic_dev_setattr_cmd_timeout(struct ionic *ionic, u8 timeout)
+{
+	union ionic_dev_cmd cmd = {
+		.setattr.opcode = IONIC_CMD_SETATTR,
+		.setattr.attr = IONIC_DEV_ATTR_CMD_TIMEOUT,
+		.setattr.cmd_timeout = timeout,
+	};
+	int err;
+
+	mutex_lock(&ionic->dev_cmd_lock);
+	ionic_dev_cmd_go(&ionic->idev, &cmd);
+	err = ionic_dev_cmd_wait(ionic, ionic->cmd_timeout);
+	mutex_unlock(&ionic->dev_cmd_lock);
+
+	return err;
+}
+
+void ionic_update_cmd_timeout(struct ionic *ionic)
+{
+	u8 timeout = IONIC_CMD_TIMEOUT_DEF;
+	int err;
+	u8 val;
+
+	if (!ionic_cmd_timeout_supported(ionic))
+		goto out;
+
+	err = ionic_dev_getattr_cmd_timeout(ionic, &val);
+	if (err) {
+		dev_warn(ionic->dev,
+			 "Cannot get cmd_timeout: %d, using default\n", err);
+		goto out;
+	}
+	if (!ionic_cmd_timeout_valid(val)) {
+		dev_warn(ionic->dev, "Ignoring out of range cmd_timeout %u\n",
+			 val);
+		goto out;
+	}
+	if (val)
+		timeout = val;
+out:
+	mutex_lock(&ionic->dev_cmd_lock);
+	WRITE_ONCE(ionic->cmd_timeout, timeout);
+	mutex_unlock(&ionic->dev_cmd_lock);
+}
+
 /* Port commands */
 void ionic_dev_cmd_port_identify(struct ionic_dev *idev)
 {
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index db90e39a1442d..06ca7584ee369 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -340,6 +340,9 @@ void ionic_dev_cmd_comp(struct ionic_dev *idev, union ionic_dev_cmd_comp *comp);
 void ionic_dev_cmd_identify(struct ionic_dev *idev, u8 ver);
 void ionic_dev_cmd_init(struct ionic_dev *idev);
 void ionic_dev_cmd_reset(struct ionic_dev *idev);
+int ionic_dev_getattr_cmd_timeout(struct ionic *ionic, u8 *timeout);
+int ionic_dev_setattr_cmd_timeout(struct ionic *ionic, u8 timeout);
+void ionic_update_cmd_timeout(struct ionic *ionic);
 
 void ionic_dev_cmd_port_identify(struct ionic_dev *idev);
 void ionic_dev_cmd_port_init(struct ionic_dev *idev);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
index 4ec66a6be0738..d775fa5bb43d7 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
@@ -51,6 +51,55 @@ static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 	return err;
 }
 
+enum ionic_devlink_param_id {
+	IONIC_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
+	IONIC_DEVLINK_PARAM_ID_CMD_TIMEOUT,
+};
+
+static int ionic_dl_cmd_timeout_get(struct devlink *dl, u32 id,
+				    struct devlink_param_gset_ctx *ctx,
+				    struct netlink_ext_ack *extack)
+{
+	struct ionic *ionic = devlink_priv(dl);
+
+	return ionic_dev_getattr_cmd_timeout(ionic, &ctx->val.vu8);
+}
+
+static int ionic_dl_cmd_timeout_set(struct devlink *dl, u32 id,
+				    struct devlink_param_gset_ctx *ctx,
+				    struct netlink_ext_ack *extack)
+{
+	struct ionic *ionic = devlink_priv(dl);
+
+	if (test_bit(IONIC_LIF_F_FW_RESET, ionic->lif->state))
+		return -EBUSY;
+
+	return ionic_dev_setattr_cmd_timeout(ionic, ctx->val.vu8);
+}
+
+static int ionic_dl_cmd_timeout_validate(struct devlink *dl, u32 id,
+					 union devlink_param_value *val,
+					 struct netlink_ext_ack *extack)
+{
+	if (!ionic_cmd_timeout_valid(val->vu8)) {
+		NL_SET_ERR_MSG_FMT_MOD(extack,
+				       "cmd_timeout must be 0 or at least %d seconds",
+				       IONIC_CMD_TIMEOUT_DEF);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct devlink_param ionic_dl_params[] = {
+	DEVLINK_PARAM_DRIVER(IONIC_DEVLINK_PARAM_ID_CMD_TIMEOUT,
+			     "cmd_timeout", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     ionic_dl_cmd_timeout_get,
+			     ionic_dl_cmd_timeout_set,
+			     ionic_dl_cmd_timeout_validate),
+};
+
 static const struct devlink_ops ionic_dl_ops = {
 	.info_get	= ionic_dl_info_get,
 	.flash_update	= ionic_dl_flash_update,
@@ -74,6 +123,43 @@ void ionic_devlink_free(struct ionic *ionic)
 	devlink_free(dl);
 }
 
+void ionic_devlink_params_register(struct ionic *ionic)
+{
+	struct devlink *dl = priv_to_devlink(ionic);
+	int err;
+
+	if (!ionic_cmd_timeout_supported(ionic))
+		return;
+
+	devl_lock(dl);
+	if (ionic->dl_params_registered)
+		goto out;
+
+	err = devl_params_register(dl, ionic_dl_params,
+				   ARRAY_SIZE(ionic_dl_params));
+	if (err) {
+		dev_err(ionic->dev, "devl_params_register failed: %d\n", err);
+		goto out;
+	}
+
+	ionic->dl_params_registered = true;
+out:
+	devl_unlock(dl);
+}
+
+void ionic_devlink_params_unregister(struct ionic *ionic)
+{
+	struct devlink *dl = priv_to_devlink(ionic);
+
+	devl_lock(dl);
+	if (ionic->dl_params_registered) {
+		devl_params_unregister(dl, ionic_dl_params,
+				       ARRAY_SIZE(ionic_dl_params));
+		ionic->dl_params_registered = false;
+	}
+	devl_unlock(dl);
+}
+
 int ionic_devlink_register(struct ionic *ionic)
 {
 	struct devlink *dl = priv_to_devlink(ionic);
@@ -88,6 +174,8 @@ int ionic_devlink_register(struct ionic *ionic)
 		return err;
 	}
 
+	ionic_devlink_params_register(ionic);
+
 	SET_NETDEV_DEVLINK_PORT(ionic->lif->netdev, &ionic->dl_port);
 	devlink_register(dl);
 	return 0;
@@ -98,5 +186,6 @@ void ionic_devlink_unregister(struct ionic *ionic)
 	struct devlink *dl = priv_to_devlink(ionic);
 
 	devlink_unregister(dl);
+	ionic_devlink_params_unregister(ionic);
 	devlink_port_unregister(&ionic->dl_port);
 }
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.h b/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
index 0a77e8e810c5d..02e1ea559a0e6 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
@@ -13,5 +13,7 @@ struct ionic *ionic_devlink_alloc(struct device *dev);
 void ionic_devlink_free(struct ionic *ionic);
 int ionic_devlink_register(struct ionic *ionic);
 void ionic_devlink_unregister(struct ionic *ionic);
+void ionic_devlink_params_register(struct ionic *ionic);
+void ionic_devlink_params_unregister(struct ionic *ionic);
 
 #endif /* _IONIC_DEVLINK_H_ */
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_if.h b/drivers/net/ethernet/pensando/ionic/ionic_if.h
index e911fad545d67..311d205c5b4b7 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_if.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_if.h
@@ -275,11 +275,13 @@ union ionic_drv_identity {
  * @IONIC_DEV_CAP_VF_CTRL:     Device supports VF ctrl operations
  * @IONIC_DEV_CAP_DISC_CMB:    Device supports CMB discovery operations
  * @IONIC_DEV_CAP_EXTRA_STATS: Device supports extra stats schema
+ * @IONIC_DEV_CAP_CMD_TIMEOUT: Device supports the command timeout attribute
  */
 enum ionic_dev_capability {
 	IONIC_DEV_CAP_VF_CTRL        = BIT(0),
 	IONIC_DEV_CAP_DISC_CMB       = BIT(1),
 	IONIC_DEV_CAP_EXTRA_STATS    = BIT(4),
+	IONIC_DEV_CAP_CMD_TIMEOUT    = BIT(5),
 };
 
 /**
@@ -1778,11 +1780,13 @@ enum ionic_dev_state {
  * @IONIC_DEV_ATTR_STATE:     Device state attribute
  * @IONIC_DEV_ATTR_NAME:      Device name attribute
  * @IONIC_DEV_ATTR_FEATURES:  Device feature attributes
+ * @IONIC_DEV_ATTR_CMD_TIMEOUT: Command timeout
  */
 enum ionic_dev_attr {
 	IONIC_DEV_ATTR_STATE    = 0,
 	IONIC_DEV_ATTR_NAME     = 1,
 	IONIC_DEV_ATTR_FEATURES = 2,
+	IONIC_DEV_ATTR_CMD_TIMEOUT = 3,
 };
 
 /**
@@ -1793,6 +1797,7 @@ enum ionic_dev_attr {
  * @state:      Device state (enum ionic_dev_state)
  * @name:       The bus info, e.g. PCI slot-device-function, 0 terminated
  * @features:   Device features
+ * @cmd_timeout: Command timeout in seconds, 0 for the default
  * @rsvd2:      reserved byte(s)
  */
 struct ionic_dev_setattr_cmd {
@@ -1803,6 +1808,7 @@ struct ionic_dev_setattr_cmd {
 		u8      state;
 		char    name[IONIC_IFNAMSIZ];
 		__le64  features;
+		u8      cmd_timeout;
 		u8      rsvd2[60];
 	} __packed;
 };
@@ -1842,6 +1848,7 @@ struct ionic_dev_getattr_cmd {
  * @status:     Status of the command (enum ionic_status_code)
  * @rsvd:       reserved byte(s)
  * @features:   Device features
+ * @cmd_timeout: Command timeout in seconds, 0 for the default
  * @rsvd2:      reserved byte(s)
  * @color:      Color bit
  */
@@ -1850,6 +1857,7 @@ struct ionic_dev_getattr_comp {
 	u8     rsvd[3];
 	union {
 		__le64  features;
+		u8      cmd_timeout;
 		u8      rsvd2[11];
 	} __packed;
 	u8     color;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index b0868ab139dd5..a89f9c72eb39d 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -3398,6 +3398,7 @@ static void ionic_lif_handle_fw_down(struct ionic_lif *lif)
 	netif_device_detach(lif->netdev);
 
 	ionic_auxbus_unregister(ionic->lif);
+	ionic_devlink_params_unregister(ionic);
 	mutex_lock(&lif->queue_lock);
 	if (test_bit(IONIC_LIF_F_UP, lif->state)) {
 		dev_info(ionic->dev, "Surprise FW stop, stopping queues\n");
@@ -3460,6 +3461,7 @@ int ionic_restart_lif(struct ionic_lif *lif)
 	netif_device_attach(lif->netdev);
 	ionic_queue_doorbell_check(ionic, IONIC_NAPI_DEADLINE);
 
+	ionic_devlink_params_register(ionic);
 	ionic_auxbus_register(ionic->lif);
 
 	return 0;
@@ -3495,6 +3497,7 @@ static void ionic_lif_handle_fw_up(struct ionic_lif *lif)
 	err = ionic_identify(ionic);
 	if (err)
 		goto err_out;
+	ionic_update_cmd_timeout(ionic);
 	err = ionic_port_identify(ionic);
 	if (err)
 		goto err_out;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 16df13f23c90d..a6a5206d6605b 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -435,7 +435,8 @@ int ionic_adminq_wait(struct ionic_lif *lif, struct ionic_admin_ctx *ctx,
 	}
 
 	time_start = jiffies;
-	time_limit = time_start + HZ * (ulong)lif->ionic->cmd_timeout;
+	time_limit = time_start +
+		     HZ * (ulong)READ_ONCE(lif->ionic->cmd_timeout);
 	do {
 		remaining = wait_for_completion_timeout(&ctx->work,
 							IONIC_ADMINQ_TIME_SLICE);

-- 
2.43.0