From bbd7e1c4da0ede52771cf4a35685b5edbaa5a0c7 Mon Sep 17 00:00:00 2001
From: Daniel Verkamp <daniel.verkamp@intel.com>
Date: Mon, 31 Oct 2016 16:29:52 -0700
Subject: [PATCH] env: add spdk_pci_addr_parse()

Add a helper function that converts a PCI address from a string into a
struct spdk_pci_addr and use it in place of the various sscanf()
invocations throughout SPDK.

Change-Id: Id2749723f76db741567e01b4bcb0fffb0e425fcd
Signed-off-by: Daniel Verkamp <daniel.verkamp@intel.com>
---
 app/nvmf_tgt/conf.c                     | 25 +++++-------------
 examples/nvme/nvme_manage/nvme_manage.c | 20 +--------------
 include/spdk/env.h                      | 10 ++++++++
 lib/bdev/nvme/blockdev_nvme.c           | 12 ++-------
 lib/bdev/nvme/blockdev_nvme_rpc.c       | 11 ++------
 lib/copy/ioat/copy_engine_ioat.c        | 11 ++++----
 lib/env/pci.c                           | 34 +++++++++++++++++++++++++
 7 files changed, 61 insertions(+), 62 deletions(-)

diff --git a/app/nvmf_tgt/conf.c b/app/nvmf_tgt/conf.c
index ad28df4c9..95ff6ef06 100644
--- a/app/nvmf_tgt/conf.c
+++ b/app/nvmf_tgt/conf.c
@@ -62,10 +62,7 @@ struct spdk_nvmf_probe_ctx {
 	struct spdk_nvmf_subsystem	*subsystem;
 	bool				any;
 	bool				found;
-	uint32_t			domain;
-	uint32_t			bus;
-	uint32_t			device;
-	uint32_t			function;
+	struct spdk_pci_addr		pci_addr;
 };
 
 #define MAX_STRING_LEN 255
@@ -340,20 +337,14 @@ static bool
 probe_cb(void *cb_ctx, struct spdk_pci_device *dev, struct spdk_nvme_ctrlr_opts *opts)
 {
 	struct spdk_nvmf_probe_ctx *ctx = cb_ctx;
-	uint16_t found_domain = spdk_pci_device_get_domain(dev);
-	uint8_t found_bus    = spdk_pci_device_get_bus(dev);
-	uint8_t found_dev    = spdk_pci_device_get_dev(dev);
-	uint8_t found_func   = spdk_pci_device_get_func(dev);
+	struct spdk_pci_addr pci_addr = spdk_pci_device_get_addr(dev);
 
 	if (ctx->any && !ctx->found) {
 		ctx->found = true;
 		return true;
 	}
 
-	if (found_domain == ctx->domain &&
-	    found_bus == ctx->bus &&
-	    found_dev == ctx->device &&
-	    found_func == ctx->function) {
+	if (spdk_pci_addr_compare(&pci_addr, &ctx->pci_addr) == 0) {
 		ctx->found = true;
 		return true;
 	}
@@ -549,8 +540,7 @@ spdk_nvmf_parse_subsystem(struct spdk_conf_section *sp)
 		if (strcmp(bdf, "*") == 0) {
 			ctx.any = true;
 		} else {
-			ret = sscanf(bdf, "%x:%x:%x.%x", &ctx.domain, &ctx.bus, &ctx.device, &ctx.function);
-			if (ret != 4) {
+			if (spdk_pci_addr_parse(&ctx.pci_addr, bdf) < 0) {
 				SPDK_ERRLOG("Invalid format for NVMe BDF: %s\n", bdf);
 				return -1;
 			}
@@ -667,7 +657,7 @@ spdk_nvmf_parse_subsystem_for_rpc(const char *name,
 	struct spdk_nvmf_subsystem *subsystem;
 	struct nvmf_tgt_subsystem *app_subsys;
 	enum spdk_nvmf_subsystem_mode mode;
-	int i, ret;
+	int i;
 	uint64_t mask;
 	int num = 0;
 
@@ -757,8 +747,7 @@ spdk_nvmf_parse_subsystem_for_rpc(const char *name,
 		if (strcmp(bdf, "*") == 0) {
 			ctx.any = true;
 		} else {
-			ret = sscanf(bdf, "%x:%x:%x.%x", &ctx.domain, &ctx.bus, &ctx.device, &ctx.function);
-			if (ret != 4) {
+			if (spdk_pci_addr_parse(&ctx.pci_addr, bdf) < 0) {
 				SPDK_ERRLOG("Invalid format for NVMe BDF: %s\n", bdf);
 				return -1;
 			}
@@ -771,7 +760,7 @@ spdk_nvmf_parse_subsystem_for_rpc(const char *name,
 
 		if (!ctx.found) {
 			SPDK_ERRLOG("Could not find NVMe controller at PCI address %04x:%02x:%02x.%x\n",
-				    ctx.domain, ctx.bus, ctx.device, ctx.function);
+				    ctx.pci_addr.domain, ctx.pci_addr.bus, ctx.pci_addr.dev, ctx.pci_addr.func);
 			return -1;
 		}
 	} else {
diff --git a/examples/nvme/nvme_manage/nvme_manage.c b/examples/nvme/nvme_manage/nvme_manage.c
index 16f2b352b..0525656bf 100644
--- a/examples/nvme/nvme_manage/nvme_manage.c
+++ b/examples/nvme/nvme_manage/nvme_manage.c
@@ -299,10 +299,6 @@ display_controller_list(void)
 static struct dev *
 get_controller(void)
 {
-	unsigned int				domain;
-	unsigned int				bus;
-	unsigned int				devid;
-	unsigned int				function;
 	struct spdk_pci_addr			pci_addr;
 	char					address[64];
 	char					*p;
@@ -327,24 +323,10 @@ get_controller(void)
 		p++;
 	}
 
-	if (sscanf(p, "%x:%x:%x.%x", &domain, &bus, &devid, &function) == 4) {
-		/* Matched a full address - all variables are initialized */
-	} else if (sscanf(p, "%x:%x:%x", &domain, &bus, &devid) == 3) {
-		function = 0;
-	} else if (sscanf(p, "%x:%x.%x", &bus, &devid, &function) == 3) {
-		domain = 0;
-	} else if (sscanf(p, "%x:%x", &bus, &devid) == 2) {
-		domain = 0;
-		function = 0;
-	} else {
+	if (spdk_pci_addr_parse(&pci_addr, p) < 0) {
 		return NULL;
 	}
 
-	pci_addr.domain = domain;
-	pci_addr.bus = bus;
-	pci_addr.dev = devid;
-	pci_addr.func = function;
-
 	foreach_dev(iter) {
 		struct spdk_pci_addr iter_addr = spdk_pci_device_get_addr(iter->pci_dev);
 
diff --git a/include/spdk/env.h b/include/spdk/env.h
index f944019f7..6b236940c 100644
--- a/include/spdk/env.h
+++ b/include/spdk/env.h
@@ -209,6 +209,16 @@ int spdk_pci_device_cfg_write32(struct spdk_pci_device *dev, uint32_t value, uin
  */
 int spdk_pci_addr_compare(const struct spdk_pci_addr *a1, const struct spdk_pci_addr *a2);
 
+/**
+ * Convert a string representation of a PCI address into a struct spdk_pci_addr.
+ *
+ * \param addr PCI adddress output on success
+ * \param bdf PCI address in domain:bus:device.function format
+ *
+ * \return 0 on success, or a negated errno value on failure.
+ */
+int spdk_pci_addr_parse(struct spdk_pci_addr *addr, const char *bdf);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/bdev/nvme/blockdev_nvme.c b/lib/bdev/nvme/blockdev_nvme.c
index 0b1efe532..2d592ccfc 100644
--- a/lib/bdev/nvme/blockdev_nvme.c
+++ b/lib/bdev/nvme/blockdev_nvme.c
@@ -453,7 +453,7 @@ nvme_library_init(void)
 {
 	struct spdk_conf_section *sp;
 	const char *val;
-	int i, rc;
+	int i;
 	struct nvme_probe_ctx probe_ctx;
 
 	sp = spdk_conf_find_section(NULL, "Nvme");
@@ -496,24 +496,16 @@ nvme_library_init(void)
 
 	if (num_controllers > 0) {
 		for (i = 0; ; i++) {
-			unsigned int domain, bus, dev, func;
-
 			val = spdk_conf_section_get_nmval(sp, "BDF", i, 0);
 			if (val == NULL) {
 				break;
 			}
 
-			rc = sscanf(val, "%x:%x:%x.%x", &domain, &bus, &dev, &func);
-			if (rc != 4) {
+			if (spdk_pci_addr_parse(&probe_ctx.whitelist[probe_ctx.num_whitelist_controllers], val) < 0) {
 				SPDK_ERRLOG("Invalid format for BDF: %s\n", val);
 				return -1;
 			}
 
-			probe_ctx.whitelist[probe_ctx.num_whitelist_controllers].domain = domain;
-			probe_ctx.whitelist[probe_ctx.num_whitelist_controllers].bus = bus;
-			probe_ctx.whitelist[probe_ctx.num_whitelist_controllers].dev = dev;
-			probe_ctx.whitelist[probe_ctx.num_whitelist_controllers].func = func;
-
 			probe_ctx.num_whitelist_controllers++;
 		}
 	}
diff --git a/lib/bdev/nvme/blockdev_nvme_rpc.c b/lib/bdev/nvme/blockdev_nvme_rpc.c
index 8e68388d0..1fc4ce8e0 100644
--- a/lib/bdev/nvme/blockdev_nvme_rpc.c
+++ b/lib/bdev/nvme/blockdev_nvme_rpc.c
@@ -57,8 +57,7 @@ spdk_rpc_construct_nvme_bdev(struct spdk_jsonrpc_server_conn *conn,
 	struct rpc_construct_nvme req = {};
 	struct spdk_json_write_ctx *w;
 	struct nvme_probe_ctx ctx = {};
-	unsigned int domain, bus, dev, func;
-	int rc, i;
+	int i;
 
 	if (spdk_json_decode_object(params, rpc_construct_nvme_decoders,
 				    sizeof(rpc_construct_nvme_decoders) / sizeof(*rpc_construct_nvme_decoders),
@@ -70,16 +69,10 @@ spdk_rpc_construct_nvme_bdev(struct spdk_jsonrpc_server_conn *conn,
 	ctx.controllers_remaining = 1;
 	ctx.num_whitelist_controllers = 1;
 
-	rc = sscanf(req.pci_address, "%x:%x:%x.%x", &domain, &bus, &dev, &func);
-	if (rc != 4) {
+	if (spdk_pci_addr_parse(&ctx.whitelist[0], req.pci_address) < 0) {
 		goto invalid;
 	}
 
-	ctx.whitelist[0].domain = domain;
-	ctx.whitelist[0].bus = bus;
-	ctx.whitelist[0].dev = dev;
-	ctx.whitelist[0].func = func;
-
 	if (spdk_bdev_nvme_create(&ctx)) {
 		goto invalid;
 	}
diff --git a/lib/copy/ioat/copy_engine_ioat.c b/lib/copy/ioat/copy_engine_ioat.c
index d0393b67b..77e1d2125 100644
--- a/lib/copy/ioat/copy_engine_ioat.c
+++ b/lib/copy/ioat/copy_engine_ioat.c
@@ -279,7 +279,6 @@ copy_engine_ioat_init(void)
 	const char *val, *pci_bdf;
 	int i;
 	struct ioat_probe_ctx probe_ctx = {};
-	unsigned bus, dev, func;
 
 	if (sp != NULL) {
 		val = spdk_conf_section_get_val(sp, "Disable");
@@ -294,11 +293,11 @@ copy_engine_ioat_init(void)
 			pci_bdf = spdk_conf_section_get_nmval(sp, "Whitelist", i, 0);
 			if (!pci_bdf)
 				break;
-			sscanf(pci_bdf, "%02x:%02x.%1u", &bus, &dev, &func);
-			probe_ctx.whitelist[probe_ctx.num_whitelist_devices].domain = 0;
-			probe_ctx.whitelist[probe_ctx.num_whitelist_devices].bus = bus;
-			probe_ctx.whitelist[probe_ctx.num_whitelist_devices].dev = dev;
-			probe_ctx.whitelist[probe_ctx.num_whitelist_devices].func = func;
+
+			if (spdk_pci_addr_parse(&probe_ctx.whitelist[probe_ctx.num_whitelist_devices], pci_bdf) < 0) {
+				SPDK_ERRLOG("Invalid Ioat Whitelist address %s\n", pci_bdf);
+				return -1;
+			}
 			probe_ctx.num_whitelist_devices++;
 		}
 	}
diff --git a/lib/env/pci.c b/lib/env/pci.c
index 53c78fade..a988ca181 100644
--- a/lib/env/pci.c
+++ b/lib/env/pci.c
@@ -505,3 +505,37 @@ spdk_pci_device_claim(const struct spdk_pci_addr *pci_addr)
 	return 0;
 }
 #endif /* __FreeBSD__ */
+
+int
+spdk_pci_addr_parse(struct spdk_pci_addr *addr, const char *bdf)
+{
+	unsigned domain, bus, dev, func;
+
+	if (addr == NULL || bdf == NULL) {
+		return -EINVAL;
+	}
+
+	if (sscanf(bdf, "%x:%x:%x.%x", &domain, &bus, &dev, &func) == 4) {
+		/* Matched a full address - all variables are initialized */
+	} else if (sscanf(bdf, "%x:%x:%x", &domain, &bus, &dev) == 3) {
+		func = 0;
+	} else if (sscanf(bdf, "%x:%x.%x", &bus, &dev, &func) == 3) {
+		domain = 0;
+	} else if (sscanf(bdf, "%x:%x", &bus, &dev) == 2) {
+		domain = 0;
+		func = 0;
+	} else {
+		return -EINVAL;
+	}
+
+	if (domain > 0xFFFF || bus > 0xFF || dev > 0x1F || func > 7) {
+		return -EINVAL;
+	}
+
+	addr->domain = domain;
+	addr->bus = bus;
+	addr->dev = dev;
+	addr->func = func;
+
+	return 0;
+}