* [PATCH iwl-next v1 0/2] i40e: Log FW state in recovery mode @ 2024-01-12 9:59 Jedrzej Jagielski 2024-01-12 9:59 ` [PATCH iwl-next v1 1/2] i40e: Add read alternate indirect command Jedrzej Jagielski 2024-01-12 9:59 ` [PATCH iwl-next v1 2/2] i40e-linux: Add support for reading Trace Buffer Jedrzej Jagielski 0 siblings, 2 replies; 7+ messages in thread From: Jedrzej Jagielski @ 2024-01-12 9:59 UTC (permalink / raw) To: intel-wired-lan; +Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski Introduce logging FW state in recovery mode functionality. 1st patch adds implementation of admin command reading content of alt ram. 2nd patch utilices the command to get trace buffer data and logs it when entering recovery mode. Jedrzej Jagielski (1): i40e-linux: Add support for reading Trace Buffer Przemyslaw R Karpinski (1): i40e: Add read alternate indirect command drivers/net/ethernet/intel/i40e/i40e.h | 2 + .../net/ethernet/intel/i40e/i40e_adminq_cmd.h | 4 +- drivers/net/ethernet/intel/i40e/i40e_common.c | 40 +++++++++++++++++++ drivers/net/ethernet/intel/i40e/i40e_main.c | 35 ++++++++++++++++ .../net/ethernet/intel/i40e/i40e_prototype.h | 3 ++ .../net/ethernet/intel/i40e/i40e_register.h | 2 + drivers/net/ethernet/intel/i40e/i40e_type.h | 5 +++ 7 files changed, 89 insertions(+), 2 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH iwl-next v1 1/2] i40e: Add read alternate indirect command 2024-01-12 9:59 [PATCH iwl-next v1 0/2] i40e: Log FW state in recovery mode Jedrzej Jagielski @ 2024-01-12 9:59 ` Jedrzej Jagielski 2024-01-13 16:07 ` Simon Horman 2024-01-12 9:59 ` [PATCH iwl-next v1 2/2] i40e-linux: Add support for reading Trace Buffer Jedrzej Jagielski 1 sibling, 1 reply; 7+ messages in thread From: Jedrzej Jagielski @ 2024-01-12 9:59 UTC (permalink / raw) To: intel-wired-lan Cc: anthony.l.nguyen, netdev, Przemyslaw R Karpinski, Aleksandr Loktionov, Jedrzej Jagielski From: Przemyslaw R Karpinski <przemyslaw.r.karpinski@intel.com> Introduce implementation of 0x0903 Admin Queue command. This indirect command reads a block of data from the alternate structure of memory. The command defines the number of Dwords to be read and the starting address inside the alternate structure. Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> Signed-off-by: Przemyslaw R Karpinski <przemyslaw.r.karpinski@intel.com> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com> --- .../net/ethernet/intel/i40e/i40e_adminq_cmd.h | 4 +- drivers/net/ethernet/intel/i40e/i40e_common.c | 40 +++++++++++++++++++ .../net/ethernet/intel/i40e/i40e_prototype.h | 3 ++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h index 18a1c3b6d72c..50785f7e6d08 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h +++ b/drivers/net/ethernet/intel/i40e/i40e_adminq_cmd.h @@ -1983,14 +1983,14 @@ I40E_CHECK_CMD_LENGTH(i40e_aqc_alternate_write); * Indirect read (indirect 0x0903) */ -struct i40e_aqc_alternate_ind_write { +struct i40e_aqc_alternate_ind_read_write { __le32 address; __le32 length; __le32 addr_high; __le32 addr_low; }; -I40E_CHECK_CMD_LENGTH(i40e_aqc_alternate_ind_write); +I40E_CHECK_CMD_LENGTH(i40e_aqc_alternate_ind_read_write); /* Done alternate write (direct 0x0904) * uses i40e_aq_desc diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c index de6ca6295742..93971c9c98cc 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_common.c +++ b/drivers/net/ethernet/intel/i40e/i40e_common.c @@ -4375,6 +4375,46 @@ static int i40e_aq_alternate_read(struct i40e_hw *hw, return status; } +/** + * i40e_aq_alternate_read_indirect + * @hw: pointer to the hardware structure + * @addr: address of the alternate structure field + * @dw_count: number of alternate structure fields to read + * @buffer: pointer to the command buffer + * + * Read 'dw_count' dwords from alternate structure starting at 'addr' and + * place them in 'buffer'. The buffer should be allocated by caller. + * + **/ +int i40e_aq_alternate_read_indirect(struct i40e_hw *hw, u32 addr, u32 dw_count, + void *buffer) +{ + struct i40e_aqc_alternate_ind_read_write *cmd_resp; + struct i40e_aq_desc desc; + int status; + + if (!buffer) + return -EINVAL; + + cmd_resp = (struct i40e_aqc_alternate_ind_read_write *)&desc.params.raw; + + i40e_fill_default_direct_cmd_desc(&desc, + i40e_aqc_opc_alternate_read_indirect); + + desc.flags |= cpu_to_le16(I40E_AQ_FLAG_RD); + desc.flags |= cpu_to_le16(I40E_AQ_FLAG_BUF); + if (dw_count > I40E_AQ_LARGE_BUF / 4) + desc.flags |= cpu_to_le16((u16)I40E_AQ_FLAG_LB); + + cmd_resp->address = cpu_to_le32(addr); + cmd_resp->length = cpu_to_le32(dw_count); + + status = i40e_asq_send_command(hw, &desc, buffer, + lower_16_bits(4 * dw_count), NULL); + + return status; +} + /** * i40e_aq_suspend_port_tx * @hw: pointer to the hardware structure diff --git a/drivers/net/ethernet/intel/i40e/i40e_prototype.h b/drivers/net/ethernet/intel/i40e/i40e_prototype.h index af4269330581..37c23a0bded6 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_prototype.h +++ b/drivers/net/ethernet/intel/i40e/i40e_prototype.h @@ -322,6 +322,9 @@ i40e_aq_rem_cloud_filters_bb(struct i40e_hw *hw, u16 seid, u8 filter_count); int i40e_read_lldp_cfg(struct i40e_hw *hw, struct i40e_lldp_variables *lldp_cfg); + +int i40e_aq_alternate_read_indirect(struct i40e_hw *hw, u32 addr, u32 dw_count, + void *buffer); int i40e_aq_suspend_port_tx(struct i40e_hw *hw, u16 seid, struct i40e_asq_cmd_details *cmd_details); -- 2.31.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH iwl-next v1 1/2] i40e: Add read alternate indirect command 2024-01-12 9:59 ` [PATCH iwl-next v1 1/2] i40e: Add read alternate indirect command Jedrzej Jagielski @ 2024-01-13 16:07 ` Simon Horman 0 siblings, 0 replies; 7+ messages in thread From: Simon Horman @ 2024-01-13 16:07 UTC (permalink / raw) To: Jedrzej Jagielski Cc: intel-wired-lan, anthony.l.nguyen, netdev, Przemyslaw R Karpinski, Aleksandr Loktionov On Fri, Jan 12, 2024 at 10:59:44AM +0100, Jedrzej Jagielski wrote: > From: Przemyslaw R Karpinski <przemyslaw.r.karpinski@intel.com> > > Introduce implementation of 0x0903 Admin Queue command. > This indirect command reads a block of data from the alternate structure > of memory. The command defines the number of Dwords to be read and the > starting address inside the alternate structure. > > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> > Signed-off-by: Przemyslaw R Karpinski <przemyslaw.r.karpinski@intel.com> > Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@intel.com> ... > diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c > index de6ca6295742..93971c9c98cc 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_common.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_common.c > @@ -4375,6 +4375,46 @@ static int i40e_aq_alternate_read(struct i40e_hw *hw, > return status; > } > > +/** > + * i40e_aq_alternate_read_indirect > + * @hw: pointer to the hardware structure > + * @addr: address of the alternate structure field > + * @dw_count: number of alternate structure fields to read > + * @buffer: pointer to the command buffer > + * > + * Read 'dw_count' dwords from alternate structure starting at 'addr' and > + * place them in 'buffer'. The buffer should be allocated by caller. > + * > + **/ > +int i40e_aq_alternate_read_indirect(struct i40e_hw *hw, u32 addr, u32 dw_count, > + void *buffer) > +{ > + struct i40e_aqc_alternate_ind_read_write *cmd_resp; > + struct i40e_aq_desc desc; > + int status; > + > + if (!buffer) > + return -EINVAL; > + > + cmd_resp = (struct i40e_aqc_alternate_ind_read_write *)&desc.params.raw; > + > + i40e_fill_default_direct_cmd_desc(&desc, > + i40e_aqc_opc_alternate_read_indirect); > + > + desc.flags |= cpu_to_le16(I40E_AQ_FLAG_RD); > + desc.flags |= cpu_to_le16(I40E_AQ_FLAG_BUF); > + if (dw_count > I40E_AQ_LARGE_BUF / 4) > + desc.flags |= cpu_to_le16((u16)I40E_AQ_FLAG_LB); nit: Maybe the cast to (u16) can be dropped? It isn't present in usage of I40E_AQ_FLAG_LB a few lines further up. > + > + cmd_resp->address = cpu_to_le32(addr); > + cmd_resp->length = cpu_to_le32(dw_count); > + > + status = i40e_asq_send_command(hw, &desc, buffer, > + lower_16_bits(4 * dw_count), NULL); > + > + return status; > +} > + > /** > * i40e_aq_suspend_port_tx > * @hw: pointer to the hardware structure ... ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH iwl-next v1 2/2] i40e-linux: Add support for reading Trace Buffer 2024-01-12 9:59 [PATCH iwl-next v1 0/2] i40e: Log FW state in recovery mode Jedrzej Jagielski 2024-01-12 9:59 ` [PATCH iwl-next v1 1/2] i40e: Add read alternate indirect command Jedrzej Jagielski @ 2024-01-12 9:59 ` Jedrzej Jagielski 2024-01-12 12:49 ` Jiri Pirko 1 sibling, 1 reply; 7+ messages in thread From: Jedrzej Jagielski @ 2024-01-12 9:59 UTC (permalink / raw) To: intel-wired-lan Cc: anthony.l.nguyen, netdev, Jedrzej Jagielski, Jan Sokolowski 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.sokolowski@intel.com> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@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); + 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH iwl-next v1 2/2] i40e-linux: Add support for reading Trace Buffer 2024-01-12 9:59 ` [PATCH iwl-next v1 2/2] i40e-linux: Add support for reading Trace Buffer Jedrzej Jagielski @ 2024-01-12 12:49 ` Jiri Pirko 2024-01-15 10:37 ` Jagielski, Jedrzej 0 siblings, 1 reply; 7+ messages in thread From: Jiri Pirko @ 2024-01-12 12:49 UTC (permalink / raw) To: Jedrzej Jagielski Cc: intel-wired-lan, anthony.l.nguyen, netdev, Jan Sokolowski Fri, Jan 12, 2024 at 10:59:45AM CET, jedrzej.jagielski@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.sokolowski@intel.com> >Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@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. >+ 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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH iwl-next v1 2/2] i40e-linux: Add support for reading Trace Buffer 2024-01-12 12:49 ` Jiri Pirko @ 2024-01-15 10:37 ` Jagielski, Jedrzej 2024-01-15 14:06 ` Jiri Pirko 0 siblings, 1 reply; 7+ messages in thread From: Jagielski, Jedrzej @ 2024-01-15 10:37 UTC (permalink / raw) To: Jiri Pirko; +Cc: intel-wired-lan, Nguyen, Anthony L, netdev, Sokolowski, Jan From: Jiri Pirko <jiri@resnulli.us> Sent: Friday, January 12, 2024 1:49 PM >Fri, Jan 12, 2024 at 10:59:45AM CET, jedrzej.jagielski@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.sokolowski@intel.com> >>Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@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. Dumping log in that case happen rather occasionally and debug log lvl is used so this should mitigate polluting the dmesg. > > > >>+ 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 >> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iwl-next v1 2/2] i40e-linux: Add support for reading Trace Buffer 2024-01-15 10:37 ` Jagielski, Jedrzej @ 2024-01-15 14:06 ` Jiri Pirko 0 siblings, 0 replies; 7+ messages in thread From: Jiri Pirko @ 2024-01-15 14:06 UTC (permalink / raw) To: Jagielski, Jedrzej Cc: intel-wired-lan, Nguyen, Anthony L, netdev, Sokolowski, Jan Mon, Jan 15, 2024 at 11:37:22AM CET, jedrzej.jagielski@intel.com wrote: >From: Jiri Pirko <jiri@resnulli.us> >Sent: Friday, January 12, 2024 1:49 PM > >>Fri, Jan 12, 2024 at 10:59:45AM CET, jedrzej.jagielski@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.sokolowski@intel.com> >>>Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@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 >>> >>> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-01-15 14:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-12 9:59 [PATCH iwl-next v1 0/2] i40e: Log FW state in recovery mode Jedrzej Jagielski 2024-01-12 9:59 ` [PATCH iwl-next v1 1/2] i40e: Add read alternate indirect command Jedrzej Jagielski 2024-01-13 16:07 ` Simon Horman 2024-01-12 9:59 ` [PATCH iwl-next v1 2/2] i40e-linux: Add support for reading Trace Buffer Jedrzej Jagielski 2024-01-12 12:49 ` Jiri Pirko 2024-01-15 10:37 ` Jagielski, Jedrzej 2024-01-15 14:06 ` Jiri Pirko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).