* [dpdk-dev] Improved PCI hotplug support
@ 2016-11-23 19:36 Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
` (7 more replies)
0 siblings, 8 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 19:36 UTC (permalink / raw)
To: dev
This series of patches adds support for PCI hot insert and remove.
Detection of new devices or removed devices is accomplished by
polling rte_eal_pci_probe, with the registered PCI drivers being
loaded or unloaded when a new device is found or previously known
device is removed.
There are some additional considerations for hotplug that this
patch does not address. Full hotplug support actually consists of
three operations:
1) Hot insert. This patch series does correctly handle this case.
2) Planned hot remove. This patch series can also handle this case
because the user can explicitly unload the driver prior to
removal.
3) Surprise hot remove. If a driver is loaded and active, a SIGBUS
may occur on the next access to a memory mapped region.
This is currently NOT handled by this patch series.
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH 1/7] pci: If a driver's probe function fails, unmap resources.
2016-11-23 19:36 [dpdk-dev] Improved PCI hotplug support Ben Walker
@ 2016-11-23 19:36 ` Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 2/7] pci: Separate detaching ethernet ports from PCI devices Ben Walker
` (6 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 19:36 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
If resources were mapped prior to probe, unmap them
if probe fails.
This does not handle the case where the kernel driver was
forcibly unbound prior to probe.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/common/eal_common_pci.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 6bff675..4f8c3a0 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -215,8 +215,11 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
/* call the driver probe() function */
ret = dr->probe(dr, dev);
- if (ret)
+ if (ret) {
dev->driver = NULL;
+ if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+ rte_eal_pci_unmap_device(dev);
+ }
return ret;
}
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH 2/7] pci: Separate detaching ethernet ports from PCI devices
2016-11-23 19:36 [dpdk-dev] Improved PCI hotplug support Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
@ 2016-11-23 19:36 ` Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 3/7] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
` (5 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 19:36 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
Attaching and detaching ethernet ports from an application
is not the same thing as physically removing a PCI device,
so clarify the flags indicating support. All PCI devices
are assumed to be physically removable, so no flag is
necessary in the PCI layer.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
doc/guides/prog_guide/port_hotplug_framework.rst | 2 +-
drivers/net/bnxt/bnxt_ethdev.c | 3 ++-
drivers/net/e1000/em_ethdev.c | 4 ++--
drivers/net/e1000/igb_ethdev.c | 7 ++++---
drivers/net/fm10k/fm10k_ethdev.c | 4 ++--
drivers/net/i40e/i40e_ethdev.c | 4 ++--
drivers/net/i40e/i40e_ethdev_vf.c | 3 ++-
drivers/net/ixgbe/ixgbe_ethdev.c | 7 ++++---
drivers/net/nfp/nfp_net.c | 4 ++--
drivers/net/virtio/virtio_ethdev.c | 3 ++-
drivers/net/vmxnet3/vmxnet3_ethdev.c | 3 ++-
drivers/net/xenvirt/rte_eth_xenvirt.c | 2 +-
lib/librte_eal/common/include/rte_pci.h | 2 --
lib/librte_ether/rte_ethdev.c | 2 --
14 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/doc/guides/prog_guide/port_hotplug_framework.rst b/doc/guides/prog_guide/port_hotplug_framework.rst
index 6e4436e..d68d08e 100644
--- a/doc/guides/prog_guide/port_hotplug_framework.rst
+++ b/doc/guides/prog_guide/port_hotplug_framework.rst
@@ -106,5 +106,5 @@ Limitations
* Not all PMDs support detaching feature.
To know whether a PMD can support detaching, search for the
- "RTE_PCI_DRV_DETACHABLE" flag in PMD implementation. If the flag is
+ "RTE_ETH_DEV_DETAHABLE" flag in rte_eth_dev::data::dev_flags. If the flag is
defined in the PMD, detaching is supported.
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 035fe07..a2100f6 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1051,6 +1051,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
RTE_LOG(INFO, PMD, "%s", bnxt_version);
rte_eth_copy_pci_info(eth_dev, eth_dev->pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
bp = eth_dev->data->dev_private;
if (bnxt_vf_pciid(eth_dev->pci_dev->id.device_id))
@@ -1162,7 +1163,7 @@ static struct eth_driver bnxt_rte_pmd = {
.pci_drv = {
.id_table = bnxt_pci_id_map,
.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
- RTE_PCI_DRV_DETACHABLE | RTE_PCI_DRV_INTR_LSC,
+ RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove
},
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index aee3d34..9af429b 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -312,6 +312,7 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
hw->device_id = pci_dev->id.device_id;
@@ -392,8 +393,7 @@ eth_em_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_em_pmd = {
.pci_drv = {
.id_table = pci_id_em_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 2fddf0c..b014b8b 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -771,6 +771,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->hw_addr= (void *)pci_dev->mem_resource[0].addr;
@@ -976,6 +977,7 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
pci_dev = eth_dev->pci_dev;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->device_id = pci_dev->id.device_id;
hw->vendor_id = pci_dev->id.vendor_id;
@@ -1079,8 +1081,7 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_igb_pmd = {
.pci_drv = {
.id_table = pci_id_igb_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
@@ -1095,7 +1096,7 @@ static struct eth_driver rte_igb_pmd = {
static struct eth_driver rte_igbvf_pmd = {
.pci_drv = {
.id_table = pci_id_igbvf_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 923690c..578de1f 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -2841,6 +2841,7 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
return 0;
rte_eth_copy_pci_info(dev, dev->pci_dev);
+ dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
memset(macvlan, 0, sizeof(*macvlan));
@@ -3062,8 +3063,7 @@ static const struct rte_pci_id pci_id_fm10k_map[] = {
static struct eth_driver rte_pmd_fm10k = {
.pci_drv = {
.id_table = pci_id_fm10k_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..7877cc5 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -671,8 +671,7 @@ static const struct rte_i40e_xstats_name_off rte_i40e_txq_prio_strings[] = {
static struct eth_driver rte_i40e_pmd = {
.pci_drv = {
.id_table = pci_id_i40e_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
@@ -955,6 +954,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
pci_dev = dev->pci_dev;
rte_eth_copy_pci_info(dev, pci_dev);
+ dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
pf->adapter = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
pf->adapter->eth_dev = dev;
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index aa306d6..6aaee37 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1459,6 +1459,7 @@ i40evf_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, eth_dev->pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->vendor_id = eth_dev->pci_dev->id.vendor_id;
hw->device_id = eth_dev->pci_dev->id.device_id;
@@ -1528,7 +1529,7 @@ i40evf_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_i40evf_pmd = {
.pci_drv = {
.id_table = pci_id_i40evf_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index edc9b22..66dd0c9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1130,6 +1130,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
pci_dev = eth_dev->pci_dev;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
/* Vendor and Device ID need to be set before init of shared code */
hw->device_id = pci_dev->id.device_id;
@@ -1422,6 +1423,7 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
pci_dev = eth_dev->pci_dev;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->device_id = pci_dev->id.device_id;
hw->vendor_id = pci_dev->id.vendor_id;
@@ -1565,8 +1567,7 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_ixgbe_pmd = {
.pci_drv = {
.id_table = pci_id_ixgbe_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
@@ -1581,7 +1582,7 @@ static struct eth_driver rte_ixgbe_pmd = {
static struct eth_driver rte_ixgbevf_pmd = {
.pci_drv = {
.id_table = pci_id_ixgbevf_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index c6b1587..00d2ba2 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -2332,6 +2332,7 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
pci_dev = eth_dev->pci_dev;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->device_id = pci_dev->id.device_id;
hw->vendor_id = pci_dev->id.vendor_id;
@@ -2470,8 +2471,7 @@ static struct rte_pci_id pci_id_nfp_net_map[] = {
static struct eth_driver rte_nfp_net_pmd = {
.pci_drv = {
.id_table = pci_id_nfp_net_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 079fd6c..74255c5 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1211,6 +1211,7 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
rx_func_get(eth_dev);
@@ -1379,7 +1380,7 @@ static struct eth_driver rte_virtio_pmd = {
.name = "net_virtio",
},
.id_table = pci_id_virtio_map,
- .drv_flags = RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = 0,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 8bb13e5..dd8dca5 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -247,6 +247,7 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev)
return 0;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
/* Vendor and Device ID need to be set before init of shared code */
hw->device_id = pci_dev->id.device_id;
@@ -336,7 +337,7 @@ eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_vmxnet3_pmd = {
.pci_drv = {
.id_table = pci_id_vmxnet3_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/xenvirt/rte_eth_xenvirt.c b/drivers/net/xenvirt/rte_eth_xenvirt.c
index c08a056..ec5d00b 100644
--- a/drivers/net/xenvirt/rte_eth_xenvirt.c
+++ b/drivers/net/xenvirt/rte_eth_xenvirt.c
@@ -673,7 +673,7 @@ eth_dev_xenvirt_create(const char *name, const char *params,
eth_dev->data = data;
eth_dev->dev_ops = &ops;
- eth_dev->data->dev_flags = RTE_PCI_DRV_DETACHABLE;
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
eth_dev->data->kdrv = RTE_KDRV_NONE;
eth_dev->data->drv_name = drivername;
eth_dev->driver = NULL;
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 9ce8847..74be1f5 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -212,8 +212,6 @@ struct rte_pci_driver {
#define RTE_PCI_DRV_FORCE_UNBIND 0x0004
/** Device driver supports link state interrupt */
#define RTE_PCI_DRV_INTR_LSC 0x0008
-/** Device driver supports detaching capability */
-#define RTE_PCI_DRV_DETACHABLE 0x0010
/**
* A structure describing a PCI mapping.
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..3771ffc 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3209,8 +3209,6 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev, struct rte_pci_device *pci_de
eth_dev->data->dev_flags = 0;
if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
- if (pci_dev->driver->drv_flags & RTE_PCI_DRV_DETACHABLE)
- eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE;
eth_dev->data->kdrv = pci_dev->kdrv;
eth_dev->data->numa_node = pci_dev->device.numa_node;
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH 3/7] pci: Pass rte_pci_addr to functions instead of separate args
2016-11-23 19:36 [dpdk-dev] Improved PCI hotplug support Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 2/7] pci: Separate detaching ethernet ports from PCI devices Ben Walker
@ 2016-11-23 19:36 ` Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices Ben Walker
` (4 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 19:36 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
Instead of passing domain, bus, devid, func, just pass
an rte_pci_addr.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 876ba38..073af5f 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -267,8 +267,7 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
/* Scan one pci sysfs entry, and fill the devices list from it. */
static int
-pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
- uint8_t devid, uint8_t function)
+pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
{
char filename[PATH_MAX];
unsigned long tmp;
@@ -281,10 +280,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
return -1;
memset(dev, 0, sizeof(*dev));
- dev->addr.domain = domain;
- dev->addr.bus = bus;
- dev->addr.devid = devid;
- dev->addr.function = function;
+ dev->addr = *addr;
/* get vendor id */
snprintf(filename, sizeof(filename), "%s/vendor", dirname);
@@ -429,16 +425,14 @@ pci_update_device(const struct rte_pci_addr *addr)
pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
addr->function);
- return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
- addr->function);
+ return pci_scan_one(filename, addr);
}
/*
* split up a pci address into its constituent parts.
*/
static int
-parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
- uint8_t *bus, uint8_t *devid, uint8_t *function)
+parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
{
/* first split on ':' */
union splitaddr {
@@ -466,10 +460,10 @@ parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
/* now convert to int values */
errno = 0;
- *domain = (uint16_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);
+ addr->domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
+ addr->bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
+ addr->devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
+ addr->function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
if (errno != 0)
goto error;
@@ -490,8 +484,7 @@ rte_eal_pci_scan(void)
struct dirent *e;
DIR *dir;
char dirname[PATH_MAX];
- uint16_t domain;
- uint8_t bus, devid, function;
+ struct rte_pci_addr addr;
dir = opendir(pci_get_sysfs_path());
if (dir == NULL) {
@@ -500,20 +493,21 @@ rte_eal_pci_scan(void)
return -1;
}
+
while ((e = readdir(dir)) != NULL) {
if (e->d_name[0] == '.')
continue;
- if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &domain,
- &bus, &devid, &function) != 0)
+ if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &addr) != 0)
continue;
snprintf(dirname, sizeof(dirname), "%s/%s",
pci_get_sysfs_path(), e->d_name);
- if (pci_scan_one(dirname, domain, bus, devid, function) < 0)
+ if (pci_scan_one(dirname, &addr) < 0)
goto error;
}
closedir(dir);
+
return 0;
error:
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices
2016-11-23 19:36 [dpdk-dev] Improved PCI hotplug support Ben Walker
` (2 preceding siblings ...)
2016-11-23 19:36 ` [dpdk-dev] [PATCH 3/7] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
@ 2016-11-23 19:36 ` Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 5/7] pci: Move driver registration above pci scan Ben Walker
` (3 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 19:36 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
rte_eal_pci_scan can be called repeatedly to re-scan the PCI
bus. If a device was removed from the system, the associated
driver will automatically be unloaded.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/linuxapp/eal/eal_pci.c | 58 +++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 073af5f..f237864 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -485,7 +485,65 @@ rte_eal_pci_scan(void)
DIR *dir;
char dirname[PATH_MAX];
struct rte_pci_addr addr;
+ struct rte_pci_device *dev, *tmp;
+
+ /* Search the device list for devices that are no longer present on the system
+ * and remove them.
+ */
+ TAILQ_FOREACH_SAFE(dev, &pci_device_list, next, tmp) {
+ int found = 0;
+
+ dir = opendir(pci_get_sysfs_path());
+ if (dir == NULL) {
+ RTE_LOG(ERR, EAL, "%s(): opendir failed: %s\n",
+ __func__, strerror(errno));
+ return -1;
+ }
+
+ while ((e = readdir(dir)) != NULL) {
+ if (e->d_name[0] == '.')
+ continue;
+
+ if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &addr) != 0)
+ continue;
+
+ if (rte_eal_compare_pci_addr(&addr, &dev->addr) == 0) {
+ found = 1;
+ break;
+ }
+ }
+
+ if (!found) {
+
+ RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" was removed from the system.\n",
+ addr.domain, addr.bus, addr.devid,
+ addr.function);
+
+ if (dev->driver) {
+ /* A driver was loaded against this device. Unload it. */
+ RTE_LOG(DEBUG, EAL, " Unload driver: %x:%x %s\n", dev->id.vendor_id,
+ dev->id.device_id, dev->driver->driver.name);
+
+ if (dev->driver->remove) {
+ /* It doesn't matter what remove returns - we're removing the device either way. */
+ dev->driver->remove(dev);
+ }
+
+ /* clear driver structure */
+ dev->driver = NULL;
+
+ if (dev->driver->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+ rte_eal_pci_unmap_device(dev);
+ }
+
+ TAILQ_REMOVE(&pci_device_list, dev, next);
+ free(dev);
+ }
+
+ closedir(dir);
+ }
+ /* Search sysfs for all PCI devices and add them to the device list */
dir = opendir(pci_get_sysfs_path());
if (dir == NULL) {
RTE_LOG(ERR, EAL, "%s(): opendir failed: %s\n",
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH 5/7] pci: Move driver registration above pci scan
2016-11-23 19:36 [dpdk-dev] Improved PCI hotplug support Ben Walker
` (3 preceding siblings ...)
2016-11-23 19:36 ` [dpdk-dev] [PATCH 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices Ben Walker
@ 2016-11-23 19:36 ` Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe Ben Walker
` (2 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 19:36 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
The user needs to register drivers before scanning, so
it makes the most sense to put the registration
functions above the scan function in the header file.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/common/include/rte_pci.h | 56 ++++++++++++++++-----------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 74be1f5..5d0feac 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -358,6 +358,34 @@ rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
}
/**
+ * Register a PCI driver.
+ *
+ * @param driver
+ * A pointer to a rte_pci_driver structure describing the driver
+ * to be registered.
+ */
+void rte_eal_pci_register(struct rte_pci_driver *driver);
+
+/** Statically register a PCI driver at start up */
+#define RTE_PMD_REGISTER_PCI(nm, pci_drv) \
+RTE_INIT(pciinitfn_ ##nm); \
+static void pciinitfn_ ##nm(void) \
+{\
+ (pci_drv).driver.name = RTE_STR(nm);\
+ rte_eal_pci_register(&pci_drv); \
+} \
+RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
+
+/**
+ * Unregister a PCI driver.
+ *
+ * @param driver
+ * A pointer to a rte_pci_driver structure describing the driver
+ * to be unregistered.
+ */
+void rte_eal_pci_unregister(struct rte_pci_driver *driver);
+
+/**
* Scan the content of the PCI bus, and the devices in the devices
* list
*
@@ -476,34 +504,6 @@ int rte_eal_pci_detach(const struct rte_pci_addr *addr);
void rte_eal_pci_dump(FILE *f);
/**
- * Register a PCI driver.
- *
- * @param driver
- * A pointer to a rte_pci_driver structure describing the driver
- * to be registered.
- */
-void rte_eal_pci_register(struct rte_pci_driver *driver);
-
-/** Helper for PCI device registration from driver (eth, crypto) instance */
-#define RTE_PMD_REGISTER_PCI(nm, pci_drv) \
-RTE_INIT(pciinitfn_ ##nm); \
-static void pciinitfn_ ##nm(void) \
-{\
- (pci_drv).driver.name = RTE_STR(nm);\
- rte_eal_pci_register(&pci_drv); \
-} \
-RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
-
-/**
- * Unregister a PCI driver.
- *
- * @param driver
- * A pointer to a rte_pci_driver structure describing the driver
- * to be unregistered.
- */
-void rte_eal_pci_unregister(struct rte_pci_driver *driver);
-
-/**
* Read PCI config space.
*
* @param device
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe
2016-11-23 19:36 [dpdk-dev] Improved PCI hotplug support Ben Walker
` (4 preceding siblings ...)
2016-11-23 19:36 ` [dpdk-dev] [PATCH 5/7] pci: Move driver registration above pci scan Ben Walker
@ 2016-11-23 19:36 ` Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
7 siblings, 0 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 19:36 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
Two functions is both confusing and unnecessary. Previously,
rte_eal_pci_scan populated an internal list of devices by
scanning sysfs. Then, rte_eal_pci_probe would match registered
drivers to that internal list. These are not really useful
operations to perform separately independently, though, so
simplify the api down to just rte_eal_pci_probe which can
be called repeatedly through the lifetime of the application
to scan for new or removed PCI devices and load or unload
drivers as required.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
app/test/test_pci.c | 2 +-
lib/librte_eal/bsdapp/eal/eal.c | 3 ---
lib/librte_eal/bsdapp/eal/eal_pci.c | 17 +----------------
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 -
lib/librte_eal/common/eal_common_pci.c | 7 +++++++
lib/librte_eal/common/eal_private.h | 14 +++++---------
lib/librte_eal/common/include/rte_pci.h | 17 +++++------------
lib/librte_eal/linuxapp/eal/eal.c | 3 ---
lib/librte_eal/linuxapp/eal/eal_pci.c | 18 +-----------------
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 -
10 files changed, 20 insertions(+), 63 deletions(-)
diff --git a/app/test/test_pci.c b/app/test/test_pci.c
index cda186d..fdd84f7 100644
--- a/app/test/test_pci.c
+++ b/app/test/test_pci.c
@@ -180,7 +180,7 @@ test_pci_setup(void)
TAILQ_INSERT_TAIL(&real_pci_device_list, dev, next);
}
- ret = rte_eal_pci_scan();
+ ret = rte_eal_pci_probe();
TEST_ASSERT_SUCCESS(ret, "failed to scan PCI bus");
rte_eal_pci_dump(stdout);
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 35e3117..fd44528 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -561,9 +561,6 @@ rte_eal_init(int argc, char **argv)
if (rte_eal_timer_init() < 0)
rte_panic("Cannot init HPET or TSC timers\n");
- if (rte_eal_pci_init() < 0)
- rte_panic("Cannot init PCI\n");
-
eal_check_mem_on_local_socket();
if (eal_plugins_init() < 0)
diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 8b3ed88..6c3a169 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -361,7 +361,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
* list. Call pci_scan_one() for each pci entry found.
*/
int
-rte_eal_pci_scan(void)
+pci_scan(void)
{
int fd;
unsigned dev_count = 0;
@@ -667,18 +667,3 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p)
return ret;
}
-
-/* Init the PCI EAL subsystem */
-int
-rte_eal_pci_init(void)
-{
- /* for debug purposes, PCI can be disabled */
- if (internal_config.no_pci)
- return 0;
-
- if (rte_eal_pci_scan() < 0) {
- RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
- return -1;
- }
- return 0;
-}
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 2f81f7c..67c469c 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -44,7 +44,6 @@ DPDK_2.0 {
rte_eal_pci_probe;
rte_eal_pci_probe_one;
rte_eal_pci_register;
- rte_eal_pci_scan;
rte_eal_pci_unregister;
rte_eal_process_type;
rte_eal_remote_launch;
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 4f8c3a0..62b996d 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -81,6 +81,7 @@
#include <rte_devargs.h>
#include "eal_private.h"
+#include "eal_internal_cfg.h"
struct pci_driver_list pci_driver_list =
TAILQ_HEAD_INITIALIZER(pci_driver_list);
@@ -423,6 +424,12 @@ rte_eal_pci_probe(void)
int probe_all = 0;
int ret = 0;
+ if (internal_config.no_pci) {
+ return 0;
+ }
+
+ pci_scan();
+
if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) == 0)
probe_all = 1;
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 9e7d8f6..54f18ea 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -108,18 +108,14 @@ int rte_eal_timer_init(void);
*/
int rte_eal_log_init(const char *id, int facility);
-/**
- * Init the PCI infrastructure
+struct rte_pci_driver;
+struct rte_pci_device;
+
+/* Scan the PCI bus for devices
*
* This function is private to EAL.
- *
- * @return
- * 0 on success, negative on error
*/
-int rte_eal_pci_init(void);
-
-struct rte_pci_driver;
-struct rte_pci_device;
+int pci_scan(void);
/**
* Update a pci device object by asking the kernel for the latest information.
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 5d0feac..2154a54 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -386,20 +386,13 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
void rte_eal_pci_unregister(struct rte_pci_driver *driver);
/**
- * Scan the content of the PCI bus, and the devices in the devices
- * list
- *
- * @return
- * 0 on success, negative on error
- */
-int rte_eal_pci_scan(void);
-
-/**
- * Probe the PCI bus for registered drivers.
+ * Scan the PCI bus for devices and match them to their driver.
*
* Scan the content of the PCI bus, and call the probe() function for
- * all registered drivers that have a matching entry in its id_table
- * for discovered devices.
+ * all registered drivers that have a matching entry in their id_table.
+ * If a device already has a driver loaded, probe will not be called.
+ * If a previously discovered device is no longer present on the system,
+ * the associated driver's remove() callback will be called.
*
* @return
* - 0 on success.
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 2075282..f47f361 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -802,9 +802,6 @@ rte_eal_init(int argc, char **argv)
if (rte_eal_log_init(logid, internal_config.syslog_facility) < 0)
rte_panic("Cannot init logs\n");
- if (rte_eal_pci_init() < 0)
- rte_panic("Cannot init PCI\n");
-
#ifdef VFIO_PRESENT
if (rte_eal_vfio_setup() < 0)
rte_panic("Cannot init VFIO\n");
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index f237864..663e106 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -479,7 +479,7 @@ parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
* list
*/
int
-rte_eal_pci_scan(void)
+pci_scan(void)
{
struct dirent *e;
DIR *dir;
@@ -806,19 +806,3 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p)
return ret;
}
-
-/* Init the PCI EAL subsystem */
-int
-rte_eal_pci_init(void)
-{
- /* for debug purposes, PCI can be disabled */
- if (internal_config.no_pci)
- return 0;
-
- if (rte_eal_pci_scan() < 0) {
- RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
- return -1;
- }
-
- return 0;
-}
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 83721ba..856728e 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -44,7 +44,6 @@ DPDK_2.0 {
rte_eal_pci_probe;
rte_eal_pci_probe_one;
rte_eal_pci_register;
- rte_eal_pci_scan;
rte_eal_pci_unregister;
rte_eal_process_type;
rte_eal_remote_launch;
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers
2016-11-23 19:36 [dpdk-dev] Improved PCI hotplug support Ben Walker
` (5 preceding siblings ...)
2016-11-23 19:36 ` [dpdk-dev] [PATCH 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe Ben Walker
@ 2016-11-23 19:36 ` Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
7 siblings, 0 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 19:36 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
There are now two functions - rte_eal_pci_attach_driver and
rte_eal_pci_detach_driver - that dynamically attempt to attach
and detach drivers from PCI devices. These only control
whether a registered PCI driver is loaded or not - they are
independent of whether the PCI device exists on the system.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/common/eal_common_dev.c | 4 +-
lib/librte_eal/common/eal_common_pci.c | 111 +++++++++-----------------------
lib/librte_eal/common/include/rte_pci.h | 22 ++++---
3 files changed, 45 insertions(+), 92 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 4f3b493..1c6834e 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -114,7 +114,7 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
}
if (eal_parse_pci_DomBDF(name, &addr) == 0) {
- if (rte_eal_pci_probe_one(&addr) < 0)
+ if (rte_eal_pci_attach_driver(&addr) < 0)
goto err;
} else {
@@ -139,7 +139,7 @@ int rte_eal_dev_detach(const char *name)
}
if (eal_parse_pci_DomBDF(name, &addr) == 0) {
- if (rte_eal_pci_detach(&addr) < 0)
+ if (rte_eal_pci_detach_driver(&addr) < 0)
goto err;
} else {
if (rte_eal_vdev_uninit(name))
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 62b996d..f2879af 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -228,59 +228,37 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
return 1;
}
-/*
- * If vendor/device ID match, call the remove() function of the
- * driver.
- */
static int
-rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
- struct rte_pci_device *dev)
+rte_eal_pci_remove_driver(struct rte_pci_device *dev)
{
- const struct rte_pci_id *id_table;
-
- if ((dr == NULL) || (dev == NULL))
- return -EINVAL;
-
- for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
-
- /* check if device's identifiers match the driver's ones */
- if (id_table->vendor_id != dev->id.vendor_id &&
- id_table->vendor_id != PCI_ANY_ID)
- continue;
- if (id_table->device_id != dev->id.device_id &&
- id_table->device_id != PCI_ANY_ID)
- continue;
- if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
- id_table->subsystem_vendor_id != PCI_ANY_ID)
- continue;
- if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
- id_table->subsystem_device_id != PCI_ANY_ID)
- continue;
+ struct rte_pci_driver *driver;
+ struct rte_pci_addr *loc;
- struct rte_pci_addr *loc = &dev->addr;
+ loc = &dev->addr;
+ driver = dev->driver;
- RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
- loc->domain, loc->bus, loc->devid,
- loc->function, dev->device.numa_node);
+ if (driver == NULL) {
+ return 0;
+ }
- RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
- dev->id.device_id, dr->driver.name);
+ RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+ loc->domain, loc->bus, loc->devid,
+ loc->function, dev->device.numa_node);
- if (dr->remove && (dr->remove(dev) < 0))
- return -1; /* negative value is an error */
+ RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
+ dev->id.device_id, driver->driver.name);
- /* clear driver structure */
- dev->driver = NULL;
+ if (driver->remove && (driver->remove(dev) < 0))
+ return -1; /* negative value is an error */
- if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
- /* unmap resources for devices that use igb_uio */
- rte_eal_pci_unmap_device(dev);
+ /* clear driver structure */
+ dev->driver = NULL;
- return 0;
- }
+ if (driver->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+ /* unmap resources for devices that use igb_uio */
+ rte_eal_pci_unmap_device(dev);
- /* return positive value if driver doesn't support this device */
- return 1;
+ return 0;
}
/*
@@ -315,38 +293,11 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
}
/*
- * If vendor/device ID match, call the remove() function of all
- * registered driver for the given device. Return -1 if initialization
- * failed, return 1 if no driver is found for this device.
- */
-static int
-pci_detach_all_drivers(struct rte_pci_device *dev)
-{
- struct rte_pci_driver *dr = NULL;
- int rc = 0;
-
- if (dev == NULL)
- return -1;
-
- TAILQ_FOREACH(dr, &pci_driver_list, next) {
- rc = rte_eal_pci_detach_dev(dr, dev);
- if (rc < 0)
- /* negative value is an error */
- return -1;
- if (rc > 0)
- /* positive value means driver doesn't support it */
- continue;
- return 0;
- }
- return 1;
-}
-
-/*
* Find the pci device specified by pci address, then invoke probe function of
* the driver of the devive.
*/
int
-rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
+rte_eal_pci_attach_driver(const struct rte_pci_addr *addr)
{
struct rte_pci_device *dev = NULL;
int ret = 0;
@@ -382,10 +333,9 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
* Detach device specified by its pci address.
*/
int
-rte_eal_pci_detach(const struct rte_pci_addr *addr)
+rte_eal_pci_detach_driver(const struct rte_pci_addr *addr)
{
struct rte_pci_device *dev = NULL;
- int ret = 0;
if (addr == NULL)
return -1;
@@ -394,17 +344,18 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
if (rte_eal_compare_pci_addr(&dev->addr, addr))
continue;
- ret = pci_detach_all_drivers(dev);
- if (ret < 0)
- goto err_return;
+ if (dev->driver == NULL) {
+ /* The device at that address does not have a driver loaded */
+ return 0;
+ }
+
+ if (rte_eal_pci_remove_driver(dev) < 0) {
+ return -1;
+ }
- TAILQ_REMOVE(&pci_device_list, dev, next);
- free(dev);
return 0;
}
- return -1;
-err_return:
RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT
" cannot be used\n", dev->addr.domain, dev->addr.bus,
dev->addr.devid, dev->addr.function);
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 2154a54..e304853 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -459,11 +459,14 @@ void *pci_map_resource(void *requested_addr, int fd, off_t offset,
void pci_unmap_resource(void *requested_addr, size_t size);
/**
- * Probe the single PCI device.
+ * Attempt to load the registered driver for the PCI device at addr.
*
- * Scan the content of the PCI bus, and find the pci device specified by pci
- * address, then call the probe() function for registered driver that has a
- * matching entry in its id_table for discovered device.
+ * Find the pci device specified by addr, then call the probe() function
+ * for each registered driver that has a matching entry in its id_table until
+ * the correct driver is found.
+ *
+ * If the PCI address is known, this is considerably more efficient than
+ * calling rte_eal_pci_probe.
*
* @param addr
* The PCI Bus-Device-Function address to probe.
@@ -471,14 +474,13 @@ void pci_unmap_resource(void *requested_addr, size_t size);
* - 0 on success.
* - Negative on error.
*/
-int rte_eal_pci_probe_one(const struct rte_pci_addr *addr);
+int rte_eal_pci_attach_driver(const struct rte_pci_addr *addr);
/**
- * Close the single PCI device.
+ * Unload the driver for the PCI device at addr.
*
- * Scan the content of the PCI bus, and find the pci device specified by pci
- * address, then call the remove() function for registered driver that has a
- * matching entry in its id_table for discovered device.
+ * Find the pci device specified by addr, then call the remove() function
+ * for the currently loaded driver.
*
* @param addr
* The PCI Bus-Device-Function address to close.
@@ -486,7 +488,7 @@ int rte_eal_pci_probe_one(const struct rte_pci_addr *addr);
* - 0 on success.
* - Negative on error.
*/
-int rte_eal_pci_detach(const struct rte_pci_addr *addr);
+int rte_eal_pci_detach_driver(const struct rte_pci_addr *addr);
/**
* Dump the content of the PCI bus.
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources.
2016-11-23 19:36 [dpdk-dev] Improved PCI hotplug support Ben Walker
` (6 preceding siblings ...)
2016-11-23 19:36 ` [dpdk-dev] [PATCH 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers Ben Walker
@ 2016-11-23 20:07 ` Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 2/7] pci: Separate detaching ethernet ports from PCI devices Ben Walker
` (7 more replies)
7 siblings, 8 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 20:07 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
If resources were mapped prior to probe, unmap them
if probe fails.
This does not handle the case where the kernel driver was
forcibly unbound prior to probe.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/common/eal_common_pci.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 6bff675..4f8c3a0 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -215,8 +215,11 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
/* call the driver probe() function */
ret = dr->probe(dr, dev);
- if (ret)
+ if (ret) {
dev->driver = NULL;
+ if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+ rte_eal_pci_unmap_device(dev);
+ }
return ret;
}
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v2 2/7] pci: Separate detaching ethernet ports from PCI devices
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
@ 2016-11-23 20:07 ` Ben Walker
2016-11-25 9:25 ` Shreyansh Jain
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
` (6 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Ben Walker @ 2016-11-23 20:07 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
Attaching and detaching ethernet ports from an application
is not the same thing as physically removing a PCI device,
so clarify the flags indicating support. All PCI devices
are assumed to be physically removable, so no flag is
necessary in the PCI layer.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
doc/guides/prog_guide/port_hotplug_framework.rst | 2 +-
drivers/net/bnxt/bnxt_ethdev.c | 3 ++-
drivers/net/e1000/em_ethdev.c | 4 ++--
drivers/net/e1000/igb_ethdev.c | 7 ++++---
drivers/net/fm10k/fm10k_ethdev.c | 4 ++--
drivers/net/i40e/i40e_ethdev.c | 4 ++--
drivers/net/i40e/i40e_ethdev_vf.c | 3 ++-
drivers/net/ixgbe/ixgbe_ethdev.c | 7 ++++---
drivers/net/nfp/nfp_net.c | 4 ++--
drivers/net/virtio/virtio_ethdev.c | 3 ++-
drivers/net/vmxnet3/vmxnet3_ethdev.c | 3 ++-
drivers/net/xenvirt/rte_eth_xenvirt.c | 2 +-
lib/librte_eal/common/include/rte_pci.h | 2 --
lib/librte_ether/rte_ethdev.c | 2 --
14 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/doc/guides/prog_guide/port_hotplug_framework.rst b/doc/guides/prog_guide/port_hotplug_framework.rst
index 6e4436e..d68d08e 100644
--- a/doc/guides/prog_guide/port_hotplug_framework.rst
+++ b/doc/guides/prog_guide/port_hotplug_framework.rst
@@ -106,5 +106,5 @@ Limitations
* Not all PMDs support detaching feature.
To know whether a PMD can support detaching, search for the
- "RTE_PCI_DRV_DETACHABLE" flag in PMD implementation. If the flag is
+ "RTE_ETH_DEV_DETAHABLE" flag in rte_eth_dev::data::dev_flags. If the flag is
defined in the PMD, detaching is supported.
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 035fe07..a2100f6 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1051,6 +1051,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
RTE_LOG(INFO, PMD, "%s", bnxt_version);
rte_eth_copy_pci_info(eth_dev, eth_dev->pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
bp = eth_dev->data->dev_private;
if (bnxt_vf_pciid(eth_dev->pci_dev->id.device_id))
@@ -1162,7 +1163,7 @@ static struct eth_driver bnxt_rte_pmd = {
.pci_drv = {
.id_table = bnxt_pci_id_map,
.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
- RTE_PCI_DRV_DETACHABLE | RTE_PCI_DRV_INTR_LSC,
+ RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove
},
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index aee3d34..9af429b 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -312,6 +312,7 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
hw->device_id = pci_dev->id.device_id;
@@ -392,8 +393,7 @@ eth_em_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_em_pmd = {
.pci_drv = {
.id_table = pci_id_em_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 2fddf0c..b014b8b 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -771,6 +771,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->hw_addr= (void *)pci_dev->mem_resource[0].addr;
@@ -976,6 +977,7 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
pci_dev = eth_dev->pci_dev;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->device_id = pci_dev->id.device_id;
hw->vendor_id = pci_dev->id.vendor_id;
@@ -1079,8 +1081,7 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_igb_pmd = {
.pci_drv = {
.id_table = pci_id_igb_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
@@ -1095,7 +1096,7 @@ static struct eth_driver rte_igb_pmd = {
static struct eth_driver rte_igbvf_pmd = {
.pci_drv = {
.id_table = pci_id_igbvf_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 923690c..578de1f 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -2841,6 +2841,7 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
return 0;
rte_eth_copy_pci_info(dev, dev->pci_dev);
+ dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
memset(macvlan, 0, sizeof(*macvlan));
@@ -3062,8 +3063,7 @@ static const struct rte_pci_id pci_id_fm10k_map[] = {
static struct eth_driver rte_pmd_fm10k = {
.pci_drv = {
.id_table = pci_id_fm10k_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..7877cc5 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -671,8 +671,7 @@ static const struct rte_i40e_xstats_name_off rte_i40e_txq_prio_strings[] = {
static struct eth_driver rte_i40e_pmd = {
.pci_drv = {
.id_table = pci_id_i40e_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
@@ -955,6 +954,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
pci_dev = dev->pci_dev;
rte_eth_copy_pci_info(dev, pci_dev);
+ dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
pf->adapter = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
pf->adapter->eth_dev = dev;
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index aa306d6..6aaee37 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1459,6 +1459,7 @@ i40evf_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, eth_dev->pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->vendor_id = eth_dev->pci_dev->id.vendor_id;
hw->device_id = eth_dev->pci_dev->id.device_id;
@@ -1528,7 +1529,7 @@ i40evf_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_i40evf_pmd = {
.pci_drv = {
.id_table = pci_id_i40evf_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index edc9b22..66dd0c9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1130,6 +1130,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
pci_dev = eth_dev->pci_dev;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
/* Vendor and Device ID need to be set before init of shared code */
hw->device_id = pci_dev->id.device_id;
@@ -1422,6 +1423,7 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
pci_dev = eth_dev->pci_dev;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->device_id = pci_dev->id.device_id;
hw->vendor_id = pci_dev->id.vendor_id;
@@ -1565,8 +1567,7 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_ixgbe_pmd = {
.pci_drv = {
.id_table = pci_id_ixgbe_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
@@ -1581,7 +1582,7 @@ static struct eth_driver rte_ixgbe_pmd = {
static struct eth_driver rte_ixgbevf_pmd = {
.pci_drv = {
.id_table = pci_id_ixgbevf_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index c6b1587..00d2ba2 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -2332,6 +2332,7 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
pci_dev = eth_dev->pci_dev;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->device_id = pci_dev->id.device_id;
hw->vendor_id = pci_dev->id.vendor_id;
@@ -2470,8 +2471,7 @@ static struct rte_pci_id pci_id_nfp_net_map[] = {
static struct eth_driver rte_nfp_net_pmd = {
.pci_drv = {
.id_table = pci_id_nfp_net_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 079fd6c..74255c5 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1211,6 +1211,7 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
rx_func_get(eth_dev);
@@ -1379,7 +1380,7 @@ static struct eth_driver rte_virtio_pmd = {
.name = "net_virtio",
},
.id_table = pci_id_virtio_map,
- .drv_flags = RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = 0,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 8bb13e5..dd8dca5 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -247,6 +247,7 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev)
return 0;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
/* Vendor and Device ID need to be set before init of shared code */
hw->device_id = pci_dev->id.device_id;
@@ -336,7 +337,7 @@ eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_vmxnet3_pmd = {
.pci_drv = {
.id_table = pci_id_vmxnet3_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/xenvirt/rte_eth_xenvirt.c b/drivers/net/xenvirt/rte_eth_xenvirt.c
index c08a056..ec5d00b 100644
--- a/drivers/net/xenvirt/rte_eth_xenvirt.c
+++ b/drivers/net/xenvirt/rte_eth_xenvirt.c
@@ -673,7 +673,7 @@ eth_dev_xenvirt_create(const char *name, const char *params,
eth_dev->data = data;
eth_dev->dev_ops = &ops;
- eth_dev->data->dev_flags = RTE_PCI_DRV_DETACHABLE;
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
eth_dev->data->kdrv = RTE_KDRV_NONE;
eth_dev->data->drv_name = drivername;
eth_dev->driver = NULL;
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 9ce8847..74be1f5 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -212,8 +212,6 @@ struct rte_pci_driver {
#define RTE_PCI_DRV_FORCE_UNBIND 0x0004
/** Device driver supports link state interrupt */
#define RTE_PCI_DRV_INTR_LSC 0x0008
-/** Device driver supports detaching capability */
-#define RTE_PCI_DRV_DETACHABLE 0x0010
/**
* A structure describing a PCI mapping.
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fde8112..3771ffc 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3209,8 +3209,6 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev, struct rte_pci_device *pci_de
eth_dev->data->dev_flags = 0;
if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
- if (pci_dev->driver->drv_flags & RTE_PCI_DRV_DETACHABLE)
- eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE;
eth_dev->data->kdrv = pci_dev->kdrv;
eth_dev->data->numa_node = pci_dev->device.numa_node;
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 2/7] pci: Separate detaching ethernet ports from PCI devices Ben Walker
@ 2016-11-23 20:07 ` Ben Walker
2016-11-25 10:33 ` Shreyansh Jain
2016-12-01 6:26 ` Shreyansh Jain
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices Ben Walker
` (5 subsequent siblings)
7 siblings, 2 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 20:07 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
Instead of passing domain, bus, devid, func, just pass
an rte_pci_addr.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 876ba38..073af5f 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -267,8 +267,7 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
/* Scan one pci sysfs entry, and fill the devices list from it. */
static int
-pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
- uint8_t devid, uint8_t function)
+pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
{
char filename[PATH_MAX];
unsigned long tmp;
@@ -281,10 +280,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
return -1;
memset(dev, 0, sizeof(*dev));
- dev->addr.domain = domain;
- dev->addr.bus = bus;
- dev->addr.devid = devid;
- dev->addr.function = function;
+ dev->addr = *addr;
/* get vendor id */
snprintf(filename, sizeof(filename), "%s/vendor", dirname);
@@ -429,16 +425,14 @@ pci_update_device(const struct rte_pci_addr *addr)
pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
addr->function);
- return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
- addr->function);
+ return pci_scan_one(filename, addr);
}
/*
* split up a pci address into its constituent parts.
*/
static int
-parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
- uint8_t *bus, uint8_t *devid, uint8_t *function)
+parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
{
/* first split on ':' */
union splitaddr {
@@ -466,10 +460,10 @@ parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
/* now convert to int values */
errno = 0;
- *domain = (uint16_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);
+ addr->domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
+ addr->bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
+ addr->devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
+ addr->function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
if (errno != 0)
goto error;
@@ -490,8 +484,7 @@ rte_eal_pci_scan(void)
struct dirent *e;
DIR *dir;
char dirname[PATH_MAX];
- uint16_t domain;
- uint8_t bus, devid, function;
+ struct rte_pci_addr addr;
dir = opendir(pci_get_sysfs_path());
if (dir == NULL) {
@@ -500,20 +493,21 @@ rte_eal_pci_scan(void)
return -1;
}
+
while ((e = readdir(dir)) != NULL) {
if (e->d_name[0] == '.')
continue;
- if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &domain,
- &bus, &devid, &function) != 0)
+ if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &addr) != 0)
continue;
snprintf(dirname, sizeof(dirname), "%s/%s",
pci_get_sysfs_path(), e->d_name);
- if (pci_scan_one(dirname, domain, bus, devid, function) < 0)
+ if (pci_scan_one(dirname, &addr) < 0)
goto error;
}
closedir(dir);
+
return 0;
error:
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v2 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 2/7] pci: Separate detaching ethernet ports from PCI devices Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
@ 2016-11-23 20:07 ` Ben Walker
2016-11-25 10:44 ` Shreyansh Jain
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 5/7] pci: Move driver registration above pci scan Ben Walker
` (4 subsequent siblings)
7 siblings, 2 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 20:07 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
rte_eal_pci_scan can be called repeatedly to re-scan the PCI
bus. If a device was removed from the system, the associated
driver will automatically be unloaded.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/linuxapp/eal/eal_pci.c | 62 +++++++++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 073af5f..936f076 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -485,7 +485,69 @@ rte_eal_pci_scan(void)
DIR *dir;
char dirname[PATH_MAX];
struct rte_pci_addr addr;
+ struct rte_pci_device *dev, *tmp;
+
+ /* Search the device list for devices that are no longer present on the system
+ * and remove them.
+ */
+ TAILQ_FOREACH_SAFE(dev, &pci_device_list, next, tmp) {
+ int found = 0;
+
+ dir = opendir(pci_get_sysfs_path());
+ if (dir == NULL) {
+ RTE_LOG(ERR, EAL, "%s(): opendir failed: %s\n",
+ __func__, strerror(errno));
+ return -1;
+ }
+
+ while ((e = readdir(dir)) != NULL) {
+ if (e->d_name[0] == '.')
+ continue;
+
+ if (parse_pci_addr_format(e->d_name, sizeof(e->d_name),
+ &addr) != 0)
+ continue;
+
+ if (rte_eal_compare_pci_addr(&addr, &dev->addr) == 0) {
+ found = 1;
+ break;
+ }
+ }
+
+ if (!found) {
+
+ RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" was removed.\n",
+ addr.domain, addr.bus, addr.devid,
+ addr.function);
+
+ if (dev->driver) {
+ /* A driver was loaded against this device.
+ * Unload it. */
+ RTE_LOG(DEBUG, EAL, " Unload driver: %x:%x %s\n",
+ dev->id.vendor_id, dev->id.device_id,
+ dev->driver->driver.name);
+
+ if (dev->driver->remove) {
+ /* It doesn't matter what remove returns -
+ * we're removing the device either way. */
+ dev->driver->remove(dev);
+ }
+
+ /* clear driver structure */
+ dev->driver = NULL;
+
+ if (dev->driver->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+ rte_eal_pci_unmap_device(dev);
+ }
+
+ TAILQ_REMOVE(&pci_device_list, dev, next);
+ free(dev);
+ }
+
+ closedir(dir);
+ }
+ /* Search sysfs for all PCI devices and add them to the device list */
dir = opendir(pci_get_sysfs_path());
if (dir == NULL) {
RTE_LOG(ERR, EAL, "%s(): opendir failed: %s\n",
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v2 5/7] pci: Move driver registration above pci scan
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
` (2 preceding siblings ...)
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices Ben Walker
@ 2016-11-23 20:07 ` Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe Ben Walker
` (3 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Ben Walker @ 2016-11-23 20:07 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
The user needs to register drivers before scanning, so
it makes the most sense to put the registration
functions above the scan function in the header file.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/common/include/rte_pci.h | 56 ++++++++++++++++-----------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 74be1f5..5d0feac 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -358,6 +358,34 @@ rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
}
/**
+ * Register a PCI driver.
+ *
+ * @param driver
+ * A pointer to a rte_pci_driver structure describing the driver
+ * to be registered.
+ */
+void rte_eal_pci_register(struct rte_pci_driver *driver);
+
+/** Statically register a PCI driver at start up */
+#define RTE_PMD_REGISTER_PCI(nm, pci_drv) \
+RTE_INIT(pciinitfn_ ##nm); \
+static void pciinitfn_ ##nm(void) \
+{\
+ (pci_drv).driver.name = RTE_STR(nm);\
+ rte_eal_pci_register(&pci_drv); \
+} \
+RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
+
+/**
+ * Unregister a PCI driver.
+ *
+ * @param driver
+ * A pointer to a rte_pci_driver structure describing the driver
+ * to be unregistered.
+ */
+void rte_eal_pci_unregister(struct rte_pci_driver *driver);
+
+/**
* Scan the content of the PCI bus, and the devices in the devices
* list
*
@@ -476,34 +504,6 @@ int rte_eal_pci_detach(const struct rte_pci_addr *addr);
void rte_eal_pci_dump(FILE *f);
/**
- * Register a PCI driver.
- *
- * @param driver
- * A pointer to a rte_pci_driver structure describing the driver
- * to be registered.
- */
-void rte_eal_pci_register(struct rte_pci_driver *driver);
-
-/** Helper for PCI device registration from driver (eth, crypto) instance */
-#define RTE_PMD_REGISTER_PCI(nm, pci_drv) \
-RTE_INIT(pciinitfn_ ##nm); \
-static void pciinitfn_ ##nm(void) \
-{\
- (pci_drv).driver.name = RTE_STR(nm);\
- rte_eal_pci_register(&pci_drv); \
-} \
-RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
-
-/**
- * Unregister a PCI driver.
- *
- * @param driver
- * A pointer to a rte_pci_driver structure describing the driver
- * to be unregistered.
- */
-void rte_eal_pci_unregister(struct rte_pci_driver *driver);
-
-/**
* Read PCI config space.
*
* @param device
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v2 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
` (3 preceding siblings ...)
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 5/7] pci: Move driver registration above pci scan Ben Walker
@ 2016-11-23 20:07 ` Ben Walker
2016-11-25 10:56 ` Shreyansh Jain
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers Ben Walker
` (2 subsequent siblings)
7 siblings, 1 reply; 34+ messages in thread
From: Ben Walker @ 2016-11-23 20:07 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
Two functions is both confusing and unnecessary. Previously,
rte_eal_pci_scan populated an internal list of devices by
scanning sysfs. Then, rte_eal_pci_probe would match registered
drivers to that internal list. These are not really useful
operations to perform separately, though, so
simplify the api down to just rte_eal_pci_probe which can
be called repeatedly through the lifetime of the application
to scan for new or removed PCI devices and load or unload
drivers as required.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
app/test/test_pci.c | 2 +-
lib/librte_eal/bsdapp/eal/eal.c | 3 ---
lib/librte_eal/bsdapp/eal/eal_pci.c | 17 +----------------
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 -
lib/librte_eal/common/eal_common_pci.c | 6 ++++++
lib/librte_eal/common/eal_private.h | 14 +++++---------
lib/librte_eal/common/include/rte_pci.h | 17 +++++------------
lib/librte_eal/linuxapp/eal/eal.c | 3 ---
lib/librte_eal/linuxapp/eal/eal_pci.c | 18 +-----------------
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 -
10 files changed, 19 insertions(+), 63 deletions(-)
diff --git a/app/test/test_pci.c b/app/test/test_pci.c
index cda186d..fdd84f7 100644
--- a/app/test/test_pci.c
+++ b/app/test/test_pci.c
@@ -180,7 +180,7 @@ test_pci_setup(void)
TAILQ_INSERT_TAIL(&real_pci_device_list, dev, next);
}
- ret = rte_eal_pci_scan();
+ ret = rte_eal_pci_probe();
TEST_ASSERT_SUCCESS(ret, "failed to scan PCI bus");
rte_eal_pci_dump(stdout);
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 35e3117..fd44528 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -561,9 +561,6 @@ rte_eal_init(int argc, char **argv)
if (rte_eal_timer_init() < 0)
rte_panic("Cannot init HPET or TSC timers\n");
- if (rte_eal_pci_init() < 0)
- rte_panic("Cannot init PCI\n");
-
eal_check_mem_on_local_socket();
if (eal_plugins_init() < 0)
diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 8b3ed88..6c3a169 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -361,7 +361,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
* list. Call pci_scan_one() for each pci entry found.
*/
int
-rte_eal_pci_scan(void)
+pci_scan(void)
{
int fd;
unsigned dev_count = 0;
@@ -667,18 +667,3 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p)
return ret;
}
-
-/* Init the PCI EAL subsystem */
-int
-rte_eal_pci_init(void)
-{
- /* for debug purposes, PCI can be disabled */
- if (internal_config.no_pci)
- return 0;
-
- if (rte_eal_pci_scan() < 0) {
- RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
- return -1;
- }
- return 0;
-}
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 2f81f7c..67c469c 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -44,7 +44,6 @@ DPDK_2.0 {
rte_eal_pci_probe;
rte_eal_pci_probe_one;
rte_eal_pci_register;
- rte_eal_pci_scan;
rte_eal_pci_unregister;
rte_eal_process_type;
rte_eal_remote_launch;
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 4f8c3a0..d50a534 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -81,6 +81,7 @@
#include <rte_devargs.h>
#include "eal_private.h"
+#include "eal_internal_cfg.h"
struct pci_driver_list pci_driver_list =
TAILQ_HEAD_INITIALIZER(pci_driver_list);
@@ -423,6 +424,11 @@ rte_eal_pci_probe(void)
int probe_all = 0;
int ret = 0;
+ if (internal_config.no_pci)
+ return 0;
+
+ pci_scan();
+
if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) == 0)
probe_all = 1;
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 9e7d8f6..54f18ea 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -108,18 +108,14 @@ int rte_eal_timer_init(void);
*/
int rte_eal_log_init(const char *id, int facility);
-/**
- * Init the PCI infrastructure
+struct rte_pci_driver;
+struct rte_pci_device;
+
+/* Scan the PCI bus for devices
*
* This function is private to EAL.
- *
- * @return
- * 0 on success, negative on error
*/
-int rte_eal_pci_init(void);
-
-struct rte_pci_driver;
-struct rte_pci_device;
+int pci_scan(void);
/**
* Update a pci device object by asking the kernel for the latest information.
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 5d0feac..2154a54 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -386,20 +386,13 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
void rte_eal_pci_unregister(struct rte_pci_driver *driver);
/**
- * Scan the content of the PCI bus, and the devices in the devices
- * list
- *
- * @return
- * 0 on success, negative on error
- */
-int rte_eal_pci_scan(void);
-
-/**
- * Probe the PCI bus for registered drivers.
+ * Scan the PCI bus for devices and match them to their driver.
*
* Scan the content of the PCI bus, and call the probe() function for
- * all registered drivers that have a matching entry in its id_table
- * for discovered devices.
+ * all registered drivers that have a matching entry in their id_table.
+ * If a device already has a driver loaded, probe will not be called.
+ * If a previously discovered device is no longer present on the system,
+ * the associated driver's remove() callback will be called.
*
* @return
* - 0 on success.
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 2075282..f47f361 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -802,9 +802,6 @@ rte_eal_init(int argc, char **argv)
if (rte_eal_log_init(logid, internal_config.syslog_facility) < 0)
rte_panic("Cannot init logs\n");
- if (rte_eal_pci_init() < 0)
- rte_panic("Cannot init PCI\n");
-
#ifdef VFIO_PRESENT
if (rte_eal_vfio_setup() < 0)
rte_panic("Cannot init VFIO\n");
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 936f076..5146385 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -479,7 +479,7 @@ parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
* list
*/
int
-rte_eal_pci_scan(void)
+pci_scan(void)
{
struct dirent *e;
DIR *dir;
@@ -810,19 +810,3 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p)
return ret;
}
-
-/* Init the PCI EAL subsystem */
-int
-rte_eal_pci_init(void)
-{
- /* for debug purposes, PCI can be disabled */
- if (internal_config.no_pci)
- return 0;
-
- if (rte_eal_pci_scan() < 0) {
- RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
- return -1;
- }
-
- return 0;
-}
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 83721ba..856728e 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -44,7 +44,6 @@ DPDK_2.0 {
rte_eal_pci_probe;
rte_eal_pci_probe_one;
rte_eal_pci_register;
- rte_eal_pci_scan;
rte_eal_pci_unregister;
rte_eal_process_type;
rte_eal_remote_launch;
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v2 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
` (4 preceding siblings ...)
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe Ben Walker
@ 2016-11-23 20:07 ` Ben Walker
2016-12-03 6:55 ` Shreyansh Jain
2016-11-25 9:21 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Shreyansh Jain
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
7 siblings, 1 reply; 34+ messages in thread
From: Ben Walker @ 2016-11-23 20:07 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
There are now two functions - rte_eal_pci_attach_driver and
rte_eal_pci_detach_driver - that dynamically attempt to attach
and detach drivers from PCI devices. These only control
whether a registered PCI driver is loaded or not - they are
independent of whether the PCI device exists on the system.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/common/eal_common_dev.c | 4 +-
lib/librte_eal/common/eal_common_pci.c | 109 +++++++++-----------------------
lib/librte_eal/common/include/rte_pci.h | 22 ++++---
3 files changed, 43 insertions(+), 92 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 4f3b493..1c6834e 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -114,7 +114,7 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
}
if (eal_parse_pci_DomBDF(name, &addr) == 0) {
- if (rte_eal_pci_probe_one(&addr) < 0)
+ if (rte_eal_pci_attach_driver(&addr) < 0)
goto err;
} else {
@@ -139,7 +139,7 @@ int rte_eal_dev_detach(const char *name)
}
if (eal_parse_pci_DomBDF(name, &addr) == 0) {
- if (rte_eal_pci_detach(&addr) < 0)
+ if (rte_eal_pci_detach_driver(&addr) < 0)
goto err;
} else {
if (rte_eal_vdev_uninit(name))
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index d50a534..67c6ce6 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -228,59 +228,36 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
return 1;
}
-/*
- * If vendor/device ID match, call the remove() function of the
- * driver.
- */
static int
-rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
- struct rte_pci_device *dev)
+rte_eal_pci_remove_driver(struct rte_pci_device *dev)
{
- const struct rte_pci_id *id_table;
+ struct rte_pci_driver *driver;
+ struct rte_pci_addr *loc;
- if ((dr == NULL) || (dev == NULL))
- return -EINVAL;
+ loc = &dev->addr;
+ driver = dev->driver;
- for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
-
- /* check if device's identifiers match the driver's ones */
- if (id_table->vendor_id != dev->id.vendor_id &&
- id_table->vendor_id != PCI_ANY_ID)
- continue;
- if (id_table->device_id != dev->id.device_id &&
- id_table->device_id != PCI_ANY_ID)
- continue;
- if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
- id_table->subsystem_vendor_id != PCI_ANY_ID)
- continue;
- if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
- id_table->subsystem_device_id != PCI_ANY_ID)
- continue;
-
- struct rte_pci_addr *loc = &dev->addr;
+ if (driver == NULL)
+ return 0;
- RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
- loc->domain, loc->bus, loc->devid,
- loc->function, dev->device.numa_node);
+ RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+ loc->domain, loc->bus, loc->devid,
+ loc->function, dev->device.numa_node);
- RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
- dev->id.device_id, dr->driver.name);
+ RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
+ dev->id.device_id, driver->driver.name);
- if (dr->remove && (dr->remove(dev) < 0))
- return -1; /* negative value is an error */
+ if (driver->remove && (driver->remove(dev) < 0))
+ return -1; /* negative value is an error */
- /* clear driver structure */
- dev->driver = NULL;
+ /* clear driver structure */
+ dev->driver = NULL;
- if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
- /* unmap resources for devices that use igb_uio */
- rte_eal_pci_unmap_device(dev);
+ if (driver->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+ /* unmap resources for devices that use igb_uio */
+ rte_eal_pci_unmap_device(dev);
- return 0;
- }
-
- /* return positive value if driver doesn't support this device */
- return 1;
+ return 0;
}
/*
@@ -315,38 +292,11 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
}
/*
- * If vendor/device ID match, call the remove() function of all
- * registered driver for the given device. Return -1 if initialization
- * failed, return 1 if no driver is found for this device.
- */
-static int
-pci_detach_all_drivers(struct rte_pci_device *dev)
-{
- struct rte_pci_driver *dr = NULL;
- int rc = 0;
-
- if (dev == NULL)
- return -1;
-
- TAILQ_FOREACH(dr, &pci_driver_list, next) {
- rc = rte_eal_pci_detach_dev(dr, dev);
- if (rc < 0)
- /* negative value is an error */
- return -1;
- if (rc > 0)
- /* positive value means driver doesn't support it */
- continue;
- return 0;
- }
- return 1;
-}
-
-/*
* Find the pci device specified by pci address, then invoke probe function of
* the driver of the devive.
*/
int
-rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
+rte_eal_pci_attach_driver(const struct rte_pci_addr *addr)
{
struct rte_pci_device *dev = NULL;
int ret = 0;
@@ -382,10 +332,9 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
* Detach device specified by its pci address.
*/
int
-rte_eal_pci_detach(const struct rte_pci_addr *addr)
+rte_eal_pci_detach_driver(const struct rte_pci_addr *addr)
{
struct rte_pci_device *dev = NULL;
- int ret = 0;
if (addr == NULL)
return -1;
@@ -394,17 +343,17 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
if (rte_eal_compare_pci_addr(&dev->addr, addr))
continue;
- ret = pci_detach_all_drivers(dev);
- if (ret < 0)
- goto err_return;
+ if (dev->driver == NULL) {
+ /* The device at that address does not have a driver loaded */
+ return 0;
+ }
+
+ if (rte_eal_pci_remove_driver(dev) < 0)
+ return -1;
- TAILQ_REMOVE(&pci_device_list, dev, next);
- free(dev);
return 0;
}
- return -1;
-err_return:
RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT
" cannot be used\n", dev->addr.domain, dev->addr.bus,
dev->addr.devid, dev->addr.function);
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 2154a54..e304853 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -459,11 +459,14 @@ void *pci_map_resource(void *requested_addr, int fd, off_t offset,
void pci_unmap_resource(void *requested_addr, size_t size);
/**
- * Probe the single PCI device.
+ * Attempt to load the registered driver for the PCI device at addr.
*
- * Scan the content of the PCI bus, and find the pci device specified by pci
- * address, then call the probe() function for registered driver that has a
- * matching entry in its id_table for discovered device.
+ * Find the pci device specified by addr, then call the probe() function
+ * for each registered driver that has a matching entry in its id_table until
+ * the correct driver is found.
+ *
+ * If the PCI address is known, this is considerably more efficient than
+ * calling rte_eal_pci_probe.
*
* @param addr
* The PCI Bus-Device-Function address to probe.
@@ -471,14 +474,13 @@ void pci_unmap_resource(void *requested_addr, size_t size);
* - 0 on success.
* - Negative on error.
*/
-int rte_eal_pci_probe_one(const struct rte_pci_addr *addr);
+int rte_eal_pci_attach_driver(const struct rte_pci_addr *addr);
/**
- * Close the single PCI device.
+ * Unload the driver for the PCI device at addr.
*
- * Scan the content of the PCI bus, and find the pci device specified by pci
- * address, then call the remove() function for registered driver that has a
- * matching entry in its id_table for discovered device.
+ * Find the pci device specified by addr, then call the remove() function
+ * for the currently loaded driver.
*
* @param addr
* The PCI Bus-Device-Function address to close.
@@ -486,7 +488,7 @@ int rte_eal_pci_probe_one(const struct rte_pci_addr *addr);
* - 0 on success.
* - Negative on error.
*/
-int rte_eal_pci_detach(const struct rte_pci_addr *addr);
+int rte_eal_pci_detach_driver(const struct rte_pci_addr *addr);
/**
* Dump the content of the PCI bus.
--
2.7.4
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources.
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
` (5 preceding siblings ...)
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers Ben Walker
@ 2016-11-25 9:21 ` Shreyansh Jain
2016-12-21 16:19 ` Thomas Monjalon
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
7 siblings, 1 reply; 34+ messages in thread
From: Shreyansh Jain @ 2016-11-25 9:21 UTC (permalink / raw)
To: Ben Walker; +Cc: dev
On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> If resources were mapped prior to probe, unmap them
> if probe fails.
>
> This does not handle the case where the kernel driver was
> forcibly unbound prior to probe.
>
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
> lib/librte_eal/common/eal_common_pci.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
> index 6bff675..4f8c3a0 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -215,8 +215,11 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
>
> /* call the driver probe() function */
> ret = dr->probe(dr, dev);
> - if (ret)
> + if (ret) {
> dev->driver = NULL;
> + if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
> + rte_eal_pci_unmap_device(dev);
> + }
>
> return ret;
> }
>
Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 2/7] pci: Separate detaching ethernet ports from PCI devices
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 2/7] pci: Separate detaching ethernet ports from PCI devices Ben Walker
@ 2016-11-25 9:25 ` Shreyansh Jain
0 siblings, 0 replies; 34+ messages in thread
From: Shreyansh Jain @ 2016-11-25 9:25 UTC (permalink / raw)
To: Ben Walker; +Cc: dev
On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> Attaching and detaching ethernet ports from an application
> is not the same thing as physically removing a PCI device,
> so clarify the flags indicating support. All PCI devices
> are assumed to be physically removable, so no flag is
> necessary in the PCI layer.
>
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
> doc/guides/prog_guide/port_hotplug_framework.rst | 2 +-
> drivers/net/bnxt/bnxt_ethdev.c | 3 ++-
> drivers/net/e1000/em_ethdev.c | 4 ++--
> drivers/net/e1000/igb_ethdev.c | 7 ++++---
> drivers/net/fm10k/fm10k_ethdev.c | 4 ++--
> drivers/net/i40e/i40e_ethdev.c | 4 ++--
> drivers/net/i40e/i40e_ethdev_vf.c | 3 ++-
> drivers/net/ixgbe/ixgbe_ethdev.c | 7 ++++---
> drivers/net/nfp/nfp_net.c | 4 ++--
> drivers/net/virtio/virtio_ethdev.c | 3 ++-
> drivers/net/vmxnet3/vmxnet3_ethdev.c | 3 ++-
> drivers/net/xenvirt/rte_eth_xenvirt.c | 2 +-
> lib/librte_eal/common/include/rte_pci.h | 2 --
> lib/librte_ether/rte_ethdev.c | 2 --
> 14 files changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/doc/guides/prog_guide/port_hotplug_framework.rst b/doc/guides/prog_guide/port_hotplug_framework.rst
> index 6e4436e..d68d08e 100644
> --- a/doc/guides/prog_guide/port_hotplug_framework.rst
> +++ b/doc/guides/prog_guide/port_hotplug_framework.rst
> @@ -106,5 +106,5 @@ Limitations
>
> * Not all PMDs support detaching feature.
> To know whether a PMD can support detaching, search for the
> - "RTE_PCI_DRV_DETACHABLE" flag in PMD implementation. If the flag is
> + "RTE_ETH_DEV_DETAHABLE" flag in rte_eth_dev::data::dev_flags. If the flag is
Incorrect spelling. Should be 'RTE_ETH_DEV_DETACHABLE'.
> defined in the PMD, detaching is supported.
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index 035fe07..a2100f6 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -1051,6 +1051,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
> RTE_LOG(INFO, PMD, "%s", bnxt_version);
>
> rte_eth_copy_pci_info(eth_dev, eth_dev->pci_dev);
> + eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
> bp = eth_dev->data->dev_private;
>
> if (bnxt_vf_pciid(eth_dev->pci_dev->id.device_id))
[...]
-
Shreyansh
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
@ 2016-11-25 10:33 ` Shreyansh Jain
2016-12-01 6:26 ` Shreyansh Jain
1 sibling, 0 replies; 34+ messages in thread
From: Shreyansh Jain @ 2016-11-25 10:33 UTC (permalink / raw)
To: Ben Walker; +Cc: dev
On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> Instead of passing domain, bus, devid, func, just pass
> an rte_pci_addr.
>
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
> lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++-------------------
> 1 file changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index 876ba38..073af5f 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -267,8 +267,7 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
>
> /* Scan one pci sysfs entry, and fill the devices list from it. */
> static int
> -pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
> - uint8_t devid, uint8_t function)
> +pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
> {
> char filename[PATH_MAX];
> unsigned long tmp;
> @@ -281,10 +280,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
> return -1;
>
> memset(dev, 0, sizeof(*dev));
> - dev->addr.domain = domain;
> - dev->addr.bus = bus;
> - dev->addr.devid = devid;
> - dev->addr.function = function;
> + dev->addr = *addr;
>
> /* get vendor id */
> snprintf(filename, sizeof(filename), "%s/vendor", dirname);
> @@ -429,16 +425,14 @@ pci_update_device(const struct rte_pci_addr *addr)
> pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
> addr->function);
>
> - return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
> - addr->function);
> + return pci_scan_one(filename, addr);
> }
>
> /*
> * split up a pci address into its constituent parts.
> */
> static int
> -parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
> - uint8_t *bus, uint8_t *devid, uint8_t *function)
> +parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
> {
> /* first split on ':' */
> union splitaddr {
> @@ -466,10 +460,10 @@ parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
>
> /* now convert to int values */
> errno = 0;
> - *domain = (uint16_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);
> + addr->domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
> + addr->bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
> + addr->devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
> + addr->function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
> if (errno != 0)
> goto error;
>
> @@ -490,8 +484,7 @@ rte_eal_pci_scan(void)
> struct dirent *e;
> DIR *dir;
> char dirname[PATH_MAX];
> - uint16_t domain;
> - uint8_t bus, devid, function;
> + struct rte_pci_addr addr;
>
> dir = opendir(pci_get_sysfs_path());
> if (dir == NULL) {
> @@ -500,20 +493,21 @@ rte_eal_pci_scan(void)
> return -1;
> }
>
> +
Unnecessary new line.
> while ((e = readdir(dir)) != NULL) {
> if (e->d_name[0] == '.')
> continue;
>
> - if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &domain,
> - &bus, &devid, &function) != 0)
> + if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &addr) != 0)
> continue;
>
> snprintf(dirname, sizeof(dirname), "%s/%s",
> pci_get_sysfs_path(), e->d_name);
> - if (pci_scan_one(dirname, domain, bus, devid, function) < 0)
> + if (pci_scan_one(dirname, &addr) < 0)
> goto error;
> }
> closedir(dir);
> +
> return 0;
>
> error:
>
This is much more cleaner than passing all the BDF entries.
Except the above unnecessary new line:
Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices Ben Walker
@ 2016-11-25 10:44 ` Shreyansh Jain
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
1 sibling, 0 replies; 34+ messages in thread
From: Shreyansh Jain @ 2016-11-25 10:44 UTC (permalink / raw)
To: Ben Walker; +Cc: dev
Hi Ben,
On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> rte_eal_pci_scan can be called repeatedly to re-scan the PCI
> bus. If a device was removed from the system, the associated
> driver will automatically be unloaded.
>
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
[...]
While reviewing, I found that there are some checkpatch warnings on this
patch:
--->8---
### [PATCH v2 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices
WARNING:LONG_LINE_COMMENT: line over 80 characters
#76: FILE: lib/librte_eal/linuxapp/eal/eal_pci.c:490:
+ /* Search the device list for devices that are no longer present
on the system
WARNING:LONG_LINE_STRING: line over 80 characters
#105: FILE: lib/librte_eal/linuxapp/eal/eal_pci.c:519:
+ RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT"
was removed.\n",
WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a
separate line
#111: FILE: lib/librte_eal/linuxapp/eal/eal_pci.c:525:
+ * Unload it. */
WARNING:LONG_LINE_STRING: line over 80 characters
#112: FILE: lib/librte_eal/linuxapp/eal/eal_pci.c:526:
+ RTE_LOG(DEBUG, EAL, " Unload driver:
%x:%x %s\n",
WARNING:LONG_LINE: line over 80 characters
#113: FILE: lib/librte_eal/linuxapp/eal/eal_pci.c:527:
+ dev->id.vendor_id,
dev->id.device_id,
WARNING:LONG_LINE_COMMENT: line over 80 characters
#117: FILE: lib/librte_eal/linuxapp/eal/eal_pci.c:531:
+ /* It doesn't matter what remove
returns -
WARNING:LONG_LINE_COMMENT: line over 80 characters
#118: FILE: lib/librte_eal/linuxapp/eal/eal_pci.c:532:
+ * we're removing the device
either way. */
WARNING:BLOCK_COMMENT_STYLE: Block comments use a trailing */ on a
separate line
#118: FILE: lib/librte_eal/linuxapp/eal/eal_pci.c:532:
+ * we're removing the device
either way. */
WARNING:LONG_LINE: line over 80 characters
#125: FILE: lib/librte_eal/linuxapp/eal/eal_pci.c:539:
+ if (dev->driver->drv_flags &
RTE_PCI_DRV_NEED_MAPPING)
total: 0 errors, 9 warnings, 69 lines checked
--->8---
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe Ben Walker
@ 2016-11-25 10:56 ` Shreyansh Jain
0 siblings, 0 replies; 34+ messages in thread
From: Shreyansh Jain @ 2016-11-25 10:56 UTC (permalink / raw)
To: Ben Walker; +Cc: dev, david.marchand, Ferruh Yigit
On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> Two functions is both confusing and unnecessary. Previously,
> rte_eal_pci_scan populated an internal list of devices by
> scanning sysfs. Then, rte_eal_pci_probe would match registered
> drivers to that internal list. These are not really useful
> operations to perform separately, though, so
> simplify the api down to just rte_eal_pci_probe which can
> be called repeatedly through the lifetime of the application
> to scan for new or removed PCI devices and load or unload
> drivers as required.
Agree with this.
And similar case exists for rte_eal_dev_init() for which there is
another patch floated by Jerin [1].
Also, I am already merging these two (EAL Bus model) [2]. So, we have:
- Only a probe called from EAL for all devices, whether PCI, VDEV or
another other type
- Probe in turns performs all scans and driver->probes()
Concern I have is that with the change of placement for device scan,
would it impact the external modules/PMDs as currently the scanned list
is created *before* eal_plugins_init(). But, now the list is created
*after* plugin initialization.
There are other similar inits like creation of slave threads. As well as
concern raised by Ferruh about device enumeration.
I don't have clear idea of all such dependencies. Maybe David and Ferruh
in CC might be able to comment better.
[1] http://dpdk.org/ml/archives/dev/2016-November/050415.html
[2] http://dpdk.org/ml/archives/dev/2016-November/050301.html
>
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
> app/test/test_pci.c | 2 +-
> lib/librte_eal/bsdapp/eal/eal.c | 3 ---
> lib/librte_eal/bsdapp/eal/eal_pci.c | 17 +----------------
> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 -
> lib/librte_eal/common/eal_common_pci.c | 6 ++++++
> lib/librte_eal/common/eal_private.h | 14 +++++---------
> lib/librte_eal/common/include/rte_pci.h | 17 +++++------------
> lib/librte_eal/linuxapp/eal/eal.c | 3 ---
> lib/librte_eal/linuxapp/eal/eal_pci.c | 18 +-----------------
> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 -
> 10 files changed, 19 insertions(+), 63 deletions(-)
>
> diff --git a/app/test/test_pci.c b/app/test/test_pci.c
> index cda186d..fdd84f7 100644
> --- a/app/test/test_pci.c
> +++ b/app/test/test_pci.c
> @@ -180,7 +180,7 @@ test_pci_setup(void)
> TAILQ_INSERT_TAIL(&real_pci_device_list, dev, next);
> }
>
> - ret = rte_eal_pci_scan();
> + ret = rte_eal_pci_probe();
> TEST_ASSERT_SUCCESS(ret, "failed to scan PCI bus");
> rte_eal_pci_dump(stdout);
>
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 35e3117..fd44528 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -561,9 +561,6 @@ rte_eal_init(int argc, char **argv)
> if (rte_eal_timer_init() < 0)
> rte_panic("Cannot init HPET or TSC timers\n");
>
> - if (rte_eal_pci_init() < 0)
> - rte_panic("Cannot init PCI\n");
> -
> eal_check_mem_on_local_socket();
>
> if (eal_plugins_init() < 0)
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index 8b3ed88..6c3a169 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> @@ -361,7 +361,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
> * list. Call pci_scan_one() for each pci entry found.
> */
> int
> -rte_eal_pci_scan(void)
> +pci_scan(void)
> {
> int fd;
> unsigned dev_count = 0;
> @@ -667,18 +667,3 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p)
>
> return ret;
> }
> -
> -/* Init the PCI EAL subsystem */
> -int
> -rte_eal_pci_init(void)
> -{
> - /* for debug purposes, PCI can be disabled */
> - if (internal_config.no_pci)
> - return 0;
> -
> - if (rte_eal_pci_scan() < 0) {
> - RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
> - return -1;
> - }
> - return 0;
> -}
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 2f81f7c..67c469c 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -44,7 +44,6 @@ DPDK_2.0 {
> rte_eal_pci_probe;
> rte_eal_pci_probe_one;
> rte_eal_pci_register;
> - rte_eal_pci_scan;
> rte_eal_pci_unregister;
> rte_eal_process_type;
> rte_eal_remote_launch;
> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
> index 4f8c3a0..d50a534 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -81,6 +81,7 @@
> #include <rte_devargs.h>
>
> #include "eal_private.h"
> +#include "eal_internal_cfg.h"
>
> struct pci_driver_list pci_driver_list =
> TAILQ_HEAD_INITIALIZER(pci_driver_list);
> @@ -423,6 +424,11 @@ rte_eal_pci_probe(void)
> int probe_all = 0;
> int ret = 0;
>
> + if (internal_config.no_pci)
> + return 0;
> +
> + pci_scan();
> +
So, no check for error reported by scan?
I think we should, 1) because pci_scan() returns one, and 2) because in
case scan failed, there is no reason probe would do anything worthwhile.
> if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) == 0)
> probe_all = 1;
>
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index 9e7d8f6..54f18ea 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -108,18 +108,14 @@ int rte_eal_timer_init(void);
> */
> int rte_eal_log_init(const char *id, int facility);
>
> -/**
> - * Init the PCI infrastructure
> +struct rte_pci_driver;
> +struct rte_pci_device;
> +
> +/* Scan the PCI bus for devices
> *
> * This function is private to EAL.
> - *
> - * @return
> - * 0 on success, negative on error
> */
> -int rte_eal_pci_init(void);
> -
> -struct rte_pci_driver;
> -struct rte_pci_device;
> +int pci_scan(void);
>
> /**
> * Update a pci device object by asking the kernel for the latest information.
> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
> index 5d0feac..2154a54 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -386,20 +386,13 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
> void rte_eal_pci_unregister(struct rte_pci_driver *driver);
>
> /**
> - * Scan the content of the PCI bus, and the devices in the devices
> - * list
> - *
> - * @return
> - * 0 on success, negative on error
> - */
> -int rte_eal_pci_scan(void);
> -
> -/**
> - * Probe the PCI bus for registered drivers.
> + * Scan the PCI bus for devices and match them to their driver.
> *
> * Scan the content of the PCI bus, and call the probe() function for
> - * all registered drivers that have a matching entry in its id_table
> - * for discovered devices.
> + * all registered drivers that have a matching entry in their id_table.
> + * If a device already has a driver loaded, probe will not be called.
> + * If a previously discovered device is no longer present on the system,
> + * the associated driver's remove() callback will be called.
> *
> * @return
> * - 0 on success.
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 2075282..f47f361 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -802,9 +802,6 @@ rte_eal_init(int argc, char **argv)
> if (rte_eal_log_init(logid, internal_config.syslog_facility) < 0)
> rte_panic("Cannot init logs\n");
>
> - if (rte_eal_pci_init() < 0)
> - rte_panic("Cannot init PCI\n");
> -
> #ifdef VFIO_PRESENT
> if (rte_eal_vfio_setup() < 0)
> rte_panic("Cannot init VFIO\n");
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index 936f076..5146385 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -479,7 +479,7 @@ parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
> * list
> */
> int
> -rte_eal_pci_scan(void)
> +pci_scan(void)
> {
> struct dirent *e;
> DIR *dir;
> @@ -810,19 +810,3 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p)
>
> return ret;
> }
> -
> -/* Init the PCI EAL subsystem */
> -int
> -rte_eal_pci_init(void)
> -{
> - /* for debug purposes, PCI can be disabled */
> - if (internal_config.no_pci)
> - return 0;
> -
> - if (rte_eal_pci_scan() < 0) {
> - RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
> - return -1;
> - }
> -
> - return 0;
> -}
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 83721ba..856728e 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -44,7 +44,6 @@ DPDK_2.0 {
> rte_eal_pci_probe;
> rte_eal_pci_probe_one;
> rte_eal_pci_register;
> - rte_eal_pci_scan;
> rte_eal_pci_unregister;
> rte_eal_process_type;
> rte_eal_remote_launch;
>
--
-
Shreyansh
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
2016-11-25 10:33 ` Shreyansh Jain
@ 2016-12-01 6:26 ` Shreyansh Jain
2016-12-02 16:16 ` Walker, Benjamin
1 sibling, 1 reply; 34+ messages in thread
From: Shreyansh Jain @ 2016-12-01 6:26 UTC (permalink / raw)
To: Ben Walker; +Cc: dev
Hello Ben,
On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> Instead of passing domain, bus, devid, func, just pass
> an rte_pci_addr.
>
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
> lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++-------------------
> 1 file changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index 876ba38..073af5f 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -267,8 +267,7 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
>
> /* Scan one pci sysfs entry, and fill the devices list from it. */
> static int
> -pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
> - uint8_t devid, uint8_t function)
> +pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
> {
> char filename[PATH_MAX];
> unsigned long tmp;
> @@ -281,10 +280,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
> return -1;
>
> memset(dev, 0, sizeof(*dev));
> - dev->addr.domain = domain;
> - dev->addr.bus = bus;
> - dev->addr.devid = devid;
> - dev->addr.function = function;
> + dev->addr = *addr;
>
> /* get vendor id */
> snprintf(filename, sizeof(filename), "%s/vendor", dirname);
> @@ -429,16 +425,14 @@ pci_update_device(const struct rte_pci_addr *addr)
> pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
> addr->function);
>
> - return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
> - addr->function);
> + return pci_scan_one(filename, addr);
> }
>
> /*
> * split up a pci address into its constituent parts.
> */
> static int
> -parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
> - uint8_t *bus, uint8_t *devid, uint8_t *function)
> +parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
> {
> /* first split on ':' */
> union splitaddr {
> @@ -466,10 +460,10 @@ parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
>
> /* now convert to int values */
> errno = 0;
> - *domain = (uint16_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);
> + addr->domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
> + addr->bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
> + addr->devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
> + addr->function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
> if (errno != 0)
> goto error;
>
> @@ -490,8 +484,7 @@ rte_eal_pci_scan(void)
> struct dirent *e;
> DIR *dir;
> char dirname[PATH_MAX];
> - uint16_t domain;
> - uint8_t bus, devid, function;
> + struct rte_pci_addr addr;
>
> dir = opendir(pci_get_sysfs_path());
> if (dir == NULL) {
> @@ -500,20 +493,21 @@ rte_eal_pci_scan(void)
> return -1;
> }
>
> +
> while ((e = readdir(dir)) != NULL) {
> if (e->d_name[0] == '.')
> continue;
>
> - if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &domain,
> - &bus, &devid, &function) != 0)
> + if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &addr) != 0)
> continue;
>
> snprintf(dirname, sizeof(dirname), "%s/%s",
> pci_get_sysfs_path(), e->d_name);
> - if (pci_scan_one(dirname, domain, bus, devid, function) < 0)
> + if (pci_scan_one(dirname, &addr) < 0)
> goto error;
> }
> closedir(dir);
> +
> return 0;
>
> error:
>
Do you mind if I use this patch directly as part of my patchset for bus
Model? I was doing a similar change to make the pci_scan_one simpler
(and pass along a new argument)..
-
Shreyansh
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args
2016-12-01 6:26 ` Shreyansh Jain
@ 2016-12-02 16:16 ` Walker, Benjamin
0 siblings, 0 replies; 34+ messages in thread
From: Walker, Benjamin @ 2016-12-02 16:16 UTC (permalink / raw)
To: shreyansh.jain; +Cc: dev
On Thu, 2016-12-01 at 11:56 +0530, Shreyansh Jain wrote:
> Hello Ben,
>
> On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> >
> > Instead of passing domain, bus, devid, func, just pass
> > an rte_pci_addr.
> >
> > Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> > ---
> > lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++-------------------
> > 1 file changed, 13 insertions(+), 19 deletions(-)
> >
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c
> > b/lib/librte_eal/linuxapp/eal/eal_pci.c
> > index 876ba38..073af5f 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> > @@ -267,8 +267,7 @@ pci_parse_sysfs_resource(const char *filename, struct
> > rte_pci_device *dev)
> >
> > /* Scan one pci sysfs entry, and fill the devices list from it. */
> > static int
> > -pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
> > - uint8_t devid, uint8_t function)
> > +pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
> > {
> > char filename[PATH_MAX];
> > unsigned long tmp;
> > @@ -281,10 +280,7 @@ pci_scan_one(const char *dirname, uint16_t domain,
> > uint8_t bus,
> > return -1;
> >
> > memset(dev, 0, sizeof(*dev));
> > - dev->addr.domain = domain;
> > - dev->addr.bus = bus;
> > - dev->addr.devid = devid;
> > - dev->addr.function = function;
> > + dev->addr = *addr;
> >
> > /* get vendor id */
> > snprintf(filename, sizeof(filename), "%s/vendor", dirname);
> > @@ -429,16 +425,14 @@ pci_update_device(const struct rte_pci_addr *addr)
> > pci_get_sysfs_path(), addr->domain, addr->bus, addr-
> > >devid,
> > addr->function);
> >
> > - return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
> > - addr->function);
> > + return pci_scan_one(filename, addr);
> > }
> >
> > /*
> > * split up a pci address into its constituent parts.
> > */
> > static int
> > -parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
> > - uint8_t *bus, uint8_t *devid, uint8_t *function)
> > +parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr
> > *addr)
> > {
> > /* first split on ':' */
> > union splitaddr {
> > @@ -466,10 +460,10 @@ parse_pci_addr_format(const char *buf, int bufsize,
> > uint16_t *domain,
> >
> > /* now convert to int values */
> > errno = 0;
> > - *domain = (uint16_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);
> > + addr->domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
> > + addr->bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
> > + addr->devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
> > + addr->function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
> > if (errno != 0)
> > goto error;
> >
> > @@ -490,8 +484,7 @@ rte_eal_pci_scan(void)
> > struct dirent *e;
> > DIR *dir;
> > char dirname[PATH_MAX];
> > - uint16_t domain;
> > - uint8_t bus, devid, function;
> > + struct rte_pci_addr addr;
> >
> > dir = opendir(pci_get_sysfs_path());
> > if (dir == NULL) {
> > @@ -500,20 +493,21 @@ rte_eal_pci_scan(void)
> > return -1;
> > }
> >
> > +
> > while ((e = readdir(dir)) != NULL) {
> > if (e->d_name[0] == '.')
> > continue;
> >
> > - if (parse_pci_addr_format(e->d_name, sizeof(e->d_name),
> > &domain,
> > - &bus, &devid, &function) != 0)
> > + if (parse_pci_addr_format(e->d_name, sizeof(e->d_name),
> > &addr) != 0)
> > continue;
> >
> > snprintf(dirname, sizeof(dirname), "%s/%s",
> > pci_get_sysfs_path(), e->d_name);
> > - if (pci_scan_one(dirname, domain, bus, devid, function) <
> > 0)
> > + if (pci_scan_one(dirname, &addr) < 0)
> > goto error;
> > }
> > closedir(dir);
> > +
> > return 0;
> >
> > error:
> >
>
> Do you mind if I use this patch directly as part of my patchset for bus
> Model? I was doing a similar change to make the pci_scan_one simpler
> (and pass along a new argument)..
Sure, go ahead.
>
> -
> Shreyansh
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers Ben Walker
@ 2016-12-03 6:55 ` Shreyansh Jain
0 siblings, 0 replies; 34+ messages in thread
From: Shreyansh Jain @ 2016-12-03 6:55 UTC (permalink / raw)
To: Ben Walker; +Cc: dev
On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> There are now two functions - rte_eal_pci_attach_driver and
> rte_eal_pci_detach_driver - that dynamically attempt to attach
> and detach drivers from PCI devices. These only control
> whether a registered PCI driver is loaded or not - they are
> independent of whether the PCI device exists on the system.
>
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> ---
> lib/librte_eal/common/eal_common_dev.c | 4 +-
> lib/librte_eal/common/eal_common_pci.c | 109 +++++++++-----------------------
> lib/librte_eal/common/include/rte_pci.h | 22 ++++---
> 3 files changed, 43 insertions(+), 92 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
> index 4f3b493..1c6834e 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -114,7 +114,7 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
> }
>
> if (eal_parse_pci_DomBDF(name, &addr) == 0) {
IMO, ideally all the PCI specific code should be within eal_common_pci.
For example, the above function (eal_common_pci_DomBDF) shouldn't be
part of the generic fn rte_eal_dev_attach. Somehow this should be
hidden/closed with PCI specific files.
In the eal_common_dev, rte_eal_dev_attach and rte_eal_dev_detach are
directly referring to PCI as devices.
isn't it?
> - if (rte_eal_pci_probe_one(&addr) < 0)
> + if (rte_eal_pci_attach_driver(&addr) < 0)
> goto err;
>
> } else {
> @@ -139,7 +139,7 @@ int rte_eal_dev_detach(const char *name)
> }
>
> if (eal_parse_pci_DomBDF(name, &addr) == 0) {
> - if (rte_eal_pci_detach(&addr) < 0)
> + if (rte_eal_pci_detach_driver(&addr) < 0)
> goto err;
> } else {
> if (rte_eal_vdev_uninit(name))
> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
> index d50a534..67c6ce6 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -228,59 +228,36 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
> return 1;
> }
>
> -/*
> - * If vendor/device ID match, call the remove() function of the
> - * driver.
> - */
> static int
> -rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
> - struct rte_pci_device *dev)
> +rte_eal_pci_remove_driver(struct rte_pci_device *dev)
> {
> - const struct rte_pci_id *id_table;
> + struct rte_pci_driver *driver;
> + struct rte_pci_addr *loc;
>
> - if ((dr == NULL) || (dev == NULL))
> - return -EINVAL;
> + loc = &dev->addr;
> + driver = dev->driver;
>
> - for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
> -
> - /* check if device's identifiers match the driver's ones */
> - if (id_table->vendor_id != dev->id.vendor_id &&
> - id_table->vendor_id != PCI_ANY_ID)
> - continue;
> - if (id_table->device_id != dev->id.device_id &&
> - id_table->device_id != PCI_ANY_ID)
> - continue;
> - if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
> - id_table->subsystem_vendor_id != PCI_ANY_ID)
> - continue;
> - if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
> - id_table->subsystem_device_id != PCI_ANY_ID)
> - continue;
> -
> - struct rte_pci_addr *loc = &dev->addr;
> + if (driver == NULL)
> + return 0;
>
> - RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> - loc->domain, loc->bus, loc->devid,
> - loc->function, dev->device.numa_node);
> + RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
> + loc->domain, loc->bus, loc->devid,
> + loc->function, dev->device.numa_node);
>
> - RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
> - dev->id.device_id, dr->driver.name);
> + RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
> + dev->id.device_id, driver->driver.name);
>
> - if (dr->remove && (dr->remove(dev) < 0))
> - return -1; /* negative value is an error */
> + if (driver->remove && (driver->remove(dev) < 0))
> + return -1; /* negative value is an error */
>
> - /* clear driver structure */
> - dev->driver = NULL;
> + /* clear driver structure */
> + dev->driver = NULL;
>
> - if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
> - /* unmap resources for devices that use igb_uio */
> - rte_eal_pci_unmap_device(dev);
> + if (driver->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
> + /* unmap resources for devices that use igb_uio */
> + rte_eal_pci_unmap_device(dev);
>
> - return 0;
> - }
> -
> - /* return positive value if driver doesn't support this device */
> - return 1;
> + return 0;
> }
>
> /*
> @@ -315,38 +292,11 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
> }
>
> /*
> - * If vendor/device ID match, call the remove() function of all
> - * registered driver for the given device. Return -1 if initialization
> - * failed, return 1 if no driver is found for this device.
> - */
> -static int
> -pci_detach_all_drivers(struct rte_pci_device *dev)
> -{
> - struct rte_pci_driver *dr = NULL;
> - int rc = 0;
> -
> - if (dev == NULL)
> - return -1;
> -
> - TAILQ_FOREACH(dr, &pci_driver_list, next) {
> - rc = rte_eal_pci_detach_dev(dr, dev);
> - if (rc < 0)
> - /* negative value is an error */
> - return -1;
> - if (rc > 0)
> - /* positive value means driver doesn't support it */
> - continue;
> - return 0;
> - }
> - return 1;
> -}
> -
> -/*
> * Find the pci device specified by pci address, then invoke probe function of
> * the driver of the devive.
> */
> int
> -rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
> +rte_eal_pci_attach_driver(const struct rte_pci_addr *addr)
> {
> struct rte_pci_device *dev = NULL;
> int ret = 0;
> @@ -382,10 +332,9 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
> * Detach device specified by its pci address.
> */
> int
> -rte_eal_pci_detach(const struct rte_pci_addr *addr)
> +rte_eal_pci_detach_driver(const struct rte_pci_addr *addr)
> {
> struct rte_pci_device *dev = NULL;
> - int ret = 0;
>
> if (addr == NULL)
> return -1;
> @@ -394,17 +343,17 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
> if (rte_eal_compare_pci_addr(&dev->addr, addr))
> continue;
>
> - ret = pci_detach_all_drivers(dev);
> - if (ret < 0)
> - goto err_return;
> + if (dev->driver == NULL) {
> + /* The device at that address does not have a driver loaded */
> + return 0;
> + }
> +
> + if (rte_eal_pci_remove_driver(dev) < 0)
> + return -1;
>
> - TAILQ_REMOVE(&pci_device_list, dev, next);
> - free(dev);
> return 0;
> }
> - return -1;
>
> -err_return:
> RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT
> " cannot be used\n", dev->addr.domain, dev->addr.bus,
> dev->addr.devid, dev->addr.function);
> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
> index 2154a54..e304853 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -459,11 +459,14 @@ void *pci_map_resource(void *requested_addr, int fd, off_t offset,
> void pci_unmap_resource(void *requested_addr, size_t size);
>
> /**
> - * Probe the single PCI device.
> + * Attempt to load the registered driver for the PCI device at addr.
> *
> - * Scan the content of the PCI bus, and find the pci device specified by pci
> - * address, then call the probe() function for registered driver that has a
> - * matching entry in its id_table for discovered device.
> + * Find the pci device specified by addr, then call the probe() function
> + * for each registered driver that has a matching entry in its id_table until
> + * the correct driver is found.
> + *
> + * If the PCI address is known, this is considerably more efficient than
> + * calling rte_eal_pci_probe.
> *
> * @param addr
> * The PCI Bus-Device-Function address to probe.
> @@ -471,14 +474,13 @@ void pci_unmap_resource(void *requested_addr, size_t size);
> * - 0 on success.
> * - Negative on error.
> */
> -int rte_eal_pci_probe_one(const struct rte_pci_addr *addr);
> +int rte_eal_pci_attach_driver(const struct rte_pci_addr *addr);
>
> /**
> - * Close the single PCI device.
> + * Unload the driver for the PCI device at addr.
> *
> - * Scan the content of the PCI bus, and find the pci device specified by pci
> - * address, then call the remove() function for registered driver that has a
> - * matching entry in its id_table for discovered device.
> + * Find the pci device specified by addr, then call the remove() function
> + * for the currently loaded driver.
> *
> * @param addr
> * The PCI Bus-Device-Function address to close.
> @@ -486,7 +488,7 @@ int rte_eal_pci_probe_one(const struct rte_pci_addr *addr);
> * - 0 on success.
> * - Negative on error.
> */
> -int rte_eal_pci_detach(const struct rte_pci_addr *addr);
> +int rte_eal_pci_detach_driver(const struct rte_pci_addr *addr);
>
> /**
> * Dump the content of the PCI bus.
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources.
2016-11-25 9:21 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Shreyansh Jain
@ 2016-12-21 16:19 ` Thomas Monjalon
2017-01-04 17:39 ` Thomas Monjalon
0 siblings, 1 reply; 34+ messages in thread
From: Thomas Monjalon @ 2016-12-21 16:19 UTC (permalink / raw)
To: Shreyansh Jain, Ben Walker; +Cc: dev
2016-11-25 14:51, Shreyansh Jain:
> On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> > If resources were mapped prior to probe, unmap them
> > if probe fails.
> >
> > This does not handle the case where the kernel driver was
> > forcibly unbound prior to probe.
> >
> > Signed-off-by: Ben Walker <benjamin.walker@intel.com>
>
> Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
The 3 first patches of this series seems good and easy to agree,
except some minor nits.
Please Ben or Shreyansh, could you respin only them to help make
progress before applying a PCI/bus rework from Shreyansh?
As soon as one of you shoot a partial v3, I'll apply them.
Thanks
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources.
2016-12-21 16:19 ` Thomas Monjalon
@ 2017-01-04 17:39 ` Thomas Monjalon
2017-01-09 17:12 ` Thomas Monjalon
0 siblings, 1 reply; 34+ messages in thread
From: Thomas Monjalon @ 2017-01-04 17:39 UTC (permalink / raw)
To: Ben Walker; +Cc: Shreyansh Jain, dev
2016-12-21 17:19, Thomas Monjalon:
> 2016-11-25 14:51, Shreyansh Jain:
> > On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> > > If resources were mapped prior to probe, unmap them
> > > if probe fails.
> > >
> > > This does not handle the case where the kernel driver was
> > > forcibly unbound prior to probe.
> > >
> > > Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> >
> > Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>
> The 3 first patches of this series seems good and easy to agree,
> except some minor nits.
>
> Please Ben or Shreyansh, could you respin only them to help make
> progress before applying a PCI/bus rework from Shreyansh?
> As soon as one of you shoot a partial v3, I'll apply them.
> Thanks
Please Ben, do you want to make a partial v3?
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources.
2017-01-04 17:39 ` Thomas Monjalon
@ 2017-01-09 17:12 ` Thomas Monjalon
0 siblings, 0 replies; 34+ messages in thread
From: Thomas Monjalon @ 2017-01-09 17:12 UTC (permalink / raw)
To: Ben Walker; +Cc: Shreyansh Jain, dev
2017-01-04 18:39, Thomas Monjalon:
> 2016-12-21 17:19, Thomas Monjalon:
> > 2016-11-25 14:51, Shreyansh Jain:
> > > On Thursday 24 November 2016 01:37 AM, Ben Walker wrote:
> > > > If resources were mapped prior to probe, unmap them
> > > > if probe fails.
> > > >
> > > > This does not handle the case where the kernel driver was
> > > > forcibly unbound prior to probe.
> > > >
> > > > Signed-off-by: Ben Walker <benjamin.walker@intel.com>
> > >
> > > Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> >
> > The 3 first patches of this series seems good and easy to agree,
> > except some minor nits.
> >
> > Please Ben or Shreyansh, could you respin only them to help make
> > progress before applying a PCI/bus rework from Shreyansh?
> > As soon as one of you shoot a partial v3, I'll apply them.
> > Thanks
>
> Please Ben, do you want to make a partial v3?
Please, would you mind to re-send the first 3 patches as v3?
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v3 1/3] pci: If a driver's probe function fails, unmap resources.
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
` (6 preceding siblings ...)
2016-11-25 9:21 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Shreyansh Jain
@ 2017-01-11 17:10 ` Ben Walker
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 2/3] pci: Separate detaching ethernet ports from PCI devices Ben Walker
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 3/3] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
7 siblings, 2 replies; 34+ messages in thread
From: Ben Walker @ 2017-01-11 17:10 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
If resources were mapped prior to probe, unmap them
if probe fails.
This does not handle the case where the kernel driver was
forcibly unbound prior to probe.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/common/eal_common_pci.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 33485bc..72547bd 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -210,8 +210,11 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
/* call the driver probe() function */
ret = dr->probe(dr, dev);
- if (ret)
+ if (ret) {
dev->driver = NULL;
+ if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+ rte_eal_pci_unmap_device(dev);
+ }
return ret;
}
--
2.9.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v3 2/3] pci: Separate detaching ethernet ports from PCI devices
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
@ 2017-01-11 17:10 ` Ben Walker
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 3/3] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
1 sibling, 0 replies; 34+ messages in thread
From: Ben Walker @ 2017-01-11 17:10 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
Attaching and detaching ethernet ports from an application
is not the same thing as physically removing a PCI device,
so clarify the flags indicating support. All PCI devices
are assumed to be physically removable, so no flag is
necessary in the PCI layer.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
doc/guides/prog_guide/port_hotplug_framework.rst | 2 +-
drivers/net/bnxt/bnxt_ethdev.c | 3 ++-
drivers/net/e1000/em_ethdev.c | 4 ++--
drivers/net/e1000/igb_ethdev.c | 7 ++++---
drivers/net/fm10k/fm10k_ethdev.c | 4 ++--
drivers/net/i40e/i40e_ethdev.c | 4 ++--
drivers/net/i40e/i40e_ethdev_vf.c | 3 ++-
drivers/net/ixgbe/ixgbe_ethdev.c | 7 ++++---
drivers/net/nfp/nfp_net.c | 4 ++--
drivers/net/virtio/virtio_ethdev.c | 6 ++++--
drivers/net/vmxnet3/vmxnet3_ethdev.c | 3 ++-
drivers/net/xenvirt/rte_eth_xenvirt.c | 2 +-
lib/librte_eal/common/include/rte_pci.h | 2 --
lib/librte_ether/rte_ethdev.c | 2 --
14 files changed, 28 insertions(+), 25 deletions(-)
diff --git a/doc/guides/prog_guide/port_hotplug_framework.rst b/doc/guides/prog_guide/port_hotplug_framework.rst
index 6e4436e..8c49319 100644
--- a/doc/guides/prog_guide/port_hotplug_framework.rst
+++ b/doc/guides/prog_guide/port_hotplug_framework.rst
@@ -106,5 +106,5 @@ Limitations
* Not all PMDs support detaching feature.
To know whether a PMD can support detaching, search for the
- "RTE_PCI_DRV_DETACHABLE" flag in PMD implementation. If the flag is
+ "RTE_ETH_DEV_DETACHABLE" flag in rte_eth_dev::data::dev_flags. If the flag is
defined in the PMD, detaching is supported.
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 7518b6b..eead73b 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1057,6 +1057,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
RTE_LOG(INFO, PMD, "%s", bnxt_version);
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
bp = eth_dev->data->dev_private;
if (bnxt_vf_pciid(pci_dev->id.device_id))
@@ -1168,7 +1169,7 @@ static struct eth_driver bnxt_rte_pmd = {
.pci_drv = {
.id_table = bnxt_pci_id_map,
.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
- RTE_PCI_DRV_DETACHABLE | RTE_PCI_DRV_INTR_LSC,
+ RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove
},
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 5f6e66d..17e30db 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -313,6 +313,7 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
hw->device_id = pci_dev->id.device_id;
@@ -392,8 +393,7 @@ eth_em_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_em_pmd = {
.pci_drv = {
.id_table = pci_id_em_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 2bb57f5..2301235 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -774,6 +774,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->hw_addr= (void *)pci_dev->mem_resource[0].addr;
@@ -982,6 +983,7 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
pci_dev = E1000_DEV_TO_PCI(eth_dev);
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->device_id = pci_dev->id.device_id;
hw->vendor_id = pci_dev->id.vendor_id;
@@ -1085,8 +1087,7 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_igb_pmd = {
.pci_drv = {
.id_table = pci_id_igb_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
@@ -1101,7 +1102,7 @@ static struct eth_driver rte_igb_pmd = {
static struct eth_driver rte_igbvf_pmd = {
.pci_drv = {
.id_table = pci_id_igbvf_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index d8353e9..dd021e4 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -2858,6 +2858,7 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
return 0;
rte_eth_copy_pci_info(dev, pdev);
+ dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
memset(macvlan, 0, sizeof(*macvlan));
@@ -3080,8 +3081,7 @@ static const struct rte_pci_id pci_id_fm10k_map[] = {
static struct eth_driver rte_pmd_fm10k = {
.pci_drv = {
.id_table = pci_id_fm10k_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 0eb4c99..1687144 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -671,8 +671,7 @@ static const struct rte_i40e_xstats_name_off rte_i40e_txq_prio_strings[] = {
static struct eth_driver rte_i40e_pmd = {
.pci_drv = {
.id_table = pci_id_i40e_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
@@ -959,6 +958,7 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
intr_handle = &pci_dev->intr_handle;
rte_eth_copy_pci_info(dev, pci_dev);
+ dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
pf->adapter = I40E_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
pf->adapter->eth_dev = dev;
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index 0dc0af5..b32f9e7 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1460,6 +1460,7 @@ i40evf_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->vendor_id = pci_dev->id.vendor_id;
hw->device_id = pci_dev->id.device_id;
@@ -1529,7 +1530,7 @@ i40evf_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_i40evf_pmd = {
.pci_drv = {
.id_table = pci_id_i40evf_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 060772d..b7415e8 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1134,6 +1134,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
/* Vendor and Device ID need to be set before init of shared code */
hw->device_id = pci_dev->id.device_id;
@@ -1424,6 +1425,7 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
}
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->device_id = pci_dev->id.device_id;
hw->vendor_id = pci_dev->id.vendor_id;
@@ -1566,8 +1568,7 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_ixgbe_pmd = {
.pci_drv = {
.id_table = pci_id_ixgbe_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
@@ -1582,7 +1583,7 @@ static struct eth_driver rte_ixgbe_pmd = {
static struct eth_driver rte_ixgbevf_pmd = {
.pci_drv = {
.id_table = pci_id_ixgbevf_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index e85315f..88169a7 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -2338,6 +2338,7 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
pci_dev = RTE_DEV_TO_PCI(eth_dev->device);
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
hw->device_id = pci_dev->id.device_id;
hw->vendor_id = pci_dev->id.vendor_id;
@@ -2475,8 +2476,7 @@ static struct rte_pci_id pci_id_nfp_net_map[] = {
static struct eth_driver rte_nfp_net_pmd = {
.pci_drv = {
.id_table = pci_id_nfp_net_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
- RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 54ea7d7..d1747f6 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1212,8 +1212,10 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
else
eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
- if (pci_dev)
+ if (pci_dev) {
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
+ }
rx_func_get(eth_dev);
@@ -1383,7 +1385,7 @@ static struct eth_driver rte_virtio_pmd = {
.name = "net_virtio",
},
.id_table = pci_id_virtio_map,
- .drv_flags = RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = 0,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 54533ca..52e3644 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -250,6 +250,7 @@ eth_vmxnet3_dev_init(struct rte_eth_dev *eth_dev)
return 0;
rte_eth_copy_pci_info(eth_dev, pci_dev);
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
/* Vendor and Device ID need to be set before init of shared code */
hw->device_id = pci_dev->id.device_id;
@@ -340,7 +341,7 @@ eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev)
static struct eth_driver rte_vmxnet3_pmd = {
.pci_drv = {
.id_table = pci_id_vmxnet3_map,
- .drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
+ .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
.probe = rte_eth_dev_pci_probe,
.remove = rte_eth_dev_pci_remove,
},
diff --git a/drivers/net/xenvirt/rte_eth_xenvirt.c b/drivers/net/xenvirt/rte_eth_xenvirt.c
index 609824b..e7640ab 100644
--- a/drivers/net/xenvirt/rte_eth_xenvirt.c
+++ b/drivers/net/xenvirt/rte_eth_xenvirt.c
@@ -672,7 +672,7 @@ eth_dev_xenvirt_create(const char *name, const char *params,
eth_dev->data = data;
eth_dev->dev_ops = &ops;
- eth_dev->data->dev_flags = RTE_PCI_DRV_DETACHABLE;
+ eth_dev->data->dev_flags = RTE_ETH_DEV_DETACHABLE;
eth_dev->data->kdrv = RTE_KDRV_NONE;
eth_dev->data->drv_name = drivername;
eth_dev->driver = NULL;
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 87cad59..8557e47 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -216,8 +216,6 @@ struct rte_pci_driver {
#define RTE_PCI_DRV_NEED_MAPPING 0x0001
/** Device driver supports link state interrupt */
#define RTE_PCI_DRV_INTR_LSC 0x0008
-/** Device driver supports detaching capability */
-#define RTE_PCI_DRV_DETACHABLE 0x0010
/**
* A structure describing a PCI mapping.
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 9dea1f1..222e267 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -3272,8 +3272,6 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev, struct rte_pci_device *pci_de
eth_dev->data->dev_flags = 0;
if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC;
- if (pci_dev->driver->drv_flags & RTE_PCI_DRV_DETACHABLE)
- eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE;
eth_dev->data->kdrv = pci_dev->kdrv;
eth_dev->data->numa_node = pci_dev->device.numa_node;
--
2.9.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v3 3/3] pci: Pass rte_pci_addr to functions instead of separate args
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 2/3] pci: Separate detaching ethernet ports from PCI devices Ben Walker
@ 2017-01-11 17:10 ` Ben Walker
2017-01-12 14:58 ` Thomas Monjalon
1 sibling, 1 reply; 34+ messages in thread
From: Ben Walker @ 2017-01-11 17:10 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
Instead of passing domain, bus, devid, func, just pass
an rte_pci_addr.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++-------------------
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 4350134..2933f00 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -228,8 +228,7 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
/* Scan one pci sysfs entry, and fill the devices list from it. */
static int
-pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
- uint8_t devid, uint8_t function)
+pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
{
char filename[PATH_MAX];
unsigned long tmp;
@@ -242,10 +241,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
return -1;
memset(dev, 0, sizeof(*dev));
- dev->addr.domain = domain;
- dev->addr.bus = bus;
- dev->addr.devid = devid;
- dev->addr.function = function;
+ dev->addr = *addr;
/* get vendor id */
snprintf(filename, sizeof(filename), "%s/vendor", dirname);
@@ -390,16 +386,14 @@ pci_update_device(const struct rte_pci_addr *addr)
pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
addr->function);
- return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
- addr->function);
+ return pci_scan_one(filename, addr);
}
/*
* split up a pci address into its constituent parts.
*/
static int
-parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
- uint8_t *bus, uint8_t *devid, uint8_t *function)
+parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
{
/* first split on ':' */
union splitaddr {
@@ -427,10 +421,10 @@ parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
/* now convert to int values */
errno = 0;
- *domain = (uint16_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);
+ addr->domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
+ addr->bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
+ addr->devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
+ addr->function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
if (errno != 0)
goto error;
@@ -451,8 +445,7 @@ rte_eal_pci_scan(void)
struct dirent *e;
DIR *dir;
char dirname[PATH_MAX];
- uint16_t domain;
- uint8_t bus, devid, function;
+ struct rte_pci_addr addr;
dir = opendir(pci_get_sysfs_path());
if (dir == NULL) {
@@ -461,20 +454,21 @@ rte_eal_pci_scan(void)
return -1;
}
+
while ((e = readdir(dir)) != NULL) {
if (e->d_name[0] == '.')
continue;
- if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &domain,
- &bus, &devid, &function) != 0)
+ if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &addr) != 0)
continue;
snprintf(dirname, sizeof(dirname), "%s/%s",
pci_get_sysfs_path(), e->d_name);
- if (pci_scan_one(dirname, domain, bus, devid, function) < 0)
+ if (pci_scan_one(dirname, &addr) < 0)
goto error;
}
closedir(dir);
+
return 0;
error:
--
2.9.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v3 3/3] pci: Pass rte_pci_addr to functions instead of separate args
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 3/3] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
@ 2017-01-12 14:58 ` Thomas Monjalon
0 siblings, 0 replies; 34+ messages in thread
From: Thomas Monjalon @ 2017-01-12 14:58 UTC (permalink / raw)
To: Ben Walker; +Cc: dev
2017-01-11 10:10, Ben Walker:
> Instead of passing domain, bus, devid, func, just pass
> an rte_pci_addr.
>
> Signed-off-by: Ben Walker <benjamin.walker@intel.com>
Added v2 acks from Shreyansh and removed unnecessary empty lines as Shreyansh commented, and
Series applied, thanks
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v3 1/3] pci: rte_eal_pci_scan now handles removal of PCI devices
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices Ben Walker
2016-11-25 10:44 ` Shreyansh Jain
@ 2017-02-09 16:59 ` Ben Walker
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 2/3] pci: Move driver registration above pci scan Ben Walker
` (2 more replies)
1 sibling, 3 replies; 34+ messages in thread
From: Ben Walker @ 2017-02-09 16:59 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
rte_eal_pci_scan can be called repeatedly to re-scan the PCI
bus. If a device was removed from the system, the associated
driver will automatically be unloaded.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
Only code style changes compared to last submission. It is also rebased
onto the latest from master, which had no conflicts.
lib/librte_eal/linuxapp/eal/eal_pci.c | 70 +++++++++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index e2fc219..2eed405 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -446,7 +446,77 @@ rte_eal_pci_scan(void)
DIR *dir;
char dirname[PATH_MAX];
struct rte_pci_addr addr;
+ struct rte_pci_device *dev, *tmp;
+
+ /* Search the device list for devices that are no longer present
+ * on the system and remove them.
+ */
+ TAILQ_FOREACH_SAFE(dev, &pci_device_list, next, tmp) {
+ int found = 0;
+
+ dir = opendir(pci_get_sysfs_path());
+ if (dir == NULL) {
+ RTE_LOG(ERR, EAL, "%s(): opendir failed: %s\n",
+ __func__, strerror(errno));
+ return -1;
+ }
+
+ while ((e = readdir(dir)) != NULL) {
+ if (e->d_name[0] == '.')
+ continue;
+
+ if (parse_pci_addr_format(e->d_name, sizeof(e->d_name),
+ &addr) != 0)
+ continue;
+
+ if (rte_eal_compare_pci_addr(&addr, &dev->addr) == 0) {
+ found = 1;
+ break;
+ }
+ }
+
+ if (!found) {
+
+ RTE_LOG(DEBUG, EAL,
+ "PCI device "PCI_PRI_FMT" was removed.\n",
+ addr.domain, addr.bus, addr.devid,
+ addr.function);
+
+ if (dev->driver) {
+ /* A driver was loaded against this device.
+ * Unload it.
+ */
+ RTE_LOG(DEBUG, EAL,
+ " Unload driver: %x:%x %s\n",
+ dev->id.vendor_id, dev->id.device_id,
+ dev->driver->driver.name);
+
+ if (dev->driver->remove) {
+ /* It doesn't matter what remove
+ * returns - we're removing the
+ * device either way.
+ */
+ dev->driver->remove(dev);
+ }
+
+ /* clear driver structure */
+ dev->driver = NULL;
+
+ if (dev->driver->drv_flags &
+ RTE_PCI_DRV_NEED_MAPPING)
+ rte_eal_pci_unmap_device(dev);
+ }
+
+ TAILQ_REMOVE(&pci_device_list, dev, next);
+ free(dev);
+ }
+
+ closedir(dir);
+ }
+ /* Search sysfs for all PCI devices and add them to the
+ * device list
+ */
dir = opendir(pci_get_sysfs_path());
if (dir == NULL) {
RTE_LOG(ERR, EAL, "%s(): opendir failed: %s\n",
--
2.9.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v3 2/3] pci: Move driver registration above pci scan
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
@ 2017-02-09 16:59 ` Ben Walker
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 3/3] pci: Clarify interfaces for dynamic attach/detach of drivers Ben Walker
2019-01-23 16:19 ` [dpdk-dev] [PATCH v3 1/3] pci: rte_eal_pci_scan now handles removal of PCI devices Ferruh Yigit
2 siblings, 0 replies; 34+ messages in thread
From: Ben Walker @ 2017-02-09 16:59 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
The user needs to register drivers before scanning, so
it makes the most sense to put the registration
functions above the scan function in the header file.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
Only rebased onto latest master with the rest of this series.
No changes.
lib/librte_eal/common/include/rte_pci.h | 56 ++++++++++++++++-----------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 8557e47..5640896 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -362,6 +362,34 @@ rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
}
/**
+ * Register a PCI driver.
+ *
+ * @param driver
+ * A pointer to a rte_pci_driver structure describing the driver
+ * to be registered.
+ */
+void rte_eal_pci_register(struct rte_pci_driver *driver);
+
+/** Statically register a PCI driver at start up */
+#define RTE_PMD_REGISTER_PCI(nm, pci_drv) \
+RTE_INIT(pciinitfn_ ##nm); \
+static void pciinitfn_ ##nm(void) \
+{\
+ (pci_drv).driver.name = RTE_STR(nm);\
+ rte_eal_pci_register(&pci_drv); \
+} \
+RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
+
+/**
+ * Unregister a PCI driver.
+ *
+ * @param driver
+ * A pointer to a rte_pci_driver structure describing the driver
+ * to be unregistered.
+ */
+void rte_eal_pci_unregister(struct rte_pci_driver *driver);
+
+/**
* Scan the content of the PCI bus, and the devices in the devices
* list
*
@@ -480,34 +508,6 @@ int rte_eal_pci_detach(const struct rte_pci_addr *addr);
void rte_eal_pci_dump(FILE *f);
/**
- * Register a PCI driver.
- *
- * @param driver
- * A pointer to a rte_pci_driver structure describing the driver
- * to be registered.
- */
-void rte_eal_pci_register(struct rte_pci_driver *driver);
-
-/** Helper for PCI device registration from driver (eth, crypto) instance */
-#define RTE_PMD_REGISTER_PCI(nm, pci_drv) \
-RTE_INIT(pciinitfn_ ##nm); \
-static void pciinitfn_ ##nm(void) \
-{\
- (pci_drv).driver.name = RTE_STR(nm);\
- rte_eal_pci_register(&pci_drv); \
-} \
-RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
-
-/**
- * Unregister a PCI driver.
- *
- * @param driver
- * A pointer to a rte_pci_driver structure describing the driver
- * to be unregistered.
- */
-void rte_eal_pci_unregister(struct rte_pci_driver *driver);
-
-/**
* Read PCI config space.
*
* @param device
--
2.9.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* [dpdk-dev] [PATCH v3 3/3] pci: Clarify interfaces for dynamic attach/detach of drivers
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 2/3] pci: Move driver registration above pci scan Ben Walker
@ 2017-02-09 16:59 ` Ben Walker
2019-01-23 16:19 ` [dpdk-dev] [PATCH v3 1/3] pci: rte_eal_pci_scan now handles removal of PCI devices Ferruh Yigit
2 siblings, 0 replies; 34+ messages in thread
From: Ben Walker @ 2017-02-09 16:59 UTC (permalink / raw)
To: dev; +Cc: Ben Walker
There are now two functions - rte_eal_pci_attach_driver and
rte_eal_pci_detach_driver - that dynamically attempt to attach
and detach drivers from PCI devices. These only control
whether a registered PCI driver is loaded or not - they are
independent of whether the PCI device exists on the system.
Signed-off-by: Ben Walker <benjamin.walker@intel.com>
---
This patch was previously the fourth in the series, behind
another more controversial change. That change has been
removed and this patch was reordered to be independent
of those changes.
lib/librte_eal/common/eal_common_dev.c | 4 +-
lib/librte_eal/common/eal_common_pci.c | 109 +++++++++-----------------------
lib/librte_eal/common/include/rte_pci.h | 22 ++++---
3 files changed, 43 insertions(+), 92 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 4f3b493..1c6834e 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -114,7 +114,7 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
}
if (eal_parse_pci_DomBDF(name, &addr) == 0) {
- if (rte_eal_pci_probe_one(&addr) < 0)
+ if (rte_eal_pci_attach_driver(&addr) < 0)
goto err;
} else {
@@ -139,7 +139,7 @@ int rte_eal_dev_detach(const char *name)
}
if (eal_parse_pci_DomBDF(name, &addr) == 0) {
- if (rte_eal_pci_detach(&addr) < 0)
+ if (rte_eal_pci_detach_driver(&addr) < 0)
goto err;
} else {
if (rte_eal_vdev_uninit(name))
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 72547bd..00668b6 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -222,59 +222,36 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
return 1;
}
-/*
- * If vendor/device ID match, call the remove() function of the
- * driver.
- */
static int
-rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
- struct rte_pci_device *dev)
+rte_eal_pci_remove_driver(struct rte_pci_device *dev)
{
- const struct rte_pci_id *id_table;
+ struct rte_pci_driver *driver;
+ struct rte_pci_addr *loc;
- if ((dr == NULL) || (dev == NULL))
- return -EINVAL;
+ loc = &dev->addr;
+ driver = dev->driver;
- for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
-
- /* check if device's identifiers match the driver's ones */
- if (id_table->vendor_id != dev->id.vendor_id &&
- id_table->vendor_id != PCI_ANY_ID)
- continue;
- if (id_table->device_id != dev->id.device_id &&
- id_table->device_id != PCI_ANY_ID)
- continue;
- if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
- id_table->subsystem_vendor_id != PCI_ANY_ID)
- continue;
- if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
- id_table->subsystem_device_id != PCI_ANY_ID)
- continue;
-
- struct rte_pci_addr *loc = &dev->addr;
+ if (driver == NULL)
+ return 0;
- RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
- loc->domain, loc->bus, loc->devid,
- loc->function, dev->device.numa_node);
+ RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+ loc->domain, loc->bus, loc->devid,
+ loc->function, dev->device.numa_node);
- RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
- dev->id.device_id, dr->driver.name);
+ RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", dev->id.vendor_id,
+ dev->id.device_id, driver->driver.name);
- if (dr->remove && (dr->remove(dev) < 0))
- return -1; /* negative value is an error */
+ if (driver->remove && (driver->remove(dev) < 0))
+ return -1; /* negative value is an error */
- /* clear driver structure */
- dev->driver = NULL;
+ /* clear driver structure */
+ dev->driver = NULL;
- if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
- /* unmap resources for devices that use igb_uio */
- rte_eal_pci_unmap_device(dev);
+ if (driver->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+ /* unmap resources for devices that use igb_uio */
+ rte_eal_pci_unmap_device(dev);
- return 0;
- }
-
- /* return positive value if driver doesn't support this device */
- return 1;
+ return 0;
}
/*
@@ -309,38 +286,11 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
}
/*
- * If vendor/device ID match, call the remove() function of all
- * registered driver for the given device. Return -1 if initialization
- * failed, return 1 if no driver is found for this device.
- */
-static int
-pci_detach_all_drivers(struct rte_pci_device *dev)
-{
- struct rte_pci_driver *dr = NULL;
- int rc = 0;
-
- if (dev == NULL)
- return -1;
-
- TAILQ_FOREACH(dr, &pci_driver_list, next) {
- rc = rte_eal_pci_detach_dev(dr, dev);
- if (rc < 0)
- /* negative value is an error */
- return -1;
- if (rc > 0)
- /* positive value means driver doesn't support it */
- continue;
- return 0;
- }
- return 1;
-}
-
-/*
* Find the pci device specified by pci address, then invoke probe function of
* the driver of the devive.
*/
int
-rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
+rte_eal_pci_attach_driver(const struct rte_pci_addr *addr)
{
struct rte_pci_device *dev = NULL;
int ret = 0;
@@ -376,10 +326,9 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
* Detach device specified by its pci address.
*/
int
-rte_eal_pci_detach(const struct rte_pci_addr *addr)
+rte_eal_pci_detach_driver(const struct rte_pci_addr *addr)
{
struct rte_pci_device *dev = NULL;
- int ret = 0;
if (addr == NULL)
return -1;
@@ -388,17 +337,17 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
if (rte_eal_compare_pci_addr(&dev->addr, addr))
continue;
- ret = pci_detach_all_drivers(dev);
- if (ret < 0)
- goto err_return;
+ if (dev->driver == NULL) {
+ /* The device at that address does not have a driver loaded */
+ return 0;
+ }
+
+ if (rte_eal_pci_remove_driver(dev) < 0)
+ return -1;
- TAILQ_REMOVE(&pci_device_list, dev, next);
- free(dev);
return 0;
}
- return -1;
-err_return:
RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT
" cannot be used\n", dev->addr.domain, dev->addr.bus,
dev->addr.devid, dev->addr.function);
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 5640896..c285fd4 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -470,11 +470,14 @@ void *pci_map_resource(void *requested_addr, int fd, off_t offset,
void pci_unmap_resource(void *requested_addr, size_t size);
/**
- * Probe the single PCI device.
+ * Attempt to load the registered driver for the PCI device at addr.
*
- * Scan the content of the PCI bus, and find the pci device specified by pci
- * address, then call the probe() function for registered driver that has a
- * matching entry in its id_table for discovered device.
+ * Find the pci device specified by addr, then call the probe() function
+ * for each registered driver that has a matching entry in its id_table until
+ * the correct driver is found.
+ *
+ * If the PCI address is known, this is considerably more efficient than
+ * calling rte_eal_pci_probe.
*
* @param addr
* The PCI Bus-Device-Function address to probe.
@@ -482,14 +485,13 @@ void pci_unmap_resource(void *requested_addr, size_t size);
* - 0 on success.
* - Negative on error.
*/
-int rte_eal_pci_probe_one(const struct rte_pci_addr *addr);
+int rte_eal_pci_attach_driver(const struct rte_pci_addr *addr);
/**
- * Close the single PCI device.
+ * Unload the driver for the PCI device at addr.
*
- * Scan the content of the PCI bus, and find the pci device specified by pci
- * address, then call the remove() function for registered driver that has a
- * matching entry in its id_table for discovered device.
+ * Find the pci device specified by addr, then call the remove() function
+ * for the currently loaded driver.
*
* @param addr
* The PCI Bus-Device-Function address to close.
@@ -497,7 +499,7 @@ int rte_eal_pci_probe_one(const struct rte_pci_addr *addr);
* - 0 on success.
* - Negative on error.
*/
-int rte_eal_pci_detach(const struct rte_pci_addr *addr);
+int rte_eal_pci_detach_driver(const struct rte_pci_addr *addr);
/**
* Dump the content of the PCI bus.
--
2.9.3
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [dpdk-dev] [PATCH v3 1/3] pci: rte_eal_pci_scan now handles removal of PCI devices
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 2/3] pci: Move driver registration above pci scan Ben Walker
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 3/3] pci: Clarify interfaces for dynamic attach/detach of drivers Ben Walker
@ 2019-01-23 16:19 ` Ferruh Yigit
2 siblings, 0 replies; 34+ messages in thread
From: Ferruh Yigit @ 2019-01-23 16:19 UTC (permalink / raw)
To: Ben Walker; +Cc: dpdk-dev, Thomas Monjalon
On 2/9/2017 4:59 PM, benjamin.walker at intel.com (Ben Walker) wrote:
> rte_eal_pci_scan can be called repeatedly to re-scan the PCI
> bus. If a device was removed from the system, the associated
> driver will automatically be unloaded.
>
> Signed-off-by: Ben Walker <benjamin.walker at intel.com>
Hi Ben,
This patch is waiting for a long time for a comment. And eal code changed a lot
in last two years.
I am updating the patchset as rejected, if it is still relevant please send a
new version on top of latest repo.
Sorry for any inconvenience caused.
For reference patches:
https://patches.dpdk.org/patch/20328/
https://patches.dpdk.org/patch/20329/
https://patches.dpdk.org/patch/20330/
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2019-01-23 16:19 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-23 19:36 [dpdk-dev] Improved PCI hotplug support Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 2/7] pci: Separate detaching ethernet ports from PCI devices Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 3/7] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 5/7] pci: Move driver registration above pci scan Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe Ben Walker
2016-11-23 19:36 ` [dpdk-dev] [PATCH 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 2/7] pci: Separate detaching ethernet ports from PCI devices Ben Walker
2016-11-25 9:25 ` Shreyansh Jain
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
2016-11-25 10:33 ` Shreyansh Jain
2016-12-01 6:26 ` Shreyansh Jain
2016-12-02 16:16 ` Walker, Benjamin
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices Ben Walker
2016-11-25 10:44 ` Shreyansh Jain
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 2/3] pci: Move driver registration above pci scan Ben Walker
2017-02-09 16:59 ` [dpdk-dev] [PATCH v3 3/3] pci: Clarify interfaces for dynamic attach/detach of drivers Ben Walker
2019-01-23 16:19 ` [dpdk-dev] [PATCH v3 1/3] pci: rte_eal_pci_scan now handles removal of PCI devices Ferruh Yigit
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 5/7] pci: Move driver registration above pci scan Ben Walker
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe Ben Walker
2016-11-25 10:56 ` Shreyansh Jain
2016-11-23 20:07 ` [dpdk-dev] [PATCH v2 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers Ben Walker
2016-12-03 6:55 ` Shreyansh Jain
2016-11-25 9:21 ` [dpdk-dev] [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources Shreyansh Jain
2016-12-21 16:19 ` Thomas Monjalon
2017-01-04 17:39 ` Thomas Monjalon
2017-01-09 17:12 ` Thomas Monjalon
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 1/3] " Ben Walker
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 2/3] pci: Separate detaching ethernet ports from PCI devices Ben Walker
2017-01-11 17:10 ` [dpdk-dev] [PATCH v3 3/3] pci: Pass rte_pci_addr to functions instead of separate args Ben Walker
2017-01-12 14:58 ` Thomas Monjalon
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).