[07/14] net/nfp: support NFDK firmware

Message ID 20220602015304.710197-8-jin.liu@corigine.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series Add support of NFP3800 chip and firmware with NFDk |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Jin Liu June 2, 2022, 1:52 a.m. UTC
  Modify nfp driver logic, add firmware version (NFD3 or NFDK) judgment, will
according to the firmware version, mount different driver functions.

Signed-off-by: Jin Liu <jin.liu@corigine.com>
Signed-off-by: Diana Wang <na.wang@corigine.com>
Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Signed-off-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
 drivers/net/nfp/nfp_ctrl.h      |  2 ++
 drivers/net/nfp/nfp_ethdev.c    | 51 ++++++++++++++++++++++++++-------
 drivers/net/nfp/nfp_ethdev_vf.c | 37 +++++++++++++++++-------
 3 files changed, 69 insertions(+), 21 deletions(-)
  

Comments

Ferruh Yigit June 2, 2022, 10:53 p.m. UTC | #1
On 6/2/2022 2:52 AM, Jin Liu wrote:
> Modify nfp driver logic, add firmware version (NFD3 or NFDK) judgment, will
> according to the firmware version, mount different driver functions.
> 

Creating a new set of dev_ops for new FW is a way and it works, but it 
looks like it creates some duplication of the code, and maintaining 
multiple dev_ops can be difficult (driver already has different ones for 
PF & VF).

Another option can be keeping ethdev interface same, but manage 
different FWs closer to FW, where directly interacted with FW.
Like keeping dev_ops as 'nfp_net_tx_queue_release()' and managing 
different FW within this function, instead of having two dev_ops,
'nfp_net_nfdk_tx_queue_release()' & 'nfp_net_nfd3_tx_queue_release()'.
If difference is small, this can be better to reduce duplication.

What is the difference between two FWs, as far as I can see Tx 
descriptor is different and queue setup is affected, is it only diff?

> Signed-off-by: Jin Liu <jin.liu@corigine.com>
> Signed-off-by: Diana Wang <na.wang@corigine.com>
> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@corigine.com>

<...>

> @@ -296,6 +296,32 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
>   	eth_dev->rx_pkt_burst = &nfp_net_recv_pkts;
>   	eth_dev->tx_pkt_burst = &nfp_net_nfd3_xmit_pkts;
>   
> +	hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0].addr;
> +	if (hw->ctrl_bar == NULL) {
> +		PMD_DRV_LOG(ERR,
> +			"hw->ctrl_bar is NULL. BAR0 not configured");
> +		return -ENODEV;
> +	}
> +
> +	PMD_INIT_LOG(DEBUG, "ctrl bar: %p", hw->ctrl_bar);
> +
> +	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
> +
> +	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
> +	case NFP_NET_CFG_VERSION_DP_NFD3:
> +		break;
> +	case NFP_NET_CFG_VERSION_DP_NFDK:
> +		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
> +			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer,found: %d",
> +				NFD_CFG_MAJOR_VERSION_of(hw->ver));
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
> +		return -EINVAL;
> +	}
> +

This part seems extracted to its own function for PF 
('nfp_net_ethdev_ops_mount()'), why not do the same for VF, to have same 
logic between them.
  
Jin Liu June 14, 2022, 8:49 a.m. UTC | #2
We also want to just use one function 'nfp_net_tx_queue_release()' to service both NFD3 and NFDk, But we can not get the version of NFD in function 'nfp_net_tx_queue_release()',  now get NFD version through 'hw->ver'

For the function 'nfp_net_ethdev_ops_mount()', the logic below is in two different C files, nfp_ethdev.c and nfp_ethdev_vf.c And the variable of struct eth_dev_ops is defined as static, if we want to use function both in nfp_ethdev.c and nfp_ethdev_vf.c We need to change the eth_dev_ops variable as non-static, this is not we want.

	> +	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
	> +	case NFP_NET_CFG_VERSION_DP_NFD3:
	> +		break;
	> +	case NFP_NET_CFG_VERSION_DP_NFDK:
	> +		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
	> +			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer,found: %d",
	> +				NFD_CFG_MAJOR_VERSION_of(hw->ver));
	> +			return -EINVAL;
	> +		}
	> +		break;
	> +	default:
	> +		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
	> +		return -EINVAL;

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@xilinx.com> 
Sent: Friday, June 3, 2022 06:54
To: Kevin Liu <jin.liu@corigine.com>; dev@dpdk.org
Cc: Niklas Soderlund <niklas.soderlund@corigine.com>; Diana Wang <na.wang@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Chaoyong He <chaoyong.he@corigine.com>
Subject: Re: [PATCH 07/14] net/nfp: support NFDK firmware

On 6/2/2022 2:52 AM, Jin Liu wrote:
> Modify nfp driver logic, add firmware version (NFD3 or NFDK) judgment, 
> will according to the firmware version, mount different driver functions.
> 

Creating a new set of dev_ops for new FW is a way and it works, but it looks like it creates some duplication of the code, and maintaining multiple dev_ops can be difficult (driver already has different ones for PF & VF).

Another option can be keeping ethdev interface same, but manage different FWs closer to FW, where directly interacted with FW.
Like keeping dev_ops as 'nfp_net_tx_queue_release()' and managing different FW within this function, instead of having two dev_ops, 'nfp_net_nfdk_tx_queue_release()' & 'nfp_net_nfd3_tx_queue_release()'.
If difference is small, this can be better to reduce duplication.

What is the difference between two FWs, as far as I can see Tx descriptor is different and queue setup is affected, is it only diff?

> Signed-off-by: Jin Liu <jin.liu@corigine.com>
> Signed-off-by: Diana Wang <na.wang@corigine.com>
> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@corigine.com>

<...>

> @@ -296,6 +296,32 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
>   	eth_dev->rx_pkt_burst = &nfp_net_recv_pkts;
>   	eth_dev->tx_pkt_burst = &nfp_net_nfd3_xmit_pkts;
>   
> +	hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0].addr;
> +	if (hw->ctrl_bar == NULL) {
> +		PMD_DRV_LOG(ERR,
> +			"hw->ctrl_bar is NULL. BAR0 not configured");
> +		return -ENODEV;
> +	}
> +
> +	PMD_INIT_LOG(DEBUG, "ctrl bar: %p", hw->ctrl_bar);
> +
> +	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
> +
> +	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
> +	case NFP_NET_CFG_VERSION_DP_NFD3:
> +		break;
> +	case NFP_NET_CFG_VERSION_DP_NFDK:
> +		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
> +			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer,found: %d",
> +				NFD_CFG_MAJOR_VERSION_of(hw->ver));
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
> +		return -EINVAL;
> +	}
> +

This part seems extracted to its own function for PF ('nfp_net_ethdev_ops_mount()'), why not do the same for VF, to have same logic between them.
  
Ferruh Yigit June 14, 2022, 9:21 a.m. UTC | #3
On 6/14/2022 9:49 AM, Kevin Liu wrote:
> We also want to just use one function 'nfp_net_tx_queue_release()' to service both NFD3 and NFDk, But we can not get the version of NFD in function 'nfp_net_tx_queue_release()',  now get NFD version through 'hw->ver'
> 

Again, it is up to you, but it should be possible to add 'dev' or 'hw' 
reference to the queue struct, to be able to access the version information.
And it can be possible to have something like 'struct fw_ops', set it 
during initialization and use in rest of the dev_ops.

> For the function 'nfp_net_ethdev_ops_mount()', the logic below is in two different C files, nfp_ethdev.c and nfp_ethdev_vf.c And the variable of struct eth_dev_ops is defined as static, if we want to use function both in nfp_ethdev.c and nfp_ethdev_vf.c We need to change the eth_dev_ops variable as non-static, this is not we want.
> 
> 	> +	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
> 	> +	case NFP_NET_CFG_VERSION_DP_NFD3:
> 	> +		break;
> 	> +	case NFP_NET_CFG_VERSION_DP_NFDK:
> 	> +		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
> 	> +			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer,found: %d",
> 	> +				NFD_CFG_MAJOR_VERSION_of(hw->ver));
> 	> +			return -EINVAL;
> 	> +		}
> 	> +		break;
> 	> +	default:
> 	> +		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
> 	> +		return -EINVAL;
> 

My comment was to extract the logic into its own function as it is done 
is PF, so to have something like 'nfp_netvf_ethdev_ops_mount()'.


> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Friday, June 3, 2022 06:54
> To: Kevin Liu <jin.liu@corigine.com>; dev@dpdk.org
> Cc: Niklas Soderlund <niklas.soderlund@corigine.com>; Diana Wang <na.wang@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Chaoyong He <chaoyong.he@corigine.com>
> Subject: Re: [PATCH 07/14] net/nfp: support NFDK firmware
> 
> On 6/2/2022 2:52 AM, Jin Liu wrote:
>> Modify nfp driver logic, add firmware version (NFD3 or NFDK) judgment,
>> will according to the firmware version, mount different driver functions.
>>
> 
> Creating a new set of dev_ops for new FW is a way and it works, but it looks like it creates some duplication of the code, and maintaining multiple dev_ops can be difficult (driver already has different ones for PF & VF).
> 
> Another option can be keeping ethdev interface same, but manage different FWs closer to FW, where directly interacted with FW.
> Like keeping dev_ops as 'nfp_net_tx_queue_release()' and managing different FW within this function, instead of having two dev_ops, 'nfp_net_nfdk_tx_queue_release()' & 'nfp_net_nfd3_tx_queue_release()'.
> If difference is small, this can be better to reduce duplication.
> 
> What is the difference between two FWs, as far as I can see Tx descriptor is different and queue setup is affected, is it only diff?
> 
>> Signed-off-by: Jin Liu <jin.liu@corigine.com>
>> Signed-off-by: Diana Wang <na.wang@corigine.com>
>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
>> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
>> Signed-off-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> 
> <...>
> 
>> @@ -296,6 +296,32 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
>>    	eth_dev->rx_pkt_burst = &nfp_net_recv_pkts;
>>    	eth_dev->tx_pkt_burst = &nfp_net_nfd3_xmit_pkts;
>>    
>> +	hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0].addr;
>> +	if (hw->ctrl_bar == NULL) {
>> +		PMD_DRV_LOG(ERR,
>> +			"hw->ctrl_bar is NULL. BAR0 not configured");
>> +		return -ENODEV;
>> +	}
>> +
>> +	PMD_INIT_LOG(DEBUG, "ctrl bar: %p", hw->ctrl_bar);
>> +
>> +	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
>> +
>> +	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
>> +	case NFP_NET_CFG_VERSION_DP_NFD3:
>> +		break;
>> +	case NFP_NET_CFG_VERSION_DP_NFDK:
>> +		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
>> +			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer,found: %d",
>> +				NFD_CFG_MAJOR_VERSION_of(hw->ver));
>> +			return -EINVAL;
>> +		}
>> +		break;
>> +	default:
>> +		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
>> +		return -EINVAL;
>> +	}
>> +
> 
> This part seems extracted to its own function for PF ('nfp_net_ethdev_ops_mount()'), why not do the same for VF, to have same logic between them.
> 
>
  
Jin Liu June 14, 2022, 9:30 a.m. UTC | #4
Now, I modify logic, just keep 'nfp_net_tx_queue_release()', delete 'nfp_net_nfdk_tx_queue_release()' & 'nfp_net_nfd3_tx_queue_release()'


>>>   My comment was to extract the logic into its own function as it is done is PF, so to have something like 'nfp_netvf_ethdev_ops_mount()'.

the logic I just use once in nfp_ethdev_vf.c, so not create a function. I will create 'nfp_netvf_ethdev_ops_mount()' in nfp_ethdev_vf.c  later

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@xilinx.com> 
Sent: Tuesday, June 14, 2022 17:22
To: Kevin Liu <jin.liu@corigine.com>; dev@dpdk.org
Cc: Niklas Soderlund <niklas.soderlund@corigine.com>; Diana Wang <na.wang@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Chaoyong He <chaoyong.he@corigine.com>
Subject: Re: [PATCH 07/14] net/nfp: support NFDK firmware

On 6/14/2022 9:49 AM, Kevin Liu wrote:
> We also want to just use one function 'nfp_net_tx_queue_release()' to service both NFD3 and NFDk, But we can not get the version of NFD in function 'nfp_net_tx_queue_release()',  now get NFD version through 'hw->ver'
> 

Again, it is up to you, but it should be possible to add 'dev' or 'hw' 
reference to the queue struct, to be able to access the version information.
And it can be possible to have something like 'struct fw_ops', set it during initialization and use in rest of the dev_ops.

> For the function 'nfp_net_ethdev_ops_mount()', the logic below is in two different C files, nfp_ethdev.c and nfp_ethdev_vf.c And the variable of struct eth_dev_ops is defined as static, if we want to use function both in nfp_ethdev.c and nfp_ethdev_vf.c We need to change the eth_dev_ops variable as non-static, this is not we want.
> 
> 	> +	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
> 	> +	case NFP_NET_CFG_VERSION_DP_NFD3:
> 	> +		break;
> 	> +	case NFP_NET_CFG_VERSION_DP_NFDK:
> 	> +		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
> 	> +			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer,found: %d",
> 	> +				NFD_CFG_MAJOR_VERSION_of(hw->ver));
> 	> +			return -EINVAL;
> 	> +		}
> 	> +		break;
> 	> +	default:
> 	> +		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
> 	> +		return -EINVAL;
> 

My comment was to extract the logic into its own function as it is done is PF, so to have something like 'nfp_netvf_ethdev_ops_mount()'.


> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@xilinx.com>
> Sent: Friday, June 3, 2022 06:54
> To: Kevin Liu <jin.liu@corigine.com>; dev@dpdk.org
> Cc: Niklas Soderlund <niklas.soderlund@corigine.com>; Diana Wang <na.wang@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Chaoyong He <chaoyong.he@corigine.com>
> Subject: Re: [PATCH 07/14] net/nfp: support NFDK firmware
> 
> On 6/2/2022 2:52 AM, Jin Liu wrote:
>> Modify nfp driver logic, add firmware version (NFD3 or NFDK) judgment,
>> will according to the firmware version, mount different driver functions.
>>
> 
> Creating a new set of dev_ops for new FW is a way and it works, but it looks like it creates some duplication of the code, and maintaining multiple dev_ops can be difficult (driver already has different ones for PF & VF).
> 
> Another option can be keeping ethdev interface same, but manage different FWs closer to FW, where directly interacted with FW.
> Like keeping dev_ops as 'nfp_net_tx_queue_release()' and managing different FW within this function, instead of having two dev_ops, 'nfp_net_nfdk_tx_queue_release()' & 'nfp_net_nfd3_tx_queue_release()'.
> If difference is small, this can be better to reduce duplication.
> 
> What is the difference between two FWs, as far as I can see Tx descriptor is different and queue setup is affected, is it only diff?
> 
>> Signed-off-by: Jin Liu <jin.liu@corigine.com>
>> Signed-off-by: Diana Wang <na.wang@corigine.com>
>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
>> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
>> Signed-off-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> 
> <...>
> 
>> @@ -296,6 +296,32 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
>>    	eth_dev->rx_pkt_burst = &nfp_net_recv_pkts;
>>    	eth_dev->tx_pkt_burst = &nfp_net_nfd3_xmit_pkts;
>>    
>> +	hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0].addr;
>> +	if (hw->ctrl_bar == NULL) {
>> +		PMD_DRV_LOG(ERR,
>> +			"hw->ctrl_bar is NULL. BAR0 not configured");
>> +		return -ENODEV;
>> +	}
>> +
>> +	PMD_INIT_LOG(DEBUG, "ctrl bar: %p", hw->ctrl_bar);
>> +
>> +	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
>> +
>> +	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
>> +	case NFP_NET_CFG_VERSION_DP_NFD3:
>> +		break;
>> +	case NFP_NET_CFG_VERSION_DP_NFDK:
>> +		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
>> +			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer,found: %d",
>> +				NFD_CFG_MAJOR_VERSION_of(hw->ver));
>> +			return -EINVAL;
>> +		}
>> +		break;
>> +	default:
>> +		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
>> +		return -EINVAL;
>> +	}
>> +
> 
> This part seems extracted to its own function for PF ('nfp_net_ethdev_ops_mount()'), why not do the same for VF, to have same logic between them.
> 
>
  

Patch

diff --git a/drivers/net/nfp/nfp_ctrl.h b/drivers/net/nfp/nfp_ctrl.h
index 4dd62ef194..e73715e2aa 100644
--- a/drivers/net/nfp/nfp_ctrl.h
+++ b/drivers/net/nfp/nfp_ctrl.h
@@ -135,6 +135,8 @@ 
  * - define more STS bits
  */
 #define NFP_NET_CFG_VERSION             0x0030
+#define   NFP_NET_CFG_VERSION_DP_NFD3   0
+#define   NFP_NET_CFG_VERSION_DP_NFDK   1
 #define   NFP_NET_CFG_VERSION_RESERVED_MASK	(0xff << 24)
 #define   NFP_NET_CFG_VERSION_CLASS_MASK  (0xff << 16)
 #define   NFP_NET_CFG_VERSION_CLASS(x)    (((x) & 0xff) << 16)
diff --git a/drivers/net/nfp/nfp_ethdev.c b/drivers/net/nfp/nfp_ethdev.c
index 4d2cd9b0b3..c09a035323 100644
--- a/drivers/net/nfp/nfp_ethdev.c
+++ b/drivers/net/nfp/nfp_ethdev.c
@@ -358,6 +358,32 @@  static const struct eth_dev_ops nfp_net_nfd3_eth_dev_ops = {
 	.rx_queue_intr_disable  = nfp_rx_queue_intr_disable,
 };
 
+static inline int
+nfp_net_ethdev_ops_mount(struct nfp_net_hw *hw, struct rte_eth_dev *eth_dev)
+{
+	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
+	case NFP_NET_CFG_VERSION_DP_NFD3:
+		break;
+	case NFP_NET_CFG_VERSION_DP_NFDK:
+		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
+			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer, found: %d",
+				NFD_CFG_MAJOR_VERSION_of(hw->ver));
+			return -EINVAL;
+		}
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
+		return -EINVAL;
+	}
+
+	eth_dev->dev_ops = &nfp_net_nfd3_eth_dev_ops;
+	eth_dev->rx_queue_count = nfp_net_rx_queue_count;
+	eth_dev->rx_pkt_burst = &nfp_net_recv_pkts;
+	eth_dev->tx_pkt_burst = &nfp_net_nfd3_xmit_pkts;
+
+	return 0;
+}
+
 static int
 nfp_net_init(struct rte_eth_dev *eth_dev)
 {
@@ -384,7 +410,7 @@  nfp_net_init(struct rte_eth_dev *eth_dev)
 		RTE_LOG(ERR, PMD, "device %s can not be used: restricted dma "
 			"mask to 40 bits!\n", pci_dev->device.name);
 		return -ENODEV;
-	};
+	}
 
 	port = ((struct nfp_net_hw *)eth_dev->data->dev_private)->idx;
 	if (port < 0 || port > 7) {
@@ -401,11 +427,6 @@  nfp_net_init(struct rte_eth_dev *eth_dev)
 	PMD_INIT_LOG(DEBUG, "Working with physical port number: %d, "
 			"NFP internal port number: %d", port, hw->nfp_idx);
 
-	eth_dev->dev_ops = &nfp_net_nfd3_eth_dev_ops;
-	eth_dev->rx_queue_count = nfp_net_rx_queue_count;
-	eth_dev->rx_pkt_burst = &nfp_net_recv_pkts;
-	eth_dev->tx_pkt_burst = &nfp_net_nfd3_xmit_pkts;
-
 	/* For secondary processes, the primary has done all the work */
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
@@ -440,6 +461,11 @@  nfp_net_init(struct rte_eth_dev *eth_dev)
 
 	PMD_INIT_LOG(DEBUG, "ctrl bar: %p", hw->ctrl_bar);
 
+	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
+
+	if (nfp_net_ethdev_ops_mount(hw, eth_dev))
+		return -EINVAL;
+
 	hw->max_rx_queues = nn_cfg_readl(hw, NFP_NET_CFG_MAX_RXRINGS);
 	hw->max_tx_queues = nn_cfg_readl(hw, NFP_NET_CFG_MAX_TXRINGS);
 
@@ -472,7 +498,6 @@  nfp_net_init(struct rte_eth_dev *eth_dev)
 	nfp_net_cfg_queue_setup(hw);
 
 	/* Get some of the read-only fields from the config BAR */
-	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
 	hw->cap = nn_cfg_readl(hw, NFP_NET_CFG_CAP);
 	hw->max_mtu = nn_cfg_readl(hw, NFP_NET_CFG_MAX_MTU);
 	hw->mtu = RTE_ETHER_MTU;
@@ -938,6 +963,7 @@  nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
 	int err;
 	int total_ports;
 	struct nfp_cpp *cpp;
+	struct nfp_net_hw *hw;
 	struct nfp_rtsym_table *sym_tbl;
 
 	if (pci_dev == NULL)
@@ -987,11 +1013,14 @@  nfp_pf_secondary_init(struct rte_pci_device *pci_dev)
 				"ethdev doesn't exist");
 			return -ENODEV;
 		}
+
+		hw = NFP_NET_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
+
+		if (nfp_net_ethdev_ops_mount(hw, eth_dev))
+			return -EINVAL;
+
 		eth_dev->process_private = cpp;
-		eth_dev->dev_ops = &nfp_net_nfd3_eth_dev_ops;
-		eth_dev->rx_queue_count = nfp_net_rx_queue_count;
-		eth_dev->rx_pkt_burst = &nfp_net_recv_pkts;
-		eth_dev->tx_pkt_burst = &nfp_net_nfd3_xmit_pkts;
+
 		rte_eth_dev_probing_finish(eth_dev);
 	}
 
diff --git a/drivers/net/nfp/nfp_ethdev_vf.c b/drivers/net/nfp/nfp_ethdev_vf.c
index f5a0406e7d..e83c9dbcaf 100644
--- a/drivers/net/nfp/nfp_ethdev_vf.c
+++ b/drivers/net/nfp/nfp_ethdev_vf.c
@@ -287,7 +287,7 @@  nfp_netvf_init(struct rte_eth_dev *eth_dev)
 		RTE_LOG(ERR, PMD, "device %s can not be used: restricted dma "
 			"mask to 40 bits!\n", pci_dev->device.name);
 		return -ENODEV;
-	};
+	}
 
 	hw = NFP_NET_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
 
@@ -296,6 +296,32 @@  nfp_netvf_init(struct rte_eth_dev *eth_dev)
 	eth_dev->rx_pkt_burst = &nfp_net_recv_pkts;
 	eth_dev->tx_pkt_burst = &nfp_net_nfd3_xmit_pkts;
 
+	hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0].addr;
+	if (hw->ctrl_bar == NULL) {
+		PMD_DRV_LOG(ERR,
+			"hw->ctrl_bar is NULL. BAR0 not configured");
+		return -ENODEV;
+	}
+
+	PMD_INIT_LOG(DEBUG, "ctrl bar: %p", hw->ctrl_bar);
+
+	hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
+
+	switch (NFD_CFG_CLASS_VER_of(hw->ver)) {
+	case NFP_NET_CFG_VERSION_DP_NFD3:
+		break;
+	case NFP_NET_CFG_VERSION_DP_NFDK:
+		if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 5) {
+			PMD_DRV_LOG(ERR, "NFDK must use ABI 5 or newer,found: %d",
+				NFD_CFG_MAJOR_VERSION_of(hw->ver));
+			return -EINVAL;
+		}
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "The version of firmware is not correct.");
+		return -EINVAL;
+	}
+
 	/* For secondary processes, the primary has done all the work */
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
@@ -312,15 +338,6 @@  nfp_netvf_init(struct rte_eth_dev *eth_dev)
 		     pci_dev->addr.domain, pci_dev->addr.bus,
 		     pci_dev->addr.devid, pci_dev->addr.function);
 
-	hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0].addr;
-	if (hw->ctrl_bar == NULL) {
-		PMD_DRV_LOG(ERR,
-			"hw->ctrl_bar is NULL. BAR0 not configured");
-		return -ENODEV;
-	}
-
-	PMD_INIT_LOG(DEBUG, "ctrl bar: %p", hw->ctrl_bar);
-
 	hw->max_rx_queues = nn_cfg_readl(hw, NFP_NET_CFG_MAX_RXRINGS);
 	hw->max_tx_queues = nn_cfg_readl(hw, NFP_NET_CFG_MAX_TXRINGS);