On Wed, Aug 28, 2024 at 11:53:35AM +0800, Zhichao Zeng wrote: > The "Dynamic Device Personalization" package is loaded at initialization > time by the driver, but the specific package file loaded depends upon > what package file is found first by searching through a hard-coded list > of firmware paths. > > To enable greater control over the package loading, this commit two ways > to support custom DDP packages: > 1. Add device option to choose a specific DDP package file to load. > For example: > -a 80:00.0,ddp_pkg_file=/path/to/ice-version.pkg > 2. Read firmware search path from > "/sys/module/firmware_class/parameters/path" like the kernel behavior. > > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com> > Signed-off-by: Zhichao Zeng <zhichaox.z...@intel.com>
Hi Zhichao, since there are two different methods being supported for picking a DDP package this patch would be better split into two, one for each method added. The support for #1 above is already on-list as a standalone patch[1], so you really only need to do a new patch for #2 above. However, I'm ok for you to take my patch and include it in a 2-patch set for this if you prefer, since both patches will be related to choosing a DDP file. I'll leave it up to you whether v2 is a single patch for the search path, or a 2-patch set including [1]. Regards, /Bruce [1] https://patches.dpdk.org/project/dpdk/patch/20240812152815.1132697-5-bruce.richard...@intel.com/ > --- > doc/guides/nics/ice.rst | 12 +++++++ > drivers/net/ice/ice_ethdev.c | 61 ++++++++++++++++++++++++++++++++++++ > drivers/net/ice/ice_ethdev.h | 2 ++ > 3 files changed, 75 insertions(+) > > diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst > index ae975d19ad..0484fafbc1 100644 > --- a/doc/guides/nics/ice.rst > +++ b/doc/guides/nics/ice.rst > @@ -108,6 +108,18 @@ Runtime Configuration > > -a 80:00.0,default-mac-disable=1 > > +- ``DDP Package File`` > + > + Rather than have the driver search for the DDP package to load, > + or to override what package is used, > + the ``ddp_pkg_file`` option can be used to provide the path to a specific > package file. > + For example:: > + > + -a 80:00.0,ddp_pkg_file=/path/to/ice-version.pkg > + > + There is also support for customizing the firmware search path, will read > the search path > + from "/sys/module/firmware_class/parameters/path" and try to load DDP > package. > + > - ``Protocol extraction for per queue`` > > Configure the RX queues to do protocol extraction into mbuf for protocol > diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c > index 304f959b7e..bd78c14000 100644 > --- a/drivers/net/ice/ice_ethdev.c > +++ b/drivers/net/ice/ice_ethdev.c > @@ -36,6 +36,7 @@ > #define ICE_ONE_PPS_OUT_ARG "pps_out" > #define ICE_RX_LOW_LATENCY_ARG "rx_low_latency" > #define ICE_MBUF_CHECK_ARG "mbuf_check" > +#define ICE_DDP_FILENAME "ddp_pkg_file" > > #define ICE_CYCLECOUNTER_MASK 0xffffffffffffffffULL > > @@ -52,6 +53,7 @@ static const char * const ice_valid_args[] = { > ICE_RX_LOW_LATENCY_ARG, > ICE_DEFAULT_MAC_DISABLE, > ICE_MBUF_CHECK_ARG, > + ICE_DDP_FILENAME, > NULL > }; > > @@ -692,6 +694,18 @@ handle_field_name_arg(__rte_unused const char *key, > const char *value, > return 0; > } > > +static int > +handle_ddp_filename_arg(__rte_unused const char *key, const char *value, > void *name_args) > +{ > + const char **filename = name_args; > + if (strlen(value) >= ICE_MAX_PKG_FILENAME_SIZE) { > + PMD_DRV_LOG(ERR, "The DDP package filename is too long : '%s'", > value); > + return -1; > + } > + *filename = strdup(value); > + return 0; > +} > + > static void > ice_check_proto_xtr_support(struct ice_hw *hw) > { > @@ -1873,6 +1887,22 @@ ice_load_pkg_type(struct ice_hw *hw) > return package_type; > } > > +static int ice_read_customized_path(char *pkg_file) > +{ > + char buf[ICE_MAX_PKG_FILENAME_SIZE]; > + FILE *fp = fopen(ICE_PKG_FILE_CUSTOMIZED_PATH, "r"); > + if (fp == NULL) { > + PMD_INIT_LOG(ERR, "Failed to read CUSTOMIZED_PATH"); > + return -EIO; > + } > + if (fscanf(fp, "%s\n", buf) > 0) > + strncpy(pkg_file, buf, ICE_MAX_PKG_FILENAME_SIZE); > + else > + return -EIO; > + > + return 0; > +} > + > int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn) > { > struct ice_hw *hw = &adapter->hw; > @@ -1882,12 +1912,28 @@ int ice_load_pkg(struct ice_adapter *adapter, bool > use_dsn, uint64_t dsn) > size_t bufsz; > int err; > > + if (adapter->devargs.ddp_filename != NULL) { > + strlcpy(pkg_file, adapter->devargs.ddp_filename, > sizeof(pkg_file)); > + if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0) { > + goto load_fw; > + } else { > + PMD_INIT_LOG(ERR, "Cannot load DDP file: %s\n", > pkg_file); > + return -1; > + } > + } > + > if (!use_dsn) > goto no_dsn; > > memset(opt_ddp_filename, 0, ICE_MAX_PKG_FILENAME_SIZE); > snprintf(opt_ddp_filename, ICE_MAX_PKG_FILENAME_SIZE, > "ice-%016" PRIx64 ".pkg", dsn); > + > + ice_read_customized_path(pkg_file); > + strcat(pkg_file, opt_ddp_filename); > + if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0) > + goto load_fw; > + > strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_UPDATES, > ICE_MAX_PKG_FILENAME_SIZE); > strcat(pkg_file, opt_ddp_filename); > @@ -1901,6 +1947,10 @@ int ice_load_pkg(struct ice_adapter *adapter, bool > use_dsn, uint64_t dsn) > goto load_fw; > > no_dsn: > + ice_read_customized_path(pkg_file); > + if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0) > + goto load_fw; > + > strncpy(pkg_file, ICE_PKG_FILE_UPDATES, ICE_MAX_PKG_FILENAME_SIZE); > if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0) > goto load_fw; > @@ -2217,6 +2267,14 @@ static int ice_parse_devargs(struct rte_eth_dev *dev) > ret = rte_kvargs_process(kvlist, ICE_RX_LOW_LATENCY_ARG, > &parse_bool, &ad->devargs.rx_low_latency); > > + if (ret) > + goto bail; > + > + ret = rte_kvargs_process(kvlist, ICE_DDP_FILENAME, > + &handle_ddp_filename_arg, > &ad->devargs.ddp_filename); > + if (ret) > + goto bail; > + > bail: > rte_kvargs_free(kvlist); > return ret; > @@ -2689,6 +2747,8 @@ ice_dev_close(struct rte_eth_dev *dev) > ice_free_hw_tbls(hw); > rte_free(hw->port_info); > hw->port_info = NULL; > + free((void *)(uintptr_t)ad->devargs.ddp_filename); > + ad->devargs.ddp_filename = NULL; > ice_shutdown_all_ctrlq(hw, true); > rte_free(pf->proto_xtr); > pf->proto_xtr = NULL; > @@ -6981,6 +7041,7 @@ RTE_PMD_REGISTER_PARAM_STRING(net_ice, > ICE_PROTO_XTR_ARG > "=[queue:]<vlan|ipv4|ipv6|ipv6_flow|tcp|ip_offset>" > ICE_SAFE_MODE_SUPPORT_ARG "=<0|1>" > ICE_DEFAULT_MAC_DISABLE "=<0|1>" > + ICE_DDP_FILENAME "=</path/to/file>" > ICE_RX_LOW_LATENCY_ARG "=<0|1>"); > > RTE_LOG_REGISTER_SUFFIX(ice_logtype_init, init, NOTICE); > diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h > index 3ea9f37dc8..2781362d04 100644 > --- a/drivers/net/ice/ice_ethdev.h > +++ b/drivers/net/ice/ice_ethdev.h > @@ -51,6 +51,7 @@ > #define ICE_PKG_FILE_UPDATES "/lib/firmware/updates/intel/ice/ddp/ice.pkg" > #define ICE_PKG_FILE_SEARCH_PATH_DEFAULT "/lib/firmware/intel/ice/ddp/" > #define ICE_PKG_FILE_SEARCH_PATH_UPDATES > "/lib/firmware/updates/intel/ice/ddp/" > +#define ICE_PKG_FILE_CUSTOMIZED_PATH > "/sys/module/firmware_class/parameters/path" > #define ICE_MAX_PKG_FILENAME_SIZE 256 > > #define MAX_ACL_NORMAL_ENTRIES 256 > @@ -568,6 +569,7 @@ struct ice_devargs { > /* Name of the field. */ > char xtr_field_name[RTE_MBUF_DYN_NAMESIZE]; > uint64_t mbuf_check; > + const char *ddp_filename; > }; > > /** > -- > 2.34.1 >
