> -----Original Message-----
> From: Stillwell Jr, Paul M
> Sent: Friday, August 30, 2019 2:08 AM
> To: Xu, Ting <ting...@intel.com>; dev@dpdk.org
> Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Yang, Qiming
> <qiming.y...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v1] net/ice: support device-specific DDP
> package loading
>
> > -----Original Message-----
> > From: dev <dev-boun...@dpdk.org> On Behalf Of Ting Xu
> > Sent: Thursday, August 29, 2019 6:18 AM
> > To: dev@dpdk.org
> > Cc: Lu, Wenzhuo <wenzhuo...@intel.com>; Yang, Qiming
> > <qiming.y...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>
> > Subject: [dpdk-dev] [PATCH v1] net/ice: support device-specific DDP
> > package loading
> >
> > This patch adds the feature that supports loading DDP package
> > according to the device serial number. Prior to loading the default
> > DDP package (ice.pkg), the driver will check for the presence of a
> > device-specific DDP package with the name containing 64-bit PCIe
> > Device Serial Number (ice-
> > xxxxxxxxxxxxxxxx.pkg) during initialization. Users can use "lspci -vs"
> > to get the device serial number.
> > The pkg search path are /lib/firmware/intel/ice/ddp/ and
> > /lib/firmware/updates/intel/ice/ddp/. If the package exists, the
> > driver will download it to the device instead of the default one. The
> > loaded package type (OS default and COMMS) will be stored in
> > ice_adapter-
> > >active_pkg_type. The package version is stored in
> ice_hw->active_pkg_ver.
> >
> > Signed-off-by: Ting Xu <ting...@intel.com>
> > ---
> > drivers/net/ice/ice_ethdev.c | 132
> > ++++++++++++++++++++++++++++++++++-
> > drivers/net/ice/ice_ethdev.h | 8 +++
> > 2 files changed, 139 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ice/ice_ethdev.c
> > b/drivers/net/ice/ice_ethdev.c index 686d6f00f..92eafdd1f 100644
> > --- a/drivers/net/ice/ice_ethdev.c
> > +++ b/drivers/net/ice/ice_ethdev.c
> > @@ -28,7 +28,15 @@ static const char * const ice_valid_args[] = { };
> >
> > #define ICE_DFLT_OUTER_TAG_TYPE
> ICE_AQ_VSI_OUTER_TAG_VLAN_9100
> > +
> > +/* DDP package search path */
> > #define ICE_DFLT_PKG_FILE "/lib/firmware/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_OS_DEFAULT_PKG_NAME "ICE OS Default
> > Package"
> > +#define ICE_COMMS_PKG_NAME "ICE COMMS
> > Package"
> > +#define ICE_MAX_PKG_FILENAME_SIZE 256
> >
> > int ice_logtype_init;
> > int ice_logtype_driver;
> > @@ -1265,15 +1273,134 @@ ice_pf_setup(struct ice_pf *pf)
> > return 0;
> > }
> >
> > +/* PCIe configuration space setting */
> > +#define PCI_CFG_SPACE_SIZE 256
> > +#define PCI_CFG_SPACE_EXP_SIZE 4096
> > +#define PCI_EXT_CAP_ID(header) (int)((header) & 0x0000ffff)
> > +#define PCI_EXT_CAP_NEXT(header) (((header) >> 20) & 0xffc)
> > +#define PCI_EXT_CAP_ID_DSN 0x03
> > +
> > +static int
> > +ice_pci_find_next_ext_capability(struct rte_pci_device *dev, int cap) {
> > + uint32_t header;
> > + int ttl;
> > + int pos = PCI_CFG_SPACE_SIZE;
> > +
> > + /* minimum 8 bytes per capability */
> > + ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8;
> > +
> > + if (rte_pci_read_config(dev, &header, 4, pos) < 0) {
> > + PMD_INIT_LOG(ERR, "ice error reading extended
> > capabilities\n");
> > + return -1;
> > + }
> > +
> > + /*
> > + * If we have no capabilities, this is indicated by cap ID,
> > + * cap version and next pointer all being 0.
> > + */
> > + if (header == 0)
> > + return 0;
> > +
> > + while (ttl-- > 0) {
> > + if (PCI_EXT_CAP_ID(header) == cap)
> > + return pos;
> > +
> > + pos = PCI_EXT_CAP_NEXT(header);
> > +
> > + if (pos < PCI_CFG_SPACE_SIZE)
> > + break;
> > +
> > + if (rte_pci_read_config(dev, &header, 4, pos) < 0) {
> > + PMD_INIT_LOG(ERR, "ice error reading extended
> > capabilities\n");
> > + return -1;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* Extract device serial number from PCIe Configuration Space and
> > + * determine the pkg file path according to the DSN.
> > + */
> > +static int
> > +ice_pkg_file_search_path(struct rte_pci_device *pci_dev, char
> > +*pkg_file) {
> > + int pos;
> > + char opt_ddp_filename[ICE_MAX_PKG_FILENAME_SIZE];
> > + uint32_t dword;
> > + uint32_t dsn_low, dsn_high;
> > +
> > + pos = ice_pci_find_next_ext_capability(pci_dev,
> > PCI_EXT_CAP_ID_DSN);
> > +
> > + if (pos) {
> > + rte_pci_read_config(pci_dev, &dword, 4, pos + 4);
> > + dsn_low = dword;
> > + rte_pci_read_config(pci_dev, &dword, 4, pos + 8);
> > + dsn_high = dword;
> > + snprintf(opt_ddp_filename, ICE_MAX_PKG_FILENAME_SIZE,
> > + "ice-%08x%08x.pkg", dsn_high, dsn_low);
> > + } else {
> > + PMD_INIT_LOG(INFO, "Failed to read device serial
> > number\n");
> > + strncpy(pkg_file, ICE_DFLT_PKG_FILE,
> > ICE_MAX_PKG_FILENAME_SIZE);
> > +
> > + return 0;
> > + }
> > +
> > + strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_DEFAULT,
> > + ICE_MAX_PKG_FILENAME_SIZE);
> > + if (!access(strncat(pkg_file, opt_ddp_filename,
> > + ICE_MAX_PKG_FILENAME_SIZE), 0))
> > + return 0;
> > +
> > + strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_UPDATES,
> > + ICE_MAX_PKG_FILENAME_SIZE);
> > + if (!access(strncat(pkg_file, opt_ddp_filename,
> > + ICE_MAX_PKG_FILENAME_SIZE), 0))
> > + return 0;
> > +
>
> The above search order is not correct. You should search the updates path
> first and then the default path. That is the order that the kernel interfaces
> search them.
>
> > + strncpy(pkg_file, ICE_DFLT_PKG_FILE,
> > ICE_MAX_PKG_FILENAME_SIZE);
> > +
> > + return 0;
> > +}
> > +
> > +static enum ice_pkg_type
> > +ice_get_pkg_type(struct ice_hw *hw)
> > +{
> > + enum ice_pkg_type package_type;
> > +
> > + /* store the activated package type (OS default or Comms) */
> > + if (!strncmp((char *)hw->active_pkg_name,
> > ICE_OS_DEFAULT_PKG_NAME,
> > + ICE_PKG_NAME_SIZE))
> > + package_type = ICE_PKG_TYPE_OS_DEFAULT;
> > + else if (!strncmp((char *)hw->active_pkg_name,
> > ICE_COMMS_PKG_NAME,
> > + ICE_PKG_NAME_SIZE))
> > + package_type = ICE_PKG_TYPE_COMMS;
> > + else
> > + package_type = ICE_PKG_TYPE_UNKNOWN;
> > +
> > + PMD_INIT_LOG(INFO, "Activated package is: %d.%d.%d.%d, %s\n",
>
> Activated => Active
>
> > + hw->active_pkg_ver.major, hw->active_pkg_ver.minor,
> > + hw->active_pkg_ver.update, hw->active_pkg_ver.draft,
> > + hw->active_pkg_name);
> > +
> > + return package_type;
> > +}
> > +
> > static int ice_load_pkg(struct rte_eth_dev *dev) {
> > struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > - const char *pkg_file = ICE_DFLT_PKG_FILE;
> > + char pkg_file[ICE_MAX_PKG_FILENAME_SIZE];
> > int err;
> > uint8_t *buf;
> > int buf_len;
> > FILE *file;
> > struct stat fstat;
> > + struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
> > + struct ice_adapter *ad =
> > + ICE_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> > +
> > + ice_pkg_file_search_path(pci_dev, pkg_file);
> >
> > file = fopen(pkg_file, "rb");
> > if (!file) {
> > @@ -1313,6 +1440,9 @@ static int ice_load_pkg(struct rte_eth_dev *dev)
> > PMD_INIT_LOG(ERR, "ice_copy_and_init_hw failed: %d\n", err);
> > goto fail_exit;
> > }
> > +
> > + ad->active_pkg_type = ice_get_pkg_type(hw);
> > +
>
> How does active_pkg_type get used? What is the purpose?
It will be used in following patches.
Basically we need this flag to expose package corresponding device capability
at runtime for example rte_flow, supported ptype ...
>
> > err = ice_init_hw_tbls(hw);
> > if (err) {
> > PMD_INIT_LOG(ERR, "ice_init_hw_tbls failed: %d\n", err); diff
> > --git
> > a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h index
> > 5063960a8..d1d07641d 100644
> > --- a/drivers/net/ice/ice_ethdev.h
> > +++ b/drivers/net/ice/ice_ethdev.h
> > @@ -124,6 +124,13 @@
> > #define ICE_ETH_OVERHEAD \
> > (RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN + ICE_VLAN_TAG_SIZE *
> 2)
> >
> > +/* DDP package type */
> > +enum ice_pkg_type {
> > + ICE_PKG_TYPE_UNKNOWN,
> > + ICE_PKG_TYPE_OS_DEFAULT,
> > + ICE_PKG_TYPE_COMMS,
> > +};
> > +
> > struct ice_adapter;
> >
> > /**
> > @@ -296,6 +303,7 @@ struct ice_adapter {
> > uint32_t ptype_tbl[ICE_MAX_PKT_TYPE] __rte_cache_min_aligned;
> > bool is_safe_mode;
> > struct ice_devargs devargs;
> > + enum ice_pkg_type active_pkg_type; /* loaded ddp package type */
> > };
> >
> > struct ice_vsi_vlan_pvid_info {
> > --
> > 2.17.1
