[PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Vasundhara Volam]

This patchset adds support for a "enable_hot_fw_reset" generic devlink
parameter and use it in bnxt_en driver.

Also, firmware spec. is updated to 1.10.1.40.

Vasundhara Volam (4):
  devlink: Add new "enable_hot_fw_reset" generic device parameter.
  bnxt_en: Update firmware spec. to 1.10.1.40.
  bnxt_en: Use enable_hot_fw_reset generic devlink parameter
  bnxt_en: Check if hot_fw_reset is allowed before doing ETHTOOL_RESET

 Documentation/networking/devlink/bnxt.rst          |  6 ++
 .../networking/devlink/devlink-params.rst          |  6 ++
 drivers/net/ethernet/broadcom/bnxt/bnxt.c          | 28 +++++++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h          |  2 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c  | 61 +++++++++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h  |  1 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c  | 17 +++---
 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h      | 64 +++++++++++++---------
 include/net/devlink.h                              |  4 ++
 net/core/devlink.c                                 |  5 ++
 10 files changed, 158 insertions(+), 36 deletions(-)

-- 
1.8.3.1

[PATCH net-next 1/4] devlink: Add new "enable_hot_fw_reset" generic device parameter.

[Vasundhara Volam]

Add a new "enable_hot_fw_reset" generic device parameter to enable
or disable hot firmware reset capability on the device.

Cc: Jiri Pirko <jiri@mellanox.com>
Cc: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
---
 Documentation/networking/devlink/devlink-params.rst | 6 ++++++
 include/net/devlink.h                               | 4 ++++
 net/core/devlink.c                                  | 5 +++++
 3 files changed, 15 insertions(+)

diff --git a/Documentation/networking/devlink/devlink-params.rst b/Documentation/networking/devlink/devlink-params.rst
index d075fd0..ba75437 100644
--- a/Documentation/networking/devlink/devlink-params.rst
+++ b/Documentation/networking/devlink/devlink-params.rst
@@ -108,3 +108,9 @@ own name.
    * - ``region_snapshot_enable``
      - Boolean
      - Enable capture of ``devlink-region`` snapshots.
+   * - ``enable_hot_fw_reset``
+     - Boolean
+     - Enable hot firmware reset on the device. When enabled, device supports
+       hot firmware reset capability which will be able to reset the running
+       firmware or upgrade/downgrade of flashed firmware without a driver
+       reload.
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 8ffc1b5c..57377bf 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -406,6 +406,7 @@ enum devlink_param_generic_id {
 	DEVLINK_PARAM_GENERIC_ID_FW_LOAD_POLICY,
 	DEVLINK_PARAM_GENERIC_ID_RESET_DEV_ON_DRV_PROBE,
 	DEVLINK_PARAM_GENERIC_ID_ENABLE_ROCE,
+	DEVLINK_PARAM_GENERIC_ID_ENABLE_HOT_FW_RESET,
 
 	/* add new param generic ids above here*/
 	__DEVLINK_PARAM_GENERIC_ID_MAX,
@@ -443,6 +444,9 @@ enum devlink_param_generic_id {
 #define DEVLINK_PARAM_GENERIC_ENABLE_ROCE_NAME "enable_roce"
 #define DEVLINK_PARAM_GENERIC_ENABLE_ROCE_TYPE DEVLINK_PARAM_TYPE_BOOL
 
+#define DEVLINK_PARAM_GENERIC_ENABLE_HOT_FW_RESET_NAME "enable_hot_fw_reset"
+#define DEVLINK_PARAM_GENERIC_ENABLE_HOT_FW_RESET_TYPE DEVLINK_PARAM_TYPE_BOOL
+
 #define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate)	\
 {									\
 	.id = DEVLINK_PARAM_GENERIC_ID_##_id,				\
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7b76e5f..32fb8e6 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3011,6 +3011,11 @@ static int devlink_nl_cmd_flash_update(struct sk_buff *skb,
 		.name = DEVLINK_PARAM_GENERIC_ENABLE_ROCE_NAME,
 		.type = DEVLINK_PARAM_GENERIC_ENABLE_ROCE_TYPE,
 	},
+	{
+		.id = DEVLINK_PARAM_GENERIC_ID_ENABLE_HOT_FW_RESET,
+		.name = DEVLINK_PARAM_GENERIC_ENABLE_HOT_FW_RESET_NAME,
+		.type = DEVLINK_PARAM_GENERIC_ENABLE_HOT_FW_RESET_TYPE,
+	},
 };
 
 static int devlink_param_generic_verify(const struct devlink_param *param)
-- 
1.8.3.1

[PATCH net-next 2/4] bnxt_en: Update firmware spec. to 1.10.1.40.

[Vasundhara Volam]

Major changes are to add additional flags to configure hot firmware
reset.

Cc: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h | 64 ++++++++++++++++-----------
 1 file changed, 37 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
index 7e9235c..0a6e60e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_hsi.h
@@ -367,6 +367,8 @@ struct cmd_nums {
 	#define HWRM_TF_EXT_EM_OP                         0x2ddUL
 	#define HWRM_TF_EXT_EM_CFG                        0x2deUL
 	#define HWRM_TF_EXT_EM_QCFG                       0x2dfUL
+	#define HWRM_TF_EM_INSERT                         0x2e0UL
+	#define HWRM_TF_EM_DELETE                         0x2e1UL
 	#define HWRM_TF_TCAM_SET                          0x2eeUL
 	#define HWRM_TF_TCAM_GET                          0x2efUL
 	#define HWRM_TF_TCAM_MOVE                         0x2f0UL
@@ -391,6 +393,7 @@ struct cmd_nums {
 	#define HWRM_DBG_QCAPS                            0xff20UL
 	#define HWRM_DBG_QCFG                             0xff21UL
 	#define HWRM_DBG_CRASHDUMP_MEDIUM_CFG             0xff22UL
+	#define HWRM_NVM_REQ_ARBITRATION                  0xffedUL
 	#define HWRM_NVM_FACTORY_DEFAULTS                 0xffeeUL
 	#define HWRM_NVM_VALIDATE_OPTION                  0xffefUL
 	#define HWRM_NVM_FLUSH                            0xfff0UL
@@ -464,8 +467,8 @@ struct hwrm_err_output {
 #define HWRM_VERSION_MAJOR 1
 #define HWRM_VERSION_MINOR 10
 #define HWRM_VERSION_UPDATE 1
-#define HWRM_VERSION_RSVD 33
-#define HWRM_VERSION_STR "1.10.1.33"
+#define HWRM_VERSION_RSVD 40
+#define HWRM_VERSION_STR "1.10.1.40"
 
 /* hwrm_ver_get_input (size:192b/24B) */
 struct hwrm_ver_get_input {
@@ -1192,6 +1195,7 @@ struct hwrm_func_qcaps_output {
 	#define FUNC_QCAPS_RESP_FLAGS_EXT_ECN_MARK_SUPPORTED         0x1UL
 	#define FUNC_QCAPS_RESP_FLAGS_EXT_ECN_STATS_SUPPORTED        0x2UL
 	#define FUNC_QCAPS_RESP_FLAGS_EXT_EXT_HW_STATS_SUPPORTED     0x4UL
+	#define FUNC_QCAPS_RESP_FLAGS_EXT_HOT_RESET_IF_SUPPORT       0x8UL
 	u8	unused_1[3];
 	u8	valid;
 };
@@ -1226,6 +1230,7 @@ struct hwrm_func_qcfg_output {
 	#define FUNC_QCFG_RESP_FLAGS_TRUSTED_VF                   0x40UL
 	#define FUNC_QCFG_RESP_FLAGS_SECURE_MODE_ENABLED          0x80UL
 	#define FUNC_QCFG_RESP_FLAGS_PREBOOT_LEGACY_L2_RINGS      0x100UL
+	#define FUNC_QCFG_RESP_FLAGS_HOT_RESET_ALLOWED            0x200UL
 	u8	mac_address[6];
 	__le16	pci_id;
 	__le16	alloc_rsscos_ctx;
@@ -1352,30 +1357,32 @@ struct hwrm_func_cfg_input {
 	#define FUNC_CFG_REQ_FLAGS_NQ_ASSETS_TEST                 0x800000UL
 	#define FUNC_CFG_REQ_FLAGS_TRUSTED_VF_DISABLE             0x1000000UL
 	#define FUNC_CFG_REQ_FLAGS_PREBOOT_LEGACY_L2_RINGS        0x2000000UL
+	#define FUNC_CFG_REQ_FLAGS_HOT_RESET_IF_EN_DIS            0x4000000UL
 	__le32	enables;
-	#define FUNC_CFG_REQ_ENABLES_MTU                     0x1UL
-	#define FUNC_CFG_REQ_ENABLES_MRU                     0x2UL
-	#define FUNC_CFG_REQ_ENABLES_NUM_RSSCOS_CTXS         0x4UL
-	#define FUNC_CFG_REQ_ENABLES_NUM_CMPL_RINGS          0x8UL
-	#define FUNC_CFG_REQ_ENABLES_NUM_TX_RINGS            0x10UL
-	#define FUNC_CFG_REQ_ENABLES_NUM_RX_RINGS            0x20UL
-	#define FUNC_CFG_REQ_ENABLES_NUM_L2_CTXS             0x40UL
-	#define FUNC_CFG_REQ_ENABLES_NUM_VNICS               0x80UL
-	#define FUNC_CFG_REQ_ENABLES_NUM_STAT_CTXS           0x100UL
-	#define FUNC_CFG_REQ_ENABLES_DFLT_MAC_ADDR           0x200UL
-	#define FUNC_CFG_REQ_ENABLES_DFLT_VLAN               0x400UL
-	#define FUNC_CFG_REQ_ENABLES_DFLT_IP_ADDR            0x800UL
-	#define FUNC_CFG_REQ_ENABLES_MIN_BW                  0x1000UL
-	#define FUNC_CFG_REQ_ENABLES_MAX_BW                  0x2000UL
-	#define FUNC_CFG_REQ_ENABLES_ASYNC_EVENT_CR          0x4000UL
-	#define FUNC_CFG_REQ_ENABLES_VLAN_ANTISPOOF_MODE     0x8000UL
-	#define FUNC_CFG_REQ_ENABLES_ALLOWED_VLAN_PRIS       0x10000UL
-	#define FUNC_CFG_REQ_ENABLES_EVB_MODE                0x20000UL
-	#define FUNC_CFG_REQ_ENABLES_NUM_MCAST_FILTERS       0x40000UL
-	#define FUNC_CFG_REQ_ENABLES_NUM_HW_RING_GRPS        0x80000UL
-	#define FUNC_CFG_REQ_ENABLES_CACHE_LINESIZE          0x100000UL
-	#define FUNC_CFG_REQ_ENABLES_NUM_MSIX                0x200000UL
-	#define FUNC_CFG_REQ_ENABLES_ADMIN_LINK_STATE        0x400000UL
+	#define FUNC_CFG_REQ_ENABLES_MTU                      0x1UL
+	#define FUNC_CFG_REQ_ENABLES_MRU                      0x2UL
+	#define FUNC_CFG_REQ_ENABLES_NUM_RSSCOS_CTXS          0x4UL
+	#define FUNC_CFG_REQ_ENABLES_NUM_CMPL_RINGS           0x8UL
+	#define FUNC_CFG_REQ_ENABLES_NUM_TX_RINGS             0x10UL
+	#define FUNC_CFG_REQ_ENABLES_NUM_RX_RINGS             0x20UL
+	#define FUNC_CFG_REQ_ENABLES_NUM_L2_CTXS              0x40UL
+	#define FUNC_CFG_REQ_ENABLES_NUM_VNICS                0x80UL
+	#define FUNC_CFG_REQ_ENABLES_NUM_STAT_CTXS            0x100UL
+	#define FUNC_CFG_REQ_ENABLES_DFLT_MAC_ADDR            0x200UL
+	#define FUNC_CFG_REQ_ENABLES_DFLT_VLAN                0x400UL
+	#define FUNC_CFG_REQ_ENABLES_DFLT_IP_ADDR             0x800UL
+	#define FUNC_CFG_REQ_ENABLES_MIN_BW                   0x1000UL
+	#define FUNC_CFG_REQ_ENABLES_MAX_BW                   0x2000UL
+	#define FUNC_CFG_REQ_ENABLES_ASYNC_EVENT_CR           0x4000UL
+	#define FUNC_CFG_REQ_ENABLES_VLAN_ANTISPOOF_MODE      0x8000UL
+	#define FUNC_CFG_REQ_ENABLES_ALLOWED_VLAN_PRIS        0x10000UL
+	#define FUNC_CFG_REQ_ENABLES_EVB_MODE                 0x20000UL
+	#define FUNC_CFG_REQ_ENABLES_NUM_MCAST_FILTERS        0x40000UL
+	#define FUNC_CFG_REQ_ENABLES_NUM_HW_RING_GRPS         0x80000UL
+	#define FUNC_CFG_REQ_ENABLES_CACHE_LINESIZE           0x100000UL
+	#define FUNC_CFG_REQ_ENABLES_NUM_MSIX                 0x200000UL
+	#define FUNC_CFG_REQ_ENABLES_ADMIN_LINK_STATE         0x400000UL
+	#define FUNC_CFG_REQ_ENABLES_HOT_RESET_IF_SUPPORT     0x800000UL
 	__le16	mtu;
 	__le16	mru;
 	__le16	num_rsscos_ctxs;
@@ -7620,7 +7627,8 @@ struct hwrm_dbg_ring_info_get_input {
 	#define DBG_RING_INFO_GET_REQ_RING_TYPE_L2_CMPL 0x0UL
 	#define DBG_RING_INFO_GET_REQ_RING_TYPE_TX      0x1UL
 	#define DBG_RING_INFO_GET_REQ_RING_TYPE_RX      0x2UL
-	#define DBG_RING_INFO_GET_REQ_RING_TYPE_LAST   DBG_RING_INFO_GET_REQ_RING_TYPE_RX
+	#define DBG_RING_INFO_GET_REQ_RING_TYPE_NQ      0x3UL
+	#define DBG_RING_INFO_GET_REQ_RING_TYPE_LAST   DBG_RING_INFO_GET_REQ_RING_TYPE_NQ
 	u8	unused_0[3];
 	__le32	fw_ring_id;
 };
@@ -7633,7 +7641,8 @@ struct hwrm_dbg_ring_info_get_output {
 	__le16	resp_len;
 	__le32	producer_index;
 	__le32	consumer_index;
-	u8	unused_0[7];
+	__le32	cag_vector_ctrl;
+	u8	unused_0[3];
 	u8	valid;
 };
 
@@ -7922,6 +7931,7 @@ struct hwrm_nvm_install_update_input {
 	#define NVM_INSTALL_UPDATE_REQ_FLAGS_ERASE_UNUSED_SPACE     0x1UL
 	#define NVM_INSTALL_UPDATE_REQ_FLAGS_REMOVE_UNUSED_PKG      0x2UL
 	#define NVM_INSTALL_UPDATE_REQ_FLAGS_ALLOWED_TO_DEFRAG      0x4UL
+	#define NVM_INSTALL_UPDATE_REQ_FLAGS_VERIFY_ONLY            0x8UL
 	u8	unused_0[2];
 };
 
-- 
1.8.3.1

[PATCH net-next 3/4] bnxt_en: Use enable_hot_fw_reset generic devlink parameter

[Vasundhara Volam]

enable_hot_fw_reset parameter supports both permanent and runtime
configuration modes. This parameter enables or disables the hot
firmware reset capability of the device which either resets the
currently running firmware or upgrades/downgrades flashed
firmware.

For the runtime parameter to be true, all the loaded host driver
interfaces must support hot firmware reset. If any driver's
interface capability is set to false by the user, the device cannot
support hot firmware reset. To re-enable the capability, all the
driver interfaces have to set the value to true.

Cc: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
---
 Documentation/networking/devlink/bnxt.rst         |  6 +++
 drivers/net/ethernet/broadcom/bnxt/bnxt.c         | 28 ++++++++++-
 drivers/net/ethernet/broadcom/bnxt/bnxt.h         |  2 +
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c | 61 +++++++++++++++++++++++
 drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h |  1 +
 5 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/devlink/bnxt.rst b/Documentation/networking/devlink/bnxt.rst
index 3dfd84c..69393d3 100644
--- a/Documentation/networking/devlink/bnxt.rst
+++ b/Documentation/networking/devlink/bnxt.rst
@@ -14,6 +14,7 @@ Parameters
 
    * - Name
      - Mode
+     - Description
    * - ``enable_sriov``
      - Permanent
    * - ``ignore_ari``
@@ -22,6 +23,11 @@ Parameters
      - Permanent
    * - ``msix_vec_per_pf_min``
      - Permanent
+   * - ``enable_hot_fw_reset``
+     - Permanent, runtime
+     - For the runtime parameter to be true, all the loaded host driver
+       interfaces must support hot firmware reset. To re-enable the
+       capability, all the driver interfaces have to set the value to true.
 
 The ``bnxt`` driver also implements the following driver-specific
 parameters.
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index f86b621..535fe8f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6955,7 +6955,7 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
 	struct hwrm_func_qcaps_input req = {0};
 	struct hwrm_func_qcaps_output *resp = bp->hwrm_cmd_resp_addr;
 	struct bnxt_hw_resc *hw_resc = &bp->hw_resc;
-	u32 flags;
+	u32 flags, flags_ext;
 
 	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_FUNC_QCAPS, -1, -1);
 	req.fid = cpu_to_le16(0xffff);
@@ -6985,6 +6985,10 @@ static int __bnxt_hwrm_func_qcaps(struct bnxt *bp)
 	if (flags & FUNC_QCAPS_RESP_FLAGS_PUSH_MODE_SUPPORTED)
 		bp->tx_push_thresh = BNXT_TX_PUSH_THRESH;
 
+	flags_ext = le32_to_cpu(resp->flags_ext);
+	if (flags_ext & FUNC_QCAPS_RESP_FLAGS_EXT_HOT_RESET_IF_SUPPORT)
+		bp->fw_cap |= BNXT_FW_CAP_HOT_RESET_IF_SUPPORT;
+
 	hw_resc->max_rsscos_ctxs = le16_to_cpu(resp->max_rsscos_ctx);
 	hw_resc->max_cp_rings = le16_to_cpu(resp->max_cmpl_rings);
 	hw_resc->max_tx_rings = le16_to_cpu(resp->max_tx_rings);
@@ -8773,6 +8777,28 @@ static int bnxt_hwrm_shutdown_link(struct bnxt *bp)
 
 static int bnxt_fw_init_one(struct bnxt *bp);
 
+int bnxt_hwrm_get_hot_reset(struct bnxt *bp, bool *hot_reset_allowed)
+{
+	struct hwrm_func_qcfg_output *resp = bp->hwrm_cmd_resp_addr;
+	struct hwrm_func_qcfg_input req = {0};
+	int rc;
+
+	if (!(bp->fw_cap & BNXT_FW_CAP_HOT_RESET_IF_SUPPORT)) {
+		*hot_reset_allowed = !!(bp->fw_cap & BNXT_FW_CAP_HOT_RESET);
+		return 0;
+	}
+
+	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_FUNC_QCFG, -1, -1);
+	req.fid = cpu_to_le16(0xffff);
+	mutex_lock(&bp->hwrm_cmd_lock);
+	rc = _hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+	if (!rc)
+		*hot_reset_allowed = !!(le16_to_cpu(resp->flags) &
+					FUNC_QCFG_RESP_FLAGS_HOT_RESET_ALLOWED);
+	mutex_unlock(&bp->hwrm_cmd_lock);
+	return rc;
+}
+
 static int bnxt_hwrm_if_change(struct bnxt *bp, bool up)
 {
 	struct hwrm_func_drv_if_change_output *resp = bp->hwrm_cmd_resp_addr;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index c04ac4a..fd6592e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1710,6 +1710,7 @@ struct bnxt {
 	#define BNXT_FW_CAP_ERR_RECOVER_RELOAD		0x00100000
 	#define BNXT_FW_CAP_HOT_RESET			0x00200000
 	#define BNXT_FW_CAP_SHARED_PORT_CFG		0x00400000
+	#define BNXT_FW_CAP_HOT_RESET_IF_SUPPORT	0x08000000
 
 #define BNXT_NEW_RM(bp)		((bp)->fw_cap & BNXT_FW_CAP_NEW_RM)
 	u32			hwrm_spec_code;
@@ -2062,5 +2063,6 @@ int bnxt_get_port_parent_id(struct net_device *dev,
 			    struct netdev_phys_item_id *ppid);
 void bnxt_dim_work(struct work_struct *work);
 int bnxt_hwrm_set_ring_coal(struct bnxt *bp, struct bnxt_napi *bnapi);
+int bnxt_hwrm_get_hot_reset(struct bnxt *bp, bool *hot_reset_allowed);
 
 #endif
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index a812beb..61bf630 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -314,6 +314,8 @@ enum bnxt_dl_param_id {
 	 NVM_OFF_MSIX_VEC_PER_PF_MIN, BNXT_NVM_SHARED_CFG, 7, 4},
 	{BNXT_DEVLINK_PARAM_ID_GRE_VER_CHECK, NVM_OFF_DIS_GRE_VER_CHECK,
 	 BNXT_NVM_SHARED_CFG, 1, 1},
+	{DEVLINK_PARAM_GENERIC_ID_ENABLE_HOT_FW_RESET,
+	 NVM_OFF_LIVE_FW_UPGRADE, BNXT_NVM_SHARED_CFG, 1, 1},
 };
 
 union bnxt_nvm_data {
@@ -618,6 +620,61 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
 	return 0;
 }
 
+static int bnxt_dl_param_get(struct devlink *dl, u32 id,
+			     struct devlink_param_gset_ctx *ctx)
+{
+	struct bnxt *bp = bnxt_get_bp_from_dl(dl);
+
+	if (ctx->cmode == DEVLINK_PARAM_CMODE_PERMANENT)
+		return bnxt_dl_nvm_param_get(dl, id, ctx);
+	else if (ctx->cmode == DEVLINK_PARAM_CMODE_RUNTIME)
+		return bnxt_hwrm_get_hot_reset(bp, &ctx->val.vbool);
+
+	return -EOPNOTSUPP;
+}
+
+static int bnxt_hwrm_set_hot_reset(struct bnxt *bp, bool hot_reset_if_support)
+{
+	struct hwrm_func_cfg_input req = {0};
+	bool hot_reset_allowed;
+	int rc;
+
+	if (!(bp->fw_cap & BNXT_FW_CAP_HOT_RESET_IF_SUPPORT))
+		return -EOPNOTSUPP;
+
+	bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_FUNC_CFG, -1, -1);
+	req.fid = cpu_to_le16(0xffff);
+	req.enables = cpu_to_le32(FUNC_CFG_REQ_ENABLES_HOT_RESET_IF_SUPPORT);
+	if (hot_reset_if_support)
+		req.flags = cpu_to_le32(FUNC_CFG_REQ_FLAGS_HOT_RESET_IF_EN_DIS);
+
+	rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
+	if (rc)
+		return rc;
+
+	rc = bnxt_hwrm_get_hot_reset(bp, &hot_reset_allowed);
+	if (rc)
+		return rc;
+
+	if (hot_reset_if_support && !hot_reset_allowed)
+		netdev_info(bp->dev, "Hot FW Reset enabled, but is disallowed as it is disabled on other interface(s) of this device.\n");
+
+	return 0;
+}
+
+static int bnxt_dl_param_set(struct devlink *dl, u32 id,
+			     struct devlink_param_gset_ctx *ctx)
+{
+	struct bnxt *bp = bnxt_get_bp_from_dl(dl);
+
+	if (ctx->cmode == DEVLINK_PARAM_CMODE_PERMANENT)
+		return bnxt_dl_nvm_param_set(dl, id, ctx);
+	else if (ctx->cmode == DEVLINK_PARAM_CMODE_RUNTIME)
+		return bnxt_hwrm_set_hot_reset(bp, ctx->val.vbool);
+
+	return -EOPNOTSUPP;
+}
+
 static const struct devlink_param bnxt_dl_params[] = {
 	DEVLINK_PARAM_GENERIC(ENABLE_SRIOV,
 			      BIT(DEVLINK_PARAM_CMODE_PERMANENT),
@@ -640,6 +697,10 @@ static int bnxt_dl_msix_validate(struct devlink *dl, u32 id,
 			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
 			     bnxt_dl_nvm_param_get, bnxt_dl_nvm_param_set,
 			     NULL),
+	DEVLINK_PARAM_GENERIC(ENABLE_HOT_FW_RESET,
+			      BIT(DEVLINK_PARAM_CMODE_PERMANENT) |
+			      BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			      bnxt_dl_param_get, bnxt_dl_param_set, NULL),
 };
 
 static const struct devlink_param bnxt_dl_port_params[] = {
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
index d5c8bd4..0a5efff 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.h
@@ -39,6 +39,7 @@ static inline void bnxt_link_bp_to_dl(struct bnxt *bp, struct devlink *dl)
 #define NVM_OFF_DIS_GRE_VER_CHECK	171
 #define NVM_OFF_ENABLE_SRIOV		401
 #define NVM_OFF_NVM_CFG_VER		602
+#define NVM_OFF_LIVE_FW_UPGRADE		917
 
 #define BNXT_NVM_CFG_VER_BITS		24
 #define BNXT_NVM_CFG_VER_BYTES		4
-- 
1.8.3.1

[PATCH net-next 4/4] bnxt_en: Check if hot_fw_reset is allowed before doing ETHTOOL_RESET

[Vasundhara Volam]

If device does not allow hot_fw_reset, issue firmware reset
without graceful flag.

Cc: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index dd0c3f2..e5eb8d2 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1888,12 +1888,11 @@ static int bnxt_firmware_reset(struct net_device *dev,
 	return bnxt_hwrm_firmware_reset(dev, proc_type, self_reset, flags);
 }
 
-static int bnxt_firmware_reset_chip(struct net_device *dev)
+static int bnxt_firmware_reset_chip(struct net_device *dev, bool hot_reset)
 {
-	struct bnxt *bp = netdev_priv(dev);
 	u8 flags = 0;
 
-	if (bp->fw_cap & BNXT_FW_CAP_HOT_RESET)
+	if (hot_reset)
 		flags = FW_RESET_REQ_FLAGS_RESET_GRACEFUL;
 
 	return bnxt_hwrm_firmware_reset(dev,
@@ -3082,7 +3081,7 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
 static int bnxt_reset(struct net_device *dev, u32 *flags)
 {
 	struct bnxt *bp = netdev_priv(dev);
-	bool reload = false;
+	bool reload = false, hot_reset;
 	u32 req = *flags;
 
 	if (!req)
@@ -3093,8 +3092,10 @@ static int bnxt_reset(struct net_device *dev, u32 *flags)
 		return -EOPNOTSUPP;
 	}
 
-	if (pci_vfs_assigned(bp->pdev) &&
-	    !(bp->fw_cap & BNXT_FW_CAP_HOT_RESET)) {
+	if (bnxt_hwrm_get_hot_reset(bp, &hot_reset))
+		hot_reset = !!(bp->fw_cap & BNXT_FW_CAP_HOT_RESET);
+
+	if (pci_vfs_assigned(bp->pdev) && !hot_reset) {
 		netdev_err(dev,
 			   "Reset not allowed when VFs are assigned to VMs\n");
 		return -EBUSY;
@@ -3103,9 +3104,9 @@ static int bnxt_reset(struct net_device *dev, u32 *flags)
 	if ((req & BNXT_FW_RESET_CHIP) == BNXT_FW_RESET_CHIP) {
 		/* This feature is not supported in older firmware versions */
 		if (bp->hwrm_spec_code >= 0x10803) {
-			if (!bnxt_firmware_reset_chip(dev)) {
+			if (!bnxt_firmware_reset_chip(dev, hot_reset)) {
 				netdev_info(dev, "Firmware reset request successful.\n");
-				if (!(bp->fw_cap & BNXT_FW_CAP_HOT_RESET))
+				if (!hot_reset)
 					reload = true;
 				*flags &= ~BNXT_FW_RESET_CHIP;
 			}
-- 
1.8.3.1

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Jiri Pirko]

Mon, May 18, 2020 at 10:27:15AM CEST, vasundhara-v.volam@broadcom.com wrote:
>This patchset adds support for a "enable_hot_fw_reset" generic devlink
>parameter and use it in bnxt_en driver.
>
>Also, firmware spec. is updated to 1.10.1.40.

Hi.

We've been discussing this internally for some time.
I don't like to use params for this purpose.
We already have "devlink dev flash" and "devlink dev reload" commands.
Combination of these two with appropriate attributes should provide what
you want. The "param" you are introducing is related to either "flash"
or "reload", so I don't think it is good to have separate param, when we
can extend the command attributes.

How does flash&reload work for mlxsw now:

# devlink flash
Now new version is pending, old FW is running
# devlink reload
Driver resets the device, new FW is loaded

I propose to extend reload like this:

 devlink dev reload DEV [ level { driver-default | fw-reset | driver-only | fw-live-patch } ]
   driver-default - means one of following to, according to what is
                    default for the driver
   fw-reset - does FW reset and driver entities re-instantiation
   driver-only - does driver entities re-instantiation only
   fw-live-patch - does only FW live patching - no effect on kernel

Could be an enum or bitfield. Does not matter. The point is to use
reload with attribute to achieve what user wants. In your usecase, user
would do:

# devlink flash
# devlink reload level fw-live-patch

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Jakub Kicinski]

On Mon, 18 May 2020 13:01:52 +0200 Jiri Pirko wrote:
> Mon, May 18, 2020 at 10:27:15AM CEST, vasundhara-v.volam@broadcom.com wrote:
> >This patchset adds support for a "enable_hot_fw_reset" generic devlink
> >parameter and use it in bnxt_en driver.
> >
> >Also, firmware spec. is updated to 1.10.1.40.  
> 
> Hi.
> 
> We've been discussing this internally for some time.
> I don't like to use params for this purpose.
> We already have "devlink dev flash" and "devlink dev reload" commands.
> Combination of these two with appropriate attributes should provide what
> you want. The "param" you are introducing is related to either "flash"
> or "reload", so I don't think it is good to have separate param, when we
> can extend the command attributes.
> 
> How does flash&reload work for mlxsw now:
> 
> # devlink flash
> Now new version is pending, old FW is running
> # devlink reload
> Driver resets the device, new FW is loaded
> 
> I propose to extend reload like this:
> 
>  devlink dev reload DEV [ level { driver-default | fw-reset | driver-only | fw-live-patch } ]
>    driver-default - means one of following to, according to what is
>                     default for the driver
>    fw-reset - does FW reset and driver entities re-instantiation
>    driver-only - does driver entities re-instantiation only
>    fw-live-patch - does only FW live patching - no effect on kernel
> 
> Could be an enum or bitfield. Does not matter. The point is to use
> reload with attribute to achieve what user wants. In your usecase, user
> would do:
> 
> # devlink flash
> # devlink reload level fw-live-patch

Unfortunately for SmartNICs and MultiHost systems the reset may not be
initiated locally. I agree it'd be great to have a normal netlink knob
for this instead of a param. But it has to be some form of a policy of
allowing the reset to happen, rather than an action/trigger kind of
thing.

Also user space notification should be generated when reset happens,
IMO. devlink dev info contents will likely change after reset, if
nothing else.

Plus this functionality will need proper documentation.

FWIW - I am unconvinced that applications will be happy to experience
network black outs, rather than being fully killed and re-spawned. For
a micro-service worker shutdown + re-spawn should be bread and butter.
But we already have ionic doing this, so seems like vendors are
convinced otherwise, so a common interface is probably a good step.

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Vasundhara Volam]

On Mon, May 18, 2020 at 4:31 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Mon, May 18, 2020 at 10:27:15AM CEST, vasundhara-v.volam@broadcom.com wrote:
> >This patchset adds support for a "enable_hot_fw_reset" generic devlink
> >parameter and use it in bnxt_en driver.
> >
> >Also, firmware spec. is updated to 1.10.1.40.
>
> Hi.
>
> We've been discussing this internally for some time.
> I don't like to use params for this purpose.
> We already have "devlink dev flash" and "devlink dev reload" commands.
> Combination of these two with appropriate attributes should provide what
> you want. The "param" you are introducing is related to either "flash"
> or "reload", so I don't think it is good to have separate param, when we
> can extend the command attributes.
>
> How does flash&reload work for mlxsw now:
>
> # devlink flash
> Now new version is pending, old FW is running
> # devlink reload
> Driver resets the device, new FW is loaded
>
> I propose to extend reload like this:
>
>  devlink dev reload DEV [ level { driver-default | fw-reset | driver-only | fw-live-patch } ]
>    driver-default - means one of following to, according to what is
>                     default for the driver
>    fw-reset - does FW reset and driver entities re-instantiation
>    driver-only - does driver entities re-instantiation only
>    fw-live-patch - does only FW live patching - no effect on kernel
>
> Could be an enum or bitfield. Does not matter. The point is to use
> reload with attribute to achieve what user wants. In your usecase, user
> would do:
>
> # devlink flash
> # devlink reload level fw-live-patch

Jiri,

I am adding this param to control the fw hot reset capability of the device.
Permanent configuration mode will toggle the NVM config space which is
very similar to enable_roce/enable_sriov param and runtime configuration
mode will toggle the driver level knob to avoid/allow fw-live-reset.

From above. I see that you are suggesting how to trigger the fw hot reset.
This is good to have, but does not serve the purpose of enabling or disabling
of the feature. Our driver is currently using "ethtool --reset" for
triggering fw-reset
or fw-live-patch.

Thanks,
Vasundhara

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Jiri Pirko]

Tue, May 19, 2020 at 01:43:09AM CEST, kuba@kernel.org wrote:
>On Mon, 18 May 2020 13:01:52 +0200 Jiri Pirko wrote:
>> Mon, May 18, 2020 at 10:27:15AM CEST, vasundhara-v.volam@broadcom.com wrote:
>> >This patchset adds support for a "enable_hot_fw_reset" generic devlink
>> >parameter and use it in bnxt_en driver.
>> >
>> >Also, firmware spec. is updated to 1.10.1.40.  
>> 
>> Hi.
>> 
>> We've been discussing this internally for some time.
>> I don't like to use params for this purpose.
>> We already have "devlink dev flash" and "devlink dev reload" commands.
>> Combination of these two with appropriate attributes should provide what
>> you want. The "param" you are introducing is related to either "flash"
>> or "reload", so I don't think it is good to have separate param, when we
>> can extend the command attributes.
>> 
>> How does flash&reload work for mlxsw now:
>> 
>> # devlink flash
>> Now new version is pending, old FW is running
>> # devlink reload
>> Driver resets the device, new FW is loaded
>> 
>> I propose to extend reload like this:
>> 
>>  devlink dev reload DEV [ level { driver-default | fw-reset | driver-only | fw-live-patch } ]
>>    driver-default - means one of following to, according to what is
>>                     default for the driver
>>    fw-reset - does FW reset and driver entities re-instantiation
>>    driver-only - does driver entities re-instantiation only
>>    fw-live-patch - does only FW live patching - no effect on kernel
>> 
>> Could be an enum or bitfield. Does not matter. The point is to use
>> reload with attribute to achieve what user wants. In your usecase, user
>> would do:
>> 
>> # devlink flash
>> # devlink reload level fw-live-patch
>
>Unfortunately for SmartNICs and MultiHost systems the reset may not be
>initiated locally. I agree it'd be great to have a normal netlink knob

I don't follow. Locally initiated or not, why what I suggested is not
enough to cover that?


>for this instead of a param. But it has to be some form of a policy of
>allowing the reset to happen, rather than an action/trigger kind of
>thing.

The "host" allows to reset himself by the "smartnic", right? For that, I
can imagine a param. But that is not the case of this patchset.


>
>Also user space notification should be generated when reset happens,
>IMO. devlink dev info contents will likely change after reset, if
>nothing else.

I agree.


>
>Plus this functionality will need proper documentation.

Also agreed.


>
>FWIW - I am unconvinced that applications will be happy to experience
>network black outs, rather than being fully killed and re-spawned. For
>a micro-service worker shutdown + re-spawn should be bread and butter.
>But we already have ionic doing this, so seems like vendors are
>convinced otherwise, so a common interface is probably a good step.

Hmm, not sure I follow what you mean by this para in context of this
patchset. Could you please explain? Thanks!

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Jiri Pirko]

Tue, May 19, 2020 at 06:31:27AM CEST, vasundhara-v.volam@broadcom.com wrote:
>On Mon, May 18, 2020 at 4:31 PM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Mon, May 18, 2020 at 10:27:15AM CEST, vasundhara-v.volam@broadcom.com wrote:
>> >This patchset adds support for a "enable_hot_fw_reset" generic devlink
>> >parameter and use it in bnxt_en driver.
>> >
>> >Also, firmware spec. is updated to 1.10.1.40.
>>
>> Hi.
>>
>> We've been discussing this internally for some time.
>> I don't like to use params for this purpose.
>> We already have "devlink dev flash" and "devlink dev reload" commands.
>> Combination of these two with appropriate attributes should provide what
>> you want. The "param" you are introducing is related to either "flash"
>> or "reload", so I don't think it is good to have separate param, when we
>> can extend the command attributes.
>>
>> How does flash&reload work for mlxsw now:
>>
>> # devlink flash
>> Now new version is pending, old FW is running
>> # devlink reload
>> Driver resets the device, new FW is loaded
>>
>> I propose to extend reload like this:
>>
>>  devlink dev reload DEV [ level { driver-default | fw-reset | driver-only | fw-live-patch } ]
>>    driver-default - means one of following to, according to what is
>>                     default for the driver
>>    fw-reset - does FW reset and driver entities re-instantiation
>>    driver-only - does driver entities re-instantiation only
>>    fw-live-patch - does only FW live patching - no effect on kernel
>>
>> Could be an enum or bitfield. Does not matter. The point is to use
>> reload with attribute to achieve what user wants. In your usecase, user
>> would do:
>>
>> # devlink flash
>> # devlink reload level fw-live-patch
>
>Jiri,
>
>I am adding this param to control the fw hot reset capability of the device.

I don't follow, sorry. Could you be more verbose about what you are
trying to achieve here?


>Permanent configuration mode will toggle the NVM config space which is
>very similar to enable_roce/enable_sriov param and runtime configuration
>mode will toggle the driver level knob to avoid/allow fw-live-reset.
>
>From above. I see that you are suggesting how to trigger the fw hot reset.
>This is good to have, but does not serve the purpose of enabling or disabling
>of the feature. Our driver is currently using "ethtool --reset" for
>triggering fw-reset
>or fw-live-patch.
>
>Thanks,
>Vasundhara

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Vasundhara Volam]

On Tue, May 19, 2020 at 10:57 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Tue, May 19, 2020 at 06:31:27AM CEST, vasundhara-v.volam@broadcom.com wrote:
> >On Mon, May 18, 2020 at 4:31 PM Jiri Pirko <jiri@resnulli.us> wrote:
> >>
> >> Mon, May 18, 2020 at 10:27:15AM CEST, vasundhara-v.volam@broadcom.com wrote:
> >> >This patchset adds support for a "enable_hot_fw_reset" generic devlink
> >> >parameter and use it in bnxt_en driver.
> >> >
> >> >Also, firmware spec. is updated to 1.10.1.40.
> >>
> >> Hi.
> >>
> >> We've been discussing this internally for some time.
> >> I don't like to use params for this purpose.
> >> We already have "devlink dev flash" and "devlink dev reload" commands.
> >> Combination of these two with appropriate attributes should provide what
> >> you want. The "param" you are introducing is related to either "flash"
> >> or "reload", so I don't think it is good to have separate param, when we
> >> can extend the command attributes.
> >>
> >> How does flash&reload work for mlxsw now:
> >>
> >> # devlink flash
> >> Now new version is pending, old FW is running
> >> # devlink reload
> >> Driver resets the device, new FW is loaded
> >>
> >> I propose to extend reload like this:
> >>
> >>  devlink dev reload DEV [ level { driver-default | fw-reset | driver-only | fw-live-patch } ]
> >>    driver-default - means one of following to, according to what is
> >>                     default for the driver
> >>    fw-reset - does FW reset and driver entities re-instantiation
> >>    driver-only - does driver entities re-instantiation only
> >>    fw-live-patch - does only FW live patching - no effect on kernel
> >>
> >> Could be an enum or bitfield. Does not matter. The point is to use
> >> reload with attribute to achieve what user wants. In your usecase, user
> >> would do:
> >>
> >> # devlink flash
> >> # devlink reload level fw-live-patch
> >
> >Jiri,
> >
> >I am adding this param to control the fw hot reset capability of the device.
>
> I don't follow, sorry. Could you be more verbose about what you are
> trying to achieve here?
As mentioned below, hot_fw_reset is a device capability similar to roce.
Capability can be enabled or disabled on the device, if the device supports.
When enabled and if supported firmware and driver are loaded, user can
utilise the capability to fw_reset or fw_live_patch.

So, we need a policy to enable/disable the capability.

Thanks.
>
> >Permanent configuration mode will toggle the NVM config space which is
> >very similar to enable_roce/enable_sriov param and runtime configuration
> >mode will toggle the driver level knob to avoid/allow fw-live-reset.
> >
> >From above. I see that you are suggesting how to trigger the fw hot reset.
> >This is good to have, but does not serve the purpose of enabling or disabling
> >of the feature. Our driver is currently using "ethtool --reset" for
> >triggering fw-reset
> >or fw-live-patch.
> >
> >Thanks,
> >Vasundhara

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Edwin Peer]

On Mon, May 18, 2020 at 10:27 PM Jiri Pirko <jiri@resnulli.us> wrote:

> >I am adding this param to control the fw hot reset capability of the device.
>
> I don't follow, sorry. Could you be more verbose about what you are
> trying to achieve here?

Hi Jiri,

Vasundhara is not adding a mechanism to trigger the actual reset here.
This is a parameter to enable or disable the hot reset functionality.
It's configuration, not an action.

Regards,
Edwin Peer

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Jiri Pirko]

Tue, May 19, 2020 at 09:13:46AM CEST, edwin.peer@broadcom.com wrote:
>On Mon, May 18, 2020 at 10:27 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
>> >I am adding this param to control the fw hot reset capability of the device.
>>
>> I don't follow, sorry. Could you be more verbose about what you are
>> trying to achieve here?
>
>Hi Jiri,
>
>Vasundhara is not adding a mechanism to trigger the actual reset here.
>This is a parameter to enable or disable the hot reset functionality.
>It's configuration, not an action.

I understand is it not an action.

But I think I missed what exactly it configures. I still don't have a
clue.

>
>Regards,
>Edwin Peer

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Jiri Pirko]

Tue, May 19, 2020 at 07:43:01AM CEST, vasundhara-v.volam@broadcom.com wrote:
>On Tue, May 19, 2020 at 10:57 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Tue, May 19, 2020 at 06:31:27AM CEST, vasundhara-v.volam@broadcom.com wrote:
>> >On Mon, May 18, 2020 at 4:31 PM Jiri Pirko <jiri@resnulli.us> wrote:
>> >>
>> >> Mon, May 18, 2020 at 10:27:15AM CEST, vasundhara-v.volam@broadcom.com wrote:
>> >> >This patchset adds support for a "enable_hot_fw_reset" generic devlink
>> >> >parameter and use it in bnxt_en driver.
>> >> >
>> >> >Also, firmware spec. is updated to 1.10.1.40.
>> >>
>> >> Hi.
>> >>
>> >> We've been discussing this internally for some time.
>> >> I don't like to use params for this purpose.
>> >> We already have "devlink dev flash" and "devlink dev reload" commands.
>> >> Combination of these two with appropriate attributes should provide what
>> >> you want. The "param" you are introducing is related to either "flash"
>> >> or "reload", so I don't think it is good to have separate param, when we
>> >> can extend the command attributes.
>> >>
>> >> How does flash&reload work for mlxsw now:
>> >>
>> >> # devlink flash
>> >> Now new version is pending, old FW is running
>> >> # devlink reload
>> >> Driver resets the device, new FW is loaded
>> >>
>> >> I propose to extend reload like this:
>> >>
>> >>  devlink dev reload DEV [ level { driver-default | fw-reset | driver-only | fw-live-patch } ]
>> >>    driver-default - means one of following to, according to what is
>> >>                     default for the driver
>> >>    fw-reset - does FW reset and driver entities re-instantiation
>> >>    driver-only - does driver entities re-instantiation only
>> >>    fw-live-patch - does only FW live patching - no effect on kernel
>> >>
>> >> Could be an enum or bitfield. Does not matter. The point is to use
>> >> reload with attribute to achieve what user wants. In your usecase, user
>> >> would do:
>> >>
>> >> # devlink flash
>> >> # devlink reload level fw-live-patch
>> >
>> >Jiri,
>> >
>> >I am adding this param to control the fw hot reset capability of the device.
>>
>> I don't follow, sorry. Could you be more verbose about what you are
>> trying to achieve here?
>As mentioned below, hot_fw_reset is a device capability similar to roce.
>Capability can be enabled or disabled on the device, if the device supports.
>When enabled and if supported firmware and driver are loaded, user can
>utilise the capability to fw_reset or fw_live_patch.

I don't undestand what exactly is this supposed to enable/disable. Could
you be more specific?


>
>So, we need a policy to enable/disable the capability.
>
>Thanks.
>>
>> >Permanent configuration mode will toggle the NVM config space which is
>> >very similar to enable_roce/enable_sriov param and runtime configuration
>> >mode will toggle the driver level knob to avoid/allow fw-live-reset.
>> >
>> >From above. I see that you are suggesting how to trigger the fw hot reset.
>> >This is good to have, but does not serve the purpose of enabling or disabling
>> >of the feature. Our driver is currently using "ethtool --reset" for
>> >triggering fw-reset
>> >or fw-live-patch.
>> >
>> >Thanks,
>> >Vasundhara

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Michael Chan]

On Tue, May 19, 2020 at 12:30 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Tue, May 19, 2020 at 07:43:01AM CEST, vasundhara-v.volam@broadcom.com wrote:
> >On Tue, May 19, 2020 at 10:57 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >>
> >> I don't follow, sorry. Could you be more verbose about what you are
> >> trying to achieve here?
> >As mentioned below, hot_fw_reset is a device capability similar to roce.
> >Capability can be enabled or disabled on the device, if the device supports.
> >When enabled and if supported firmware and driver are loaded, user can
> >utilise the capability to fw_reset or fw_live_patch.
>
> I don't undestand what exactly is this supposed to enable/disable. Could
> you be more specific?

Let me see if I can help clarify.  Here's a little background.  Hot
reset is a feature supported by the firmware and requires the
coordinated support of all function drivers.  The firmware will only
initiate this hot reset when all function drivers can support it.  For
example, if one function is running a really old driver that doesn't
support it, the firmware will not support this until this old driver
gets unloaded or upgraded.  Until then, a PCI reset is needed to reset
the firmware.

Now, let's say one function driver that normally supports this
firmware hot reset now wants to disable this feature.  For example,
the function is running a mission critical application and does not
want any hot firmware reset that may cause a hiccup during this time.
It will use this devlink parameter to disable it.  When the critical
app is done, it can then re-enable the parameter.  Of course other
functions can also disable it and it is only enabled when all
functions have enabled it.

Hope this clarifies it.  Thanks.

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Jiri Pirko]

Tue, May 19, 2020 at 10:41:44AM CEST, michael.chan@broadcom.com wrote:
>On Tue, May 19, 2020 at 12:30 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Tue, May 19, 2020 at 07:43:01AM CEST, vasundhara-v.volam@broadcom.com wrote:
>> >On Tue, May 19, 2020 at 10:57 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >>
>> >> I don't follow, sorry. Could you be more verbose about what you are
>> >> trying to achieve here?
>> >As mentioned below, hot_fw_reset is a device capability similar to roce.
>> >Capability can be enabled or disabled on the device, if the device supports.
>> >When enabled and if supported firmware and driver are loaded, user can
>> >utilise the capability to fw_reset or fw_live_patch.
>>
>> I don't undestand what exactly is this supposed to enable/disable. Could
>> you be more specific?
>
>Let me see if I can help clarify.  Here's a little background.  Hot
>reset is a feature supported by the firmware and requires the
>coordinated support of all function drivers.  The firmware will only
>initiate this hot reset when all function drivers can support it.  For
>example, if one function is running a really old driver that doesn't
>support it, the firmware will not support this until this old driver
>gets unloaded or upgraded.  Until then, a PCI reset is needed to reset
>the firmware.
>
>Now, let's say one function driver that normally supports this
>firmware hot reset now wants to disable this feature.  For example,
>the function is running a mission critical application and does not
>want any hot firmware reset that may cause a hiccup during this time.
>It will use this devlink parameter to disable it.  When the critical
>app is done, it can then re-enable the parameter.  Of course other
>functions can also disable it and it is only enabled when all
>functions have enabled it.
>
>Hope this clarifies it.  Thanks.

Okay. So this is about the "allowing to be reseted from the outside".
I see. For that I think it makes sense to have the devlink param.
However, I think that it would be fine to find more suitable name and
describe this properly in the docs.

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Vasundhara Volam]

On Tue, May 19, 2020 at 3:11 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Tue, May 19, 2020 at 10:41:44AM CEST, michael.chan@broadcom.com wrote:
> >On Tue, May 19, 2020 at 12:30 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >>
> >> Tue, May 19, 2020 at 07:43:01AM CEST, vasundhara-v.volam@broadcom.com wrote:
> >> >On Tue, May 19, 2020 at 10:57 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >> >>
> >> >> I don't follow, sorry. Could you be more verbose about what you are
> >> >> trying to achieve here?
> >> >As mentioned below, hot_fw_reset is a device capability similar to roce.
> >> >Capability can be enabled or disabled on the device, if the device supports.
> >> >When enabled and if supported firmware and driver are loaded, user can
> >> >utilise the capability to fw_reset or fw_live_patch.
> >>
> >> I don't undestand what exactly is this supposed to enable/disable. Could
> >> you be more specific?
> >
> >Let me see if I can help clarify.  Here's a little background.  Hot
> >reset is a feature supported by the firmware and requires the
> >coordinated support of all function drivers.  The firmware will only
> >initiate this hot reset when all function drivers can support it.  For
> >example, if one function is running a really old driver that doesn't
> >support it, the firmware will not support this until this old driver
> >gets unloaded or upgraded.  Until then, a PCI reset is needed to reset
> >the firmware.
> >
> >Now, let's say one function driver that normally supports this
> >firmware hot reset now wants to disable this feature.  For example,
> >the function is running a mission critical application and does not
> >want any hot firmware reset that may cause a hiccup during this time.
> >It will use this devlink parameter to disable it.  When the critical
> >app is done, it can then re-enable the parameter.  Of course other
> >functions can also disable it and it is only enabled when all
> >functions have enabled it.
> >
> >Hope this clarifies it.  Thanks.
>
> Okay. So this is about the "allowing to be reseted from the outside".
> I see. For that I think it makes sense to have the devlink param.

> However, I think that it would be fine to find more suitable name and
> describe this properly in the docs.
>
I felt enable_hot_fw_reset is a self-descriptive name.

But to make it more common, is the name enable_live_fw_reset good?
or simply fw_reset?

Thanks.

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Jiri Pirko]

Tue, May 19, 2020 at 12:50:14PM CEST, vasundhara-v.volam@broadcom.com wrote:
>On Tue, May 19, 2020 at 3:11 PM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Tue, May 19, 2020 at 10:41:44AM CEST, michael.chan@broadcom.com wrote:
>> >On Tue, May 19, 2020 at 12:30 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >>
>> >> Tue, May 19, 2020 at 07:43:01AM CEST, vasundhara-v.volam@broadcom.com wrote:
>> >> >On Tue, May 19, 2020 at 10:57 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >> >>
>> >> >> I don't follow, sorry. Could you be more verbose about what you are
>> >> >> trying to achieve here?
>> >> >As mentioned below, hot_fw_reset is a device capability similar to roce.
>> >> >Capability can be enabled or disabled on the device, if the device supports.
>> >> >When enabled and if supported firmware and driver are loaded, user can
>> >> >utilise the capability to fw_reset or fw_live_patch.
>> >>
>> >> I don't undestand what exactly is this supposed to enable/disable. Could
>> >> you be more specific?
>> >
>> >Let me see if I can help clarify.  Here's a little background.  Hot
>> >reset is a feature supported by the firmware and requires the
>> >coordinated support of all function drivers.  The firmware will only
>> >initiate this hot reset when all function drivers can support it.  For
>> >example, if one function is running a really old driver that doesn't
>> >support it, the firmware will not support this until this old driver
>> >gets unloaded or upgraded.  Until then, a PCI reset is needed to reset
>> >the firmware.
>> >
>> >Now, let's say one function driver that normally supports this
>> >firmware hot reset now wants to disable this feature.  For example,
>> >the function is running a mission critical application and does not
>> >want any hot firmware reset that may cause a hiccup during this time.
>> >It will use this devlink parameter to disable it.  When the critical
>> >app is done, it can then re-enable the parameter.  Of course other
>> >functions can also disable it and it is only enabled when all
>> >functions have enabled it.
>> >
>> >Hope this clarifies it.  Thanks.
>>
>> Okay. So this is about the "allowing to be reseted from the outside".
>> I see. For that I think it makes sense to have the devlink param.
>
>> However, I think that it would be fine to find more suitable name and
>> describe this properly in the docs.
>>
>I felt enable_hot_fw_reset is a self-descriptive name.
>
>But to make it more common, is the name enable_live_fw_reset good?
>or simply fw_reset?

I think it is important to emhasize that this setting is related to
"remote" reset.


>
>Thanks.

Re: [PATCH net-next 0/4] bnxt_en: Add new "enable_hot_fw_reset" generic devlink parameter

[Jakub Kicinski]

On Tue, 19 May 2020 07:24:00 +0200 Jiri Pirko wrote:
> Tue, May 19, 2020 at 01:43:09AM CEST, kuba@kernel.org wrote:
> >On Mon, 18 May 2020 13:01:52 +0200 Jiri Pirko wrote:  
> >> Mon, May 18, 2020 at 10:27:15AM CEST, vasundhara-v.volam@broadcom.com wrote:  
> >> >This patchset adds support for a "enable_hot_fw_reset" generic devlink
> >> >parameter and use it in bnxt_en driver.
> >> >
> >> >Also, firmware spec. is updated to 1.10.1.40.    
> >> 
> >> Hi.
> >> 
> >> We've been discussing this internally for some time.
> >> I don't like to use params for this purpose.
> >> We already have "devlink dev flash" and "devlink dev reload" commands.
> >> Combination of these two with appropriate attributes should provide what
> >> you want. The "param" you are introducing is related to either "flash"
> >> or "reload", so I don't think it is good to have separate param, when we
> >> can extend the command attributes.
> >> 
> >> How does flash&reload work for mlxsw now:
> >> 
> >> # devlink flash
> >> Now new version is pending, old FW is running
> >> # devlink reload
> >> Driver resets the device, new FW is loaded
> >> 
> >> I propose to extend reload like this:
> >> 
> >>  devlink dev reload DEV [ level { driver-default | fw-reset | driver-only | fw-live-patch } ]
> >>    driver-default - means one of following to, according to what is
> >>                     default for the driver
> >>    fw-reset - does FW reset and driver entities re-instantiation
> >>    driver-only - does driver entities re-instantiation only
> >>    fw-live-patch - does only FW live patching - no effect on kernel
> >> 
> >> Could be an enum or bitfield. Does not matter. The point is to use
> >> reload with attribute to achieve what user wants. In your usecase, user
> >> would do:
> >> 
> >> # devlink flash
> >> # devlink reload level fw-live-patch  
> >
> >Unfortunately for SmartNICs and MultiHost systems the reset may not be
> >initiated locally. I agree it'd be great to have a normal netlink knob  
> 
> I don't follow. Locally initiated or not, why what I suggested is not
> enough to cover that?

Hopefully clear now after Michael's explanation :)

> >for this instead of a param. But it has to be some form of a policy of
> >allowing the reset to happen, rather than an action/trigger kind of
> >thing.  
> 
> The "host" allows to reset himself by the "smartnic", right? For that, I
> can imagine a param. But that is not the case of this patchset.
> 
> >Also user space notification should be generated when reset happens,
> >IMO. devlink dev info contents will likely change after reset, if
> >nothing else.  
> 
> I agree.
> 
> >Plus this functionality will need proper documentation.  
> 
> Also agreed.
> 
> >FWIW - I am unconvinced that applications will be happy to experience
> >network black outs, rather than being fully killed and re-spawned. For
> >a micro-service worker shutdown + re-spawn should be bread and butter.
> >But we already have ionic doing this, so seems like vendors are
> >convinced otherwise, so a common interface is probably a good step.  
> 
> Hmm, not sure I follow what you mean by this para in context of this
> patchset. Could you please explain? Thanks!

I'm saying I'm dubious users will actually enable the async remote
reset.