Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 net-next 0/4] bnxt_en: Add new "allow_fw_live_reset" generic devlink parameter
@ 2020-05-23  6:08 Vasundhara Volam
  2020-05-23  6:08 ` [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter Vasundhara Volam
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Vasundhara Volam @ 2020-05-23  6:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, Vasundhara Volam

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

Firmware live reset allows users to reset the firmware in real time. For
example, after firmware upgrade, this feature can immediately reset to run
the new firmware without reloading the driver or rebooting the system. When
device reset is initiated, services running on the host interfaces will
momentarily pause and resume once reset is completed which is very similar
to momentary network outage.

User can initiate the fw reset by using "ethtool --reset ethX all" command.
Where ethX is any PF of the device with administrative privileges.

Firmware can initiate the live reset only when all the installed host
driver(s) also support the feature. For example, if a function is loaded
with a very old driver that is not aware of live reset capability,
firmware cannot initiate the reset until that driver is unloaded or
upgraded.

"allow_fw_live_reset" runtime configuration mode allows the user to control
this feature by enabling or disabling it in the host driver. And permanent
configuration mode allows the user to enable the firmware live reset
capability in NVRAM configuration of the device.

Also, firmware spec. is updated to 1.10.1.40.

v1->v2: Rename param to "allow_fw_live_reset" from "enable_hot_fw_reset".
Update documentation files and commit messages with more details of the
feature.

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

 Documentation/networking/devlink/bnxt.rst          | 13 +++++
 .../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, 165 insertions(+), 36 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter.
  2020-05-23  6:08 [PATCH v2 net-next 0/4] bnxt_en: Add new "allow_fw_live_reset" generic devlink parameter Vasundhara Volam
@ 2020-05-23  6:08 ` Vasundhara Volam
  2020-05-24  4:53   ` Jiri Pirko
  2020-05-23  6:08 ` [PATCH v2 net-next 2/4] bnxt_en: Update firmware spec. to 1.10.1.40 Vasundhara Volam
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Vasundhara Volam @ 2020-05-23  6:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, Vasundhara Volam, Jiri Pirko, Michael Chan

Add a new "allow_fw_live_reset" generic device bool parameter. When
parameter is set, user is allowed to reset the firmware in real time.

This parameter is employed to communicate user consent or dissent for
the live reset to happen. A separate command triggers the actual live
reset.

Cc: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
v2: Rename param name to "allow_fw_live_reset" from
"enable_hot_fw_reset".
Update documentation for the param in devlink-params.rst file.
---
 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..ad54dfb 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.
+   * - ``allow_fw_live_reset``
+     - Boolean
+     - Firmware live reset allows users to reset the firmware in real time.
+       For example, after firmware upgrade, this feature can immediately reset
+       to run the new firmware without reloading the driver or rebooting the
+       system.
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 8ffc1b5c..488b61c 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_ALLOW_FW_LIVE_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_ALLOW_FW_LIVE_RESET_NAME "allow_fw_live_reset"
+#define DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_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..8544f23 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_ALLOW_FW_LIVE_RESET,
+		.name = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_NAME,
+		.type = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_TYPE,
+	},
 };
 
 static int devlink_param_generic_verify(const struct devlink_param *param)
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 net-next 2/4] bnxt_en: Update firmware spec. to 1.10.1.40.
  2020-05-23  6:08 [PATCH v2 net-next 0/4] bnxt_en: Add new "allow_fw_live_reset" generic devlink parameter Vasundhara Volam
  2020-05-23  6:08 ` [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter Vasundhara Volam
@ 2020-05-23  6:08 ` Vasundhara Volam
  2020-05-23  6:08 ` [PATCH v2 net-next 3/4] bnxt_en: Use allow_fw_live_reset generic devlink parameter Vasundhara Volam
  2020-05-23  6:08 ` [PATCH v2 net-next 4/4] bnxt_en: Check if fw_live_reset is allowed before doing ETHTOOL_RESET Vasundhara Volam
  3 siblings, 0 replies; 10+ messages in thread
From: Vasundhara Volam @ 2020-05-23  6:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, Vasundhara Volam, Michael Chan

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

Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 net-next 3/4] bnxt_en: Use allow_fw_live_reset generic devlink parameter
  2020-05-23  6:08 [PATCH v2 net-next 0/4] bnxt_en: Add new "allow_fw_live_reset" generic devlink parameter Vasundhara Volam
  2020-05-23  6:08 ` [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter Vasundhara Volam
  2020-05-23  6:08 ` [PATCH v2 net-next 2/4] bnxt_en: Update firmware spec. to 1.10.1.40 Vasundhara Volam
@ 2020-05-23  6:08 ` Vasundhara Volam
  2020-05-23  6:08 ` [PATCH v2 net-next 4/4] bnxt_en: Check if fw_live_reset is allowed before doing ETHTOOL_RESET Vasundhara Volam
  3 siblings, 0 replies; 10+ messages in thread
From: Vasundhara Volam @ 2020-05-23  6:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, Vasundhara Volam, Michael Chan

allow_fw_live_reset parameter supports both permanent and runtime
configuration modes. Permanent configuration mode allows user to
enable the firmware live reset device capability in NVRAM
configuration.

Runtime configuration mode allows the user to prevent firmware
live reset by setting it to false in a host driver. For parameter
to be true, all the installed host driver(s) must support the
feature, and the runtime value must also be true for all of them.
For example, if the host is running a mission critical application,
it can disable this parameter until the application has completed
to avoid a potential firmware live reset disrupting it.

Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
v2: Update commit message. Rename devlink param name to
"allow_fw_live_reset".
Add more documentation for param in bnxt.rst file.
---
 Documentation/networking/devlink/bnxt.rst         | 13 +++++
 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, 104 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/devlink/bnxt.rst b/Documentation/networking/devlink/bnxt.rst
index 3dfd84c..c4f84c9 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,18 @@ Parameters
      - Permanent
    * - ``msix_vec_per_pf_min``
      - Permanent
+   * - ``allow_fw_live_reset``
+     - Permanent, runtime
+     - The parameter's runtime configuration mode allows the user to prevent
+       firmware live reset by setting it to false in a host driver. For the
+       runtime parameter to be true, all the installed host driver(s) must
+       support the feature, and the runtime value must also be true for all of
+       them. For example, if the host is running a mission critical application,
+       user can disable this parameter until the application has completed to
+       avoid a potential firmware live reset disrupting it.
+
+       The parameter's permanent configuration mode allows user to control if
+       firmware live reset device capability is advertised to the drivers.
 
 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..9d679a8 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_ALLOW_FW_LIVE_RESET,
+	 NVM_OFF_FW_LIVE_RESET, 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, "FW live 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(ALLOW_FW_LIVE_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..0c786fb 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_FW_LIVE_RESET		917
 
 #define BNXT_NVM_CFG_VER_BITS		24
 #define BNXT_NVM_CFG_VER_BYTES		4
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 net-next 4/4] bnxt_en: Check if fw_live_reset is allowed before doing ETHTOOL_RESET
  2020-05-23  6:08 [PATCH v2 net-next 0/4] bnxt_en: Add new "allow_fw_live_reset" generic devlink parameter Vasundhara Volam
                   ` (2 preceding siblings ...)
  2020-05-23  6:08 ` [PATCH v2 net-next 3/4] bnxt_en: Use allow_fw_live_reset generic devlink parameter Vasundhara Volam
@ 2020-05-23  6:08 ` Vasundhara Volam
  3 siblings, 0 replies; 10+ messages in thread
From: Vasundhara Volam @ 2020-05-23  6:08 UTC (permalink / raw)
  To: davem; +Cc: netdev, Vasundhara Volam, Michael Chan

If device does not allow fw_live_reset, issue FW_RESET command
without graceful flag, which requires a driver reload to reset
the firmware.

Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Reviewed-by: Edwin Peer <edwin.peer@broadcom.com>
---
v2: Rephrase the subject and elaborate commit message
---
 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter.
  2020-05-23  6:08 ` [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter Vasundhara Volam
@ 2020-05-24  4:53   ` Jiri Pirko
  2020-05-24  6:29     ` Vasundhara Volam
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Pirko @ 2020-05-24  4:53 UTC (permalink / raw)
  To: Vasundhara Volam; +Cc: davem, netdev, Jiri Pirko, Michael Chan

Sat, May 23, 2020 at 08:08:22AM CEST, vasundhara-v.volam@broadcom.com wrote:
>Add a new "allow_fw_live_reset" generic device bool parameter. When
>parameter is set, user is allowed to reset the firmware in real time.
>
>This parameter is employed to communicate user consent or dissent for
>the live reset to happen. A separate command triggers the actual live
>reset.
>
>Cc: Jiri Pirko <jiri@mellanox.com>
>Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
>Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>---
>v2: Rename param name to "allow_fw_live_reset" from
>"enable_hot_fw_reset".
>Update documentation for the param in devlink-params.rst file.
>---
> 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..ad54dfb 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.
>+   * - ``allow_fw_live_reset``
>+     - Boolean
>+     - Firmware live reset allows users to reset the firmware in real time.
>+       For example, after firmware upgrade, this feature can immediately reset
>+       to run the new firmware without reloading the driver or rebooting the

This does not tell me anything about the reset being done on another
host. You need to emhasize that, in the name of the param too.



>+       system.
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 8ffc1b5c..488b61c 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_ALLOW_FW_LIVE_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_ALLOW_FW_LIVE_RESET_NAME "allow_fw_live_reset"
>+#define DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_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..8544f23 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_ALLOW_FW_LIVE_RESET,
>+		.name = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_NAME,
>+		.type = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_TYPE,
>+	},
> };
> 
> static int devlink_param_generic_verify(const struct devlink_param *param)
>-- 
>1.8.3.1
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter.
  2020-05-24  4:53   ` Jiri Pirko
@ 2020-05-24  6:29     ` Vasundhara Volam
  2020-05-25 17:26       ` Jiri Pirko
  0 siblings, 1 reply; 10+ messages in thread
From: Vasundhara Volam @ 2020-05-24  6:29 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: David Miller, Netdev, Jiri Pirko, Michael Chan

On Sun, May 24, 2020 at 10:23 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Sat, May 23, 2020 at 08:08:22AM CEST, vasundhara-v.volam@broadcom.com wrote:
> >Add a new "allow_fw_live_reset" generic device bool parameter. When
> >parameter is set, user is allowed to reset the firmware in real time.
> >
> >This parameter is employed to communicate user consent or dissent for
> >the live reset to happen. A separate command triggers the actual live
> >reset.
> >
> >Cc: Jiri Pirko <jiri@mellanox.com>
> >Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
> >Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> >---
> >v2: Rename param name to "allow_fw_live_reset" from
> >"enable_hot_fw_reset".
> >Update documentation for the param in devlink-params.rst file.
> >---
> > 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..ad54dfb 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.
> >+   * - ``allow_fw_live_reset``
> >+     - Boolean
> >+     - Firmware live reset allows users to reset the firmware in real time.
> >+       For example, after firmware upgrade, this feature can immediately reset
> >+       to run the new firmware without reloading the driver or rebooting the
>
> This does not tell me anything about the reset being done on another
> host. You need to emhasize that, in the name of the param too.
I am not sure if I completely understand your query.

Reset is actually initiated by one of the PF/host of the device, which
resets the entire same device.

Reset is not initiated by any other remote device/host.

Thanks,
Vasundhara
>
>
>
> >+       system.
> >diff --git a/include/net/devlink.h b/include/net/devlink.h
> >index 8ffc1b5c..488b61c 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_ALLOW_FW_LIVE_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_ALLOW_FW_LIVE_RESET_NAME "allow_fw_live_reset"
> >+#define DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_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..8544f23 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_ALLOW_FW_LIVE_RESET,
> >+              .name = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_NAME,
> >+              .type = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_TYPE,
> >+      },
> > };
> >
> > static int devlink_param_generic_verify(const struct devlink_param *param)
> >--
> >1.8.3.1
> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter.
  2020-05-24  6:29     ` Vasundhara Volam
@ 2020-05-25 17:26       ` Jiri Pirko
  2020-05-26  4:28         ` Vasundhara Volam
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Pirko @ 2020-05-25 17:26 UTC (permalink / raw)
  To: Vasundhara Volam; +Cc: David Miller, Netdev, Jiri Pirko, Michael Chan

Sun, May 24, 2020 at 08:29:56AM CEST, vasundhara-v.volam@broadcom.com wrote:
>On Sun, May 24, 2020 at 10:23 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Sat, May 23, 2020 at 08:08:22AM CEST, vasundhara-v.volam@broadcom.com wrote:
>> >Add a new "allow_fw_live_reset" generic device bool parameter. When
>> >parameter is set, user is allowed to reset the firmware in real time.
>> >
>> >This parameter is employed to communicate user consent or dissent for
>> >the live reset to happen. A separate command triggers the actual live
>> >reset.
>> >
>> >Cc: Jiri Pirko <jiri@mellanox.com>
>> >Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
>> >Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>> >---
>> >v2: Rename param name to "allow_fw_live_reset" from
>> >"enable_hot_fw_reset".
>> >Update documentation for the param in devlink-params.rst file.
>> >---
>> > 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..ad54dfb 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.
>> >+   * - ``allow_fw_live_reset``
>> >+     - Boolean
>> >+     - Firmware live reset allows users to reset the firmware in real time.
>> >+       For example, after firmware upgrade, this feature can immediately reset
>> >+       to run the new firmware without reloading the driver or rebooting the
>>
>> This does not tell me anything about the reset being done on another
>> host. You need to emhasize that, in the name of the param too.
>I am not sure if I completely understand your query.
>
>Reset is actually initiated by one of the PF/host of the device, which
>resets the entire same device.
>
>Reset is not initiated by any other remote device/host.

Well, in case of multihost system, it might be, right?


>
>Thanks,
>Vasundhara
>>
>>
>>
>> >+       system.
>> >diff --git a/include/net/devlink.h b/include/net/devlink.h
>> >index 8ffc1b5c..488b61c 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_ALLOW_FW_LIVE_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_ALLOW_FW_LIVE_RESET_NAME "allow_fw_live_reset"
>> >+#define DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_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..8544f23 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_ALLOW_FW_LIVE_RESET,
>> >+              .name = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_NAME,
>> >+              .type = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_TYPE,
>> >+      },
>> > };
>> >
>> > static int devlink_param_generic_verify(const struct devlink_param *param)
>> >--
>> >1.8.3.1
>> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter.
  2020-05-25 17:26       ` Jiri Pirko
@ 2020-05-26  4:28         ` Vasundhara Volam
  2020-05-26  4:47           ` Jiri Pirko
  0 siblings, 1 reply; 10+ messages in thread
From: Vasundhara Volam @ 2020-05-26  4:28 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: David Miller, Netdev, Jiri Pirko, Michael Chan

On Mon, May 25, 2020 at 10:56 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Sun, May 24, 2020 at 08:29:56AM CEST, vasundhara-v.volam@broadcom.com wrote:
> >On Sun, May 24, 2020 at 10:23 AM Jiri Pirko <jiri@resnulli.us> wrote:
> >>
> >> Sat, May 23, 2020 at 08:08:22AM CEST, vasundhara-v.volam@broadcom.com wrote:
> >> >Add a new "allow_fw_live_reset" generic device bool parameter. When
> >> >parameter is set, user is allowed to reset the firmware in real time.
> >> >
> >> >This parameter is employed to communicate user consent or dissent for
> >> >the live reset to happen. A separate command triggers the actual live
> >> >reset.
> >> >
> >> >Cc: Jiri Pirko <jiri@mellanox.com>
> >> >Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
> >> >Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> >> >---
> >> >v2: Rename param name to "allow_fw_live_reset" from
> >> >"enable_hot_fw_reset".
> >> >Update documentation for the param in devlink-params.rst file.
> >> >---
> >> > 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..ad54dfb 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.
> >> >+   * - ``allow_fw_live_reset``
> >> >+     - Boolean
> >> >+     - Firmware live reset allows users to reset the firmware in real time.
> >> >+       For example, after firmware upgrade, this feature can immediately reset
> >> >+       to run the new firmware without reloading the driver or rebooting the
> >>
> >> This does not tell me anything about the reset being done on another
> >> host. You need to emhasize that, in the name of the param too.
> >I am not sure if I completely understand your query.
> >
> >Reset is actually initiated by one of the PF/host of the device, which
> >resets the entire same device.
> >
> >Reset is not initiated by any other remote device/host.
>
> Well, in case of multihost system, it might be, right?
>
In case of multi-host system also, it is one of the host that triggers
the reset, which resets the entire same device. I don't think this is
remote.

As the parameter is a device parameter, it is applicable to the entire
device. When a user initiates the reset from any of the host in case
of multi-host and any of the PF in case of stand-alone or smartNIC
device, the entire device goes for a reset.

I will be expanding the description to the following to make it more clear.

------------------------
- Firmware live reset allows users to reset the firmware in real time.
For example, after firmware upgrade, this feature can immediately
reset to run the new firmware without reloading the driver or
rebooting the system.
When a user initiates the reset from any of the host (in case of
multi-host system) / PF (in case of stand-alone or smartNIC device),
the entire device goes for a reset when the parameter is enabled.
------------------------

Thanks,
Vasundhara
>
> >
> >Thanks,
> >Vasundhara
> >>
> >>
> >>
> >> >+       system.
> >> >diff --git a/include/net/devlink.h b/include/net/devlink.h
> >> >index 8ffc1b5c..488b61c 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_ALLOW_FW_LIVE_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_ALLOW_FW_LIVE_RESET_NAME "allow_fw_live_reset"
> >> >+#define DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_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..8544f23 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_ALLOW_FW_LIVE_RESET,
> >> >+              .name = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_NAME,
> >> >+              .type = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_TYPE,
> >> >+      },
> >> > };
> >> >
> >> > static int devlink_param_generic_verify(const struct devlink_param *param)
> >> >--
> >> >1.8.3.1
> >> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter.
  2020-05-26  4:28         ` Vasundhara Volam
@ 2020-05-26  4:47           ` Jiri Pirko
  0 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2020-05-26  4:47 UTC (permalink / raw)
  To: Vasundhara Volam; +Cc: David Miller, Netdev, Jiri Pirko, Michael Chan

Tue, May 26, 2020 at 06:28:59AM CEST, vasundhara-v.volam@broadcom.com wrote:
>On Mon, May 25, 2020 at 10:56 PM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> Sun, May 24, 2020 at 08:29:56AM CEST, vasundhara-v.volam@broadcom.com wrote:
>> >On Sun, May 24, 2020 at 10:23 AM Jiri Pirko <jiri@resnulli.us> wrote:
>> >>
>> >> Sat, May 23, 2020 at 08:08:22AM CEST, vasundhara-v.volam@broadcom.com wrote:
>> >> >Add a new "allow_fw_live_reset" generic device bool parameter. When
>> >> >parameter is set, user is allowed to reset the firmware in real time.
>> >> >
>> >> >This parameter is employed to communicate user consent or dissent for
>> >> >the live reset to happen. A separate command triggers the actual live
>> >> >reset.
>> >> >
>> >> >Cc: Jiri Pirko <jiri@mellanox.com>
>> >> >Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
>> >> >Signed-off-by: Michael Chan <michael.chan@broadcom.com>
>> >> >---
>> >> >v2: Rename param name to "allow_fw_live_reset" from
>> >> >"enable_hot_fw_reset".
>> >> >Update documentation for the param in devlink-params.rst file.
>> >> >---
>> >> > 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..ad54dfb 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.
>> >> >+   * - ``allow_fw_live_reset``
>> >> >+     - Boolean
>> >> >+     - Firmware live reset allows users to reset the firmware in real time.
>> >> >+       For example, after firmware upgrade, this feature can immediately reset
>> >> >+       to run the new firmware without reloading the driver or rebooting the
>> >>
>> >> This does not tell me anything about the reset being done on another
>> >> host. You need to emhasize that, in the name of the param too.
>> >I am not sure if I completely understand your query.
>> >
>> >Reset is actually initiated by one of the PF/host of the device, which
>> >resets the entire same device.
>> >
>> >Reset is not initiated by any other remote device/host.
>>
>> Well, in case of multihost system, it might be, right?
>>
>In case of multi-host system also, it is one of the host that triggers
>the reset, which resets the entire same device. I don't think this is
>remote.
>
>As the parameter is a device parameter, it is applicable to the entire
>device. When a user initiates the reset from any of the host in case
>of multi-host and any of the PF in case of stand-alone or smartNIC
>device, the entire device goes for a reset.
>
>I will be expanding the description to the following to make it more clear.
>
>------------------------
>- Firmware live reset allows users to reset the firmware in real time.
>For example, after firmware upgrade, this feature can immediately
>reset to run the new firmware without reloading the driver or
>rebooting the system.
>When a user initiates the reset from any of the host (in case of
>multi-host system) / PF (in case of stand-alone or smartNIC device),
>the entire device goes for a reset when the parameter is enabled.

Sorry, this is still not clear. I think that you are mixing up two
different things:
1) option of devlink reload to indicate that user is interested in "live
   reset" of firmware without reloading driver
2) devlink param that would indicate "I am okay if someone else (not by
   my devlink instance) resets my firmware".

Could you please split?


>------------------------
>
>Thanks,
>Vasundhara
>>
>> >
>> >Thanks,
>> >Vasundhara
>> >>
>> >>
>> >>
>> >> >+       system.
>> >> >diff --git a/include/net/devlink.h b/include/net/devlink.h
>> >> >index 8ffc1b5c..488b61c 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_ALLOW_FW_LIVE_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_ALLOW_FW_LIVE_RESET_NAME "allow_fw_live_reset"
>> >> >+#define DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_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..8544f23 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_ALLOW_FW_LIVE_RESET,
>> >> >+              .name = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_NAME,
>> >> >+              .type = DEVLINK_PARAM_GENERIC_ALLOW_FW_LIVE_RESET_TYPE,
>> >> >+      },
>> >> > };
>> >> >
>> >> > static int devlink_param_generic_verify(const struct devlink_param *param)
>> >> >--
>> >> >1.8.3.1
>> >> >

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-23  6:08 [PATCH v2 net-next 0/4] bnxt_en: Add new "allow_fw_live_reset" generic devlink parameter Vasundhara Volam
2020-05-23  6:08 ` [PATCH v2 net-next 1/4] devlink: Add new "allow_fw_live_reset" generic device parameter Vasundhara Volam
2020-05-24  4:53   ` Jiri Pirko
2020-05-24  6:29     ` Vasundhara Volam
2020-05-25 17:26       ` Jiri Pirko
2020-05-26  4:28         ` Vasundhara Volam
2020-05-26  4:47           ` Jiri Pirko
2020-05-23  6:08 ` [PATCH v2 net-next 2/4] bnxt_en: Update firmware spec. to 1.10.1.40 Vasundhara Volam
2020-05-23  6:08 ` [PATCH v2 net-next 3/4] bnxt_en: Use allow_fw_live_reset generic devlink parameter Vasundhara Volam
2020-05-23  6:08 ` [PATCH v2 net-next 4/4] bnxt_en: Check if fw_live_reset is allowed before doing ETHTOOL_RESET Vasundhara Volam

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git