[v5,3/5] eal: add bus pointer in device structure

Message ID 20181003231046.26772-4-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series eal: simplify devargs and hotplug functions |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Thomas Monjalon Oct. 3, 2018, 11:10 p.m. UTC
  When a device is added with a devargs (hotplug or whitelist),
the bus pointer can be retrieved via its devargs.
But there is no such devargs.bus in case of standard scan.

A pointer to the rte_bus handle is added to rte_device.
When a device is allocated (during a scan),
the pointer to its bus is assigned.

It will make possible to remove a rte_device,
using the function pointer from its bus.

The function rte_bus_find_by_device() becomes useless,
and may be removed later.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 doc/guides/rel_notes/release_18_11.rst  | 2 ++
 drivers/bus/dpaa/dpaa_bus.c             | 2 ++
 drivers/bus/fslmc/fslmc_bus.c           | 2 ++
 drivers/bus/ifpga/ifpga_bus.c           | 1 +
 drivers/bus/pci/bsd/pci.c               | 2 ++
 drivers/bus/pci/linux/pci.c             | 1 +
 drivers/bus/pci/private.h               | 2 ++
 drivers/bus/vdev/vdev.c                 | 1 +
 drivers/bus/vmbus/linux/vmbus_bus.c     | 1 +
 drivers/bus/vmbus/private.h             | 3 +++
 lib/librte_eal/common/include/rte_dev.h | 1 +
 11 files changed, 18 insertions(+)
  

Comments

Gaëtan Rivet Oct. 4, 2018, 9:31 a.m. UTC | #1
On Thu, Oct 04, 2018 at 01:10:44AM +0200, Thomas Monjalon wrote:
> When a device is added with a devargs (hotplug or whitelist),
> the bus pointer can be retrieved via its devargs.
> But there is no such devargs.bus in case of standard scan.
> 
> A pointer to the rte_bus handle is added to rte_device.
> When a device is allocated (during a scan),
> the pointer to its bus is assigned.
> 
> It will make possible to remove a rte_device,
> using the function pointer from its bus.
> 
> The function rte_bus_find_by_device() becomes useless,
> and may be removed later.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

I agree with this change, but I think this can break ABI of
buses defining their structure by composition with rte_device (e.g. PCI
bus). Have you checked ABI?

Personally I don't care, I prefer a clean framework to a littered lib.

Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
  
Thomas Monjalon Oct. 4, 2018, 9:48 a.m. UTC | #2
04/10/2018 11:31, Gaëtan Rivet:
> On Thu, Oct 04, 2018 at 01:10:44AM +0200, Thomas Monjalon wrote:
> > When a device is added with a devargs (hotplug or whitelist),
> > the bus pointer can be retrieved via its devargs.
> > But there is no such devargs.bus in case of standard scan.
> > 
> > A pointer to the rte_bus handle is added to rte_device.
> > When a device is allocated (during a scan),
> > the pointer to its bus is assigned.
> > 
> > It will make possible to remove a rte_device,
> > using the function pointer from its bus.
> > 
> > The function rte_bus_find_by_device() becomes useless,
> > and may be removed later.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> I agree with this change, but I think this can break ABI of
> buses defining their structure by composition with rte_device (e.g. PCI
> bus). Have you checked ABI?

Yes I forgot it changes the size of the bus structures.
I can spin a v6 with a bump of ABI version of the bus drivers,
and an additional note in release notes.

> Personally I don't care, I prefer a clean framework to a littered lib.
> 
> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Adding bus drivers maintainers to get more opinions.
  

Patch

diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index d534bb71c..2c6791e5e 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -164,6 +164,8 @@  ABI Changes
        ``rte_config`` structure on account of improving DPDK usability when
        using either ``--legacy-mem`` or ``--single-file-segments`` flags.
 
+* eal: The structure ``rte_device`` got a new field to reference a ``rte_bus``.
+
 
 Removed Items
 -------------
diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 49cd04dbb..138e0f98d 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -165,6 +165,8 @@  dpaa_create_device_list(void)
 			goto cleanup;
 		}
 
+		dev->device.bus = &rte_dpaa_bus.bus;
+
 		cfg = &dpaa_netcfg->port_cfg[i];
 		fman_intf = cfg->fman_if;
 
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index bfe81e236..960f55071 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -161,6 +161,8 @@  scan_one_fslmc_device(char *dev_name)
 		return -ENOMEM;
 	}
 
+	dev->device.bus = &rte_fslmc_bus.bus;
+
 	/* Parse the device name and ID */
 	t_ptr = strtok(dup_dev_name, ".");
 	if (!t_ptr) {
diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index 3ef035b7e..80663328a 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -142,6 +142,7 @@  ifpga_scan_one(struct rte_rawdev *rawdev,
 	if (!afu_dev)
 		goto end;
 
+	afu_dev->device.bus = &rte_ifpga_bus;
 	afu_dev->device.devargs = devargs;
 	afu_dev->device.numa_node = SOCKET_ID_ANY;
 	afu_dev->device.name = devargs->name;
diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
index 655b34b7e..40641cad4 100644
--- a/drivers/bus/pci/bsd/pci.c
+++ b/drivers/bus/pci/bsd/pci.c
@@ -223,6 +223,8 @@  pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 	}
 
 	memset(dev, 0, sizeof(*dev));
+	dev->device.bus = &rte_pci_bus.bus;
+
 	dev->addr.domain = conf->pc_sel.pc_domain;
 	dev->addr.bus = conf->pc_sel.pc_bus;
 	dev->addr.devid = conf->pc_sel.pc_dev;
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 04648ac93..e31bbb370 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -228,6 +228,7 @@  pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 		return -1;
 
 	memset(dev, 0, sizeof(*dev));
+	dev->device.bus = &rte_pci_bus.bus;
 	dev->addr = *addr;
 
 	/* get vendor id */
diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h
index 0e689fa74..04bffa6e7 100644
--- a/drivers/bus/pci/private.h
+++ b/drivers/bus/pci/private.h
@@ -15,6 +15,8 @@  extern struct rte_pci_bus rte_pci_bus;
 struct rte_pci_driver;
 struct rte_pci_device;
 
+extern struct rte_pci_bus rte_pci_bus;
+
 /**
  * Probe the PCI bus
  *
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index efca962f7..0142fb2c8 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -456,6 +456,7 @@  vdev_scan(void)
 			continue;
 		}
 
+		dev->device.bus = &rte_vdev_bus;
 		dev->device.devargs = devargs;
 		dev->device.numa_node = SOCKET_ID_ANY;
 		dev->device.name = devargs->name;
diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 527a6a39f..a4755a387 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -229,6 +229,7 @@  vmbus_scan_one(const char *name)
 	if (dev == NULL)
 		return -1;
 
+	dev->device.bus = &rte_vmbus_bus.bus;
 	dev->device.name = strdup(name);
 	if (!dev->device.name)
 		goto error;
diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h
index f2022a68c..211127dd8 100644
--- a/drivers/bus/vmbus/private.h
+++ b/drivers/bus/vmbus/private.h
@@ -10,11 +10,14 @@ 
 #include <sys/uio.h>
 #include <rte_log.h>
 #include <rte_vmbus_reg.h>
+#include <rte_bus_vmbus.h>
 
 #ifndef PAGE_SIZE
 #define PAGE_SIZE	4096
 #endif
 
+extern struct rte_vmbus_bus rte_vmbus_bus;
+
 extern int vmbus_logtype_bus;
 #define VMBUS_LOG(level, fmt, args...) \
 	rte_log(RTE_LOG_ ## level, vmbus_logtype_bus, "%s(): " fmt "\n", \
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index b80a80598..d82cba847 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -157,6 +157,7 @@  struct rte_device {
 	TAILQ_ENTRY(rte_device) next; /**< Next device */
 	const char *name;             /**< Device name */
 	const struct rte_driver *driver;/**< Associated driver */
+	const struct rte_bus *bus;    /**< Bus handle assigned on scan */
 	int numa_node;                /**< NUMA node connection */
 	struct rte_devargs *devargs;  /**< Device user arguments */
 };