Mon, Jan 15, 2024 at 11:37:22AM CET, jedrzej.jagiel...@intel.com wrote:
>From: Jiri Pirko <j...@resnulli.us>
>Sent: Friday, January 12, 2024 1:49 PM
>
>>Fri, Jan 12, 2024 at 10:59:45AM CET, jedrzej.jagiel...@intel.com wrote:
>>>Currently after entering FW Recovery Mode we have no info in logs
>>>regarding current FW state.
>>>
>>>Add function reading content of the alternate RAM storing that info and
>>>print it into the log. Additionally print state of CSR register.
>>>
>>>Reviewed-by: Jan Sokolowski <jan.sokolow...@intel.com>
>>>Signed-off-by: Jedrzej Jagielski <jedrzej.jagiel...@intel.com>
>>>---
>>> drivers/net/ethernet/intel/i40e/i40e.h | 2 ++
>>> drivers/net/ethernet/intel/i40e/i40e_main.c | 35 +++++++++++++++++++
>>> .../net/ethernet/intel/i40e/i40e_register.h | 2 ++
>>> drivers/net/ethernet/intel/i40e/i40e_type.h | 5 +++
>>> 4 files changed, 44 insertions(+)
>>>
>>>diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
>>>b/drivers/net/ethernet/intel/i40e/i40e.h
>>>index ba24f3fa92c3..6ebd2fd15e0e 100644
>>>--- a/drivers/net/ethernet/intel/i40e/i40e.h
>>>+++ b/drivers/net/ethernet/intel/i40e/i40e.h
>>>@@ -23,6 +23,8 @@
>>> /* Useful i40e defaults */
>>> #define I40E_MAX_VEB 16
>>>
>>>+#define I40_BYTES_PER_WORD 2
>>>+
>>> #define I40E_MAX_NUM_DESCRIPTORS 4096
>>> #define I40E_MAX_NUM_DESCRIPTORS_XL710 8160
>>> #define I40E_MAX_CSR_SPACE (4 * 1024 * 1024 - 64 * 1024)
>>>diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
>>>b/drivers/net/ethernet/intel/i40e/i40e_main.c
>>>index 4977ff391fed..f5abe8c9a88d 100644
>>>--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>>>+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>>>@@ -15414,6 +15414,39 @@ static int i40e_handle_resets(struct i40e_pf *pf)
>>> return is_empr ? -EIO : pfr;
>>> }
>>>
>>>+/**
>>>+ * i40e_log_fw_recovery_mode - log current FW state in Recovery Mode
>>>+ * @pf: board private structure
>>>+ *
>>>+ * Read alternate RAM and CSR registers and print them to the log
>>>+ **/
>>>+static void i40e_log_fw_recovery_mode(struct i40e_pf *pf)
>>>+{
>>>+ u8 buf[I40E_FW_STATE_BUFF_SIZE] = {0};
>>>+ struct i40e_hw *hw = &pf->hw;
>>>+ u8 fws0b, fws1b;
>>>+ u32 fwsts;
>>>+ int ret;
>>>+
>>>+ ret = i40e_aq_alternate_read_indirect(hw, I40E_ALT_CANARY,
>>>+ I40E_ALT_BUFF_DWORD_SIZE, buf);
>>>+ if (ret) {
>>>+ dev_warn(&pf->pdev->dev,
>>>+ "Cannot get FW trace buffer due to FW err %d aq_err
>>>%s\n",
>>>+ ret, i40e_aq_str(hw, hw->aq.asq_last_status));
>>>+ return;
>>>+ }
>>>+
>>>+ fwsts = rd32(&pf->hw, I40E_GL_FWSTS);
>>>+ fws0b = FIELD_GET(I40E_GL_FWSTS_FWS0B_MASK, fwsts);
>>>+ fws1b = FIELD_GET(I40E_GL_FWSTS_FWS1B_MASK, fwsts);
>>>+
>>>+ print_hex_dump(KERN_DEBUG, "Trace Buffer: ", DUMP_PREFIX_NONE,
>>>+ BITS_PER_BYTE * I40_BYTES_PER_WORD, 1, buf,
>>>+ I40E_FW_STATE_BUFF_SIZE, true);
>>
>>I don't follow. Why exactly you want to pollute dmesg with another
>>messages? Can't you use some other interface? Devlink health reporter
>>looks like a suitable alternative for this kind of operations.
>
>There is no devlink support for the i40e driver at this point.
So add it, what can I say...
>Dumping log in that case happen rather occasionally and debug log lvl is used
>so this should mitigate polluting the dmesg.
Nope, please don't put thing in logs when we have proper interfaces for
them.
pw-bot: cr
>
>>
>>
>>
>>>+ dev_dbg(&pf->pdev->dev, "FWS0B=0x%x, FWS1B=0x%x\n", fws0b, fws1b);
>>>+}
>>>+
>>> /**
>>> * i40e_init_recovery_mode - initialize subsystems needed in recovery mode
>>> * @pf: board private structure
>>>@@ -15497,6 +15530,8 @@ static int i40e_init_recovery_mode(struct i40e_pf
>>>*pf, struct i40e_hw *hw)
>>> mod_timer(&pf->service_timer,
>>> round_jiffies(jiffies + pf->service_timer_period));
>>>
>>>+ i40e_log_fw_recovery_mode(pf);
>>>+
>>> return 0;
>>>
>>> err_switch_setup:
>>>diff --git a/drivers/net/ethernet/intel/i40e/i40e_register.h
>>>b/drivers/net/ethernet/intel/i40e/i40e_register.h
>>>index 14ab642cafdb..8e254ff9c035 100644
>>>--- a/drivers/net/ethernet/intel/i40e/i40e_register.h
>>>+++ b/drivers/net/ethernet/intel/i40e/i40e_register.h
>>>@@ -169,6 +169,8 @@
>>> #define I40E_PRTDCB_TPFCTS_PFCTIMER_SHIFT 0
>>> #define I40E_PRTDCB_TPFCTS_PFCTIMER_MASK I40E_MASK(0x3FFF,
>>> I40E_PRTDCB_TPFCTS_PFCTIMER_SHIFT)
>>> #define I40E_GL_FWSTS 0x00083048 /* Reset: POR */
>>>+#define I40E_GL_FWSTS_FWS0B_SHIFT 0
>>>+#define I40E_GL_FWSTS_FWS0B_MASK I40E_MASK(0xFF, I40E_GL_FWSTS_FWS0B_SHIFT)
>>> #define I40E_GL_FWSTS_FWS1B_SHIFT 16
>>> #define I40E_GL_FWSTS_FWS1B_MASK I40E_MASK(0xFF, I40E_GL_FWSTS_FWS1B_SHIFT)
>>> #define I40E_GL_FWSTS_FWS1B_EMPR_0 I40E_MASK(0x20,
>>> I40E_GL_FWSTS_FWS1B_SHIFT)
>>>diff --git a/drivers/net/ethernet/intel/i40e/i40e_type.h
>>>b/drivers/net/ethernet/intel/i40e/i40e_type.h
>>>index 725da7edbca3..0372a8d519ad 100644
>>>--- a/drivers/net/ethernet/intel/i40e/i40e_type.h
>>>+++ b/drivers/net/ethernet/intel/i40e/i40e_type.h
>>>@@ -1372,6 +1372,11 @@ struct i40e_lldp_variables {
>>> #define I40E_ALT_BW_VALUE_MASK 0xFF
>>> #define I40E_ALT_BW_VALID_MASK 0x80000000
>>>
>>>+/* Alternate Ram Trace Buffer*/
>>>+#define I40E_ALT_CANARY 0xABCDEFAB
>>>+#define I40E_ALT_BUFF_DWORD_SIZE 0x14 /* in dwords */
>>>+#define I40E_FW_STATE_BUFF_SIZE 80
>>>+
>>> /* RSS Hash Table Size */
>>> #define I40E_PFQF_CTL_0_HASHLUTSIZE_512 0x00010000
>>>
>>>--
>>>2.31.1
>>>
>>>
