* [Intel-wired-lan] [PATCH net-next v5 1/5] ice: remove FW logging code
2023-01-11 19:19 [Intel-wired-lan] [PATCH net-next v5 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
@ 2023-01-11 19:19 ` Paul M Stillwell Jr
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 2/5] ice: enable devlink to check FW logging status Paul M Stillwell Jr
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Paul M Stillwell Jr @ 2023-01-11 19:19 UTC (permalink / raw)
To: intel-wired-lan
The FW logging code doesn't work because there is no way to set
cq_ena or uart_ena so remove the code. This code is the original
(v1) way of FW logging so it should be replaced with the v2 way.
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
---
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 78 -------
drivers/net/ethernet/intel/ice/ice_common.c | 218 ------------------
drivers/net/ethernet/intel/ice/ice_common.h | 1 -
drivers/net/ethernet/intel/ice/ice_main.c | 3 -
drivers/net/ethernet/intel/ice/ice_type.h | 20 --
5 files changed, 320 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 838d9b274d68..e1b95cc3a538 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -1953,78 +1953,6 @@ struct ice_aqc_add_rdma_qset_data {
struct ice_aqc_add_tx_rdma_qset_entry rdma_qsets[];
};
-/* Configure Firmware Logging Command (indirect 0xFF09)
- * Logging Information Read Response (indirect 0xFF10)
- * Note: The 0xFF10 command has no input parameters.
- */
-struct ice_aqc_fw_logging {
- u8 log_ctrl;
-#define ICE_AQC_FW_LOG_AQ_EN BIT(0)
-#define ICE_AQC_FW_LOG_UART_EN BIT(1)
- u8 rsvd0;
- u8 log_ctrl_valid; /* Not used by 0xFF10 Response */
-#define ICE_AQC_FW_LOG_AQ_VALID BIT(0)
-#define ICE_AQC_FW_LOG_UART_VALID BIT(1)
- u8 rsvd1[5];
- __le32 addr_high;
- __le32 addr_low;
-};
-
-enum ice_aqc_fw_logging_mod {
- ICE_AQC_FW_LOG_ID_GENERAL = 0,
- ICE_AQC_FW_LOG_ID_CTRL,
- ICE_AQC_FW_LOG_ID_LINK,
- ICE_AQC_FW_LOG_ID_LINK_TOPO,
- ICE_AQC_FW_LOG_ID_DNL,
- ICE_AQC_FW_LOG_ID_I2C,
- ICE_AQC_FW_LOG_ID_SDP,
- ICE_AQC_FW_LOG_ID_MDIO,
- ICE_AQC_FW_LOG_ID_ADMINQ,
- ICE_AQC_FW_LOG_ID_HDMA,
- ICE_AQC_FW_LOG_ID_LLDP,
- ICE_AQC_FW_LOG_ID_DCBX,
- ICE_AQC_FW_LOG_ID_DCB,
- ICE_AQC_FW_LOG_ID_NETPROXY,
- ICE_AQC_FW_LOG_ID_NVM,
- ICE_AQC_FW_LOG_ID_AUTH,
- ICE_AQC_FW_LOG_ID_VPD,
- ICE_AQC_FW_LOG_ID_IOSF,
- ICE_AQC_FW_LOG_ID_PARSER,
- ICE_AQC_FW_LOG_ID_SW,
- ICE_AQC_FW_LOG_ID_SCHEDULER,
- ICE_AQC_FW_LOG_ID_TXQ,
- ICE_AQC_FW_LOG_ID_RSVD,
- ICE_AQC_FW_LOG_ID_POST,
- ICE_AQC_FW_LOG_ID_WATCHDOG,
- ICE_AQC_FW_LOG_ID_TASK_DISPATCH,
- ICE_AQC_FW_LOG_ID_MNG,
- ICE_AQC_FW_LOG_ID_MAX,
-};
-
-/* Defines for both above FW logging command/response buffers */
-#define ICE_AQC_FW_LOG_ID_S 0
-#define ICE_AQC_FW_LOG_ID_M (0xFFF << ICE_AQC_FW_LOG_ID_S)
-
-#define ICE_AQC_FW_LOG_CONF_SUCCESS 0 /* Used by response */
-#define ICE_AQC_FW_LOG_CONF_BAD_INDX BIT(12) /* Used by response */
-
-#define ICE_AQC_FW_LOG_EN_S 12
-#define ICE_AQC_FW_LOG_EN_M (0xF << ICE_AQC_FW_LOG_EN_S)
-#define ICE_AQC_FW_LOG_INFO_EN BIT(12) /* Used by command */
-#define ICE_AQC_FW_LOG_INIT_EN BIT(13) /* Used by command */
-#define ICE_AQC_FW_LOG_FLOW_EN BIT(14) /* Used by command */
-#define ICE_AQC_FW_LOG_ERR_EN BIT(15) /* Used by command */
-
-/* Get/Clear FW Log (indirect 0xFF11) */
-struct ice_aqc_get_clear_fw_log {
- u8 flags;
-#define ICE_AQC_FW_LOG_CLEAR BIT(0)
-#define ICE_AQC_FW_LOG_MORE_DATA_AVAIL BIT(1)
- u8 rsvd1[7];
- __le32 addr_high;
- __le32 addr_low;
-};
-
/* Download Package (indirect 0x0C40) */
/* Also used for Update Package (indirect 0x0C41 and 0x0C42) */
struct ice_aqc_download_pkg {
@@ -2184,8 +2112,6 @@ struct ice_aq_desc {
struct ice_aqc_add_rdma_qset add_rdma_qset;
struct ice_aqc_add_get_update_free_vsi vsi_cmd;
struct ice_aqc_add_update_free_vsi_resp add_update_free_vsi_res;
- struct ice_aqc_fw_logging fw_logging;
- struct ice_aqc_get_clear_fw_log get_clear_fw_log;
struct ice_aqc_download_pkg download_pkg;
struct ice_aqc_driver_shared_params drv_shared_params;
struct ice_aqc_set_mac_lb set_mac_lb;
@@ -2368,10 +2294,6 @@ enum ice_adminq_opc {
/* Standalone Commands/Events */
ice_aqc_opc_event_lan_overflow = 0x1001,
-
- /* debug commands */
- ice_aqc_opc_fw_logging = 0xFF09,
- ice_aqc_opc_fw_logging_info = 0xFF10,
};
#endif /* _ICE_ADMINQ_CMD_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index c2fda4fa4188..39ed1c6877f4 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -822,217 +822,6 @@ static void ice_cleanup_fltr_mgmt_struct(struct ice_hw *hw)
devm_kfree(ice_hw_to_dev(hw), sw);
}
-/**
- * ice_get_fw_log_cfg - get FW logging configuration
- * @hw: pointer to the HW struct
- */
-static int ice_get_fw_log_cfg(struct ice_hw *hw)
-{
- struct ice_aq_desc desc;
- __le16 *config;
- int status;
- u16 size;
-
- size = sizeof(*config) * ICE_AQC_FW_LOG_ID_MAX;
- config = devm_kzalloc(ice_hw_to_dev(hw), size, GFP_KERNEL);
- if (!config)
- return -ENOMEM;
-
- ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_fw_logging_info);
-
- status = ice_aq_send_cmd(hw, &desc, config, size, NULL);
- if (!status) {
- u16 i;
-
- /* Save FW logging information into the HW structure */
- for (i = 0; i < ICE_AQC_FW_LOG_ID_MAX; i++) {
- u16 v, m, flgs;
-
- v = le16_to_cpu(config[i]);
- m = (v & ICE_AQC_FW_LOG_ID_M) >> ICE_AQC_FW_LOG_ID_S;
- flgs = (v & ICE_AQC_FW_LOG_EN_M) >> ICE_AQC_FW_LOG_EN_S;
-
- if (m < ICE_AQC_FW_LOG_ID_MAX)
- hw->fw_log.evnts[m].cur = flgs;
- }
- }
-
- devm_kfree(ice_hw_to_dev(hw), config);
-
- return status;
-}
-
-/**
- * ice_cfg_fw_log - configure FW logging
- * @hw: pointer to the HW struct
- * @enable: enable certain FW logging events if true, disable all if false
- *
- * This function enables/disables the FW logging via Rx CQ events and a UART
- * port based on predetermined configurations. FW logging via the Rx CQ can be
- * enabled/disabled for individual PF's. However, FW logging via the UART can
- * only be enabled/disabled for all PFs on the same device.
- *
- * To enable overall FW logging, the "cq_en" and "uart_en" enable bits in
- * hw->fw_log need to be set accordingly, e.g. based on user-provided input,
- * before initializing the device.
- *
- * When re/configuring FW logging, callers need to update the "cfg" elements of
- * the hw->fw_log.evnts array with the desired logging event configurations for
- * modules of interest. When disabling FW logging completely, the callers can
- * just pass false in the "enable" parameter. On completion, the function will
- * update the "cur" element of the hw->fw_log.evnts array with the resulting
- * logging event configurations of the modules that are being re/configured. FW
- * logging modules that are not part of a reconfiguration operation retain their
- * previous states.
- *
- * Before resetting the device, it is recommended that the driver disables FW
- * logging before shutting down the control queue. When disabling FW logging
- * ("enable" = false), the latest configurations of FW logging events stored in
- * hw->fw_log.evnts[] are not overridden to allow them to be reconfigured after
- * a device reset.
- *
- * When enabling FW logging to emit log messages via the Rx CQ during the
- * device's initialization phase, a mechanism alternative to interrupt handlers
- * needs to be used to extract FW log messages from the Rx CQ periodically and
- * to prevent the Rx CQ from being full and stalling other types of control
- * messages from FW to SW. Interrupts are typically disabled during the device's
- * initialization phase.
- */
-static int ice_cfg_fw_log(struct ice_hw *hw, bool enable)
-{
- struct ice_aqc_fw_logging *cmd;
- u16 i, chgs = 0, len = 0;
- struct ice_aq_desc desc;
- __le16 *data = NULL;
- u8 actv_evnts = 0;
- void *buf = NULL;
- int status = 0;
-
- if (!hw->fw_log.cq_en && !hw->fw_log.uart_en)
- return 0;
-
- /* Disable FW logging only when the control queue is still responsive */
- if (!enable &&
- (!hw->fw_log.actv_evnts || !ice_check_sq_alive(hw, &hw->adminq)))
- return 0;
-
- /* Get current FW log settings */
- status = ice_get_fw_log_cfg(hw);
- if (status)
- return status;
-
- ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_fw_logging);
- cmd = &desc.params.fw_logging;
-
- /* Indicate which controls are valid */
- if (hw->fw_log.cq_en)
- cmd->log_ctrl_valid |= ICE_AQC_FW_LOG_AQ_VALID;
-
- if (hw->fw_log.uart_en)
- cmd->log_ctrl_valid |= ICE_AQC_FW_LOG_UART_VALID;
-
- if (enable) {
- /* Fill in an array of entries with FW logging modules and
- * logging events being reconfigured.
- */
- for (i = 0; i < ICE_AQC_FW_LOG_ID_MAX; i++) {
- u16 val;
-
- /* Keep track of enabled event types */
- actv_evnts |= hw->fw_log.evnts[i].cfg;
-
- if (hw->fw_log.evnts[i].cfg == hw->fw_log.evnts[i].cur)
- continue;
-
- if (!data) {
- data = devm_kcalloc(ice_hw_to_dev(hw),
- ICE_AQC_FW_LOG_ID_MAX,
- sizeof(*data),
- GFP_KERNEL);
- if (!data)
- return -ENOMEM;
- }
-
- val = i << ICE_AQC_FW_LOG_ID_S;
- val |= hw->fw_log.evnts[i].cfg << ICE_AQC_FW_LOG_EN_S;
- data[chgs++] = cpu_to_le16(val);
- }
-
- /* Only enable FW logging if at least one module is specified.
- * If FW logging is currently enabled but all modules are not
- * enabled to emit log messages, disable FW logging altogether.
- */
- if (actv_evnts) {
- /* Leave if there is effectively no change */
- if (!chgs)
- goto out;
-
- if (hw->fw_log.cq_en)
- cmd->log_ctrl |= ICE_AQC_FW_LOG_AQ_EN;
-
- if (hw->fw_log.uart_en)
- cmd->log_ctrl |= ICE_AQC_FW_LOG_UART_EN;
-
- buf = data;
- len = sizeof(*data) * chgs;
- desc.flags |= cpu_to_le16(ICE_AQ_FLAG_RD);
- }
- }
-
- status = ice_aq_send_cmd(hw, &desc, buf, len, NULL);
- if (!status) {
- /* Update the current configuration to reflect events enabled.
- * hw->fw_log.cq_en and hw->fw_log.uart_en indicate if the FW
- * logging mode is enabled for the device. They do not reflect
- * actual modules being enabled to emit log messages. So, their
- * values remain unchanged even when all modules are disabled.
- */
- u16 cnt = enable ? chgs : (u16)ICE_AQC_FW_LOG_ID_MAX;
-
- hw->fw_log.actv_evnts = actv_evnts;
- for (i = 0; i < cnt; i++) {
- u16 v, m;
-
- if (!enable) {
- /* When disabling all FW logging events as part
- * of device's de-initialization, the original
- * configurations are retained, and can be used
- * to reconfigure FW logging later if the device
- * is re-initialized.
- */
- hw->fw_log.evnts[i].cur = 0;
- continue;
- }
-
- v = le16_to_cpu(data[i]);
- m = (v & ICE_AQC_FW_LOG_ID_M) >> ICE_AQC_FW_LOG_ID_S;
- hw->fw_log.evnts[m].cur = hw->fw_log.evnts[m].cfg;
- }
- }
-
-out:
- if (data)
- devm_kfree(ice_hw_to_dev(hw), data);
-
- return status;
-}
-
-/**
- * ice_output_fw_log
- * @hw: pointer to the HW struct
- * @desc: pointer to the AQ message descriptor
- * @buf: pointer to the buffer accompanying the AQ message
- *
- * Formats a FW Log message and outputs it via the standard driver logs.
- */
-void ice_output_fw_log(struct ice_hw *hw, struct ice_aq_desc *desc, void *buf)
-{
- ice_debug(hw, ICE_DBG_FW_LOG, "[ FW Log Msg Start ]\n");
- ice_debug_array(hw, ICE_DBG_FW_LOG, 16, 1, (u8 *)buf,
- le16_to_cpu(desc->datalen));
- ice_debug(hw, ICE_DBG_FW_LOG, "[ FW Log Msg End ]\n");
-}
-
/**
* ice_get_itr_intrl_gran
* @hw: pointer to the HW struct
@@ -1090,11 +879,6 @@ int ice_init_hw(struct ice_hw *hw)
if (status)
goto err_unroll_cqinit;
- /* Enable FW logging. Not fatal if this fails. */
- status = ice_cfg_fw_log(hw, true);
- if (status)
- ice_debug(hw, ICE_DBG_INIT, "Failed to enable FW logging.\n");
-
status = ice_clear_pf_cfg(hw);
if (status)
goto err_unroll_cqinit;
@@ -1244,8 +1028,6 @@ void ice_deinit_hw(struct ice_hw *hw)
ice_free_hw_tbls(hw);
mutex_destroy(&hw->tnl_lock);
- /* Attempt to disable FW logging before shutting down control queues */
- ice_cfg_fw_log(hw, false);
ice_destroy_all_ctrlq(hw);
/* Clear VSI contexts if not already cleared */
diff --git a/drivers/net/ethernet/intel/ice/ice_common.h b/drivers/net/ethernet/intel/ice/ice_common.h
index 8ba5f935a092..831212824d84 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.h
+++ b/drivers/net/ethernet/intel/ice/ice_common.h
@@ -188,7 +188,6 @@ ice_ena_vsi_txq(struct ice_port_info *pi, u16 vsi_handle, u8 tc, u16 q_handle,
struct ice_sq_cd *cd);
int ice_replay_vsi(struct ice_hw *hw, u16 vsi_handle);
void ice_replay_post(struct ice_hw *hw);
-void ice_output_fw_log(struct ice_hw *hw, struct ice_aq_desc *desc, void *buf);
struct ice_q_ctx *
ice_get_lan_q_ctx(struct ice_hw *hw, u16 vsi_handle, u8 tc, u16 q_handle);
int ice_sbq_rw_reg(struct ice_hw *hw, struct ice_sbq_msg_input *in);
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 59d4b2781ebc..1b5debc3109d 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1485,9 +1485,6 @@ static int __ice_clean_ctrlq(struct ice_pf *pf, enum ice_ctl_q q_type)
if (!ice_is_malicious_vf(pf, &event, i, pending))
ice_vc_process_vf_msg(pf, &event);
break;
- case ice_aqc_opc_fw_logging:
- ice_output_fw_log(hw, &event.desc, event.msg_buf);
- break;
case ice_aqc_opc_lldp_set_mib_change:
ice_dcb_process_lldp_set_mib_change(pf, &event);
break;
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index e3f622cad425..126605b7eb3b 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -727,24 +727,6 @@ struct ice_switch_info {
DECLARE_BITMAP(prof_res_bm[ICE_MAX_NUM_PROFILES], ICE_MAX_FV_WORDS);
};
-/* FW logging configuration */
-struct ice_fw_log_evnt {
- u8 cfg : 4; /* New event enables to configure */
- u8 cur : 4; /* Current/active event enables */
-};
-
-struct ice_fw_log_cfg {
- u8 cq_en : 1; /* FW logging is enabled via the control queue */
- u8 uart_en : 1; /* FW logging is enabled via UART for all PFs */
- u8 actv_evnts; /* Cumulation of currently enabled log events */
-
-#define ICE_FW_LOG_EVNT_INFO (ICE_AQC_FW_LOG_INFO_EN >> ICE_AQC_FW_LOG_EN_S)
-#define ICE_FW_LOG_EVNT_INIT (ICE_AQC_FW_LOG_INIT_EN >> ICE_AQC_FW_LOG_EN_S)
-#define ICE_FW_LOG_EVNT_FLOW (ICE_AQC_FW_LOG_FLOW_EN >> ICE_AQC_FW_LOG_EN_S)
-#define ICE_FW_LOG_EVNT_ERR (ICE_AQC_FW_LOG_ERR_EN >> ICE_AQC_FW_LOG_EN_S)
- struct ice_fw_log_evnt evnts[ICE_AQC_FW_LOG_ID_MAX];
-};
-
/* Enum defining the different states of the mailbox snapshot in the
* PF-VF mailbox overflow detection algorithm. The snapshot can be in
* states:
@@ -877,8 +859,6 @@ struct ice_hw {
u8 fw_patch; /* firmware patch version */
u32 fw_build; /* firmware build number */
- struct ice_fw_log_cfg fw_log;
-
/* Device max aggregate bandwidths corresponding to the GL_PWR_MODE_CTL
* register. Used for determining the ITR/INTRL granularity during
* initialization.
--
2.35.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Intel-wired-lan] [PATCH net-next v5 2/5] ice: enable devlink to check FW logging status
2023-01-11 19:19 [Intel-wired-lan] [PATCH net-next v5 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 1/5] ice: remove FW logging code Paul M Stillwell Jr
@ 2023-01-11 19:19 ` Paul M Stillwell Jr
2023-01-12 9:38 ` Michal Swiatkowski
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 3/5] ice: add ability to query/set FW log level and resolution Paul M Stillwell Jr
` (2 subsequent siblings)
4 siblings, 1 reply; 14+ messages in thread
From: Paul M Stillwell Jr @ 2023-01-11 19:19 UTC (permalink / raw)
To: intel-wired-lan
Users want the ability to debug FW issues by retrieving the
FW logs from the E8xx devices. Enable devlink to query the driver
to see if the NVM image allows FW logging and to see if FW
logging is currently running. The set command is not supported
at this time.
This is the beginning of the v2 for FW logging.
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
pick bfdfb2dc6192 ice: add ability to query/set FW log level and resolution
---
drivers/net/ethernet/intel/ice/Makefile | 3 +-
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 81 ++++++++++++
drivers/net/ethernet/intel/ice/ice_common.c | 2 +
drivers/net/ethernet/intel/ice/ice_devlink.c | 73 ++++++++++-
drivers/net/ethernet/intel/ice/ice_fwlog.c | 118 ++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_fwlog.h | 52 ++++++++
drivers/net/ethernet/intel/ice/ice_type.h | 4 +
7 files changed, 331 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.c
create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.h
diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile
index f269952d207d..6e4680ad097c 100644
--- a/drivers/net/ethernet/intel/ice/Makefile
+++ b/drivers/net/ethernet/intel/ice/Makefile
@@ -33,7 +33,8 @@ ice-y := ice_main.o \
ice_lag.o \
ice_ethtool.o \
ice_repr.o \
- ice_tc_lib.o
+ ice_tc_lib.o \
+ ice_fwlog.o
ice-$(CONFIG_PCI_IOV) += \
ice_sriov.o \
ice_virtchnl.o \
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index e1b95cc3a538..0d560287ec23 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -2038,6 +2038,83 @@ struct ice_aqc_event_lan_overflow {
u8 reserved[8];
};
+enum ice_aqc_fw_logging_mod {
+ ICE_AQC_FW_LOG_ID_GENERAL = 0,
+ ICE_AQC_FW_LOG_ID_CTRL,
+ ICE_AQC_FW_LOG_ID_LINK,
+ ICE_AQC_FW_LOG_ID_LINK_TOPO,
+ ICE_AQC_FW_LOG_ID_DNL,
+ ICE_AQC_FW_LOG_ID_I2C,
+ ICE_AQC_FW_LOG_ID_SDP,
+ ICE_AQC_FW_LOG_ID_MDIO,
+ ICE_AQC_FW_LOG_ID_ADMINQ,
+ ICE_AQC_FW_LOG_ID_HDMA,
+ ICE_AQC_FW_LOG_ID_LLDP,
+ ICE_AQC_FW_LOG_ID_DCBX,
+ ICE_AQC_FW_LOG_ID_DCB,
+ ICE_AQC_FW_LOG_ID_XLR,
+ ICE_AQC_FW_LOG_ID_NVM,
+ ICE_AQC_FW_LOG_ID_AUTH,
+ ICE_AQC_FW_LOG_ID_VPD,
+ ICE_AQC_FW_LOG_ID_IOSF,
+ ICE_AQC_FW_LOG_ID_PARSER,
+ ICE_AQC_FW_LOG_ID_SW,
+ ICE_AQC_FW_LOG_ID_SCHEDULER,
+ ICE_AQC_FW_LOG_ID_TXQ,
+ ICE_AQC_FW_LOG_ID_RSVD,
+ ICE_AQC_FW_LOG_ID_POST,
+ ICE_AQC_FW_LOG_ID_WATCHDOG,
+ ICE_AQC_FW_LOG_ID_TASK_DISPATCH,
+ ICE_AQC_FW_LOG_ID_MNG,
+ ICE_AQC_FW_LOG_ID_SYNCE,
+ ICE_AQC_FW_LOG_ID_HEALTH,
+ ICE_AQC_FW_LOG_ID_TSDRV,
+ ICE_AQC_FW_LOG_ID_PFREG,
+ ICE_AQC_FW_LOG_ID_MDLVER,
+ ICE_AQC_FW_LOG_ID_MAX,
+};
+
+/* Set FW Logging configuration (indirect 0xFF30)
+ * Register for FW Logging (indirect 0xFF31)
+ * Query FW Logging (indirect 0xFF32)
+ */
+struct ice_aqc_fw_log {
+ u8 cmd_flags;
+#define ICE_AQC_FW_LOG_CONF_UART_EN BIT(0)
+#define ICE_AQC_FW_LOG_CONF_AQ_EN BIT(1)
+#define ICE_AQC_FW_LOG_QUERY_REGISTERED BIT(2)
+#define ICE_AQC_FW_LOG_CONF_SET_VALID BIT(3)
+#define ICE_AQC_FW_LOG_AQ_REGISTER BIT(0)
+#define ICE_AQC_FW_LOG_AQ_QUERY BIT(2)
+
+ u8 rsp_flag;
+ __le16 fw_rt_msb;
+ union {
+ struct {
+ __le32 fw_rt_lsb;
+ } sync;
+ struct {
+ __le16 log_resolution;
+#define ICE_AQC_FW_LOG_MIN_RESOLUTION (1)
+#define ICE_AQC_FW_LOG_MAX_RESOLUTION (128)
+
+ __le16 mdl_cnt;
+ } cfg;
+ } ops;
+ __le32 addr_high;
+ __le32 addr_low;
+};
+
+/* Response Buffer for:
+ * Set Firmware Logging Configuration (0xFF30)
+ * Query FW Logging (0xFF32)
+ */
+struct ice_aqc_fw_log_cfg_resp {
+ __le16 module_identifier;
+ u8 log_level;
+ u8 rsvd0;
+};
+
/**
* struct ice_aq_desc - Admin Queue (AQ) descriptor
* @flags: ICE_AQ_FLAG_* flags
@@ -2114,6 +2191,7 @@ struct ice_aq_desc {
struct ice_aqc_add_update_free_vsi_resp add_update_free_vsi_res;
struct ice_aqc_download_pkg download_pkg;
struct ice_aqc_driver_shared_params drv_shared_params;
+ struct ice_aqc_fw_log fw_log;
struct ice_aqc_set_mac_lb set_mac_lb;
struct ice_aqc_alloc_free_res_cmd sw_res_ctrl;
struct ice_aqc_set_mac_cfg set_mac_cfg;
@@ -2294,6 +2372,9 @@ enum ice_adminq_opc {
/* Standalone Commands/Events */
ice_aqc_opc_event_lan_overflow = 0x1001,
+
+ /* FW Logging Commands */
+ ice_aqc_opc_fw_logs_query = 0xFF32,
};
#endif /* _ICE_ADMINQ_CMD_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index 39ed1c6877f4..ef7e5d7c18f2 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -879,6 +879,8 @@ int ice_init_hw(struct ice_hw *hw)
if (status)
goto err_unroll_cqinit;
+ ice_fwlog_set_supported(hw);
+
status = ice_clear_pf_cfg(hw);
if (status)
goto err_unroll_cqinit;
diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index 24023852a1d7..0e76fd19b61a 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -369,6 +369,13 @@ static int ice_devlink_info_get(struct devlink *devlink,
return err;
}
+enum ice_devlink_param_id {
+ ICE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
+ ICE_DEVLINK_PARAM_ID_TX_BALANCE,
+ ICE_DEVLINK_PARAM_ID_FWLOG_SUPPORTED,
+ ICE_DEVLINK_PARAM_ID_FWLOG_ENABLED,
+};
+
/**
* ice_devlink_reload_empr_start - Start EMP reset to activate new firmware
* @pf: pointer to the pf instance
@@ -1383,6 +1390,50 @@ ice_devlink_enable_iw_validate(struct devlink *devlink, u32 id,
return 0;
}
+static int
+ice_devlink_fwlog_supported_get(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct ice_pf *pf = devlink_priv(devlink);
+
+ /* only support fw log commands on PF 0 */
+ if (pf->hw.bus.func)
+ return -EOPNOTSUPP;
+
+ ctx->val.vbool = ice_fwlog_supported(&pf->hw);
+ return 0;
+}
+
+static int
+ice_devlink_fwlog_supported_set(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ /* set operation is unsupported */
+ return -EOPNOTSUPP;
+}
+
+static int
+ice_devlink_fwlog_enabled_get(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct ice_pf *pf = devlink_priv(devlink);
+
+ /* only support fw log commands on PF 0 */
+ if (pf->hw.bus.func)
+ return -EOPNOTSUPP;
+
+ ctx->val.vbool = pf->hw.fwlog_ena;
+ return 0;
+}
+
+static int
+ice_devlink_fwlog_enabled_set(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ /* set operation is unsupported at this time */
+ return -EOPNOTSUPP;
+}
+
static const struct devlink_param ice_devlink_params[] = {
DEVLINK_PARAM_GENERIC(ENABLE_ROCE, BIT(DEVLINK_PARAM_CMODE_RUNTIME),
ice_devlink_enable_roce_get,
@@ -1392,7 +1443,18 @@ static const struct devlink_param ice_devlink_params[] = {
ice_devlink_enable_iw_get,
ice_devlink_enable_iw_set,
ice_devlink_enable_iw_validate),
-
+ DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_FWLOG_SUPPORTED,
+ "fwlog_supported", DEVLINK_PARAM_TYPE_BOOL,
+ BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+ ice_devlink_fwlog_supported_get,
+ ice_devlink_fwlog_supported_set,
+ NULL),
+ DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_FWLOG_ENABLED,
+ "fwlog_enabled", DEVLINK_PARAM_TYPE_BOOL,
+ BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+ ice_devlink_fwlog_enabled_get,
+ ice_devlink_fwlog_enabled_set,
+ NULL),
};
static void ice_devlink_free(void *devlink_ptr)
@@ -1488,6 +1550,15 @@ int ice_devlink_register_params(struct ice_pf *pf)
DEVLINK_PARAM_GENERIC_ID_ENABLE_ROCE,
value);
+ value.vbool = false;
+ devlink_param_driverinit_value_set(devlink,
+ ICE_DEVLINK_PARAM_ID_FWLOG_SUPPORTED,
+ value);
+
+ devlink_param_driverinit_value_set(devlink,
+ ICE_DEVLINK_PARAM_ID_FWLOG_ENABLED,
+ value);
+
return 0;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.c b/drivers/net/ethernet/intel/ice/ice_fwlog.c
new file mode 100644
index 000000000000..efea71b6c9f8
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_fwlog.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022, Intel Corporation. */
+
+#include "ice_common.h"
+#include "ice_fwlog.h"
+
+/**
+ * ice_fwlog_supported - Cached for whether FW supports FW logging or not
+ * @hw: pointer to the HW structure
+ *
+ * This will always return false if called before ice_init_hw(), so it must be
+ * called after ice_init_hw().
+ */
+bool ice_fwlog_supported(struct ice_hw *hw)
+{
+ return hw->fwlog_supported;
+}
+
+/**
+ * ice_aq_fwlog_get - Get the current firmware logging configuration (0xFF32)
+ * @hw: pointer to the HW structure
+ * @cfg: firmware logging configuration to populate
+ */
+static int
+ice_aq_fwlog_get(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
+{
+ struct ice_aqc_fw_log_cfg_resp *fw_modules;
+ struct ice_aqc_fw_log *cmd;
+ struct ice_aq_desc desc;
+ u16 module_id_cnt;
+ int status;
+ void *buf;
+ int i;
+
+ memset(cfg, 0, sizeof(*cfg));
+
+ buf = kzalloc(ICE_AQ_MAX_BUF_LEN, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_fw_logs_query);
+ cmd = &desc.params.fw_log;
+
+ cmd->cmd_flags = ICE_AQC_FW_LOG_AQ_QUERY;
+
+ status = ice_aq_send_cmd(hw, &desc, buf, ICE_AQ_MAX_BUF_LEN, NULL);
+ if (status) {
+ ice_debug(hw, ICE_DBG_FW_LOG, "Failed to get FW log configuration\n");
+ goto status_out;
+ }
+
+ module_id_cnt = le16_to_cpu(cmd->ops.cfg.mdl_cnt);
+ if (module_id_cnt < ICE_AQC_FW_LOG_ID_MAX) {
+ ice_debug(hw, ICE_DBG_FW_LOG, "FW returned less than the expected number of FW log module IDs\n");
+ } else {
+ if (module_id_cnt > ICE_AQC_FW_LOG_ID_MAX)
+ ice_debug(hw, ICE_DBG_FW_LOG, "FW returned more than expected number of FW log module IDs, setting module_id_cnt to software expected max %u\n",
+ ICE_AQC_FW_LOG_ID_MAX);
+ module_id_cnt = ICE_AQC_FW_LOG_ID_MAX;
+ }
+
+ cfg->log_resolution = le16_to_cpu(cmd->ops.cfg.log_resolution);
+ if (cmd->cmd_flags & ICE_AQC_FW_LOG_CONF_AQ_EN)
+ cfg->options |= ICE_FWLOG_OPTION_ARQ_ENA;
+ if (cmd->cmd_flags & ICE_AQC_FW_LOG_CONF_UART_EN)
+ cfg->options |= ICE_FWLOG_OPTION_UART_ENA;
+ if (cmd->cmd_flags & ICE_AQC_FW_LOG_QUERY_REGISTERED)
+ cfg->options |= ICE_FWLOG_OPTION_IS_REGISTERED;
+
+ fw_modules = (struct ice_aqc_fw_log_cfg_resp *)buf;
+
+ for (i = 0; i < module_id_cnt; i++) {
+ struct ice_aqc_fw_log_cfg_resp *fw_module = &fw_modules[i];
+
+ cfg->module_entries[i].module_id =
+ le16_to_cpu(fw_module->module_identifier);
+ cfg->module_entries[i].log_level = fw_module->log_level;
+ }
+
+status_out:
+ kfree(buf);
+ return status;
+}
+
+/**
+ * ice_fwlog_set_supported - Set if FW logging is supported by FW
+ * @hw: pointer to the HW struct
+ *
+ * If FW returns success to the ice_aq_fwlog_get call then it supports FW
+ * logging, else it doesn't. Set the fwlog_supported flag accordingly.
+ *
+ * This function is only meant to be called during driver init to determine if
+ * the FW support FW logging.
+ */
+void ice_fwlog_set_supported(struct ice_hw *hw)
+{
+ struct ice_fwlog_cfg *cfg;
+ int status;
+
+ hw->fwlog_supported = false;
+
+ cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return;
+
+ /* don't call ice_fwlog_get() because that would overwrite the cached
+ * configuration from the call to ice_fwlog_init(), which is expected to
+ * be called prior to this function
+ */
+ status = ice_aq_fwlog_get(hw, cfg);
+ if (status)
+ ice_debug(hw, ICE_DBG_FW_LOG, "ice_aq_fwlog_get failed, FW logging is not supported on this version of FW, status %d\n",
+ status);
+ else
+ hw->fwlog_supported = true;
+
+ kfree(cfg);
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.h b/drivers/net/ethernet/intel/ice/ice_fwlog.h
new file mode 100644
index 000000000000..3a2c83502763
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_fwlog.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2022, Intel Corporation. */
+
+#ifndef _ICE_FWLOG_H_
+#define _ICE_FWLOG_H_
+#include "ice_adminq_cmd.h"
+
+struct ice_hw;
+
+/* Only a single log level should be set and all log levels under the set value
+ * are enabled, e.g. if log level is set to ICE_FW_LOG_LEVEL_VERBOSE, then all
+ * other log levels are included (except ICE_FW_LOG_LEVEL_NONE)
+ */
+enum ice_fwlog_level {
+ ICE_FWLOG_LEVEL_NONE = 0,
+ ICE_FWLOG_LEVEL_ERROR = 1,
+ ICE_FWLOG_LEVEL_WARNING = 2,
+ ICE_FWLOG_LEVEL_NORMAL = 3,
+ ICE_FWLOG_LEVEL_VERBOSE = 4,
+ ICE_FWLOG_LEVEL_INVALID, /* all values >= this entry are invalid */
+};
+
+struct ice_fwlog_module_entry {
+ /* module ID for the corresponding firmware logging event */
+ u16 module_id;
+ /* verbosity level for the module_id */
+ u8 log_level;
+};
+
+struct ice_fwlog_cfg {
+ /* list of modules for configuring log level */
+ struct ice_fwlog_module_entry module_entries[ICE_AQC_FW_LOG_ID_MAX];
+ /* options used to configure firmware logging */
+ u16 options;
+#define ICE_FWLOG_OPTION_ARQ_ENA BIT(0)
+#define ICE_FWLOG_OPTION_UART_ENA BIT(1)
+ /* set before calling ice_fwlog_init() so the PF registers for firmware
+ * logging on initialization
+ */
+#define ICE_FWLOG_OPTION_REGISTER_ON_INIT BIT(2)
+ /* set in the ice_fwlog_get() response if the PF is registered for FW
+ * logging events over ARQ
+ */
+#define ICE_FWLOG_OPTION_IS_REGISTERED BIT(3)
+
+ /* minimum number of log events sent per Admin Receive Queue event */
+ u16 log_resolution;
+};
+
+void ice_fwlog_set_supported(struct ice_hw *hw);
+bool ice_fwlog_supported(struct ice_hw *hw);
+#endif /* _ICE_FWLOG_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 126605b7eb3b..1284fe8d78f2 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -17,6 +17,7 @@
#include "ice_protocol_type.h"
#include "ice_sbq_cmd.h"
#include "ice_vlan_mode.h"
+#include "ice_fwlog.h"
static inline bool ice_is_tc_ena(unsigned long bitmap, u8 tc)
{
@@ -859,6 +860,9 @@ struct ice_hw {
u8 fw_patch; /* firmware patch version */
u32 fw_build; /* firmware build number */
+ bool fwlog_supported; /* does hardware support FW logging? */
+ bool fwlog_ena; /* currently logging? */
+
/* Device max aggregate bandwidths corresponding to the GL_PWR_MODE_CTL
* register. Used for determining the ITR/INTRL granularity during
* initialization.
--
2.35.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next v5 2/5] ice: enable devlink to check FW logging status
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 2/5] ice: enable devlink to check FW logging status Paul M Stillwell Jr
@ 2023-01-12 9:38 ` Michal Swiatkowski
2023-01-12 22:53 ` Paul M Stillwell Jr
0 siblings, 1 reply; 14+ messages in thread
From: Michal Swiatkowski @ 2023-01-12 9:38 UTC (permalink / raw)
To: Paul M Stillwell Jr; +Cc: intel-wired-lan
On Wed, Jan 11, 2023 at 11:19:03AM -0800, Paul M Stillwell Jr wrote:
> Users want the ability to debug FW issues by retrieving the
> FW logs from the E8xx devices. Enable devlink to query the driver
> to see if the NVM image allows FW logging and to see if FW
> logging is currently running. The set command is not supported
> at this time.
>
> This is the beginning of the v2 for FW logging.
>
> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
>
> pick bfdfb2dc6192 ice: add ability to query/set FW log level and resolution
> ---
> drivers/net/ethernet/intel/ice/Makefile | 3 +-
> .../net/ethernet/intel/ice/ice_adminq_cmd.h | 81 ++++++++++++
> drivers/net/ethernet/intel/ice/ice_common.c | 2 +
> drivers/net/ethernet/intel/ice/ice_devlink.c | 73 ++++++++++-
> drivers/net/ethernet/intel/ice/ice_fwlog.c | 118 ++++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_fwlog.h | 52 ++++++++
> drivers/net/ethernet/intel/ice/ice_type.h | 4 +
> 7 files changed, 331 insertions(+), 2 deletions(-)
> create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.c
> create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.h
>
Hi,
Great changes, does it mean that after applying the patchset we will be
able to for example dump whole switch block?
Looks good, only cosmetic comments.
[...]
> +
> +/**
> + * ice_aq_fwlog_get - Get the current firmware logging configuration (0xFF32)
> + * @hw: pointer to the HW structure
> + * @cfg: firmware logging configuration to populate
> + */
> +static int
> +ice_aq_fwlog_get(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
> +{
> + struct ice_aqc_fw_log_cfg_resp *fw_modules;
> + struct ice_aqc_fw_log *cmd;
> + struct ice_aq_desc desc;
> + u16 module_id_cnt;
> + int status;
> + void *buf;
> + int i;
> +
> + memset(cfg, 0, sizeof(*cfg));
> +
> + buf = kzalloc(ICE_AQ_MAX_BUF_LEN, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_fw_logs_query);
> + cmd = &desc.params.fw_log;
> +
> + cmd->cmd_flags = ICE_AQC_FW_LOG_AQ_QUERY;
> +
> + status = ice_aq_send_cmd(hw, &desc, buf, ICE_AQ_MAX_BUF_LEN, NULL);
> + if (status) {
> + ice_debug(hw, ICE_DBG_FW_LOG, "Failed to get FW log configuration\n");
> + goto status_out;
> + }
> +
> + module_id_cnt = le16_to_cpu(cmd->ops.cfg.mdl_cnt);
> + if (module_id_cnt < ICE_AQC_FW_LOG_ID_MAX) {
> + ice_debug(hw, ICE_DBG_FW_LOG, "FW returned less than the expected number of FW log module IDs\n");
> + } else {
> + if (module_id_cnt > ICE_AQC_FW_LOG_ID_MAX)
> + ice_debug(hw, ICE_DBG_FW_LOG, "FW returned more than expected number of FW log module IDs, setting module_id_cnt to software expected max %u\n",
> + ICE_AQC_FW_LOG_ID_MAX);
> + module_id_cnt = ICE_AQC_FW_LOG_ID_MAX;
Maybe:
else if (module_id_cnt > ICE_AQC_FW_LOG_ID_MAX) {
ice_debug();
module_id_cnt = ICE_AQC_FW_LOG_ID_MAX;
}
We will save one indent, but it is cosmetic, so feel free to ignore.
> + }
> +
> + cfg->log_resolution = le16_to_cpu(cmd->ops.cfg.log_resolution);
> + if (cmd->cmd_flags & ICE_AQC_FW_LOG_CONF_AQ_EN)
> + cfg->options |= ICE_FWLOG_OPTION_ARQ_ENA;
> + if (cmd->cmd_flags & ICE_AQC_FW_LOG_CONF_UART_EN)
> + cfg->options |= ICE_FWLOG_OPTION_UART_ENA;
> + if (cmd->cmd_flags & ICE_AQC_FW_LOG_QUERY_REGISTERED)
> + cfg->options |= ICE_FWLOG_OPTION_IS_REGISTERED;
> +
> + fw_modules = (struct ice_aqc_fw_log_cfg_resp *)buf;
> +
> + for (i = 0; i < module_id_cnt; i++) {
> + struct ice_aqc_fw_log_cfg_resp *fw_module = &fw_modules[i];
> +
> + cfg->module_entries[i].module_id =
> + le16_to_cpu(fw_module->module_identifier);
> + cfg->module_entries[i].log_level = fw_module->log_level;
> + }
> +
> +status_out:
> + kfree(buf);
> + return status;
> +}
> +
> +/**
> + * ice_fwlog_set_supported - Set if FW logging is supported by FW
> + * @hw: pointer to the HW struct
> + *
> + * If FW returns success to the ice_aq_fwlog_get call then it supports FW
> + * logging, else it doesn't. Set the fwlog_supported flag accordingly.
> + *
> + * This function is only meant to be called during driver init to determine if
> + * the FW support FW logging.
> + */
> +void ice_fwlog_set_supported(struct ice_hw *hw)
> +{
> + struct ice_fwlog_cfg *cfg;
> + int status;
> +
> + hw->fwlog_supported = false;
> +
> + cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> + if (!cfg)
> + return;
> +
> + /* don't call ice_fwlog_get() because that would overwrite the cached
> + * configuration from the call to ice_fwlog_init(), which is expected to
> + * be called prior to this function
> + */
> + status = ice_aq_fwlog_get(hw, cfg);
> + if (status)
> + ice_debug(hw, ICE_DBG_FW_LOG, "ice_aq_fwlog_get failed, FW logging is not supported on this version of FW, status %d\n",
> + status);
> + else
> + hw->fwlog_supported = true;
> +
> + kfree(cfg);
> +}
> diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.h b/drivers/net/ethernet/intel/ice/ice_fwlog.h
> new file mode 100644
> index 000000000000..3a2c83502763
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/ice/ice_fwlog.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2022, Intel Corporation. */
> +
> +#ifndef _ICE_FWLOG_H_
> +#define _ICE_FWLOG_H_
> +#include "ice_adminq_cmd.h"
> +
> +struct ice_hw;
> +
> +/* Only a single log level should be set and all log levels under the set value
> + * are enabled, e.g. if log level is set to ICE_FW_LOG_LEVEL_VERBOSE, then all
> + * other log levels are included (except ICE_FW_LOG_LEVEL_NONE)
> + */
> +enum ice_fwlog_level {
> + ICE_FWLOG_LEVEL_NONE = 0,
> + ICE_FWLOG_LEVEL_ERROR = 1,
> + ICE_FWLOG_LEVEL_WARNING = 2,
> + ICE_FWLOG_LEVEL_NORMAL = 3,
> + ICE_FWLOG_LEVEL_VERBOSE = 4,
> + ICE_FWLOG_LEVEL_INVALID, /* all values >= this entry are invalid */
> +};
> +
> +struct ice_fwlog_module_entry {
> + /* module ID for the corresponding firmware logging event */
> + u16 module_id;
> + /* verbosity level for the module_id */
> + u8 log_level;
> +};
> +
> +struct ice_fwlog_cfg {
> + /* list of modules for configuring log level */
> + struct ice_fwlog_module_entry module_entries[ICE_AQC_FW_LOG_ID_MAX];
> + /* options used to configure firmware logging */
> + u16 options;
> +#define ICE_FWLOG_OPTION_ARQ_ENA BIT(0)
> +#define ICE_FWLOG_OPTION_UART_ENA BIT(1)
> + /* set before calling ice_fwlog_init() so the PF registers for firmware
> + * logging on initialization
> + */
> +#define ICE_FWLOG_OPTION_REGISTER_ON_INIT BIT(2)
> + /* set in the ice_fwlog_get() response if the PF is registered for FW
> + * logging events over ARQ
> + */
> +#define ICE_FWLOG_OPTION_IS_REGISTERED BIT(3)
> +
> + /* minimum number of log events sent per Admin Receive Queue event */
> + u16 log_resolution;
> +};
> +
> +void ice_fwlog_set_supported(struct ice_hw *hw);
> +bool ice_fwlog_supported(struct ice_hw *hw);
> +#endif /* _ICE_FWLOG_H_ */
> diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
> index 126605b7eb3b..1284fe8d78f2 100644
> --- a/drivers/net/ethernet/intel/ice/ice_type.h
> +++ b/drivers/net/ethernet/intel/ice/ice_type.h
> @@ -17,6 +17,7 @@
> #include "ice_protocol_type.h"
> #include "ice_sbq_cmd.h"
> #include "ice_vlan_mode.h"
> +#include "ice_fwlog.h"
>
> static inline bool ice_is_tc_ena(unsigned long bitmap, u8 tc)
> {
> @@ -859,6 +860,9 @@ struct ice_hw {
> u8 fw_patch; /* firmware patch version */
> u32 fw_build; /* firmware build number */
>
> + bool fwlog_supported; /* does hardware support FW logging? */
> + bool fwlog_ena; /* currently logging? */
> +
> /* Device max aggregate bandwidths corresponding to the GL_PWR_MODE_CTL
> * register. Used for determining the ITR/INTRL granularity during
> * initialization.
> --
> 2.35.1
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next v5 2/5] ice: enable devlink to check FW logging status
2023-01-12 9:38 ` Michal Swiatkowski
@ 2023-01-12 22:53 ` Paul M Stillwell Jr
2023-01-13 6:10 ` Michal Swiatkowski
0 siblings, 1 reply; 14+ messages in thread
From: Paul M Stillwell Jr @ 2023-01-12 22:53 UTC (permalink / raw)
To: Michal Swiatkowski; +Cc: intel-wired-lan
On 1/12/2023 1:38 AM, Michal Swiatkowski wrote:
> On Wed, Jan 11, 2023 at 11:19:03AM -0800, Paul M Stillwell Jr wrote:
>> Users want the ability to debug FW issues by retrieving the
>> FW logs from the E8xx devices. Enable devlink to query the driver
>> to see if the NVM image allows FW logging and to see if FW
>> logging is currently running. The set command is not supported
>> at this time.
>>
>> This is the beginning of the v2 for FW logging.
>>
>> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
>>
>> pick bfdfb2dc6192 ice: add ability to query/set FW log level and resolution
>> ---
>> drivers/net/ethernet/intel/ice/Makefile | 3 +-
>> .../net/ethernet/intel/ice/ice_adminq_cmd.h | 81 ++++++++++++
>> drivers/net/ethernet/intel/ice/ice_common.c | 2 +
>> drivers/net/ethernet/intel/ice/ice_devlink.c | 73 ++++++++++-
>> drivers/net/ethernet/intel/ice/ice_fwlog.c | 118 ++++++++++++++++++
>> drivers/net/ethernet/intel/ice/ice_fwlog.h | 52 ++++++++
>> drivers/net/ethernet/intel/ice/ice_type.h | 4 +
>> 7 files changed, 331 insertions(+), 2 deletions(-)
>> create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.c
>> create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.h
>>
> Hi,
>
> Great changes, does it mean that after applying the patchset we will be
> able to for example dump whole switch block?
>
I'll provide an answer but I'm not sure I fully understand your question:
The switch block is part of the FW logs, but there isn't (currently) a
way to specify just the switch block. This is because there isn't
(currently) a way to handle hex numbers in devlink as far as I can tell.
If I'm wrong then feel free to point out how to do it; all my testing
has not worked. We need to use a hex number because the modules for FW
logs are all bits within a 16-bit value. It doesn't make sense to me to
have the user take some hex number and convert it to decimal to specify
the module we want to log so currently we enable all the modules. So we
will get the switch stuff in the FW logs, but we will get other stuff also.
> Looks good, only cosmetic comments.
>
> [...]
>
>> +
>> +/**
>> + * ice_aq_fwlog_get - Get the current firmware logging configuration (0xFF32)
>> + * @hw: pointer to the HW structure
>> + * @cfg: firmware logging configuration to populate
>> + */
>> +static int
>> +ice_aq_fwlog_get(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
>> +{
>> + struct ice_aqc_fw_log_cfg_resp *fw_modules;
>> + struct ice_aqc_fw_log *cmd;
>> + struct ice_aq_desc desc;
>> + u16 module_id_cnt;
>> + int status;
>> + void *buf;
>> + int i;
>> +
>> + memset(cfg, 0, sizeof(*cfg));
>> +
>> + buf = kzalloc(ICE_AQ_MAX_BUF_LEN, GFP_KERNEL);
>> + if (!buf)
>> + return -ENOMEM;
>> +
>> + ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_fw_logs_query);
>> + cmd = &desc.params.fw_log;
>> +
>> + cmd->cmd_flags = ICE_AQC_FW_LOG_AQ_QUERY;
>> +
>> + status = ice_aq_send_cmd(hw, &desc, buf, ICE_AQ_MAX_BUF_LEN, NULL);
>> + if (status) {
>> + ice_debug(hw, ICE_DBG_FW_LOG, "Failed to get FW log configuration\n");
>> + goto status_out;
>> + }
>> +
>> + module_id_cnt = le16_to_cpu(cmd->ops.cfg.mdl_cnt);
>> + if (module_id_cnt < ICE_AQC_FW_LOG_ID_MAX) {
>> + ice_debug(hw, ICE_DBG_FW_LOG, "FW returned less than the expected number of FW log module IDs\n");
>> + } else {
>> + if (module_id_cnt > ICE_AQC_FW_LOG_ID_MAX)
>> + ice_debug(hw, ICE_DBG_FW_LOG, "FW returned more than expected number of FW log module IDs, setting module_id_cnt to software expected max %u\n",
>> + ICE_AQC_FW_LOG_ID_MAX);
>> + module_id_cnt = ICE_AQC_FW_LOG_ID_MAX;
> Maybe:
> else if (module_id_cnt > ICE_AQC_FW_LOG_ID_MAX) {
> ice_debug();
> module_id_cnt = ICE_AQC_FW_LOG_ID_MAX;
> }
> We will save one indent, but it is cosmetic, so feel free to ignore.
>
Will change
>> + }
>> +
>> + cfg->log_resolution = le16_to_cpu(cmd->ops.cfg.log_resolution);
>> + if (cmd->cmd_flags & ICE_AQC_FW_LOG_CONF_AQ_EN)
>> + cfg->options |= ICE_FWLOG_OPTION_ARQ_ENA;
>> + if (cmd->cmd_flags & ICE_AQC_FW_LOG_CONF_UART_EN)
>> + cfg->options |= ICE_FWLOG_OPTION_UART_ENA;
>> + if (cmd->cmd_flags & ICE_AQC_FW_LOG_QUERY_REGISTERED)
>> + cfg->options |= ICE_FWLOG_OPTION_IS_REGISTERED;
>> +
>> + fw_modules = (struct ice_aqc_fw_log_cfg_resp *)buf;
>> +
>> + for (i = 0; i < module_id_cnt; i++) {
>> + struct ice_aqc_fw_log_cfg_resp *fw_module = &fw_modules[i];
>> +
>> + cfg->module_entries[i].module_id =
>> + le16_to_cpu(fw_module->module_identifier);
>> + cfg->module_entries[i].log_level = fw_module->log_level;
>> + }
>> +
>> +status_out:
>> + kfree(buf);
>> + return status;
>> +}
>> +
>> +/**
>> + * ice_fwlog_set_supported - Set if FW logging is supported by FW
>> + * @hw: pointer to the HW struct
>> + *
>> + * If FW returns success to the ice_aq_fwlog_get call then it supports FW
>> + * logging, else it doesn't. Set the fwlog_supported flag accordingly.
>> + *
>> + * This function is only meant to be called during driver init to determine if
>> + * the FW support FW logging.
>> + */
>> +void ice_fwlog_set_supported(struct ice_hw *hw)
>> +{
>> + struct ice_fwlog_cfg *cfg;
>> + int status;
>> +
>> + hw->fwlog_supported = false;
>> +
>> + cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
>> + if (!cfg)
>> + return;
>> +
>> + /* don't call ice_fwlog_get() because that would overwrite the cached
>> + * configuration from the call to ice_fwlog_init(), which is expected to
>> + * be called prior to this function
>> + */
>> + status = ice_aq_fwlog_get(hw, cfg);
>> + if (status)
>> + ice_debug(hw, ICE_DBG_FW_LOG, "ice_aq_fwlog_get failed, FW logging is not supported on this version of FW, status %d\n",
>> + status);
>> + else
>> + hw->fwlog_supported = true;
>> +
>> + kfree(cfg);
>> +}
>> diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.h b/drivers/net/ethernet/intel/ice/ice_fwlog.h
>> new file mode 100644
>> index 000000000000..3a2c83502763
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/ice/ice_fwlog.h
>> @@ -0,0 +1,52 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright (C) 2022, Intel Corporation. */
>> +
>> +#ifndef _ICE_FWLOG_H_
>> +#define _ICE_FWLOG_H_
>> +#include "ice_adminq_cmd.h"
>> +
>> +struct ice_hw;
>> +
>> +/* Only a single log level should be set and all log levels under the set value
>> + * are enabled, e.g. if log level is set to ICE_FW_LOG_LEVEL_VERBOSE, then all
>> + * other log levels are included (except ICE_FW_LOG_LEVEL_NONE)
>> + */
>> +enum ice_fwlog_level {
>> + ICE_FWLOG_LEVEL_NONE = 0,
>> + ICE_FWLOG_LEVEL_ERROR = 1,
>> + ICE_FWLOG_LEVEL_WARNING = 2,
>> + ICE_FWLOG_LEVEL_NORMAL = 3,
>> + ICE_FWLOG_LEVEL_VERBOSE = 4,
>> + ICE_FWLOG_LEVEL_INVALID, /* all values >= this entry are invalid */
>> +};
>> +
>> +struct ice_fwlog_module_entry {
>> + /* module ID for the corresponding firmware logging event */
>> + u16 module_id;
>> + /* verbosity level for the module_id */
>> + u8 log_level;
>> +};
>> +
>> +struct ice_fwlog_cfg {
>> + /* list of modules for configuring log level */
>> + struct ice_fwlog_module_entry module_entries[ICE_AQC_FW_LOG_ID_MAX];
>> + /* options used to configure firmware logging */
>> + u16 options;
>> +#define ICE_FWLOG_OPTION_ARQ_ENA BIT(0)
>> +#define ICE_FWLOG_OPTION_UART_ENA BIT(1)
>> + /* set before calling ice_fwlog_init() so the PF registers for firmware
>> + * logging on initialization
>> + */
>> +#define ICE_FWLOG_OPTION_REGISTER_ON_INIT BIT(2)
>> + /* set in the ice_fwlog_get() response if the PF is registered for FW
>> + * logging events over ARQ
>> + */
>> +#define ICE_FWLOG_OPTION_IS_REGISTERED BIT(3)
>> +
>> + /* minimum number of log events sent per Admin Receive Queue event */
>> + u16 log_resolution;
>> +};
>> +
>> +void ice_fwlog_set_supported(struct ice_hw *hw);
>> +bool ice_fwlog_supported(struct ice_hw *hw);
>> +#endif /* _ICE_FWLOG_H_ */
>> diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
>> index 126605b7eb3b..1284fe8d78f2 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_type.h
>> +++ b/drivers/net/ethernet/intel/ice/ice_type.h
>> @@ -17,6 +17,7 @@
>> #include "ice_protocol_type.h"
>> #include "ice_sbq_cmd.h"
>> #include "ice_vlan_mode.h"
>> +#include "ice_fwlog.h"
>>
>> static inline bool ice_is_tc_ena(unsigned long bitmap, u8 tc)
>> {
>> @@ -859,6 +860,9 @@ struct ice_hw {
>> u8 fw_patch; /* firmware patch version */
>> u32 fw_build; /* firmware build number */
>>
>> + bool fwlog_supported; /* does hardware support FW logging? */
>> + bool fwlog_ena; /* currently logging? */
>> +
>> /* Device max aggregate bandwidths corresponding to the GL_PWR_MODE_CTL
>> * register. Used for determining the ITR/INTRL granularity during
>> * initialization.
>> --
>> 2.35.1
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next v5 2/5] ice: enable devlink to check FW logging status
2023-01-12 22:53 ` Paul M Stillwell Jr
@ 2023-01-13 6:10 ` Michal Swiatkowski
0 siblings, 0 replies; 14+ messages in thread
From: Michal Swiatkowski @ 2023-01-13 6:10 UTC (permalink / raw)
To: Paul M Stillwell Jr; +Cc: intel-wired-lan
On Thu, Jan 12, 2023 at 02:53:07PM -0800, Paul M Stillwell Jr wrote:
> On 1/12/2023 1:38 AM, Michal Swiatkowski wrote:
> > On Wed, Jan 11, 2023 at 11:19:03AM -0800, Paul M Stillwell Jr wrote:
> > > Users want the ability to debug FW issues by retrieving the
> > > FW logs from the E8xx devices. Enable devlink to query the driver
> > > to see if the NVM image allows FW logging and to see if FW
> > > logging is currently running. The set command is not supported
> > > at this time.
> > >
> > > This is the beginning of the v2 for FW logging.
> > >
> > > Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> > >
> > > pick bfdfb2dc6192 ice: add ability to query/set FW log level and resolution
> > > ---
> > > drivers/net/ethernet/intel/ice/Makefile | 3 +-
> > > .../net/ethernet/intel/ice/ice_adminq_cmd.h | 81 ++++++++++++
> > > drivers/net/ethernet/intel/ice/ice_common.c | 2 +
> > > drivers/net/ethernet/intel/ice/ice_devlink.c | 73 ++++++++++-
> > > drivers/net/ethernet/intel/ice/ice_fwlog.c | 118 ++++++++++++++++++
> > > drivers/net/ethernet/intel/ice/ice_fwlog.h | 52 ++++++++
> > > drivers/net/ethernet/intel/ice/ice_type.h | 4 +
> > > 7 files changed, 331 insertions(+), 2 deletions(-)
> > > create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.c
> > > create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.h
> > >
> > Hi,
> >
> > Great changes, does it mean that after applying the patchset we will be
> > able to for example dump whole switch block?
> >
>
> I'll provide an answer but I'm not sure I fully understand your question:
>
> The switch block is part of the FW logs, but there isn't (currently) a way
> to specify just the switch block. This is because there isn't (currently) a
> way to handle hex numbers in devlink as far as I can tell. If I'm wrong then
> feel free to point out how to do it; all my testing has not worked. We need
> to use a hex number because the modules for FW logs are all bits within a
> 16-bit value. It doesn't make sense to me to have the user take some hex
> number and convert it to decimal to specify the module we want to log so
> currently we enable all the modules. So we will get the switch stuff in the
> FW logs, but we will get other stuff also.
>
>
Thanks, got it. I think the script You mentioned in cover letter is
able to process switch stuff from the whole logs. I will check that in
the meantime.
> > Looks good, only cosmetic comments.
> >
> > [...]
> >
> > > +
> > > +/**
> > > + * ice_aq_fwlog_get - Get the current firmware logging configuration (0xFF32)
> > > + * @hw: pointer to the HW structure
> > > + * @cfg: firmware logging configuration to populate
> > > + */
> > > +static int
> > > +ice_aq_fwlog_get(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
> > > +{
> > > + struct ice_aqc_fw_log_cfg_resp *fw_modules;
> > > + struct ice_aqc_fw_log *cmd;
> > > + struct ice_aq_desc desc;
> > > + u16 module_id_cnt;
> > > + int status;
> > > + void *buf;
> > > + int i;
> > > +
> > > + memset(cfg, 0, sizeof(*cfg));
> > > +
> > > + buf = kzalloc(ICE_AQ_MAX_BUF_LEN, GFP_KERNEL);
> > > + if (!buf)
> > > + return -ENOMEM;
> > > +
> > > + ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_fw_logs_query);
> > > + cmd = &desc.params.fw_log;
> > > +
> > > + cmd->cmd_flags = ICE_AQC_FW_LOG_AQ_QUERY;
> > > +
> > > + status = ice_aq_send_cmd(hw, &desc, buf, ICE_AQ_MAX_BUF_LEN, NULL);
> > > + if (status) {
> > > + ice_debug(hw, ICE_DBG_FW_LOG, "Failed to get FW log configuration\n");
> > > + goto status_out;
> > > + }
> > > +
> > > + module_id_cnt = le16_to_cpu(cmd->ops.cfg.mdl_cnt);
> > > + if (module_id_cnt < ICE_AQC_FW_LOG_ID_MAX) {
> > > + ice_debug(hw, ICE_DBG_FW_LOG, "FW returned less than the expected number of FW log module IDs\n");
> > > + } else {
> > > + if (module_id_cnt > ICE_AQC_FW_LOG_ID_MAX)
> > > + ice_debug(hw, ICE_DBG_FW_LOG, "FW returned more than expected number of FW log module IDs, setting module_id_cnt to software expected max %u\n",
> > > + ICE_AQC_FW_LOG_ID_MAX);
> > > + module_id_cnt = ICE_AQC_FW_LOG_ID_MAX;
> > Maybe:
> > else if (module_id_cnt > ICE_AQC_FW_LOG_ID_MAX) {
> > ice_debug();
> > module_id_cnt = ICE_AQC_FW_LOG_ID_MAX;
> > }
> > We will save one indent, but it is cosmetic, so feel free to ignore.
> >
>
> Will change
>
> > > + }
> > > +
> > > + cfg->log_resolution = le16_to_cpu(cmd->ops.cfg.log_resolution);
> > > + if (cmd->cmd_flags & ICE_AQC_FW_LOG_CONF_AQ_EN)
> > > + cfg->options |= ICE_FWLOG_OPTION_ARQ_ENA;
> > > + if (cmd->cmd_flags & ICE_AQC_FW_LOG_CONF_UART_EN)
> > > + cfg->options |= ICE_FWLOG_OPTION_UART_ENA;
> > > + if (cmd->cmd_flags & ICE_AQC_FW_LOG_QUERY_REGISTERED)
> > > + cfg->options |= ICE_FWLOG_OPTION_IS_REGISTERED;
> > > +
> > > + fw_modules = (struct ice_aqc_fw_log_cfg_resp *)buf;
> > > +
> > > + for (i = 0; i < module_id_cnt; i++) {
> > > + struct ice_aqc_fw_log_cfg_resp *fw_module = &fw_modules[i];
> > > +
> > > + cfg->module_entries[i].module_id =
> > > + le16_to_cpu(fw_module->module_identifier);
> > > + cfg->module_entries[i].log_level = fw_module->log_level;
> > > + }
> > > +
> > > +status_out:
> > > + kfree(buf);
> > > + return status;
> > > +}
> > > +
> > > +/**
> > > + * ice_fwlog_set_supported - Set if FW logging is supported by FW
> > > + * @hw: pointer to the HW struct
> > > + *
> > > + * If FW returns success to the ice_aq_fwlog_get call then it supports FW
> > > + * logging, else it doesn't. Set the fwlog_supported flag accordingly.
> > > + *
> > > + * This function is only meant to be called during driver init to determine if
> > > + * the FW support FW logging.
> > > + */
> > > +void ice_fwlog_set_supported(struct ice_hw *hw)
> > > +{
> > > + struct ice_fwlog_cfg *cfg;
> > > + int status;
> > > +
> > > + hw->fwlog_supported = false;
> > > +
> > > + cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> > > + if (!cfg)
> > > + return;
> > > +
> > > + /* don't call ice_fwlog_get() because that would overwrite the cached
> > > + * configuration from the call to ice_fwlog_init(), which is expected to
> > > + * be called prior to this function
> > > + */
> > > + status = ice_aq_fwlog_get(hw, cfg);
> > > + if (status)
> > > + ice_debug(hw, ICE_DBG_FW_LOG, "ice_aq_fwlog_get failed, FW logging is not supported on this version of FW, status %d\n",
> > > + status);
> > > + else
> > > + hw->fwlog_supported = true;
> > > +
> > > + kfree(cfg);
> > > +}
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.h b/drivers/net/ethernet/intel/ice/ice_fwlog.h
> > > new file mode 100644
> > > index 000000000000..3a2c83502763
> > > --- /dev/null
> > > +++ b/drivers/net/ethernet/intel/ice/ice_fwlog.h
> > > @@ -0,0 +1,52 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/* Copyright (C) 2022, Intel Corporation. */
> > > +
> > > +#ifndef _ICE_FWLOG_H_
> > > +#define _ICE_FWLOG_H_
> > > +#include "ice_adminq_cmd.h"
> > > +
> > > +struct ice_hw;
> > > +
> > > +/* Only a single log level should be set and all log levels under the set value
> > > + * are enabled, e.g. if log level is set to ICE_FW_LOG_LEVEL_VERBOSE, then all
> > > + * other log levels are included (except ICE_FW_LOG_LEVEL_NONE)
> > > + */
> > > +enum ice_fwlog_level {
> > > + ICE_FWLOG_LEVEL_NONE = 0,
> > > + ICE_FWLOG_LEVEL_ERROR = 1,
> > > + ICE_FWLOG_LEVEL_WARNING = 2,
> > > + ICE_FWLOG_LEVEL_NORMAL = 3,
> > > + ICE_FWLOG_LEVEL_VERBOSE = 4,
> > > + ICE_FWLOG_LEVEL_INVALID, /* all values >= this entry are invalid */
> > > +};
> > > +
> > > +struct ice_fwlog_module_entry {
> > > + /* module ID for the corresponding firmware logging event */
> > > + u16 module_id;
> > > + /* verbosity level for the module_id */
> > > + u8 log_level;
> > > +};
> > > +
> > > +struct ice_fwlog_cfg {
> > > + /* list of modules for configuring log level */
> > > + struct ice_fwlog_module_entry module_entries[ICE_AQC_FW_LOG_ID_MAX];
> > > + /* options used to configure firmware logging */
> > > + u16 options;
> > > +#define ICE_FWLOG_OPTION_ARQ_ENA BIT(0)
> > > +#define ICE_FWLOG_OPTION_UART_ENA BIT(1)
> > > + /* set before calling ice_fwlog_init() so the PF registers for firmware
> > > + * logging on initialization
> > > + */
> > > +#define ICE_FWLOG_OPTION_REGISTER_ON_INIT BIT(2)
> > > + /* set in the ice_fwlog_get() response if the PF is registered for FW
> > > + * logging events over ARQ
> > > + */
> > > +#define ICE_FWLOG_OPTION_IS_REGISTERED BIT(3)
> > > +
> > > + /* minimum number of log events sent per Admin Receive Queue event */
> > > + u16 log_resolution;
> > > +};
> > > +
> > > +void ice_fwlog_set_supported(struct ice_hw *hw);
> > > +bool ice_fwlog_supported(struct ice_hw *hw);
> > > +#endif /* _ICE_FWLOG_H_ */
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
> > > index 126605b7eb3b..1284fe8d78f2 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_type.h
> > > +++ b/drivers/net/ethernet/intel/ice/ice_type.h
> > > @@ -17,6 +17,7 @@
> > > #include "ice_protocol_type.h"
> > > #include "ice_sbq_cmd.h"
> > > #include "ice_vlan_mode.h"
> > > +#include "ice_fwlog.h"
> > > static inline bool ice_is_tc_ena(unsigned long bitmap, u8 tc)
> > > {
> > > @@ -859,6 +860,9 @@ struct ice_hw {
> > > u8 fw_patch; /* firmware patch version */
> > > u32 fw_build; /* firmware build number */
> > > + bool fwlog_supported; /* does hardware support FW logging? */
> > > + bool fwlog_ena; /* currently logging? */
> > > +
> > > /* Device max aggregate bandwidths corresponding to the GL_PWR_MODE_CTL
> > > * register. Used for determining the ITR/INTRL granularity during
> > > * initialization.
> > > --
> > > 2.35.1
> > >
> > > _______________________________________________
> > > Intel-wired-lan mailing list
> > > Intel-wired-lan@osuosl.org
> > > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-wired-lan] [PATCH net-next v5 3/5] ice: add ability to query/set FW log level and resolution
2023-01-11 19:19 [Intel-wired-lan] [PATCH net-next v5 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 1/5] ice: remove FW logging code Paul M Stillwell Jr
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 2/5] ice: enable devlink to check FW logging status Paul M Stillwell Jr
@ 2023-01-11 19:19 ` Paul M Stillwell Jr
2023-01-12 10:00 ` Michal Swiatkowski
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 4/5] ice: disable FW logging on driver unload Paul M Stillwell Jr
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 5/5] ice: use debugfs to output FW log data Paul M Stillwell Jr
4 siblings, 1 reply; 14+ messages in thread
From: Paul M Stillwell Jr @ 2023-01-11 19:19 UTC (permalink / raw)
To: intel-wired-lan
The E8xx has the ability to change the FW log level and
the granularity at which the logs get output. Enable
the ability to see what the current values are and to
change them.
The following devlink commands are now supported:
devlink dev param set <pci dev> name fwlog_enabled value <true/false> cmode runtime
devlink dev param set <pci dev> name fwlog_level value <0-4> cmode runtime
devlink dev param set <pci dev> name fwlog_resolution value <1-128> cmode runtime
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
---
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 2 +
drivers/net/ethernet/intel/ice/ice_common.c | 4 +-
drivers/net/ethernet/intel/ice/ice_devlink.c | 131 ++++++++-
drivers/net/ethernet/intel/ice/ice_fwlog.c | 278 +++++++++++++++++-
drivers/net/ethernet/intel/ice/ice_fwlog.h | 5 +
drivers/net/ethernet/intel/ice/ice_type.h | 1 +
6 files changed, 416 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 0d560287ec23..1af036beeb45 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -2374,6 +2374,8 @@ enum ice_adminq_opc {
ice_aqc_opc_event_lan_overflow = 0x1001,
/* FW Logging Commands */
+ ice_aqc_opc_fw_logs_config = 0xFF30,
+ ice_aqc_opc_fw_logs_register = 0xFF31,
ice_aqc_opc_fw_logs_query = 0xFF32,
};
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c
index ef7e5d7c18f2..da55db0170dd 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -879,7 +879,9 @@ int ice_init_hw(struct ice_hw *hw)
if (status)
goto err_unroll_cqinit;
- ice_fwlog_set_supported(hw);
+ status = ice_fwlog_init(hw);
+ if (status)
+ ice_debug(hw, ICE_DBG_FW_LOG, "Error initializing FW logging: %d\n", status);
status = ice_clear_pf_cfg(hw);
if (status)
diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index 0e76fd19b61a..307ae6be2b8c 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -374,6 +374,8 @@ enum ice_devlink_param_id {
ICE_DEVLINK_PARAM_ID_TX_BALANCE,
ICE_DEVLINK_PARAM_ID_FWLOG_SUPPORTED,
ICE_DEVLINK_PARAM_ID_FWLOG_ENABLED,
+ ICE_DEVLINK_PARAM_ID_FWLOG_LEVEL,
+ ICE_DEVLINK_PARAM_ID_FWLOG_RESOLUTION,
};
/**
@@ -1430,8 +1432,111 @@ static int
ice_devlink_fwlog_enabled_set(struct devlink *devlink, u32 id,
struct devlink_param_gset_ctx *ctx)
{
- /* set operation is unsupported at this time */
- return -EOPNOTSUPP;
+ struct ice_pf *pf = devlink_priv(devlink);
+ struct ice_hw *hw = &pf->hw;
+ int status;
+
+ /* only support fw log commands on PF 0 */
+ if (hw->bus.func)
+ return -EOPNOTSUPP;
+
+ if (hw->fwlog_ena == ctx->val.vbool)
+ return 0;
+
+ hw->fwlog_ena = ctx->val.vbool;
+
+ if (hw->fwlog_ena)
+ hw->fwlog_cfg.options |= ICE_FWLOG_OPTION_ARQ_ENA;
+ else
+ hw->fwlog_cfg.options &= ~ICE_FWLOG_OPTION_ARQ_ENA;
+
+ /* send the cfg to FW and register for events */
+ status = ice_fwlog_set(hw, &hw->fwlog_cfg);
+ if (status)
+ return status;
+
+ return hw->fwlog_ena ? ice_fwlog_register(hw) : ice_fwlog_unregister(hw);
+}
+
+static int
+ice_devlink_fwlog_level_get(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct ice_pf *pf = devlink_priv(devlink);
+
+ /* only support fw log commands on PF 0 */
+ if (pf->hw.bus.func)
+ return -EOPNOTSUPP;
+
+ /* all the log levels are the same so pick the first one */
+ ctx->val.vu8 = pf->hw.fwlog_cfg.module_entries[0].log_level;
+
+ return 0;
+}
+
+static int
+ice_devlink_fwlog_level_set(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct ice_pf *pf = devlink_priv(devlink);
+ struct ice_fwlog_cfg *cfg;
+ int i;
+
+ if (pf->hw.bus.func)
+ return -EOPNOTSUPP;
+
+ if (ctx->val.vu8 >= ICE_FWLOG_LEVEL_INVALID) {
+ dev_err(ice_pf_to_dev(pf), "Log level is greater than allowed! %d\n",
+ ctx->val.vu8);
+ return -EINVAL;
+ }
+
+ cfg = &pf->hw.fwlog_cfg;
+
+ /* update the log level for all modules to the same thing. this gets
+ * written to the FW when the user says enable logging
+ */
+ for (i = 0; i < ICE_AQC_FW_LOG_ID_MAX; i++)
+ cfg->module_entries[i].log_level = ctx->val.vu8;
+
+ return 0;
+}
+
+static int
+ice_devlink_fwlog_resolution_get(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct ice_pf *pf = devlink_priv(devlink);
+
+ /* only support fw log commands on PF 0 */
+ if (pf->hw.bus.func)
+ return -EOPNOTSUPP;
+
+ ctx->val.vu8 = pf->hw.fwlog_cfg.log_resolution;
+
+ return 0;
+}
+
+static int
+ice_devlink_fwlog_resolution_set(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct ice_pf *pf = devlink_priv(devlink);
+
+ /* only support fw log commands on PF 0 */
+ if (pf->hw.bus.func)
+ return -EOPNOTSUPP;
+
+ if (ctx->val.vu8 < ICE_AQC_FW_LOG_MIN_RESOLUTION ||
+ ctx->val.vu8 > ICE_AQC_FW_LOG_MAX_RESOLUTION) {
+ dev_err(ice_pf_to_dev(pf), "Log resolution is out of range. Should be between 1 - 128: %d\n",
+ ctx->val.vu8);
+ return -EINVAL;
+ }
+
+ pf->hw.fwlog_cfg.log_resolution = ctx->val.vu8;
+
+ return 0;
}
static const struct devlink_param ice_devlink_params[] = {
@@ -1455,6 +1560,18 @@ static const struct devlink_param ice_devlink_params[] = {
ice_devlink_fwlog_enabled_get,
ice_devlink_fwlog_enabled_set,
NULL),
+ DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_FWLOG_LEVEL,
+ "fwlog_level", DEVLINK_PARAM_TYPE_U8,
+ BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+ ice_devlink_fwlog_level_get,
+ ice_devlink_fwlog_level_set,
+ NULL),
+ DEVLINK_PARAM_DRIVER(ICE_DEVLINK_PARAM_ID_FWLOG_RESOLUTION,
+ "fwlog_resolution", DEVLINK_PARAM_TYPE_U8,
+ BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+ ice_devlink_fwlog_resolution_get,
+ ice_devlink_fwlog_resolution_set,
+ NULL),
};
static void ice_devlink_free(void *devlink_ptr)
@@ -1559,6 +1676,16 @@ int ice_devlink_register_params(struct ice_pf *pf)
ICE_DEVLINK_PARAM_ID_FWLOG_ENABLED,
value);
+ value.vu8 = ICE_FWLOG_LEVEL_NORMAL;
+ devlink_param_driverinit_value_set(devlink,
+ ICE_DEVLINK_PARAM_ID_FWLOG_LEVEL,
+ value);
+
+ /* set the default value for the FW to pack 10 messages per AQ event */
+ value.vu8 = 10;
+ devlink_param_driverinit_value_set(devlink,
+ ICE_DEVLINK_PARAM_ID_FWLOG_RESOLUTION,
+ value);
return 0;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.c b/drivers/net/ethernet/intel/ice/ice_fwlog.c
index efea71b6c9f8..6973f01749f9 100644
--- a/drivers/net/ethernet/intel/ice/ice_fwlog.c
+++ b/drivers/net/ethernet/intel/ice/ice_fwlog.c
@@ -4,6 +4,165 @@
#include "ice_common.h"
#include "ice_fwlog.h"
+/**
+ * cache_cfg - Cache FW logging config
+ * @hw: pointer to the HW structure
+ * @cfg: config to cache
+ */
+static void cache_cfg(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
+{
+ hw->fwlog_cfg = *cfg;
+}
+
+/**
+ * valid_module_entries - validate all the module entry IDs and log levels
+ * @hw: pointer to the HW structure
+ * @entries: entries to validate
+ * @num_entries: number of entries to validate
+ */
+static bool
+valid_module_entries(struct ice_hw *hw, struct ice_fwlog_module_entry *entries,
+ u16 num_entries)
+{
+ int i;
+
+ if (!entries) {
+ ice_debug(hw, ICE_DBG_FW_LOG, "Null ice_fwlog_module_entry array\n");
+ return false;
+ }
+
+ if (!num_entries) {
+ ice_debug(hw, ICE_DBG_FW_LOG, "num_entries must be non-zero\n");
+ return false;
+ }
+
+ for (i = 0; i < num_entries; i++) {
+ struct ice_fwlog_module_entry *entry = &entries[i];
+
+ if (entry->module_id >= ICE_AQC_FW_LOG_ID_MAX) {
+ ice_debug(hw, ICE_DBG_FW_LOG, "Invalid module_id %u, max valid module_id is %u\n",
+ entry->module_id, ICE_AQC_FW_LOG_ID_MAX - 1);
+ return false;
+ }
+
+ if (entry->log_level >= ICE_FWLOG_LEVEL_INVALID) {
+ ice_debug(hw, ICE_DBG_FW_LOG, "Invalid log_level %u, max valid log_level is %u\n",
+ entry->log_level,
+ ICE_AQC_FW_LOG_ID_MAX - 1);
+ return false;
+ }
+ }
+
+ return true;
+}
+
+/**
+ * valid_cfg - validate entire configuration
+ * @hw: pointer to the HW structure
+ * @cfg: config to validate
+ */
+static bool valid_cfg(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
+{
+ if (!cfg) {
+ ice_debug(hw, ICE_DBG_FW_LOG, "Null ice_fwlog_cfg\n");
+ return false;
+ }
+
+ if (cfg->log_resolution < ICE_AQC_FW_LOG_MIN_RESOLUTION ||
+ cfg->log_resolution > ICE_AQC_FW_LOG_MAX_RESOLUTION) {
+ ice_debug(hw, ICE_DBG_FW_LOG, "Unsupported log_resolution %u, must be between %u and %u\n",
+ cfg->log_resolution, ICE_AQC_FW_LOG_MIN_RESOLUTION,
+ ICE_AQC_FW_LOG_MAX_RESOLUTION);
+ return false;
+ }
+
+ if (!valid_module_entries(hw, cfg->module_entries,
+ ICE_AQC_FW_LOG_ID_MAX))
+ return false;
+
+ return true;
+}
+
+/**
+ * ice_fwlog_init - Initialize FW logging variables
+ * @hw: pointer to the HW structure
+ *
+ * This function should be called on driver initialization during
+ * ice_init_hw().
+ */
+int ice_fwlog_init(struct ice_hw *hw)
+{
+ int status;
+
+ ice_fwlog_set_supported(hw);
+
+ if (ice_fwlog_supported(hw)) {
+ struct ice_fwlog_cfg *cfg;
+
+ cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return -ENOMEM;
+
+ /* read the current config from the FW and store it */
+ status = ice_fwlog_get(hw, cfg);
+ if (status)
+ return status;
+ }
+
+ return 0;
+}
+
+/**
+ * ice_aq_fwlog_set - Set FW logging configuration AQ command (0xFF30)
+ * @hw: pointer to the HW structure
+ * @entries: entries to configure
+ * @num_entries: number of @entries
+ * @options: options from ice_fwlog_cfg->options structure
+ * @log_resolution: logging resolution
+ */
+static int
+ice_aq_fwlog_set(struct ice_hw *hw, struct ice_fwlog_module_entry *entries,
+ u16 num_entries, u16 options, u16 log_resolution)
+{
+ struct ice_aqc_fw_log_cfg_resp *fw_modules;
+ struct ice_aqc_fw_log *cmd;
+ struct ice_aq_desc desc;
+ int status;
+ int i;
+
+ fw_modules = kcalloc(num_entries, sizeof(*fw_modules), GFP_KERNEL);
+ if (!fw_modules)
+ return -ENOMEM;
+
+ for (i = 0; i < num_entries; i++) {
+ fw_modules[i].module_identifier =
+ cpu_to_le16(entries[i].module_id);
+ fw_modules[i].log_level = entries[i].log_level;
+ }
+
+ ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_fw_logs_config);
+ desc.flags |= cpu_to_le16(ICE_AQ_FLAG_RD);
+
+ cmd = &desc.params.fw_log;
+
+ cmd->cmd_flags = ICE_AQC_FW_LOG_CONF_SET_VALID;
+ cmd->ops.cfg.log_resolution = cpu_to_le16(log_resolution);
+ cmd->ops.cfg.mdl_cnt = cpu_to_le16(num_entries);
+
+ if (options & ICE_FWLOG_OPTION_ARQ_ENA)
+ cmd->cmd_flags |= ICE_AQC_FW_LOG_CONF_AQ_EN;
+ if (options & ICE_FWLOG_OPTION_UART_ENA)
+ cmd->cmd_flags |= ICE_AQC_FW_LOG_CONF_UART_EN;
+
+ status = ice_aq_send_cmd(hw, &desc, fw_modules,
+ sizeof(*fw_modules) * num_entries,
+ NULL);
+
+ kfree(fw_modules);
+
+ return status;
+}
+
/**
* ice_fwlog_supported - Cached for whether FW supports FW logging or not
* @hw: pointer to the HW structure
@@ -16,13 +175,104 @@ bool ice_fwlog_supported(struct ice_hw *hw)
return hw->fwlog_supported;
}
+/**
+ * ice_fwlog_set - Set the firmware logging settings
+ * @hw: pointer to the HW structure
+ * @cfg: config used to set firmware logging
+ *
+ * This function should be called whenever the driver needs to set the firmware
+ * logging configuration. It can be called on initialization, reset, or during
+ * runtime.
+ *
+ * If the PF wishes to receive FW logging then it must register via
+ * ice_fwlog_register. Note, that ice_fwlog_register does not need to be called
+ * for init.
+ */
+int ice_fwlog_set(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
+{
+ int status;
+
+ if (!ice_fwlog_supported(hw))
+ return -EOPNOTSUPP;
+
+ if (!valid_cfg(hw, cfg))
+ return -EINVAL;
+
+ status = ice_aq_fwlog_set(hw, cfg->module_entries,
+ ICE_AQC_FW_LOG_ID_MAX, cfg->options,
+ cfg->log_resolution);
+ if (!status)
+ cache_cfg(hw, cfg);
+
+ return status;
+}
+
+/**
+ * ice_aq_fwlog_register - Register PF for firmware logging events (0xFF31)
+ * @hw: pointer to the HW structure
+ * @reg: true to register and false to unregister
+ */
+static int ice_aq_fwlog_register(struct ice_hw *hw, bool reg)
+{
+ struct ice_aq_desc desc;
+
+ ice_fill_dflt_direct_cmd_desc(&desc, ice_aqc_opc_fw_logs_register);
+
+ if (reg)
+ desc.params.fw_log.cmd_flags = ICE_AQC_FW_LOG_AQ_REGISTER;
+
+ return ice_aq_send_cmd(hw, &desc, NULL, 0, NULL);
+}
+
+/**
+ * ice_fwlog_register - Register the PF for firmware logging
+ * @hw: pointer to the HW structure
+ *
+ * After this call the PF will start to receive firmware logging based on the
+ * configuration set in ice_fwlog_set.
+ */
+int ice_fwlog_register(struct ice_hw *hw)
+{
+ int status;
+
+ if (!ice_fwlog_supported(hw))
+ return -EOPNOTSUPP;
+
+ status = ice_aq_fwlog_register(hw, true);
+ if (status)
+ ice_debug(hw, ICE_DBG_FW_LOG, "Failed to register for firmware logging events over ARQ\n");
+ else
+ hw->fwlog_cfg.options |= ICE_FWLOG_OPTION_IS_REGISTERED;
+
+ return status;
+}
+
+/**
+ * ice_fwlog_unregister - Unregister the PF from firmware logging
+ * @hw: pointer to the HW structure
+ */
+int ice_fwlog_unregister(struct ice_hw *hw)
+{
+ int status;
+
+ if (!ice_fwlog_supported(hw))
+ return -EOPNOTSUPP;
+
+ status = ice_aq_fwlog_register(hw, false);
+ if (status)
+ ice_debug(hw, ICE_DBG_FW_LOG, "Failed to unregister from firmware logging events over ARQ\n");
+ else
+ hw->fwlog_cfg.options &= ~ICE_FWLOG_OPTION_IS_REGISTERED;
+
+ return status;
+}
+
/**
* ice_aq_fwlog_get - Get the current firmware logging configuration (0xFF32)
* @hw: pointer to the HW structure
* @cfg: firmware logging configuration to populate
*/
-static int
-ice_aq_fwlog_get(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
+static int ice_aq_fwlog_get(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
{
struct ice_aqc_fw_log_cfg_resp *fw_modules;
struct ice_aqc_fw_log *cmd;
@@ -116,3 +366,27 @@ void ice_fwlog_set_supported(struct ice_hw *hw)
kfree(cfg);
}
+
+/**
+ * ice_fwlog_get - Get the firmware logging settings
+ * @hw: pointer to the HW structure
+ * @cfg: config to populate based on current firmware logging settings
+ */
+int ice_fwlog_get(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
+{
+ int status;
+
+ if (!ice_fwlog_supported(hw))
+ return -EOPNOTSUPP;
+
+ if (!cfg)
+ return -EINVAL;
+
+ status = ice_aq_fwlog_get(hw, cfg);
+ if (status)
+ return status;
+
+ cache_cfg(hw, cfg);
+
+ return 0;
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.h b/drivers/net/ethernet/intel/ice/ice_fwlog.h
index 3a2c83502763..fcfceb9f6ec2 100644
--- a/drivers/net/ethernet/intel/ice/ice_fwlog.h
+++ b/drivers/net/ethernet/intel/ice/ice_fwlog.h
@@ -49,4 +49,9 @@ struct ice_fwlog_cfg {
void ice_fwlog_set_supported(struct ice_hw *hw);
bool ice_fwlog_supported(struct ice_hw *hw);
+int ice_fwlog_init(struct ice_hw *hw);
+int ice_fwlog_set(struct ice_hw *hw, struct ice_fwlog_cfg *cfg);
+int ice_fwlog_get(struct ice_hw *hw, struct ice_fwlog_cfg *cfg);
+int ice_fwlog_register(struct ice_hw *hw);
+int ice_fwlog_unregister(struct ice_hw *hw);
#endif /* _ICE_FWLOG_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 1284fe8d78f2..5161757f07a6 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -860,6 +860,7 @@ struct ice_hw {
u8 fw_patch; /* firmware patch version */
u32 fw_build; /* firmware build number */
+ struct ice_fwlog_cfg fwlog_cfg;
bool fwlog_supported; /* does hardware support FW logging? */
bool fwlog_ena; /* currently logging? */
--
2.35.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next v5 3/5] ice: add ability to query/set FW log level and resolution
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 3/5] ice: add ability to query/set FW log level and resolution Paul M Stillwell Jr
@ 2023-01-12 10:00 ` Michal Swiatkowski
2023-01-13 22:07 ` Paul M Stillwell Jr
0 siblings, 1 reply; 14+ messages in thread
From: Michal Swiatkowski @ 2023-01-12 10:00 UTC (permalink / raw)
To: Paul M Stillwell Jr; +Cc: intel-wired-lan
On Wed, Jan 11, 2023 at 11:19:04AM -0800, Paul M Stillwell Jr wrote:
> The E8xx has the ability to change the FW log level and
> the granularity at which the logs get output. Enable
> the ability to see what the current values are and to
> change them.
>
> The following devlink commands are now supported:
>
> devlink dev param set <pci dev> name fwlog_enabled value <true/false> cmode runtime
> devlink dev param set <pci dev> name fwlog_level value <0-4> cmode runtime
> devlink dev param set <pci dev> name fwlog_resolution value <1-128> cmode runtime
>
> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> ---
> .../net/ethernet/intel/ice/ice_adminq_cmd.h | 2 +
> drivers/net/ethernet/intel/ice/ice_common.c | 4 +-
> drivers/net/ethernet/intel/ice/ice_devlink.c | 131 ++++++++-
> drivers/net/ethernet/intel/ice/ice_fwlog.c | 278 +++++++++++++++++-
> drivers/net/ethernet/intel/ice/ice_fwlog.h | 5 +
> drivers/net/ethernet/intel/ice/ice_type.h | 1 +
> 6 files changed, 416 insertions(+), 5 deletions(-)
>
[...]
> diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.c b/drivers/net/ethernet/intel/ice/ice_fwlog.c
> index efea71b6c9f8..6973f01749f9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_fwlog.c
> +++ b/drivers/net/ethernet/intel/ice/ice_fwlog.c
> @@ -4,6 +4,165 @@
> #include "ice_common.h"
> #include "ice_fwlog.h"
>
> +/**
> + * cache_cfg - Cache FW logging config
> + * @hw: pointer to the HW structure
> + * @cfg: config to cache
> + */
> +static void cache_cfg(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
> +{
> + hw->fwlog_cfg = *cfg;
> +}
> +
> +/**
> + * valid_module_entries - validate all the module entry IDs and log levels
> + * @hw: pointer to the HW structure
> + * @entries: entries to validate
> + * @num_entries: number of entries to validate
> + */
> +static bool
> +valid_module_entries(struct ice_hw *hw, struct ice_fwlog_module_entry *entries,
> + u16 num_entries)
> +{
> + int i;
> +
> + if (!entries) {
> + ice_debug(hw, ICE_DBG_FW_LOG, "Null ice_fwlog_module_entry array\n");
> + return false;
> + }
> +
> + if (!num_entries) {
> + ice_debug(hw, ICE_DBG_FW_LOG, "num_entries must be non-zero\n");
> + return false;
> + }
> +
> + for (i = 0; i < num_entries; i++) {
> + struct ice_fwlog_module_entry *entry = &entries[i];
> +
> + if (entry->module_id >= ICE_AQC_FW_LOG_ID_MAX) {
> + ice_debug(hw, ICE_DBG_FW_LOG, "Invalid module_id %u, max valid module_id is %u\n",
> + entry->module_id, ICE_AQC_FW_LOG_ID_MAX - 1);
> + return false;
> + }
> +
> + if (entry->log_level >= ICE_FWLOG_LEVEL_INVALID) {
> + ice_debug(hw, ICE_DBG_FW_LOG, "Invalid log_level %u, max valid log_level is %u\n",
> + entry->log_level,
> + ICE_AQC_FW_LOG_ID_MAX - 1);
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> +/**
> + * valid_cfg - validate entire configuration
> + * @hw: pointer to the HW structure
> + * @cfg: config to validate
> + */
> +static bool valid_cfg(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
> +{
> + if (!cfg) {
> + ice_debug(hw, ICE_DBG_FW_LOG, "Null ice_fwlog_cfg\n");
> + return false;
> + }
> +
> + if (cfg->log_resolution < ICE_AQC_FW_LOG_MIN_RESOLUTION ||
> + cfg->log_resolution > ICE_AQC_FW_LOG_MAX_RESOLUTION) {
> + ice_debug(hw, ICE_DBG_FW_LOG, "Unsupported log_resolution %u, must be between %u and %u\n",
> + cfg->log_resolution, ICE_AQC_FW_LOG_MIN_RESOLUTION,
> + ICE_AQC_FW_LOG_MAX_RESOLUTION);
> + return false;
> + }
> +
> + if (!valid_module_entries(hw, cfg->module_entries,
> + ICE_AQC_FW_LOG_ID_MAX))
> + return false;
> +
> + return true;
return valid_module_entries();
> +}
> +
> +/**
> + * ice_fwlog_init - Initialize FW logging variables
> + * @hw: pointer to the HW structure
> + *
> + * This function should be called on driver initialization during
> + * ice_init_hw().
> + */
> +int ice_fwlog_init(struct ice_hw *hw)
> +{
> + int status;
> +
> + ice_fwlog_set_supported(hw);
> +
> + if (ice_fwlog_supported(hw)) {
> + struct ice_fwlog_cfg *cfg;
> +
> + cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
I don't understand why hw->cfg can't be used here? It will simplify all
this cache_cfg. Is there any reason to alloc config and than set hw->cfg
to it in get flow?
When config parameters are being set from devlink context hw->cfg is
used instead of new allocated config like here (and than casched).
> + if (!cfg)
> + return -ENOMEM;
> +
> + /* read the current config from the FW and store it */
> + status = ice_fwlog_get(hw, cfg);
> + if (status)
> + return status;
Shouldn't cfg be free here? If ice_fwlog_get fails cfg isn't casched.
> + }
> +
> + return 0;
> +}
> +
[...]
>
> --
> 2.35.1
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next v5 3/5] ice: add ability to query/set FW log level and resolution
2023-01-12 10:00 ` Michal Swiatkowski
@ 2023-01-13 22:07 ` Paul M Stillwell Jr
0 siblings, 0 replies; 14+ messages in thread
From: Paul M Stillwell Jr @ 2023-01-13 22:07 UTC (permalink / raw)
To: Michal Swiatkowski; +Cc: intel-wired-lan
On 1/12/2023 2:00 AM, Michal Swiatkowski wrote:
> On Wed, Jan 11, 2023 at 11:19:04AM -0800, Paul M Stillwell Jr wrote:
>> The E8xx has the ability to change the FW log level and
>> the granularity at which the logs get output. Enable
>> the ability to see what the current values are and to
>> change them.
>>
>> The following devlink commands are now supported:
>>
>> devlink dev param set <pci dev> name fwlog_enabled value <true/false> cmode runtime
>> devlink dev param set <pci dev> name fwlog_level value <0-4> cmode runtime
>> devlink dev param set <pci dev> name fwlog_resolution value <1-128> cmode runtime
>>
>> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
>> ---
>> .../net/ethernet/intel/ice/ice_adminq_cmd.h | 2 +
>> drivers/net/ethernet/intel/ice/ice_common.c | 4 +-
>> drivers/net/ethernet/intel/ice/ice_devlink.c | 131 ++++++++-
>> drivers/net/ethernet/intel/ice/ice_fwlog.c | 278 +++++++++++++++++-
>> drivers/net/ethernet/intel/ice/ice_fwlog.h | 5 +
>> drivers/net/ethernet/intel/ice/ice_type.h | 1 +
>> 6 files changed, 416 insertions(+), 5 deletions(-)
>>
> [...]
>> diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.c b/drivers/net/ethernet/intel/ice/ice_fwlog.c
>> index efea71b6c9f8..6973f01749f9 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_fwlog.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_fwlog.c
>> @@ -4,6 +4,165 @@
>> #include "ice_common.h"
>> #include "ice_fwlog.h"
>>
>> +/**
>> + * cache_cfg - Cache FW logging config
>> + * @hw: pointer to the HW structure
>> + * @cfg: config to cache
>> + */
>> +static void cache_cfg(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
>> +{
>> + hw->fwlog_cfg = *cfg;
>> +}
>> +
>> +/**
>> + * valid_module_entries - validate all the module entry IDs and log levels
>> + * @hw: pointer to the HW structure
>> + * @entries: entries to validate
>> + * @num_entries: number of entries to validate
>> + */
>> +static bool
>> +valid_module_entries(struct ice_hw *hw, struct ice_fwlog_module_entry *entries,
>> + u16 num_entries)
>> +{
>> + int i;
>> +
>> + if (!entries) {
>> + ice_debug(hw, ICE_DBG_FW_LOG, "Null ice_fwlog_module_entry array\n");
>> + return false;
>> + }
>> +
>> + if (!num_entries) {
>> + ice_debug(hw, ICE_DBG_FW_LOG, "num_entries must be non-zero\n");
>> + return false;
>> + }
>> +
>> + for (i = 0; i < num_entries; i++) {
>> + struct ice_fwlog_module_entry *entry = &entries[i];
>> +
>> + if (entry->module_id >= ICE_AQC_FW_LOG_ID_MAX) {
>> + ice_debug(hw, ICE_DBG_FW_LOG, "Invalid module_id %u, max valid module_id is %u\n",
>> + entry->module_id, ICE_AQC_FW_LOG_ID_MAX - 1);
>> + return false;
>> + }
>> +
>> + if (entry->log_level >= ICE_FWLOG_LEVEL_INVALID) {
>> + ice_debug(hw, ICE_DBG_FW_LOG, "Invalid log_level %u, max valid log_level is %u\n",
>> + entry->log_level,
>> + ICE_AQC_FW_LOG_ID_MAX - 1);
>> + return false;
>> + }
>> + }
>> +
>> + return true;
>> +}
>> +
>> +/**
>> + * valid_cfg - validate entire configuration
>> + * @hw: pointer to the HW structure
>> + * @cfg: config to validate
>> + */
>> +static bool valid_cfg(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
>> +{
>> + if (!cfg) {
>> + ice_debug(hw, ICE_DBG_FW_LOG, "Null ice_fwlog_cfg\n");
>> + return false;
>> + }
>> +
>> + if (cfg->log_resolution < ICE_AQC_FW_LOG_MIN_RESOLUTION ||
>> + cfg->log_resolution > ICE_AQC_FW_LOG_MAX_RESOLUTION) {
>> + ice_debug(hw, ICE_DBG_FW_LOG, "Unsupported log_resolution %u, must be between %u and %u\n",
>> + cfg->log_resolution, ICE_AQC_FW_LOG_MIN_RESOLUTION,
>> + ICE_AQC_FW_LOG_MAX_RESOLUTION);
>> + return false;
>> + }
>> +
>> + if (!valid_module_entries(hw, cfg->module_entries,
>> + ICE_AQC_FW_LOG_ID_MAX))
>> + return false;
>> +
>> + return true;
> return valid_module_entries();
>
Done
>> +}
>> +
>> +/**
>> + * ice_fwlog_init - Initialize FW logging variables
>> + * @hw: pointer to the HW structure
>> + *
>> + * This function should be called on driver initialization during
>> + * ice_init_hw().
>> + */
>> +int ice_fwlog_init(struct ice_hw *hw)
>> +{
>> + int status;
>> +
>> + ice_fwlog_set_supported(hw);
>> +
>> + if (ice_fwlog_supported(hw)) {
>> + struct ice_fwlog_cfg *cfg;
>> +
>> + cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> I don't understand why hw->cfg can't be used here? It will simplify all
> this cache_cfg. Is there any reason to alloc config and than set hw->cfg
> to it in get flow?
>
> When config parameters are being set from devlink context hw->cfg is
> used instead of new allocated config like here (and than casched).
>
Good point, I removed the allocation of cfg here and removed the
cache_cfg function.
>> + if (!cfg)
>> + return -ENOMEM;
>> +
>> + /* read the current config from the FW and store it */
>> + status = ice_fwlog_get(hw, cfg);
>> + if (status)
>> + return status;
> Shouldn't cfg be free here? If ice_fwlog_get fails cfg isn't casched.
>
Not needed since I refactored the code to remove the allocation of cfg
>> + }
>> +
>> + return 0;
>> +}
>> +
> [...]
>>
>> --
>> 2.35.1
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-wired-lan] [PATCH net-next v5 4/5] ice: disable FW logging on driver unload
2023-01-11 19:19 [Intel-wired-lan] [PATCH net-next v5 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
` (2 preceding siblings ...)
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 3/5] ice: add ability to query/set FW log level and resolution Paul M Stillwell Jr
@ 2023-01-11 19:19 ` Paul M Stillwell Jr
2023-01-12 10:49 ` Michal Swiatkowski
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 5/5] ice: use debugfs to output FW log data Paul M Stillwell Jr
4 siblings, 1 reply; 14+ messages in thread
From: Paul M Stillwell Jr @ 2023-01-11 19:19 UTC (permalink / raw)
To: intel-wired-lan
The FW is running in it's own context irregardless of what the driver
is doing. In this case, if the driver previously registered for FW
log events and then the driver unloads without informing the FW to
unregister for FW log events then the FW still has a timer running to
output FW logs.
The next time the driver loads and tries to register for FW log events
then the FW returns an error, but still enables the continued
outputting of FW logs. This causes an IO error to devlink which isn't
intuitive since the logs are still being output.
Fix this by disabling FW logging when the driver is being unloaded.
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
---
drivers/net/ethernet/intel/ice/ice_main.c | 29 +++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 1b5debc3109d..593efc064f5b 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4500,6 +4500,33 @@ static void ice_unregister_netdev(struct ice_vsi *vsi)
clear_bit(ICE_VSI_NETDEV_REGISTERED, vsi->state);
}
+/**
+ * ice_pf_fwlog_deinit - clear FW logging metadata on device exit
+ * @pf: pointer to the PF struct
+ */
+static void ice_pf_fwlog_deinit(struct ice_pf *pf)
+{
+ struct ice_hw *hw = &pf->hw;
+
+ /* make sure FW logging is disabled to not put the FW in a weird state
+ * for the next driver load
+ */
+ if (hw->fwlog_ena) {
+ int status;
+
+ hw->fwlog_cfg.options &= ~ICE_FWLOG_OPTION_ARQ_ENA;
+ status = ice_fwlog_set(hw, &hw->fwlog_cfg);
+ if (status)
+ dev_warn(ice_pf_to_dev(pf), "Unable to turn off FW logging, status: %d\n",
+ status);
+
+ status = ice_fwlog_unregister(hw);
+ if (status)
+ dev_warn(ice_pf_to_dev(pf), "Unable to unregister FW logging, status: %d\n",
+ status);
+ }
+}
+
/**
* ice_cfg_netdev - Allocate, configure and register a netdev
* @vsi: the VSI associated with the new netdev
@@ -5237,6 +5264,8 @@ static void ice_remove(struct pci_dev *pdev)
msleep(100);
}
+ ice_pf_fwlog_deinit(pf);
+
if (test_bit(ICE_FLAG_SRIOV_ENA, pf->flags)) {
set_bit(ICE_VF_RESETS_DISABLED, pf->state);
ice_free_vfs(pf);
--
2.35.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next v5 4/5] ice: disable FW logging on driver unload
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 4/5] ice: disable FW logging on driver unload Paul M Stillwell Jr
@ 2023-01-12 10:49 ` Michal Swiatkowski
2023-01-13 22:13 ` Paul M Stillwell Jr
0 siblings, 1 reply; 14+ messages in thread
From: Michal Swiatkowski @ 2023-01-12 10:49 UTC (permalink / raw)
To: Paul M Stillwell Jr; +Cc: intel-wired-lan
On Wed, Jan 11, 2023 at 11:19:05AM -0800, Paul M Stillwell Jr wrote:
> The FW is running in it's own context irregardless of what the driver
> is doing. In this case, if the driver previously registered for FW
> log events and then the driver unloads without informing the FW to
> unregister for FW log events then the FW still has a timer running to
> output FW logs.
>
> The next time the driver loads and tries to register for FW log events
> then the FW returns an error, but still enables the continued
> outputting of FW logs. This causes an IO error to devlink which isn't
> intuitive since the logs are still being output.
>
> Fix this by disabling FW logging when the driver is being unloaded.
>
> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice_main.c | 29 +++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 1b5debc3109d..593efc064f5b 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -4500,6 +4500,33 @@ static void ice_unregister_netdev(struct ice_vsi *vsi)
> clear_bit(ICE_VSI_NETDEV_REGISTERED, vsi->state);
> }
>
> +/**
> + * ice_pf_fwlog_deinit - clear FW logging metadata on device exit
> + * @pf: pointer to the PF struct
> + */
> +static void ice_pf_fwlog_deinit(struct ice_pf *pf)
> +{
> + struct ice_hw *hw = &pf->hw;
> +
> + /* make sure FW logging is disabled to not put the FW in a weird state
> + * for the next driver load
> + */
> + if (hw->fwlog_ena) {
> + int status;
> +
> + hw->fwlog_cfg.options &= ~ICE_FWLOG_OPTION_ARQ_ENA;
> + status = ice_fwlog_set(hw, &hw->fwlog_cfg);
> + if (status)
> + dev_warn(ice_pf_to_dev(pf), "Unable to turn off FW logging, status: %d\n",
> + status);
> +
> + status = ice_fwlog_unregister(hw);
> + if (status)
> + dev_warn(ice_pf_to_dev(pf), "Unable to unregister FW logging, status: %d\n",
> + status);
Shouldn't hw->fwlog_cfg be free on deinit? Or if not here, where does
this happen?
> + }
> +}
> +
> /**
> * ice_cfg_netdev - Allocate, configure and register a netdev
> * @vsi: the VSI associated with the new netdev
> @@ -5237,6 +5264,8 @@ static void ice_remove(struct pci_dev *pdev)
> msleep(100);
> }
>
> + ice_pf_fwlog_deinit(pf);
> +
> if (test_bit(ICE_FLAG_SRIOV_ENA, pf->flags)) {
> set_bit(ICE_VF_RESETS_DISABLED, pf->state);
> ice_free_vfs(pf);
> --
> 2.35.1
>
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next v5 4/5] ice: disable FW logging on driver unload
2023-01-12 10:49 ` Michal Swiatkowski
@ 2023-01-13 22:13 ` Paul M Stillwell Jr
2023-01-16 7:41 ` Michal Swiatkowski
0 siblings, 1 reply; 14+ messages in thread
From: Paul M Stillwell Jr @ 2023-01-13 22:13 UTC (permalink / raw)
To: Michal Swiatkowski; +Cc: intel-wired-lan
On 1/12/2023 2:49 AM, Michal Swiatkowski wrote:
> On Wed, Jan 11, 2023 at 11:19:05AM -0800, Paul M Stillwell Jr wrote:
>> The FW is running in it's own context irregardless of what the driver
>> is doing. In this case, if the driver previously registered for FW
>> log events and then the driver unloads without informing the FW to
>> unregister for FW log events then the FW still has a timer running to
>> output FW logs.
>>
>> The next time the driver loads and tries to register for FW log events
>> then the FW returns an error, but still enables the continued
>> outputting of FW logs. This causes an IO error to devlink which isn't
>> intuitive since the logs are still being output.
>>
>> Fix this by disabling FW logging when the driver is being unloaded.
>>
>> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
>> ---
>> drivers/net/ethernet/intel/ice/ice_main.c | 29 +++++++++++++++++++++++
>> 1 file changed, 29 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
>> index 1b5debc3109d..593efc064f5b 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_main.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
>> @@ -4500,6 +4500,33 @@ static void ice_unregister_netdev(struct ice_vsi *vsi)
>> clear_bit(ICE_VSI_NETDEV_REGISTERED, vsi->state);
>> }
>>
>> +/**
>> + * ice_pf_fwlog_deinit - clear FW logging metadata on device exit
>> + * @pf: pointer to the PF struct
>> + */
>> +static void ice_pf_fwlog_deinit(struct ice_pf *pf)
>> +{
>> + struct ice_hw *hw = &pf->hw;
>> +
>> + /* make sure FW logging is disabled to not put the FW in a weird state
>> + * for the next driver load
>> + */
>> + if (hw->fwlog_ena) {
>> + int status;
>> +
>> + hw->fwlog_cfg.options &= ~ICE_FWLOG_OPTION_ARQ_ENA;
>> + status = ice_fwlog_set(hw, &hw->fwlog_cfg);
>> + if (status)
>> + dev_warn(ice_pf_to_dev(pf), "Unable to turn off FW logging, status: %d\n",
>> + status);
>> +
>> + status = ice_fwlog_unregister(hw);
>> + if (status)
>> + dev_warn(ice_pf_to_dev(pf), "Unable to unregister FW logging, status: %d\n",
>> + status);
> Shouldn't hw->fwlog_cfg be free on deinit? Or if not here, where does
> this happen?
>
No need to free hw->fwlog_cfg since it's not a pointer with memory
allocated, it's a member of ice_hw. See ice_type.h, 863
>> + }
>> +}
>> +
>> /**
>> * ice_cfg_netdev - Allocate, configure and register a netdev
>> * @vsi: the VSI associated with the new netdev
>> @@ -5237,6 +5264,8 @@ static void ice_remove(struct pci_dev *pdev)
>> msleep(100);
>> }
>>
>> + ice_pf_fwlog_deinit(pf);
>> +
>> if (test_bit(ICE_FLAG_SRIOV_ENA, pf->flags)) {
>> set_bit(ICE_VF_RESETS_DISABLED, pf->state);
>> ice_free_vfs(pf);
>> --
>> 2.35.1
>>
>> _______________________________________________
>> Intel-wired-lan mailing list
>> Intel-wired-lan@osuosl.org
>> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [Intel-wired-lan] [PATCH net-next v5 4/5] ice: disable FW logging on driver unload
2023-01-13 22:13 ` Paul M Stillwell Jr
@ 2023-01-16 7:41 ` Michal Swiatkowski
0 siblings, 0 replies; 14+ messages in thread
From: Michal Swiatkowski @ 2023-01-16 7:41 UTC (permalink / raw)
To: Paul M Stillwell Jr; +Cc: intel-wired-lan
On Fri, Jan 13, 2023 at 02:13:04PM -0800, Paul M Stillwell Jr wrote:
> On 1/12/2023 2:49 AM, Michal Swiatkowski wrote:
> > On Wed, Jan 11, 2023 at 11:19:05AM -0800, Paul M Stillwell Jr wrote:
> > > The FW is running in it's own context irregardless of what the driver
> > > is doing. In this case, if the driver previously registered for FW
> > > log events and then the driver unloads without informing the FW to
> > > unregister for FW log events then the FW still has a timer running to
> > > output FW logs.
> > >
> > > The next time the driver loads and tries to register for FW log events
> > > then the FW returns an error, but still enables the continued
> > > outputting of FW logs. This causes an IO error to devlink which isn't
> > > intuitive since the logs are still being output.
> > >
> > > Fix this by disabling FW logging when the driver is being unloaded.
> > >
> > > Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> > > ---
> > > drivers/net/ethernet/intel/ice/ice_main.c | 29 +++++++++++++++++++++++
> > > 1 file changed, 29 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> > > index 1b5debc3109d..593efc064f5b 100644
> > > --- a/drivers/net/ethernet/intel/ice/ice_main.c
> > > +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> > > @@ -4500,6 +4500,33 @@ static void ice_unregister_netdev(struct ice_vsi *vsi)
> > > clear_bit(ICE_VSI_NETDEV_REGISTERED, vsi->state);
> > > }
> > > +/**
> > > + * ice_pf_fwlog_deinit - clear FW logging metadata on device exit
> > > + * @pf: pointer to the PF struct
> > > + */
> > > +static void ice_pf_fwlog_deinit(struct ice_pf *pf)
> > > +{
> > > + struct ice_hw *hw = &pf->hw;
> > > +
> > > + /* make sure FW logging is disabled to not put the FW in a weird state
> > > + * for the next driver load
> > > + */
> > > + if (hw->fwlog_ena) {
> > > + int status;
> > > +
> > > + hw->fwlog_cfg.options &= ~ICE_FWLOG_OPTION_ARQ_ENA;
> > > + status = ice_fwlog_set(hw, &hw->fwlog_cfg);
> > > + if (status)
> > > + dev_warn(ice_pf_to_dev(pf), "Unable to turn off FW logging, status: %d\n",
> > > + status);
> > > +
> > > + status = ice_fwlog_unregister(hw);
> > > + if (status)
> > > + dev_warn(ice_pf_to_dev(pf), "Unable to unregister FW logging, status: %d\n",
> > > + status);
> > Shouldn't hw->fwlog_cfg be free on deinit? Or if not here, where does
> > this happen?
> >
>
> No need to free hw->fwlog_cfg since it's not a pointer with memory
> allocated, it's a member of ice_hw. See ice_type.h, 863
>
Sure, didn't notice that, thanks
> > > + }
> > > +}
> > > +
> > > /**
> > > * ice_cfg_netdev - Allocate, configure and register a netdev
> > > * @vsi: the VSI associated with the new netdev
> > > @@ -5237,6 +5264,8 @@ static void ice_remove(struct pci_dev *pdev)
> > > msleep(100);
> > > }
> > > + ice_pf_fwlog_deinit(pf);
> > > +
> > > if (test_bit(ICE_FLAG_SRIOV_ENA, pf->flags)) {
> > > set_bit(ICE_VF_RESETS_DISABLED, pf->state);
> > > ice_free_vfs(pf);
> > > --
> > > 2.35.1
> > >
> > > _______________________________________________
> > > Intel-wired-lan mailing list
> > > Intel-wired-lan@osuosl.org
> > > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-wired-lan] [PATCH net-next v5 5/5] ice: use debugfs to output FW log data
2023-01-11 19:19 [Intel-wired-lan] [PATCH net-next v5 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
` (3 preceding siblings ...)
2023-01-11 19:19 ` [Intel-wired-lan] [PATCH net-next v5 4/5] ice: disable FW logging on driver unload Paul M Stillwell Jr
@ 2023-01-11 19:19 ` Paul M Stillwell Jr
4 siblings, 0 replies; 14+ messages in thread
From: Paul M Stillwell Jr @ 2023-01-11 19:19 UTC (permalink / raw)
To: intel-wired-lan
The FW log data can be quite large so we don't want to use syslog.
Instead take advantage of debugfs to write the data to.
The file is binary data and users should send them to us to
work with the FW team to decode them.
An example of how to retrieve the data using debugfs is:
cat /sys/kernel/debug/ice/0000\:18\:00.0/fwlog > fwlog
Also updated the documentation to add the new parameters.
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
---
Documentation/networking/devlink/ice.rst | 39 ++++++
drivers/net/ethernet/intel/ice/Makefile | 4 +-
drivers/net/ethernet/intel/ice/ice.h | 22 ++++
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 1 +
drivers/net/ethernet/intel/ice/ice_debugfs.c | 109 +++++++++++++++
drivers/net/ethernet/intel/ice/ice_main.c | 124 ++++++++++++++----
6 files changed, 271 insertions(+), 28 deletions(-)
create mode 100644 drivers/net/ethernet/intel/ice/ice_debugfs.c
diff --git a/Documentation/networking/devlink/ice.rst b/Documentation/networking/devlink/ice.rst
index 625efb3777d5..546a618ae7a2 100644
--- a/Documentation/networking/devlink/ice.rst
+++ b/Documentation/networking/devlink/ice.rst
@@ -7,6 +7,45 @@ ice devlink support
This document describes the devlink features implemented by the ``ice``
device driver.
+Parameters
+=============
+
+.. list-table:: Driver-specific parameters implemented
+ :widths: 5 5 5 85
+
+ * - Name
+ - Type
+ - Mode
+ - Description
+ * - ``fwlog_supported``
+ - Boolean
+ - runtime
+ - This parameter indicates to the user whether FW loggiing is supported
+ or not in the currently loaded FW.
+ * - ``fwlog_enabled``
+ - Boolean
+ - runtime
+ - This parameter indicates to the user whether the driver is currently
+ getting FW logs or not.
+ * - ``fwlog_level``
+ - u8
+ - runtime
+ - This parameter indicates the current log level. Each level includes the
+ messages from the previous/lower level. Valid values are
+
+ * ``0`` - no logging
+ * ``1`` - error logging
+ * ``2`` - warning logging
+ * ``3`` - normal logging
+ * ``4`` - verbose logging
+ * - ``fwlog_resolution``
+ - u8
+ - runtime
+ - This parameter indicates the number of log messages to included in a
+ single ARQ event. The range is 1-128 (1 means push every log message,
+ 128 means push only when the max AQ command buffer is full). The
+ suggested value is 10.
+
Info versions
=============
diff --git a/drivers/net/ethernet/intel/ice/Makefile b/drivers/net/ethernet/intel/ice/Makefile
index 6e4680ad097c..452a440a9810 100644
--- a/drivers/net/ethernet/intel/ice/Makefile
+++ b/drivers/net/ethernet/intel/ice/Makefile
@@ -34,7 +34,8 @@ ice-y := ice_main.o \
ice_ethtool.o \
ice_repr.o \
ice_tc_lib.o \
- ice_fwlog.o
+ ice_fwlog.o \
+ ice_debugfs.o
ice-$(CONFIG_PCI_IOV) += \
ice_sriov.o \
ice_virtchnl.o \
@@ -49,3 +50,4 @@ ice-$(CONFIG_RFS_ACCEL) += ice_arfs.o
ice-$(CONFIG_XDP_SOCKETS) += ice_xsk.o
ice-$(CONFIG_ICE_SWITCHDEV) += ice_eswitch.o
ice-$(CONFIG_ICE_GNSS) += ice_gnss.o
+ice-$(CONFIG_DEBUG_FS) += ice_debugfs.o
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index c3475834d536..b3e45d92cb20 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -549,6 +549,7 @@ struct ice_pf {
struct ice_vsi_stats **vsi_stats;
struct ice_sw *first_sw; /* first switch created by firmware */
u16 eswitch_mode; /* current mode of eswitch */
+ struct dentry *ice_debugfs_pf;
struct ice_vfs vfs;
DECLARE_BITMAP(features, ICE_F_MAX);
DECLARE_BITMAP(state, ICE_STATE_NBITS);
@@ -631,6 +632,8 @@ struct ice_pf {
#define ICE_VF_AGG_NODE_ID_START 65
#define ICE_MAX_VF_AGG_NODES 32
struct ice_agg_node vf_agg_node[ICE_MAX_VF_AGG_NODES];
+ struct list_head fwlog_data_list;
+ u8 fwlog_list_count;
};
struct ice_netdev_priv {
@@ -645,6 +648,15 @@ struct ice_netdev_priv {
struct list_head tc_indr_block_priv_list;
};
+struct ice_fwlog_data {
+ struct list_head list;
+ u16 data_size;
+ u8 *data;
+};
+
+/* define the maximum number of items that can be in the list */
+#define ICE_FWLOG_MAX_SIZE 128
+
/**
* ice_vector_ch_enabled
* @qv: pointer to q_vector, can be NULL
@@ -869,6 +881,16 @@ static inline bool ice_is_adq_active(struct ice_pf *pf)
return false;
}
+#ifdef CONFIG_DEBUG_FS
+void ice_debugfs_fwlog_init(struct ice_pf *pf);
+void ice_debugfs_init(void);
+void ice_debugfs_exit(void);
+#else
+static inline void ice_debugfs_fwlog_init(struct ice_pf *pf) { }
+static inline void ice_debugfs_init(void) { }
+static inline void ice_debugfs_exit(void) { }
+#endif /* CONFIG_DEBUG_FS */
+
bool netif_is_ice(struct net_device *dev);
int ice_vsi_setup_tx_rings(struct ice_vsi *vsi);
int ice_vsi_setup_rx_rings(struct ice_vsi *vsi);
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 1af036beeb45..27c2cea29c51 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -2377,6 +2377,7 @@ enum ice_adminq_opc {
ice_aqc_opc_fw_logs_config = 0xFF30,
ice_aqc_opc_fw_logs_register = 0xFF31,
ice_aqc_opc_fw_logs_query = 0xFF32,
+ ice_aqc_opc_fw_logs_event = 0xFF33,
};
#endif /* _ICE_ADMINQ_CMD_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_debugfs.c b/drivers/net/ethernet/intel/ice/ice_debugfs.c
new file mode 100644
index 000000000000..682bef0b62e8
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_debugfs.c
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022, Intel Corporation. */
+
+#include <linux/fs.h>
+#include <linux/debugfs.h>
+#include <linux/random.h>
+#include "ice.h"
+
+static struct dentry *ice_debugfs_root;
+
+/**
+ * ice_debugfs_command_read - read from command datum
+ * @filp: the opened file
+ * @buffer: where to write the data for the user to read
+ * @count: the size of the user's buffer
+ * @ppos: file position offset
+ */
+static ssize_t ice_debugfs_command_read(struct file *filp, char __user *buffer,
+ size_t count, loff_t *ppos)
+{
+ struct ice_pf *pf = filp->private_data;
+ struct device *dev = ice_pf_to_dev(pf);
+ struct ice_fwlog_data *log, *tmp_log;
+ int data_copied = 0;
+
+ if (list_empty(&pf->fwlog_data_list)) {
+ dev_info(dev, "FW log is empty\n");
+ return 0;
+ }
+
+ list_for_each_entry_safe(log, tmp_log, &pf->fwlog_data_list, list) {
+ u16 cur_buf_len = log->data_size;
+ int retval;
+
+ if (cur_buf_len > count)
+ break;
+
+ retval = copy_to_user(buffer, log->data, cur_buf_len);
+ if (retval)
+ return -EFAULT;
+
+ data_copied += cur_buf_len;
+ buffer += cur_buf_len;
+ count -= cur_buf_len;
+ *ppos += cur_buf_len;
+
+ /* don't delete the list element until we know it got copied */
+ kfree(log->data);
+ list_del(&log->list);
+ kfree(log);
+ pf->fwlog_list_count--;
+ }
+
+ return data_copied;
+}
+
+static const struct file_operations ice_debugfs_command_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .read = ice_debugfs_command_read,
+};
+
+/**
+ * ice_debugfs_fwlog_init - setup the debugfs directory
+ * @pf: the ice that is starting up
+ */
+void ice_debugfs_fwlog_init(struct ice_pf *pf)
+{
+ const char *name = pci_name(pf->pdev);
+ struct dentry *pfile;
+
+ /* only support fw log commands on PF 0 */
+ if (pf->hw.bus.func)
+ return;
+
+ pf->ice_debugfs_pf = debugfs_create_dir(name, ice_debugfs_root);
+ if (IS_ERR(pf->ice_debugfs_pf))
+ return;
+
+ pfile = debugfs_create_file("fwlog", 0400, pf->ice_debugfs_pf, pf,
+ &ice_debugfs_command_fops);
+ if (!pfile)
+ goto create_failed;
+
+ return;
+
+create_failed:
+ dev_err(ice_pf_to_dev(pf), "debugfs dir/file for %s failed\n", name);
+ debugfs_remove_recursive(pf->ice_debugfs_pf);
+}
+
+/**
+ * ice_debugfs_init - create root directory for debugfs entries
+ */
+void ice_debugfs_init(void)
+{
+ ice_debugfs_root = debugfs_create_dir(KBUILD_MODNAME, NULL);
+ if (IS_ERR(ice_debugfs_root))
+ pr_info("init of debugfs failed\n");
+}
+
+/**
+ * ice_debugfs_exit - remove debugfs entries
+ */
+void ice_debugfs_exit(void)
+{
+ debugfs_remove_recursive(ice_debugfs_root);
+ ice_debugfs_root = NULL;
+}
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 593efc064f5b..7bcc08a9e64f 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1212,6 +1212,43 @@ ice_handle_link_event(struct ice_pf *pf, struct ice_rq_event_info *event)
return status;
}
+/**
+ * ice_get_fwlog_data - copy the FW log data from ARQ event
+ * @pf: PF that the FW log event is associated with
+ * @event: event structure containing FW log data
+ */
+static void
+ice_get_fwlog_data(struct ice_pf *pf, struct ice_rq_event_info *event)
+{
+ struct device *dev = ice_pf_to_dev(pf);
+ struct ice_fwlog_data *fwlog;
+
+ if (pf->fwlog_list_count >= ICE_FWLOG_MAX_SIZE)
+ return;
+
+ fwlog = kzalloc(sizeof(*fwlog), GFP_KERNEL);
+ if (!fwlog) {
+ dev_warn(dev, "Couldn't allocate memory for FWlog element\n");
+ return;
+ }
+
+ INIT_LIST_HEAD(&fwlog->list);
+
+ fwlog->data_size = le16_to_cpu(event->desc.datalen);
+ fwlog->data = kzalloc(fwlog->data_size, GFP_KERNEL);
+ if (!fwlog->data) {
+ dev_warn(dev, "Couldn't allocate memory for FWlog data\n");
+ kfree(fwlog);
+ return;
+ }
+
+ memcpy(fwlog->data, event->msg_buf, fwlog->data_size);
+
+ list_add_tail(&fwlog->list, &pf->fwlog_data_list);
+
+ pf->fwlog_list_count++;
+}
+
enum ice_aq_task_state {
ICE_AQ_TASK_WAITING = 0,
ICE_AQ_TASK_COMPLETE,
@@ -1485,6 +1522,9 @@ static int __ice_clean_ctrlq(struct ice_pf *pf, enum ice_ctl_q q_type)
if (!ice_is_malicious_vf(pf, &event, i, pending))
ice_vc_process_vf_msg(pf, &event);
break;
+ case ice_aqc_opc_fw_logs_event:
+ ice_get_fwlog_data(pf, &event);
+ break;
case ice_aqc_opc_lldp_set_mib_change:
ice_dcb_process_lldp_set_mib_change(pf, &event);
break;
@@ -4500,33 +4540,6 @@ static void ice_unregister_netdev(struct ice_vsi *vsi)
clear_bit(ICE_VSI_NETDEV_REGISTERED, vsi->state);
}
-/**
- * ice_pf_fwlog_deinit - clear FW logging metadata on device exit
- * @pf: pointer to the PF struct
- */
-static void ice_pf_fwlog_deinit(struct ice_pf *pf)
-{
- struct ice_hw *hw = &pf->hw;
-
- /* make sure FW logging is disabled to not put the FW in a weird state
- * for the next driver load
- */
- if (hw->fwlog_ena) {
- int status;
-
- hw->fwlog_cfg.options &= ~ICE_FWLOG_OPTION_ARQ_ENA;
- status = ice_fwlog_set(hw, &hw->fwlog_cfg);
- if (status)
- dev_warn(ice_pf_to_dev(pf), "Unable to turn off FW logging, status: %d\n",
- status);
-
- status = ice_fwlog_unregister(hw);
- if (status)
- dev_warn(ice_pf_to_dev(pf), "Unable to unregister FW logging, status: %d\n",
- status);
- }
-}
-
/**
* ice_cfg_netdev - Allocate, configure and register a netdev
* @vsi: the VSI associated with the new netdev
@@ -4653,6 +4666,56 @@ static void ice_deinit_eth(struct ice_pf *pf)
ice_decfg_netdev(vsi);
}
+/**
+ * ice_pf_fwlog_init - initialize FW logging on device init
+ * @pf: pointer to the PF struct
+ */
+static void ice_pf_fwlog_init(struct ice_pf *pf)
+{
+ /* only supported on PF 0 */
+ if (pf->hw.bus.func)
+ return;
+
+ INIT_LIST_HEAD(&pf->fwlog_data_list);
+}
+
+/**
+ * ice_pf_fwlog_deinit - clear FW logging metadata on device exit
+ * @pf: pointer to the PF struct
+ */
+static void ice_pf_fwlog_deinit(struct ice_pf *pf)
+{
+ struct ice_fwlog_data *fwlog, *fwlog_tmp;
+ struct ice_hw *hw = &pf->hw;
+
+ /* only supported on PF 0 */
+ if (pf->hw.bus.func)
+ return;
+
+ /* make sure FW logging is disabled to not put the FW in a weird state
+ * for the next driver load
+ */
+ if (hw->fwlog_ena) {
+ int status;
+
+ hw->fwlog_cfg.options &= ~ICE_FWLOG_OPTION_ARQ_ENA;
+ status = ice_fwlog_set(hw, &hw->fwlog_cfg);
+ if (status)
+ dev_warn(ice_pf_to_dev(pf), "Unable to turn off FW logging, status: %d\n",
+ status);
+
+ status = ice_fwlog_unregister(hw);
+ if (status)
+ dev_warn(ice_pf_to_dev(pf), "Unable to unregister FW logging, status: %d\n",
+ status);
+ }
+
+ list_for_each_entry_safe(fwlog, fwlog_tmp, &pf->fwlog_data_list, list) {
+ kfree(fwlog->data);
+ kfree(fwlog);
+ }
+}
+
static int ice_init_dev(struct ice_pf *pf)
{
struct device *dev = ice_pf_to_dev(pf);
@@ -5156,6 +5219,9 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
hw->debug_mask = debug;
#endif
+ ice_pf_fwlog_init(pf);
+ ice_debugfs_fwlog_init(pf);
+
err = ice_init(pf);
if (err)
goto err_init;
@@ -5265,6 +5331,7 @@ static void ice_remove(struct pci_dev *pdev)
}
ice_pf_fwlog_deinit(pf);
+ ice_debugfs_exit();
if (test_bit(ICE_FLAG_SRIOV_ENA, pf->flags)) {
set_bit(ICE_VF_RESETS_DISABLED, pf->state);
@@ -5728,10 +5795,13 @@ static int __init ice_module_init(void)
return -ENOMEM;
}
+ ice_debugfs_init();
+
status = pci_register_driver(&ice_driver);
if (status) {
pr_err("failed to register PCI driver, err %d\n", status);
destroy_workqueue(ice_wq);
+ ice_debugfs_exit();
}
return status;
--
2.35.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 14+ messages in thread