On 5/16/2022 12:56 PM, Rahul Lakkireddy wrote:
> On Monday, May 05/16/22, 2022 at 12:06:01 +0100, Ferruh Yigit wrote:
>> On 5/16/2022 11:27 AM, Rahul Lakkireddy wrote:
>>> Add support to read firmware configuration file from
>>> /lib/firmware/cxgb4/ path in the filesystem. The firmware
>>> config file is used to enable or disable NIC features before
>>> firmware initialization to help retrieve better debug data to
>>> analyze firmware init failures. The config file can also
>>> be used to redistribute resources, like queues, TCAMs, etc.,
>>> from disabled physical functions (PFs) to main PF, before
>>> firmware init.
>>
>> Hi Rahul,
>>
>> Please find comments below.
>>
>> Also can you please send a new version for both 4/5 and 5/5 (this patch)
>> from original series?
>>
>
> I had already posted v2 for 4/5 from original series at below location.
>
> https://patches.dpdk.org/project/dpdk/patch/61ae717665c2f4e38712652bec2f9d0fe5ca7d32.1651842841.git.rahul.lakkireddy@chelsio.com/
>
> Do you want me to send a new patch series with 4/5 and 5/5 in a
> single patch series?
>
Nope, I missed above patch, it is good as separate patches.
>>>
>>> Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
>>> ---
>>> v2:
>>> - Update cxgbe pmd doc about the firmware config file
>>> - Update commit message to explain more about firmware config file
>>>
>>> doc/guides/nics/cxgbe.rst | 28 ++
>>> drivers/net/cxgbe/base/t4fw_interface.h | 1 +
>>> drivers/net/cxgbe/cxgbe_main.c | 329 ++++++++++++++++--------
>>> 3 files changed, 245 insertions(+), 113 deletions(-)
>>>
>>> diff --git a/doc/guides/nics/cxgbe.rst b/doc/guides/nics/cxgbe.rst
>>> index a1d30c488b..fc8a5751f1 100644
>>> --- a/doc/guides/nics/cxgbe.rst
>>> +++ b/doc/guides/nics/cxgbe.rst
>>> @@ -838,3 +838,31 @@ to configure the mtu of all the ports with a single command.
>>> testpmd> port stop all
>>> testpmd> port config all max-pkt-len 9000
>>> +
>>> +Hardware Configuration and Debugging
>>> +------------------------------------
>>> +
>>> +Firmware Configuration File
>>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +To enable or disable Chelsio NIC features before firmware initialization,
>>> +the Chelsio firmware configuration file can be placed in following
>>> +directory. The CXGBE PMD will search and pick up the firmware
>>> +configuration file during the Chelsio NIC probe, before initializing
>>> +the firmware.
>>> +
>>
>> Does it worth to mention what happens if the FW config file doesn't exist?
>> Or mention from FW config file load order, as you described in mail list, to
>> understand the relation with 'cxgbtool'?
>>
>
> Will do.
>
>>> +.. code-block:: console
>>> +
>>> + cp <path_to_config_file>/t6-config.txt /lib/firmware/cxgb4/t6-config.txt
>>> +
>>
>> There is also 't5-config.txt' in the code.
>>
>
> Yes, t5-config.txt is for Chelsio T5 NIC series and t6-config.txt
> is for Chelsio T6 NIC series. Will update t5-config.txt as well.
>
>>> +The firmware configuration file is mainly intended to be used to debug
>>> +firmware initialization failures. It can also be used to redistribute
>>> +NIC resources from disabled physical functions (PFs) to main PF before
>>> +initializing firmware.
>>> +
>>> +.. warning::
>>> +
>>> + Note that the Chelsio firmware configuration file contains very low
>>> + level details that is specific to the Chelsio NIC. Hence, the
>>> + firmware configuration file must not be modified without expert
>>> + guidance from Chelsio support team.
>>
>> Will it be too much detail to document what config can be changed via this
>> FW config?
>>
>
> Yes, this will be difficult to document. The config file just looks like
> register=value pairs and are often dependent on each other based on
> the selected configuration. A portion of sample config file is given
> below. I'm not sure how to document this kind of info.
>
> [global]
> reg[0x100c] = 0x22222222
> reg[0x10a0] = 0x01040810
> reg[0x1044] = 4096
> reg[0x1048] = 65536
> reg[0x104c] = 1536
> reg[0x1050] = 9024
> reg[0x1054] = 9216
> reg[0x1058] = 2048
> reg[0x105c] = 128
> reg[0x1060] = 8192
> reg[0x1064] = 16384
> [...]
>
Above won't be much useful to user anyway (without description of the
values), if there is already some documentation a link to that can be
good, otherwise OK to skip this.
@@ -838,3 +838,31 @@ to configure the mtu of all the ports with a single command.
testpmd> port stop all
testpmd> port config all max-pkt-len 9000
+
+Hardware Configuration and Debugging
+------------------------------------
+
+Firmware Configuration File
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+To enable or disable Chelsio NIC features before firmware initialization,
+the Chelsio firmware configuration file can be placed in following
+directory. The CXGBE PMD will search and pick up the firmware
+configuration file during the Chelsio NIC probe, before initializing
+the firmware.
+
+.. code-block:: console
+
+ cp <path_to_config_file>/t6-config.txt /lib/firmware/cxgb4/t6-config.txt
+
+The firmware configuration file is mainly intended to be used to debug
+firmware initialization failures. It can also be used to redistribute
+NIC resources from disabled physical functions (PFs) to main PF before
+initializing firmware.
+
+.. warning::
+
+ Note that the Chelsio firmware configuration file contains very low
+ level details that is specific to the Chelsio NIC. Hence, the
+ firmware configuration file must not be modified without expert
+ guidance from Chelsio support team.
@@ -697,6 +697,7 @@ enum fw_params_param_dev {
*/
FW_PARAMS_PARAM_DEV_FWREV = 0x0B, /* fw version */
FW_PARAMS_PARAM_DEV_TPREV = 0x0C, /* tp version */
+ FW_PARAMS_PARAM_DEV_CF = 0x0D,
FW_PARAMS_PARAM_DEV_ULPTX_MEMWRITE_DSGL = 0x17,
FW_PARAMS_PARAM_DEV_FILTER2_WR = 0x1D,
FW_PARAMS_PARAM_DEV_OPAQUE_VIID_SMT_EXTN = 0x27,
@@ -4,6 +4,7 @@
*/
#include <sys/queue.h>
+#include <sys/stat.h>
#include <stdio.h>
#include <errno.h>
#include <stdint.h>
@@ -11,6 +12,7 @@
#include <unistd.h>
#include <stdarg.h>
#include <inttypes.h>
+#include <fcntl.h>
#include <netinet/in.h>
#include <rte_byteorder.h>
@@ -1006,6 +1008,218 @@ static int configure_filter_mode_mask(struct adapter *adap)
params, val);
}
+#define CXGBE_FW_CONFIG_PATH_T5 "/lib/firmware/cxgb4/t5-config.txt"
+#define CXGBE_FW_CONFIG_PATH_T6 "/lib/firmware/cxgb4/t6-config.txt"
+
+/*
+ * Load firmware configuration from file in /lib/firmware/cxgb4/ path,
+ * if it is present.
+ */
+static int cxgbe_load_fw_config_from_filesystem(struct adapter *adap,
+ const char **config_name,
+ u32 *mem_type, u32 *mem_addr)
+{
+ u32 param, val, mtype, maddr;
+ const char *fw_cfg_path;
+ char *fw_cfg = NULL;
+ struct stat st;
+ int ret, fd;
+
+ switch (CHELSIO_CHIP_VERSION(adap->params.chip)) {
+ case CHELSIO_T5:
+ fw_cfg_path = CXGBE_FW_CONFIG_PATH_T5;
+ break;
+ case CHELSIO_T6:
+ fw_cfg_path = CXGBE_FW_CONFIG_PATH_T6;
+ break;
+ default:
+ return -ENOENT;
+ }
+
+ ret = open(fw_cfg_path, O_RDONLY);
+ if (ret < 0) {
+ dev_debug(adap, "Couldn't open FW config file\n");
+ return ret;
+ }
+
+ fd = ret;
+
+ ret = fstat(fd, &st);
+ if (ret < 0) {
+ dev_debug(adap, "Couldn't get FW config file size\n");
+ goto out_err;
+ }
+
+ if (st.st_size >= FLASH_CFG_MAX_SIZE) {
+ dev_debug(adap, "FW config file size >= max(%u)\n",
+ FLASH_CFG_MAX_SIZE);
+ ret = -ENOMEM;
+ goto out_err;
+ }
+
+ fw_cfg = rte_zmalloc(NULL, st.st_size, 0);
+ if (fw_cfg == NULL) {
+ ret = -ENOMEM;
+ goto out_err;
+ }
+
+ if (read(fd, fw_cfg, st.st_size) != st.st_size) {
+ dev_debug(adap, "Couldn't read FW config file data\n");
+ ret = -EIO;
+ goto out_err;
+ }
+
+ close(fd);
+
+ /* Send it to FW to verify and update to new configuration */
+ param = V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_DEV) |
+ V_FW_PARAMS_PARAM_X(FW_PARAMS_PARAM_DEV_CF);
+ ret = t4_query_params(adap, adap->mbox, adap->pf, 0, 1, ¶m, &val);
+ if (ret < 0) {
+ dev_debug(adap, "FW config param query failed: %d\n", ret);
+ goto out_free;
+ }
+
+ mtype = val >> 8;
+ maddr = (val & 0xff) << 16;
+
+ t4_os_lock(&adap->win0_lock);
+ ret = t4_memory_rw(adap, MEMWIN_NIC, mtype, maddr, st.st_size,
+ fw_cfg, T4_MEMORY_WRITE);
+ t4_os_unlock(&adap->win0_lock);
+ if (ret < 0) {
+ dev_debug(adap, "FW config file update failed: %d\n", ret);
+ goto out_free;
+ }
+
+ rte_free(fw_cfg);
+
+ *mem_type = mtype;
+ *mem_addr = maddr;
+ *config_name = fw_cfg_path;
+ return 0;
+
+out_err:
+ close(fd);
+out_free:
+ rte_free(fw_cfg);
+ return ret;
+}
+
+static int cxgbe_load_fw_config(struct adapter *adap)
+{
+ u32 finiver, finicsum, cfcsum, mtype, maddr, param, val;
+ struct fw_caps_config_cmd caps_cmd = { 0 };
+ const char *config_name = NULL;
+ int ret;
+
+ ret = cxgbe_load_fw_config_from_filesystem(adap, &config_name,
+ &mtype, &maddr);
+ if (ret < 0) {
+ config_name = "On Flash";
+
+ ret = t4_flash_cfg_addr(adap);
+ if (ret < 0) {
+ dev_warn(adap,
+ "Finding address for FW config file in flash failed: %d\n",
+ ret);
+ goto out_default_config;
+ }
+
+ mtype = FW_MEMTYPE_CF_FLASH;
+ maddr = ret;
+ }
+
+ /* Enable HASH filter region when support is available. */
+ val = 1;
+ param = CXGBE_FW_PARAM_DEV(HASHFILTER_WITH_OFLD);
+ t4_set_params(adap, adap->mbox, adap->pf, 0, 1, ¶m, &val);
+
+ /*
+ * Issue a Capability Configuration command to the firmware to get it
+ * to parse the Configuration File.
+ */
+ caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+ F_FW_CMD_REQUEST | F_FW_CMD_READ);
+ caps_cmd.cfvalid_to_len16 =
+ cpu_to_be32(F_FW_CAPS_CONFIG_CMD_CFVALID |
+ V_FW_CAPS_CONFIG_CMD_MEMTYPE_CF(mtype) |
+ V_FW_CAPS_CONFIG_CMD_MEMADDR64K_CF(maddr >> 16) |
+ FW_LEN16(caps_cmd));
+ ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+ &caps_cmd);
+
+out_default_config:
+ /*
+ * If the CAPS_CONFIG failed with an ENOENT (for a Firmware
+ * Configuration File in filesystem or FLASH), our last gasp
+ * effort is to use the Firmware Configuration File which is
+ * embedded in the firmware.
+ */
+ if (ret == -ENOENT) {
+ config_name = "Firmware Default";
+
+ memset(&caps_cmd, 0, sizeof(caps_cmd));
+ caps_cmd.op_to_write =
+ cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+ F_FW_CMD_REQUEST | F_FW_CMD_READ);
+ caps_cmd.cfvalid_to_len16 = cpu_to_be32(FW_LEN16(caps_cmd));
+ ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+ &caps_cmd);
+ }
+
+ if (ret < 0) {
+ dev_info(adap,
+ "Failed to configure using %s Firmware Configuration file: %d\n",
+ config_name, ret);
+ return ret;
+ }
+
+ finiver = be32_to_cpu(caps_cmd.finiver);
+ finicsum = be32_to_cpu(caps_cmd.finicsum);
+ cfcsum = be32_to_cpu(caps_cmd.cfcsum);
+ if (finicsum != cfcsum)
+ dev_warn(adap,
+ "Configuration File checksum mismatch: [fini] csum=0x%x, computed csum=0x%x\n",
+ finicsum, cfcsum);
+
+ /*
+ * If we're a pure NIC driver then disable all offloading facilities.
+ * This will allow the firmware to optimize aspects of the hardware
+ * configuration which will result in improved performance.
+ */
+ caps_cmd.niccaps &= cpu_to_be16(~FW_CAPS_CONFIG_NIC_ETHOFLD);
+ caps_cmd.toecaps = 0;
+ caps_cmd.iscsicaps = 0;
+ caps_cmd.rdmacaps = 0;
+ caps_cmd.fcoecaps = 0;
+ caps_cmd.cryptocaps = 0;
+
+ /*
+ * And now tell the firmware to use the configuration we just loaded.
+ */
+ caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
+ F_FW_CMD_REQUEST | F_FW_CMD_WRITE);
+ caps_cmd.cfvalid_to_len16 = htonl(FW_LEN16(caps_cmd));
+ ret = t4_wr_mbox(adap, adap->mbox, &caps_cmd, sizeof(caps_cmd),
+ NULL);
+ if (ret < 0) {
+ dev_warn(adap, "Unable to finalize Firmware Capabilities %d\n",
+ ret);
+ return ret;
+ }
+
+ /*
+ * Return successfully and note that we're operating with parameters
+ * not supplied by the driver, rather than from hard-wired
+ * initialization constants buried in the driver.
+ */
+ dev_info(adap,
+ "Successfully configured using Firmware Configuration File \"%s\", version: 0x%x, computed csum: 0x%x\n",
+ config_name, finiver, cfcsum);
+ return 0;
+}
+
static void configure_pcie_ext_tag(struct adapter *adapter)
{
u16 v;
@@ -1119,12 +1333,7 @@ static int adap_init0_tweaks(struct adapter *adapter)
*/
static int adap_init0_config(struct adapter *adapter, int reset)
{
- u32 finiver, finicsum, cfcsum, param, val;
- struct fw_caps_config_cmd caps_cmd;
- unsigned long mtype = 0, maddr = 0;
- u8 config_issued = 0;
- char config_name[20];
- int cfg_addr, ret;
+ int ret;
/*
* Reset device if necessary.
@@ -1139,98 +1348,10 @@ static int adap_init0_config(struct adapter *adapter, int reset)
}
}
- cfg_addr = t4_flash_cfg_addr(adapter);
- if (cfg_addr < 0) {
- ret = cfg_addr;
- dev_warn(adapter, "Finding address for firmware config file in flash failed, error %d\n",
- -ret);
- goto bye;
- }
-
- strcpy(config_name, "On Flash");
- mtype = FW_MEMTYPE_CF_FLASH;
- maddr = cfg_addr;
-
- /* Enable HASH filter region when support is available. */
- val = 1;
- param = CXGBE_FW_PARAM_DEV(HASHFILTER_WITH_OFLD);
- t4_set_params(adapter, adapter->mbox, adapter->pf, 0, 1,
- ¶m, &val);
-
- /*
- * Issue a Capability Configuration command to the firmware to get it
- * to parse the Configuration File. We don't use t4_fw_config_file()
- * because we want the ability to modify various features after we've
- * processed the configuration file ...
- */
- memset(&caps_cmd, 0, sizeof(caps_cmd));
- caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
- F_FW_CMD_REQUEST | F_FW_CMD_READ);
- caps_cmd.cfvalid_to_len16 =
- cpu_to_be32(F_FW_CAPS_CONFIG_CMD_CFVALID |
- V_FW_CAPS_CONFIG_CMD_MEMTYPE_CF(mtype) |
- V_FW_CAPS_CONFIG_CMD_MEMADDR64K_CF(maddr >> 16) |
- FW_LEN16(caps_cmd));
- ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd, sizeof(caps_cmd),
- &caps_cmd);
- /*
- * If the CAPS_CONFIG failed with an ENOENT (for a Firmware
- * Configuration File in FLASH), our last gasp effort is to use the
- * Firmware Configuration File which is embedded in the firmware. A
- * very few early versions of the firmware didn't have one embedded
- * but we can ignore those.
- */
- if (ret == -ENOENT) {
- dev_info(adapter, "%s: Going for embedded config in firmware..\n",
- __func__);
-
- memset(&caps_cmd, 0, sizeof(caps_cmd));
- caps_cmd.op_to_write =
- cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
- F_FW_CMD_REQUEST | F_FW_CMD_READ);
- caps_cmd.cfvalid_to_len16 = cpu_to_be32(FW_LEN16(caps_cmd));
- ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd,
- sizeof(caps_cmd), &caps_cmd);
- strcpy(config_name, "Firmware Default");
- }
-
- config_issued = 1;
+ ret = cxgbe_load_fw_config(adapter);
if (ret < 0)
goto bye;
- finiver = be32_to_cpu(caps_cmd.finiver);
- finicsum = be32_to_cpu(caps_cmd.finicsum);
- cfcsum = be32_to_cpu(caps_cmd.cfcsum);
- if (finicsum != cfcsum)
- dev_warn(adapter, "Configuration File checksum mismatch: [fini] csum=%#x, computed csum=%#x\n",
- finicsum, cfcsum);
-
- /*
- * If we're a pure NIC driver then disable all offloading facilities.
- * This will allow the firmware to optimize aspects of the hardware
- * configuration which will result in improved performance.
- */
- caps_cmd.niccaps &= cpu_to_be16(~FW_CAPS_CONFIG_NIC_ETHOFLD);
- caps_cmd.toecaps = 0;
- caps_cmd.iscsicaps = 0;
- caps_cmd.rdmacaps = 0;
- caps_cmd.fcoecaps = 0;
- caps_cmd.cryptocaps = 0;
-
- /*
- * And now tell the firmware to use the configuration we just loaded.
- */
- caps_cmd.op_to_write = cpu_to_be32(V_FW_CMD_OP(FW_CAPS_CONFIG_CMD) |
- F_FW_CMD_REQUEST | F_FW_CMD_WRITE);
- caps_cmd.cfvalid_to_len16 = htonl(FW_LEN16(caps_cmd));
- ret = t4_wr_mbox(adapter, adapter->mbox, &caps_cmd, sizeof(caps_cmd),
- NULL);
- if (ret < 0) {
- dev_warn(adapter, "Unable to finalize Firmware Capabilities %d\n",
- -ret);
- goto bye;
- }
-
/*
* Tweak configuration based on system architecture, etc.
*/
@@ -1251,27 +1372,9 @@ static int adap_init0_config(struct adapter *adapter, int reset)
goto bye;
}
- /*
- * Return successfully and note that we're operating with parameters
- * not supplied by the driver, rather than from hard-wired
- * initialization constants buried in the driver.
- */
- dev_info(adapter,
- "Successfully configured using Firmware Configuration File \"%s\", version %#x, computed checksum %#x\n",
- config_name, finiver, cfcsum);
-
return 0;
- /*
- * Something bad happened. Return the error ... (If the "error"
- * is that there's no Configuration File on the adapter we don't
- * want to issue a warning since this is fairly common.)
- */
bye:
- if (config_issued && ret != -ENOENT)
- dev_warn(adapter, "\"%s\" configuration file error %d\n",
- config_name, -ret);
-
dev_debug(adapter, "%s: returning ret = %d ..\n", __func__, ret);
return ret;
}