Date:   Thu,  8 Jun 2017 20:56:11 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     netdev@...r.kernel.org
Cc:     oss-drivers@...ronome.com,
        Jakub Kicinski <jakub.kicinski@...ronome.com>
Subject: [PATCH net-next 2/5] nfp: remove automatic caching of RTsym table

The fact that RTsym table is cached inside nfp_cpp handle is
a relic of old times when nfpcore was a library module.  All
the nfp_cpp "caches" are awkward to deal with because of
concurrency and prone to keeping stale information.  Make
the run time symbol table be an object read out from the device
and managed by whoever requested it.  Since the driver loads
FW at ->probe() and never reloads, we can hold onto the table
for ever.

Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_main.c      |   6 +-
 drivers/net/ethernet/netronome/nfp/nfp_main.h      |   3 +
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c  |   4 +-
 .../net/ethernet/netronome/nfp/nfpcore/nfp_cpp.h   |   4 -
 .../ethernet/netronome/nfp/nfpcore/nfp_cppcore.c   |  26 -----
 .../net/ethernet/netronome/nfp/nfpcore/nfp_nffw.h  |  13 ++-
 .../net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c   |   8 +-
 .../net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c | 111 ++++++++-------------
 8 files changed, 63 insertions(+), 112 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.c b/drivers/net/ethernet/netronome/nfp/nfp_main.c
index 0c2e64d217b5..51fe8de34b67 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.c
@@ -77,7 +77,7 @@ static int 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);
+	pf->limit_vfs = nfp_rtsym_read_le(pf->rtbl, "nfd_vf_cfg_max_vfs", &err);
 	if (!err)
 		return pci_sriov_set_totalvfs(pf->pdev, pf->limit_vfs);
 
@@ -373,6 +373,8 @@ static int nfp_pci_probe(struct pci_dev *pdev,
 	if (err)
 		goto err_devlink_unreg;
 
+	pf->rtbl = nfp_rtsym_table_read(pf->cpp);
+
 	err = nfp_pcie_sriov_read_nfd_limit(pf);
 	if (err)
 		goto err_fw_unload;
@@ -394,6 +396,7 @@ static int nfp_pci_probe(struct pci_dev *pdev,
 err_sriov_unlimit:
 	pci_sriov_set_totalvfs(pf->pdev, 0);
 err_fw_unload:
+	kfree(pf->rtbl);
 	if (pf->fw_loaded)
 		nfp_fw_unload(pf);
 	kfree(pf->eth_tbl);
@@ -430,6 +433,7 @@ static void nfp_pci_remove(struct pci_dev *pdev)
 
 	devlink_unregister(devlink);
 
+	kfree(pf->rtbl);
 	if (pf->fw_loaded)
 		nfp_fw_unload(pf);
 
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_main.h b/drivers/net/ethernet/netronome/nfp/nfp_main.h
index 37832853b0b3..907852f00423 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_main.h
+++ b/drivers/net/ethernet/netronome/nfp/nfp_main.h
@@ -56,6 +56,7 @@ struct nfp_cpp_area;
 struct nfp_eth_table;
 struct nfp_net;
 struct nfp_nsp_identify;
+struct nfp_rtsym_table;
 
 /**
  * struct nfp_pf - NFP PF-specific device structure
@@ -70,6 +71,7 @@ struct nfp_nsp_identify;
  * @num_vfs:		Number of SR-IOV VFs enabled
  * @fw_loaded:		Is the firmware loaded?
  * @ctrl_vnic:		Pointer to the control vNIC if available
+ * @rtbl:		RTsym table
  * @eth_tbl:		NSP ETH table
  * @nspi:		NSP identification info
  * @hwmon_dev:		pointer to hwmon device
@@ -101,6 +103,7 @@ struct nfp_pf {
 
 	struct nfp_net *ctrl_vnic;
 
+	struct nfp_rtsym_table *rtbl;
 	struct nfp_eth_table *eth_tbl;
 	struct nfp_nsp_identify *nspi;
 
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index cdd25dc5988d..c845049fcff2 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -201,7 +201,7 @@ nfp_net_pf_rtsym_read_optional(struct nfp_pf *pf, const char *format,
 
 	snprintf(name, sizeof(name), format, nfp_cppcore_pcie_unit(pf->cpp));
 
-	val = nfp_rtsym_read_le(pf->cpp, name, &err);
+	val = nfp_rtsym_read_le(pf->rtbl, name, &err);
 	if (err) {
 		if (err == -ENOENT)
 			return default_val;
@@ -234,7 +234,7 @@ nfp_net_pf_map_rtsym(struct nfp_pf *pf, const char *name, const char *sym_fmt,
 	snprintf(pf_symbol, sizeof(pf_symbol), sym_fmt,
 		 nfp_cppcore_pcie_unit(pf->cpp));
 
-	sym = nfp_rtsym_lookup(pf->cpp, pf_symbol);
+	sym = nfp_rtsym_lookup(pf->rtbl, pf_symbol);
 	if (!sym) {
 		nfp_err(pf->cpp, "Failed to find PF symbol %s\n", pf_symbol);
 		return (u8 __iomem *)ERR_PTR(-ENOENT);
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cpp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cpp.h
index 0a46c0984e68..e3a2201eb658 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cpp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cpp.h
@@ -224,10 +224,6 @@ int nfp_cpp_serial(struct nfp_cpp *cpp, const u8 **serial);
 
 void *nfp_hwinfo_cache(struct nfp_cpp *cpp);
 void nfp_hwinfo_cache_set(struct nfp_cpp *cpp, void *val);
-void *nfp_rtsym_cache(struct nfp_cpp *cpp);
-void nfp_rtsym_cache_set(struct nfp_cpp *cpp, void *val);
-
-void nfp_nffw_cache_flush(struct nfp_cpp *cpp);
 
 struct nfp_cpp_area *nfp_cpp_area_alloc_with_name(struct nfp_cpp *cpp,
 						  u32 cpp_id,
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cppcore.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cppcore.c
index 5672d309d07d..f477186558bd 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cppcore.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_cppcore.c
@@ -78,7 +78,6 @@ struct nfp_cpp_resource {
  *
  * Following fields can be used only in probe() or with rtnl held:
  * @hwinfo:		HWInfo database fetched from the device
- * @rtsym:		firmware run time symbols
  *
  * Following fields use explicit locking:
  * @resource_list:	NFP CPP resource list
@@ -109,7 +108,6 @@ struct nfp_cpp {
 	struct list_head area_cache_list;
 
 	void *hwinfo;
-	void *rtsym;
 };
 
 /* Element of the area_cache_list */
@@ -234,7 +232,6 @@ void nfp_cpp_free(struct nfp_cpp *cpp)
 		cpp->op->free(cpp);
 
 	kfree(cpp->hwinfo);
-	kfree(cpp->rtsym);
 
 	device_unregister(&cpp->dev);
 
@@ -286,29 +283,6 @@ void nfp_hwinfo_cache_set(struct nfp_cpp *cpp, void *val)
 	cpp->hwinfo = val;
 }
 
-void *nfp_rtsym_cache(struct nfp_cpp *cpp)
-{
-	return cpp->rtsym;
-}
-
-void nfp_rtsym_cache_set(struct nfp_cpp *cpp, void *val)
-{
-	cpp->rtsym = val;
-}
-
-/**
- * nfp_nffw_cache_flush() - Flush cached firmware information
- * @cpp:	NFP CPP handle
- *
- * Flush cached firmware information.  This function should be called
- * every time firmware is loaded on unloaded.
- */
-void nfp_nffw_cache_flush(struct nfp_cpp *cpp)
-{
-	kfree(nfp_rtsym_cache(cpp));
-	nfp_rtsym_cache_set(cpp, NULL);
-}
-
 /**
  * nfp_cpp_area_alloc_with_name() - allocate a new CPP area
  * @cpp:	CPP device handle
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nffw.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nffw.h
index 988badd230d1..f845cf5dd762 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nffw.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nffw.h
@@ -87,9 +87,14 @@ struct nfp_rtsym {
 	int domain;
 };
 
-int nfp_rtsym_count(struct nfp_cpp *cpp);
-const struct nfp_rtsym *nfp_rtsym_get(struct nfp_cpp *cpp, int idx);
-const struct nfp_rtsym *nfp_rtsym_lookup(struct nfp_cpp *cpp, const char *name);
-u64 nfp_rtsym_read_le(struct nfp_cpp *cpp, const char *name, int *error);
+struct nfp_rtsym_table;
+
+struct nfp_rtsym_table *nfp_rtsym_table_read(struct nfp_cpp *cpp);
+int nfp_rtsym_count(struct nfp_rtsym_table *rtbl);
+const struct nfp_rtsym *nfp_rtsym_get(struct nfp_rtsym_table *rtbl, int idx);
+const struct nfp_rtsym *
+nfp_rtsym_lookup(struct nfp_rtsym_table *rtbl, const char *name);
+u64 nfp_rtsym_read_le(struct nfp_rtsym_table *rtbl, const char *name,
+		      int *error);
 
 #endif /* NFP_NFFW_H */
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
index eefdb756d74e..37364555c42b 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
@@ -474,13 +474,7 @@ int nfp_nsp_wait(struct nfp_nsp *state)
 
 int nfp_nsp_device_soft_reset(struct nfp_nsp *state)
 {
-	int err;
-
-	err = nfp_nsp_command(state, SPCODE_SOFT_RESET, 0, 0, 0);
-
-	nfp_nffw_cache_flush(state->cpp);
-
-	return err;
+	return nfp_nsp_command(state, SPCODE_SOFT_RESET, 0, 0, 0);
 }
 
 int nfp_nsp_load_fw(struct nfp_nsp *state, const struct firmware *fw)
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
index 0e3870ecfb8c..ef3566163cb0 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_rtsym.c
@@ -65,7 +65,8 @@ struct nfp_rtsym_entry {
 	__le32	size_lo;
 };
 
-struct nfp_rtsym_cache {
+struct nfp_rtsym_table {
+	struct nfp_cpp *cpp;
 	int num;
 	char *strtab;
 	struct nfp_rtsym symtab[];
@@ -78,7 +79,7 @@ static int nfp_meid(u8 island_id, u8 menum)
 }
 
 static void
-nfp_rtsym_sw_entry_init(struct nfp_rtsym_cache *cache, u32 strtab_size,
+nfp_rtsym_sw_entry_init(struct nfp_rtsym_table *cache, u32 strtab_size,
 			struct nfp_rtsym *sw, struct nfp_rtsym_entry *fw)
 {
 	sw->type = fw->type;
@@ -106,26 +107,26 @@ nfp_rtsym_sw_entry_init(struct nfp_rtsym_cache *cache, u32 strtab_size,
 		sw->domain = -1;
 }
 
-static int nfp_rtsymtab_probe(struct nfp_cpp *cpp)
+struct nfp_rtsym_table *nfp_rtsym_table_read(struct nfp_cpp *cpp)
 {
 	const u32 dram = NFP_CPP_ID(NFP_CPP_TARGET_MU, NFP_CPP_ACTION_RW, 0) |
 		NFP_ISL_EMEM0;
 	u32 strtab_addr, symtab_addr, strtab_size, symtab_size;
 	struct nfp_rtsym_entry *rtsymtab;
-	struct nfp_rtsym_cache *cache;
+	struct nfp_rtsym_table *cache;
 	const struct nfp_mip *mip;
 	int err, n, size;
 
 	mip = nfp_mip_open(cpp);
 	if (!mip)
-		return -EIO;
+		return NULL;
 
 	nfp_mip_strtab(mip, &strtab_addr, &strtab_size);
 	nfp_mip_symtab(mip, &symtab_addr, &symtab_size);
 	nfp_mip_close(mip);
 
 	if (!symtab_size || !strtab_size || symtab_size % sizeof(*rtsymtab))
-		return -ENXIO;
+		return NULL;
 
 	/* Align to 64 bits */
 	symtab_size = round_up(symtab_size, 8);
@@ -133,27 +134,26 @@ static int nfp_rtsymtab_probe(struct nfp_cpp *cpp)
 
 	rtsymtab = kmalloc(symtab_size, GFP_KERNEL);
 	if (!rtsymtab)
-		return -ENOMEM;
+		return NULL;
 
 	size = sizeof(*cache);
 	size += symtab_size / sizeof(*rtsymtab) * sizeof(struct nfp_rtsym);
 	size +=	strtab_size + 1;
 	cache = kmalloc(size, GFP_KERNEL);
-	if (!cache) {
-		err = -ENOMEM;
-		goto err_free_rtsym_raw;
-	}
+	if (!cache)
+		goto exit_free_rtsym_raw;
 
+	cache->cpp = cpp;
 	cache->num = symtab_size / sizeof(*rtsymtab);
 	cache->strtab = (void *)&cache->symtab[cache->num];
 
 	err = nfp_cpp_read(cpp, dram, symtab_addr, rtsymtab, symtab_size);
 	if (err != symtab_size)
-		goto err_free_cache;
+		goto exit_free_cache;
 
 	err = nfp_cpp_read(cpp, dram, strtab_addr, cache->strtab, strtab_size);
 	if (err != strtab_size)
-		goto err_free_cache;
+		goto exit_free_cache;
 	cache->strtab[strtab_size] = '\0';
 
 	for (n = 0; n < cache->num; n++)
@@ -161,97 +161,71 @@ static int nfp_rtsymtab_probe(struct nfp_cpp *cpp)
 					&cache->symtab[n], &rtsymtab[n]);
 
 	kfree(rtsymtab);
-	nfp_rtsym_cache_set(cpp, cache);
-	return 0;
 
-err_free_cache:
+	return cache;
+
+exit_free_cache:
 	kfree(cache);
-err_free_rtsym_raw:
+exit_free_rtsym_raw:
 	kfree(rtsymtab);
-	return err;
-}
-
-static struct nfp_rtsym_cache *nfp_rtsym(struct nfp_cpp *cpp)
-{
-	struct nfp_rtsym_cache *cache;
-	int err;
-
-	cache = nfp_rtsym_cache(cpp);
-	if (cache)
-		return cache;
-
-	err = nfp_rtsymtab_probe(cpp);
-	if (err < 0)
-		return ERR_PTR(err);
-
-	return nfp_rtsym_cache(cpp);
+	return NULL;
 }
 
 /**
  * nfp_rtsym_count() - Get the number of RTSYM descriptors
- * @cpp:	NFP CPP handle
+ * @rtbl:	NFP RTsym table
  *
- * Return: Number of RTSYM descriptors, or -ERRNO
+ * Return: Number of RTSYM descriptors
  */
-int nfp_rtsym_count(struct nfp_cpp *cpp)
+int nfp_rtsym_count(struct nfp_rtsym_table *rtbl)
 {
-	struct nfp_rtsym_cache *cache;
-
-	cache = nfp_rtsym(cpp);
-	if (IS_ERR(cache))
-		return PTR_ERR(cache);
-
-	return cache->num;
+	if (!rtbl)
+		return -EINVAL;
+	return rtbl->num;
 }
 
 /**
  * nfp_rtsym_get() - Get the Nth RTSYM descriptor
- * @cpp:	NFP CPP handle
+ * @rtbl:	NFP RTsym table
  * @idx:	Index (0-based) of the RTSYM descriptor
  *
  * Return: const pointer to a struct nfp_rtsym descriptor, or NULL
  */
-const struct nfp_rtsym *nfp_rtsym_get(struct nfp_cpp *cpp, int idx)
+const struct nfp_rtsym *nfp_rtsym_get(struct nfp_rtsym_table *rtbl, int idx)
 {
-	struct nfp_rtsym_cache *cache;
-
-	cache = nfp_rtsym(cpp);
-	if (IS_ERR(cache))
+	if (!rtbl)
 		return NULL;
-
-	if (idx >= cache->num)
+	if (idx >= rtbl->num)
 		return NULL;
 
-	return &cache->symtab[idx];
+	return &rtbl->symtab[idx];
 }
 
 /**
  * nfp_rtsym_lookup() - Return the RTSYM descriptor for a symbol name
- * @cpp:	NFP CPP handle
+ * @rtbl:	NFP RTsym table
  * @name:	Symbol name
  *
  * Return: const pointer to a struct nfp_rtsym descriptor, or NULL
  */
-const struct nfp_rtsym *nfp_rtsym_lookup(struct nfp_cpp *cpp, const char *name)
+const struct nfp_rtsym *
+nfp_rtsym_lookup(struct nfp_rtsym_table *rtbl, const char *name)
 {
-	struct nfp_rtsym_cache *cache;
 	int n;
 
-	cache = nfp_rtsym(cpp);
-	if (IS_ERR(cache))
+	if (!rtbl)
 		return NULL;
 
-	for (n = 0; n < cache->num; n++) {
-		if (strcmp(name, cache->symtab[n].name) == 0)
-			return &cache->symtab[n];
-	}
+	for (n = 0; n < rtbl->num; n++)
+		if (strcmp(name, rtbl->symtab[n].name) == 0)
+			return &rtbl->symtab[n];
 
 	return NULL;
 }
 
 /**
  * nfp_rtsym_read_le() - Read a simple unsigned scalar value from symbol
- * @cpp:	NFP CPP handle
+ * @rtbl:	NFP RTsym table
  * @name:	Symbol name
  * @error:	Poniter to error code (optional)
  *
@@ -261,14 +235,15 @@ const struct nfp_rtsym *nfp_rtsym_lookup(struct nfp_cpp *cpp, const char *name)
  *
  * Return: value read, on error sets the error and returns ~0ULL.
  */
-u64 nfp_rtsym_read_le(struct nfp_cpp *cpp, const char *name, int *error)
+u64 nfp_rtsym_read_le(struct nfp_rtsym_table *rtbl, const char *name,
+		      int *error)
 {
 	const struct nfp_rtsym *sym;
 	u32 val32, id;
 	u64 val;
 	int err;
 
-	sym = nfp_rtsym_lookup(cpp, name);
+	sym = nfp_rtsym_lookup(rtbl, name);
 	if (!sym) {
 		err = -ENOENT;
 		goto exit;
@@ -278,14 +253,14 @@ u64 nfp_rtsym_read_le(struct nfp_cpp *cpp, const char *name, int *error)
 
 	switch (sym->size) {
 	case 4:
-		err = nfp_cpp_readl(cpp, id, sym->addr, &val32);
+		err = nfp_cpp_readl(rtbl->cpp, id, sym->addr, &val32);
 		val = val32;
 		break;
 	case 8:
-		err = nfp_cpp_readq(cpp, id, sym->addr, &val);
+		err = nfp_cpp_readq(rtbl->cpp, id, sym->addr, &val);
 		break;
 	default:
-		nfp_err(cpp,
+		nfp_err(rtbl->cpp,
 			"rtsym '%s' unsupported or non-scalar size: %lld\n",
 			name, sym->size);
 		err = -EINVAL;
-- 
2.11.0

Powered by blists - more mailing lists