env/pci: method for registering PCI device providers

The primary motivation for this patch is to allow the VMD driver to be
notified of when users wants to attach a device under a given BDF and to
make it more similar to the regular PCI path.  Currently, the way the
VMD driver scans for the devices is a little bit different.  The initial
scan is done during initialization and there's a separate poller for
checking hotplugs. Also, there's no device_attach() interface, so with
hotplug poller disabled, it isn't possible to attach to a device not
present in the initial scan, even if the BDF is known.

This causes a few issues.  First of all, the VMD library isn't notified
when a device is stopped being used (i.e. user calls
spdk_pci_device_detach()), so when such a device is hotremoved, it never
gets unhooked.  But we cannot simply add a spdk_pci_device.detach()
callback, as this would break cases when user detaches a device (without
hotremove) and then tries to reattach it again (via
spdk_pci_device_attach()), as the VMD doesn't get notified about the
device_attach() call.

So, in order to resolve this, a device_attach() callback is added, which
will notify the VMD library that the user wants to attach a device under
a specific PCI address.  Then, in subsequent patches, a
spdk_pci_device_provider.detach_cb() callback is added to make sure that
devices are unhooked once they're no longer used.

Once that is done, it'll be also possible to get rid of the VMD hotplug
poller by adding something like scan_cb() to spdk_pci_device_provider and
call it from spdk_pci_enumerate().

Signed-off-by: Konrad Sztyber <konrad.sztyber@intel.com>
Change-Id: I084a27dcd12455f0f841440b7692375e80d07e84
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/13883
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Tom Nabarro <tom.nabarro@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>

include/spdk/env.h

@@ -1168,6 +1168,34 @@ void spdk_pci_unhook_device(struct spdk_pci_device *dev);
  */
 const char *spdk_pci_device_get_type(const struct spdk_pci_device *dev);
 
+struct spdk_pci_device_provider {
+	const char *name;
+
+	/**
+	 * Callback executed to attach a PCI device on a given address.
+	 *
+	 * \param addr address of the device.
+	 *
+	 * \return 0 if the device was attached successfully, negative errno otherwise.
+	 */
+	int (*attach_cb)(const struct spdk_pci_addr *addr);
+
+	TAILQ_ENTRY(spdk_pci_device_provider) tailq;
+};
+
+/**
+ * Register a PCI device provdier.
+ *
+ * \param provider PCI device provider.
+ */
+void spdk_pci_register_device_provider(struct spdk_pci_device_provider *provider);
+
+#define SPDK_PCI_REGISTER_DEVICE_PROVIDER(name, provider) \
+	static void __attribute__((constructor)) _spdk_pci_register_device_provider_##name(void) \
+	{ \
+		spdk_pci_register_device_provider(provider); \
+	}
+
 /**
  * Remove any CPU affinity from the current thread.
  */

lib/env_dpdk/Makefile

@@ -7,7 +7,7 @@ SPDK_ROOT_DIR := $(abspath $(CURDIR)/../..)
 include $(SPDK_ROOT_DIR)/mk/spdk.common.mk
 
 SO_VER := 9
-SO_MINOR := 0
+SO_MINOR := 1
 
 CFLAGS += $(ENV_CFLAGS)
 C_SRCS = env.c memory.c pci.c init.c threads.c

lib/env_dpdk/pci.c

@@ -35,6 +35,8 @@ static TAILQ_HEAD(, spdk_pci_device) g_pci_devices = TAILQ_HEAD_INITIALIZER(g_pc
 static TAILQ_HEAD(, spdk_pci_device) g_pci_hotplugged_devices =
 	TAILQ_HEAD_INITIALIZER(g_pci_hotplugged_devices);
 static TAILQ_HEAD(, spdk_pci_driver) g_pci_drivers = TAILQ_HEAD_INITIALIZER(g_pci_drivers);
+static TAILQ_HEAD(, spdk_pci_device_provider) g_pci_device_providers =
+	TAILQ_HEAD_INITIALIZER(g_pci_device_providers);
 
 struct env_devargs {
 	struct rte_bus	*bus;
@@ -606,18 +608,45 @@ scan_pci_bus(bool delay_init)
 	return 0;
 }
 
+static int
+pci_attach_rte(const struct spdk_pci_addr *addr)
+{
+	char bdf[32];
+	int rc, i = 0;
+
+	spdk_pci_addr_fmt(bdf, sizeof(bdf), addr);
+
+	do {
+		rc = rte_eal_hotplug_add("pci", bdf, "");
+	} while (rc == -ENOMSG && ++i <= DPDK_HOTPLUG_RETRY_COUNT);
+
+	if (i > 1 && rc == -EEXIST) {
+		/* Even though the previous request timed out, the device
+		 * was attached successfully.
+		 */
+		rc = 0;
+	}
+
+	return rc;
+}
+
+static struct spdk_pci_device_provider g_pci_rte_provider = {
+	.name = "pci",
+	.attach_cb = pci_attach_rte,
+};
+
+SPDK_PCI_REGISTER_DEVICE_PROVIDER(pci, &g_pci_rte_provider);
+
 int
 spdk_pci_device_attach(struct spdk_pci_driver *driver,
 		       spdk_pci_enum_cb enum_cb,
 		       void *enum_ctx, struct spdk_pci_addr *pci_address)
 {
 	struct spdk_pci_device *dev;
+	struct spdk_pci_device_provider *provider;
 	struct rte_pci_device *rte_dev;
 	struct rte_devargs *da;
 	int rc;
-	char bdf[32];
-
-	spdk_pci_addr_fmt(bdf, sizeof(bdf), pci_address);
 
 	cleanup_pci_devices();
 
@@ -645,17 +674,12 @@ spdk_pci_device_attach(struct spdk_pci_driver *driver,
 	driver->cb_fn = enum_cb;
 	driver->cb_arg = enum_ctx;
 
-	int i = 0;
-
-	do {
-		rc = rte_eal_hotplug_add("pci", bdf, "");
-	} while (rc == -ENOMSG && ++i <= DPDK_HOTPLUG_RETRY_COUNT);
-
-	if (i > 1 && rc == -EEXIST) {
-		/* Even though the previous request timed out, the device
-		 * was attached successfully.
-		 */
-		rc = 0;
+	rc = -ENODEV;
+	TAILQ_FOREACH(provider, &g_pci_device_providers, tailq) {
+		rc = provider->attach_cb(pci_address);
+		if (rc == 0) {
+			break;
+		}
 	}
 
 	driver->cb_arg = NULL;
@@ -678,10 +702,12 @@ spdk_pci_device_attach(struct spdk_pci_driver *driver,
 	assert(dev != NULL);
 
 	rte_dev = dev->dev_handle;
-	da = rte_dev->device.devargs;
-	if (da && get_allowed_at(da)) {
-		set_allowed_at(da, spdk_get_ticks());
-		da->policy = RTE_DEV_ALLOWED;
+	if (rte_dev != NULL) {
+		da = rte_dev->device.devargs;
+		if (da && get_allowed_at(da)) {
+			set_allowed_at(da, spdk_get_ticks());
+			da->policy = RTE_DEV_ALLOWED;
+		}
 	}
 
 	return 0;
@@ -1139,6 +1165,12 @@ spdk_pci_unhook_device(struct spdk_pci_device *dev)
 	TAILQ_REMOVE(&g_pci_devices, dev, internal.tailq);
 }
 
+void
+spdk_pci_register_device_provider(struct spdk_pci_device_provider *provider)
+{
+	TAILQ_INSERT_TAIL(&g_pci_device_providers, provider, tailq);
+}
+
 const char *
 spdk_pci_device_get_type(const struct spdk_pci_device *dev)
 {

lib/env_dpdk/spdk_env_dpdk.map

@@ -97,6 +97,7 @@
 	spdk_pci_hook_device;
 	spdk_pci_unhook_device;
 	spdk_pci_device_get_type;
+	spdk_pci_register_device_provider;
 	spdk_unaffinitize_thread;
 	spdk_call_unaffinitized;
 	spdk_mem_map_alloc;