[PATCH net-next 0/7] nfp: expose more firmware and hw debug info

[PATCH net-next 0/7] nfp: expose more firmware and hw debug info

[Jakub Kicinski]

Hi!

This series is mostly a result of flash firmware team requesting 
access to some of the information and data necessary for debugging 
firmware problems.  Patch 1 adds a missing error message.  Patch 2 
prints manufacturing info to logs in case PCI VPD capability is 
not programmed correctly.  Patches 3 and 4 allow reporting NSP ABI
version in ethtool -i.  Patch 5 allows dumping flash application 
logs.  

Patches 6 and 7 provides a way of application firmwares to declare 
limited SR-IOV support.  

Jakub Kicinski (7):
  nfp: refactor NSP initialization and add error message
  nfp: report manufacturing info on load
  nfp: store NSP ABI version in state structure
  nfp: report NSP ABI version in ethtool FW version
  nfp: add very basic access to NSP logs
  nfp: return nfp_rtsym_read_le() errors correctly
  nfp: allow application firmware to limit number of SR-IOV VFs

 drivers/net/ethernet/netronome/nfp/nfp_main.c      |  85 ++++++++++++-----
 drivers/net/ethernet/netronome/nfp/nfp_main.h      |   2 +
 drivers/net/ethernet/netronome/nfp/nfp_net.h       |   6 ++
 .../net/ethernet/netronome/nfp/nfp_net_ethtool.c   | 101 ++++++++++++++++++++-
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c  |   1 +
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h   |   4 +
 .../net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c   |  37 +++++---
 .../net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c |   8 +-
 8 files changed, 204 insertions(+), 40 deletions(-)

-- 
2.11.0

[PATCH net-next 1/7] nfp: refactor NSP initialization and add error message

[Jakub Kicinski]

When acquiring NSP communication resource fails user is left with
"probe failed with error -2" PCI code message but no info on what
caused the problem.  Some development boards may not have NSP FW
in the flash image.  Help users with a more verbouse message.

While at it move the whole NSP init to a separate function to keep
.probe() callback nice and simple.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_main.c | 59 ++++++++++++++++-----------
 1 file changed, 36 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index db52b6a53c6f..ca2c464c15df 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -228,6 +228,40 @@ nfp_fw_load(struct pci_dev *pdev, struct nfp_pf *pf, struct nfp_nsp *nsp)
 	return err < 0 ? err : 1;
 }
 
+static int nfp_nsp_init(struct pci_dev *pdev, struct nfp_pf *pf)
+{
+	struct nfp_nsp *nsp;
+	int err;
+
+	nsp = nfp_nsp_open(pf->cpp);
+	if (IS_ERR(nsp)) {
+		err = PTR_ERR(nsp);
+		dev_err(&pdev->dev, "Failed to access the NSP: %d\n", err);
+		return err;
+	}
+
+	err = nfp_nsp_wait(nsp);
+	if (err < 0)
+		goto exit_close_nsp;
+
+	pf->eth_tbl = __nfp_eth_read_ports(pf->cpp, nsp);
+
+	err = nfp_fw_load(pdev, pf, nsp);
+	if (err < 0) {
+		kfree(pf->eth_tbl);
+		dev_err(&pdev->dev, "Failed to load FW\n");
+		goto exit_close_nsp;
+	}
+
+	pf->fw_loaded = !!err;
+	err = 0;
+
+exit_close_nsp:
+	nfp_nsp_close(nsp);
+
+	return err;
+}
+
 static void nfp_fw_unload(struct nfp_pf *pf)
 {
 	struct nfp_nsp *nsp;
@@ -251,7 +285,6 @@ static void nfp_fw_unload(struct nfp_pf *pf)
 static int nfp_pci_probe(struct pci_dev *pdev,
 			 const struct pci_device_id *pci_id)
 {
-	struct nfp_nsp *nsp;
 	struct nfp_pf *pf;
 	int err;
 
@@ -289,28 +322,9 @@ static int nfp_pci_probe(struct pci_dev *pdev,
 		goto err_disable_msix;
 	}
 
-	nsp = nfp_nsp_open(pf->cpp);
-	if (IS_ERR(nsp)) {
-		err = PTR_ERR(nsp);
-		goto err_cpp_free;
-	}
-
-	err = nfp_nsp_wait(nsp);
-	if (err < 0) {
-		nfp_nsp_close(nsp);
+	err = nfp_nsp_init(pdev, pf);
+	if (err)
 		goto err_cpp_free;
-	}
-
-	pf->eth_tbl = __nfp_eth_read_ports(pf->cpp, nsp);
-
-	err = nfp_fw_load(pdev, pf, nsp);
-	nfp_nsp_close(nsp);
-	if (err < 0) {
-		dev_err(&pdev->dev, "Failed to load FW\n");
-		goto err_eth_tbl_free;
-	}
-
-	pf->fw_loaded = !!err;
 
 	err = nfp_net_pci_probe(pf);
 	if (err)
@@ -321,7 +335,6 @@ static int nfp_pci_probe(struct pci_dev *pdev,
 err_fw_unload:
 	if (pf->fw_loaded)
 		nfp_fw_unload(pf);
-err_eth_tbl_free:
 	kfree(pf->eth_tbl);
 err_cpp_free:
 	nfp_cpp_free(pf->cpp);
-- 
2.11.0

[PATCH net-next 2/7] nfp: report manufacturing info on load

[Jakub Kicinski]

Report card manufacturing information when driver loads.  These
identify the version of the board and its subcomponents.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index ca2c464c15df..8cda6b0e7e32 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -322,6 +322,13 @@ static int nfp_pci_probe(struct pci_dev *pdev,
 		goto err_disable_msix;
 	}
 
+	dev_info(&pdev->dev, "Assembly: %s%s%s-%s CPLD: %s\n",
+		 nfp_hwinfo_lookup(pf->cpp, "assembly.vendor"),
+		 nfp_hwinfo_lookup(pf->cpp, "assembly.partno"),
+		 nfp_hwinfo_lookup(pf->cpp, "assembly.serial"),
+		 nfp_hwinfo_lookup(pf->cpp, "assembly.revision"),
+		 nfp_hwinfo_lookup(pf->cpp, "cpld.version"));
+
 	err = nfp_nsp_init(pdev, pf);
 	if (err)
 		goto err_cpp_free;
-- 
2.11.0

[PATCH net-next 3/7] nfp: store NSP ABI version in state structure

[Jakub Kicinski]

We read the status register on each NSP open, we can store the NSP
ABI version in the state structure so that we don't have to read
it again.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 .../net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c   | 27 +++++++++++-----------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
index f07f2fc3fba0..f2d737c3d1df 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
@@ -99,6 +99,10 @@ enum nfp_nsp_cmd {
 struct nfp_nsp {
 	struct nfp_cpp *cpp;
 	struct nfp_resource *res;
+	struct {
+		u16 major;
+		u16 minor;
+	} ver;
 };
 
 static int nfp_nsp_check(struct nfp_nsp *state)
@@ -120,11 +124,12 @@ static int nfp_nsp_check(struct nfp_nsp *state)
 		return -ENODEV;
 	}
 
-	if (FIELD_GET(NSP_STATUS_MAJOR, reg) != NSP_MAJOR ||
-	    FIELD_GET(NSP_STATUS_MINOR, reg) < NSP_MINOR) {
-		nfp_err(cpp, "Unsupported ABI %lld.%lld\n",
-			FIELD_GET(NSP_STATUS_MAJOR, reg),
-			FIELD_GET(NSP_STATUS_MINOR, reg));
+	state->ver.major = FIELD_GET(NSP_STATUS_MAJOR, reg);
+	state->ver.minor = FIELD_GET(NSP_STATUS_MINOR, reg);
+
+	if (state->ver.major != NSP_MAJOR || state->ver.minor < NSP_MINOR) {
+		nfp_err(cpp, "Unsupported ABI %hu.%hu\n",
+			state->ver.major, state->ver.minor);
 		return -EINVAL;
 	}
 
@@ -301,15 +306,9 @@ static int nfp_nsp_command_buf(struct nfp_nsp *nsp, u16 code, u32 option,
 	int ret, err;
 	u32 cpp_id;
 
-	err = nfp_cpp_readq(cpp, nfp_resource_cpp_id(nsp->res),
-			    nfp_resource_address(nsp->res) + NSP_STATUS, &reg);
-	if (err < 0)
-		return err;
-
-	if (FIELD_GET(NSP_STATUS_MINOR, reg) < 13) {
-		nfp_err(cpp, "NSP: Code 0x%04x with buffer not supported (ABI %lld.%lld)\n",
-			code, FIELD_GET(NSP_STATUS_MAJOR, reg),
-			FIELD_GET(NSP_STATUS_MINOR, reg));
+	if (nsp->ver.minor < 13) {
+		nfp_err(cpp, "NSP: Code 0x%04x with buffer not supported (ABI %hu.%hu)\n",
+			code, nsp->ver.major, nsp->ver.minor);
 		return -EOPNOTSUPP;
 	}
 
-- 
2.11.0

[PATCH net-next 4/7] nfp: report NSP ABI version in ethtool FW version

[Jakub Kicinski]

ethtool_drvinfo->fw_version can cantain multiple FW strings.
We already report NFD ABI version there, add NSP ABI version
if available (i.e. on PF) with 'sp:' prefix.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net.h       |  4 ++++
 .../net/ethernet/netronome/nfp/nfp_net_ethtool.c   | 25 ++++++++++++++++++++--
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c  |  1 +
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h   |  3 +++
 .../net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c   | 10 +++++++++
 5 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index d37d2391b4fe..5a0fc09dd6ea 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -111,6 +111,7 @@
 				 SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
 
 /* Forward declarations */
+struct nfp_cpp;
 struct nfp_net;
 struct nfp_net_r_vector;
 
@@ -492,6 +493,7 @@ struct nfp_stat_pair {
  * @rx_bar:             Pointer to mapped FL/RX queues
  * @debugfs_dir:	Device directory in debugfs
  * @port_list:		Entry on device port list
+ * @cpp:		CPP device handle if available
  */
 struct nfp_net {
 	struct pci_dev *pdev;
@@ -579,6 +581,8 @@ struct nfp_net {
 	struct dentry *debugfs_dir;
 
 	struct list_head port_list;
+
+	struct nfp_cpp *cpp;
 };
 
 struct nfp_net_ring_set {
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 255f30252550..48f623f68598 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -47,6 +47,7 @@
 #include <linux/pci.h>
 #include <linux/ethtool.h>
 
+#include "nfpcore/nfp.h"
 #include "nfp_net_ctrl.h"
 #include "nfp_net.h"
 
@@ -127,19 +128,39 @@ static const struct _nfp_net_et_stats nfp_net_et_stats[] = {
 #define NN_ET_STATS_LEN (NN_ET_GLOBAL_STATS_LEN + NN_ET_RVEC_GATHER_STATS + \
 			 NN_ET_RVEC_STATS_LEN + NN_ET_QUEUE_STATS_LEN)
 
+static void nfp_net_get_nspinfo(struct nfp_net *nn, char *version)
+{
+	struct nfp_nsp *nsp;
+
+	if (!nn->cpp)
+		return;
+
+	nsp = nfp_nsp_open(nn->cpp);
+	if (IS_ERR(nsp))
+		return;
+
+	snprintf(version, ETHTOOL_FWVERS_LEN, "sp:%hu.%hu",
+		 nfp_nsp_get_abi_ver_major(nsp),
+		 nfp_nsp_get_abi_ver_minor(nsp));
+
+	nfp_nsp_close(nsp);
+}
+
 static void nfp_net_get_drvinfo(struct net_device *netdev,
 				struct ethtool_drvinfo *drvinfo)
 {
+	char nsp_version[ETHTOOL_FWVERS_LEN] = {};
 	struct nfp_net *nn = netdev_priv(netdev);
 
 	strlcpy(drvinfo->driver, nn->pdev->driver->name,
 		sizeof(drvinfo->driver));
 	strlcpy(drvinfo->version, nfp_driver_version, sizeof(drvinfo->version));
 
+	nfp_net_get_nspinfo(nn, nsp_version);
 	snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
-		 "%d.%d.%d.%d",
+		 "%d.%d.%d.%d %s",
 		 nn->fw_ver.resv, nn->fw_ver.class,
-		 nn->fw_ver.major, nn->fw_ver.minor);
+		 nn->fw_ver.major, nn->fw_ver.minor, nsp_version);
 	strlcpy(drvinfo->bus_info, pci_name(nn->pdev),
 		sizeof(drvinfo->bus_info));
 
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index eccd31003b0d..3afcdc11480c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -303,6 +303,7 @@ nfp_net_pf_alloc_port_netdev(struct nfp_pf *pf, void __iomem *ctrl_bar,
 	if (IS_ERR(nn))
 		return nn;
 
+	nn->cpp = pf->cpp;
 	nn->fw_ver = *fw_ver;
 	nn->ctrl_bar = ctrl_bar;
 	nn->tx_bar = tx_bar;
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
index 3d70a8578a98..540027973a74 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
@@ -40,6 +40,7 @@
 #define __NFP_H__
 
 #include <linux/device.h>
+#include <linux/types.h>
 
 #include "nfp_cpp.h"
 
@@ -54,6 +55,8 @@ struct firmware;
 
 struct nfp_nsp *nfp_nsp_open(struct nfp_cpp *cpp);
 void nfp_nsp_close(struct nfp_nsp *state);
+u16 nfp_nsp_get_abi_ver_major(struct nfp_nsp *state);
+u16 nfp_nsp_get_abi_ver_minor(struct nfp_nsp *state);
 int nfp_nsp_wait(struct nfp_nsp *state);
 int nfp_nsp_device_soft_reset(struct nfp_nsp *state);
 int nfp_nsp_load_fw(struct nfp_nsp *state, const struct firmware *fw);
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
index f2d737c3d1df..34c50987c377 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
@@ -182,6 +182,16 @@ void nfp_nsp_close(struct nfp_nsp *state)
 	kfree(state);
 }
 
+u16 nfp_nsp_get_abi_ver_major(struct nfp_nsp *state)
+{
+	return state->ver.major;
+}
+
+u16 nfp_nsp_get_abi_ver_minor(struct nfp_nsp *state)
+{
+	return state->ver.minor;
+}
+
 static int
 nfp_nsp_wait_reg(struct nfp_cpp *cpp, u64 *reg,
 		 u32 nsp_cpp, u64 addr, u64 mask, u64 val)
-- 
2.11.0

[PATCH net-next 5/7] nfp: add very basic access to NSP logs

[Jakub Kicinski]

Allow dumping "arm.diag" resource with ethtool -w.  This resource
should contain a text log of the NSP (control processor) application.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net.h       |  2 +
 .../net/ethernet/netronome/nfp/nfp_net_ethtool.c   | 76 ++++++++++++++++++++++
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h   |  1 +
 3 files changed, 79 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
index 5a0fc09dd6ea..e614a376b595 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
@@ -492,6 +492,7 @@ struct nfp_stat_pair {
  * @tx_bar:             Pointer to mapped TX queues
  * @rx_bar:             Pointer to mapped FL/RX queues
  * @debugfs_dir:	Device directory in debugfs
+ * @ethtool_dump_flag:	Ethtool dump flag
  * @port_list:		Entry on device port list
  * @cpp:		CPP device handle if available
  */
@@ -579,6 +580,7 @@ struct nfp_net {
 	u8 __iomem *rx_bar;
 
 	struct dentry *debugfs_dir;
+	u32 ethtool_dump_flag;
 
 	struct list_head port_list;
 
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 48f623f68598..2649f7523c81 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -51,6 +51,10 @@
 #include "nfp_net_ctrl.h"
 #include "nfp_net.h"
 
+enum nfp_dump_diag {
+	NFP_DUMP_NSP_DIAG = 0,
+};
+
 /* Support for stats. Returns netdev, driver, and device stats */
 enum { NETDEV_ET_STATS, NFP_NET_DRV_ET_STATS, NFP_NET_DEV_ET_STATS };
 struct _nfp_net_et_stats {
@@ -579,6 +583,75 @@ static int nfp_net_get_coalesce(struct net_device *netdev,
 	return 0;
 }
 
+/* Other debug dumps
+ */
+static int
+nfp_dump_nsp_diag(struct nfp_net *nn, struct ethtool_dump *dump, void *buffer)
+{
+	struct nfp_resource *res;
+	int ret;
+
+	if (!nn->cpp)
+		return -EOPNOTSUPP;
+
+	dump->version = 1;
+	dump->flag = NFP_DUMP_NSP_DIAG;
+
+	res = nfp_resource_acquire(nn->cpp, NFP_RESOURCE_NSP_DIAG);
+	if (IS_ERR(res))
+		return PTR_ERR(res);
+
+	if (buffer) {
+		if (dump->len != nfp_resource_size(res)) {
+			ret = -EINVAL;
+			goto exit_release;
+		}
+
+		ret = nfp_cpp_read(nn->cpp, nfp_resource_cpp_id(res),
+				   nfp_resource_address(res),
+				   buffer, dump->len);
+		if (ret != dump->len)
+			ret = ret < 0 ? ret : -EIO;
+		else
+			ret = 0;
+	} else {
+		dump->len = nfp_resource_size(res);
+		ret = 0;
+	}
+exit_release:
+	nfp_resource_release(res);
+
+	return ret;
+}
+
+static int nfp_net_set_dump(struct net_device *netdev, struct ethtool_dump *val)
+{
+	struct nfp_net *nn = netdev_priv(netdev);
+
+	if (!nn->cpp)
+		return -EOPNOTSUPP;
+
+	if (val->flag != NFP_DUMP_NSP_DIAG)
+		return -EINVAL;
+
+	nn->ethtool_dump_flag = val->flag;
+
+	return 0;
+}
+
+static int
+nfp_net_get_dump_flag(struct net_device *netdev, struct ethtool_dump *dump)
+{
+	return nfp_dump_nsp_diag(netdev_priv(netdev), dump, NULL);
+}
+
+static int
+nfp_net_get_dump_data(struct net_device *netdev, struct ethtool_dump *dump,
+		      void *buffer)
+{
+	return nfp_dump_nsp_diag(netdev_priv(netdev), dump, buffer);
+}
+
 static int nfp_net_set_coalesce(struct net_device *netdev,
 				struct ethtool_coalesce *ec)
 {
@@ -743,6 +816,9 @@ static const struct ethtool_ops nfp_net_ethtool_ops = {
 	.set_rxfh		= nfp_net_set_rxfh,
 	.get_regs_len		= nfp_net_get_regs_len,
 	.get_regs		= nfp_net_get_regs,
+	.set_dump		= nfp_net_set_dump,
+	.get_dump_flag		= nfp_net_get_dump_flag,
+	.get_dump_data		= nfp_net_get_dump_data,
 	.get_coalesce           = nfp_net_get_coalesce,
 	.set_coalesce           = nfp_net_set_coalesce,
 	.get_channels		= nfp_net_get_channels,
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
index 540027973a74..42cb720b696d 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp.h
@@ -86,6 +86,7 @@ int nfp_nsp_write_eth_table(struct nfp_nsp *state,
 
 /* Service Processor */
 #define NFP_RESOURCE_NSP		"nfp.sp"
+#define NFP_RESOURCE_NSP_DIAG		"arm.diag"
 
 /* Netronone Flow Firmware Table */
 #define NFP_RESOURCE_NFP_NFFW		"nfp.nffw"
-- 
2.11.0

[PATCH net-next 6/7] nfp: return nfp_rtsym_read_le() errors correctly

[Jakub Kicinski]

nfp_rtsym_read_le() has an out parameter for error codes.
We have to use that instead of returning errors directly.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
index c659b1d999be..0e3870ecfb8c 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
@@ -269,8 +269,10 @@ u64 nfp_rtsym_read_le(struct nfp_cpp *cpp, const char *name, int *error)
 	int err;
 
 	sym = nfp_rtsym_lookup(cpp, name);
-	if (!sym)
-		return -ENOENT;
+	if (!sym) {
+		err = -ENOENT;
+		goto exit;
+	}
 
 	id = NFP_CPP_ISLAND_ID(sym->target, NFP_CPP_ACTION_RW, 0, sym->domain);
 
@@ -294,7 +296,7 @@ u64 nfp_rtsym_read_le(struct nfp_cpp *cpp, const char *name, int *error)
 		err = 0;
 	else if (err >= 0)
 		err = -EIO;
-
+exit:
 	if (error)
 		*error = err;
 
-- 
2.11.0

[PATCH net-next 7/7] nfp: allow application firmware to limit number of SR-IOV VFs

[Jakub Kicinski]

Some application firmware projects may choose to limit the number
of VFs available below what is specified in PCI capability to be
able to reuse the PCIe interface resources.  There may also be
projects which use cases don't require SR-IOV support at all and
therefore don't want to spend time implementing/testing it.

Check nfd_vf_cfg_max_vfs firmware symbol to see if application
firmware is reporting how many VFs it supports.  This mechanism
is an opt-in, if symbol is not present we will only look at the
PCI capability values.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_main.c | 23 +++++++++++++++++++++++
 drivers/net/ethernet/netronome/nfp/nfp_main.h |  2 ++
 2 files changed, 25 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index 8cda6b0e7e32..f73f3da1b1bc 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -47,6 +47,7 @@
 
 #include "nfpcore/nfp.h"
 #include "nfpcore/nfp_cpp.h"
+#include "nfpcore/nfp_nffw.h"
 #include "nfpcore/nfp_nsp_eth.h"
 
 #include "nfpcore/nfp6000_pcie.h"
@@ -70,12 +71,32 @@ static const struct pci_device_id nfp_pci_device_ids[] = {
 };
 MODULE_DEVICE_TABLE(pci, nfp_pci_device_ids);
 
+static void nfp_pcie_sriov_read_nfd_limit(struct nfp_pf *pf)
+{
+	int err;
+
+	pf->limit_vfs = nfp_rtsym_read_le(pf->cpp, "nfd_vf_cfg_max_vfs", &err);
+	if (!err)
+		return;
+
+	pf->limit_vfs = ~0;
+	/* Allow any setting for backwards compatibility if symbol not found */
+	if (err != -ENOENT)
+		nfp_warn(pf->cpp, "Warning: VF limit read failed: %d\n", err);
+}
+
 static int nfp_pcie_sriov_enable(struct pci_dev *pdev, int num_vfs)
 {
 #ifdef CONFIG_PCI_IOV
 	struct nfp_pf *pf = pci_get_drvdata(pdev);
 	int err;
 
+	if (num_vfs > pf->limit_vfs) {
+		nfp_info(pf->cpp, "Firmware limits number of VFs to %u\n",
+			 pf->limit_vfs);
+		return -EINVAL;
+	}
+
 	err = pci_enable_sriov(pdev, num_vfs);
 	if (err) {
 		dev_warn(&pdev->dev, "Failed to enable PCI sriov: %d\n", err);
@@ -333,6 +354,8 @@ static int nfp_pci_probe(struct pci_dev *pdev,
 	if (err)
 		goto err_cpp_free;
 
+	nfp_pcie_sriov_read_nfd_limit(pf);
+
 	err = nfp_net_pci_probe(pf);
 	if (err)
 		goto err_fw_unload;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.h b/drivers/net/ethernet/netronome/nfp/nfp_main.h
index 6c40fa322da3..39105d0435e9 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.h
@@ -59,6 +59,7 @@ struct nfp_eth_table;
  * @tx_area:		Pointer to the CPP area for the TX queues
  * @rx_area:		Pointer to the CPP area for the FL/RX queues
  * @irq_entries:	Array of MSI-X entries for all ports
+ * @limit_vfs:		Number of VFs supported by firmware (~0 for PCI limit)
  * @num_vfs:		Number of SR-IOV VFs enabled
  * @fw_loaded:		Is the firmware loaded?
  * @eth_tbl:		NSP ETH table
@@ -77,6 +78,7 @@ struct nfp_pf {
 
 	struct msix_entry *irq_entries;
 
+	unsigned int limit_vfs;
 	unsigned int num_vfs;
 
 	bool fw_loaded;
-- 
2.11.0

RE: [PATCH net-next 7/7] nfp: allow application firmware to limit number of SR-IOV VFs

[Mintz, Yuval]

> +static void nfp_pcie_sriov_read_nfd_limit(struct nfp_pf *pf)
> +{
> +	int err;
> +
> +	pf->limit_vfs = nfp_rtsym_read_le(pf->cpp, "nfd_vf_cfg_max_vfs",
> &err);
> +	if (!err)
> +		return;
> +
> +	pf->limit_vfs = ~0;
> +	/* Allow any setting for backwards compatibility if symbol not found
> */
> +	if (err != -ENOENT)
> +		nfp_warn(pf->cpp, "Warning: VF limit read failed: %d\n", err);
> +}
> +

If you're only going to use this under CONFIG_PCI_IOV,
perhaps put this underneath the ifdef as well?

>  static int nfp_pcie_sriov_enable(struct pci_dev *pdev, int num_vfs)
>  {
>  #ifdef CONFIG_PCI_IOV
>  	struct nfp_pf *pf = pci_get_drvdata(pdev);
>  	int err;
> 
> +	if (num_vfs > pf->limit_vfs) {
> +		nfp_info(pf->cpp, "Firmware limits number of VFs to %u\n",
> +			 pf->limit_vfs);
> +		return -EINVAL;
> +	}
> +
>  	err = pci_enable_sriov(pdev, num_vfs);
>  	if (err) {
>  		dev_warn(&pdev->dev, "Failed to enable PCI sriov: %d\n",
> err);
> @@ -333,6 +354,8 @@ static int nfp_pci_probe(struct pci_dev *pdev,
>  	if (err)
>  		goto err_cpp_free;
> 
> +	nfp_pcie_sriov_read_nfd_limit(pf);
> +
>  	err = nfp_net_pci_probe(pf);
>  	if (err)
>  		goto err_fw_unload;
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.h
> b/drivers/net/ethernet/netronome/nfp/nfp_main.h
> index 6c40fa322da3..39105d0435e9 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_main.h
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_main.h
> @@ -59,6 +59,7 @@ struct nfp_eth_table;
>   * @tx_area:		Pointer to the CPP area for the TX queues
>   * @rx_area:		Pointer to the CPP area for the FL/RX queues
>   * @irq_entries:	Array of MSI-X entries for all ports
> + * @limit_vfs:		Number of VFs supported by firmware (~0 for PCI
> limit)
>   * @num_vfs:		Number of SR-IOV VFs enabled
>   * @fw_loaded:		Is the firmware loaded?
>   * @eth_tbl:		NSP ETH table
> @@ -77,6 +78,7 @@ struct nfp_pf {
> 
>  	struct msix_entry *irq_entries;
> 
> +	unsigned int limit_vfs;
>  	unsigned int num_vfs;
> 
>  	bool fw_loaded;
> --
> 2.11.0

Re: [PATCH net-next 7/7] nfp: allow application firmware to limit number of SR-IOV VFs

[Jakub Kicinski]

On Sun, Feb 19, 2017 at 8:00 AM, Mintz, Yuval <Yuval.Mintz@cavium.com> wrote:
>> +static void nfp_pcie_sriov_read_nfd_limit(struct nfp_pf *pf)
>> +{
>> +     int err;
>> +
>> +     pf->limit_vfs = nfp_rtsym_read_le(pf->cpp, "nfd_vf_cfg_max_vfs",
>> &err);
>> +     if (!err)
>> +             return;
>> +
>> +     pf->limit_vfs = ~0;
>> +     /* Allow any setting for backwards compatibility if symbol not found
>> */
>> +     if (err != -ENOENT)
>> +             nfp_warn(pf->cpp, "Warning: VF limit read failed: %d\n", err);
>> +}
>> +
>
> If you're only going to use this under CONFIG_PCI_IOV,
> perhaps put this underneath the ifdef as well?

Sure, I have a slight preference to not use ifdefs unless necessary,
but I guess the body of this function could be under an ifdef without
making things too ugly..  Thanks :)  Testing almost done, v2 coming
shortly.