DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Xu, Ting" <ting.xu@intel.com>
To: "Ye, Xiaolong" <xiaolong.ye@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
	"Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
	"Yang, Qiming" <qiming.yang@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3] net/ice: support device-specific DDP package loading
Date: Wed, 4 Sep 2019 02:40:51 +0000	[thread overview]
Message-ID: <8C448DD570D7CD459EACDF44AB3869D701910286@SHSMSX106.ccr.corp.intel.com> (raw)
In-Reply-To: <20190903094353.GB15462@intel.com>

Hi, Xiaolong,

As for the pkg_file, we first determine the pkg file path according to the search order. And then it will be given to the original pkg loading function, ice_load_pkg, to check if the pkg is available. If not, an error is returned. 
I do not check the access because it will be checked in the next step, so I  want to reduce some operations. 

Thanks!
------------------
Best Regards,
Xu Ting


-----Original Message-----
From: Ye, Xiaolong 
Sent: Tuesday, September 3, 2019 5:44 PM
To: Xu, Ting <ting.xu@intel.com>
Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3] net/ice: support device-specific DDP package loading

Hi, Ting

On 09/03, Ting Xu wrote:
>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/updates/intel/ice/ddp/
>and /lib/firmware/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.xu@intel.com>
>
>-------------------------------------------------------------------
>
>v3: try to fix compile error; modify to change the pkg loading order.
>
>v2: modify codes according to the comments.
>---

You should put your change log after '---' marker, then it will be ignored when applying by `git am`, other separator like '----------------------...' 
you used will still show in the commit log.

> drivers/net/ice/ice_ethdev.c | 138 ++++++++++++++++++++++++++++++++++-
> drivers/net/ice/ice_ethdev.h |   8 ++
> 2 files changed, 144 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/ice/ice_ethdev.c 
>b/drivers/net/ice/ice_ethdev.c index 686d6f00f..922b71ac6 100644
>--- a/drivers/net/ice/ice_ethdev.c
>+++ b/drivers/net/ice/ice_ethdev.c
>@@ -28,7 +28,16 @@ static const char * const ice_valid_args[] = {  };
> 
> #define ICE_DFLT_OUTER_TAG_TYPE ICE_AQ_VSI_OUTER_TAG_VLAN_9100 
>-#define ICE_DFLT_PKG_FILE "/lib/firmware/intel/ice/ddp/ice.pkg"
>+
>+/* DDP package search path */
>+#define ICE_PKG_FILE_DEFAULT "/lib/firmware/intel/ice/ddp/ice.pkg"
>+#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_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 +1274,137 @@ 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_PKG_FILE_DEFAULT,
>+			ICE_MAX_PKG_FILENAME_SIZE);
>+
>+		return 0;
>+	}
>+
>+	strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_UPDATES,
>+		ICE_MAX_PKG_FILENAME_SIZE);
>+	if (!access(strcat(pkg_file, opt_ddp_filename), 0))
>+		return 0;
>+
>+	strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_DEFAULT,
>+		ICE_MAX_PKG_FILENAME_SIZE);
>+	if (!access(strcat(pkg_file, opt_ddp_filename), 0))
>+		return 0;
>+
>+	strncpy(pkg_file, ICE_PKG_FILE_UPDATES, ICE_MAX_PKG_FILENAME_SIZE);
>+	if (!access(pkg_file, 0))
>+		return 0;
>+
>+	strncpy(pkg_file, ICE_PKG_FILE_DEFAULT, ICE_MAX_PKG_FILENAME_SIZE);

Do we need to check access for the pkg_file here too? What if the file doesn't exist or user doesn't have access to it?

Thanks,
Xiaolong

>+
>+	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, "Active package is: %d.%d.%d.%d, %s\n",
>+		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 +1444,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);
>+
> 	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
>

  reply	other threads:[~2019-09-04  2:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-29 13:17 [dpdk-dev] [PATCH v1] " Ting Xu
2019-08-29 18:08 ` Stillwell Jr, Paul M
2019-08-29 22:05   ` Zhang, Qi Z
2019-09-03 14:29 ` [dpdk-dev] [PATCH v3] " Ting Xu
2019-09-03  9:43   ` Ye Xiaolong
2019-09-04  2:40     ` Xu, Ting [this message]
2019-09-05 12:59   ` [dpdk-dev] [PATCH v4] " Ting Xu
2019-09-06 14:51     ` [dpdk-dev] [PATCH v5] " Ting Xu
2019-09-06 13:53       ` Ye Xiaolong
2019-09-06 14:16       ` Ye Xiaolong
2019-09-11 12:35       ` [dpdk-dev] [PATCH v6] " Ting Xu
2019-09-13 10:51         ` Ye Xiaolong
2019-09-17  8:44         ` [dpdk-dev] [PATCH v7] " Ting Xu
2019-09-17 13:02           ` [dpdk-dev] [PATCH v8] " Ting Xu
2019-09-19  6:32             ` Zhang, Qi Z
2019-09-19 22:08             ` Ye Xiaolong
2019-09-23  9:50             ` Ye Xiaolong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8C448DD570D7CD459EACDF44AB3869D701910286@SHSMSX106.ccr.corp.intel.com \
    --to=ting.xu@intel.com \
    --cc=dev@dpdk.org \
    --cc=qi.z.zhang@intel.com \
    --cc=qiming.yang@intel.com \
    --cc=wenzhuo.lu@intel.com \
    --cc=xiaolong.ye@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).