DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 0/4] crypto/ccp cleanup
@ 2022-09-09 15:04 David Marchand
  2022-09-09 15:04 ` [PATCH 1/4] crypto/ccp: remove some printf David Marchand
                   ` (5 more replies)
  0 siblings, 6 replies; 28+ messages in thread
From: David Marchand @ 2022-09-09 15:04 UTC (permalink / raw)
  To: dev; +Cc: gakhil, chandu

This is a *untested* cleanup series after looking for usage of
rte_pci_device objects in DPDK drivers.
I can't test those patches by lack of hw, so I hope the driver maintainer
can look into them.

Thanks.
-- 
David Marchand

David Marchand (4):
  crypto/ccp: remove some printf
  crypto/ccp: remove some dead code for UIO
  crypto/ccp: fix IOVA handling
  crypto/ccp: fix PCI probing

 drivers/crypto/ccp/ccp_crypto.c  | 106 +++-----------
 drivers/crypto/ccp/ccp_dev.c     | 103 ++-----------
 drivers/crypto/ccp/ccp_dev.h     |  31 ++--
 drivers/crypto/ccp/ccp_pci.c     | 240 -------------------------------
 drivers/crypto/ccp/ccp_pci.h     |  27 ----
 drivers/crypto/ccp/meson.build   |   1 -
 drivers/crypto/ccp/rte_ccp_pmd.c |  23 +--
 7 files changed, 47 insertions(+), 484 deletions(-)
 delete mode 100644 drivers/crypto/ccp/ccp_pci.c
 delete mode 100644 drivers/crypto/ccp/ccp_pci.h

-- 
2.37.2


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

* [PATCH 1/4] crypto/ccp: remove some printf
  2022-09-09 15:04 [PATCH 0/4] crypto/ccp cleanup David Marchand
@ 2022-09-09 15:04 ` David Marchand
  2022-09-09 15:04 ` [PATCH 2/4] crypto/ccp: remove some dead code for UIO David Marchand
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 28+ messages in thread
From: David Marchand @ 2022-09-09 15:04 UTC (permalink / raw)
  To: dev; +Cc: gakhil, chandu, stable, Ravi Kumar

A DPDK application must _not_ use printf.
Use log framework.

Fixes: ef4b04f87fa6 ("crypto/ccp: support device init")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ccp/ccp_dev.c     | 4 ++--
 drivers/crypto/ccp/ccp_pci.c     | 3 ++-
 drivers/crypto/ccp/rte_ccp_pmd.c | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c
index 424ead82c3..9c9cb81236 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -362,7 +362,7 @@ ccp_find_lsb_regions(struct ccp_queue *cmd_q, uint64_t status)
 		if (ccp_get_bit(&cmd_q->lsbmask, j))
 			weight++;
 
-	printf("Queue %d can access %d LSB regions  of mask  %lu\n",
+	CCP_LOG_DBG("Queue %d can access %d LSB regions  of mask  %lu\n",
 	       (int)cmd_q->id, weight, cmd_q->lsbmask);
 
 	return weight ? 0 : -EINVAL;
@@ -709,7 +709,7 @@ ccp_probe_devices(struct rte_pci_device *pci_dev,
 		snprintf(dirname, sizeof(dirname), "%s/%s",
 			     SYSFS_PCI_DEVICES, d->d_name);
 		if (is_ccp_device(dirname, ccp_id, &ccp_type)) {
-			printf("CCP : Detected CCP device with ID = 0x%x\n",
+			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)
diff --git a/drivers/crypto/ccp/ccp_pci.c b/drivers/crypto/ccp/ccp_pci.c
index 38029a9081..c941e222c7 100644
--- a/drivers/crypto/ccp/ccp_pci.c
+++ b/drivers/crypto/ccp/ccp_pci.c
@@ -11,6 +11,7 @@
 #include <rte_string_fns.h>
 
 #include "ccp_pci.h"
+#include "ccp_pmd_private.h"
 
 static const char * const uio_module_names[] = {
 	"igb_uio",
@@ -41,7 +42,7 @@ ccp_check_pci_uio_module(void)
 		rewind(fp);
 	}
 	fclose(fp);
-	printf("Insert igb_uio or uio_pci_generic kernel module(s)");
+	CCP_LOG_DBG("Insert igb_uio or uio_pci_generic kernel module(s)");
 	return -1;/* uio not inserted */
 }
 
diff --git a/drivers/crypto/ccp/rte_ccp_pmd.c b/drivers/crypto/ccp/rte_ccp_pmd.c
index a35a8cd775..c5ec952e36 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -250,7 +250,7 @@ cryptodev_ccp_create(const char *name,
 		goto init_error;
 	}
 
-	printf("CCP : Crypto device count = %d\n", cryptodev_cnt);
+	CCP_LOG_DBG("CCP : Crypto device count = %d\n", cryptodev_cnt);
 	dev->device = &pci_dev->device;
 	dev->device->driver = &pci_drv->driver;
 	dev->driver_id = ccp_cryptodev_driver_id;
-- 
2.37.2


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

* [PATCH 2/4] crypto/ccp: remove some dead code for UIO
  2022-09-09 15:04 [PATCH 0/4] crypto/ccp cleanup David Marchand
  2022-09-09 15:04 ` [PATCH 1/4] crypto/ccp: remove some printf David Marchand
@ 2022-09-09 15:04 ` David Marchand
  2022-09-09 15:04 ` [PATCH 3/4] crypto/ccp: fix IOVA handling David Marchand
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 28+ messages in thread
From: David Marchand @ 2022-09-09 15:04 UTC (permalink / raw)
  To: dev; +Cc: gakhil, chandu, stable, Amaranath Somalapuram

uio_fd is unused.

Fixes: 09a0fd736a08 ("crypto/ccp: enable IOMMU")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ccp/ccp_dev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c
index 9c9cb81236..410e62121e 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -653,7 +653,6 @@ static int
 ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)
 {
 	struct ccp_device *ccp_dev = NULL;
-	int uio_fd = -1;
 
 	ccp_dev = rte_zmalloc("ccp_device", sizeof(*ccp_dev),
 			      RTE_CACHE_LINE_SIZE);
@@ -671,8 +670,6 @@ ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)
 	return 0;
 fail:
 	CCP_LOG_ERR("CCP Device probe failed");
-	if (uio_fd >= 0)
-		close(uio_fd);
 	rte_free(ccp_dev);
 	return -1;
 }
-- 
2.37.2


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

* [PATCH 3/4] crypto/ccp: fix IOVA handling
  2022-09-09 15:04 [PATCH 0/4] crypto/ccp cleanup David Marchand
  2022-09-09 15:04 ` [PATCH 1/4] crypto/ccp: remove some printf David Marchand
  2022-09-09 15:04 ` [PATCH 2/4] crypto/ccp: remove some dead code for UIO David Marchand
@ 2022-09-09 15:04 ` David Marchand
  2022-09-09 15:04 ` [PATCH 4/4] crypto/ccp: fix PCI probing David Marchand
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 28+ messages in thread
From: David Marchand @ 2022-09-09 15:04 UTC (permalink / raw)
  To: dev; +Cc: gakhil, chandu, stable, Amaranath Somalapuram

Using IOVA or physical addresses is something that the user (via
--iova-mode=) or the bus code decides.

The crypto/ccp PCI driver should only use rte_mem_virt2iova.
It should not try to decide what to use solely based on the kmod
the PCI device is bound to.

While at it, the global variable sha_ctx looks unsafe and unneeded.
Remove it.

Fixes: 09a0fd736a08 ("crypto/ccp: enable IOMMU")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ccp/ccp_crypto.c  | 105 ++++++-------------------------
 drivers/crypto/ccp/ccp_dev.c     |   9 +--
 drivers/crypto/ccp/ccp_pci.c     |  34 ----------
 drivers/crypto/ccp/ccp_pci.h     |   3 -
 drivers/crypto/ccp/rte_ccp_pmd.c |   3 -
 5 files changed, 19 insertions(+), 135 deletions(-)

diff --git a/drivers/crypto/ccp/ccp_crypto.c b/drivers/crypto/ccp/ccp_crypto.c
index 4bab18323b..351d8ac63e 100644
--- a/drivers/crypto/ccp/ccp_crypto.c
+++ b/drivers/crypto/ccp/ccp_crypto.c
@@ -33,8 +33,6 @@
 #include <openssl/err.h>
 #include <openssl/hmac.h>
 
-extern int iommu_mode;
-void *sha_ctx;
 /* SHA initial context values */
 uint32_t ccp_sha1_init[SHA_COMMON_DIGEST_SIZE / sizeof(uint32_t)] = {
 	SHA1_H4, SHA1_H3,
@@ -748,13 +746,8 @@ ccp_configure_session_cipher(struct ccp_session *sess,
 		CCP_LOG_ERR("Invalid CCP Engine");
 		return -ENOTSUP;
 	}
-	if (iommu_mode == 2) {
-		sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
-		sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
-	} else {
-		sess->cipher.nonce_phys = rte_mem_virt2phy(sess->cipher.nonce);
-		sess->cipher.key_phys = rte_mem_virt2phy(sess->cipher.key_ccp);
-	}
+	sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
+	sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
 	return 0;
 }
 
@@ -793,7 +786,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
 		sess->auth.ctx = (void *)ccp_sha1_init;
 		sess->auth.ctx_len = CCP_SB_BYTES;
 		sess->auth.offset = CCP_SB_BYTES - SHA1_DIGEST_SIZE;
-		rte_memcpy(sha_ctx, sess->auth.ctx, SHA_COMMON_DIGEST_SIZE);
 		break;
 	case RTE_CRYPTO_AUTH_SHA1_HMAC:
 		if (sess->auth_opt) {
@@ -832,7 +824,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
 		sess->auth.ctx = (void *)ccp_sha224_init;
 		sess->auth.ctx_len = CCP_SB_BYTES;
 		sess->auth.offset = CCP_SB_BYTES - SHA224_DIGEST_SIZE;
-		rte_memcpy(sha_ctx, sess->auth.ctx, SHA256_DIGEST_SIZE);
 		break;
 	case RTE_CRYPTO_AUTH_SHA224_HMAC:
 		if (sess->auth_opt) {
@@ -895,7 +886,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
 		sess->auth.ctx = (void *)ccp_sha256_init;
 		sess->auth.ctx_len = CCP_SB_BYTES;
 		sess->auth.offset = CCP_SB_BYTES - SHA256_DIGEST_SIZE;
-		rte_memcpy(sha_ctx, sess->auth.ctx, SHA256_DIGEST_SIZE);
 		break;
 	case RTE_CRYPTO_AUTH_SHA256_HMAC:
 		if (sess->auth_opt) {
@@ -958,7 +948,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
 		sess->auth.ctx = (void *)ccp_sha384_init;
 		sess->auth.ctx_len = CCP_SB_BYTES << 1;
 		sess->auth.offset = (CCP_SB_BYTES << 1) - SHA384_DIGEST_SIZE;
-		rte_memcpy(sha_ctx, sess->auth.ctx, SHA512_DIGEST_SIZE);
 		break;
 	case RTE_CRYPTO_AUTH_SHA384_HMAC:
 		if (sess->auth_opt) {
@@ -1023,7 +1012,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
 		sess->auth.ctx = (void *)ccp_sha512_init;
 		sess->auth.ctx_len = CCP_SB_BYTES << 1;
 		sess->auth.offset = (CCP_SB_BYTES << 1) - SHA512_DIGEST_SIZE;
-		rte_memcpy(sha_ctx, sess->auth.ctx, SHA512_DIGEST_SIZE);
 		break;
 	case RTE_CRYPTO_AUTH_SHA512_HMAC:
 		if (sess->auth_opt) {
@@ -1173,13 +1161,8 @@ ccp_configure_session_aead(struct ccp_session *sess,
 		CCP_LOG_ERR("Unsupported aead algo");
 		return -ENOTSUP;
 	}
-	if (iommu_mode == 2) {
-		sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
-		sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
-	} else {
-		sess->cipher.nonce_phys = rte_mem_virt2phy(sess->cipher.nonce);
-		sess->cipher.key_phys = rte_mem_virt2phy(sess->cipher.key_ccp);
-	}
+	sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
+	sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
 	return 0;
 }
 
@@ -1594,14 +1577,8 @@ ccp_perform_hmac(struct rte_crypto_op *op,
 					      op->sym->auth.data.offset);
 	append_ptr = (void *)rte_pktmbuf_append(op->sym->m_src,
 						session->auth.ctx_len);
-	if (iommu_mode == 2) {
-		dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
-		pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
-	} else {
-		dest_addr = (phys_addr_t)rte_mem_virt2phy(append_ptr);
-		pst.src_addr = (phys_addr_t)rte_mem_virt2phy((void *)addr);
-	}
-	dest_addr_t = dest_addr;
+	dest_addr_t = dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
+	pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
 
 	/** Load PHash1 to LSB*/
 	pst.dest_addr = (phys_addr_t)(cmd_q->sb_sha * CCP_SB_BYTES);
@@ -1683,10 +1660,7 @@ ccp_perform_hmac(struct rte_crypto_op *op,
 
 	/** Load PHash2 to LSB*/
 	addr += session->auth.ctx_len;
-	if (iommu_mode == 2)
-		pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
-	else
-		pst.src_addr = (phys_addr_t)rte_mem_virt2phy((void *)addr);
+	pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
 	pst.dest_addr = (phys_addr_t)(cmd_q->sb_sha * CCP_SB_BYTES);
 	pst.len = session->auth.ctx_len;
 	pst.dir = 1;
@@ -1774,14 +1748,8 @@ ccp_perform_sha(struct rte_crypto_op *op,
 					      op->sym->auth.data.offset);
 	append_ptr = (void *)rte_pktmbuf_append(op->sym->m_src,
 						session->auth.ctx_len);
-	if (iommu_mode == 2) {
-		dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
-		pst.src_addr = (phys_addr_t)sha_ctx;
-	} else {
-		dest_addr = (phys_addr_t)rte_mem_virt2phy(append_ptr);
-		pst.src_addr = (phys_addr_t)rte_mem_virt2phy((void *)
-						     session->auth.ctx);
-	}
+	pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)session->auth.ctx);
+	dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
 
 	/** Passthru sha context*/
 
@@ -1871,15 +1839,8 @@ ccp_perform_sha3_hmac(struct rte_crypto_op *op,
 		CCP_LOG_ERR("CCP MBUF append failed\n");
 		return -1;
 	}
-	if (iommu_mode == 2) {
-		dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
-		ctx_paddr = (phys_addr_t)rte_mem_virt2iova(
-					session->auth.pre_compute);
-	} else {
-		dest_addr = (phys_addr_t)rte_mem_virt2phy((void *)append_ptr);
-		ctx_paddr = (phys_addr_t)rte_mem_virt2phy(
-					session->auth.pre_compute);
-	}
+	dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
+	ctx_paddr = (phys_addr_t)rte_mem_virt2iova(session->auth.pre_compute);
 	dest_addr_t = dest_addr + (session->auth.ctx_len / 2);
 	desc = &cmd_q->qbase_desc[cmd_q->qidx];
 	memset(desc, 0, Q_DESC_SIZE);
@@ -2017,13 +1978,8 @@ ccp_perform_sha3(struct rte_crypto_op *op,
 		CCP_LOG_ERR("CCP MBUF append failed\n");
 		return -1;
 	}
-	if (iommu_mode == 2) {
-		dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
-		ctx_paddr = (phys_addr_t)rte_mem_virt2iova((void *)ctx_addr);
-	} else {
-		dest_addr = (phys_addr_t)rte_mem_virt2phy((void *)append_ptr);
-		ctx_paddr = (phys_addr_t)rte_mem_virt2phy((void *)ctx_addr);
-	}
+	dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
+	ctx_paddr = (phys_addr_t)rte_mem_virt2iova((void *)ctx_addr);
 
 	ctx_addr = session->auth.sha3_ctx;
 
@@ -2099,13 +2055,7 @@ ccp_perform_aes_cmac(struct rte_crypto_op *op,
 
 		ctx_addr = session->auth.pre_compute;
 		memset(ctx_addr, 0, AES_BLOCK_SIZE);
-		if (iommu_mode == 2)
-			pst.src_addr = (phys_addr_t)rte_mem_virt2iova(
-							(void *)ctx_addr);
-		else
-			pst.src_addr = (phys_addr_t)rte_mem_virt2phy(
-							(void *)ctx_addr);
-
+		pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)ctx_addr);
 		pst.dest_addr = (phys_addr_t)(cmd_q->sb_iv * CCP_SB_BYTES);
 		pst.len = CCP_SB_BYTES;
 		pst.dir = 1;
@@ -2143,12 +2093,7 @@ ccp_perform_aes_cmac(struct rte_crypto_op *op,
 	} else {
 		ctx_addr = session->auth.pre_compute + CCP_SB_BYTES;
 		memset(ctx_addr, 0, AES_BLOCK_SIZE);
-		if (iommu_mode == 2)
-			pst.src_addr = (phys_addr_t)rte_mem_virt2iova(
-							(void *)ctx_addr);
-		else
-			pst.src_addr = (phys_addr_t)rte_mem_virt2phy(
-							(void *)ctx_addr);
+		pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)ctx_addr);
 		pst.dest_addr = (phys_addr_t)(cmd_q->sb_iv * CCP_SB_BYTES);
 		pst.len = CCP_SB_BYTES;
 		pst.dir = 1;
@@ -2342,12 +2287,7 @@ ccp_perform_3des(struct rte_crypto_op *op,
 
 		rte_memcpy(lsb_buf + (CCP_SB_BYTES - session->iv.length),
 			   iv, session->iv.length);
-		if (iommu_mode == 2)
-			pst.src_addr = (phys_addr_t)rte_mem_virt2iova(
-							(void *) lsb_buf);
-		else
-			pst.src_addr = (phys_addr_t)rte_mem_virt2phy(
-							(void *) lsb_buf);
+		pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *) lsb_buf);
 		pst.dest_addr = (phys_addr_t)(cmd_q->sb_iv * CCP_SB_BYTES);
 		pst.len = CCP_SB_BYTES;
 		pst.dir = 1;
@@ -2370,11 +2310,7 @@ ccp_perform_3des(struct rte_crypto_op *op,
 	else
 		dest_addr = src_addr;
 
-	if (iommu_mode == 2)
-		key_addr = rte_mem_virt2iova(session->cipher.key_ccp);
-	else
-		key_addr = rte_mem_virt2phy(session->cipher.key_ccp);
-
+	key_addr = rte_mem_virt2iova(session->cipher.key_ccp);
 	desc = &cmd_q->qbase_desc[cmd_q->qidx];
 
 	memset(desc, 0, Q_DESC_SIZE);
@@ -2768,12 +2704,7 @@ process_ops_to_enqueue(struct ccp_qp *qp,
 	b_info->lsb_buf_idx = 0;
 	b_info->desccnt = 0;
 	b_info->cmd_q = cmd_q;
-	if (iommu_mode == 2)
-		b_info->lsb_buf_phys =
-			(phys_addr_t)rte_mem_virt2iova((void *)b_info->lsb_buf);
-	else
-		b_info->lsb_buf_phys =
-			(phys_addr_t)rte_mem_virt2phy((void *)b_info->lsb_buf);
+	b_info->lsb_buf_phys = (phys_addr_t)rte_mem_virt2iova((void *)b_info->lsb_buf);
 
 	rte_atomic64_sub(&b_info->cmd_q->free_slots, slots_req);
 
diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c
index 410e62121e..14c54929c4 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -23,7 +23,6 @@
 #include "ccp_pci.h"
 #include "ccp_pmd_private.h"
 
-int iommu_mode;
 struct ccp_list ccp_list = TAILQ_HEAD_INITIALIZER(ccp_list);
 static int ccp_dev_id;
 
@@ -652,7 +651,7 @@ is_ccp_device(const char *dirname,
 static int
 ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)
 {
-	struct ccp_device *ccp_dev = NULL;
+	struct ccp_device *ccp_dev;
 
 	ccp_dev = rte_zmalloc("ccp_device", sizeof(*ccp_dev),
 			      RTE_CACHE_LINE_SIZE);
@@ -683,16 +682,10 @@ ccp_probe_devices(struct rte_pci_device *pci_dev,
 	struct dirent *d;
 	DIR *dir;
 	int ret = 0;
-	int module_idx = 0;
 	uint16_t domain;
 	uint8_t bus, devid, function;
 	char dirname[PATH_MAX];
 
-	module_idx = ccp_check_pci_uio_module();
-	if (module_idx < 0)
-		return -1;
-
-	iommu_mode = module_idx;
 	TAILQ_INIT(&ccp_list);
 	dir = opendir(SYSFS_PCI_DEVICES);
 	if (dir == NULL)
diff --git a/drivers/crypto/ccp/ccp_pci.c b/drivers/crypto/ccp/ccp_pci.c
index c941e222c7..bd1a037f76 100644
--- a/drivers/crypto/ccp/ccp_pci.c
+++ b/drivers/crypto/ccp/ccp_pci.c
@@ -11,40 +11,6 @@
 #include <rte_string_fns.h>
 
 #include "ccp_pci.h"
-#include "ccp_pmd_private.h"
-
-static const char * const uio_module_names[] = {
-	"igb_uio",
-	"uio_pci_generic",
-	"vfio_pci"
-};
-
-int
-ccp_check_pci_uio_module(void)
-{
-	FILE *fp;
-	int i;
-	char buf[BUFSIZ];
-
-	fp = fopen(PROC_MODULES, "r");
-	if (fp == NULL)
-		return -1;
-	i = 0;
-	while (uio_module_names[i] != NULL) {
-		while (fgets(buf, sizeof(buf), fp) != NULL) {
-			if (!strncmp(buf, uio_module_names[i],
-				     strlen(uio_module_names[i]))) {
-				fclose(fp);
-				return i;
-			}
-		}
-		i++;
-		rewind(fp);
-	}
-	fclose(fp);
-	CCP_LOG_DBG("Insert igb_uio or uio_pci_generic kernel module(s)");
-	return -1;/* uio not inserted */
-}
 
 /*
  * split up a pci address into its constituent parts.
diff --git a/drivers/crypto/ccp/ccp_pci.h b/drivers/crypto/ccp/ccp_pci.h
index 7ed3bac406..f393a04d6f 100644
--- a/drivers/crypto/ccp/ccp_pci.h
+++ b/drivers/crypto/ccp/ccp_pci.h
@@ -10,9 +10,6 @@
 #include <rte_bus_pci.h>
 
 #define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
-#define PROC_MODULES "/proc/modules"
-
-int ccp_check_pci_uio_module(void);
 
 int ccp_parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
 			      uint8_t *bus, uint8_t *devid, uint8_t *function);
diff --git a/drivers/crypto/ccp/rte_ccp_pmd.c b/drivers/crypto/ccp/rte_ccp_pmd.c
index c5ec952e36..0d84c8cd0e 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -22,7 +22,6 @@
 static unsigned int ccp_pmd_init_done;
 uint8_t ccp_cryptodev_driver_id;
 uint8_t cryptodev_cnt;
-extern void *sha_ctx;
 
 struct ccp_pmd_init_params {
 	struct rte_cryptodev_pmd_init_params def_p;
@@ -213,7 +212,6 @@ cryptodev_ccp_remove(struct rte_pci_device *pci_dev)
 		return -ENODEV;
 
 	ccp_pmd_init_done = 0;
-	rte_free(sha_ctx);
 
 	RTE_LOG(INFO, PMD, "Closing ccp device %s on numa socket %u\n",
 			name, rte_socket_id());
@@ -300,7 +298,6 @@ cryptodev_ccp_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		.auth_opt = CCP_PMD_AUTH_OPT_CCP,
 	};
 
-	sha_ctx = (void *)rte_malloc(NULL, SHA512_DIGEST_SIZE, 64);
 	if (ccp_pmd_init_done) {
 		RTE_LOG(INFO, PMD, "CCP PMD already initialized\n");
 		return -EFAULT;
-- 
2.37.2


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

* [PATCH 4/4] crypto/ccp: fix PCI probing
  2022-09-09 15:04 [PATCH 0/4] crypto/ccp cleanup David Marchand
                   ` (2 preceding siblings ...)
  2022-09-09 15:04 ` [PATCH 3/4] crypto/ccp: fix IOVA handling David Marchand
@ 2022-09-09 15:04 ` David Marchand
  2022-10-04  9:51 ` [PATCH v2 0/4] crypto/ccp cleanup David Marchand
  2023-03-02 11:43 ` [PATCH v2] crypto/ccp: fix PCI probing David Marchand
  5 siblings, 0 replies; 28+ messages in thread
From: David Marchand @ 2022-09-09 15:04 UTC (permalink / raw)
  To: dev; +Cc: gakhil, chandu, stable, Amaranath Somalapuram

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

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 2a205cd446..e5ef811909 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 f393a04d6f..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 <rte_bus_pci.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 0d84c8cd0e..ae313d133a 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);
 
-- 
2.37.2


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

* [PATCH v2 0/4] crypto/ccp cleanup
  2022-09-09 15:04 [PATCH 0/4] crypto/ccp cleanup David Marchand
                   ` (3 preceding siblings ...)
  2022-09-09 15:04 ` [PATCH 4/4] crypto/ccp: fix PCI probing David Marchand
@ 2022-10-04  9:51 ` David Marchand
  2022-10-04  9:51   ` [PATCH v2 1/4] crypto/ccp: remove some printf David Marchand
                     ` (5 more replies)
  2023-03-02 11:43 ` [PATCH v2] crypto/ccp: fix PCI probing David Marchand
  5 siblings, 6 replies; 28+ messages in thread
From: David Marchand @ 2022-10-04  9:51 UTC (permalink / raw)
  To: dev; +Cc: gakhil, sunilprakashrao.uttarwar

This is a *untested* cleanup series after looking for usage of
rte_pci_device objects in DPDK drivers.
I can't test those patches by lack of hw, so I hope the driver maintainer
can look into them.

Thanks.
-- 
David Marchand

Changes since v1:
- rebased,
- copied new maintainer,

David Marchand (4):
  crypto/ccp: remove some printf
  crypto/ccp: remove some dead code for UIO
  crypto/ccp: fix IOVA handling
  crypto/ccp: fix PCI probing

 drivers/crypto/ccp/ccp_crypto.c  | 106 +++-----------
 drivers/crypto/ccp/ccp_dev.c     | 103 ++-----------
 drivers/crypto/ccp/ccp_dev.h     |  31 ++--
 drivers/crypto/ccp/ccp_pci.c     | 240 -------------------------------
 drivers/crypto/ccp/ccp_pci.h     |  27 ----
 drivers/crypto/ccp/meson.build   |   1 -
 drivers/crypto/ccp/rte_ccp_pmd.c |  23 +--
 7 files changed, 47 insertions(+), 484 deletions(-)
 delete mode 100644 drivers/crypto/ccp/ccp_pci.c
 delete mode 100644 drivers/crypto/ccp/ccp_pci.h

-- 
2.37.3


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

* [PATCH v2 1/4] crypto/ccp: remove some printf
  2022-10-04  9:51 ` [PATCH v2 0/4] crypto/ccp cleanup David Marchand
@ 2022-10-04  9:51   ` David Marchand
  2023-01-13 11:58     ` Uttarwar, Sunil Prakashrao
  2023-01-30 18:42     ` [EXT] " Akhil Goyal
  2022-10-04  9:51   ` [PATCH v2 2/4] crypto/ccp: remove some dead code for UIO David Marchand
                     ` (4 subsequent siblings)
  5 siblings, 2 replies; 28+ messages in thread
From: David Marchand @ 2022-10-04  9:51 UTC (permalink / raw)
  To: dev; +Cc: gakhil, sunilprakashrao.uttarwar, stable, Ravi Kumar

A DPDK application must _not_ use printf.
Use log framework.

Fixes: ef4b04f87fa6 ("crypto/ccp: support device init")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ccp/ccp_dev.c     | 4 ++--
 drivers/crypto/ccp/ccp_pci.c     | 3 ++-
 drivers/crypto/ccp/rte_ccp_pmd.c | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c
index 424ead82c3..9c9cb81236 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -362,7 +362,7 @@ ccp_find_lsb_regions(struct ccp_queue *cmd_q, uint64_t status)
 		if (ccp_get_bit(&cmd_q->lsbmask, j))
 			weight++;
 
-	printf("Queue %d can access %d LSB regions  of mask  %lu\n",
+	CCP_LOG_DBG("Queue %d can access %d LSB regions  of mask  %lu\n",
 	       (int)cmd_q->id, weight, cmd_q->lsbmask);
 
 	return weight ? 0 : -EINVAL;
@@ -709,7 +709,7 @@ ccp_probe_devices(struct rte_pci_device *pci_dev,
 		snprintf(dirname, sizeof(dirname), "%s/%s",
 			     SYSFS_PCI_DEVICES, d->d_name);
 		if (is_ccp_device(dirname, ccp_id, &ccp_type)) {
-			printf("CCP : Detected CCP device with ID = 0x%x\n",
+			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)
diff --git a/drivers/crypto/ccp/ccp_pci.c b/drivers/crypto/ccp/ccp_pci.c
index 38029a9081..c941e222c7 100644
--- a/drivers/crypto/ccp/ccp_pci.c
+++ b/drivers/crypto/ccp/ccp_pci.c
@@ -11,6 +11,7 @@
 #include <rte_string_fns.h>
 
 #include "ccp_pci.h"
+#include "ccp_pmd_private.h"
 
 static const char * const uio_module_names[] = {
 	"igb_uio",
@@ -41,7 +42,7 @@ ccp_check_pci_uio_module(void)
 		rewind(fp);
 	}
 	fclose(fp);
-	printf("Insert igb_uio or uio_pci_generic kernel module(s)");
+	CCP_LOG_DBG("Insert igb_uio or uio_pci_generic kernel module(s)");
 	return -1;/* uio not inserted */
 }
 
diff --git a/drivers/crypto/ccp/rte_ccp_pmd.c b/drivers/crypto/ccp/rte_ccp_pmd.c
index 013f3be1e6..7338ef0ae8 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -250,7 +250,7 @@ cryptodev_ccp_create(const char *name,
 		goto init_error;
 	}
 
-	printf("CCP : Crypto device count = %d\n", cryptodev_cnt);
+	CCP_LOG_DBG("CCP : Crypto device count = %d\n", cryptodev_cnt);
 	dev->device = &pci_dev->device;
 	dev->device->driver = &pci_drv->driver;
 	dev->driver_id = ccp_cryptodev_driver_id;
-- 
2.37.3


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

* [PATCH v2 2/4] crypto/ccp: remove some dead code for UIO
  2022-10-04  9:51 ` [PATCH v2 0/4] crypto/ccp cleanup David Marchand
  2022-10-04  9:51   ` [PATCH v2 1/4] crypto/ccp: remove some printf David Marchand
@ 2022-10-04  9:51   ` David Marchand
  2023-01-13 12:00     ` Uttarwar, Sunil Prakashrao
  2022-10-04  9:51   ` [PATCH v2 3/4] crypto/ccp: fix IOVA handling David Marchand
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 28+ messages in thread
From: David Marchand @ 2022-10-04  9:51 UTC (permalink / raw)
  To: dev; +Cc: gakhil, sunilprakashrao.uttarwar, stable, Amaranath Somalapuram

uio_fd is unused.

Fixes: 09a0fd736a08 ("crypto/ccp: enable IOMMU")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ccp/ccp_dev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c
index 9c9cb81236..410e62121e 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -653,7 +653,6 @@ static int
 ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)
 {
 	struct ccp_device *ccp_dev = NULL;
-	int uio_fd = -1;
 
 	ccp_dev = rte_zmalloc("ccp_device", sizeof(*ccp_dev),
 			      RTE_CACHE_LINE_SIZE);
@@ -671,8 +670,6 @@ ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)
 	return 0;
 fail:
 	CCP_LOG_ERR("CCP Device probe failed");
-	if (uio_fd >= 0)
-		close(uio_fd);
 	rte_free(ccp_dev);
 	return -1;
 }
-- 
2.37.3


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

* [PATCH v2 3/4] crypto/ccp: fix IOVA handling
  2022-10-04  9:51 ` [PATCH v2 0/4] crypto/ccp cleanup David Marchand
  2022-10-04  9:51   ` [PATCH v2 1/4] crypto/ccp: remove some printf David Marchand
  2022-10-04  9:51   ` [PATCH v2 2/4] crypto/ccp: remove some dead code for UIO David Marchand
@ 2022-10-04  9:51   ` David Marchand
  2023-01-13 12:00     ` Uttarwar, Sunil Prakashrao
  2022-10-04  9:51   ` [PATCH v2 4/4] crypto/ccp: fix PCI probing David Marchand
                     ` (2 subsequent siblings)
  5 siblings, 1 reply; 28+ messages in thread
From: David Marchand @ 2022-10-04  9:51 UTC (permalink / raw)
  To: dev; +Cc: gakhil, sunilprakashrao.uttarwar, stable, Amaranath Somalapuram

Using IOVA or physical addresses is something that the user (via
--iova-mode=) or the bus code decides.

The crypto/ccp PCI driver should only use rte_mem_virt2iova.
It should not try to decide what to use solely based on the kmod
the PCI device is bound to.

While at it, the global variable sha_ctx looks unsafe and unneeded.
Remove it.

Fixes: 09a0fd736a08 ("crypto/ccp: enable IOMMU")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ccp/ccp_crypto.c  | 105 ++++++-------------------------
 drivers/crypto/ccp/ccp_dev.c     |   9 +--
 drivers/crypto/ccp/ccp_pci.c     |  34 ----------
 drivers/crypto/ccp/ccp_pci.h     |   3 -
 drivers/crypto/ccp/rte_ccp_pmd.c |   3 -
 5 files changed, 19 insertions(+), 135 deletions(-)

diff --git a/drivers/crypto/ccp/ccp_crypto.c b/drivers/crypto/ccp/ccp_crypto.c
index 4bab18323b..351d8ac63e 100644
--- a/drivers/crypto/ccp/ccp_crypto.c
+++ b/drivers/crypto/ccp/ccp_crypto.c
@@ -33,8 +33,6 @@
 #include <openssl/err.h>
 #include <openssl/hmac.h>
 
-extern int iommu_mode;
-void *sha_ctx;
 /* SHA initial context values */
 uint32_t ccp_sha1_init[SHA_COMMON_DIGEST_SIZE / sizeof(uint32_t)] = {
 	SHA1_H4, SHA1_H3,
@@ -748,13 +746,8 @@ ccp_configure_session_cipher(struct ccp_session *sess,
 		CCP_LOG_ERR("Invalid CCP Engine");
 		return -ENOTSUP;
 	}
-	if (iommu_mode == 2) {
-		sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
-		sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
-	} else {
-		sess->cipher.nonce_phys = rte_mem_virt2phy(sess->cipher.nonce);
-		sess->cipher.key_phys = rte_mem_virt2phy(sess->cipher.key_ccp);
-	}
+	sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
+	sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
 	return 0;
 }
 
@@ -793,7 +786,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
 		sess->auth.ctx = (void *)ccp_sha1_init;
 		sess->auth.ctx_len = CCP_SB_BYTES;
 		sess->auth.offset = CCP_SB_BYTES - SHA1_DIGEST_SIZE;
-		rte_memcpy(sha_ctx, sess->auth.ctx, SHA_COMMON_DIGEST_SIZE);
 		break;
 	case RTE_CRYPTO_AUTH_SHA1_HMAC:
 		if (sess->auth_opt) {
@@ -832,7 +824,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
 		sess->auth.ctx = (void *)ccp_sha224_init;
 		sess->auth.ctx_len = CCP_SB_BYTES;
 		sess->auth.offset = CCP_SB_BYTES - SHA224_DIGEST_SIZE;
-		rte_memcpy(sha_ctx, sess->auth.ctx, SHA256_DIGEST_SIZE);
 		break;
 	case RTE_CRYPTO_AUTH_SHA224_HMAC:
 		if (sess->auth_opt) {
@@ -895,7 +886,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
 		sess->auth.ctx = (void *)ccp_sha256_init;
 		sess->auth.ctx_len = CCP_SB_BYTES;
 		sess->auth.offset = CCP_SB_BYTES - SHA256_DIGEST_SIZE;
-		rte_memcpy(sha_ctx, sess->auth.ctx, SHA256_DIGEST_SIZE);
 		break;
 	case RTE_CRYPTO_AUTH_SHA256_HMAC:
 		if (sess->auth_opt) {
@@ -958,7 +948,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
 		sess->auth.ctx = (void *)ccp_sha384_init;
 		sess->auth.ctx_len = CCP_SB_BYTES << 1;
 		sess->auth.offset = (CCP_SB_BYTES << 1) - SHA384_DIGEST_SIZE;
-		rte_memcpy(sha_ctx, sess->auth.ctx, SHA512_DIGEST_SIZE);
 		break;
 	case RTE_CRYPTO_AUTH_SHA384_HMAC:
 		if (sess->auth_opt) {
@@ -1023,7 +1012,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
 		sess->auth.ctx = (void *)ccp_sha512_init;
 		sess->auth.ctx_len = CCP_SB_BYTES << 1;
 		sess->auth.offset = (CCP_SB_BYTES << 1) - SHA512_DIGEST_SIZE;
-		rte_memcpy(sha_ctx, sess->auth.ctx, SHA512_DIGEST_SIZE);
 		break;
 	case RTE_CRYPTO_AUTH_SHA512_HMAC:
 		if (sess->auth_opt) {
@@ -1173,13 +1161,8 @@ ccp_configure_session_aead(struct ccp_session *sess,
 		CCP_LOG_ERR("Unsupported aead algo");
 		return -ENOTSUP;
 	}
-	if (iommu_mode == 2) {
-		sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
-		sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
-	} else {
-		sess->cipher.nonce_phys = rte_mem_virt2phy(sess->cipher.nonce);
-		sess->cipher.key_phys = rte_mem_virt2phy(sess->cipher.key_ccp);
-	}
+	sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
+	sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
 	return 0;
 }
 
@@ -1594,14 +1577,8 @@ ccp_perform_hmac(struct rte_crypto_op *op,
 					      op->sym->auth.data.offset);
 	append_ptr = (void *)rte_pktmbuf_append(op->sym->m_src,
 						session->auth.ctx_len);
-	if (iommu_mode == 2) {
-		dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
-		pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
-	} else {
-		dest_addr = (phys_addr_t)rte_mem_virt2phy(append_ptr);
-		pst.src_addr = (phys_addr_t)rte_mem_virt2phy((void *)addr);
-	}
-	dest_addr_t = dest_addr;
+	dest_addr_t = dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
+	pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
 
 	/** Load PHash1 to LSB*/
 	pst.dest_addr = (phys_addr_t)(cmd_q->sb_sha * CCP_SB_BYTES);
@@ -1683,10 +1660,7 @@ ccp_perform_hmac(struct rte_crypto_op *op,
 
 	/** Load PHash2 to LSB*/
 	addr += session->auth.ctx_len;
-	if (iommu_mode == 2)
-		pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
-	else
-		pst.src_addr = (phys_addr_t)rte_mem_virt2phy((void *)addr);
+	pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
 	pst.dest_addr = (phys_addr_t)(cmd_q->sb_sha * CCP_SB_BYTES);
 	pst.len = session->auth.ctx_len;
 	pst.dir = 1;
@@ -1774,14 +1748,8 @@ ccp_perform_sha(struct rte_crypto_op *op,
 					      op->sym->auth.data.offset);
 	append_ptr = (void *)rte_pktmbuf_append(op->sym->m_src,
 						session->auth.ctx_len);
-	if (iommu_mode == 2) {
-		dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
-		pst.src_addr = (phys_addr_t)sha_ctx;
-	} else {
-		dest_addr = (phys_addr_t)rte_mem_virt2phy(append_ptr);
-		pst.src_addr = (phys_addr_t)rte_mem_virt2phy((void *)
-						     session->auth.ctx);
-	}
+	pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)session->auth.ctx);
+	dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
 
 	/** Passthru sha context*/
 
@@ -1871,15 +1839,8 @@ ccp_perform_sha3_hmac(struct rte_crypto_op *op,
 		CCP_LOG_ERR("CCP MBUF append failed\n");
 		return -1;
 	}
-	if (iommu_mode == 2) {
-		dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
-		ctx_paddr = (phys_addr_t)rte_mem_virt2iova(
-					session->auth.pre_compute);
-	} else {
-		dest_addr = (phys_addr_t)rte_mem_virt2phy((void *)append_ptr);
-		ctx_paddr = (phys_addr_t)rte_mem_virt2phy(
-					session->auth.pre_compute);
-	}
+	dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
+	ctx_paddr = (phys_addr_t)rte_mem_virt2iova(session->auth.pre_compute);
 	dest_addr_t = dest_addr + (session->auth.ctx_len / 2);
 	desc = &cmd_q->qbase_desc[cmd_q->qidx];
 	memset(desc, 0, Q_DESC_SIZE);
@@ -2017,13 +1978,8 @@ ccp_perform_sha3(struct rte_crypto_op *op,
 		CCP_LOG_ERR("CCP MBUF append failed\n");
 		return -1;
 	}
-	if (iommu_mode == 2) {
-		dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
-		ctx_paddr = (phys_addr_t)rte_mem_virt2iova((void *)ctx_addr);
-	} else {
-		dest_addr = (phys_addr_t)rte_mem_virt2phy((void *)append_ptr);
-		ctx_paddr = (phys_addr_t)rte_mem_virt2phy((void *)ctx_addr);
-	}
+	dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
+	ctx_paddr = (phys_addr_t)rte_mem_virt2iova((void *)ctx_addr);
 
 	ctx_addr = session->auth.sha3_ctx;
 
@@ -2099,13 +2055,7 @@ ccp_perform_aes_cmac(struct rte_crypto_op *op,
 
 		ctx_addr = session->auth.pre_compute;
 		memset(ctx_addr, 0, AES_BLOCK_SIZE);
-		if (iommu_mode == 2)
-			pst.src_addr = (phys_addr_t)rte_mem_virt2iova(
-							(void *)ctx_addr);
-		else
-			pst.src_addr = (phys_addr_t)rte_mem_virt2phy(
-							(void *)ctx_addr);
-
+		pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)ctx_addr);
 		pst.dest_addr = (phys_addr_t)(cmd_q->sb_iv * CCP_SB_BYTES);
 		pst.len = CCP_SB_BYTES;
 		pst.dir = 1;
@@ -2143,12 +2093,7 @@ ccp_perform_aes_cmac(struct rte_crypto_op *op,
 	} else {
 		ctx_addr = session->auth.pre_compute + CCP_SB_BYTES;
 		memset(ctx_addr, 0, AES_BLOCK_SIZE);
-		if (iommu_mode == 2)
-			pst.src_addr = (phys_addr_t)rte_mem_virt2iova(
-							(void *)ctx_addr);
-		else
-			pst.src_addr = (phys_addr_t)rte_mem_virt2phy(
-							(void *)ctx_addr);
+		pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)ctx_addr);
 		pst.dest_addr = (phys_addr_t)(cmd_q->sb_iv * CCP_SB_BYTES);
 		pst.len = CCP_SB_BYTES;
 		pst.dir = 1;
@@ -2342,12 +2287,7 @@ ccp_perform_3des(struct rte_crypto_op *op,
 
 		rte_memcpy(lsb_buf + (CCP_SB_BYTES - session->iv.length),
 			   iv, session->iv.length);
-		if (iommu_mode == 2)
-			pst.src_addr = (phys_addr_t)rte_mem_virt2iova(
-							(void *) lsb_buf);
-		else
-			pst.src_addr = (phys_addr_t)rte_mem_virt2phy(
-							(void *) lsb_buf);
+		pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *) lsb_buf);
 		pst.dest_addr = (phys_addr_t)(cmd_q->sb_iv * CCP_SB_BYTES);
 		pst.len = CCP_SB_BYTES;
 		pst.dir = 1;
@@ -2370,11 +2310,7 @@ ccp_perform_3des(struct rte_crypto_op *op,
 	else
 		dest_addr = src_addr;
 
-	if (iommu_mode == 2)
-		key_addr = rte_mem_virt2iova(session->cipher.key_ccp);
-	else
-		key_addr = rte_mem_virt2phy(session->cipher.key_ccp);
-
+	key_addr = rte_mem_virt2iova(session->cipher.key_ccp);
 	desc = &cmd_q->qbase_desc[cmd_q->qidx];
 
 	memset(desc, 0, Q_DESC_SIZE);
@@ -2768,12 +2704,7 @@ process_ops_to_enqueue(struct ccp_qp *qp,
 	b_info->lsb_buf_idx = 0;
 	b_info->desccnt = 0;
 	b_info->cmd_q = cmd_q;
-	if (iommu_mode == 2)
-		b_info->lsb_buf_phys =
-			(phys_addr_t)rte_mem_virt2iova((void *)b_info->lsb_buf);
-	else
-		b_info->lsb_buf_phys =
-			(phys_addr_t)rte_mem_virt2phy((void *)b_info->lsb_buf);
+	b_info->lsb_buf_phys = (phys_addr_t)rte_mem_virt2iova((void *)b_info->lsb_buf);
 
 	rte_atomic64_sub(&b_info->cmd_q->free_slots, slots_req);
 
diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c
index 410e62121e..14c54929c4 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -23,7 +23,6 @@
 #include "ccp_pci.h"
 #include "ccp_pmd_private.h"
 
-int iommu_mode;
 struct ccp_list ccp_list = TAILQ_HEAD_INITIALIZER(ccp_list);
 static int ccp_dev_id;
 
@@ -652,7 +651,7 @@ is_ccp_device(const char *dirname,
 static int
 ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)
 {
-	struct ccp_device *ccp_dev = NULL;
+	struct ccp_device *ccp_dev;
 
 	ccp_dev = rte_zmalloc("ccp_device", sizeof(*ccp_dev),
 			      RTE_CACHE_LINE_SIZE);
@@ -683,16 +682,10 @@ ccp_probe_devices(struct rte_pci_device *pci_dev,
 	struct dirent *d;
 	DIR *dir;
 	int ret = 0;
-	int module_idx = 0;
 	uint16_t domain;
 	uint8_t bus, devid, function;
 	char dirname[PATH_MAX];
 
-	module_idx = ccp_check_pci_uio_module();
-	if (module_idx < 0)
-		return -1;
-
-	iommu_mode = module_idx;
 	TAILQ_INIT(&ccp_list);
 	dir = opendir(SYSFS_PCI_DEVICES);
 	if (dir == NULL)
diff --git a/drivers/crypto/ccp/ccp_pci.c b/drivers/crypto/ccp/ccp_pci.c
index c941e222c7..bd1a037f76 100644
--- a/drivers/crypto/ccp/ccp_pci.c
+++ b/drivers/crypto/ccp/ccp_pci.c
@@ -11,40 +11,6 @@
 #include <rte_string_fns.h>
 
 #include "ccp_pci.h"
-#include "ccp_pmd_private.h"
-
-static const char * const uio_module_names[] = {
-	"igb_uio",
-	"uio_pci_generic",
-	"vfio_pci"
-};
-
-int
-ccp_check_pci_uio_module(void)
-{
-	FILE *fp;
-	int i;
-	char buf[BUFSIZ];
-
-	fp = fopen(PROC_MODULES, "r");
-	if (fp == NULL)
-		return -1;
-	i = 0;
-	while (uio_module_names[i] != NULL) {
-		while (fgets(buf, sizeof(buf), fp) != NULL) {
-			if (!strncmp(buf, uio_module_names[i],
-				     strlen(uio_module_names[i]))) {
-				fclose(fp);
-				return i;
-			}
-		}
-		i++;
-		rewind(fp);
-	}
-	fclose(fp);
-	CCP_LOG_DBG("Insert igb_uio or uio_pci_generic kernel module(s)");
-	return -1;/* uio not inserted */
-}
 
 /*
  * split up a pci address into its constituent parts.
diff --git a/drivers/crypto/ccp/ccp_pci.h b/drivers/crypto/ccp/ccp_pci.h
index 6736bf8ad3..d9a8b9dcc6 100644
--- a/drivers/crypto/ccp/ccp_pci.h
+++ b/drivers/crypto/ccp/ccp_pci.h
@@ -10,9 +10,6 @@
 #include <bus_pci_driver.h>
 
 #define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
-#define PROC_MODULES "/proc/modules"
-
-int ccp_check_pci_uio_module(void);
 
 int ccp_parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
 			      uint8_t *bus, uint8_t *devid, uint8_t *function);
diff --git a/drivers/crypto/ccp/rte_ccp_pmd.c b/drivers/crypto/ccp/rte_ccp_pmd.c
index 7338ef0ae8..8b3a5a304b 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -22,7 +22,6 @@
 static unsigned int ccp_pmd_init_done;
 uint8_t ccp_cryptodev_driver_id;
 uint8_t cryptodev_cnt;
-extern void *sha_ctx;
 
 struct ccp_pmd_init_params {
 	struct rte_cryptodev_pmd_init_params def_p;
@@ -213,7 +212,6 @@ cryptodev_ccp_remove(struct rte_pci_device *pci_dev)
 		return -ENODEV;
 
 	ccp_pmd_init_done = 0;
-	rte_free(sha_ctx);
 
 	RTE_LOG(INFO, PMD, "Closing ccp device %s on numa socket %u\n",
 			name, rte_socket_id());
@@ -300,7 +298,6 @@ cryptodev_ccp_probe(struct rte_pci_driver *pci_drv __rte_unused,
 		.auth_opt = CCP_PMD_AUTH_OPT_CCP,
 	};
 
-	sha_ctx = (void *)rte_malloc(NULL, SHA512_DIGEST_SIZE, 64);
 	if (ccp_pmd_init_done) {
 		RTE_LOG(INFO, PMD, "CCP PMD already initialized\n");
 		return -EFAULT;
-- 
2.37.3


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

* [PATCH v2 4/4] crypto/ccp: fix PCI probing
  2022-10-04  9:51 ` [PATCH v2 0/4] crypto/ccp cleanup David Marchand
                     ` (2 preceding siblings ...)
  2022-10-04  9:51   ` [PATCH v2 3/4] crypto/ccp: fix IOVA handling David Marchand
@ 2022-10-04  9:51   ` David Marchand
  2022-10-07  6:54   ` [EXT] [PATCH v2 0/4] crypto/ccp cleanup Akhil Goyal
  2022-10-11 11:44   ` Uttarwar, Sunil Prakashrao
  5 siblings, 0 replies; 28+ messages in thread
From: David Marchand @ 2022-10-04  9:51 UTC (permalink / raw)
  To: dev; +Cc: gakhil, sunilprakashrao.uttarwar, stable, Amaranath Somalapuram

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

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);
 
-- 
2.37.3


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

* RE: [EXT] [PATCH v2 0/4] crypto/ccp cleanup
  2022-10-04  9:51 ` [PATCH v2 0/4] crypto/ccp cleanup David Marchand
                     ` (3 preceding siblings ...)
  2022-10-04  9:51   ` [PATCH v2 4/4] crypto/ccp: fix PCI probing David Marchand
@ 2022-10-07  6:54   ` Akhil Goyal
  2022-10-11 11:44   ` Uttarwar, Sunil Prakashrao
  5 siblings, 0 replies; 28+ messages in thread
From: Akhil Goyal @ 2022-10-07  6:54 UTC (permalink / raw)
  To: David Marchand, dev, sunilprakashrao.uttarwar

Hi Sunil,

Please review and ack.

Regards,
Akhil

> This is a *untested* cleanup series after looking for usage of
> rte_pci_device objects in DPDK drivers.
> I can't test those patches by lack of hw, so I hope the driver maintainer
> can look into them.
> 
> Thanks.
> --
> David Marchand
> 
> Changes since v1:
> - rebased,
> - copied new maintainer,
> 
> David Marchand (4):
>   crypto/ccp: remove some printf
>   crypto/ccp: remove some dead code for UIO
>   crypto/ccp: fix IOVA handling
>   crypto/ccp: fix PCI probing
> 
>  drivers/crypto/ccp/ccp_crypto.c  | 106 +++-----------
>  drivers/crypto/ccp/ccp_dev.c     | 103 ++-----------
>  drivers/crypto/ccp/ccp_dev.h     |  31 ++--
>  drivers/crypto/ccp/ccp_pci.c     | 240 -------------------------------
>  drivers/crypto/ccp/ccp_pci.h     |  27 ----
>  drivers/crypto/ccp/meson.build   |   1 -
>  drivers/crypto/ccp/rte_ccp_pmd.c |  23 +--
>  7 files changed, 47 insertions(+), 484 deletions(-)
>  delete mode 100644 drivers/crypto/ccp/ccp_pci.c
>  delete mode 100644 drivers/crypto/ccp/ccp_pci.h
> 
> --
> 2.37.3


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

* RE: [PATCH v2 0/4] crypto/ccp cleanup
  2022-10-04  9:51 ` [PATCH v2 0/4] crypto/ccp cleanup David Marchand
                     ` (4 preceding siblings ...)
  2022-10-07  6:54   ` [EXT] [PATCH v2 0/4] crypto/ccp cleanup Akhil Goyal
@ 2022-10-11 11:44   ` Uttarwar, Sunil Prakashrao
  2022-10-13  9:40     ` Akhil Goyal
  5 siblings, 1 reply; 28+ messages in thread
From: Uttarwar, Sunil Prakashrao @ 2022-10-11 11:44 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: gakhil

[Public]



-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Tuesday, October 4, 2022 3:21 PM
To: dev@dpdk.org
Cc: gakhil@marvell.com; Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>
Subject: [PATCH v2 0/4] crypto/ccp cleanup

For this series

Below shared patches seems fine, verified on AMD platform works fine.

crypto/ccp: fix IOVA handling	
crypto/ccp: remove some dead code for UIO	
crypto/ccp: remove some printf	

Now working on the "crypto/ccp: fix PCI probing" patch and will update on this.

Thanks
Sunil

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

* RE: [PATCH v2 0/4] crypto/ccp cleanup
  2022-10-11 11:44   ` Uttarwar, Sunil Prakashrao
@ 2022-10-13  9:40     ` Akhil Goyal
  2022-10-17 13:42       ` Uttarwar, Sunil Prakashrao
  0 siblings, 1 reply; 28+ messages in thread
From: Akhil Goyal @ 2022-10-13  9:40 UTC (permalink / raw)
  To: Uttarwar, Sunil Prakashrao, David Marchand, dev

Hi Sunil

> For this series
> 
> Below shared patches seems fine, verified on AMD platform works fine.
> 
> crypto/ccp: fix IOVA handling
> crypto/ccp: remove some dead code for UIO
> crypto/ccp: remove some printf
> 
> Now working on the "crypto/ccp: fix PCI probing" patch and will update on this.
> 
Any update on this?
Please provide your ack if no comments.

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

* RE: [PATCH v2 0/4] crypto/ccp cleanup
  2022-10-13  9:40     ` Akhil Goyal
@ 2022-10-17 13:42       ` Uttarwar, Sunil Prakashrao
  2022-10-17 13:53         ` David Marchand
  0 siblings, 1 reply; 28+ messages in thread
From: Uttarwar, Sunil Prakashrao @ 2022-10-17 13:42 UTC (permalink / raw)
  To: Akhil Goyal, David Marchand, dev

[Public]

Hi Akhil & David,

Regarding "crypto/ccp: fix PCI probing" patch, observing some issues while verifying on AMD platform(Floating point exception).

It seems there are some issues with this patch.

Thanks
Sunil

-----Original Message-----
From: Akhil Goyal <gakhil@marvell.com> 
Sent: Thursday, October 13, 2022 3:10 PM
To: Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>; David Marchand <david.marchand@redhat.com>; dev@dpdk.org
Subject: RE: [PATCH v2 0/4] crypto/ccp cleanup

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Hi Sunil

> For this series
>
> Below shared patches seems fine, verified on AMD platform works fine.
>
> crypto/ccp: fix IOVA handling
> crypto/ccp: remove some dead code for UIO
> crypto/ccp: remove some printf
>
> Now working on the "crypto/ccp: fix PCI probing" patch and will update on this.
>
Any update on this?
Please provide your ack if no comments.

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

* Re: [PATCH v2 0/4] crypto/ccp cleanup
  2022-10-17 13:42       ` Uttarwar, Sunil Prakashrao
@ 2022-10-17 13:53         ` David Marchand
  2022-10-26  6:21           ` David Marchand
  0 siblings, 1 reply; 28+ messages in thread
From: David Marchand @ 2022-10-17 13:53 UTC (permalink / raw)
  To: Uttarwar, Sunil Prakashrao; +Cc: Akhil Goyal, dev

Hello,

On Mon, Oct 17, 2022 at 3:42 PM Uttarwar, Sunil Prakashrao
<SunilPrakashrao.Uttarwar@amd.com> wrote:
>
> [Public]
>
> Hi Akhil & David,
>
> Regarding "crypto/ccp: fix PCI probing" patch, observing some issues while verifying on AMD platform(Floating point exception).
>
> It seems there are some issues with this patch.

Thanks for testing.
Can you provide a backtrace?


-- 
David Marchand


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

* Re: [PATCH v2 0/4] crypto/ccp cleanup
  2022-10-17 13:53         ` David Marchand
@ 2022-10-26  6:21           ` David Marchand
       [not found]             ` <1ec3f0fc-631f-2aa6-70f7-7f9b96caa2a2@amd.com>
  0 siblings, 1 reply; 28+ messages in thread
From: David Marchand @ 2022-10-26  6:21 UTC (permalink / raw)
  To: Uttarwar, Sunil Prakashrao; +Cc: Akhil Goyal, dev

On Mon, Oct 17, 2022 at 3:53 PM David Marchand
<david.marchand@redhat.com> wrote:
> On Mon, Oct 17, 2022 at 3:42 PM Uttarwar, Sunil Prakashrao
> <SunilPrakashrao.Uttarwar@amd.com> wrote:
> >
> > [Public]
> >
> > Hi Akhil & David,
> >
> > Regarding "crypto/ccp: fix PCI probing" patch, observing some issues while verifying on AMD platform(Floating point exception).
> >
> > It seems there are some issues with this patch.
>
> Thanks for testing.
> Can you provide a backtrace?

ping.


-- 
David Marchand


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

* Re: [PATCH v2 0/4] crypto/ccp cleanup
       [not found]                   ` <CY4PR1201MB01992328892E48006952D3C892399@CY4PR1201MB0199.namprd12.prod.outlook.com>
@ 2022-11-03 13:08                     ` David Marchand
  2022-11-18 11:58                       ` Uttarwar, Sunil Prakashrao
  0 siblings, 1 reply; 28+ messages in thread
From: David Marchand @ 2022-11-03 13:08 UTC (permalink / raw)
  To: Uttarwar, Sunil Prakashrao
  Cc: Yigit, Ferruh, Akhil Goyal, Namburu, Chandu-babu, Sebastian,
	Selwin, dev, Thomas Monjalon

Hello,

On Wed, Nov 2, 2022 at 2:54 PM Uttarwar, Sunil Prakashrao
<SunilPrakashrao.Uttarwar@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> Hi David,
>
> Please find below response.

Not sure why dev@ was dropped.
Adding it back.


>
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, November 2, 2022 6:18 PM
> To: Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>
> Cc: Yigit, Ferruh <Ferruh.Yigit@amd.com>; Akhil Goyal <gakhil@marvell.com>; Namburu, Chandu-babu <chandu@amd.com>
> Subject: Re: [PATCH v2 0/4] crypto/ccp cleanup
>
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> Hello,
>
> On Wed, Nov 2, 2022 at 11:26 AM Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com> wrote:
> > As mentioned earlier, observing issues with "crypto/ccp: fix PCI probing" patch (Floating point exception). Please find the below backtrace .
> >
> > (gdb) r -l 0,4 -n 4 -- --ptest throughput --buffer-sz 64 --burst-sz 32
> > --total-ops 3000 --silent --devtype crypto_ccp --optype cipher-only
> > --cipher-algo aes-cbc --cipher-op encrypt --cipher-key-sz 16
> > --cipher-iv-sz 16 Starting program: /home/cae/sunil/dpdk_main/dpdk/build/app/dpdk-test-crypto-perf -l 0,4 -n 4 -- --ptest throughput --buffer-sz 64 --burst-sz 32 --total-ops 3000 --silent --devtype crypto_ccp --optype cipher-only --cipher-algo aes-cbc --cipher-op encrypt --cipher-key-sz 16 --cipher-iv-sz 16 [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> > EAL: Detected CPU lcores: 24
> > EAL: Detected NUMA nodes: 2
> > EAL: Detected static linkage of DPDK
> > [New Thread 0x7ffff6dc5400 (LWP 171350)]
> > EAL: Multi-process socket /var/run/dpdk/rte/mp_socket [New Thread
> > 0x7ffff65c4400 (LWP 171351)]
> > EAL: Selected IOVA mode 'PA'
> > EAL: VFIO support initialized
> > [New Thread 0x7ffff5dc3400 (LWP 171352)]
> > EAL: Probe PCI driver: crypto_ccp (1022:1456) device: 0000:04:00.2
> > (socket 0)
> > PMD: Initialising 0000:04:00.2 on NUMA node 0
> > PMD: Max number of queue pairs = 8
> > PMD: Authentication offload to CCP
> > CRYPTODEV: User specified device name = 0000:04:00.2
> > CRYPTODEV: Creating cryptodev 0000:04:00.2
> > CRYPTODEV: Initialisation parameters - name: 0000:04:00.2,socket id:
> > 0, max queue pairs: 8
> > EAL: Probe PCI driver: crypto_ccp (1022:1468) device: 0000:05:00.1
> > (socket 0)
> > PMD: CCP PMD already initialized
> > EAL: Requested device 0000:05:00.1 cannot be used
> > EAL: Probe PCI driver: crypto_ccp (1022:1456) device: 0000:41:00.2
> > (socket 1)
> > PMD: CCP PMD already initialized
> > EAL: Requested device 0000:41:00.2 cannot be used
> > EAL: Probe PCI driver: crypto_ccp (1022:1468) device: 0000:42:00.1
> > (socket 1)
> > PMD: CCP PMD already initialized
> > EAL: Requested device 0000:42:00.1 cannot be used [New Thread
> > 0x7ffff55c2400 (LWP 171353)]
> > TELEMETRY: No legacy callbacks, legacy socket not created Allocated
> > pool "sess_mp_0" on socket 0
> >
> > Thread 4 "rte-worker-4" received signal SIGFPE, Arithmetic exception.
> > [Switching to Thread 0x7ffff5dc3400 (LWP 171352)] 0x000055555767397a
> > in ccp_pmd_enqueue_burst (queue_pair=0x17fefe940, ops=0x7ffff5dbe6e0, nb_ops=32) at ../drivers/crypto/ccp/rte_ccp_pmd.c:97
> > 97                      cur_ops = nb_ops / cryptodev_cnt + (nb_ops)%cryptodev_cnt;
> > (gdb) bt
>
> I have a hard time understanding the logic in this enqueue code...
>
> Is this driver exposing a single crypto device and will "balance"
> crypto operations across all pci devices on the system?
>
> Driver is exposing a single crypto device as physical device and only one device can be used by the driver for single instance for all operations.
>

Afaik, this is the only crypto driver that implements this.

I see two issues with the approach.
- only one DPDK application can use ccp crypto engines (PCI bus
allow/blocklist is not respected, right?),
- since only one crypto device is exposed, there is no way for the
application to dedicate/decide how to distribute crypto operations
over the different ccp crypto engines available on the system.


-- 
David Marchand


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

* RE: [PATCH v2 0/4] crypto/ccp cleanup
  2022-11-03 13:08                     ` David Marchand
@ 2022-11-18 11:58                       ` Uttarwar, Sunil Prakashrao
  2023-01-11 15:14                         ` David Marchand
  0 siblings, 1 reply; 28+ messages in thread
From: Uttarwar, Sunil Prakashrao @ 2022-11-18 11:58 UTC (permalink / raw)
  To: David Marchand
  Cc: Yigit, Ferruh, Akhil Goyal, Namburu, Chandu-babu, Sebastian,
	Selwin, dev, Thomas Monjalon

[AMD Official Use Only - General]

Hi David,

Please find the below update 

- only one DPDK application can use ccp crypto engines (PCI bus allow/blocklist is not respected, right?),
Yes, only one crypto device can be used in a DPDK application for the crypto operations. This is introduced from the patch crypto/ccp: convert driver from vdev to PCI. This is implemented as per community suggestion.

- since only one crypto device is exposed, there is no way for the application to dedicate/decide how to distribute crypto operations over the different ccp crypto engines available on the system.

When there is no ccp device passed from the application dpdk-test-crypto-perf, it tries to probe all CCP devices present on a system and only one device can be used. It seems this is bug in the patch implemented for crypto/ccp: convert driver from vdev to PCI and we are looking into this. 

Thanks
Sunil

-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Thursday, November 3, 2022 6:39 PM
To: Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>
Cc: Yigit, Ferruh <Ferruh.Yigit@amd.com>; Akhil Goyal <gakhil@marvell.com>; Namburu, Chandu-babu <chandu@amd.com>; Sebastian, Selwin <Selwin.Sebastian@amd.com>; dev <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v2 0/4] crypto/ccp cleanup

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Hello,

On Wed, Nov 2, 2022 at 2:54 PM Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> Hi David,
>
> Please find below response.

Not sure why dev@ was dropped.
Adding it back.


>
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Wednesday, November 2, 2022 6:18 PM
> To: Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>
> Cc: Yigit, Ferruh <Ferruh.Yigit@amd.com>; Akhil Goyal 
> <gakhil@marvell.com>; Namburu, Chandu-babu <chandu@amd.com>
> Subject: Re: [PATCH v2 0/4] crypto/ccp cleanup
>
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> Hello,
>
> On Wed, Nov 2, 2022 at 11:26 AM Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com> wrote:
> > As mentioned earlier, observing issues with "crypto/ccp: fix PCI probing" patch (Floating point exception). Please find the below backtrace .
> >
> > (gdb) r -l 0,4 -n 4 -- --ptest throughput --buffer-sz 64 --burst-sz 
> > 32 --total-ops 3000 --silent --devtype crypto_ccp --optype 
> > cipher-only --cipher-algo aes-cbc --cipher-op encrypt 
> > --cipher-key-sz 16 --cipher-iv-sz 16 Starting program: /home/cae/sunil/dpdk_main/dpdk/build/app/dpdk-test-crypto-perf -l 0,4 -n 4 -- --ptest throughput --buffer-sz 64 --burst-sz 32 --total-ops 3000 --silent --devtype crypto_ccp --optype cipher-only --cipher-algo aes-cbc --cipher-op encrypt --cipher-key-sz 16 --cipher-iv-sz 16 [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
> > EAL: Detected CPU lcores: 24
> > EAL: Detected NUMA nodes: 2
> > EAL: Detected static linkage of DPDK [New Thread 0x7ffff6dc5400 (LWP 
> > 171350)]
> > EAL: Multi-process socket /var/run/dpdk/rte/mp_socket [New Thread
> > 0x7ffff65c4400 (LWP 171351)]
> > EAL: Selected IOVA mode 'PA'
> > EAL: VFIO support initialized
> > [New Thread 0x7ffff5dc3400 (LWP 171352)]
> > EAL: Probe PCI driver: crypto_ccp (1022:1456) device: 0000:04:00.2 
> > (socket 0)
> > PMD: Initialising 0000:04:00.2 on NUMA node 0
> > PMD: Max number of queue pairs = 8
> > PMD: Authentication offload to CCP
> > CRYPTODEV: User specified device name = 0000:04:00.2
> > CRYPTODEV: Creating cryptodev 0000:04:00.2
> > CRYPTODEV: Initialisation parameters - name: 0000:04:00.2,socket id:
> > 0, max queue pairs: 8
> > EAL: Probe PCI driver: crypto_ccp (1022:1468) device: 0000:05:00.1 
> > (socket 0)
> > PMD: CCP PMD already initialized
> > EAL: Requested device 0000:05:00.1 cannot be used
> > EAL: Probe PCI driver: crypto_ccp (1022:1456) device: 0000:41:00.2 
> > (socket 1)
> > PMD: CCP PMD already initialized
> > EAL: Requested device 0000:41:00.2 cannot be used
> > EAL: Probe PCI driver: crypto_ccp (1022:1468) device: 0000:42:00.1 
> > (socket 1)
> > PMD: CCP PMD already initialized
> > EAL: Requested device 0000:42:00.1 cannot be used [New Thread
> > 0x7ffff55c2400 (LWP 171353)]
> > TELEMETRY: No legacy callbacks, legacy socket not created Allocated 
> > pool "sess_mp_0" on socket 0
> >
> > Thread 4 "rte-worker-4" received signal SIGFPE, Arithmetic exception.
> > [Switching to Thread 0x7ffff5dc3400 (LWP 171352)] 0x000055555767397a 
> > in ccp_pmd_enqueue_burst (queue_pair=0x17fefe940, ops=0x7ffff5dbe6e0, nb_ops=32) at ../drivers/crypto/ccp/rte_ccp_pmd.c:97
> > 97                      cur_ops = nb_ops / cryptodev_cnt + (nb_ops)%cryptodev_cnt;
> > (gdb) bt
>
> I have a hard time understanding the logic in this enqueue code...
>
> Is this driver exposing a single crypto device and will "balance"
> crypto operations across all pci devices on the system?
>
> Driver is exposing a single crypto device as physical device and only one device can be used by the driver for single instance for all operations.
>

Afaik, this is the only crypto driver that implements this.

I see two issues with the approach.
- only one DPDK application can use ccp crypto engines (PCI bus allow/blocklist is not respected, right?),
- since only one crypto device is exposed, there is no way for the application to dedicate/decide how to distribute crypto operations over the different ccp crypto engines available on the system.


--
David Marchand

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

* Re: [PATCH v2 0/4] crypto/ccp cleanup
  2022-11-18 11:58                       ` Uttarwar, Sunil Prakashrao
@ 2023-01-11 15:14                         ` David Marchand
  2023-01-12 12:28                           ` Uttarwar, Sunil Prakashrao
  0 siblings, 1 reply; 28+ messages in thread
From: David Marchand @ 2023-01-11 15:14 UTC (permalink / raw)
  To: Uttarwar, Sunil Prakashrao
  Cc: Yigit, Ferruh, Akhil Goyal, Namburu, Chandu-babu, Sebastian,
	Selwin, dev, Thomas Monjalon

On Fri, Nov 18, 2022 at 12:58 PM Uttarwar, Sunil Prakashrao
<SunilPrakashrao.Uttarwar@amd.com> wrote:
> Hi David,
>
> Please find the below update
>
> - only one DPDK application can use ccp crypto engines (PCI bus allow/blocklist is not respected, right?),
> Yes, only one crypto device can be used in a DPDK application for the crypto operations. This is introduced from the patch crypto/ccp: convert driver from vdev to PCI. This is implemented as per community suggestion.

Community suggested to have this driver a standard PCI driver, not a vdev one.
I don't remember anything about the limitation.
Can you point at the discussion that leaded to this?


>
> - since only one crypto device is exposed, there is no way for the application to dedicate/decide how to distribute crypto operations over the different ccp crypto engines available on the system.
>
> When there is no ccp device passed from the application dpdk-test-crypto-perf, it tries to probe all CCP devices present on a system and only one device can be used. It seems this is bug in the patch implemented for crypto/ccp: convert driver from vdev to PCI and we are looking into this.

Indeed.


So how should we proceed?
Patches 1 to 3 are ready and can be merged.

I don't mind dropping patch 4 if you have a better solution/alternative.


-- 
David Marchand


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

* RE: [PATCH v2 0/4] crypto/ccp cleanup
  2023-01-11 15:14                         ` David Marchand
@ 2023-01-12 12:28                           ` Uttarwar, Sunil Prakashrao
  2023-01-17 13:56                             ` Uttarwar, Sunil Prakashrao
  0 siblings, 1 reply; 28+ messages in thread
From: Uttarwar, Sunil Prakashrao @ 2023-01-12 12:28 UTC (permalink / raw)
  To: David Marchand
  Cc: Yigit, Ferruh, Akhil Goyal, Namburu, Chandu-babu, Sebastian,
	Selwin, dev, Thomas Monjalon

[AMD Official Use Only - General]

Hi David,

Please find the below update.

Thanks
Sunil

-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Wednesday, January 11, 2023 8:44 PM
To: Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>
Cc: Yigit, Ferruh <Ferruh.Yigit@amd.com>; Akhil Goyal <gakhil@marvell.com>; Namburu, Chandu-babu <chandu@amd.com>; Sebastian, Selwin <Selwin.Sebastian@amd.com>; dev <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v2 0/4] crypto/ccp cleanup

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Fri, Nov 18, 2022 at 12:58 PM Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com> wrote:
> Hi David,
>
> Please find the below update
>
> - only one DPDK application can use ccp crypto engines (PCI bus 
> allow/blocklist is not respected, right?), Yes, only one crypto device can be used in a DPDK application for the crypto operations. This is introduced from the patch crypto/ccp: convert driver from vdev to PCI. This is implemented as per community suggestion.

Community suggested to have this driver a standard PCI driver, not a vdev one.
I don't remember anything about the limitation.
Can you point at the discussion that leaded to this?

Sunil -> Please find the discussion about CCP driver as a PCI driver @ https://patches.dpdk.org/project/dpdk/patch/20201225080358.366162-1-asomalap@amd.com/

>
> - since only one crypto device is exposed, there is no way for the application to dedicate/decide how to distribute crypto operations over the different ccp crypto engines available on the system.
>
> When there is no ccp device passed from the application dpdk-test-crypto-perf, it tries to probe all CCP devices present on a system and only one device can be used. It seems this is bug in the patch implemented for crypto/ccp: convert driver from vdev to PCI and we are looking into this.

Indeed.


So how should we proceed?
Patches 1 to 3 are ready and can be merged.

I don't mind dropping patch 4 if you have a better solution/alternative.

Sunil->

As mentioned earlier, there is some issue with the patch https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-5-david.marchand@redhat.com/.

I worked on this patch and found issue. Did change in this patch and observing no issues. 

Still need some more time test, confirm changes and will share.

For now, we can merge below changes, I will ack it.
https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-2-david.marchand@redhat.com/
https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-3-david.marchand@redhat.com/
https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-4-david.marchand@redhat.com/

We will merge the below patch once we fix it with modifications 
https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-5-david.marchand@redhat.com/

Please let me know if any suggestions.

--
David Marchand

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

* RE: [PATCH v2 1/4] crypto/ccp: remove some printf
  2022-10-04  9:51   ` [PATCH v2 1/4] crypto/ccp: remove some printf David Marchand
@ 2023-01-13 11:58     ` Uttarwar, Sunil Prakashrao
  2023-01-30 18:42     ` [EXT] " Akhil Goyal
  1 sibling, 0 replies; 28+ messages in thread
From: Uttarwar, Sunil Prakashrao @ 2023-01-13 11:58 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: gakhil, stable, Kumar, Ravi1

[Public]

Acked-by: Sunil Uttarwar <sunilprakashrao.uttarwar@amd.com>

-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Tuesday, October 4, 2022 3:21 PM
To: dev@dpdk.org
Cc: gakhil@marvell.com; Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>; stable@dpdk.org; Kumar, Ravi1 <Ravi1.Kumar@amd.com>
Subject: [PATCH v2 1/4] crypto/ccp: remove some printf

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


A DPDK application must _not_ use printf.
Use log framework.

Fixes: ef4b04f87fa6 ("crypto/ccp: support device init")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ccp/ccp_dev.c     | 4 ++--
 drivers/crypto/ccp/ccp_pci.c     | 3 ++-
 drivers/crypto/ccp/rte_ccp_pmd.c | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c index 424ead82c3..9c9cb81236 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -362,7 +362,7 @@ ccp_find_lsb_regions(struct ccp_queue *cmd_q, uint64_t status)
                if (ccp_get_bit(&cmd_q->lsbmask, j))
                        weight++;

-       printf("Queue %d can access %d LSB regions  of mask  %lu\n",
+       CCP_LOG_DBG("Queue %d can access %d LSB regions  of mask  
+ %lu\n",
               (int)cmd_q->id, weight, cmd_q->lsbmask);

        return weight ? 0 : -EINVAL;
@@ -709,7 +709,7 @@ ccp_probe_devices(struct rte_pci_device *pci_dev,
                snprintf(dirname, sizeof(dirname), "%s/%s",
                             SYSFS_PCI_DEVICES, d->d_name);
                if (is_ccp_device(dirname, ccp_id, &ccp_type)) {
-                       printf("CCP : Detected CCP device with ID = 0x%x\n",
+                       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)
diff --git a/drivers/crypto/ccp/ccp_pci.c b/drivers/crypto/ccp/ccp_pci.c index 38029a9081..c941e222c7 100644
--- a/drivers/crypto/ccp/ccp_pci.c
+++ b/drivers/crypto/ccp/ccp_pci.c
@@ -11,6 +11,7 @@
 #include <rte_string_fns.h>

 #include "ccp_pci.h"
+#include "ccp_pmd_private.h"

 static const char * const uio_module_names[] = {
        "igb_uio",
@@ -41,7 +42,7 @@ ccp_check_pci_uio_module(void)
                rewind(fp);
        }
        fclose(fp);
-       printf("Insert igb_uio or uio_pci_generic kernel module(s)");
+       CCP_LOG_DBG("Insert igb_uio or uio_pci_generic kernel 
+ module(s)");
        return -1;/* uio not inserted */  }

diff --git a/drivers/crypto/ccp/rte_ccp_pmd.c b/drivers/crypto/ccp/rte_ccp_pmd.c
index 013f3be1e6..7338ef0ae8 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -250,7 +250,7 @@ cryptodev_ccp_create(const char *name,
                goto init_error;
        }

-       printf("CCP : Crypto device count = %d\n", cryptodev_cnt);
+       CCP_LOG_DBG("CCP : Crypto device count = %d\n", cryptodev_cnt);
        dev->device = &pci_dev->device;
        dev->device->driver = &pci_drv->driver;
        dev->driver_id = ccp_cryptodev_driver_id;
--
2.37.3

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

* RE: [PATCH v2 2/4] crypto/ccp: remove some dead code for UIO
  2022-10-04  9:51   ` [PATCH v2 2/4] crypto/ccp: remove some dead code for UIO David Marchand
@ 2023-01-13 12:00     ` Uttarwar, Sunil Prakashrao
  0 siblings, 0 replies; 28+ messages in thread
From: Uttarwar, Sunil Prakashrao @ 2023-01-13 12:00 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: gakhil, stable, Somalapuram, Amaranath

[Public]

Acked-by: Sunil Uttarwar <sunilprakashrao.uttarwar@amd.com>

-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Tuesday, October 4, 2022 3:22 PM
To: dev@dpdk.org
Cc: gakhil@marvell.com; Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>; stable@dpdk.org; Somalapuram, Amaranath <Amaranath.Somalapuram@amd.com>
Subject: [PATCH v2 2/4] crypto/ccp: remove some dead code for UIO

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


uio_fd is unused.

Fixes: 09a0fd736a08 ("crypto/ccp: enable IOMMU")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ccp/ccp_dev.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c index 9c9cb81236..410e62121e 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -653,7 +653,6 @@ static int
 ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)  {
        struct ccp_device *ccp_dev = NULL;
-       int uio_fd = -1;

        ccp_dev = rte_zmalloc("ccp_device", sizeof(*ccp_dev),
                              RTE_CACHE_LINE_SIZE); @@ -671,8 +670,6 @@ ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)
        return 0;
 fail:
        CCP_LOG_ERR("CCP Device probe failed");
-       if (uio_fd >= 0)
-               close(uio_fd);
        rte_free(ccp_dev);
        return -1;
 }
--
2.37.3

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

* RE: [PATCH v2 3/4] crypto/ccp: fix IOVA handling
  2022-10-04  9:51   ` [PATCH v2 3/4] crypto/ccp: fix IOVA handling David Marchand
@ 2023-01-13 12:00     ` Uttarwar, Sunil Prakashrao
  0 siblings, 0 replies; 28+ messages in thread
From: Uttarwar, Sunil Prakashrao @ 2023-01-13 12:00 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: gakhil, stable, Somalapuram, Amaranath

[Public]

Acked-by: Sunil Uttarwar <sunilprakashrao.uttarwar@amd.com>

-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Tuesday, October 4, 2022 3:22 PM
To: dev@dpdk.org
Cc: gakhil@marvell.com; Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>; stable@dpdk.org; Somalapuram, Amaranath <Amaranath.Somalapuram@amd.com>
Subject: [PATCH v2 3/4] crypto/ccp: fix IOVA handling

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Using IOVA or physical addresses is something that the user (via
--iova-mode=) or the bus code decides.

The crypto/ccp PCI driver should only use rte_mem_virt2iova.
It should not try to decide what to use solely based on the kmod the PCI device is bound to.

While at it, the global variable sha_ctx looks unsafe and unneeded.
Remove it.

Fixes: 09a0fd736a08 ("crypto/ccp: enable IOMMU")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/crypto/ccp/ccp_crypto.c  | 105 ++++++-------------------------
 drivers/crypto/ccp/ccp_dev.c     |   9 +--
 drivers/crypto/ccp/ccp_pci.c     |  34 ----------
 drivers/crypto/ccp/ccp_pci.h     |   3 -
 drivers/crypto/ccp/rte_ccp_pmd.c |   3 -
 5 files changed, 19 insertions(+), 135 deletions(-)

diff --git a/drivers/crypto/ccp/ccp_crypto.c b/drivers/crypto/ccp/ccp_crypto.c index 4bab18323b..351d8ac63e 100644
--- a/drivers/crypto/ccp/ccp_crypto.c
+++ b/drivers/crypto/ccp/ccp_crypto.c
@@ -33,8 +33,6 @@
 #include <openssl/err.h>
 #include <openssl/hmac.h>

-extern int iommu_mode;
-void *sha_ctx;
 /* SHA initial context values */
 uint32_t ccp_sha1_init[SHA_COMMON_DIGEST_SIZE / sizeof(uint32_t)] = {
        SHA1_H4, SHA1_H3,
@@ -748,13 +746,8 @@ ccp_configure_session_cipher(struct ccp_session *sess,
                CCP_LOG_ERR("Invalid CCP Engine");
                return -ENOTSUP;
        }
-       if (iommu_mode == 2) {
-               sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
-               sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
-       } else {
-               sess->cipher.nonce_phys = rte_mem_virt2phy(sess->cipher.nonce);
-               sess->cipher.key_phys = rte_mem_virt2phy(sess->cipher.key_ccp);
-       }
+       sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
+       sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
        return 0;
 }

@@ -793,7 +786,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
                sess->auth.ctx = (void *)ccp_sha1_init;
                sess->auth.ctx_len = CCP_SB_BYTES;
                sess->auth.offset = CCP_SB_BYTES - SHA1_DIGEST_SIZE;
-               rte_memcpy(sha_ctx, sess->auth.ctx, SHA_COMMON_DIGEST_SIZE);
                break;
        case RTE_CRYPTO_AUTH_SHA1_HMAC:
                if (sess->auth_opt) {
@@ -832,7 +824,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
                sess->auth.ctx = (void *)ccp_sha224_init;
                sess->auth.ctx_len = CCP_SB_BYTES;
                sess->auth.offset = CCP_SB_BYTES - SHA224_DIGEST_SIZE;
-               rte_memcpy(sha_ctx, sess->auth.ctx, SHA256_DIGEST_SIZE);
                break;
        case RTE_CRYPTO_AUTH_SHA224_HMAC:
                if (sess->auth_opt) {
@@ -895,7 +886,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
                sess->auth.ctx = (void *)ccp_sha256_init;
                sess->auth.ctx_len = CCP_SB_BYTES;
                sess->auth.offset = CCP_SB_BYTES - SHA256_DIGEST_SIZE;
-               rte_memcpy(sha_ctx, sess->auth.ctx, SHA256_DIGEST_SIZE);
                break;
        case RTE_CRYPTO_AUTH_SHA256_HMAC:
                if (sess->auth_opt) {
@@ -958,7 +948,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
                sess->auth.ctx = (void *)ccp_sha384_init;
                sess->auth.ctx_len = CCP_SB_BYTES << 1;
                sess->auth.offset = (CCP_SB_BYTES << 1) - SHA384_DIGEST_SIZE;
-               rte_memcpy(sha_ctx, sess->auth.ctx, SHA512_DIGEST_SIZE);
                break;
        case RTE_CRYPTO_AUTH_SHA384_HMAC:
                if (sess->auth_opt) {
@@ -1023,7 +1012,6 @@ ccp_configure_session_auth(struct ccp_session *sess,
                sess->auth.ctx = (void *)ccp_sha512_init;
                sess->auth.ctx_len = CCP_SB_BYTES << 1;
                sess->auth.offset = (CCP_SB_BYTES << 1) - SHA512_DIGEST_SIZE;
-               rte_memcpy(sha_ctx, sess->auth.ctx, SHA512_DIGEST_SIZE);
                break;
        case RTE_CRYPTO_AUTH_SHA512_HMAC:
                if (sess->auth_opt) {
@@ -1173,13 +1161,8 @@ ccp_configure_session_aead(struct ccp_session *sess,
                CCP_LOG_ERR("Unsupported aead algo");
                return -ENOTSUP;
        }
-       if (iommu_mode == 2) {
-               sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
-               sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
-       } else {
-               sess->cipher.nonce_phys = rte_mem_virt2phy(sess->cipher.nonce);
-               sess->cipher.key_phys = rte_mem_virt2phy(sess->cipher.key_ccp);
-       }
+       sess->cipher.nonce_phys = rte_mem_virt2iova(sess->cipher.nonce);
+       sess->cipher.key_phys = rte_mem_virt2iova(sess->cipher.key_ccp);
        return 0;
 }

@@ -1594,14 +1577,8 @@ ccp_perform_hmac(struct rte_crypto_op *op,
                                              op->sym->auth.data.offset);
        append_ptr = (void *)rte_pktmbuf_append(op->sym->m_src,
                                                session->auth.ctx_len);
-       if (iommu_mode == 2) {
-               dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
-               pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
-       } else {
-               dest_addr = (phys_addr_t)rte_mem_virt2phy(append_ptr);
-               pst.src_addr = (phys_addr_t)rte_mem_virt2phy((void *)addr);
-       }
-       dest_addr_t = dest_addr;
+       dest_addr_t = dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
+       pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);

        /** Load PHash1 to LSB*/
        pst.dest_addr = (phys_addr_t)(cmd_q->sb_sha * CCP_SB_BYTES); @@ -1683,10 +1660,7 @@ ccp_perform_hmac(struct rte_crypto_op *op,

        /** Load PHash2 to LSB*/
        addr += session->auth.ctx_len;
-       if (iommu_mode == 2)
-               pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
-       else
-               pst.src_addr = (phys_addr_t)rte_mem_virt2phy((void *)addr);
+       pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)addr);
        pst.dest_addr = (phys_addr_t)(cmd_q->sb_sha * CCP_SB_BYTES);
        pst.len = session->auth.ctx_len;
        pst.dir = 1;
@@ -1774,14 +1748,8 @@ ccp_perform_sha(struct rte_crypto_op *op,
                                              op->sym->auth.data.offset);
        append_ptr = (void *)rte_pktmbuf_append(op->sym->m_src,
                                                session->auth.ctx_len);
-       if (iommu_mode == 2) {
-               dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);
-               pst.src_addr = (phys_addr_t)sha_ctx;
-       } else {
-               dest_addr = (phys_addr_t)rte_mem_virt2phy(append_ptr);
-               pst.src_addr = (phys_addr_t)rte_mem_virt2phy((void *)
-                                                    session->auth.ctx);
-       }
+       pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *)session->auth.ctx);
+       dest_addr = (phys_addr_t)rte_mem_virt2iova(append_ptr);

        /** Passthru sha context*/

@@ -1871,15 +1839,8 @@ ccp_perform_sha3_hmac(struct rte_crypto_op *op,
                CCP_LOG_ERR("CCP MBUF append failed\n");
                return -1;
        }
-       if (iommu_mode == 2) {
-               dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
-               ctx_paddr = (phys_addr_t)rte_mem_virt2iova(
-                                       session->auth.pre_compute);
-       } else {
-               dest_addr = (phys_addr_t)rte_mem_virt2phy((void *)append_ptr);
-               ctx_paddr = (phys_addr_t)rte_mem_virt2phy(
-                                       session->auth.pre_compute);
-       }
+       dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
+       ctx_paddr = 
+ (phys_addr_t)rte_mem_virt2iova(session->auth.pre_compute);
        dest_addr_t = dest_addr + (session->auth.ctx_len / 2);
        desc = &cmd_q->qbase_desc[cmd_q->qidx];
        memset(desc, 0, Q_DESC_SIZE);
@@ -2017,13 +1978,8 @@ ccp_perform_sha3(struct rte_crypto_op *op,
                CCP_LOG_ERR("CCP MBUF append failed\n");
                return -1;
        }
-       if (iommu_mode == 2) {
-               dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
-               ctx_paddr = (phys_addr_t)rte_mem_virt2iova((void *)ctx_addr);
-       } else {
-               dest_addr = (phys_addr_t)rte_mem_virt2phy((void *)append_ptr);
-               ctx_paddr = (phys_addr_t)rte_mem_virt2phy((void *)ctx_addr);
-       }
+       dest_addr = (phys_addr_t)rte_mem_virt2iova((void *)append_ptr);
+       ctx_paddr = (phys_addr_t)rte_mem_virt2iova((void *)ctx_addr);

        ctx_addr = session->auth.sha3_ctx;

@@ -2099,13 +2055,7 @@ ccp_perform_aes_cmac(struct rte_crypto_op *op,

                ctx_addr = session->auth.pre_compute;
                memset(ctx_addr, 0, AES_BLOCK_SIZE);
-               if (iommu_mode == 2)
-                       pst.src_addr = (phys_addr_t)rte_mem_virt2iova(
-                                                       (void *)ctx_addr);
-               else
-                       pst.src_addr = (phys_addr_t)rte_mem_virt2phy(
-                                                       (void *)ctx_addr);
-
+               pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void 
+ *)ctx_addr);
                pst.dest_addr = (phys_addr_t)(cmd_q->sb_iv * CCP_SB_BYTES);
                pst.len = CCP_SB_BYTES;
                pst.dir = 1;
@@ -2143,12 +2093,7 @@ ccp_perform_aes_cmac(struct rte_crypto_op *op,
        } else {
                ctx_addr = session->auth.pre_compute + CCP_SB_BYTES;
                memset(ctx_addr, 0, AES_BLOCK_SIZE);
-               if (iommu_mode == 2)
-                       pst.src_addr = (phys_addr_t)rte_mem_virt2iova(
-                                                       (void *)ctx_addr);
-               else
-                       pst.src_addr = (phys_addr_t)rte_mem_virt2phy(
-                                                       (void *)ctx_addr);
+               pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void 
+ *)ctx_addr);
                pst.dest_addr = (phys_addr_t)(cmd_q->sb_iv * CCP_SB_BYTES);
                pst.len = CCP_SB_BYTES;
                pst.dir = 1;
@@ -2342,12 +2287,7 @@ ccp_perform_3des(struct rte_crypto_op *op,

                rte_memcpy(lsb_buf + (CCP_SB_BYTES - session->iv.length),
                           iv, session->iv.length);
-               if (iommu_mode == 2)
-                       pst.src_addr = (phys_addr_t)rte_mem_virt2iova(
-                                                       (void *) lsb_buf);
-               else
-                       pst.src_addr = (phys_addr_t)rte_mem_virt2phy(
-                                                       (void *) lsb_buf);
+               pst.src_addr = (phys_addr_t)rte_mem_virt2iova((void *) 
+ lsb_buf);
                pst.dest_addr = (phys_addr_t)(cmd_q->sb_iv * CCP_SB_BYTES);
                pst.len = CCP_SB_BYTES;
                pst.dir = 1;
@@ -2370,11 +2310,7 @@ ccp_perform_3des(struct rte_crypto_op *op,
        else
                dest_addr = src_addr;

-       if (iommu_mode == 2)
-               key_addr = rte_mem_virt2iova(session->cipher.key_ccp);
-       else
-               key_addr = rte_mem_virt2phy(session->cipher.key_ccp);
-
+       key_addr = rte_mem_virt2iova(session->cipher.key_ccp);
        desc = &cmd_q->qbase_desc[cmd_q->qidx];

        memset(desc, 0, Q_DESC_SIZE);
@@ -2768,12 +2704,7 @@ process_ops_to_enqueue(struct ccp_qp *qp,
        b_info->lsb_buf_idx = 0;
        b_info->desccnt = 0;
        b_info->cmd_q = cmd_q;
-       if (iommu_mode == 2)
-               b_info->lsb_buf_phys =
-                       (phys_addr_t)rte_mem_virt2iova((void *)b_info->lsb_buf);
-       else
-               b_info->lsb_buf_phys =
-                       (phys_addr_t)rte_mem_virt2phy((void *)b_info->lsb_buf);
+       b_info->lsb_buf_phys = (phys_addr_t)rte_mem_virt2iova((void 
+ *)b_info->lsb_buf);

        rte_atomic64_sub(&b_info->cmd_q->free_slots, slots_req);

diff --git a/drivers/crypto/ccp/ccp_dev.c b/drivers/crypto/ccp/ccp_dev.c index 410e62121e..14c54929c4 100644
--- a/drivers/crypto/ccp/ccp_dev.c
+++ b/drivers/crypto/ccp/ccp_dev.c
@@ -23,7 +23,6 @@
 #include "ccp_pci.h"
 #include "ccp_pmd_private.h"

-int iommu_mode;
 struct ccp_list ccp_list = TAILQ_HEAD_INITIALIZER(ccp_list);  static int ccp_dev_id;

@@ -652,7 +651,7 @@ is_ccp_device(const char *dirname,  static int  ccp_probe_device(int ccp_type, struct rte_pci_device *pci_dev)  {
-       struct ccp_device *ccp_dev = NULL;
+       struct ccp_device *ccp_dev;

        ccp_dev = rte_zmalloc("ccp_device", sizeof(*ccp_dev),
                              RTE_CACHE_LINE_SIZE); @@ -683,16 +682,10 @@ ccp_probe_devices(struct rte_pci_device *pci_dev,
        struct dirent *d;
        DIR *dir;
        int ret = 0;
-       int module_idx = 0;
        uint16_t domain;
        uint8_t bus, devid, function;
        char dirname[PATH_MAX];

-       module_idx = ccp_check_pci_uio_module();
-       if (module_idx < 0)
-               return -1;
-
-       iommu_mode = module_idx;
        TAILQ_INIT(&ccp_list);
        dir = opendir(SYSFS_PCI_DEVICES);
        if (dir == NULL)
diff --git a/drivers/crypto/ccp/ccp_pci.c b/drivers/crypto/ccp/ccp_pci.c index c941e222c7..bd1a037f76 100644
--- a/drivers/crypto/ccp/ccp_pci.c
+++ b/drivers/crypto/ccp/ccp_pci.c
@@ -11,40 +11,6 @@
 #include <rte_string_fns.h>

 #include "ccp_pci.h"
-#include "ccp_pmd_private.h"
-
-static const char * const uio_module_names[] = {
-       "igb_uio",
-       "uio_pci_generic",
-       "vfio_pci"
-};
-
-int
-ccp_check_pci_uio_module(void)
-{
-       FILE *fp;
-       int i;
-       char buf[BUFSIZ];
-
-       fp = fopen(PROC_MODULES, "r");
-       if (fp == NULL)
-               return -1;
-       i = 0;
-       while (uio_module_names[i] != NULL) {
-               while (fgets(buf, sizeof(buf), fp) != NULL) {
-                       if (!strncmp(buf, uio_module_names[i],
-                                    strlen(uio_module_names[i]))) {
-                               fclose(fp);
-                               return i;
-                       }
-               }
-               i++;
-               rewind(fp);
-       }
-       fclose(fp);
-       CCP_LOG_DBG("Insert igb_uio or uio_pci_generic kernel module(s)");
-       return -1;/* uio not inserted */
-}

 /*
  * split up a pci address into its constituent parts.
diff --git a/drivers/crypto/ccp/ccp_pci.h b/drivers/crypto/ccp/ccp_pci.h index 6736bf8ad3..d9a8b9dcc6 100644
--- a/drivers/crypto/ccp/ccp_pci.h
+++ b/drivers/crypto/ccp/ccp_pci.h
@@ -10,9 +10,6 @@
 #include <bus_pci_driver.h>

 #define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
-#define PROC_MODULES "/proc/modules"
-
-int ccp_check_pci_uio_module(void);

 int ccp_parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
                              uint8_t *bus, uint8_t *devid, uint8_t *function); diff --git a/drivers/crypto/ccp/rte_ccp_pmd.c b/drivers/crypto/ccp/rte_ccp_pmd.c
index 7338ef0ae8..8b3a5a304b 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -22,7 +22,6 @@
 static unsigned int ccp_pmd_init_done;
 uint8_t ccp_cryptodev_driver_id;
 uint8_t cryptodev_cnt;
-extern void *sha_ctx;

 struct ccp_pmd_init_params {
        struct rte_cryptodev_pmd_init_params def_p; @@ -213,7 +212,6 @@ cryptodev_ccp_remove(struct rte_pci_device *pci_dev)
                return -ENODEV;

        ccp_pmd_init_done = 0;
-       rte_free(sha_ctx);

        RTE_LOG(INFO, PMD, "Closing ccp device %s on numa socket %u\n",
                        name, rte_socket_id()); @@ -300,7 +298,6 @@ cryptodev_ccp_probe(struct rte_pci_driver *pci_drv __rte_unused,
                .auth_opt = CCP_PMD_AUTH_OPT_CCP,
        };

-       sha_ctx = (void *)rte_malloc(NULL, SHA512_DIGEST_SIZE, 64);
        if (ccp_pmd_init_done) {
                RTE_LOG(INFO, PMD, "CCP PMD already initialized\n");
                return -EFAULT;
--
2.37.3

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

* RE: [PATCH v2 0/4] crypto/ccp cleanup
  2023-01-12 12:28                           ` Uttarwar, Sunil Prakashrao
@ 2023-01-17 13:56                             ` Uttarwar, Sunil Prakashrao
  0 siblings, 0 replies; 28+ messages in thread
From: Uttarwar, Sunil Prakashrao @ 2023-01-17 13:56 UTC (permalink / raw)
  To: David Marchand
  Cc: Yigit, Ferruh, Akhil Goyal, Namburu, Chandu-babu, Sebastian,
	Selwin, dev, Thomas Monjalon

[AMD Official Use Only - General]

Hi David

Regarding patch  https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-5-david.marchand@redhat.com/

As mentioned earlier, observed floating point exception with using patch. Found issue that cryptodev_cnt variable is not getting updated and set to zero.

Please find the below change and share the modified patch, I will ack it.

--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -225,6 +225,8 @@ cryptodev_ccp_create(const char *name,
                CCP_LOG_ERR("failed to detect CCP crypto device");
                goto init_error;
        }
+       else
+               cryptodev_cnt++;

        CCP_LOG_DBG("CCP : Crypto device count = %d\n", cryptodev_cnt);
        dev->device = &pci_dev->device;



Thanks
Sunil

-----Original Message-----
From: Uttarwar, Sunil Prakashrao 
Sent: Thursday, January 12, 2023 5:58 PM
To: David Marchand <david.marchand@redhat.com>
Cc: Yigit, Ferruh <Ferruh.Yigit@amd.com>; Akhil Goyal <gakhil@marvell.com>; Namburu, Chandu-babu <chandu@amd.com>; Sebastian, Selwin <Selwin.Sebastian@amd.com>; dev <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>
Subject: RE: [PATCH v2 0/4] crypto/ccp cleanup

[AMD Official Use Only - General]

Hi David,

Please find the below update.

Thanks
Sunil

-----Original Message-----
From: David Marchand <david.marchand@redhat.com>
Sent: Wednesday, January 11, 2023 8:44 PM
To: Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>
Cc: Yigit, Ferruh <Ferruh.Yigit@amd.com>; Akhil Goyal <gakhil@marvell.com>; Namburu, Chandu-babu <chandu@amd.com>; Sebastian, Selwin <Selwin.Sebastian@amd.com>; dev <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH v2 0/4] crypto/ccp cleanup

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Fri, Nov 18, 2022 at 12:58 PM Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com> wrote:
> Hi David,
>
> Please find the below update
>
> - only one DPDK application can use ccp crypto engines (PCI bus 
> allow/blocklist is not respected, right?), Yes, only one crypto device can be used in a DPDK application for the crypto operations. This is introduced from the patch crypto/ccp: convert driver from vdev to PCI. This is implemented as per community suggestion.

Community suggested to have this driver a standard PCI driver, not a vdev one.
I don't remember anything about the limitation.
Can you point at the discussion that leaded to this?

Sunil -> Please find the discussion about CCP driver as a PCI driver @ https://patches.dpdk.org/project/dpdk/patch/20201225080358.366162-1-asomalap@amd.com/

>
> - since only one crypto device is exposed, there is no way for the application to dedicate/decide how to distribute crypto operations over the different ccp crypto engines available on the system.
>
> When there is no ccp device passed from the application dpdk-test-crypto-perf, it tries to probe all CCP devices present on a system and only one device can be used. It seems this is bug in the patch implemented for crypto/ccp: convert driver from vdev to PCI and we are looking into this.

Indeed.


So how should we proceed?
Patches 1 to 3 are ready and can be merged.

I don't mind dropping patch 4 if you have a better solution/alternative.

Sunil->

As mentioned earlier, there is some issue with the patch https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-5-david.marchand@redhat.com/.

I worked on this patch and found issue. Did change in this patch and observing no issues. 

Still need some more time test, confirm changes and will share.

For now, we can merge below changes, I will ack it.
https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-2-david.marchand@redhat.com/
https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-3-david.marchand@redhat.com/
https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-4-david.marchand@redhat.com/

We will merge the below patch once we fix it with modifications https://patches.dpdk.org/project/dpdk/patch/20221004095132.198777-5-david.marchand@redhat.com/

Please let me know if any suggestions.

--
David Marchand

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

* RE: [EXT] [PATCH v2 1/4] crypto/ccp: remove some printf
  2022-10-04  9:51   ` [PATCH v2 1/4] crypto/ccp: remove some printf David Marchand
  2023-01-13 11:58     ` Uttarwar, Sunil Prakashrao
@ 2023-01-30 18:42     ` Akhil Goyal
  1 sibling, 0 replies; 28+ messages in thread
From: Akhil Goyal @ 2023-01-30 18:42 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: sunilprakashrao.uttarwar, stable, Ravi Kumar

> A DPDK application must _not_ use printf.
> Use log framework.
> 
> Fixes: ef4b04f87fa6 ("crypto/ccp: support device init")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
First 3 patches of the series applied to dpdk-next-crypto

Please submit the last patch again with the required changes.

Thanks.

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

* [PATCH v2] crypto/ccp: fix PCI probing
  2022-09-09 15:04 [PATCH 0/4] crypto/ccp cleanup David Marchand
                   ` (4 preceding siblings ...)
  2022-10-04  9:51 ` [PATCH v2 0/4] crypto/ccp cleanup David Marchand
@ 2023-03-02 11:43 ` David Marchand
  2023-03-06 12:05   ` Uttarwar, Sunil Prakashrao
  5 siblings, 1 reply; 28+ messages in thread
From: David Marchand @ 2023-03-02 11:43 UTC (permalink / raw)
  To: dev; +Cc: stable, Sunil Uttarwar, Amaranath Somalapuram

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>
---
Changes since v2:
- restored cryptodev_cnt update,

---
 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 |  17 +--
 7 files changed, 26 insertions(+), 344 deletions(-)
 delete mode 100644 drivers/crypto/ccp/ccp_pci.c
 delete mode 100644 drivers/crypto/ccp/ccp_pci.h

diff --git a/drivers/crypto/ccp/ccp_crypto.c b/drivers/crypto/ccp/ccp_crypto.c
index 2758187d93..4b84b3303e 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 661a796116..a5271d7227 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -167,15 +167,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},
 };
 
@@ -228,12 +222,11 @@ 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;
 	}
+	cryptodev_cnt++;
 
 	CCP_LOG_DBG("CCP : Crypto device count = %d\n", cryptodev_cnt);
 	dev->device = &pci_dev->device;
-- 
2.39.2


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

* RE: [PATCH v2] crypto/ccp: fix PCI probing
  2023-03-02 11:43 ` [PATCH v2] crypto/ccp: fix PCI probing David Marchand
@ 2023-03-06 12:05   ` Uttarwar, Sunil Prakashrao
  2023-03-11 18:49     ` Akhil Goyal
  0 siblings, 1 reply; 28+ messages in thread
From: Uttarwar, Sunil Prakashrao @ 2023-03-06 12:05 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: stable, Somalapuram, Amaranath, Sebastian, Selwin

[Public]

Acked-by: Sunil Uttarwar <sunilprakashrao.uttarwar@amd.com>

-----Original Message-----
From: David Marchand <david.marchand@redhat.com> 
Sent: Thursday, March 2, 2023 5:14 PM
To: dev@dpdk.org
Cc: stable@dpdk.org; Uttarwar, Sunil Prakashrao <SunilPrakashrao.Uttarwar@amd.com>; Somalapuram, Amaranath <Amaranath.Somalapuram@amd.com>
Subject: [PATCH v2] crypto/ccp: fix PCI probing

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


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>
---
Changes since v2:
- restored cryptodev_cnt update,

---
 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 |  17 +--
 7 files changed, 26 insertions(+), 344 deletions(-)  delete mode 100644 drivers/crypto/ccp/ccp_pci.c  delete mode 100644 drivers/crypto/ccp/ccp_pci.h

diff --git a/drivers/crypto/ccp/ccp_crypto.c b/drivers/crypto/ccp/ccp_crypto.c index 2758187d93..4b84b3303e 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 661a796116..a5271d7227 100644
--- a/drivers/crypto/ccp/rte_ccp_pmd.c
+++ b/drivers/crypto/ccp/rte_ccp_pmd.c
@@ -167,15 +167,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},
 };

@@ -228,12 +222,11 @@ 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;
        }
+       cryptodev_cnt++;

        CCP_LOG_DBG("CCP : Crypto device count = %d\n", cryptodev_cnt);
        dev->device = &pci_dev->device;
--
2.39.2

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

* RE: [PATCH v2] crypto/ccp: fix PCI probing
  2023-03-06 12:05   ` Uttarwar, Sunil Prakashrao
@ 2023-03-11 18:49     ` Akhil Goyal
  0 siblings, 0 replies; 28+ messages in thread
From: Akhil Goyal @ 2023-03-11 18:49 UTC (permalink / raw)
  To: Uttarwar, Sunil Prakashrao, David Marchand, dev
  Cc: stable, Somalapuram, Amaranath, Sebastian, Selwin

> 
> Acked-by: Sunil Uttarwar <sunilprakashrao.uttarwar@amd.com>
> 
> Subject: [PATCH v2] crypto/ccp: fix PCI probing
> 
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
> 
> 
> 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>

Applied to dpdk-next-crypto.

Thanks.

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

end of thread, other threads:[~2023-03-11 18:50 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-09 15:04 [PATCH 0/4] crypto/ccp cleanup David Marchand
2022-09-09 15:04 ` [PATCH 1/4] crypto/ccp: remove some printf David Marchand
2022-09-09 15:04 ` [PATCH 2/4] crypto/ccp: remove some dead code for UIO David Marchand
2022-09-09 15:04 ` [PATCH 3/4] crypto/ccp: fix IOVA handling David Marchand
2022-09-09 15:04 ` [PATCH 4/4] crypto/ccp: fix PCI probing David Marchand
2022-10-04  9:51 ` [PATCH v2 0/4] crypto/ccp cleanup David Marchand
2022-10-04  9:51   ` [PATCH v2 1/4] crypto/ccp: remove some printf David Marchand
2023-01-13 11:58     ` Uttarwar, Sunil Prakashrao
2023-01-30 18:42     ` [EXT] " Akhil Goyal
2022-10-04  9:51   ` [PATCH v2 2/4] crypto/ccp: remove some dead code for UIO David Marchand
2023-01-13 12:00     ` Uttarwar, Sunil Prakashrao
2022-10-04  9:51   ` [PATCH v2 3/4] crypto/ccp: fix IOVA handling David Marchand
2023-01-13 12:00     ` Uttarwar, Sunil Prakashrao
2022-10-04  9:51   ` [PATCH v2 4/4] crypto/ccp: fix PCI probing David Marchand
2022-10-07  6:54   ` [EXT] [PATCH v2 0/4] crypto/ccp cleanup Akhil Goyal
2022-10-11 11:44   ` Uttarwar, Sunil Prakashrao
2022-10-13  9:40     ` Akhil Goyal
2022-10-17 13:42       ` Uttarwar, Sunil Prakashrao
2022-10-17 13:53         ` David Marchand
2022-10-26  6:21           ` David Marchand
     [not found]             ` <1ec3f0fc-631f-2aa6-70f7-7f9b96caa2a2@amd.com>
     [not found]               ` <CY4PR1201MB019989039CF117EE75F9025692399@CY4PR1201MB0199.namprd12.prod.outlook.com>
     [not found]                 ` <CAJFAV8xpZM96qDzaezO64LsRSdhWu8A2quCo8xGpufRrfjrMSw@mail.gmail.com>
     [not found]                   ` <CY4PR1201MB01992328892E48006952D3C892399@CY4PR1201MB0199.namprd12.prod.outlook.com>
2022-11-03 13:08                     ` David Marchand
2022-11-18 11:58                       ` Uttarwar, Sunil Prakashrao
2023-01-11 15:14                         ` David Marchand
2023-01-12 12:28                           ` Uttarwar, Sunil Prakashrao
2023-01-17 13:56                             ` Uttarwar, Sunil Prakashrao
2023-03-02 11:43 ` [PATCH v2] crypto/ccp: fix PCI probing David Marchand
2023-03-06 12:05   ` Uttarwar, Sunil Prakashrao
2023-03-11 18:49     ` Akhil Goyal

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