[v2,4/4] crypto/ccp: fix PCI probing

Message ID 20221004095132.198777-5-david.marchand@redhat.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series crypto/ccp cleanup |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS

Commit Message

David Marchand Oct. 4, 2022, 9:51 a.m. UTC
  This driver has been converted from a vdev driver to a pci driver some
time ago.  This conversion is buggy as it tries to probe any pci devices
present on a system for *each* probe request from the PCI bus.

Rely on the passed pci device and only probe what is requested.

While at it:
- stop copying the pci device object content into a local private copy,
- rely on the PCI identifier and remove internal ccp_device_version
  identifier,
- ccp_list can be made static,

With this done, all the code parsing Linux sysfs can be dropped.

Fixes: 889317b7ecb3 ("crypto/ccp: convert driver from vdev to PCI")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ccp/ccp_crypto.c  |   1 -
 drivers/crypto/ccp/ccp_dev.c     |  89 ++-----------
 drivers/crypto/ccp/ccp_dev.h     |  31 ++---
 drivers/crypto/ccp/ccp_pci.c     | 207 -------------------------------
 drivers/crypto/ccp/ccp_pci.h     |  24 ----
 drivers/crypto/ccp/meson.build   |   1 -
 drivers/crypto/ccp/rte_ccp_pmd.c |  18 +--
 7 files changed, 26 insertions(+), 345 deletions(-)
 delete mode 100644 drivers/crypto/ccp/ccp_pci.c
 delete mode 100644 drivers/crypto/ccp/ccp_pci.h
  

Patch

diff --git a/drivers/crypto/ccp/ccp_crypto.c b/drivers/crypto/ccp/ccp_crypto.c
index 351d8ac63e..461f18ca2e 100644
--- a/drivers/crypto/ccp/ccp_crypto.c
+++ b/drivers/crypto/ccp/ccp_crypto.c
@@ -26,7 +26,6 @@ 
 
 #include "ccp_dev.h"
 #include "ccp_crypto.h"
-#include "ccp_pci.h"
 #include "ccp_pmd_private.h"
 
 #include <openssl/conf.h>
diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c
index 14c54929c4..ee30f5ac30 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -20,10 +20,9 @@ 
 #include <rte_string_fns.h>
 
 #include "ccp_dev.h"
-#include "ccp_pci.h"
 #include "ccp_pmd_private.h"
 
-struct ccp_list ccp_list = TAILQ_HEAD_INITIALIZER(ccp_list);
+static TAILQ_HEAD(, ccp_device) ccp_list = TAILQ_HEAD_INITIALIZER(ccp_list);
 static int ccp_dev_id;
 
 int
@@ -68,7 +67,7 @@  ccp_read_hwrng(uint32_t *value)
 	struct ccp_device *dev;
 
 	TAILQ_FOREACH(dev, &ccp_list, next) {
-		void *vaddr = (void *)(dev->pci.mem_resource[2].addr);
+		void *vaddr = (void *)(dev->pci->mem_resource[2].addr);
 
 		while (dev->hwrng_retries++ < CCP_MAX_TRNG_RETRIES) {
 			*value = CCP_READ_REG(vaddr, TRNG_OUT_REG);
@@ -480,7 +479,7 @@  ccp_assign_lsbs(struct ccp_device *ccp)
 }
 
 static int
-ccp_add_device(struct ccp_device *dev, int type)
+ccp_add_device(struct ccp_device *dev)
 {
 	int i;
 	uint32_t qmr, status_lo, status_hi, dma_addr_lo, dma_addr_hi;
@@ -494,9 +493,9 @@  ccp_add_device(struct ccp_device *dev, int type)
 
 	dev->id = ccp_dev_id++;
 	dev->qidx = 0;
-	vaddr = (void *)(dev->pci.mem_resource[2].addr);
+	vaddr = (void *)(dev->pci->mem_resource[2].addr);
 
-	if (type == CCP_VERSION_5B) {
+	if (dev->pci->id.device_id == AMD_PCI_CCP_5B) {
 		CCP_WRITE_REG(vaddr, CMD_TRNG_CTL_OFFSET, 0x00012D57);
 		CCP_WRITE_REG(vaddr, CMD_CONFIG_0_OFFSET, 0x00000003);
 		for (i = 0; i < 12; i++) {
@@ -615,41 +614,8 @@  ccp_remove_device(struct ccp_device *dev)
 	TAILQ_REMOVE(&ccp_list, dev, next);
 }
 
-static int
-is_ccp_device(const char *dirname,
-	      const struct rte_pci_id *ccp_id,
-	      int *type)
-{
-	char filename[PATH_MAX];
-	const struct rte_pci_id *id;
-	uint16_t vendor, device_id;
-	int i;
-	unsigned long tmp;
-
-	/* get vendor id */
-	snprintf(filename, sizeof(filename), "%s/vendor", dirname);
-	if (ccp_pci_parse_sysfs_value(filename, &tmp) < 0)
-		return 0;
-	vendor = (uint16_t)tmp;
-
-	/* get device id */
-	snprintf(filename, sizeof(filename), "%s/device", dirname);
-	if (ccp_pci_parse_sysfs_value(filename, &tmp) < 0)
-		return 0;
-	device_id = (uint16_t)tmp;
-
-	for (id = ccp_id, i = 0; id->vendor_id != 0; id++, i++) {
-		if (vendor == id->vendor_id &&
-		    device_id == id->device_id) {
-			*type = i;
-			return 1; /* Matched device */
-		}
-	}
-	return 0;
-}
-
-static int
-ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)
+int
+ccp_probe_device(struct rte_pci_device *pci_dev)
 {
 	struct ccp_device *ccp_dev;
 
@@ -658,10 +624,10 @@  ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)
 	if (ccp_dev == NULL)
 		goto fail;
 
-	ccp_dev->pci = *pci_dev;
+	ccp_dev->pci = pci_dev;
 
 	/* device is valid, add in list */
-	if (ccp_add_device(ccp_dev, ccp_type)) {
+	if (ccp_add_device(ccp_dev)) {
 		ccp_remove_device(ccp_dev);
 		goto fail;
 	}
@@ -672,40 +638,3 @@  ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)
 	rte_free(ccp_dev);
 	return -1;
 }
-
-int
-ccp_probe_devices(struct rte_pci_device *pci_dev,
-		const struct rte_pci_id *ccp_id)
-{
-	int dev_cnt = 0;
-	int ccp_type = 0;
-	struct dirent *d;
-	DIR *dir;
-	int ret = 0;
-	uint16_t domain;
-	uint8_t bus, devid, function;
-	char dirname[PATH_MAX];
-
-	TAILQ_INIT(&ccp_list);
-	dir = opendir(SYSFS_PCI_DEVICES);
-	if (dir == NULL)
-		return -1;
-	while ((d = readdir(dir)) != NULL) {
-		if (d->d_name[0] == '.')
-			continue;
-		if (ccp_parse_pci_addr_format(d->d_name, sizeof(d->d_name),
-					&domain, &bus, &devid, &function) != 0)
-			continue;
-		snprintf(dirname, sizeof(dirname), "%s/%s",
-			     SYSFS_PCI_DEVICES, d->d_name);
-		if (is_ccp_device(dirname, ccp_id, &ccp_type)) {
-			CCP_LOG_DBG("CCP : Detected CCP device with ID = 0x%x\n",
-			       ccp_id[ccp_type].device_id);
-			ret = ccp_probe_device(ccp_type, pci_dev);
-			if (ret == 0)
-				dev_cnt++;
-		}
-	}
-	closedir(dir);
-	return dev_cnt;
-}
diff --git a/drivers/crypto/ccp/ccp_dev.h b/drivers/crypto/ccp/ccp_dev.h
index 9deaae7980..e3ec481dd3 100644
--- a/drivers/crypto/ccp/ccp_dev.h
+++ b/drivers/crypto/ccp/ccp_dev.h
@@ -19,6 +19,12 @@ 
 #include <rte_crypto_sym.h>
 #include <cryptodev_pmd.h>
 
+/* CCP PCI device identifiers */
+#define AMD_PCI_VENDOR_ID 0x1022
+#define AMD_PCI_CCP_5A 0x1456
+#define AMD_PCI_CCP_5B 0x1468
+#define AMD_PCI_CCP_RV 0x15df
+
 /**< CCP specific */
 #define MAX_HW_QUEUES                   5
 #define CCP_MAX_TRNG_RETRIES		10
@@ -169,18 +175,6 @@  static inline uint32_t ccp_pci_reg_read(void *base, int offset)
 #define CCP_WRITE_REG(hw_addr, reg_offset, value) \
 	ccp_pci_reg_write(hw_addr, reg_offset, value)
 
-TAILQ_HEAD(ccp_list, ccp_device);
-
-extern struct ccp_list ccp_list;
-
-/**
- * CCP device version
- */
-enum ccp_device_version {
-	CCP_VERSION_5A = 0,
-	CCP_VERSION_5B,
-};
-
 /**
  * A structure describing a CCP command queue.
  */
@@ -233,8 +227,8 @@  struct ccp_device {
 	/**< ccp queue */
 	int cmd_q_count;
 	/**< no. of ccp Queues */
-	struct rte_pci_device pci;
-	/**< ccp pci identifier */
+	struct rte_pci_device *pci;
+	/**< ccp pci device */
 	unsigned long lsbmap[CCP_BITMAP_SIZE(SLSB_MAP_SIZE)];
 	/**< shared lsb mask of ccp */
 	rte_spinlock_t lsb_lock;
@@ -468,13 +462,12 @@  high32_value(unsigned long addr)
 int ccp_dev_start(struct rte_cryptodev *dev);
 
 /**
- * Detect ccp platform and initialize all ccp devices
+ * Initialize one ccp device
  *
- * @param ccp_id rte_pci_id list for supported CCP devices
- * @return no. of successfully initialized CCP devices
+ * @dev rte pci device
+ * @return 0 on success otherwise -1
  */
-int ccp_probe_devices(struct rte_pci_device *pci_dev,
-		const struct rte_pci_id *ccp_id);
+int ccp_probe_device(struct rte_pci_device *pci_dev);
 
 /**
  * allocate a ccp command queue
diff --git a/drivers/crypto/ccp/ccp_pci.c b/drivers/crypto/ccp/ccp_pci.c
deleted file mode 100644
index bd1a037f76..0000000000
--- a/drivers/crypto/ccp/ccp_pci.c
+++ /dev/null
@@ -1,207 +0,0 @@ 
-/*   SPDX-License-Identifier: BSD-3-Clause
- *   Copyright(c) 2018 Advanced Micro Devices, Inc. All rights reserved.
- */
-
-#include <dirent.h>
-#include <fcntl.h>
-#include <stdio.h>
-#include <string.h>
-#include <unistd.h>
-
-#include <rte_string_fns.h>
-
-#include "ccp_pci.h"
-
-/*
- * split up a pci address into its constituent parts.
- */
-int
-ccp_parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
-			  uint8_t *bus, uint8_t *devid, uint8_t *function)
-{
-	/* first split on ':' */
-	union splitaddr {
-		struct {
-			char *domain;
-			char *bus;
-			char *devid;
-			char *function;
-		};
-		char *str[PCI_FMT_NVAL];
-		/* last element-separator is "." not ":" */
-	} splitaddr;
-
-	char *buf_copy = strndup(buf, bufsize);
-
-	if (buf_copy == NULL)
-		return -1;
-
-	if (rte_strsplit(buf_copy, bufsize, splitaddr.str, PCI_FMT_NVAL, ':')
-			!= PCI_FMT_NVAL - 1)
-		goto error;
-	/* final split is on '.' between devid and function */
-	splitaddr.function = strchr(splitaddr.devid, '.');
-	if (splitaddr.function == NULL)
-		goto error;
-	*splitaddr.function++ = '\0';
-
-	/* now convert to int values */
-	errno = 0;
-	*domain = (uint8_t)strtoul(splitaddr.domain, NULL, 16);
-	*bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
-	*devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
-	*function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
-	if (errno != 0)
-		goto error;
-
-	free(buf_copy); /* free the copy made with strdup */
-	return 0;
-error:
-	free(buf_copy);
-	return -1;
-}
-
-int
-ccp_pci_parse_sysfs_value(const char *filename, unsigned long *val)
-{
-	FILE *f;
-	char buf[BUFSIZ];
-	char *end = NULL;
-
-	f = fopen(filename, "r");
-	if (f == NULL)
-		return -1;
-	if (fgets(buf, sizeof(buf), f) == NULL) {
-		fclose(f);
-		return -1;
-	}
-	*val = strtoul(buf, &end, 0);
-	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
-		fclose(f);
-		return -1;
-	}
-	fclose(f);
-	return 0;
-}
-
-/** IO resource type: */
-#define IORESOURCE_IO         0x00000100
-#define IORESOURCE_MEM        0x00000200
-
-/* parse one line of the "resource" sysfs file (note that the 'line'
- * string is modified)
- */
-static int
-ccp_pci_parse_one_sysfs_resource(char *line, size_t len, uint64_t *phys_addr,
-				 uint64_t *end_addr, uint64_t *flags)
-{
-	union pci_resource_info {
-		struct {
-			char *phys_addr;
-			char *end_addr;
-			char *flags;
-		};
-		char *ptrs[PCI_RESOURCE_FMT_NVAL];
-	} res_info;
-
-	if (rte_strsplit(line, len, res_info.ptrs, 3, ' ') != 3)
-		return -1;
-	errno = 0;
-	*phys_addr = strtoull(res_info.phys_addr, NULL, 16);
-	*end_addr = strtoull(res_info.end_addr, NULL, 16);
-	*flags = strtoull(res_info.flags, NULL, 16);
-	if (errno != 0)
-		return -1;
-
-	return 0;
-}
-
-/* parse the "resource" sysfs file */
-int
-ccp_pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
-{
-	FILE *fp;
-	char buf[BUFSIZ];
-	int i;
-	uint64_t phys_addr, end_addr, flags;
-
-	fp = fopen(filename, "r");
-	if (fp == NULL)
-		return -1;
-
-	for (i = 0; i < PCI_MAX_RESOURCE; i++) {
-		if (fgets(buf, sizeof(buf), fp) == NULL)
-			goto error;
-		if (ccp_pci_parse_one_sysfs_resource(buf, sizeof(buf),
-				&phys_addr, &end_addr, &flags) < 0)
-			goto error;
-
-		if (flags & IORESOURCE_MEM) {
-			dev->mem_resource[i].phys_addr = phys_addr;
-			dev->mem_resource[i].len = end_addr - phys_addr + 1;
-			/* not mapped for now */
-			dev->mem_resource[i].addr = NULL;
-		}
-	}
-	fclose(fp);
-	return 0;
-
-error:
-	fclose(fp);
-	return -1;
-}
-
-int
-ccp_find_uio_devname(const char *dirname)
-{
-
-	DIR *dir;
-	struct dirent *e;
-	char dirname_uio[PATH_MAX];
-	unsigned int uio_num;
-	int ret = -1;
-
-	/* depending on kernel version, uio can be located in uio/uioX
-	 * or uio:uioX
-	 */
-	snprintf(dirname_uio, sizeof(dirname_uio), "%s/uio", dirname);
-	dir = opendir(dirname_uio);
-	if (dir == NULL) {
-	/* retry with the parent directory might be different kernel version*/
-		dir = opendir(dirname);
-		if (dir == NULL)
-			return -1;
-	}
-
-	/* take the first file starting with "uio" */
-	while ((e = readdir(dir)) != NULL) {
-		/* format could be uio%d ...*/
-		int shortprefix_len = sizeof("uio") - 1;
-		/* ... or uio:uio%d */
-		int longprefix_len = sizeof("uio:uio") - 1;
-		char *endptr;
-
-		if (strncmp(e->d_name, "uio", 3) != 0)
-			continue;
-
-		/* first try uio%d */
-		errno = 0;
-		uio_num = strtoull(e->d_name + shortprefix_len, &endptr, 10);
-		if (errno == 0 && endptr != (e->d_name + shortprefix_len)) {
-			ret = uio_num;
-			break;
-		}
-
-		/* then try uio:uio%d */
-		errno = 0;
-		uio_num = strtoull(e->d_name + longprefix_len, &endptr, 10);
-		if (errno == 0 && endptr != (e->d_name + longprefix_len)) {
-			ret = uio_num;
-			break;
-		}
-	}
-	closedir(dir);
-	return ret;
-
-
-}
diff --git a/drivers/crypto/ccp/ccp_pci.h b/drivers/crypto/ccp/ccp_pci.h
deleted file mode 100644
index d9a8b9dcc6..0000000000
--- a/drivers/crypto/ccp/ccp_pci.h
+++ /dev/null
@@ -1,24 +0,0 @@ 
-/*   SPDX-License-Identifier: BSD-3-Clause
- *   Copyright(c) 2018 Advanced Micro Devices, Inc. All rights reserved.
- */
-
-#ifndef _CCP_PCI_H_
-#define _CCP_PCI_H_
-
-#include <stdint.h>
-
-#include <bus_pci_driver.h>
-
-#define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
-
-int ccp_parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
-			      uint8_t *bus, uint8_t *devid, uint8_t *function);
-
-int ccp_pci_parse_sysfs_value(const char *filename, unsigned long *val);
-
-int ccp_pci_parse_sysfs_resource(const char *filename,
-				 struct rte_pci_device *dev);
-
-int ccp_find_uio_devname(const char *dirname);
-
-#endif /* _CCP_PCI_H_ */
diff --git a/drivers/crypto/ccp/meson.build b/drivers/crypto/ccp/meson.build
index a4f3406009..a9abaa4da0 100644
--- a/drivers/crypto/ccp/meson.build
+++ b/drivers/crypto/ccp/meson.build
@@ -18,7 +18,6 @@  sources = files(
         'rte_ccp_pmd.c',
         'ccp_crypto.c',
         'ccp_dev.c',
-        'ccp_pci.c',
         'ccp_pmd_ops.c',
 )
 
diff --git a/drivers/crypto/ccp/rte_ccp_pmd.c b/drivers/crypto/ccp/rte_ccp_pmd.c
index 8b3a5a304b..ac05d63ade 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -180,15 +180,9 @@  ccp_pmd_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
  * The set of PCI devices this driver supports
  */
 static struct rte_pci_id ccp_pci_id[] = {
-	{
-		RTE_PCI_DEVICE(0x1022, 0x1456), /* AMD CCP-5a */
-	},
-	{
-		RTE_PCI_DEVICE(0x1022, 0x1468), /* AMD CCP-5b */
-	},
-	{
-		RTE_PCI_DEVICE(0x1022, 0x15df), /* AMD CCP RV */
-	},
+	{ RTE_PCI_DEVICE(AMD_PCI_VENDOR_ID, AMD_PCI_CCP_5A), },
+	{ RTE_PCI_DEVICE(AMD_PCI_VENDOR_ID, AMD_PCI_CCP_5B), },
+	{ RTE_PCI_DEVICE(AMD_PCI_VENDOR_ID, AMD_PCI_CCP_RV), },
 	{.device_id = 0},
 };
 
@@ -241,9 +235,7 @@  cryptodev_ccp_create(const char *name,
 		goto init_error;
 	}
 
-	cryptodev_cnt = ccp_probe_devices(pci_dev, ccp_pci_id);
-
-	if (cryptodev_cnt == 0) {
+	if (ccp_probe_device(pci_dev) != 0) {
 		CCP_LOG_ERR("failed to detect CCP crypto device");
 		goto init_error;
 	}
@@ -267,7 +259,7 @@  cryptodev_ccp_create(const char *name,
 
 	internals->max_nb_qpairs = init_params->def_p.max_nb_queue_pairs;
 	internals->auth_opt = init_params->auth_opt;
-	internals->crypto_num_dev = cryptodev_cnt;
+	internals->crypto_num_dev = 1;
 
 	rte_cryptodev_pmd_probing_finish(dev);