DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/8] optimize the firmware loading process
@ 2024-01-15  2:54 Chaoyong He
  2024-01-15  2:54 ` [PATCH 1/8] net/nfp: add the interface for getting the firmware name Chaoyong He
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Chaoyong He @ 2024-01-15  2:54 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Chaoyong He

This patch series aims to speedup the DPDK application start by optimize
the firmware loading process in sereval places.
We also simplify the port name in multiple PF firmware case to make the
customer happy.

Peng Zhang (8):
  net/nfp: add the interface for getting the firmware name
  net/nfp: speed up the firmware loading process
  net/nfp: optimize loading the firmware process
  net/nfp: enlarge the range of skipping loading the firmware
  net/nfp: add the step of clearing the beat time
  net/nfp: add the elf module
  net/nfp: reload the firmware only when firmware changed
  net/nfp: simplify the port name for multiple PFs

 drivers/net/nfp/meson.build       |    1 +
 drivers/net/nfp/nfp_ethdev.c      |  264 +++++--
 drivers/net/nfp/nfp_net_common.c  |   17 +
 drivers/net/nfp/nfp_net_common.h  |    2 +
 drivers/net/nfp/nfpcore/nfp_elf.c | 1078 +++++++++++++++++++++++++++++
 drivers/net/nfp/nfpcore/nfp_elf.h |   13 +
 drivers/net/nfp/nfpcore/nfp_mip.c |   30 +-
 drivers/net/nfp/nfpcore/nfp_mip.h |   70 +-
 8 files changed, 1388 insertions(+), 87 deletions(-)
 create mode 100644 drivers/net/nfp/nfpcore/nfp_elf.c
 create mode 100644 drivers/net/nfp/nfpcore/nfp_elf.h

-- 
2.39.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/8] net/nfp: add the interface for getting the firmware name
  2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
@ 2024-01-15  2:54 ` Chaoyong He
  2024-01-15  2:54 ` [PATCH 2/8] net/nfp: speed up the firmware loading process Chaoyong He
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Chaoyong He @ 2024-01-15  2:54 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Peng Zhang, Chaoyong He, Long Wu

From: Peng Zhang <peng.zhang@corigine.com>

Add the interfce for getting the suitable firmware name.

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 79 ++++++++++++++++++++++--------------
 1 file changed, 48 insertions(+), 31 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 886b568d96..434d664573 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -954,14 +954,13 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
 #define DEFAULT_FW_PATH       "/lib/firmware/netronome"
 
 static int
-nfp_fw_upload(struct rte_pci_device *dev,
+nfp_fw_get_name(struct rte_pci_device *dev,
 		struct nfp_nsp *nsp,
-		char *card)
+		char *card,
+		char *fw_name,
+		size_t fw_size)
 {
-	void *fw_buf;
-	size_t fsize;
 	char serial[40];
-	char fw_name[125];
 	uint16_t interface;
 	uint32_t cpp_serial_len;
 	const uint8_t *cpp_serial;
@@ -980,30 +979,43 @@ nfp_fw_upload(struct rte_pci_device *dev,
 			"serial-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x",
 			cpp_serial[0], cpp_serial[1], cpp_serial[2], cpp_serial[3],
 			cpp_serial[4], cpp_serial[5], interface >> 8, interface & 0xff);
-	snprintf(fw_name, sizeof(fw_name), "%s/%s.nffw", DEFAULT_FW_PATH, serial);
+	snprintf(fw_name, fw_size, "%s/%s.nffw", DEFAULT_FW_PATH, serial);
 
 	PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
-	if (rte_firmware_read(fw_name, &fw_buf, &fsize) == 0)
-		goto load_fw;
+	if (access(fw_name, F_OK) == 0)
+		return 0;
 
 	/* Then try the PCI name */
-	snprintf(fw_name, sizeof(fw_name), "%s/pci-%s.nffw", DEFAULT_FW_PATH,
+	snprintf(fw_name, fw_size, "%s/pci-%s.nffw", DEFAULT_FW_PATH,
 			dev->name);
 
 	PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
-	if (rte_firmware_read(fw_name, &fw_buf, &fsize) == 0)
-		goto load_fw;
+	if (access(fw_name, F_OK) == 0)
+		return 0;
 
 	/* Finally try the card type and media */
-	snprintf(fw_name, sizeof(fw_name), "%s/%s", DEFAULT_FW_PATH, card);
+	snprintf(fw_name, fw_size, "%s/%s", DEFAULT_FW_PATH, card);
 	PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
-	if (rte_firmware_read(fw_name, &fw_buf, &fsize) == 0)
-		goto load_fw;
+	if (access(fw_name, F_OK) == 0)
+		return 0;
 
-	PMD_DRV_LOG(ERR, "Can't find suitable firmware.");
 	return -ENOENT;
+}
+
+static int
+nfp_fw_upload(struct nfp_nsp *nsp,
+		char *fw_name)
+{
+	int err;
+	void *fw_buf;
+	size_t fsize;
+
+	err = rte_firmware_read(fw_name, &fw_buf, &fsize);
+	if (err != 0) {
+		PMD_DRV_LOG(ERR, "firmware %s not found!", fw_name);
+		return -ENOENT;
+	}
 
-load_fw:
 	PMD_DRV_LOG(INFO, "Firmware file found at %s with size: %zu",
 			fw_name, fsize);
 	PMD_DRV_LOG(INFO, "Uploading the firmware ...");
@@ -1034,14 +1046,13 @@ nfp_fw_unload(struct nfp_cpp *cpp)
 }
 
 static int
-nfp_fw_reload(struct rte_pci_device *dev,
-		struct nfp_nsp *nsp,
-		char *card_desc)
+nfp_fw_reload(struct nfp_nsp *nsp,
+		char *fw_name)
 {
 	int err;
 
 	nfp_nsp_device_soft_reset(nsp);
-	err = nfp_fw_upload(dev, nsp, card_desc);
+	err = nfp_fw_upload(nsp, fw_name);
 	if (err != 0)
 		PMD_DRV_LOG(ERR, "NFP firmware load failed");
 
@@ -1049,9 +1060,8 @@ nfp_fw_reload(struct rte_pci_device *dev,
 }
 
 static int
-nfp_fw_loaded_check_alive(struct rte_pci_device *dev,
-		struct nfp_nsp *nsp,
-		char *card_desc,
+nfp_fw_loaded_check_alive(struct nfp_nsp *nsp,
+		char *fw_name,
 		const struct nfp_dev_info *dev_info,
 		struct nfp_multi_pf *multi_pf)
 {
@@ -1077,13 +1087,12 @@ nfp_fw_loaded_check_alive(struct rte_pci_device *dev,
 		}
 	}
 
-	return nfp_fw_reload(dev, nsp, card_desc);
+	return nfp_fw_reload(nsp, fw_name);
 }
 
 static int
-nfp_fw_reload_for_multipf(struct rte_pci_device *dev,
-		struct nfp_nsp *nsp,
-		char *card_desc,
+nfp_fw_reload_for_multipf(struct nfp_nsp *nsp,
+		char *fw_name,
 		struct nfp_cpp *cpp,
 		const struct nfp_dev_info *dev_info,
 		struct nfp_multi_pf *multi_pf)
@@ -1095,9 +1104,9 @@ nfp_fw_reload_for_multipf(struct rte_pci_device *dev,
 		PMD_DRV_LOG(ERR, "NFP write beat failed");
 
 	if (nfp_nsp_fw_loaded(nsp))
-		err = nfp_fw_loaded_check_alive(dev, nsp, card_desc, dev_info, multi_pf);
+		err = nfp_fw_loaded_check_alive(nsp, fw_name, dev_info, multi_pf);
 	else
-		err = nfp_fw_reload(dev, nsp, card_desc);
+		err = nfp_fw_reload(nsp, fw_name);
 	if (err != 0) {
 		nfp_net_keepalive_uninit(multi_pf);
 		return err;
@@ -1121,6 +1130,7 @@ nfp_fw_setup(struct rte_pci_device *dev,
 		struct nfp_multi_pf *multi_pf)
 {
 	int err;
+	char fw_name[125];
 	char card_desc[100];
 	struct nfp_nsp *nsp;
 	const char *nfp_fw_model;
@@ -1157,10 +1167,17 @@ nfp_fw_setup(struct rte_pci_device *dev,
 		return -EIO;
 	}
 
+	err = nfp_fw_get_name(dev, nsp, card_desc, fw_name, sizeof(fw_name));
+	if (err != 0) {
+		PMD_DRV_LOG(ERR, "Can't find suitable firmware.");
+		nfp_nsp_close(nsp);
+		return err;
+	}
+
 	if (multi_pf->enabled)
-		err = nfp_fw_reload_for_multipf(dev, nsp, card_desc, cpp, dev_info, multi_pf);
+		err = nfp_fw_reload_for_multipf(nsp, fw_name, cpp, dev_info, multi_pf);
 	else
-		err = nfp_fw_reload(dev, nsp, card_desc);
+		err = nfp_fw_reload(nsp, fw_name);
 
 	nfp_nsp_close(nsp);
 	return err;
-- 
2.39.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/8] net/nfp: speed up the firmware loading process
  2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
  2024-01-15  2:54 ` [PATCH 1/8] net/nfp: add the interface for getting the firmware name Chaoyong He
@ 2024-01-15  2:54 ` Chaoyong He
  2024-01-15  2:54 ` [PATCH 3/8] net/nfp: optimize loading the firmware process Chaoyong He
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Chaoyong He @ 2024-01-15  2:54 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Peng Zhang, Chaoyong He, Long Wu

From: Peng Zhang <peng.zhang@corigine.com>

The previous design spent too much time checking whether
the port was alive, now optimize this process and speed
up the firmware loading process.

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 434d664573..e06e6b7f59 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1065,24 +1065,29 @@ nfp_fw_loaded_check_alive(struct nfp_nsp *nsp,
 		const struct nfp_dev_info *dev_info,
 		struct nfp_multi_pf *multi_pf)
 {
-	int offset;
-	uint32_t i;
-	uint64_t beat;
+	uint8_t i;
+	uint64_t tmp_beat;
 	uint32_t port_num;
+	uint64_t beat[dev_info->pf_num_per_unit];
+	uint32_t offset[dev_info->pf_num_per_unit];
+
+	for (port_num = 0; port_num < dev_info->pf_num_per_unit; port_num++) {
+		offset[port_num] = NFP_BEAT_OFFSET(port_num);
+		beat[port_num] = nn_readq(multi_pf->beat_addr + offset[port_num]);
+	}
 
 	/*
 	 * If the beats of any other port changed in 3s,
 	 * we should not reload the firmware.
 	 */
-	for (port_num = 0; port_num < dev_info->pf_num_per_unit; port_num++) {
-		if (port_num == multi_pf->function_id)
-			continue;
-
-		offset = NFP_BEAT_OFFSET(port_num);
-		beat = nn_readq(multi_pf->beat_addr + offset);
-		for (i = 0; i < 3; i++) {
-			sleep(1);
-			if (nn_readq(multi_pf->beat_addr + offset) != beat)
+	for (i = 0; i < 3; i++) {
+		sleep(1);
+		for (port_num = 0; port_num < dev_info->pf_num_per_unit; port_num++) {
+			if (port_num == multi_pf->function_id)
+				continue;
+
+			tmp_beat = nn_readq(multi_pf->beat_addr + offset[port_num]);
+			if (tmp_beat != beat[port_num])
 				return 0;
 		}
 	}
-- 
2.39.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 3/8] net/nfp: optimize loading the firmware process
  2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
  2024-01-15  2:54 ` [PATCH 1/8] net/nfp: add the interface for getting the firmware name Chaoyong He
  2024-01-15  2:54 ` [PATCH 2/8] net/nfp: speed up the firmware loading process Chaoyong He
@ 2024-01-15  2:54 ` Chaoyong He
  2024-01-15  2:54 ` [PATCH 4/8] net/nfp: enlarge the range of skipping loading the firmware Chaoyong He
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Chaoyong He @ 2024-01-15  2:54 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Peng Zhang, Chaoyong He, Long Wu

From: Peng Zhang <peng.zhang@corigine.com>

Add the skip loading firmware flag, when any other
port is alive, this flag will be true.
Also make sure the order of starting the keepalive
is earlier than getting the firmware status.

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index e06e6b7f59..e908fc8472 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1059,10 +1059,8 @@ nfp_fw_reload(struct nfp_nsp *nsp,
 	return err;
 }
 
-static int
-nfp_fw_loaded_check_alive(struct nfp_nsp *nsp,
-		char *fw_name,
-		const struct nfp_dev_info *dev_info,
+static bool
+nfp_fw_skip_load(const struct nfp_dev_info *dev_info,
 		struct nfp_multi_pf *multi_pf)
 {
 	uint8_t i;
@@ -1088,11 +1086,11 @@ nfp_fw_loaded_check_alive(struct nfp_nsp *nsp,
 
 			tmp_beat = nn_readq(multi_pf->beat_addr + offset[port_num]);
 			if (tmp_beat != beat[port_num])
-				return 0;
+				return true;
 		}
 	}
 
-	return nfp_fw_reload(nsp, fw_name);
+	return false;
 }
 
 static int
@@ -1103,17 +1101,11 @@ nfp_fw_reload_for_multipf(struct nfp_nsp *nsp,
 		struct nfp_multi_pf *multi_pf)
 {
 	int err;
+	bool skip_load_fw = false;
 
 	err = nfp_net_keepalive_init(cpp, multi_pf);
-	if (err != 0)
-		PMD_DRV_LOG(ERR, "NFP write beat failed");
-
-	if (nfp_nsp_fw_loaded(nsp))
-		err = nfp_fw_loaded_check_alive(nsp, fw_name, dev_info, multi_pf);
-	else
-		err = nfp_fw_reload(nsp, fw_name);
 	if (err != 0) {
-		nfp_net_keepalive_uninit(multi_pf);
+		PMD_DRV_LOG(ERR, "NFP init beat failed");
 		return err;
 	}
 
@@ -1121,9 +1113,23 @@ nfp_fw_reload_for_multipf(struct nfp_nsp *nsp,
 	if (err != 0) {
 		nfp_net_keepalive_uninit(multi_pf);
 		PMD_DRV_LOG(ERR, "NFP write beat failed");
+		return err;
 	}
 
-	return err;
+	if (nfp_nsp_fw_loaded(nsp))
+		skip_load_fw = nfp_fw_skip_load(dev_info, multi_pf);
+
+	if (skip_load_fw)
+		return 0;
+
+	err = nfp_fw_reload(nsp, fw_name);
+	if (err != 0) {
+		nfp_net_keepalive_stop(multi_pf);
+		nfp_net_keepalive_uninit(multi_pf);
+		return err;
+	}
+
+	return 0;
 }
 
 static int
-- 
2.39.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 4/8] net/nfp: enlarge the range of skipping loading the firmware
  2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
                   ` (2 preceding siblings ...)
  2024-01-15  2:54 ` [PATCH 3/8] net/nfp: optimize loading the firmware process Chaoyong He
@ 2024-01-15  2:54 ` Chaoyong He
  2024-01-15  2:54 ` [PATCH 5/8] net/nfp: add the step of clearing the beat time Chaoyong He
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Chaoyong He @ 2024-01-15  2:54 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Peng Zhang, Chaoyong He, Long Wu

From: Peng Zhang <peng.zhang@corigine.com>

Enlarge the range of skip loading firmware by add the condition
there is no port with the condition of abnormal.

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index e908fc8472..25f5fe483f 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1066,28 +1066,46 @@ nfp_fw_skip_load(const struct nfp_dev_info *dev_info,
 	uint8_t i;
 	uint64_t tmp_beat;
 	uint32_t port_num;
+	uint8_t in_use = 0;
 	uint64_t beat[dev_info->pf_num_per_unit];
 	uint32_t offset[dev_info->pf_num_per_unit];
+	uint8_t abnormal = dev_info->pf_num_per_unit;
 
 	for (port_num = 0; port_num < dev_info->pf_num_per_unit; port_num++) {
 		offset[port_num] = NFP_BEAT_OFFSET(port_num);
 		beat[port_num] = nn_readq(multi_pf->beat_addr + offset[port_num]);
+		if (beat[port_num] == 0)
+			abnormal--;
 	}
 
-	/*
-	 * If the beats of any other port changed in 3s,
-	 * we should not reload the firmware.
-	 */
+	if (abnormal == 0)
+		return true;
+
 	for (i = 0; i < 3; i++) {
 		sleep(1);
 		for (port_num = 0; port_num < dev_info->pf_num_per_unit; port_num++) {
 			if (port_num == multi_pf->function_id)
 				continue;
 
+			if (beat[port_num] == 0)
+				continue;
+
 			tmp_beat = nn_readq(multi_pf->beat_addr + offset[port_num]);
-			if (tmp_beat != beat[port_num])
-				return true;
+			if (tmp_beat != beat[port_num]) {
+				in_use++;
+				abnormal--;
+				beat[port_num] = 0;
+			}
 		}
+
+		if (abnormal == 0)
+			return true;
+	}
+
+	if (in_use != 0) {
+		PMD_DRV_LOG(WARNING, "Abnormal %u != 0, the nic has port which is exit abnormally.",
+				abnormal);
+		return true;
 	}
 
 	return false;
-- 
2.39.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 5/8] net/nfp: add the step of clearing the beat time
  2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
                   ` (3 preceding siblings ...)
  2024-01-15  2:54 ` [PATCH 4/8] net/nfp: enlarge the range of skipping loading the firmware Chaoyong He
@ 2024-01-15  2:54 ` Chaoyong He
  2024-01-15  2:54 ` [PATCH 6/8] net/nfp: add the elf module Chaoyong He
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Chaoyong He @ 2024-01-15  2:54 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Peng Zhang, Chaoyong He, Long Wu

From: Peng Zhang <peng.zhang@corigine.com>

Add the step of clearing the beat time of specific
port when DPDK exit. Add the step of clearing the
beat time of other ports when reload firmware.

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 25f5fe483f..46bb09a211 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -470,6 +470,27 @@ nfp_net_keepalive_start(struct nfp_multi_pf *multi_pf)
 	return 0;
 }
 
+static void
+nfp_net_keepalive_clear(uint8_t *beat_addr,
+		uint8_t function_id)
+{
+	nn_writeq(0, beat_addr + NFP_BEAT_OFFSET(function_id));
+}
+
+static void
+nfp_net_keepalive_clear_others(const struct nfp_dev_info *dev_info,
+		struct nfp_multi_pf *multi_pf)
+{
+	uint8_t port_num;
+
+	for (port_num = 0; port_num < dev_info->pf_num_per_unit; port_num++) {
+		if (port_num == multi_pf->function_id)
+			continue;
+
+		nfp_net_keepalive_clear(multi_pf->beat_addr, port_num);
+	}
+}
+
 static void
 nfp_net_keepalive_stop(struct nfp_multi_pf *multi_pf)
 {
@@ -524,6 +545,7 @@ nfp_pf_uninit(struct nfp_pf_dev *pf_dev)
 	free(pf_dev->sym_tbl);
 	if (pf_dev->multi_pf.enabled) {
 		nfp_net_keepalive_stop(&pf_dev->multi_pf);
+		nfp_net_keepalive_clear(pf_dev->multi_pf.beat_addr, pf_dev->multi_pf.function_id);
 		nfp_net_keepalive_uninit(&pf_dev->multi_pf);
 	}
 	free(pf_dev->nfp_eth_table);
@@ -1147,6 +1169,8 @@ nfp_fw_reload_for_multipf(struct nfp_nsp *nsp,
 		return err;
 	}
 
+	nfp_net_keepalive_clear_others(dev_info, multi_pf);
+
 	return 0;
 }
 
@@ -1779,6 +1803,7 @@ nfp_pf_init(struct rte_pci_device *pci_dev)
 fw_cleanup:
 	nfp_fw_unload(cpp);
 	nfp_net_keepalive_stop(&pf_dev->multi_pf);
+	nfp_net_keepalive_clear(pf_dev->multi_pf.beat_addr, pf_dev->multi_pf.function_id);
 	nfp_net_keepalive_uninit(&pf_dev->multi_pf);
 eth_table_cleanup:
 	free(nfp_eth_table);
-- 
2.39.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 6/8] net/nfp: add the elf module
  2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
                   ` (4 preceding siblings ...)
  2024-01-15  2:54 ` [PATCH 5/8] net/nfp: add the step of clearing the beat time Chaoyong He
@ 2024-01-15  2:54 ` Chaoyong He
  2024-01-15  2:54 ` [PATCH 7/8] net/nfp: reload the firmware only when firmware changed Chaoyong He
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Chaoyong He @ 2024-01-15  2:54 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Peng Zhang, Chaoyong He, Long Wu

From: Peng Zhang <peng.zhang@corigine.com>

Add the elf module, which can get mip information from the
firmware ELF file.

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
---
 drivers/net/nfp/meson.build       |    1 +
 drivers/net/nfp/nfpcore/nfp_elf.c | 1078 +++++++++++++++++++++++++++++
 drivers/net/nfp/nfpcore/nfp_elf.h |   13 +
 drivers/net/nfp/nfpcore/nfp_mip.c |   30 +-
 drivers/net/nfp/nfpcore/nfp_mip.h |   70 +-
 5 files changed, 1167 insertions(+), 25 deletions(-)
 create mode 100644 drivers/net/nfp/nfpcore/nfp_elf.c
 create mode 100644 drivers/net/nfp/nfpcore/nfp_elf.h

diff --git a/drivers/net/nfp/meson.build b/drivers/net/nfp/meson.build
index 46be6f60cd..a4060b4cd5 100644
--- a/drivers/net/nfp/meson.build
+++ b/drivers/net/nfp/meson.build
@@ -17,6 +17,7 @@ sources = files(
         'nfdk/nfp_nfdk_dp.c',
         'nfpcore/nfp_cppcore.c',
         'nfpcore/nfp_crc.c',
+        'nfpcore/nfp_elf.c',
         'nfpcore/nfp_hwinfo.c',
         'nfpcore/nfp_mip.c',
         'nfpcore/nfp_mutex.c',
diff --git a/drivers/net/nfp/nfpcore/nfp_elf.c b/drivers/net/nfp/nfpcore/nfp_elf.c
new file mode 100644
index 0000000000..2826fcd410
--- /dev/null
+++ b/drivers/net/nfp/nfpcore/nfp_elf.c
@@ -0,0 +1,1078 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2023 Corigine, Inc.
+ * All rights reserved.
+ */
+
+#include "nfp_elf.h"
+
+#include <malloc.h>
+#include <stdbool.h>
+#include <ethdev_pci.h>
+
+#include <nfp_platform.h>
+#include <rte_common.h>
+#include <eal_firmware.h>
+
+#include "nfp_logs.h"
+#include "nfp_mip.h"
+
+/*
+ * NFP Chip Families.
+ *
+ * These are not enums, because they need to be microcode compatible.
+ * They are also not maskable.
+ *
+ * Note: The NFP-4xxx family is handled as NFP-6xxx in most software
+ * components.
+ */
+#define NFP_CHIP_FAMILY_NFP3800 0x3800
+#define NFP_CHIP_FAMILY_NFP6000 0x6000
+
+/* Standard ELF */
+#define NFP_ELF_EI_NIDENT     16
+#define NFP_ELF_EI_MAG0       0
+#define NFP_ELF_EI_MAG1       1
+#define NFP_ELF_EI_MAG2       2
+#define NFP_ELF_EI_MAG3       3
+#define NFP_ELF_EI_CLASS      4
+#define NFP_ELF_EI_DATA       5
+#define NFP_ELF_EI_VERSION    6
+#define NFP_ELF_EI_PAD        7
+#define NFP_ELF_ELFMAG0       0x7f
+#define NFP_ELF_ELFMAG1       'E'
+#define NFP_ELF_ELFMAG2       'L'
+#define NFP_ELF_ELFMAG3       'F'
+#define NFP_ELF_ELFCLASSNONE  0
+#define NFP_ELF_ELFCLASS32    1
+#define NFP_ELF_ELFCLASS64    2
+#define NFP_ELF_ELFDATANONE   0
+#define NFP_ELF_ELFDATA2LSB   1
+#define NFP_ELF_ELFDATA2MSB   2
+
+#define NFP_ELF_ET_NONE       0
+#define NFP_ELF_ET_REL        1
+#define NFP_ELF_ET_EXEC       2
+#define NFP_ELF_ET_DYN        3
+#define NFP_ELF_ET_CORE       4
+#define NFP_ELF_ET_LOPROC     0xFF00
+#define NFP_ELF_ET_HIPROC     0xFFFF
+#define NFP_ELF_ET_NFP_PARTIAL_REL   (NFP_ELF_ET_LOPROC + NFP_ELF_ET_REL)
+#define NFP_ELF_ET_NFP_PARTIAL_EXEC  (NFP_ELF_ET_LOPROC + NFP_ELF_ET_EXEC)
+
+#define NFP_ELF_EM_NFP        250
+#define NFP_ELF_EM_NFP6000    0x6000
+
+#define NFP_ELF_SHT_NULL      0
+#define NFP_ELF_SHT_PROGBITS  1
+#define NFP_ELF_SHT_SYMTAB    2
+#define NFP_ELF_SHT_STRTAB    3
+#define NFP_ELF_SHT_RELA      4
+#define NFP_ELF_SHT_HASH      5
+#define NFP_ELF_SHT_DYNAMIC   6
+#define NFP_ELF_SHT_NOTE      7
+#define NFP_ELF_SHT_NOBITS    8
+#define NFP_ELF_SHT_REL       9
+#define NFP_ELF_SHT_SHLIB     10
+#define NFP_ELF_SHT_DYNSYM    11
+#define NFP_ELF_SHT_LOPROC    0x70000000
+#define NFP_ELF_SHT_HIPROC    0x7fffffff
+#define NFP_ELF_SHT_LOUSER    0x80000000
+#define NFP_ELF_SHT_HIUSER    0x8fffffff
+
+#define NFP_ELF_EV_NONE       0
+#define NFP_ELF_EV_CURRENT    1
+
+#define NFP_ELF_SHN_UNDEF     0
+
+/* EM_NFP ELF flags */
+
+/*
+ * Valid values for FAMILY are:
+ * 0x6000 - NFP-6xxx/NFP-4xxx
+ * 0x3800 - NFP-38xx
+ */
+#define NFP_ELF_EF_NFP_FAMILY_MASK        0xFFFF
+#define NFP_ELF_EF_NFP_FAMILY_LSB         8
+
+#define NFP_ELF_SHT_NFP_MECONFIG          (NFP_ELF_SHT_LOPROC + 1)
+#define NFP_ELF_SHT_NFP_INITREG           (NFP_ELF_SHT_LOPROC + 2)
+#define NFP_ELF_SHT_UOF_DEBUG             (NFP_ELF_SHT_LOUSER)
+
+/* NFP target revision note type */
+#define NFP_ELT_NOTE_NAME_NFP             "NFP\0"
+#define NFP_ELT_NOTE_NAME_NFP_SZ          4
+#define NFP_ELT_NOTE_NAME_NFP_USER        "NFP_USR\0"
+#define NFP_ELT_NOTE_NAME_NFP_USER_SZ     8
+#define NFP_ELF_NT_NFP_BUILD_INFO         0x100
+#define NFP_ELF_NT_NFP_REVS               0x101
+#define NFP_ELF_NT_NFP_MIP_LOCATION       0x102
+#define NFP_ELF_NT_NFP_USER               0xf0000000
+
+
+/* Standard ELF structures */
+struct nfp_elf_elf64_ehdr {
+	uint8_t e_ident[NFP_ELF_EI_NIDENT];
+	rte_le16_t e_type;
+	rte_le16_t e_machine;
+	rte_le32_t e_version;
+	rte_le64_t e_entry;
+	rte_le64_t e_phoff;
+	rte_le64_t e_shoff;
+	rte_le32_t e_flags;
+	rte_le16_t e_ehsize;
+	rte_le16_t e_phentsize;
+	rte_le16_t e_phnum;
+	rte_le16_t e_shentsize;
+	rte_le16_t e_shnum;
+	rte_le16_t e_shstrndx;
+};
+
+struct nfp_elf_elf64_shdr {
+	rte_le32_t sh_name;
+	rte_le32_t sh_type;
+	rte_le64_t sh_flags;
+	rte_le64_t sh_addr;
+	rte_le64_t sh_offset;
+	rte_le64_t sh_size;
+	rte_le32_t sh_link;
+	rte_le32_t sh_info;
+	rte_le64_t sh_addralign;
+	rte_le64_t sh_entsize;
+};
+
+struct nfp_elf_elf64_sym {
+	rte_le32_t st_name;
+	uint8_t st_info;
+	uint8_t st_other;
+	rte_le16_t st_shndx;
+	rte_le64_t st_value;
+	rte_le64_t st_size;
+};
+
+struct nfp_elf_elf64_rel {
+	rte_le64_t r_offset;
+	rte_le64_t r_info;
+};
+
+struct nfp_elf_elf64_nhdr {
+	rte_le32_t n_namesz;
+	rte_le32_t n_descsz;
+	rte_le32_t n_type;
+};
+
+/* NFP specific structures */
+struct nfp_elf_elf_meconfig {
+	rte_le32_t ctx_enables;
+	rte_le32_t entry;
+	rte_le32_t misc_control;
+	rte_le32_t reserved;
+};
+
+struct nfp_elf_elf_initregentry {
+	rte_le32_t w0;
+	rte_le32_t cpp_offset_lo;
+	rte_le32_t val;
+	rte_le32_t mask;
+};
+
+/* NFP NFFW ELF struct and API */
+struct nfp_elf_user_note {
+	const char *name;
+	uint32_t data_sz;
+	void *data;
+};
+
+/*
+ * nfp_elf_fw_mip contains firmware related fields from the MIP as well as the
+ * MIP location in the NFFW file. All fields are only valid if shndx > 0.
+ *
+ * This struct will only be available if the firmware contains a .note section
+ * with a note of type NFP_ELF_NT_NFP_MIP_LOCATION.
+ */
+struct nfp_elf_fw_mip {
+	size_t shndx;
+	uint64_t sh_offset;
+	rte_le32_t mip_ver;      /**< Version of the format of the MIP itself */
+
+	rte_le32_t fw_version;
+	rte_le32_t fw_buildnum;
+	rte_le32_t fw_buildtime;
+	char fw_name[20];        /**< At most 16 chars, 17 ensures '\0', round up */
+	const char *fw_typeid;   /**< NULL if none set */
+};
+
+/*
+ * It is preferred to access this struct via the nfp_elf functions
+ * rather than directly.
+ */
+struct nfp_elf {
+	struct nfp_elf_elf64_ehdr *ehdr;
+	struct nfp_elf_elf64_shdr *shdrs;
+	size_t shdrs_cnt;
+	void **shdrs_data;
+
+	/** True if section data has been endian swapped */
+	uint8_t *shdrs_host_endian;
+
+	size_t shdr_idx_symtab;
+
+	struct nfp_elf_elf64_sym *syms;
+	size_t syms_cnt;
+
+	char *shstrtab;
+	size_t shstrtab_sz;
+
+	char *symstrtab;
+	size_t symstrtab_sz;
+
+	struct nfp_elf_elf_meconfig *meconfs;
+	size_t meconfs_cnt;
+
+	/* ==== .note data start ==== */
+
+	/**
+	 * Following data derived from SHT_NOTE sections for read-only usage.
+	 * These fields are not used in nfp_elf_to_buf()
+	 */
+	int rev_min; /**< -1 if file did not specify */
+	int rev_max; /**< -1 if file did not specify */
+
+	/**
+	 * If mip_shndx == 0 and mip_sh_off == 0, the .note stated there is no MIP.
+	 * If mip_shndx == 0 and mip_sh_off == UINT64_MAX, there was no .note and
+	 * a MIP _may_ still be found in the first 256KiB of DRAM/EMEM data.
+	 */
+	size_t mip_shndx; /**< Section in which MIP resides, 0 if no MIP */
+	uint64_t mip_sh_off; /**< Offset within section (not address) */
+
+	struct nfp_elf_fw_mip fw_mip;
+	const char *fw_info_strtab;
+	size_t fw_info_strtab_sz;
+
+	/* ==== .note.user data start ==== */
+	size_t user_note_cnt;
+	struct nfp_elf_user_note *user_notes;
+
+	void *dbgdata;
+
+	int family;
+
+	/**
+	 * For const entry points in the API, we allocate and keep a buffer
+	 * and for mutable entry points we assume the buffer remains valid
+	 * and we just set pointers to it.
+	 */
+	void *_buf;
+	size_t _bufsz;
+};
+
+static void
+nfp_elf_free(struct nfp_elf *ectx)
+{
+	if (ectx == NULL)
+		return;
+
+	free(ectx->shdrs);
+	free(ectx->shdrs_data);
+	free(ectx->shdrs_host_endian);
+	if (ectx->_bufsz != 0)
+		free(ectx->_buf);
+
+	free(ectx);
+}
+
+static size_t
+nfp_elf_get_sec_ent_cnt(struct nfp_elf *ectx,
+		size_t idx)
+{
+	uint64_t sh_size = rte_le_to_cpu_64(ectx->shdrs[idx].sh_size);
+	uint64_t sh_entsize = rte_le_to_cpu_64(ectx->shdrs[idx].sh_entsize);
+
+	if (sh_entsize != 0)
+		return sh_size / sh_entsize;
+
+	return 0;
+}
+
+static bool
+nfp_elf_check_sh_size(uint64_t sh_size)
+{
+	if (sh_size == 0 || sh_size > UINT32_MAX)
+		return false;
+
+	return true;
+}
+
+static const char *
+nfp_elf_fwinfo_next(struct nfp_elf *ectx,
+		const char *key_val)
+{
+	size_t s_len;
+	const char *strtab = ectx->fw_info_strtab;
+	ssize_t tab_sz = (ssize_t)ectx->fw_info_strtab_sz;
+
+	if (key_val == NULL)
+		return strtab;
+
+	s_len = strlen(key_val);
+	if (key_val < strtab || ((key_val + s_len + 1) >= (strtab + tab_sz - 1)))
+		return NULL;
+
+	key_val += s_len + 1;
+
+	return key_val;
+}
+
+static const char *
+nfp_elf_fwinfo_lookup(struct nfp_elf *ectx,
+		const char *key)
+{
+	size_t s_len;
+	const char *s;
+	size_t key_len = strlen(key);
+	const char *strtab = ectx->fw_info_strtab;
+	ssize_t tab_sz = (ssize_t)ectx->fw_info_strtab_sz;
+
+	if (strtab == NULL)
+		return NULL;
+
+	for (s = strtab, s_len = strlen(s) + 1;
+			(s[0] != '\0') && (tab_sz > 0);
+			s_len = strlen(s) + 1, tab_sz -= s_len, s += s_len) {
+		if ((strncmp(s, key, key_len) == 0) && (s[key_len] == '='))
+			return &s[key_len + 1];
+	}
+
+	return NULL;
+}
+
+static bool
+nfp_elf_arch_is_thornham(struct nfp_elf *ectx)
+{
+	if (ectx == NULL)
+		return false;
+
+	if (ectx->family == NFP_CHIP_FAMILY_NFP6000 || ectx->family == NFP_CHIP_FAMILY_NFP3800)
+		return true;
+
+	return false;
+}
+
+static int
+nfp_elf_parse_sht_rel(struct nfp_elf_elf64_shdr *sec,
+		struct nfp_elf *ectx,
+		size_t idx,
+		uint8_t *buf8)
+{
+	uint64_t sh_size = rte_le_to_cpu_64(sec->sh_size);
+	uint64_t sh_offset = rte_le_to_cpu_64(sec->sh_offset);
+	uint64_t sh_entsize = rte_le_to_cpu_64(sec->sh_entsize);
+
+	if (sh_entsize != sizeof(struct nfp_elf_elf64_rel)) {
+		PMD_DRV_LOG(ERR, "Invalid ELF section header, index %zu.", idx);
+		return -EINVAL;
+	}
+
+	if (!nfp_elf_check_sh_size(sh_size)) {
+		PMD_DRV_LOG(ERR, "Invalid ELF section header, index %zu.", idx);
+		return -EINVAL;
+	}
+
+	ectx->shdrs_data[idx] = buf8 + sh_offset;
+	ectx->shdrs_host_endian[idx] = 1;
+
+	return 0;
+}
+
+static int
+nfp_elf_parse_note_name_nfp(struct nfp_elf *ectx,
+		size_t idx,
+		uint32_t ndescsz,
+		uint32_t ntype,
+		const char *nname,
+		rte_le32_t *descword)
+{
+	if (strncmp(nname, NFP_ELT_NOTE_NAME_NFP, NFP_ELT_NOTE_NAME_NFP_SZ) == 0) {
+		switch (ntype) {
+		case NFP_ELF_NT_NFP_REVS:
+			if (ndescsz != 8) {
+				PMD_DRV_LOG(ERR, "Invalid ELF NOTE descsz in section %zu.", idx);
+				return -EINVAL;
+			}
+
+			ectx->rev_min = (int)rte_le_to_cpu_32(descword[0]);
+			ectx->rev_max = (int)rte_le_to_cpu_32(descword[1]);
+			break;
+		case NFP_ELF_NT_NFP_MIP_LOCATION:
+			if (ndescsz != 12) {
+				PMD_DRV_LOG(ERR, "Invalid ELF NOTE descsz in section %zu.", idx);
+				return -EINVAL;
+			}
+
+			ectx->mip_shndx = rte_le_to_cpu_32(descword[0]);
+			if (ectx->mip_shndx == 0) {
+				ectx->mip_sh_off = 0;
+				break;
+			}
+
+			if (ectx->mip_shndx >= ectx->shdrs_cnt) {
+				PMD_DRV_LOG(ERR, "Invalid ELF NOTE shndx in section %zu.", idx);
+				return -EINVAL;
+			}
+
+			ectx->mip_sh_off = rte_le_to_cpu_32(descword[1]) |
+					(uint64_t)rte_le_to_cpu_32(descword[2]) << 32;
+			break;
+		default:
+			break;
+		}
+	} else if (strncmp(nname, NFP_ELT_NOTE_NAME_NFP_USER,
+			NFP_ELT_NOTE_NAME_NFP_USER_SZ) == 0 && ntype == NFP_ELF_NT_NFP_USER) {
+		ectx->user_note_cnt++;
+	}
+
+	return 0;
+}
+
+static int
+nfp_elf_parse_sht_note(struct nfp_elf_elf64_shdr *sec,
+		struct nfp_elf *ectx,
+		size_t idx,
+		uint8_t *buf8)
+{
+	int err;
+	size_t nsz;
+	uint8_t *desc;
+	uint32_t ntype;
+	uint32_t nnamesz;
+	uint32_t ndescsz;
+	const char *nname;
+	uint8_t *shdrs_data;
+	rte_le32_t *descword;
+	struct nfp_elf_elf64_nhdr *nhdr;
+	struct nfp_elf_user_note *unote;
+	uint64_t sh_size = rte_le_to_cpu_64(sec->sh_size);
+	uint64_t sh_offset = rte_le_to_cpu_64(sec->sh_offset);
+
+	if (!nfp_elf_check_sh_size(sh_size)) {
+		PMD_DRV_LOG(ERR, "Invalid ELF section header, index %zu.", idx);
+		return -EINVAL;
+	}
+
+	shdrs_data = buf8 + sh_offset;
+	ectx->shdrs_data[idx] = shdrs_data;
+	ectx->shdrs_host_endian[idx] = 0;
+
+	/* Extract notes that we recognise */
+	nhdr = (struct nfp_elf_elf64_nhdr *)shdrs_data;
+
+	while ((uint8_t *)nhdr < (shdrs_data + sh_size)) {
+		nnamesz  = rte_le_to_cpu_32(nhdr->n_namesz);
+		ndescsz  = rte_le_to_cpu_32(nhdr->n_descsz);
+		ntype    = rte_le_to_cpu_32(nhdr->n_type);
+		nname    = (const char *)((uint8_t *)nhdr + sizeof(*nhdr));
+		descword = (rte_le32_t *)((uint8_t *)nhdr + sizeof(*nhdr) +
+				((nnamesz + UINT32_C(3)) & ~UINT32_C(3)));
+
+		err = nfp_elf_parse_note_name_nfp(ectx, idx, ndescsz, ntype, nname, descword);
+		if (err != 0)
+			return err;
+
+		nhdr = (struct nfp_elf_elf64_nhdr *)((uint8_t *)descword +
+				((ndescsz + UINT32_C(3)) & ~UINT32_C(3)));
+	}
+
+	if (ectx->user_note_cnt == 0)
+		return 0;
+
+	ectx->user_notes = calloc(ectx->user_note_cnt, sizeof(*ectx->user_notes));
+	if (ectx->user_notes == NULL) {
+		PMD_DRV_LOG(ERR, "Out of memory.");
+		return -ENOMEM;
+	}
+
+	nhdr = (struct nfp_elf_elf64_nhdr *)shdrs_data;
+	unote = ectx->user_notes;
+	while ((uint8_t *)nhdr < (shdrs_data + sh_size)) {
+		nnamesz = rte_le_to_cpu_32(nhdr->n_namesz);
+		ndescsz = rte_le_to_cpu_32(nhdr->n_descsz);
+		ntype   = rte_le_to_cpu_32(nhdr->n_type);
+		nname   = (const char *)((uint8_t *)nhdr + sizeof(*nhdr));
+		desc    = (uint8_t *)nhdr + sizeof(*nhdr) +
+				((nnamesz + UINT32_C(3)) & ~UINT32_C(3));
+
+		if (strncmp(nname, NFP_ELT_NOTE_NAME_NFP_USER,
+				NFP_ELT_NOTE_NAME_NFP_USER_SZ) != 0)
+			continue;
+
+		if (ntype != NFP_ELF_NT_NFP_USER)
+			continue;
+
+		unote->name = (const char *)desc;
+		nsz = strlen(unote->name) + 1;
+		if (nsz % 4 != 0)
+			nsz = ((nsz / 4) + 1) * 4;
+		if (nsz > ndescsz) {
+			PMD_DRV_LOG(ERR, "Invalid ELF USER NOTE descsz in section %zu.", idx);
+			return -EINVAL;
+		}
+
+		unote->data_sz = ndescsz - (uint32_t)nsz;
+		if (unote->data_sz != 0)
+			unote->data = desc + nsz;
+		unote++;
+
+		nhdr = (struct nfp_elf_elf64_nhdr *)
+				(desc + ((ndescsz + UINT32_C(3)) & ~UINT32_C(3)));
+	}
+
+	return 0;
+}
+
+static int
+nfp_elf_parse_sht_meconfig(struct nfp_elf_elf64_shdr *sec,
+		struct nfp_elf *ectx,
+		size_t idx,
+		uint8_t *buf8)
+{
+	size_t ent_cnt;
+	uint8_t *shdrs_data;
+	uint64_t sh_size = rte_le_to_cpu_64(sec->sh_size);
+	uint64_t sh_offset = rte_le_to_cpu_64(sec->sh_offset);
+
+	if (!nfp_elf_check_sh_size(sh_size)) {
+		PMD_DRV_LOG(ERR, "Invalid ELF section header, index %zu.", idx);
+		return -EINVAL;
+	}
+
+	shdrs_data = buf8 + sh_offset;
+	ent_cnt = nfp_elf_get_sec_ent_cnt(ectx, idx);
+	ectx->shdrs_data[idx] = shdrs_data;
+	ectx->meconfs = (struct nfp_elf_elf_meconfig *)shdrs_data;
+	ectx->meconfs_cnt = ent_cnt;
+	ectx->shdrs_host_endian[idx] = 1;
+
+	return 0;
+}
+
+static int
+nfp_elf_parse_sht_initreg(struct nfp_elf_elf64_shdr *sec,
+		struct nfp_elf *ectx,
+		size_t idx,
+		uint8_t *buf8)
+{
+	uint64_t sh_size = rte_le_to_cpu_64(sec->sh_size);
+	uint64_t sh_offset = rte_le_to_cpu_64(sec->sh_offset);
+	uint64_t sh_entsize = rte_le_to_cpu_64(sec->sh_entsize);
+
+	if (!nfp_elf_arch_is_thornham(ectx)) {
+		PMD_DRV_LOG(ERR, "Section not supported for target arch.");
+		return -ENOTSUP;
+	}
+
+	if (sh_entsize != sizeof(struct nfp_elf_elf_initregentry) ||
+			!nfp_elf_check_sh_size(sh_size)) {
+		PMD_DRV_LOG(ERR, "Invalid ELF section header, index %zu.", idx);
+		return -EINVAL;
+	}
+
+	ectx->shdrs_data[idx] = buf8 + sh_offset;
+	ectx->shdrs_host_endian[idx] = 1;
+
+	return 0;
+}
+
+static int
+nfp_elf_parse_sht_symtab(struct nfp_elf_elf64_shdr *sec,
+		struct nfp_elf *ectx,
+		size_t idx,
+		uint8_t *buf8)
+{
+	uint64_t sh_size = rte_le_to_cpu_64(sec->sh_size);
+	uint64_t sh_offset = rte_le_to_cpu_64(sec->sh_offset);
+	uint64_t sh_entsize = rte_le_to_cpu_64(sec->sh_entsize);
+
+	if (sh_entsize != sizeof(struct nfp_elf_elf64_sym) ||
+			!nfp_elf_check_sh_size(sh_size)) {
+		PMD_DRV_LOG(ERR, "Invalid ELF section header, index %zu.", idx);
+		return -EINVAL;
+	}
+
+	ectx->shdrs_data[idx] = buf8 + sh_offset;
+	ectx->shdrs_host_endian[ectx->shdr_idx_symtab] = 1;
+
+	return 0;
+}
+
+static int
+nfp_elf_populate_fw_mip(struct nfp_elf *ectx,
+		uint8_t *buf8)
+{
+	uint8_t *pu8;
+	const char *nx;
+	uint64_t sh_size;
+	uint64_t sh_offset;
+	uint32_t first_entry;
+	const struct nfp_mip *mip;
+	struct nfp_elf_elf64_shdr *sec;
+	const struct nfp_mip_entry *ent;
+	const struct nfp_mip_fwinfo_entry *fwinfo;
+
+	sec = &ectx->shdrs[ectx->mip_shndx];
+	sh_size = rte_le_to_cpu_64(sec->sh_size);
+	sh_offset = rte_le_to_cpu_64(sec->sh_offset);
+	pu8 = buf8 + sh_offset + ectx->mip_sh_off;
+	mip = (const struct nfp_mip *)pu8;
+	first_entry = rte_le_to_cpu_32(mip->first_entry);
+
+	if (mip->signature != NFP_MIP_SIGNATURE) {
+		PMD_DRV_LOG(ERR, "Incorrect MIP signature %#08x",
+				rte_le_to_cpu_32(mip->signature));
+		return -EINVAL;
+	}
+
+	ectx->fw_mip.shndx = ectx->mip_shndx;
+	ectx->fw_mip.sh_offset = ectx->mip_sh_off;
+	ectx->fw_mip.mip_ver = mip->mip_version;
+
+	if (ectx->fw_mip.mip_ver != NFP_MIP_VERSION) {
+		PMD_DRV_LOG(ERR, "MIP note pointer does not point to recognised version.");
+		return -EINVAL;
+	}
+
+	ectx->fw_mip.fw_version   = mip->version;
+	ectx->fw_mip.fw_buildnum  = mip->buildnum;
+	ectx->fw_mip.fw_buildtime = mip->buildtime;
+	strncpy(ectx->fw_mip.fw_name, mip->name, 16);
+
+	/*
+	 * If there is a FWINFO v1 entry, it will be first and
+	 * right after the MIP itself, so in the same section.
+	 */
+	if (ectx->mip_sh_off + first_entry + sizeof(*ent) < sh_size) {
+		pu8 += first_entry;
+		ent = (const struct nfp_mip_entry *)pu8;
+		if (ent->type == NFP_MIP_TYPE_FWINFO && ent->version == 1) {
+			pu8 += sizeof(*ent);
+			fwinfo = (const struct nfp_mip_fwinfo_entry *)pu8;
+			if (fwinfo->kv_len != 0) {
+				ectx->fw_info_strtab_sz = fwinfo->kv_len;
+				ectx->fw_info_strtab = fwinfo->key_value_strs;
+			}
+		}
+	}
+
+	ectx->fw_mip.fw_typeid = nfp_elf_fwinfo_lookup(ectx, "TypeId");
+
+	/*
+	 * TypeId will be the last reserved key-value pair, so skip
+	 * to the first entry after it for the user values.
+	 */
+	if (ectx->fw_mip.fw_typeid == NULL)
+		return 0;
+
+	nx = nfp_elf_fwinfo_next(ectx, ectx->fw_mip.fw_typeid);
+	if (nx == NULL)
+		ectx->fw_info_strtab_sz = 0;
+	else
+		ectx->fw_info_strtab_sz -= (nx - ectx->fw_info_strtab);
+	ectx->fw_info_strtab = nx;
+
+	return 0;
+}
+
+static int
+nfp_elf_read_flie_headers(struct nfp_elf *ectx,
+		void *buf)
+{
+	uint16_t e_type;
+	uint32_t e_flags;
+	uint32_t e_version;
+	uint16_t e_machine;
+
+	ectx->ehdr = buf;
+	e_type = rte_le_to_cpu_16(ectx->ehdr->e_type);
+	e_flags = rte_le_to_cpu_32(ectx->ehdr->e_flags);
+	e_version = rte_le_to_cpu_32(ectx->ehdr->e_version);
+	e_machine = rte_le_to_cpu_16(ectx->ehdr->e_machine);
+
+	switch (e_machine) {
+	case NFP_ELF_EM_NFP:
+		ectx->family = (e_flags >> NFP_ELF_EF_NFP_FAMILY_LSB)
+				& NFP_ELF_EF_NFP_FAMILY_MASK;
+		break;
+	case NFP_ELF_EM_NFP6000:
+		ectx->family = NFP_CHIP_FAMILY_NFP6000;
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "Invalid ELF machine type.");
+		return -EINVAL;
+	}
+
+	if ((e_type != NFP_ELF_ET_EXEC && e_type != NFP_ELF_ET_REL &&
+			e_type != NFP_ELF_ET_NFP_PARTIAL_EXEC &&
+			e_type != NFP_ELF_ET_NFP_PARTIAL_REL) ||
+			e_version != NFP_ELF_EV_CURRENT ||
+			ectx->ehdr->e_ehsize != sizeof(struct nfp_elf_elf64_ehdr) ||
+			ectx->ehdr->e_shentsize != sizeof(struct nfp_elf_elf64_shdr)) {
+		PMD_DRV_LOG(ERR, "Invalid ELF file header.");
+		return -EINVAL;
+	}
+
+	if (ectx->ehdr->e_shoff < ectx->ehdr->e_ehsize) {
+		PMD_DRV_LOG(ERR, "Invalid ELF header content.");
+		return -EINVAL;
+	}
+
+	if (ectx->ehdr->e_shstrndx >= ectx->ehdr->e_shnum) {
+		PMD_DRV_LOG(ERR, "Invalid ELF header content.");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+nfp_elf_read_section_headers(struct nfp_elf *ectx,
+		uint8_t *buf8,
+		size_t buf_len)
+{
+	size_t idx;
+	int err = 0;
+	uint8_t *pu8;
+	uint64_t sh_size;
+	uint64_t sh_offset;
+	uint64_t sh_entsize;
+	struct nfp_elf_elf64_shdr *sec;
+	uint64_t e_shoff = rte_le_to_cpu_16(ectx->ehdr->e_shoff);
+	uint16_t e_shnum = rte_le_to_cpu_16(ectx->ehdr->e_shnum);
+
+	if (buf_len < e_shoff + ((size_t)e_shnum * sizeof(*sec))) {
+		PMD_DRV_LOG(ERR, "ELF data too short.");
+		return -EINVAL;
+	}
+
+	pu8 = buf8 + e_shoff;
+
+	if (e_shnum == 0) {
+		ectx->shdrs = NULL;
+		ectx->shdrs_data = NULL;
+		ectx->shdrs_host_endian = NULL;
+		ectx->shdrs_cnt = 0;
+		return 0;
+	}
+
+	ectx->shdrs = calloc(e_shnum, sizeof(*ectx->shdrs));
+	if (ectx->shdrs == NULL) {
+		PMD_DRV_LOG(ERR, "Out of memory.");
+		return -ENOMEM;
+	}
+
+	ectx->shdrs_data = calloc(e_shnum, sizeof(void *));
+	if (ectx->shdrs_data == NULL) {
+		PMD_DRV_LOG(ERR, "Out of memory.");
+		err = -ENOMEM;
+		goto free_shdrs;
+	}
+
+	ectx->shdrs_host_endian = calloc(e_shnum, sizeof(ectx->shdrs_host_endian[0]));
+	if (ectx->shdrs_host_endian == NULL) {
+		PMD_DRV_LOG(ERR, "Out of memory.");
+		err = -ENOMEM;
+		goto free_shdrs_data;
+	}
+
+	memcpy(ectx->shdrs, pu8, e_shnum * sizeof(*ectx->shdrs));
+	ectx->shdrs_cnt = e_shnum;
+
+	for (idx = 0, sec = ectx->shdrs; idx < ectx->shdrs_cnt; idx++, sec++) {
+		sh_size = rte_le_to_cpu_64(sec->sh_size);
+		sh_offset = rte_le_to_cpu_64(sec->sh_offset);
+		sh_entsize = rte_le_to_cpu_64(sec->sh_entsize);
+
+		if (sh_entsize != 0 && (sh_size % sh_entsize != 0)) {
+			PMD_DRV_LOG(ERR, "Invalid ELF section header, index %zu.", idx);
+			err = -EINVAL;
+			goto free_shdrs_host_endian;
+		}
+
+		switch (rte_le_to_cpu_32(sec->sh_type)) {
+		case NFP_ELF_SHT_REL:
+			err = nfp_elf_parse_sht_rel(sec, ectx, idx, buf8);
+			if (err != 0) {
+				PMD_DRV_LOG(ERR, "Failed to parse sht rel.");
+				goto free_shdrs_host_endian;
+			}
+			break;
+		case NFP_ELF_SHT_NOTE:
+			err = nfp_elf_parse_sht_note(sec, ectx, idx, buf8);
+			if (err != 0) {
+				PMD_DRV_LOG(ERR, "Failed to parse sht note.");
+				goto free_shdrs_host_endian;
+			}
+			break;
+		case NFP_ELF_SHT_NFP_MECONFIG:
+			err = nfp_elf_parse_sht_meconfig(sec, ectx, idx, buf8);
+			if (err != 0) {
+				PMD_DRV_LOG(ERR, "Failed to parse sht meconfig.");
+				goto free_shdrs_host_endian;
+			}
+			break;
+		case NFP_ELF_SHT_NFP_INITREG:
+			err = nfp_elf_parse_sht_initreg(sec, ectx, idx, buf8);
+			if (err != 0) {
+				PMD_DRV_LOG(ERR, "Failed to parse sht initregp.");
+				goto free_shdrs_host_endian;
+			}
+			break;
+		case NFP_ELF_SHT_SYMTAB:
+			err = nfp_elf_parse_sht_symtab(sec, ectx, idx, buf8);
+			if (err != 0) {
+				PMD_DRV_LOG(ERR, "Failed to parse sht symtab.");
+				goto free_shdrs_host_endian;
+			}
+			break;
+		case NFP_ELF_SHT_NOBITS:
+		case NFP_ELF_SHT_NULL:
+			break;
+		default:
+			if (sh_offset > 0 && sh_size <= 0)
+				break;
+
+			/*
+			 * Limit sections to 4GiB, because they won't need to be this large
+			 * and this ensures we can handle the file on 32-bit hosts without
+			 * unexpected problems.
+			 */
+			if (sh_size > UINT32_MAX) {
+				PMD_DRV_LOG(ERR, "Invalid ELF section header, index %zu.", idx);
+				err = -EINVAL;
+				goto free_shdrs_host_endian;
+			}
+
+			pu8 = buf8 + sh_offset;
+			ectx->shdrs_data[idx] = pu8;
+			ectx->shdrs_host_endian[idx] = 0;
+			break;
+		}
+	}
+
+	return 0;
+
+free_shdrs_host_endian:
+	free(ectx->shdrs_host_endian);
+free_shdrs_data:
+	free(ectx->shdrs_data);
+free_shdrs:
+	free(ectx->shdrs);
+
+	return err;
+}
+
+static int
+nfp_elf_read_shstrtab(struct nfp_elf *ectx)
+{
+	struct nfp_elf_elf64_shdr *sec;
+	uint16_t e_shstrndx = rte_le_to_cpu_16(ectx->ehdr->e_shstrndx);
+
+	if (ectx->ehdr->e_shnum <= ectx->ehdr->e_shstrndx) {
+		PMD_DRV_LOG(ERR, "Invalid Index.");
+		return -EINVAL;
+	}
+
+	sec = &ectx->shdrs[e_shstrndx];
+	if (sec == NULL || rte_le_to_cpu_32(sec->sh_type) != NFP_ELF_SHT_STRTAB) {
+		PMD_DRV_LOG(ERR, "Invalid ELF shstrtab.");
+		return -EINVAL;
+	}
+
+	ectx->shstrtab = ectx->shdrs_data[e_shstrndx];
+	ectx->shstrtab_sz = rte_le_to_cpu_64(sec->sh_size);
+
+	return 0;
+}
+
+static int
+nfp_elf_read_first_symtab(struct nfp_elf *ectx)
+{
+	size_t idx;
+	uint32_t sh_type;
+	uint64_t sh_size;
+	struct nfp_elf_elf64_shdr *sec;
+
+	for (idx = 0, sec = ectx->shdrs; idx < ectx->shdrs_cnt; idx++, sec++) {
+		if (sec != NULL) {
+			sh_type = rte_le_to_cpu_32(sec->sh_type);
+			if (sh_type == NFP_ELF_SHT_SYMTAB)
+				break;
+		}
+	}
+
+	sh_size = rte_le_to_cpu_64(sec->sh_size);
+
+	if (idx < ectx->shdrs_cnt && sh_type == NFP_ELF_SHT_SYMTAB) {
+		ectx->shdr_idx_symtab = idx;
+		ectx->syms = ectx->shdrs_data[idx];
+		ectx->syms_cnt = nfp_elf_get_sec_ent_cnt(ectx, idx);
+
+		/* Load symtab's strtab */
+		idx = rte_le_to_cpu_32(sec->sh_link);
+
+		if (idx == NFP_ELF_SHN_UNDEF || idx >= ectx->shdrs_cnt) {
+			PMD_DRV_LOG(ERR, "ELF symtab has no strtab.");
+			return -EINVAL;
+		}
+
+		sec = &ectx->shdrs[idx];
+		sh_type = rte_le_to_cpu_32(sec->sh_type);
+		if (sh_type != NFP_ELF_SHT_STRTAB) {
+			PMD_DRV_LOG(ERR, "ELF symtab has no strtab.");
+			return -EINVAL;
+		}
+
+		if (!nfp_elf_check_sh_size(sh_size)) {
+			PMD_DRV_LOG(ERR, "ELF symtab has invalid strtab.");
+			return -EINVAL;
+		}
+
+		ectx->symstrtab = ectx->shdrs_data[idx];
+		ectx->symstrtab_sz = sh_size;
+	}
+
+	return 0;
+}
+
+static int
+nfp_elf_is_valid_file(uint8_t *buf8)
+{
+	if (buf8[NFP_ELF_EI_MAG0] != NFP_ELF_ELFMAG0 ||
+			buf8[NFP_ELF_EI_MAG1] != NFP_ELF_ELFMAG1 ||
+			buf8[NFP_ELF_EI_MAG2] != NFP_ELF_ELFMAG2 ||
+			buf8[NFP_ELF_EI_MAG3] != NFP_ELF_ELFMAG3 ||
+			buf8[NFP_ELF_EI_VERSION] != NFP_ELF_EV_CURRENT ||
+			buf8[NFP_ELF_EI_DATA] != NFP_ELF_ELFDATA2LSB)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int
+nfp_elf_is_valid_class(uint8_t *buf8)
+{
+	if (buf8[NFP_ELF_EI_CLASS] != NFP_ELF_ELFCLASS64)
+		return -EINVAL;
+
+	return 0;
+}
+
+static struct nfp_elf *
+nfp_elf_mutable_buf(void *buf,
+		size_t buf_len)
+{
+	int err = 0;
+	uint8_t *buf8 = buf;
+	struct nfp_elf *ectx;
+
+	if (buf == NULL) {
+		PMD_DRV_LOG(ERR, "Invalid parameters.");
+		return NULL;
+	}
+
+	if (buf_len < sizeof(struct nfp_elf_elf64_ehdr)) {
+		PMD_DRV_LOG(ERR, "ELF data too short.");
+		return NULL;
+	}
+
+	ectx = calloc(1, sizeof(struct nfp_elf));
+	if (ectx == NULL) {
+		PMD_DRV_LOG(ERR, "Out of memory.");
+		return NULL;
+	}
+
+	ectx->rev_min = -1;
+	ectx->rev_max = -1;
+	ectx->mip_sh_off = UINT64_MAX;
+
+	err = nfp_elf_is_valid_file(buf8);
+	if (err != 0) {
+		PMD_DRV_LOG(ERR, "Not a valid ELF file.");
+		goto elf_free;
+	}
+
+	err = nfp_elf_is_valid_class(buf8);
+	if (err != 0) {
+		PMD_DRV_LOG(ERR, "Unknown ELF class.");
+		goto elf_free;
+	}
+
+	err = nfp_elf_read_flie_headers(ectx, buf);
+	if (err != 0) {
+		PMD_DRV_LOG(ERR, "Failed to read file headers.");
+		goto elf_free;
+	}
+
+	err = nfp_elf_read_section_headers(ectx, buf8, buf_len);
+	if (err != 0) {
+		PMD_DRV_LOG(ERR, "Failed to read section headers.");
+		goto elf_free;
+	}
+
+	err = nfp_elf_read_shstrtab(ectx);
+	if (err != 0) {
+		PMD_DRV_LOG(ERR, "Failed to read shstrtab.");
+		goto elf_free;
+	}
+
+	/* Read first symtab if any, assuming it's the primary or only one */
+	err = nfp_elf_read_first_symtab(ectx);
+	if (err != 0) {
+		PMD_DRV_LOG(ERR, "Failed to read first symtab.");
+		goto elf_free;
+	}
+
+	/* Populate the fw_mip struct if we have a .note for it */
+	if (ectx->mip_shndx != 0) {
+		err = nfp_elf_populate_fw_mip(ectx, buf8);
+		if (err != 0) {
+			PMD_DRV_LOG(ERR, "Failed to populate the fw mip.");
+			goto elf_free;
+		}
+	}
+
+	ectx->_buf = buf;
+	ectx->_bufsz = 0;
+
+	return ectx;
+
+elf_free:
+	nfp_elf_free(ectx);
+
+	return NULL;
+}
+
+int
+nfp_elf_get_fw_buildtime(uint32_t *fw_buildtime,
+		char *fw_name)
+{
+	void *fw_buf;
+	size_t fsize;
+	struct nfp_elf *elf;
+
+	if (rte_firmware_read(fw_name, &fw_buf, &fsize) != 0) {
+		PMD_DRV_LOG(ERR, "firmware %s not found!", fw_name);
+		return -ENOENT;
+	}
+
+	elf = nfp_elf_mutable_buf(fw_buf, fsize);
+	if (elf == NULL) {
+		PMD_DRV_LOG(ERR, "Parse nffw file failed.");
+		free(fw_buf);
+		return -EIO;
+	}
+
+	*fw_buildtime = elf->fw_mip.fw_buildtime;
+
+	nfp_elf_free(elf);
+	free(fw_buf);
+	return 0;
+}
diff --git a/drivers/net/nfp/nfpcore/nfp_elf.h b/drivers/net/nfp/nfpcore/nfp_elf.h
new file mode 100644
index 0000000000..5b1099fb70
--- /dev/null
+++ b/drivers/net/nfp/nfpcore/nfp_elf.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright (c) 2023 Corigine, Inc.
+ * All rights reserved.
+ */
+
+#ifndef __NFP_ELF_H__
+#define __NFP_ELF_H__
+
+#include <stdint.h>
+
+int nfp_elf_get_fw_buildtime(uint32_t *fw_buildtime, char *fw_name);
+
+#endif /* __NFP_ELF_H__ */
diff --git a/drivers/net/nfp/nfpcore/nfp_mip.c b/drivers/net/nfp/nfpcore/nfp_mip.c
index d5ada3687a..c6a25b6387 100644
--- a/drivers/net/nfp/nfpcore/nfp_mip.c
+++ b/drivers/net/nfp/nfpcore/nfp_mip.c
@@ -10,30 +10,6 @@
 #include "nfp_logs.h"
 #include "nfp_nffw.h"
 
-#define NFP_MIP_SIGNATURE        rte_cpu_to_le_32(0x0050494d)  /* "MIP\0" */
-#define NFP_MIP_VERSION          rte_cpu_to_le_32(1)
-#define NFP_MIP_MAX_OFFSET       (256 * 1024)
-
-struct nfp_mip {
-	uint32_t signature;
-	uint32_t mip_version;
-	uint32_t mip_size;
-	uint32_t first_entry;
-
-	uint32_t version;
-	uint32_t buildnum;
-	uint32_t buildtime;
-	uint32_t loadtime;
-
-	uint32_t symtab_addr;
-	uint32_t symtab_size;
-	uint32_t strtab_addr;
-	uint32_t strtab_size;
-
-	char name[16];
-	char toolchain[32];
-};
-
 /* Read memory and check if it could be a valid MIP */
 static int
 nfp_mip_try_read(struct nfp_cpp *cpp,
@@ -134,6 +110,12 @@ nfp_mip_name(const struct nfp_mip *mip)
 	return mip->name;
 }
 
+uint32_t
+nfp_mip_buildtime(const struct nfp_mip *mip)
+{
+	return mip->buildtime;
+}
+
 /**
  * Get the address and size of the MIP symbol table.
  *
diff --git a/drivers/net/nfp/nfpcore/nfp_mip.h b/drivers/net/nfp/nfpcore/nfp_mip.h
index dbd9af31ed..a67afdc1ea 100644
--- a/drivers/net/nfp/nfpcore/nfp_mip.h
+++ b/drivers/net/nfp/nfpcore/nfp_mip.h
@@ -8,12 +8,80 @@
 
 #include "nfp_cpp.h"
 
-struct nfp_mip;
+/* "MIP\0" */
+#define NFP_MIP_SIGNATURE        rte_le_to_cpu_32(0x0050494d)
+#define NFP_MIP_VERSION          rte_le_to_cpu_32(1)
+
+/* nfp_mip_entry_type */
+#define NFP_MIP_TYPE_FWINFO      0x70000002
+
+/* Each packed struct field is stored as Little Endian */
+struct nfp_mip {
+	rte_le32_t signature;
+	rte_le32_t mip_version;
+
+	rte_le32_t mip_size;
+	rte_le32_t first_entry;
+
+	rte_le32_t version;
+	rte_le32_t buildnum;
+	rte_le32_t buildtime;
+	rte_le32_t loadtime;
+
+	rte_le32_t symtab_addr;
+	rte_le32_t symtab_size;
+	rte_le32_t strtab_addr;
+	rte_le32_t strtab_size;
+
+	char name[16];
+	char toolchain[32];
+};
+
+struct nfp_mip_entry {
+	uint32_t type;
+	uint32_t version;
+	uint32_t offset_next;
+};
+
+/*
+ * A key-value pair has no imposed limit, but it is recommended that
+ * consumers only allocate enough memory for keys they plan to process and
+ * skip over unused keys or ignore values that are longer than expected.
+ *
+ * For MIPv1, this will be preceded by struct nfp_mip_entry.
+ * The entry size will be the size of key_value_strs, round to the next
+ * 4-byte multiple. If entry size is 0, then there are no key-value strings
+ * and it will not contain an empty string.
+ *
+ * The following keys are reserved and possibly set by the linker. The
+ * convention is to start linker-set keys with a capital letter. Reserved
+ * entries will be placed first in key_value_strs, user entries will be
+ * placed next and be sorted alphabetically.
+ * TypeId - Present if a user specified fw_typeid when linking.
+ *
+ * The following keys are reserved, but not used. Their values are in the
+ * root MIP struct.
+ */
+struct nfp_mip_fwinfo_entry {
+	/** The byte size of @p key_value_strs. */
+	uint32_t kv_len;
+
+	/** The number of key-value pairs in the following string. */
+	uint32_t num;
+
+	/**
+	 * A series of NUL terminated strings, terminated by an extra
+	 * NUL which is also the last byte of the entry, so an iterator
+	 * can either check on size or when key[0] == '\0'.
+	 */
+	char key_value_strs[];
+};
 
 struct nfp_mip *nfp_mip_open(struct nfp_cpp *cpp);
 void nfp_mip_close(struct nfp_mip *mip);
 
 const char *nfp_mip_name(const struct nfp_mip *mip);
+uint32_t nfp_mip_buildtime(const struct nfp_mip *mip);
 void nfp_mip_symtab(const struct nfp_mip *mip, uint32_t *addr, uint32_t *size);
 void nfp_mip_strtab(const struct nfp_mip *mip, uint32_t *addr, uint32_t *size);
 
-- 
2.39.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 7/8] net/nfp: reload the firmware only when firmware changed
  2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
                   ` (5 preceding siblings ...)
  2024-01-15  2:54 ` [PATCH 6/8] net/nfp: add the elf module Chaoyong He
@ 2024-01-15  2:54 ` Chaoyong He
  2024-01-15  2:54 ` [PATCH 8/8] net/nfp: simplify the port name for multiple PFs Chaoyong He
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Chaoyong He @ 2024-01-15  2:54 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Peng Zhang, Chaoyong He, Long Wu

From: Peng Zhang <peng.zhang@corigine.com>

Add the interfaces of getting firmware build time from BSP
and ELF file, only reloading the firmware when the build
time is different, which means the firmware has changed.

This will accelerate the average startup time for both
multi-PF and single-PF firmware.

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c     | 86 ++++++++++++++++++++++++++++----
 drivers/net/nfp/nfp_net_common.c | 17 +++++++
 drivers/net/nfp/nfp_net_common.h |  2 +
 3 files changed, 94 insertions(+), 11 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 46bb09a211..22edf11253 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -19,6 +19,7 @@
 #include "nfpcore/nfp_nsp.h"
 #include "nfpcore/nfp6000_pcie.h"
 #include "nfpcore/nfp_resource.h"
+#include "nfpcore/nfp_elf.h"
 
 #include "nfp_cpp_bridge.h"
 #include "nfp_ipsec.h"
@@ -1067,6 +1068,36 @@ nfp_fw_unload(struct nfp_cpp *cpp)
 	nfp_nsp_close(nsp);
 }
 
+/* 0 is firmware not change, 1 is firmware changed, < 0 has error */
+static int
+nfp_fw_check_change(struct nfp_cpp *cpp,
+		char *fw_name,
+		bool *fw_changed)
+{
+	int ret;
+	struct nfp_net_hw hw;
+	uint32_t new_buildtime = 0;
+	uint32_t old_buildtime = 0;
+
+	ret = nfp_elf_get_fw_buildtime(&new_buildtime, fw_name);
+	if (ret < 0)
+		return ret;
+
+	hw.cpp = cpp;
+	nfp_net_get_fw_buildtime(&hw, &old_buildtime);
+	if (new_buildtime != old_buildtime) {
+		PMD_DRV_LOG(INFO, "FW version is changed, new %u, old %u",
+				new_buildtime, old_buildtime);
+		*fw_changed = true;
+	} else {
+		PMD_DRV_LOG(INFO, "FW version is not changed, build_time is %u.",
+				new_buildtime);
+		*fw_changed = false;
+	}
+
+	return 0;
+}
+
 static int
 nfp_fw_reload(struct nfp_nsp *nsp,
 		char *fw_name)
@@ -1132,15 +1163,39 @@ nfp_fw_skip_load(const struct nfp_dev_info *dev_info,
 
 	return false;
 }
+static int
+nfp_fw_reload_for_single_pf(struct nfp_nsp *nsp,
+		char *fw_name,
+		struct nfp_cpp *cpp)
+{
+	int err;
+	bool fw_changed = true;
+
+	if (nfp_nsp_fw_loaded(nsp)) {
+		err = nfp_fw_check_change(cpp, fw_name, &fw_changed);
+		if (err < 0)
+			return err;
+	}
+
+	if (!fw_changed)
+		return 0;
+
+	err = nfp_fw_reload(nsp, fw_name);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
 
 static int
-nfp_fw_reload_for_multipf(struct nfp_nsp *nsp,
+nfp_fw_reload_for_multi_pf(struct nfp_nsp *nsp,
 		char *fw_name,
 		struct nfp_cpp *cpp,
 		const struct nfp_dev_info *dev_info,
 		struct nfp_multi_pf *multi_pf)
 {
 	int err;
+	bool fw_changed = true;
 	bool skip_load_fw = false;
 
 	err = nfp_net_keepalive_init(cpp, multi_pf);
@@ -1151,27 +1206,36 @@ nfp_fw_reload_for_multipf(struct nfp_nsp *nsp,
 
 	err = nfp_net_keepalive_start(multi_pf);
 	if (err != 0) {
-		nfp_net_keepalive_uninit(multi_pf);
 		PMD_DRV_LOG(ERR, "NFP write beat failed");
-		return err;
+		goto keepalive_uninit;
 	}
 
-	if (nfp_nsp_fw_loaded(nsp))
+	if (nfp_nsp_fw_loaded(nsp)) {
+		err = nfp_fw_check_change(cpp, fw_name, &fw_changed);
+		if (err < 0)
+			goto keepalive_stop;
+	}
+
+	if (!fw_changed)
 		skip_load_fw = nfp_fw_skip_load(dev_info, multi_pf);
 
 	if (skip_load_fw)
 		return 0;
 
 	err = nfp_fw_reload(nsp, fw_name);
-	if (err != 0) {
-		nfp_net_keepalive_stop(multi_pf);
-		nfp_net_keepalive_uninit(multi_pf);
-		return err;
-	}
+	if (err != 0)
+		goto keepalive_stop;
 
 	nfp_net_keepalive_clear_others(dev_info, multi_pf);
 
 	return 0;
+
+keepalive_stop:
+	nfp_net_keepalive_stop(multi_pf);
+keepalive_uninit:
+	nfp_net_keepalive_uninit(multi_pf);
+
+	return err;
 }
 
 static int
@@ -1228,9 +1292,9 @@ nfp_fw_setup(struct rte_pci_device *dev,
 	}
 
 	if (multi_pf->enabled)
-		err = nfp_fw_reload_for_multipf(nsp, fw_name, cpp, dev_info, multi_pf);
+		err = nfp_fw_reload_for_multi_pf(nsp, fw_name, cpp, dev_info, multi_pf);
 	else
-		err = nfp_fw_reload(nsp, fw_name);
+		err = nfp_fw_reload_for_single_pf(nsp, fw_name, cpp);
 
 	nfp_nsp_close(nsp);
 	return err;
diff --git a/drivers/net/nfp/nfp_net_common.c b/drivers/net/nfp/nfp_net_common.c
index a438eb5871..7b8c175a36 100644
--- a/drivers/net/nfp/nfp_net_common.c
+++ b/drivers/net/nfp/nfp_net_common.c
@@ -2090,6 +2090,23 @@ nfp_net_get_nsp_info(struct nfp_net_hw *hw,
 	nfp_nsp_close(nsp);
 }
 
+void
+nfp_net_get_fw_buildtime(struct nfp_net_hw *hw,
+		uint32_t *mip_buildtime)
+{
+	struct nfp_mip *mip;
+
+	mip = nfp_mip_open(hw->cpp);
+	if (mip == NULL) {
+		*mip_buildtime = 0;
+		return;
+	}
+
+	*mip_buildtime = nfp_mip_buildtime(mip);
+
+	nfp_mip_close(mip);
+}
+
 static void
 nfp_net_get_mip_name(struct nfp_net_hw *hw,
 		char *mip_name)
diff --git a/drivers/net/nfp/nfp_net_common.h b/drivers/net/nfp/nfp_net_common.h
index 66c900e3b8..69d17cae17 100644
--- a/drivers/net/nfp/nfp_net_common.h
+++ b/drivers/net/nfp/nfp_net_common.h
@@ -295,6 +295,8 @@ int nfp_net_fec_get(struct rte_eth_dev *dev,
 		uint32_t *fec_capa);
 int nfp_net_fec_set(struct rte_eth_dev *dev,
 		uint32_t fec_capa);
+void nfp_net_get_fw_buildtime(struct nfp_net_hw *hw,
+		uint32_t *fw_buildtimes);
 
 #define NFP_PRIV_TO_APP_FW_NIC(app_fw_priv)\
 	((struct nfp_app_fw_nic *)app_fw_priv)
-- 
2.39.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 8/8] net/nfp: simplify the port name for multiple PFs
  2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
                   ` (6 preceding siblings ...)
  2024-01-15  2:54 ` [PATCH 7/8] net/nfp: reload the firmware only when firmware changed Chaoyong He
@ 2024-01-15  2:54 ` Chaoyong He
  2024-01-22 15:08 ` [PATCH 0/8] optimize the firmware loading process Ferruh Yigit
  2024-01-23 15:18 ` Ferruh Yigit
  9 siblings, 0 replies; 16+ messages in thread
From: Chaoyong He @ 2024-01-15  2:54 UTC (permalink / raw)
  To: dev; +Cc: oss-drivers, Peng Zhang, Chaoyong He, Long Wu

From: Peng Zhang <peng.zhang@corigine.com>

For the firmware which does not support multiple PF feature,
there are multiple PFs corresponding to a single PCI BDF address,
so we have to distinguish them by using a '_port*' postfix for
the device name.

For the firmware which supports multiple PFs feature, there is
only one PF corresponding to a PCI BDF address, so actually we
does not need this postfix anymore, and which is in line with
the mainstream.

Add the corresponding logic to simplify the port name for multiple
PFs case, which will make the user happy.

Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Long Wu <long.wu@corigine.com>
---
 drivers/net/nfp/nfp_ethdev.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 22edf11253..31b2302db6 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -1451,9 +1451,12 @@ nfp_init_app_fw_nic(struct nfp_pf_dev *pf_dev,
 	/* Loop through all physical ports on PF */
 	numa_node = rte_socket_id();
 	for (i = 0; i < app_fw_nic->total_phyports; i++) {
-		id = nfp_function_id_get(pf_dev, i);
-		snprintf(port_name, sizeof(port_name), "%s_port%u",
-				pf_dev->pci_dev->device.name, id);
+		if (pf_dev->multi_pf.enabled)
+			snprintf(port_name, sizeof(port_name), "%s",
+					pf_dev->pci_dev->device.name);
+		else
+			snprintf(port_name, sizeof(port_name), "%s_port%u",
+					pf_dev->pci_dev->device.name, i);
 
 		/* Allocate a eth_dev for this phyport */
 		eth_dev = rte_eth_dev_allocate(port_name);
@@ -1473,6 +1476,7 @@ nfp_init_app_fw_nic(struct nfp_pf_dev *pf_dev,
 		}
 
 		hw = eth_dev->data->dev_private;
+		id = nfp_function_id_get(pf_dev, i);
 
 		/* Add this device to the PF's array of physical ports */
 		app_fw_nic->ports[id] = hw;
@@ -1902,14 +1906,15 @@ nfp_secondary_init_app_fw_nic(struct nfp_pf_dev *pf_dev)
 	}
 
 	for (i = 0; i < total_vnics; i++) {
-		uint32_t id = i;
 		struct rte_eth_dev *eth_dev;
 		char port_name[RTE_ETH_NAME_MAX_LEN];
 
 		if (nfp_check_multi_pf_from_fw(total_vnics))
-			id = function_id;
-		snprintf(port_name, sizeof(port_name), "%s_port%u",
-				pf_dev->pci_dev->device.name, id);
+			snprintf(port_name, sizeof(port_name), "%s",
+					pf_dev->pci_dev->device.name);
+		else
+			snprintf(port_name, sizeof(port_name), "%s_port%u",
+					pf_dev->pci_dev->device.name, i);
 
 		PMD_INIT_LOG(DEBUG, "Secondary attaching to port %s", port_name);
 		eth_dev = rte_eth_dev_attach_secondary(port_name);
-- 
2.39.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/8] optimize the firmware loading process
  2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
                   ` (7 preceding siblings ...)
  2024-01-15  2:54 ` [PATCH 8/8] net/nfp: simplify the port name for multiple PFs Chaoyong He
@ 2024-01-22 15:08 ` Ferruh Yigit
  2024-01-23  1:46   ` Chaoyong He
  2024-01-23 15:18 ` Ferruh Yigit
  9 siblings, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2024-01-22 15:08 UTC (permalink / raw)
  To: Chaoyong He, dev; +Cc: oss-drivers

On 1/15/2024 2:54 AM, Chaoyong He wrote:
> This patch series aims to speedup the DPDK application start by optimize
> the firmware loading process in sereval places.
> We also simplify the port name in multiple PF firmware case to make the
> customer happy.
> 

<...>

>   net/nfp: add the elf module
>   net/nfp: reload the firmware only when firmware changed

Above commit adds elf parser capability and second one loads firmware
when build time is different.

I can see this is an optimization effort, to understand FW status before
loading FW, but relying on build time seems fragile. Does it help to add
a new section to store version information and evaluate based on this
information?



^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 0/8] optimize the firmware loading process
  2024-01-22 15:08 ` [PATCH 0/8] optimize the firmware loading process Ferruh Yigit
@ 2024-01-23  1:46   ` Chaoyong He
  2024-01-23  9:14     ` Ferruh Yigit
  0 siblings, 1 reply; 16+ messages in thread
From: Chaoyong He @ 2024-01-23  1:46 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: oss-drivers



> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Monday, January 22, 2024 11:09 PM
> To: Chaoyong He <chaoyong.he@corigine.com>; dev@dpdk.org
> Cc: oss-drivers <oss-drivers@corigine.com>
> Subject: Re: [PATCH 0/8] optimize the firmware loading process
> 
> On 1/15/2024 2:54 AM, Chaoyong He wrote:
> > This patch series aims to speedup the DPDK application start by
> > optimize the firmware loading process in sereval places.
> > We also simplify the port name in multiple PF firmware case to make
> > the customer happy.
> >
> 
> <...>
> 
> >   net/nfp: add the elf module
> >   net/nfp: reload the firmware only when firmware changed
> 
> Above commit adds elf parser capability and second one loads firmware when
> build time is different.
> 
> I can see this is an optimization effort, to understand FW status before loading
> FW, but relying on build time seems fragile. Does it help to add a new section
> to store version information and evaluate based on this information?
> 

We have a branch of firmware (several app type combined with NFD3/NFDk) with the same version information(monthly publish),
so the version information can't help us, because we can't distinguish among them.

But the build time is different for every firmware, and that's the reason we choose it.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/8] optimize the firmware loading process
  2024-01-23  1:46   ` Chaoyong He
@ 2024-01-23  9:14     ` Ferruh Yigit
  2024-01-23 11:27       ` Chaoyong He
  0 siblings, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2024-01-23  9:14 UTC (permalink / raw)
  To: Chaoyong He, dev; +Cc: oss-drivers

On 1/23/2024 1:46 AM, Chaoyong He wrote:
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Monday, January 22, 2024 11:09 PM
>> To: Chaoyong He <chaoyong.he@corigine.com>; dev@dpdk.org
>> Cc: oss-drivers <oss-drivers@corigine.com>
>> Subject: Re: [PATCH 0/8] optimize the firmware loading process
>>
>> On 1/15/2024 2:54 AM, Chaoyong He wrote:
>>> This patch series aims to speedup the DPDK application start by
>>> optimize the firmware loading process in sereval places.
>>> We also simplify the port name in multiple PF firmware case to make
>>> the customer happy.
>>>
>>
>> <...>
>>
>>>   net/nfp: add the elf module
>>>   net/nfp: reload the firmware only when firmware changed
>>
>> Above commit adds elf parser capability and second one loads firmware when
>> build time is different.
>>
>> I can see this is an optimization effort, to understand FW status before loading
>> FW, but relying on build time seems fragile. Does it help to add a new section
>> to store version information and evaluate based on this information?
>>
> 
> We have a branch of firmware (several app type combined with NFD3/NFDk) with the same version information(monthly publish),
> so the version information can't help us, because we can't distinguish among them.
> 
> But the build time is different for every firmware, and that's the reason we choose it.
> 

If version is same although FW itself is different, isn't this problem
on its own? Perhaps an additional field is required in version syntax.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 0/8] optimize the firmware loading process
  2024-01-23  9:14     ` Ferruh Yigit
@ 2024-01-23 11:27       ` Chaoyong He
  2024-01-23 11:42         ` Ferruh Yigit
  0 siblings, 1 reply; 16+ messages in thread
From: Chaoyong He @ 2024-01-23 11:27 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: oss-drivers

> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@amd.com>
> >> Sent: Monday, January 22, 2024 11:09 PM
> >> To: Chaoyong He <chaoyong.he@corigine.com>; dev@dpdk.org
> >> Cc: oss-drivers <oss-drivers@corigine.com>
> >> Subject: Re: [PATCH 0/8] optimize the firmware loading process
> >>
> >> On 1/15/2024 2:54 AM, Chaoyong He wrote:
> >>> This patch series aims to speedup the DPDK application start by
> >>> optimize the firmware loading process in sereval places.
> >>> We also simplify the port name in multiple PF firmware case to make
> >>> the customer happy.
> >>>
> >>
> >> <...>
> >>
> >>>   net/nfp: add the elf module
> >>>   net/nfp: reload the firmware only when firmware changed
> >>
> >> Above commit adds elf parser capability and second one loads firmware
> >> when build time is different.
> >>
> >> I can see this is an optimization effort, to understand FW status
> >> before loading FW, but relying on build time seems fragile. Does it
> >> help to add a new section to store version information and evaluate based
> on this information?
> >>
> >
> > We have a branch of firmware (several app type combined with
> > NFD3/NFDk) with the same version information(monthly publish), so the
> version information can't help us, because we can't distinguish among them.
> >
> > But the build time is different for every firmware, and that's the reason we
> choose it.
> >
> 
> If version is same although FW itself is different, isn't this problem on its own?
> Perhaps an additional field is required in version syntax.

Actually, it's just the role the build time which embed in the firmware plays, which is unique for every firmware, and which can't be modified once the firmware was published.

What you said also has its mean, but in practice we can't accept(at least can't do it immediately), which needs to discuss with firmware team.

If you insist that we should change the design, maybe we can just kick off two commits, and upstream the other commits?
- net/nfp: add the elf module
- net/nfp: reload the firmware only when firmware changed

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/8] optimize the firmware loading process
  2024-01-23 11:27       ` Chaoyong He
@ 2024-01-23 11:42         ` Ferruh Yigit
  2024-01-26  8:25           ` Chaoyong He
  0 siblings, 1 reply; 16+ messages in thread
From: Ferruh Yigit @ 2024-01-23 11:42 UTC (permalink / raw)
  To: Chaoyong He, dev; +Cc: oss-drivers

On 1/23/2024 11:27 AM, Chaoyong He wrote:
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>>>> Sent: Monday, January 22, 2024 11:09 PM
>>>> To: Chaoyong He <chaoyong.he@corigine.com>; dev@dpdk.org
>>>> Cc: oss-drivers <oss-drivers@corigine.com>
>>>> Subject: Re: [PATCH 0/8] optimize the firmware loading process
>>>>
>>>> On 1/15/2024 2:54 AM, Chaoyong He wrote:
>>>>> This patch series aims to speedup the DPDK application start by
>>>>> optimize the firmware loading process in sereval places.
>>>>> We also simplify the port name in multiple PF firmware case to make
>>>>> the customer happy.
>>>>>
>>>>
>>>> <...>
>>>>
>>>>>   net/nfp: add the elf module
>>>>>   net/nfp: reload the firmware only when firmware changed
>>>>
>>>> Above commit adds elf parser capability and second one loads firmware
>>>> when build time is different.
>>>>
>>>> I can see this is an optimization effort, to understand FW status
>>>> before loading FW, but relying on build time seems fragile. Does it
>>>> help to add a new section to store version information and evaluate based
>> on this information?
>>>>
>>>
>>> We have a branch of firmware (several app type combined with
>>> NFD3/NFDk) with the same version information(monthly publish), so the
>> version information can't help us, because we can't distinguish among them.
>>>
>>> But the build time is different for every firmware, and that's the reason we
>> choose it.
>>>
>>
>> If version is same although FW itself is different, isn't this problem on its own?
>> Perhaps an additional field is required in version syntax.
> 
> Actually, it's just the role the build time which embed in the firmware plays, which is unique for every firmware, and which can't be modified once the firmware was published.
> 

I see it is already in the elf binary but relying on build time of a FW
to decide to load it or not looks a weak method to me, and fragile as
stated before.

> What you said also has its mean, but in practice we can't accept(at least can't do it immediately), which needs to discuss with firmware team.
> 

As it is an optimization I assume it is not urgent, so would you mind
discussing the issue with the FW team, perhaps it can lead to a better
solution, we can proceed after that.

Meanwhile I will continue with remaining patches, excluding these two
patches.

> If you insist that we should change the design, maybe we can just kick off two commits, and upstream the other commits?
> - net/nfp: add the elf module
> - net/nfp: reload the firmware only when firmware changed


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/8] optimize the firmware loading process
  2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
                   ` (8 preceding siblings ...)
  2024-01-22 15:08 ` [PATCH 0/8] optimize the firmware loading process Ferruh Yigit
@ 2024-01-23 15:18 ` Ferruh Yigit
  9 siblings, 0 replies; 16+ messages in thread
From: Ferruh Yigit @ 2024-01-23 15:18 UTC (permalink / raw)
  To: Chaoyong He, dev; +Cc: oss-drivers

On 1/15/2024 2:54 AM, Chaoyong He wrote:
> This patch series aims to speedup the DPDK application start by optimize
> the firmware loading process in sereval places.
> We also simplify the port name in multiple PF firmware case to make the
> customer happy.
> 
> Peng Zhang (8):
>   net/nfp: add the interface for getting the firmware name
>   net/nfp: speed up the firmware loading process
>   net/nfp: optimize loading the firmware process
>   net/nfp: enlarge the range of skipping loading the firmware
>   net/nfp: add the step of clearing the beat time
>   net/nfp: add the elf module
>   net/nfp: reload the firmware only when firmware changed
>   net/nfp: simplify the port name for multiple PFs
> 

Except 6/8 & 7/8,
Series applied to dpdk-next-net/main, thanks.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 0/8] optimize the firmware loading process
  2024-01-23 11:42         ` Ferruh Yigit
@ 2024-01-26  8:25           ` Chaoyong He
  0 siblings, 0 replies; 16+ messages in thread
From: Chaoyong He @ 2024-01-26  8:25 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: oss-drivers

> On 1/25/2024 2:06 AM, Chaoyong He wrote:
> >>>>>> On 1/15/2024 2:54 AM, Chaoyong He wrote:
> >>>>>>> This patch series aims to speedup the DPDK application start by
> >>>>>>> optimize the firmware loading process in sereval places.
> >>>>>>> We also simplify the port name in multiple PF firmware case to
> >>>>>>> make the customer happy.
> >>>>>>>
> >>>>>>
> >>>>>> <...>
> >>>>>>
> >>>>>>>   net/nfp: add the elf module
> >>>>>>>   net/nfp: reload the firmware only when firmware changed
> >>>>>>
> >>>>>> Above commit adds elf parser capability and second one loads
> >>>>>> firmware when build time is different.
> >>>>>>
> >>>>>> I can see this is an optimization effort, to understand FW status
> >>>>>> before loading FW, but relying on build time seems fragile. Does
> >>>>>> it help to add a new section to store version information and
> >>>>>> evaluate based
> >>>> on this information?
> >>>>>>
> >>>>>
> >>>>> We have a branch of firmware (several app type combined with
> >>>>> NFD3/NFDk) with the same version information(monthly publish), so
> >>>>> the
> >>>> version information can't help us, because we can't distinguish
> >>>> among
> >> them.
> >>>>>
> >>>>> But the build time is different for every firmware, and that's the
> >>>>> reason we
> >>>> choose it.
> >>>>>
> >>>>
> >>>> If version is same although FW itself is different, isn't this
> >>>> problem on its
> >> own?
> >>>> Perhaps an additional field is required in version syntax.
> >>>
> >>> Actually, it's just the role the build time which embed in the
> >>> firmware plays,
> >> which is unique for every firmware, and which can't be modified once
> >> the firmware was published.
> >>>
> >>
> >> I see it is already in the elf binary but relying on build time of a
> >> FW to decide to load it or not looks a weak method to me, and fragile as
> stated before.
> >>
> >>> What you said also has its mean, but in practice we can't accept(at
> >>> least can't
> >> do it immediately), which needs to discuss with firmware team.
> >>>
> >>
> >> As it is an optimization I assume it is not urgent, so would you mind
> >> discussing the issue with the FW team, perhaps it can lead to a
> >> better solution, we can proceed after that.
> >
> > After discussing with the FW team, seems we still can't just use version
> because our firmware are published monthly.
> >
> > Between two public version, we also have internal versions like 'rc0, rc1'
> scheme just as DPDK.
> > But the problem is, we may compile and share many firmware in daily
> development.
> > They are maybe only valid for one time and will never publish, so we won't
> assign a version for them.
> >
> 
> So is my understanding correct that this effort is not for customers and not for
> published FWs, but mostly for developers with development FWs.
> 
> If so perhaps it can be simplified even more, there can be a devarg that force
> loads the FW, for development scenario.
> By default, driver checks the FW version and loads according, but if flag is set it
> loads the FW even with same version. When a new development FW is out,
> developer can run DPDK app with this parameter and it loads the FW to test,
> does this make sense.

Yeah, it's a perfect method, thanks.

> 
> I am not trying to be difficult, just the proposed approach didn't satisfy me I
> am trying to understand it and help if I can. And if you please continue
> discussion in the mailing list it enables others to comment and perhaps
> provide a better option.
> 
> 
> > So the build time is the only information we can distinguish them, if just
> using version, PMD will not reload the firmware which needs to test and
> debug.
> >
> > How about we first using version to check, and then using build time to
> check for the same version?
> >
> 
> Nope, I though elf parsing and relying on build time can be removed, if you
> have them anyway, no need to add the version check, can use just the build
> time to check.
> 
> 
> And another related question, if developers are using daily FWs with exact
> same version, when a developer had an unexpected behavior, how she will
> know exactly which version of the FW (in the device), this maybe required to
> track down the problem. Shouldn't each FW have a unique identifier, except
> the "build time" info  which is stored in the elf binary not FW itself?

We will use the devarg and version as your advice, and remove the build time logics.

> 
> >>
> >> Meanwhile I will continue with remaining patches, excluding these two
> >> patches.
> >>
> >>> If you insist that we should change the design, maybe we can just
> >>> kick off
> >> two commits, and upstream the other commits?
> >>> - net/nfp: add the elf module
> >>> - net/nfp: reload the firmware only when firmware changed
> >


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-01-26  8:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-15  2:54 [PATCH 0/8] optimize the firmware loading process Chaoyong He
2024-01-15  2:54 ` [PATCH 1/8] net/nfp: add the interface for getting the firmware name Chaoyong He
2024-01-15  2:54 ` [PATCH 2/8] net/nfp: speed up the firmware loading process Chaoyong He
2024-01-15  2:54 ` [PATCH 3/8] net/nfp: optimize loading the firmware process Chaoyong He
2024-01-15  2:54 ` [PATCH 4/8] net/nfp: enlarge the range of skipping loading the firmware Chaoyong He
2024-01-15  2:54 ` [PATCH 5/8] net/nfp: add the step of clearing the beat time Chaoyong He
2024-01-15  2:54 ` [PATCH 6/8] net/nfp: add the elf module Chaoyong He
2024-01-15  2:54 ` [PATCH 7/8] net/nfp: reload the firmware only when firmware changed Chaoyong He
2024-01-15  2:54 ` [PATCH 8/8] net/nfp: simplify the port name for multiple PFs Chaoyong He
2024-01-22 15:08 ` [PATCH 0/8] optimize the firmware loading process Ferruh Yigit
2024-01-23  1:46   ` Chaoyong He
2024-01-23  9:14     ` Ferruh Yigit
2024-01-23 11:27       ` Chaoyong He
2024-01-23 11:42         ` Ferruh Yigit
2024-01-26  8:25           ` Chaoyong He
2024-01-23 15:18 ` Ferruh Yigit

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).