* [Intel-wired-lan] [PATCH net-next v9 0/5] add v2 FW logging for ice driver
@ 2023-03-02 21:51 Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 1/5] ice: remove FW logging code Paul M Stillwell Jr
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Paul M Stillwell Jr @ 2023-03-02 21:51 UTC (permalink / raw)
To: intel-wired-lan
FW log support was added to the ice driver, but that version is no
longer supported. There is a newer version of FW logging (v2) that
adds more control knobs to get the exact data out of the FW
for debugging.
The interface for FW logging is debugfs. This was chosen based on a discussion
here: https://lore.kernel.org/netdev/20230214180712.53fc8ba2@kernel.org/
We talked about using devlink in a variety of ways, but none of those
options made any sense for the way the FW reports data. We briefly talked
about using ethtool, but that seemed to go by the wayside. Ultimately it
seems like using debugfs is the way to go so re-implement the code to use
that.
Additionally, instead of dumping the FW log output to syslog,
dump it to debugfs. The FW log data is really just binary
data that the FW log team decodes to determine what happens so the
translation from binary to some text output just slows things down
and results in potential dropped data. The structure for the debugfs
entry is: /sys/kernel/debug/ice/<pci device>/fwlog
Once enabled the FW log data is received as ARQ events that the driver
processes.
The FW logging is across all the PFs on the device, so restrict the
commands to only PF0.
The following debugfs commands are added:
- dump fwlog_cfg
- update fwlog_cfg <fwlog_level> <fwlog_events> <fwlog_resolution>
where
fwlog_level is a value from 0-4 as described below
Each level includes the messages from the previous/lower level
0 - no logging
1 - error logging
2 - warning logging
3 - normal logging
4 - verbose logging
fwlog_events is a value from 0-32 that represents the module to receive
events for. The values are sent as a hex value where each bit represents
a specific module. The module values are:
0 (0x00) - General
1 (0x01) - Control (Resets + Autoload)
2 (0x02) - Link Management
3 (0x03) - Link Topology Detection
4 (0x04) - DNL
5 (0x05) - I2C
6 (0x06) - SDP
7 (0x07) - MDIO
8 (0x08) - Admin Queue
9 (0x09) - HDMA
10 (0x0A) - LLDP
11 (0x0B) - DCBX
12 (0x0C) - DCB
13 (0x0D) - XLR
14 (0x0E) - NVM
15 (0x0F) - Authentication
16 (0x10) - VPD
17 (0x11) - IOSF
18 (0x12) - Parser
19 (0x13) - Switch
20 (0x14) - Scheduler
21 (0x15) - Tx Queue Management
22 (0x16) - Unsupported
23 (0x17) - Post
24 (0x18) - Watchdog
25 (0x19) - Task Dispatcher
26 (0x1A) - Manageability
27 (0x1B) - Synce
28 (0x1C) - Health
29 (0x1D) - Time Sync
30 (0x1E) - PF Registration
31 (0x1F) - Module Version
fwlog_resolution is 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.
---
v9:
- rewrote code to use debugfs instead of devlink
v8:
- added vmalloc.h file for correct prototypes
- moved code change from patch 5 to patch 3 where it was supposed to be
- fixed a style issue
v7:
- removed dev_info() in ice_debugfs_command_read() since it wasn't needed
- refactored ice_debugfs_command_read() to split the copying of the data and
the freeing of the buffers. This allows for better error recovery in case
the copy_to_user() fails
- changed allocation of fwlog buffers and structure from kernel memory to
virtual memory (vmalloc/vzalloc)
- fixed a compile bug
v6:
- removed cache_cfg() based on feedback
- a couple of other minor changes based on feedback
v5:
- handle devlink reload path correctly so debugfs directories don't get
added twice
- fix issue where code wrapped with CONFIG_DEBUG_FS was causing sparc
compile issues with multiple defines
v4:
- actually changed the modes in ice.rst for new params
v3:
- fixed ice.rst to have proper mode for new params and fixed formatting
v2:
- removed some unused admin queue commands
- updated copyright in ice_fwlog.[ch] to 2022
- moved defines in structures under the variables and added blank line
- removed a couple of unused defines
- changed fwlog_support_ena to fwlog_supported to be clearer
- consolidated ice_devlink_param_id enum together
- changed ice_fwlog_set_support_ena() to ice_fwlog_set_supported()
- consolidated return status logic in ice_devlink_fwlog_enabled_set()
- pull up functions in ice_fwlog.c where appropriate
- add newline for FW Logging Commands comment
- changed any new u[8/16] loop variables to int
- moved ice_pf_fwlog_deinit() from patch 5 to patch 4
- changed error message to be clearer
- updated Documentation/networking/devlink/ice.rst
- updated commit messages with examples of devlink commands and using
debugfs to get log files
Paul M Stillwell Jr (5):
ice: remove FW logging code
ice: enable debugfs to check FW logging status
ice: add ability to set FW log configuration
ice: enable FW logging based on stored configuration
ice: add ability to enable FW logging and capture data
drivers/net/ethernet/intel/ice/Makefile | 4 +-
drivers/net/ethernet/intel/ice/ice.h | 34 ++
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 160 +++----
drivers/net/ethernet/intel/ice/ice_common.c | 218 +---------
drivers/net/ethernet/intel/ice/ice_common.h | 1 -
drivers/net/ethernet/intel/ice/ice_debugfs.c | 391 ++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_fwlog.c | 369 +++++++++++++++++
drivers/net/ethernet/intel/ice/ice_fwlog.h | 57 +++
drivers/net/ethernet/intel/ice/ice_main.c | 136 +++++-
drivers/net/ethernet/intel/ice/ice_type.h | 22 +-
10 files changed, 1076 insertions(+), 316 deletions(-)
create mode 100644 drivers/net/ethernet/intel/ice/ice_debugfs.c
create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.c
create mode 100644 drivers/net/ethernet/intel/ice/ice_fwlog.h
--
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 [flat|nested] 16+ messages in thread
* [Intel-wired-lan] [PATCH net-next v9 1/5] ice: remove FW logging code
2023-03-02 21:51 [Intel-wired-lan] [PATCH net-next v9 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
@ 2023-03-02 21:51 ` Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 2/5] ice: enable debugfs to check FW logging status Paul M Stillwell Jr
` (3 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Paul M Stillwell Jr @ 2023-03-02 21:51 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 8dc1ae3f8e53..f476eb05ee36 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1519,9 +1519,6 @@ static int __ice_clean_ctrlq(struct ice_pf *pf, enum ice_ctl_q q_type)
ice_vc_process_vf_msg(pf, &event, &data);
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 a09556e57803..ce0b502365e9 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:
@@ -878,8 +860,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] 16+ messages in thread
* [Intel-wired-lan] [PATCH net-next v9 2/5] ice: enable debugfs to check FW logging status
2023-03-02 21:51 [Intel-wired-lan] [PATCH net-next v9 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 1/5] ice: remove FW logging code Paul M Stillwell Jr
@ 2023-03-02 21:51 ` Paul M Stillwell Jr
2023-03-03 23:21 ` Tony Nguyen
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration Paul M Stillwell Jr
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Paul M Stillwell Jr @ 2023-03-02 21:51 UTC (permalink / raw)
To: intel-wired-lan; +Cc: Brett Creeley
Users want the ability to debug FW issues by retrieving the
FW logs from the E8xx devices. Enable debugfs to query the driver
to see if the NVM image allows FW logging and to see if FW
logging is currently running.
To see the status of FW logging use 'dump fwlog_cfg' like this:
echo dump fwlog_cfg > /sys/kernel/debug/ice/0000\:18\:00.0/fwlog
Co-developed-by: Brett Creeley <brett.creeley@intel.com>
Signed-off-by: Brett Creeley <brett.creeley@intel.com>
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
---
drivers/net/ethernet/intel/ice/Makefile | 4 +-
drivers/net/ethernet/intel/ice/ice.h | 23 ++
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 80 +++++
drivers/net/ethernet/intel/ice/ice_common.c | 4 +
drivers/net/ethernet/intel/ice/ice_debugfs.c | 282 ++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_fwlog.c | 162 ++++++++++
drivers/net/ethernet/intel/ice/ice_fwlog.h | 54 ++++
drivers/net/ethernet/intel/ice/ice_main.c | 7 +
drivers/net/ethernet/intel/ice/ice_type.h | 4 +
9 files changed, 619 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/intel/ice/ice_debugfs.c
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 5d89392f969b..3290f594286e 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 \
@@ -48,3 +49,4 @@ ice-$(CONFIG_RFS_ACCEL) += ice_arfs.o
ice-$(CONFIG_XDP_SOCKETS) += ice_xsk.o
ice-$(CONFIG_ICE_SWITCHDEV) += ice_eswitch.o
ice-$(CONFIG_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 e809249500e1..f57ad5526b19 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -553,6 +553,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);
@@ -873,6 +874,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);
@@ -934,6 +945,18 @@ int ice_aq_wait_for_event(struct ice_pf *pf, u16 opcode, unsigned long timeout,
int ice_open(struct net_device *netdev);
int ice_open_internal(struct net_device *netdev);
int ice_stop(struct net_device *netdev);
+#ifdef CONFIG_DEBUG_FS
+int
+ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
+ unsigned long events);
+#else
+static int
+ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
+ unsigned long events)
+{
+ return -EOPNOTSUPP;
+}
+#endif /* CONFIG_DEBUG_FS */
void ice_service_task_schedule(struct ice_pf *pf);
int ice_load(struct ice_pf *pf);
void ice_unload(struct ice_pf *pf);
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index e1b95cc3a538..e57c0f1b6d87 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -2038,6 +2038,81 @@ 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)
+ * 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)
+#define ICE_AQC_FW_LOG_PERSISTENT BIT(0)
+ 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 +2189,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 +2370,10 @@ enum ice_adminq_opc {
/* Standalone Commands/Events */
ice_aqc_opc_event_lan_overflow = 0x1001,
+
+ /* FW Logging Commands */
+ ice_aqc_opc_fw_logs_config = 0xFF30,
+ 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..da55db0170dd 100644
--- a/drivers/net/ethernet/intel/ice/ice_common.c
+++ b/drivers/net/ethernet/intel/ice/ice_common.c
@@ -879,6 +879,10 @@ int ice_init_hw(struct ice_hw *hw)
if (status)
goto err_unroll_cqinit;
+ 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)
goto err_unroll_cqinit;
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..8a729217ef08
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_debugfs.c
@@ -0,0 +1,282 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022, Intel Corporation. */
+
+#include <linux/vmalloc.h>
+
+#include <linux/fs.h>
+#include <linux/debugfs.h>
+#include <linux/random.h>
+#include "ice.h"
+
+static struct dentry *ice_debugfs_root;
+
+static const char *module_id_to_name(u16 module_id)
+{
+ switch (module_id) {
+ case ICE_AQC_FW_LOG_ID_GENERAL:
+ return "General";
+ case ICE_AQC_FW_LOG_ID_CTRL:
+ return "Control (Resets + Autoload)";
+ case ICE_AQC_FW_LOG_ID_LINK:
+ return "Link Management";
+ case ICE_AQC_FW_LOG_ID_LINK_TOPO:
+ return "Link Topology Detection";
+ case ICE_AQC_FW_LOG_ID_DNL:
+ return "DNL";
+ case ICE_AQC_FW_LOG_ID_I2C:
+ return "I2C";
+ case ICE_AQC_FW_LOG_ID_SDP:
+ return "SDP";
+ case ICE_AQC_FW_LOG_ID_MDIO:
+ return "MDIO";
+ case ICE_AQC_FW_LOG_ID_ADMINQ:
+ return "Admin Queue";
+ case ICE_AQC_FW_LOG_ID_HDMA:
+ return "HDMA";
+ case ICE_AQC_FW_LOG_ID_LLDP:
+ return "LLDP";
+ case ICE_AQC_FW_LOG_ID_DCBX:
+ return "DCBX";
+ case ICE_AQC_FW_LOG_ID_DCB:
+ return "DCB";
+ case ICE_AQC_FW_LOG_ID_XLR:
+ return "XLR";
+ case ICE_AQC_FW_LOG_ID_NVM:
+ return "NVM";
+ case ICE_AQC_FW_LOG_ID_AUTH:
+ return "Authentication";
+ case ICE_AQC_FW_LOG_ID_VPD:
+ return "VPD";
+ case ICE_AQC_FW_LOG_ID_IOSF:
+ return "IOSF";
+ case ICE_AQC_FW_LOG_ID_PARSER:
+ return "Parser";
+ case ICE_AQC_FW_LOG_ID_SW:
+ return "Switch";
+ case ICE_AQC_FW_LOG_ID_SCHEDULER:
+ return "Scheduler";
+ case ICE_AQC_FW_LOG_ID_TXQ:
+ return "Tx Queue Management";
+ case ICE_AQC_FW_LOG_ID_POST:
+ return "Post";
+ case ICE_AQC_FW_LOG_ID_WATCHDOG:
+ return "Watchdog";
+ case ICE_AQC_FW_LOG_ID_TASK_DISPATCH:
+ return "Task Dispatcher";
+ case ICE_AQC_FW_LOG_ID_MNG:
+ return "Manageability";
+ case ICE_AQC_FW_LOG_ID_SYNCE:
+ return "Synce";
+ case ICE_AQC_FW_LOG_ID_HEALTH:
+ return "Health";
+ case ICE_AQC_FW_LOG_ID_TSDRV:
+ return "Time Sync";
+ case ICE_AQC_FW_LOG_ID_PFREG:
+ return "PF Registration";
+ case ICE_AQC_FW_LOG_ID_MDLVER:
+ return "Module Version";
+ default:
+ return "Unsupported";
+ }
+}
+
+static const char *log_level_to_name(u8 log_level)
+{
+ switch (log_level) {
+ case ICE_FWLOG_LEVEL_NONE:
+ return "None";
+ case ICE_FWLOG_LEVEL_ERROR:
+ return "Error";
+ case ICE_FWLOG_LEVEL_WARNING:
+ return "Warning";
+ case ICE_FWLOG_LEVEL_NORMAL:
+ return "Normal";
+ case ICE_FWLOG_LEVEL_VERBOSE:
+ return "Verbose";
+ default:
+ return "Unsupported";
+ }
+}
+
+static void ice_print_fwlog_config(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
+{
+ struct device *dev = ice_pf_to_dev((struct ice_pf *)(hw->back));
+ u16 i;
+
+ dev_info(dev, "Log_resolution: %d\n", cfg->log_resolution);
+ dev_info(dev, "Options: 0x%04x\n", cfg->options);
+ dev_info(dev, "\tarq_ena: %s\n",
+ (cfg->options &
+ ICE_FWLOG_OPTION_ARQ_ENA) ? "true" : "false");
+ dev_info(dev, "\tuart_ena: %s\n",
+ (cfg->options &
+ ICE_FWLOG_OPTION_UART_ENA) ? "true" : "false");
+ dev_info(dev, "\trunning: %s\n",
+ (cfg->options &
+ ICE_FWLOG_OPTION_IS_REGISTERED) ? "true" : "false");
+
+ dev_info(dev, "Module Entries:\n");
+ for (i = 0; i < ICE_AQC_FW_LOG_ID_MAX; i++) {
+ struct ice_fwlog_module_entry *entry =
+ &cfg->module_entries[i];
+
+ dev_info(dev, "\tModule ID %d (%s) Log Level %d (%s)\n",
+ entry->module_id, module_id_to_name(entry->module_id),
+ entry->log_level, log_level_to_name(entry->log_level));
+ }
+}
+
+/**
+ * ice_fwlog_dump_cfg - Dump current FW logging configuration
+ * @hw: pointer to the HW structure
+ */
+static void ice_fwlog_dump_cfg(struct ice_hw *hw)
+{
+ struct device *dev = ice_pf_to_dev((struct ice_pf *)(hw->back));
+ struct ice_fwlog_cfg *cfg;
+ int status;
+
+ cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
+ if (!cfg)
+ return;
+
+ status = ice_fwlog_get(hw, cfg);
+ if (status) {
+ kfree(cfg);
+ return;
+ }
+
+ dev_info(dev, "Running FWLOG Configuration:\n");
+ ice_print_fwlog_config(hw, cfg);
+
+ kfree(cfg);
+}
+
+/**
+ * ice_debugfs_command_write - write into command datum
+ * @filp: the opened file
+ * @buf: where to find the user's data
+ * @count: the length of the user's data
+ * @ppos: file position offset
+ */
+static ssize_t
+ice_debugfs_command_write(struct file *filp, const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct ice_pf *pf = filp->private_data;
+ struct device *dev = ice_pf_to_dev(pf);
+ char *cmd_buf, *cmd_buf_tmp;
+ ssize_t ret;
+ char **argv;
+ int argc;
+
+ /* don't allow commands if the FW doesn't support it */
+ if (!ice_fwlog_supported(&pf->hw))
+ return -EOPNOTSUPP;
+
+ /* don't allow partial writes */
+ if (*ppos != 0)
+ return 0;
+
+ cmd_buf = memdup_user(buf, count + 1);
+ if (IS_ERR(cmd_buf))
+ return PTR_ERR(cmd_buf);
+ cmd_buf[count] = '\0';
+
+ /* the cmd_buf has a newline at the end of the command so
+ * remove it
+ */
+ cmd_buf_tmp = strchr(cmd_buf, '\n');
+ if (cmd_buf_tmp) {
+ *cmd_buf_tmp = '\0';
+ count = (size_t)cmd_buf_tmp - (size_t)cmd_buf + 1;
+ }
+
+ argv = argv_split(GFP_KERNEL, cmd_buf, &argc);
+ if (!argv) {
+ ret = -ENOMEM;
+ goto err_copy_from_user;
+ }
+
+ if (argc == 2 && !strncmp(argv[0], "dump", 4) &&
+ !strncmp(argv[1], "fwlog_cfg", 9)) {
+ ice_fwlog_dump_cfg(&pf->hw);
+ } else {
+ dev_info(dev, "unknown or invalid command '%s'\n", cmd_buf);
+ dev_info(dev, "available commands\n");
+ dev_info(dev, "\t dump fwlog_cfg\n");
+ ret = -EINVAL;
+ goto command_write_error;
+ }
+
+ /* if we get here, nothing went wrong; return bytes copied */
+ ret = (ssize_t)count;
+
+command_write_error:
+ argv_free(argv);
+err_copy_from_user:
+ kfree(cmd_buf);
+
+ /* This function always consumes all of the written input, or produces
+ * an error. Check and enforce this. Otherwise, the write operation
+ * won't complete properly.
+ */
+ if (WARN_ON(ret != (ssize_t)count && ret >= 0))
+ ret = -EIO;
+
+ return ret;
+}
+
+static const struct file_operations ice_debugfs_command_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .write = ice_debugfs_command_write,
+};
+
+/**
+ * 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_fwlog.c b/drivers/net/ethernet/intel/ice/ice_fwlog.c
new file mode 100644
index 000000000000..4856c00e1e3a
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_fwlog.c
@@ -0,0 +1,162 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022, Intel Corporation. */
+
+#include "ice_common.h"
+#include "ice_fwlog.h"
+
+/**
+ * ice_fwlog_init - Initialize FW logging configuration
+ * @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)) {
+ /* read the current config from the FW and store it */
+ status = ice_fwlog_get(hw, &hw->fwlog_cfg);
+ if (status)
+ return status;
+ } else {
+ dev_warn(ice_hw_to_dev(hw), "FW logging is not supported in this NVM image. Please update the NVM to get FW log support\n");
+ }
+
+ return 0;
+}
+
+/**
+ * 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_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;
+
+ return 0;
+}
+
+/**
+ * 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 check to see if FW
+ * logging is supported which is what the driver is determining now
+ */
+ 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..9d60e77e2c5b
--- /dev/null
+++ b/drivers/net/ethernet/intel/ice/ice_fwlog.h
@@ -0,0 +1,54 @@
+/* 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);
+int ice_fwlog_init(struct ice_hw *hw);
+int ice_fwlog_get(struct ice_hw *hw, struct ice_fwlog_cfg *cfg);
+#endif /* _ICE_FWLOG_H_ */
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index f476eb05ee36..21937c6edf85 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5216,6 +5216,8 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent)
hw->debug_mask = debug;
#endif
+ ice_debugfs_fwlog_init(pf);
+
err = ice_init(pf);
if (err)
goto err_init;
@@ -5323,6 +5325,8 @@ static void ice_remove(struct pci_dev *pdev)
msleep(100);
}
+ ice_debugfs_exit();
+
if (test_bit(ICE_FLAG_SRIOV_ENA, pf->flags)) {
set_bit(ICE_VF_RESETS_DISABLED, pf->state);
ice_free_vfs(pf);
@@ -5784,10 +5788,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;
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index ce0b502365e9..dd3e11b7e3ed 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)
{
@@ -860,6 +861,9 @@ 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? */
+
/* 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] 16+ messages in thread
* [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration
2023-03-02 21:51 [Intel-wired-lan] [PATCH net-next v9 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 1/5] ice: remove FW logging code Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 2/5] ice: enable debugfs to check FW logging status Paul M Stillwell Jr
@ 2023-03-02 21:51 ` Paul M Stillwell Jr
2023-03-03 1:23 ` kernel test robot
2023-03-03 23:21 ` Tony Nguyen
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 4/5] ice: enable FW logging based on stored configuration Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 5/5] ice: add ability to enable FW logging and capture data Paul M Stillwell Jr
4 siblings, 2 replies; 16+ messages in thread
From: Paul M Stillwell Jr @ 2023-03-02 21:51 UTC (permalink / raw)
To: intel-wired-lan; +Cc: Brett Creeley
The FW log has several configuration options to control what gets output
and how often it gets output. Allow the user to set the log level,
events, and how often the FW generates messages with data in them
(resolution).
The command to set the configuration looks like:
update fwlog_cfg <fwlog_level> <fwlog_events> <fwlog_resolution>
where
fwlog_level is a value from 0-4 as described below
Each level includes the messages from the previous/lower level
0 - no logging
1 - error logging
2 - warning logging
3 - normal logging
4 - verbose logging
fwlog_events is a value from 0-32 that represents the module to receive
events for. The values are sent as a hex value where each bit represents
a specific module. The module values are:
0 (0x00) - General
1 (0x01) - Control (Resets + Autoload)
2 (0x02) - Link Management
3 (0x03) - Link Topology Detection
4 (0x04) - DNL
5 (0x05) - I2C
6 (0x06) - SDP
7 (0x07) - MDIO
8 (0x08) - Admin Queue
9 (0x09) - HDMA
10 (0x0A) - LLDP
11 (0x0B) - DCBX
12 (0x0C) - DCB
13 (0x0D) - XLR
14 (0x0E) - NVM
15 (0x0F) - Authentication
16 (0x10) - VPD
17 (0x11) - IOSF
18 (0x12) - Parser
19 (0x13) - Switch
20 (0x14) - Scheduler
21 (0x15) - Tx Queue Management
22 (0x16) - Unsupported
23 (0x17) - Post
24 (0x18) - Watchdog
25 (0x19) - Task Dispatcher
26 (0x1A) - Manageability
27 (0x1B) - Synce
28 (0x1C) - Health
29 (0x1D) - Time Sync
30 (0x1E) - PF Registration
31 (0x1F) - Module Version
fwlog_resolution is 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.
An example command to update the FW log configuration is:
echo update fwlog_cfg 4 0x82414821 50 > /sys/kernel/debug/ice/0000\:18\:00.0/fwlog
The above command would set the log level to 4 (Verbose) for the
following modules
0 (0x00) - General
5 (0x05) - I2C
11 (0x0B) - DCBX
14 (0x0E) - NVM
16 (0x10) - VPD
22 (0x16) - Unsupported
25 (0x19) - Task Dispatcher
31 (0x1F) - Module Version
The setting of the configuration does NOT enable FW logging, that is a
separate command. There are effectively 2 configurations at this point:
the one that is currently running and the staged configuration.
To allow the user to see what is going on the 'dump fwlog_cfg' command
now shows the 'Running' config and the 'Staged' config. This allows the
user to be able to compare the currently running configuration with
what will be enabled when the user executes the command to enable the
staged configuration.
Co-developed-by: Brett Creeley <brett.creeley@intel.com>
Signed-off-by: Brett Creeley <brett.creeley@intel.com>
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
---
drivers/net/ethernet/intel/ice/ice_debugfs.c | 35 ++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_main.c | 37 ++++++++++++++++++++
2 files changed, 72 insertions(+)
diff --git a/drivers/net/ethernet/intel/ice/ice_debugfs.c b/drivers/net/ethernet/intel/ice/ice_debugfs.c
index 8a729217ef08..1df79d7d99eb 100644
--- a/drivers/net/ethernet/intel/ice/ice_debugfs.c
+++ b/drivers/net/ethernet/intel/ice/ice_debugfs.c
@@ -148,6 +148,8 @@ static void ice_fwlog_dump_cfg(struct ice_hw *hw)
dev_info(dev, "Running FWLOG Configuration:\n");
ice_print_fwlog_config(hw, cfg);
+ dev_info(dev, "Staged FWLOG Configuration:\n");
+ ice_print_fwlog_config(hw, &hw->fwlog_cfg);
kfree(cfg);
}
@@ -201,10 +203,43 @@ ice_debugfs_command_write(struct file *filp, const char __user *buf,
if (argc == 2 && !strncmp(argv[0], "dump", 4) &&
!strncmp(argv[1], "fwlog_cfg", 9)) {
ice_fwlog_dump_cfg(&pf->hw);
+ } else if (argc == 5 && !strncmp(argv[0], "update", 6) &&
+ !strncmp(argv[1], "fwlog_cfg", 9)) {
+ struct ice_hw *hw = &pf->hw;
+ u8 log_level, resolution;
+ unsigned long events;
+
+ ret = kstrtou8(argv[2], 0, &log_level);
+ if (ret)
+ goto command_write_error;
+
+ ret = kstrtoul(argv[3], 0, &events);
+ if (ret)
+ goto command_write_error;
+
+ ret = kstrtou8(argv[4], 0, &resolution);
+ if (ret)
+ goto command_write_error;
+
+ if (resolution < ICE_AQC_FW_LOG_MIN_RESOLUTION ||
+ resolution > ICE_AQC_FW_LOG_MAX_RESOLUTION) {
+ dev_err(dev, "Invalid FW log resolution %d, value must be between %d - %d\n",
+ resolution, ICE_AQC_FW_LOG_MIN_RESOLUTION,
+ ICE_AQC_FW_LOG_MAX_RESOLUTION);
+ ret = -EINVAL;
+ goto command_write_error;
+ }
+
+ ret = ice_pf_fwlog_update_modules(pf, log_level, events);
+ if (ret)
+ goto command_write_error;
+
+ hw->fwlog_cfg.log_resolution = resolution;
} else {
dev_info(dev, "unknown or invalid command '%s'\n", cmd_buf);
dev_info(dev, "available commands\n");
dev_info(dev, "\t dump fwlog_cfg\n");
+ dev_info(dev, "\t update fwlog_cfg <fwlog_level> <fwlog_events> <fwlog_resolution>");
ret = -EINVAL;
goto command_write_error;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 21937c6edf85..3610c56748b7 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4552,6 +4552,43 @@ static void ice_print_wake_reason(struct ice_pf *pf)
dev_info(ice_pf_to_dev(pf), "Wake reason: %s", wake_str);
}
+/**
+ * ice_pf_fwlog_update_modules - update 1 or more modules via debugfs
+ * @pf: pointer to the PF struct
+ * @log_level: log_level to use for the @events
+ * @events: events to update
+ */
+int ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
+ unsigned long events)
+{
+ struct ice_fwlog_module_entry *entries;
+ u16 module_id, max_bits;
+ struct ice_hw *hw = &pf->hw;
+
+ if (log_level >= ICE_FWLOG_LEVEL_INVALID) {
+ dev_err(ice_pf_to_dev(pf), "Invalid FW log level %u, all level(s) >= %u are invalid\n",
+ log_level, ICE_FWLOG_LEVEL_INVALID);
+ return -EINVAL;
+ }
+
+ if (events >= BIT(ICE_AQC_FW_LOG_ID_MAX)) {
+ dev_err(ice_pf_to_dev(pf), "Invalid FW log events 0x%lx, all FW log event bits >= 0x%lx are invalid\n",
+ events, BIT(ICE_AQC_FW_LOG_ID_MAX));
+ return -EINVAL;
+ }
+
+ entries = (struct ice_fwlog_module_entry *)hw->fwlog_cfg.module_entries;
+
+ max_bits = min_t(u16, BITS_PER_TYPE(unsigned long),
+ ICE_AQC_FW_LOG_ID_MAX);
+
+ for_each_set_bit(module_id, &events, max_bits) {
+ entries[module_id].log_level = log_level;
+ }
+
+ return 0;
+}
+
/**
* ice_register_netdev - register netdev
* @vsi: pointer to the VSI struct
--
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] 16+ messages in thread
* [Intel-wired-lan] [PATCH net-next v9 4/5] ice: enable FW logging based on stored configuration
2023-03-02 21:51 [Intel-wired-lan] [PATCH net-next v9 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
` (2 preceding siblings ...)
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration Paul M Stillwell Jr
@ 2023-03-02 21:51 ` Paul M Stillwell Jr
2023-03-03 23:22 ` Tony Nguyen
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 5/5] ice: add ability to enable FW logging and capture data Paul M Stillwell Jr
4 siblings, 1 reply; 16+ messages in thread
From: Paul M Stillwell Jr @ 2023-03-02 21:51 UTC (permalink / raw)
To: intel-wired-lan; +Cc: Brett Creeley
Give users the ability to enable/disable the FW logging configuration
that is staged. Enabling the logging causes ARQ events to start
being received by the driver with FW logging data in them.
Co-developed-by: Brett Creeley <brett.creeley@intel.com>
Signed-off-by: Brett Creeley <brett.creeley@intel.com>
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
---
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 1 +
drivers/net/ethernet/intel/ice/ice_debugfs.c | 24 ++
drivers/net/ethernet/intel/ice/ice_fwlog.c | 207 ++++++++++++++++++
drivers/net/ethernet/intel/ice/ice_fwlog.h | 3 +
4 files changed, 235 insertions(+)
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index e57c0f1b6d87..a611e0d98bd0 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -2373,6 +2373,7 @@ enum ice_adminq_opc {
/* 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_debugfs.c b/drivers/net/ethernet/intel/ice/ice_debugfs.c
index 1df79d7d99eb..cead1c71ff50 100644
--- a/drivers/net/ethernet/intel/ice/ice_debugfs.c
+++ b/drivers/net/ethernet/intel/ice/ice_debugfs.c
@@ -235,11 +235,35 @@ ice_debugfs_command_write(struct file *filp, const char __user *buf,
goto command_write_error;
hw->fwlog_cfg.log_resolution = resolution;
+ } else if (argc == 2 && !strncmp(argv[0], "enable", 6) &&
+ !strncmp(argv[1], "fwlog", 5)) {
+ struct ice_hw *hw = &pf->hw;
+
+ hw->fwlog_cfg.options |= ICE_FWLOG_OPTION_ARQ_ENA;
+
+ ret = ice_fwlog_set(hw, &hw->fwlog_cfg);
+ if (ret)
+ goto command_write_error;
+
+ ret = ice_fwlog_register(hw);
+ } else if (argc == 2 && !strncmp(argv[0], "disable", 7) &&
+ !strncmp(argv[1], "fwlog", 5)) {
+ struct ice_hw *hw = &pf->hw;
+
+ hw->fwlog_cfg.options &= ~ICE_FWLOG_OPTION_ARQ_ENA;
+
+ ret = ice_fwlog_set(hw, &hw->fwlog_cfg);
+ if (ret)
+ goto command_write_error;
+
+ ret = ice_fwlog_unregister(hw);
} else {
dev_info(dev, "unknown or invalid command '%s'\n", cmd_buf);
dev_info(dev, "available commands\n");
dev_info(dev, "\t dump fwlog_cfg\n");
dev_info(dev, "\t update fwlog_cfg <fwlog_level> <fwlog_events> <fwlog_resolution>");
+ dev_info(dev, "\t enable fwlog\n");
+ dev_info(dev, "\t disable fwlog\n");
ret = -EINVAL;
goto command_write_error;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.c b/drivers/net/ethernet/intel/ice/ice_fwlog.c
index 4856c00e1e3a..79aa01dde190 100644
--- a/drivers/net/ethernet/intel/ice/ice_fwlog.c
+++ b/drivers/net/ethernet/intel/ice/ice_fwlog.c
@@ -4,6 +4,72 @@
#include "ice_common.h"
#include "ice_fwlog.h"
+/**
+ * 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;
+ }
+
+ return valid_module_entries(hw, cfg->module_entries,
+ ICE_AQC_FW_LOG_ID_MAX);
+}
+
/**
* ice_fwlog_init - Initialize FW logging configuration
* @hw: pointer to the HW structure
@@ -41,6 +107,87 @@ bool ice_fwlog_supported(struct ice_hw *hw)
return hw->fwlog_supported;
}
+/**
+ * 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_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);
+
+ return status;
+}
+
/**
* ice_aq_fwlog_get - Get the current firmware logging configuration (0xFF32)
* @hw: pointer to the HW structure
@@ -127,6 +274,66 @@ int ice_fwlog_get(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
return 0;
}
+/**
+ * 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_fwlog_set_supported - Set if FW logging is supported by FW
* @hw: pointer to the HW struct
diff --git a/drivers/net/ethernet/intel/ice/ice_fwlog.h b/drivers/net/ethernet/intel/ice/ice_fwlog.h
index 9d60e77e2c5b..fcfceb9f6ec2 100644
--- a/drivers/net/ethernet/intel/ice/ice_fwlog.h
+++ b/drivers/net/ethernet/intel/ice/ice_fwlog.h
@@ -50,5 +50,8 @@ 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_ */
--
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] 16+ messages in thread
* [Intel-wired-lan] [PATCH net-next v9 5/5] ice: add ability to enable FW logging and capture data
2023-03-02 21:51 [Intel-wired-lan] [PATCH net-next v9 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
` (3 preceding siblings ...)
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 4/5] ice: enable FW logging based on stored configuration Paul M Stillwell Jr
@ 2023-03-02 21:51 ` Paul M Stillwell Jr
2023-03-03 23:23 ` Tony Nguyen
4 siblings, 1 reply; 16+ messages in thread
From: Paul M Stillwell Jr @ 2023-03-02 21:51 UTC (permalink / raw)
To: intel-wired-lan
Once the FW logging is configured then users need to be able to enable
logging and capture the data. To enable/disable FW logging use 'enable
fwlog' and 'disable fwlog'. An example command to enable FW logging:
echo enable fwlog > /sys/kernel/debug/ice/0000\:18\:00.0/fwlog
Once logging is enable the user should read the data using something
like:
cat /sys/kernel/debug/ice/0000\:18\:00.0/fwlog > log_data.bin
The output file is a binary file that will be decoded by the FW
team for debugging.
Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
---
drivers/net/ethernet/intel/ice/ice.h | 11 +++
.../net/ethernet/intel/ice/ice_adminq_cmd.h | 3 +
drivers/net/ethernet/intel/ice/ice_debugfs.c | 50 +++++++++++
drivers/net/ethernet/intel/ice/ice_main.c | 89 +++++++++++++++++++
4 files changed, 153 insertions(+)
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index f57ad5526b19..9d2bc06ed529 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -636,6 +636,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 {
@@ -650,6 +652,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
diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index a611e0d98bd0..68ab19678dbe 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -2075,7 +2075,9 @@ enum ice_aqc_fw_logging_mod {
};
/* Set FW Logging configuration (indirect 0xFF30)
+ * Register for FW Logging (indirect 0xFF31)
* Query FW Logging (indirect 0xFF32)
+ * FW Log Event (indirect 0xFF33)
*/
struct ice_aqc_fw_log {
u8 cmd_flags;
@@ -2375,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
index cead1c71ff50..7952cfafaa66 100644
--- a/drivers/net/ethernet/intel/ice/ice_debugfs.c
+++ b/drivers/net/ethernet/intel/ice/ice_debugfs.c
@@ -10,6 +10,55 @@
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 ice_fwlog_data *log, *tmp_log;
+ int data_copied = 0;
+ int copy_count = 0;
+
+ if (list_empty(&pf->fwlog_data_list))
+ return 0;
+
+ list_for_each_entry(log, &pf->fwlog_data_list, list) {
+ u16 cur_buf_len = log->data_size;
+
+ if (cur_buf_len >= count)
+ break;
+
+ if (copy_to_user(buffer, log->data, cur_buf_len))
+ return -EFAULT;
+
+ data_copied += cur_buf_len;
+ buffer += cur_buf_len;
+ count -= cur_buf_len;
+ *ppos += cur_buf_len;
+ copy_count++;
+ }
+
+ /* only free the data once we know there weren't any errors */
+ list_for_each_entry_safe(log, tmp_log, &pf->fwlog_data_list, list) {
+ if (!copy_count)
+ break;
+
+ vfree(log->data);
+ list_del(&log->list);
+ vfree(log);
+ pf->fwlog_list_count--;
+ copy_count--;
+ }
+
+ return data_copied;
+}
+
static const char *module_id_to_name(u16 module_id)
{
switch (module_id) {
@@ -289,6 +338,7 @@ ice_debugfs_command_write(struct file *filp, const char __user *buf,
static const struct file_operations ice_debugfs_command_fops = {
.owner = THIS_MODULE,
.open = simple_open,
+ .read = ice_debugfs_command_read,
.write = ice_debugfs_command_write,
};
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 3610c56748b7..92ff1292f8fe 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -5,6 +5,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/vmalloc.h>
#include <generated/utsrelease.h>
#include "ice.h"
#include "ice_base.h"
@@ -1239,6 +1240,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 = vmalloc(sizeof(*fwlog));
+ 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 = vzalloc(fwlog->data_size);
+ if (!fwlog->data) {
+ dev_warn(dev, "Couldn't allocate memory for FWlog data\n");
+ vfree(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,
@@ -1519,6 +1557,9 @@ static int __ice_clean_ctrlq(struct ice_pf *pf, enum ice_ctl_q q_type)
ice_vc_process_vf_msg(pf, &event, &data);
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;
@@ -4746,6 +4787,52 @@ 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;
+ int err;
+
+ /* 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
+ */
+ hw->fwlog_cfg.options &= ~ICE_FWLOG_OPTION_ARQ_ENA;
+ err = ice_fwlog_set(hw, &hw->fwlog_cfg);
+ if (err)
+ dev_warn(ice_pf_to_dev(pf), "Unable to turn off FW logging, status: %d\n",
+ err);
+
+ err = ice_fwlog_unregister(hw);
+ if (err)
+ dev_warn(ice_pf_to_dev(pf), "Unable to unregister FW logging, status: %d\n",
+ err);
+
+ list_for_each_entry_safe(fwlog, fwlog_tmp, &pf->fwlog_data_list, list) {
+ vfree(fwlog->data);
+ vfree(fwlog);
+ }
+}
static int ice_init_dev(struct ice_pf *pf)
{
struct device *dev = ice_pf_to_dev(pf);
@@ -5253,6 +5340,7 @@ 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);
@@ -5362,6 +5450,7 @@ static void ice_remove(struct pci_dev *pdev)
msleep(100);
}
+ ice_pf_fwlog_deinit(pf);
ice_debugfs_exit();
if (test_bit(ICE_FLAG_SRIOV_ENA, pf->flags)) {
--
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] 16+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration Paul M Stillwell Jr
@ 2023-03-03 1:23 ` kernel test robot
2023-03-03 23:21 ` Tony Nguyen
1 sibling, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-03-03 1:23 UTC (permalink / raw)
To: Paul M Stillwell Jr, intel-wired-lan; +Cc: llvm, oe-kbuild-all, Brett Creeley
Hi Paul,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tnguy-next-queue/dev-queue]
[also build test WARNING on linus/master next-20230302]
[cannot apply to net-next/master v6.2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Paul-M-Stillwell-Jr/ice-remove-FW-logging-code/20230303-055305
base: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue
patch link: https://lore.kernel.org/r/20230302215109.124-4-paul.m.stillwell.jr%40intel.com
patch subject: [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration
config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20230303/202303030943.vv0bsyRo-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/57ec67cfb978253912effb7408aa33e880c479c7
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Paul-M-Stillwell-Jr/ice-remove-FW-logging-code/20230303-055305
git checkout 57ec67cfb978253912effb7408aa33e880c479c7
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/ethernet/intel/ice/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303030943.vv0bsyRo-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/intel/ice/ice_main.c:4574:16: warning: shift count >= width of type [-Wshift-count-overflow]
if (events >= BIT(ICE_AQC_FW_LOG_ID_MAX)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~
include/vdso/bits.h:7:26: note: expanded from macro 'BIT'
#define BIT(nr) (UL(1) << (nr))
^ ~~~~
drivers/net/ethernet/intel/ice/ice_main.c:4576:12: warning: shift count >= width of type [-Wshift-count-overflow]
events, BIT(ICE_AQC_FW_LOG_ID_MAX));
^~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:144:65: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
^~~~~~~~~~~
include/vdso/bits.h:7:26: note: expanded from macro 'BIT'
#define BIT(nr) (UL(1) << (nr))
^ ~~~~
2 warnings generated.
vim +4574 drivers/net/ethernet/intel/ice/ice_main.c
4554
4555 /**
4556 * ice_pf_fwlog_update_modules - update 1 or more modules via debugfs
4557 * @pf: pointer to the PF struct
4558 * @log_level: log_level to use for the @events
4559 * @events: events to update
4560 */
4561 int ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
4562 unsigned long events)
4563 {
4564 struct ice_fwlog_module_entry *entries;
4565 u16 module_id, max_bits;
4566 struct ice_hw *hw = &pf->hw;
4567
4568 if (log_level >= ICE_FWLOG_LEVEL_INVALID) {
4569 dev_err(ice_pf_to_dev(pf), "Invalid FW log level %u, all level(s) >= %u are invalid\n",
4570 log_level, ICE_FWLOG_LEVEL_INVALID);
4571 return -EINVAL;
4572 }
4573
> 4574 if (events >= BIT(ICE_AQC_FW_LOG_ID_MAX)) {
4575 dev_err(ice_pf_to_dev(pf), "Invalid FW log events 0x%lx, all FW log event bits >= 0x%lx are invalid\n",
4576 events, BIT(ICE_AQC_FW_LOG_ID_MAX));
4577 return -EINVAL;
4578 }
4579
4580 entries = (struct ice_fwlog_module_entry *)hw->fwlog_cfg.module_entries;
4581
4582 max_bits = min_t(u16, BITS_PER_TYPE(unsigned long),
4583 ICE_AQC_FW_LOG_ID_MAX);
4584
4585 for_each_set_bit(module_id, &events, max_bits) {
4586 entries[module_id].log_level = log_level;
4587 }
4588
4589 return 0;
4590 }
4591
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration
@ 2023-03-03 1:23 ` kernel test robot
0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2023-03-03 1:23 UTC (permalink / raw)
To: Paul M Stillwell Jr, intel-wired-lan; +Cc: llvm, Brett Creeley, oe-kbuild-all
Hi Paul,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tnguy-next-queue/dev-queue]
[also build test WARNING on linus/master next-20230302]
[cannot apply to net-next/master v6.2]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Paul-M-Stillwell-Jr/ice-remove-FW-logging-code/20230303-055305
base: https://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue
patch link: https://lore.kernel.org/r/20230302215109.124-4-paul.m.stillwell.jr%40intel.com
patch subject: [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration
config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20230303/202303030943.vv0bsyRo-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/57ec67cfb978253912effb7408aa33e880c479c7
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Paul-M-Stillwell-Jr/ice-remove-FW-logging-code/20230303-055305
git checkout 57ec67cfb978253912effb7408aa33e880c479c7
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/net/ethernet/intel/ice/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303030943.vv0bsyRo-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/net/ethernet/intel/ice/ice_main.c:4574:16: warning: shift count >= width of type [-Wshift-count-overflow]
if (events >= BIT(ICE_AQC_FW_LOG_ID_MAX)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~
include/vdso/bits.h:7:26: note: expanded from macro 'BIT'
#define BIT(nr) (UL(1) << (nr))
^ ~~~~
drivers/net/ethernet/intel/ice/ice_main.c:4576:12: warning: shift count >= width of type [-Wshift-count-overflow]
events, BIT(ICE_AQC_FW_LOG_ID_MAX));
^~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:144:65: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
^~~~~~~~~~~
include/vdso/bits.h:7:26: note: expanded from macro 'BIT'
#define BIT(nr) (UL(1) << (nr))
^ ~~~~
2 warnings generated.
vim +4574 drivers/net/ethernet/intel/ice/ice_main.c
4554
4555 /**
4556 * ice_pf_fwlog_update_modules - update 1 or more modules via debugfs
4557 * @pf: pointer to the PF struct
4558 * @log_level: log_level to use for the @events
4559 * @events: events to update
4560 */
4561 int ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
4562 unsigned long events)
4563 {
4564 struct ice_fwlog_module_entry *entries;
4565 u16 module_id, max_bits;
4566 struct ice_hw *hw = &pf->hw;
4567
4568 if (log_level >= ICE_FWLOG_LEVEL_INVALID) {
4569 dev_err(ice_pf_to_dev(pf), "Invalid FW log level %u, all level(s) >= %u are invalid\n",
4570 log_level, ICE_FWLOG_LEVEL_INVALID);
4571 return -EINVAL;
4572 }
4573
> 4574 if (events >= BIT(ICE_AQC_FW_LOG_ID_MAX)) {
4575 dev_err(ice_pf_to_dev(pf), "Invalid FW log events 0x%lx, all FW log event bits >= 0x%lx are invalid\n",
4576 events, BIT(ICE_AQC_FW_LOG_ID_MAX));
4577 return -EINVAL;
4578 }
4579
4580 entries = (struct ice_fwlog_module_entry *)hw->fwlog_cfg.module_entries;
4581
4582 max_bits = min_t(u16, BITS_PER_TYPE(unsigned long),
4583 ICE_AQC_FW_LOG_ID_MAX);
4584
4585 for_each_set_bit(module_id, &events, max_bits) {
4586 entries[module_id].log_level = log_level;
4587 }
4588
4589 return 0;
4590 }
4591
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v9 2/5] ice: enable debugfs to check FW logging status
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 2/5] ice: enable debugfs to check FW logging status Paul M Stillwell Jr
@ 2023-03-03 23:21 ` Tony Nguyen
2023-03-06 22:00 ` Paul M Stillwell Jr
0 siblings, 1 reply; 16+ messages in thread
From: Tony Nguyen @ 2023-03-03 23:21 UTC (permalink / raw)
To: Paul M Stillwell Jr, intel-wired-lan; +Cc: Brett Creeley
On 3/2/2023 1:51 PM, Paul M Stillwell Jr wrote:
[...]
> +#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);
> @@ -934,6 +945,18 @@ int ice_aq_wait_for_event(struct ice_pf *pf, u16 opcode, unsigned long timeout,
> int ice_open(struct net_device *netdev);
> int ice_open_internal(struct net_device *netdev);
> int ice_stop(struct net_device *netdev);
> +#ifdef CONFIG_DEBUG_FS
> +int
> +ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
> + unsigned long events);
> +#else
> +static int
> +ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
> + unsigned long events)
> +{
> + return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_DEBUG_FS */
This could be put with the ifdef above
> void ice_service_task_schedule(struct ice_pf *pf);
> int ice_load(struct ice_pf *pf);
> void ice_unload(struct ice_pf *pf);
[...]
> +/* Set FW Logging configuration (indirect 0xFF30)
> + * 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)
> +#define ICE_AQC_FW_LOG_PERSISTENT BIT(0)
> + u8 rsp_flag;
Please add a newline between the member and the defines that relate to
it. Please check this for other instances/needs as well.
[...]
> +#include <linux/vmalloc.h>
> +
Any particular reason this isn't with everything else (and alphabetized)?
> +#include <linux/fs.h>
> +#include <linux/debugfs.h>
> +#include <linux/random.h>
> +#include "ice.h"
> +
> +static struct dentry *ice_debugfs_root;
> +
> +static const char *module_id_to_name(u16 module_id)
> +{
> + switch (module_id) {
> + case ICE_AQC_FW_LOG_ID_GENERAL:
> + return "General";
> + case ICE_AQC_FW_LOG_ID_CTRL:
> + return "Control (Resets + Autoload)";
> + case ICE_AQC_FW_LOG_ID_LINK:
> + return "Link Management";
> + case ICE_AQC_FW_LOG_ID_LINK_TOPO:
> + return "Link Topology Detection";
> + case ICE_AQC_FW_LOG_ID_DNL:
> + return "DNL";
> + case ICE_AQC_FW_LOG_ID_I2C:
> + return "I2C";
> + case ICE_AQC_FW_LOG_ID_SDP:
> + return "SDP";
> + case ICE_AQC_FW_LOG_ID_MDIO:
> + return "MDIO";
> + case ICE_AQC_FW_LOG_ID_ADMINQ:
> + return "Admin Queue";
> + case ICE_AQC_FW_LOG_ID_HDMA:
> + return "HDMA";
> + case ICE_AQC_FW_LOG_ID_LLDP:
> + return "LLDP";
> + case ICE_AQC_FW_LOG_ID_DCBX:
> + return "DCBX";
> + case ICE_AQC_FW_LOG_ID_DCB:
> + return "DCB";
> + case ICE_AQC_FW_LOG_ID_XLR:
> + return "XLR";
> + case ICE_AQC_FW_LOG_ID_NVM:
> + return "NVM";
> + case ICE_AQC_FW_LOG_ID_AUTH:
> + return "Authentication";
> + case ICE_AQC_FW_LOG_ID_VPD:
> + return "VPD";
> + case ICE_AQC_FW_LOG_ID_IOSF:
> + return "IOSF";
> + case ICE_AQC_FW_LOG_ID_PARSER:
> + return "Parser";
> + case ICE_AQC_FW_LOG_ID_SW:
> + return "Switch";
> + case ICE_AQC_FW_LOG_ID_SCHEDULER:
> + return "Scheduler";
> + case ICE_AQC_FW_LOG_ID_TXQ:
> + return "Tx Queue Management";
> + case ICE_AQC_FW_LOG_ID_POST:
> + return "Post";
> + case ICE_AQC_FW_LOG_ID_WATCHDOG:
> + return "Watchdog";
> + case ICE_AQC_FW_LOG_ID_TASK_DISPATCH:
> + return "Task Dispatcher";
> + case ICE_AQC_FW_LOG_ID_MNG:
> + return "Manageability";
> + case ICE_AQC_FW_LOG_ID_SYNCE:
> + return "Synce";
> + case ICE_AQC_FW_LOG_ID_HEALTH:
> + return "Health";
> + case ICE_AQC_FW_LOG_ID_TSDRV:
> + return "Time Sync";
> + case ICE_AQC_FW_LOG_ID_PFREG:
> + return "PF Registration";
> + case ICE_AQC_FW_LOG_ID_MDLVER:
> + return "Module Version";
> + default:
> + return "Unsupported";
> + }
> +}
> +
> +static const char *log_level_to_name(u8 log_level)
> +{
> + switch (log_level) {
> + case ICE_FWLOG_LEVEL_NONE:
> + return "None";
> + case ICE_FWLOG_LEVEL_ERROR:
> + return "Error";
> + case ICE_FWLOG_LEVEL_WARNING:
> + return "Warning";
> + case ICE_FWLOG_LEVEL_NORMAL:
> + return "Normal";
> + case ICE_FWLOG_LEVEL_VERBOSE:
> + return "Verbose";
> + default:
> + return "Unsupported";
> + }
> +}
> +
> +static void ice_print_fwlog_config(struct ice_hw *hw, struct ice_fwlog_cfg *cfg)
> +{
> + struct device *dev = ice_pf_to_dev((struct ice_pf *)(hw->back));
I don't believe this casting is needed.
> + u16 i;
> +
> + dev_info(dev, "Log_resolution: %d\n", cfg->log_resolution);
> + dev_info(dev, "Options: 0x%04x\n", cfg->options);
> + dev_info(dev, "\tarq_ena: %s\n",
> + (cfg->options &
> + ICE_FWLOG_OPTION_ARQ_ENA) ? "true" : "false");
> + dev_info(dev, "\tuart_ena: %s\n",
> + (cfg->options &
> + ICE_FWLOG_OPTION_UART_ENA) ? "true" : "false");
> + dev_info(dev, "\trunning: %s\n",
> + (cfg->options &
> + ICE_FWLOG_OPTION_IS_REGISTERED) ? "true" : "false");
> +
> + dev_info(dev, "Module Entries:\n");
> + for (i = 0; i < ICE_AQC_FW_LOG_ID_MAX; i++) {
> + struct ice_fwlog_module_entry *entry =
> + &cfg->module_entries[i];
> +
> + dev_info(dev, "\tModule ID %d (%s) Log Level %d (%s)\n",
> + entry->module_id, module_id_to_name(entry->module_id),
> + entry->log_level, log_level_to_name(entry->log_level));
> + }
> +}
> +
> +/**
> + * ice_fwlog_dump_cfg - Dump current FW logging configuration
> + * @hw: pointer to the HW structure
> + */
> +static void ice_fwlog_dump_cfg(struct ice_hw *hw)
> +{
> + struct device *dev = ice_pf_to_dev((struct ice_pf *)(hw->back));
same here
> + struct ice_fwlog_cfg *cfg;
> + int status;
> +
> + cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
> + if (!cfg)
> + return;
> +
> + status = ice_fwlog_get(hw, cfg);
> + if (status) {
> + kfree(cfg);
> + return;
> + }
> +
> + dev_info(dev, "Running FWLOG Configuration:\n");
> + ice_print_fwlog_config(hw, cfg);
> +
> + 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;
> +
> + return 0;
This can be 'return ice_aq_fwlog_get(hw, cfg);'
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration Paul M Stillwell Jr
2023-03-03 1:23 ` kernel test robot
@ 2023-03-03 23:21 ` Tony Nguyen
2023-03-06 22:11 ` Paul M Stillwell Jr
1 sibling, 1 reply; 16+ messages in thread
From: Tony Nguyen @ 2023-03-03 23:21 UTC (permalink / raw)
To: Paul M Stillwell Jr, intel-wired-lan; +Cc: Brett Creeley
On 3/2/2023 1:51 PM, Paul M Stillwell Jr wrote:
[...]
> +/**
> + * ice_pf_fwlog_update_modules - update 1 or more modules via debugfs
> + * @pf: pointer to the PF struct
> + * @log_level: log_level to use for the @events
> + * @events: events to update
> + */
> +int ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
> + unsigned long events)
> +{
> + struct ice_fwlog_module_entry *entries;
> + u16 module_id, max_bits;
> + struct ice_hw *hw = &pf->hw;
In addition to what lkp reported. RCT
> +
> + if (log_level >= ICE_FWLOG_LEVEL_INVALID) {
> + dev_err(ice_pf_to_dev(pf), "Invalid FW log level %u, all level(s) >= %u are invalid\n",
> + log_level, ICE_FWLOG_LEVEL_INVALID);
> + return -EINVAL;
> + }
> +
> + if (events >= BIT(ICE_AQC_FW_LOG_ID_MAX)) {
> + dev_err(ice_pf_to_dev(pf), "Invalid FW log events 0x%lx, all FW log event bits >= 0x%lx are invalid\n",
> + events, BIT(ICE_AQC_FW_LOG_ID_MAX));
> + return -EINVAL;
> + }
> +
> + entries = (struct ice_fwlog_module_entry *)hw->fwlog_cfg.module_entries;
> +
> + max_bits = min_t(u16, BITS_PER_TYPE(unsigned long),
> + ICE_AQC_FW_LOG_ID_MAX);
> +
> + for_each_set_bit(module_id, &events, max_bits) {
> + entries[module_id].log_level = log_level;
> + }
> +
> + return 0;
> +}
> +
> /**
> * ice_register_netdev - register netdev
> * @vsi: pointer to the VSI struct
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v9 4/5] ice: enable FW logging based on stored configuration
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 4/5] ice: enable FW logging based on stored configuration Paul M Stillwell Jr
@ 2023-03-03 23:22 ` Tony Nguyen
2023-03-06 22:14 ` Paul M Stillwell Jr
0 siblings, 1 reply; 16+ messages in thread
From: Tony Nguyen @ 2023-03-03 23:22 UTC (permalink / raw)
To: Paul M Stillwell Jr, intel-wired-lan; +Cc: Brett Creeley
On 3/2/2023 1:51 PM, Paul M Stillwell Jr wrote:
[...]
> +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);
Return here, no need to assign to status
> +
> + return status;
> +}
> +
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v9 5/5] ice: add ability to enable FW logging and capture data
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 5/5] ice: add ability to enable FW logging and capture data Paul M Stillwell Jr
@ 2023-03-03 23:23 ` Tony Nguyen
2023-03-08 23:22 ` Paul M Stillwell Jr
0 siblings, 1 reply; 16+ messages in thread
From: Tony Nguyen @ 2023-03-03 23:23 UTC (permalink / raw)
To: Paul M Stillwell Jr, intel-wired-lan
On 3/2/2023 1:51 PM, Paul M Stillwell Jr wrote:
> Once the FW logging is configured then users need to be able to enable
> logging and capture the data. To enable/disable FW logging use 'enable
> fwlog' and 'disable fwlog'. An example command to enable FW logging:
>
> echo enable fwlog > /sys/kernel/debug/ice/0000\:18\:00.0/fwlog
>
> Once logging is enable the user should read the data using something
> like:
>
> cat /sys/kernel/debug/ice/0000\:18\:00.0/fwlog > log_data.bin
There should be documentation for this feature;
Documentation/networking/device_drivers/ethernet/intel/ice.rst?
> The output file is a binary file that will be decoded by the FW
> team for debugging.
Perhaps 'by Intel' instead of 'by the FW team'
>
> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
> ---
[...]
> index a611e0d98bd0..68ab19678dbe 100644
> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
> @@ -2075,7 +2075,9 @@ enum ice_aqc_fw_logging_mod {
> };
>
> /* Set FW Logging configuration (indirect 0xFF30)
> + * Register for FW Logging (indirect 0xFF31)
This should go with the patch that added the
'ice_aqc_opc_fw_logs_register = 0xFF31,'
entry...
Also, the defines related to the command should be brought in with the
command instead of all at the beginning.
(These:
+#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)
+#define ICE_AQC_FW_LOG_PERSISTENT BIT(0)
)
> * Query FW Logging (indirect 0xFF32)
> + * FW Log Event (indirect 0xFF33)
> */
> struct ice_aqc_fw_log {
> u8 cmd_flags;
> @@ -2375,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,
> };
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v9 2/5] ice: enable debugfs to check FW logging status
2023-03-03 23:21 ` Tony Nguyen
@ 2023-03-06 22:00 ` Paul M Stillwell Jr
0 siblings, 0 replies; 16+ messages in thread
From: Paul M Stillwell Jr @ 2023-03-06 22:00 UTC (permalink / raw)
To: Tony Nguyen, intel-wired-lan; +Cc: Brett Creeley
On 3/3/2023 3:21 PM, Tony Nguyen wrote:
> On 3/2/2023 1:51 PM, Paul M Stillwell Jr wrote:
>
> [...]
>
>> +#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);
>> @@ -934,6 +945,18 @@ int ice_aq_wait_for_event(struct ice_pf *pf, u16
>> opcode, unsigned long timeout,
>> int ice_open(struct net_device *netdev);
>> int ice_open_internal(struct net_device *netdev);
>> int ice_stop(struct net_device *netdev);
>> +#ifdef CONFIG_DEBUG_FS
>> +int
>> +ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
>> + unsigned long events);
>> +#else
>> +static int
>> +ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
>> + unsigned long events)
>> +{
>> + return -EOPNOTSUPP;
>> +}
>> +#endif /* CONFIG_DEBUG_FS */
>
> This could be put with the ifdef above
>
Done
>> void ice_service_task_schedule(struct ice_pf *pf);
>> int ice_load(struct ice_pf *pf);
>> void ice_unload(struct ice_pf *pf);
>
> [...]
>
>> +/* Set FW Logging configuration (indirect 0xFF30)
>> + * 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)
>> +#define ICE_AQC_FW_LOG_PERSISTENT BIT(0)
>> + u8 rsp_flag;
>
> Please add a newline between the member and the defines that relate to
> it. Please check this for other instances/needs as well.
>
Done
> [...]
>
>> +#include <linux/vmalloc.h>
>> +
>
> Any particular reason this isn't with everything else (and alphabetized)?
>
This isn't needed in this patch so remove it here and add it back in the
correct patch.
>> +#include <linux/fs.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/random.h>
>> +#include "ice.h"
>> +
>> +static struct dentry *ice_debugfs_root;
>> +
>> +static const char *module_id_to_name(u16 module_id)
>> +{
>> + switch (module_id) {
>> + case ICE_AQC_FW_LOG_ID_GENERAL:
>> + return "General";
>> + case ICE_AQC_FW_LOG_ID_CTRL:
>> + return "Control (Resets + Autoload)";
>> + case ICE_AQC_FW_LOG_ID_LINK:
>> + return "Link Management";
>> + case ICE_AQC_FW_LOG_ID_LINK_TOPO:
>> + return "Link Topology Detection";
>> + case ICE_AQC_FW_LOG_ID_DNL:
>> + return "DNL";
>> + case ICE_AQC_FW_LOG_ID_I2C:
>> + return "I2C";
>> + case ICE_AQC_FW_LOG_ID_SDP:
>> + return "SDP";
>> + case ICE_AQC_FW_LOG_ID_MDIO:
>> + return "MDIO";
>> + case ICE_AQC_FW_LOG_ID_ADMINQ:
>> + return "Admin Queue";
>> + case ICE_AQC_FW_LOG_ID_HDMA:
>> + return "HDMA";
>> + case ICE_AQC_FW_LOG_ID_LLDP:
>> + return "LLDP";
>> + case ICE_AQC_FW_LOG_ID_DCBX:
>> + return "DCBX";
>> + case ICE_AQC_FW_LOG_ID_DCB:
>> + return "DCB";
>> + case ICE_AQC_FW_LOG_ID_XLR:
>> + return "XLR";
>> + case ICE_AQC_FW_LOG_ID_NVM:
>> + return "NVM";
>> + case ICE_AQC_FW_LOG_ID_AUTH:
>> + return "Authentication";
>> + case ICE_AQC_FW_LOG_ID_VPD:
>> + return "VPD";
>> + case ICE_AQC_FW_LOG_ID_IOSF:
>> + return "IOSF";
>> + case ICE_AQC_FW_LOG_ID_PARSER:
>> + return "Parser";
>> + case ICE_AQC_FW_LOG_ID_SW:
>> + return "Switch";
>> + case ICE_AQC_FW_LOG_ID_SCHEDULER:
>> + return "Scheduler";
>> + case ICE_AQC_FW_LOG_ID_TXQ:
>> + return "Tx Queue Management";
>> + case ICE_AQC_FW_LOG_ID_POST:
>> + return "Post";
>> + case ICE_AQC_FW_LOG_ID_WATCHDOG:
>> + return "Watchdog";
>> + case ICE_AQC_FW_LOG_ID_TASK_DISPATCH:
>> + return "Task Dispatcher";
>> + case ICE_AQC_FW_LOG_ID_MNG:
>> + return "Manageability";
>> + case ICE_AQC_FW_LOG_ID_SYNCE:
>> + return "Synce";
>> + case ICE_AQC_FW_LOG_ID_HEALTH:
>> + return "Health";
>> + case ICE_AQC_FW_LOG_ID_TSDRV:
>> + return "Time Sync";
>> + case ICE_AQC_FW_LOG_ID_PFREG:
>> + return "PF Registration";
>> + case ICE_AQC_FW_LOG_ID_MDLVER:
>> + return "Module Version";
>> + default:
>> + return "Unsupported";
>> + }
>> +}
>> +
>> +static const char *log_level_to_name(u8 log_level)
>> +{
>> + switch (log_level) {
>> + case ICE_FWLOG_LEVEL_NONE:
>> + return "None";
>> + case ICE_FWLOG_LEVEL_ERROR:
>> + return "Error";
>> + case ICE_FWLOG_LEVEL_WARNING:
>> + return "Warning";
>> + case ICE_FWLOG_LEVEL_NORMAL:
>> + return "Normal";
>> + case ICE_FWLOG_LEVEL_VERBOSE:
>> + return "Verbose";
>> + default:
>> + return "Unsupported";
>> + }
>> +}
>> +
>> +static void ice_print_fwlog_config(struct ice_hw *hw, struct
>> ice_fwlog_cfg *cfg)
>> +{
>> + struct device *dev = ice_pf_to_dev((struct ice_pf *)(hw->back));
>
> I don't believe this casting is needed.
>
Changed this to ice_hw_to_dev() since that makes more sense
>> + u16 i;
>> +
>> + dev_info(dev, "Log_resolution: %d\n", cfg->log_resolution);
>> + dev_info(dev, "Options: 0x%04x\n", cfg->options);
>> + dev_info(dev, "\tarq_ena: %s\n",
>> + (cfg->options &
>> + ICE_FWLOG_OPTION_ARQ_ENA) ? "true" : "false");
>> + dev_info(dev, "\tuart_ena: %s\n",
>> + (cfg->options &
>> + ICE_FWLOG_OPTION_UART_ENA) ? "true" : "false");
>> + dev_info(dev, "\trunning: %s\n",
>> + (cfg->options &
>> + ICE_FWLOG_OPTION_IS_REGISTERED) ? "true" : "false");
>> +
>> + dev_info(dev, "Module Entries:\n");
>> + for (i = 0; i < ICE_AQC_FW_LOG_ID_MAX; i++) {
>> + struct ice_fwlog_module_entry *entry =
>> + &cfg->module_entries[i];
>> +
>> + dev_info(dev, "\tModule ID %d (%s) Log Level %d (%s)\n",
>> + entry->module_id, module_id_to_name(entry->module_id),
>> + entry->log_level, log_level_to_name(entry->log_level));
>> + }
>> +}
>> +
>> +/**
>> + * ice_fwlog_dump_cfg - Dump current FW logging configuration
>> + * @hw: pointer to the HW structure
>> + */
>> +static void ice_fwlog_dump_cfg(struct ice_hw *hw)
>> +{
>> + struct device *dev = ice_pf_to_dev((struct ice_pf *)(hw->back));
>
> same here
>
Same as above
>> + struct ice_fwlog_cfg *cfg;
>> + int status;
>> +
>> + cfg = kzalloc(sizeof(*cfg), GFP_KERNEL);
>> + if (!cfg)
>> + return;
>> +
>> + status = ice_fwlog_get(hw, cfg);
>> + if (status) {
>> + kfree(cfg);
>> + return;
>> + }
>> +
>> + dev_info(dev, "Running FWLOG Configuration:\n");
>> + ice_print_fwlog_config(hw, cfg);
>> +
>> + 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;
>> +
>> + return 0;
>
> This can be 'return ice_aq_fwlog_get(hw, cfg);'
Done
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration
2023-03-03 23:21 ` Tony Nguyen
@ 2023-03-06 22:11 ` Paul M Stillwell Jr
0 siblings, 0 replies; 16+ messages in thread
From: Paul M Stillwell Jr @ 2023-03-06 22:11 UTC (permalink / raw)
To: Tony Nguyen, intel-wired-lan; +Cc: Brett Creeley
On 3/3/2023 3:21 PM, Tony Nguyen wrote:
> On 3/2/2023 1:51 PM, Paul M Stillwell Jr wrote:
>
> [...]
>
>> +/**
>> + * ice_pf_fwlog_update_modules - update 1 or more modules via debugfs
>> + * @pf: pointer to the PF struct
>> + * @log_level: log_level to use for the @events
>> + * @events: events to update
>> + */
>> +int ice_pf_fwlog_update_modules(struct ice_pf *pf, u8 log_level,
>> + unsigned long events)
>> +{
>> + struct ice_fwlog_module_entry *entries;
>> + u16 module_id, max_bits;
>> + struct ice_hw *hw = &pf->hw;
>
> In addition to what lkp reported. RCT
>
Done
>> +
>> + if (log_level >= ICE_FWLOG_LEVEL_INVALID) {
>> + dev_err(ice_pf_to_dev(pf), "Invalid FW log level %u, all
>> level(s) >= %u are invalid\n",
>> + log_level, ICE_FWLOG_LEVEL_INVALID);
>> + return -EINVAL;
>> + }
>> +
>> + if (events >= BIT(ICE_AQC_FW_LOG_ID_MAX)) {
>> + dev_err(ice_pf_to_dev(pf), "Invalid FW log events 0x%lx, all
>> FW log event bits >= 0x%lx are invalid\n",
>> + events, BIT(ICE_AQC_FW_LOG_ID_MAX));
>> + return -EINVAL;
>> + }
>> +
>> + entries = (struct ice_fwlog_module_entry
>> *)hw->fwlog_cfg.module_entries;
>> +
>> + max_bits = min_t(u16, BITS_PER_TYPE(unsigned long),
>> + ICE_AQC_FW_LOG_ID_MAX);
>> +
>> + for_each_set_bit(module_id, &events, max_bits) {
>> + entries[module_id].log_level = log_level;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /**
>> * ice_register_netdev - register netdev
>> * @vsi: pointer to the VSI struct
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v9 4/5] ice: enable FW logging based on stored configuration
2023-03-03 23:22 ` Tony Nguyen
@ 2023-03-06 22:14 ` Paul M Stillwell Jr
0 siblings, 0 replies; 16+ messages in thread
From: Paul M Stillwell Jr @ 2023-03-06 22:14 UTC (permalink / raw)
To: Tony Nguyen, intel-wired-lan; +Cc: Brett Creeley
On 3/3/2023 3:22 PM, Tony Nguyen wrote:
>
>
> On 3/2/2023 1:51 PM, Paul M Stillwell Jr wrote:
>
> [...]
>
>> +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);
>
> Return here, no need to assign to status
>
Done
>> +
>> + return status;
>> +}
>> +
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next v9 5/5] ice: add ability to enable FW logging and capture data
2023-03-03 23:23 ` Tony Nguyen
@ 2023-03-08 23:22 ` Paul M Stillwell Jr
0 siblings, 0 replies; 16+ messages in thread
From: Paul M Stillwell Jr @ 2023-03-08 23:22 UTC (permalink / raw)
To: Tony Nguyen, intel-wired-lan
On 3/3/2023 3:23 PM, Tony Nguyen wrote:
>
>
> On 3/2/2023 1:51 PM, Paul M Stillwell Jr wrote:
>> Once the FW logging is configured then users need to be able to enable
>> logging and capture the data. To enable/disable FW logging use 'enable
>> fwlog' and 'disable fwlog'. An example command to enable FW logging:
>>
>> echo enable fwlog > /sys/kernel/debug/ice/0000\:18\:00.0/fwlog
>>
>> Once logging is enable the user should read the data using something
>> like:
>>
>> cat /sys/kernel/debug/ice/0000\:18\:00.0/fwlog > log_data.bin
>
> There should be documentation for this feature;
> Documentation/networking/device_drivers/ethernet/intel/ice.rst?
>
Done
>> The output file is a binary file that will be decoded by the FW
>> team for debugging.
>
> Perhaps 'by Intel' instead of 'by the FW team'
Done
>>
>> Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>
>> ---
>
> [...]
>
>> index a611e0d98bd0..68ab19678dbe 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
>> +++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
>> @@ -2075,7 +2075,9 @@ enum ice_aqc_fw_logging_mod {
>> };
>> /* Set FW Logging configuration (indirect 0xFF30)
>> + * Register for FW Logging (indirect 0xFF31)
>
> This should go with the patch that added the
> 'ice_aqc_opc_fw_logs_register = 0xFF31,'
>
> entry...
>
Done
> Also, the defines related to the command should be brought in with the
> command instead of all at the beginning.
>
> (These:
> +#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)
> +#define ICE_AQC_FW_LOG_PERSISTENT BIT(0)
> )
>
Done
>> * Query FW Logging (indirect 0xFF32)
>> + * FW Log Event (indirect 0xFF33)
>> */
>> struct ice_aqc_fw_log {
>> u8 cmd_flags;
>> @@ -2375,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,
>> };
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-03-08 23:22 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-02 21:51 [Intel-wired-lan] [PATCH net-next v9 0/5] add v2 FW logging for ice driver Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 1/5] ice: remove FW logging code Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 2/5] ice: enable debugfs to check FW logging status Paul M Stillwell Jr
2023-03-03 23:21 ` Tony Nguyen
2023-03-06 22:00 ` Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 3/5] ice: add ability to set FW log configuration Paul M Stillwell Jr
2023-03-03 1:23 ` kernel test robot
2023-03-03 1:23 ` kernel test robot
2023-03-03 23:21 ` Tony Nguyen
2023-03-06 22:11 ` Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 4/5] ice: enable FW logging based on stored configuration Paul M Stillwell Jr
2023-03-03 23:22 ` Tony Nguyen
2023-03-06 22:14 ` Paul M Stillwell Jr
2023-03-02 21:51 ` [Intel-wired-lan] [PATCH net-next v9 5/5] ice: add ability to enable FW logging and capture data Paul M Stillwell Jr
2023-03-03 23:23 ` Tony Nguyen
2023-03-08 23:22 ` Paul M Stillwell Jr
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.