Improved PCI hotplug support
Subsystems (2)
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.
PATCH 1/7 pci: If a driver's probe function fails, unmap resources.
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 <[email protected]>
--- lib/librte_eal/common/eal_common_pci.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_pci.c +4 -1
@@ -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; }
Signature · 1 line
2.7.4
PATCH 2/7 pci: Separate detaching ethernet ports from PCI devices
In reply to Ben WalkerAttaching 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 <[email protected]>
--- 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(-)
doc/guides/prog_guide/port_hotplug_framework.rst +1 -1
@@ -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.
drivers/net/bnxt/bnxt_ethdev.c +2 -1
@@ -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 },
drivers/net/e1000/em_ethdev.c +2 -2
@@ -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, },
drivers/net/e1000/igb_ethdev.c +4 -3
@@ -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, },
drivers/net/fm10k/fm10k_ethdev.c +2 -2
@@ -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, },
drivers/net/i40e/i40e_ethdev.c +2 -2
@@ -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;
drivers/net/i40e/i40e_ethdev_vf.c +2 -1
@@ -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, },
drivers/net/ixgbe/ixgbe_ethdev.c +4 -3
@@ -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, },
drivers/net/nfp/nfp_net.c +2 -2
@@ -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, },
drivers/net/virtio/virtio_ethdev.c +2 -1
@@ -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, },
drivers/net/vmxnet3/vmxnet3_ethdev.c +2 -1
@@ -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, },
drivers/net/xenvirt/rte_eth_xenvirt.c +1 -1
@@ -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 ;
lib/librte_eal/common/include/rte_pci.h +0 -2
@@ -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.
lib/librte_ether/rte_ethdev.c +0 -2
@@ -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;
Signature · 1 line
2.7.4
PATCH 3/7 pci: Pass rte_pci_addr to functions instead of separate args
In reply to Ben WalkerInstead of passing domain, bus, devid, func, just pass an rte_pci_addr.
Signed-off-by Ben Walker <[email protected]>
--- lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-)
lib/librte_eal/linuxapp/eal/eal_pci.c +13 -19
@@ -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:
Signature · 1 line
2.7.4
PATCH 4/7 pci: rte_eal_pci_scan now handles removal of PCI devices
In reply to Ben Walkerrte_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 <[email protected]>
--- lib/librte_eal/linuxapp/eal/eal_pci.c | 58 +++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+)
lib/librte_eal/linuxapp/eal/eal_pci.c +58 -0
@@ -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",
Signature · 1 line
2.7.4
PATCH 5/7 pci: Move driver registration above pci scan
In reply to Ben WalkerThe 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 <[email protected]>
--- lib/librte_eal/common/include/rte_pci.h | 56 ++++++++++++++++----------------- 1 file changed, 28 insertions(+), 28 deletions(-)
lib/librte_eal/common/include/rte_pci.h +28 -28
@@ -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
Signature · 1 line
2.7.4
PATCH 6/7 pci: Combine rte_eal_pci_scan and rte_eal_pci_probe
In reply to Ben WalkerTwo 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 <[email protected]>
--- 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(-)
app/test/test_pci.c +1 -1
@@ -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);
lib/librte_eal/bsdapp/eal/eal.c +0 -3
@@ -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 )
lib/librte_eal/bsdapp/eal/eal_pci.c +1 -16
@@ -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 ; -}
lib/librte_eal/bsdapp/eal/rte_eal_version.map +0 -1
@@ -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;
lib/librte_eal/common/eal_common_pci.c +7 -0
@@ -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 ;
lib/librte_eal/common/eal_private.h +5 -9
@@ -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.
lib/librte_eal/common/include/rte_pci.h +5 -12
@@ -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.
lib/librte_eal/linuxapp/eal/eal.c +0 -3
@@ -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" );
lib/librte_eal/linuxapp/eal/eal_pci.c +1 -17
@@ -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 ; -}
lib/librte_eal/linuxapp/eal/rte_eal_version.map +0 -1
@@ -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;
Signature · 1 line
2.7.4
PATCH 7/7 pci: Clarify interfaces for dynamic attach/detach of drivers
In reply to Ben WalkerThere 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 <[email protected]>
--- 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(-)
lib/librte_eal/common/eal_common_dev.c +2 -2
@@ -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))
lib/librte_eal/common/eal_common_pci.c +31 -80
155 lines not on this page · raw message
lib/librte_eal/common/include/rte_pci.h +12 -10
@@ -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.
Signature · 1 line
2.7.4
PATCH v2 1/7 pci: If a driver's probe function fails, unmap resources.
In reply to Ben WalkerIf 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 <[email protected]>
--- lib/librte_eal/common/eal_common_pci.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_pci.c +4 -1
@@ -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; }
Signature · 1 line
2.7.4
PATCH v2 2/7 pci: Separate detaching ethernet ports from PCI devices
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 <[email protected]>
--- 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(-)
doc/guides/prog_guide/port_hotplug_framework.rst +1 -1
@@ -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.
drivers/net/bnxt/bnxt_ethdev.c +2 -1
@@ -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 },
drivers/net/e1000/em_ethdev.c +2 -2
@@ -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, },
drivers/net/e1000/igb_ethdev.c +4 -3
@@ -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, },
drivers/net/fm10k/fm10k_ethdev.c +2 -2
@@ -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, },
drivers/net/i40e/i40e_ethdev.c +2 -2
@@ -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;
drivers/net/i40e/i40e_ethdev_vf.c +2 -1
@@ -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, },
drivers/net/ixgbe/ixgbe_ethdev.c +4 -3
@@ -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, },
drivers/net/nfp/nfp_net.c +2 -2
@@ -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, },
drivers/net/virtio/virtio_ethdev.c +2 -1
@@ -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, },
drivers/net/vmxnet3/vmxnet3_ethdev.c +2 -1
@@ -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, },
drivers/net/xenvirt/rte_eth_xenvirt.c +1 -1
@@ -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 ;
lib/librte_eal/common/include/rte_pci.h +0 -2
@@ -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.
lib/librte_ether/rte_ethdev.c +0 -2
@@ -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;
Signature · 1 line
2.7.4
Re: [PATCH v2 2/7] pci: Separate detaching ethernet ports from PCI devices
Quoted 29 lines · 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 <[email protected]>--- 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(-)doc/guides/prog_guide/port_hotplug_framework.rst +0
-0
doc/guides/prog_guide/port_hotplug_framework.rst Limitations · line 109
Quoted 5 lines
doc/guides/prog_guide/port_hotplug_framework.rst +1
-1@@ -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'.
drivers/net/bnxt/bnxt_ethdev.c bnxt_dev_init(struct rte_eth_dev *eth_dev) · line 1057
Quoted 13 lines
doc/guides/prog_guide/port_hotplug_framework.rst +0
-0defined in the PMD, detaching is supported.drivers/net/bnxt/bnxt_ethdev.c +1
-0@@ -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
PATCH v2 3/7 pci: Pass rte_pci_addr to functions instead of separate args
In reply to Ben WalkerInstead of passing domain, bus, devid, func, just pass an rte_pci_addr.
Signed-off-by Ben Walker <[email protected]>
--- lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-)
lib/librte_eal/linuxapp/eal/eal_pci.c +13 -19
@@ -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:
Signature · 1 line
2.7.4
Re: [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args
Quoted 79 lines · Ben Walker
Instead of passing domain, bus, devid, func, just pass an
rte_pci_addr.Signed-off-by Ben Walker <[email protected]>--- lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-)lib/librte_eal/linuxapp/eal/eal_pci.c +9
-16@@ -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 ) {
lib/librte_eal/linuxapp/eal/eal_pci.c rte_eal_pci_scan(void) · line 496
Quoted 4 lines
lib/librte_eal/linuxapp/eal/eal_pci.c +1
-0@@ -500,20 +493,21 @@ rte_eal_pci_scan(void)return -1 ; } +
Unnecessary new line.
Quoted 11 lines
lib/librte_eal/linuxapp/eal/eal_pci.c +1
-2while ((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",
lib/librte_eal/linuxapp/eal/eal_pci.c rte_eal_pci_scan(void) · line 513
Quoted 10 lines
lib/librte_eal/linuxapp/eal/eal_pci.c +2
-1pci_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 <[email protected]>
Re: [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args
In reply to Ben WalkerHello Ben,
Quoted 94 lines · Ben Walker
Instead of passing domain, bus, devid, func, just pass an
rte_pci_addr.Signed-off-by Ben Walker <[email protected]>--- lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-)lib/librte_eal/linuxapp/eal/eal_pci.c +11
-18@@ -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",
lib/librte_eal/linuxapp/eal/eal_pci.c rte_eal_pci_scan(void) · line 513
Quoted 10 lines
lib/librte_eal/linuxapp/eal/eal_pci.c +2
-1pci_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
Re: [PATCH v2 3/7] pci: Pass rte_pci_addr to functions instead of separate args
Quoted 115 lines · Shreyansh Jain
Hello Ben,
Quoted 112 lines · Ben Walker
Instead of passing domain, bus, devid, func, just pass an
rte_pci_addr.Signed-off-by Ben Walker <[email protected]>---
lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++-------------------1 file changed, 13 insertions(+), 19 deletions(-)lib/librte_eal/linuxapp/eal/eal_pci.c +0
-0@@ -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
lib/librte_eal/linuxapp/eal/eal_pci.c +1
-2-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;lib/librte_eal/linuxapp/eal/eal_pci.c +1
-4@@ -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);lib/librte_eal/linuxapp/eal/eal_pci.c +0
-0@@ -429,16 +425,14 @@ pci_update_device(const struct rte_pci_addr *addr)pci_get_sysfs_path(),
addr->domain,addr->bus, addr-Quoted 1 line
devid,
addr->function);
lib/librte_eal/linuxapp/eal/eal_pci.c +1
-2-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 intlib/librte_eal/linuxapp/eal/eal_pci.c +1
-2-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 {lib/librte_eal/linuxapp/eal/eal_pci.c +9
-9@@ -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;
Quoted 6 lines
Quoted 2 lines
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_onesimpler (and pass along a new argument)..
Sure, go ahead.
Quoted 3 lines
-
Shreyansh
PATCH v2 4/7 pci: rte_eal_pci_scan now handles removal of PCI devices
In reply to Ben Walkerrte_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 <[email protected]>
--- lib/librte_eal/linuxapp/eal/eal_pci.c | 62 +++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+)
lib/librte_eal/linuxapp/eal/eal_pci.c +62 -0
@@ -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",
Signature · 1 line
2.7.4
Re: [PATCH v2 4/7] pci: rte_eal_pci_scan now handles removal of PCI devices
Hi Ben,
Quoted 6 lines · Ben Walker
rte_eal_pci_scancan 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 <[email protected]>---
[...]
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---
PATCH v3 1/3 pci: rte_eal_pci_scan now handles removal of PCI devices
In reply to Ben Walkerrte_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 <[email protected]>
---
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(+)
lib/librte_eal/linuxapp/eal/eal_pci.c +70 -0
@@ -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",
Signature · 1 line
2.9.3
PATCH v3 2/3 pci: Move driver registration above pci scan
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 <[email protected]>
---
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(-)
lib/librte_eal/common/include/rte_pci.h +28 -28
@@ -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
Signature · 1 line
2.9.3
PATCH v3 3/3 pci: Clarify interfaces for dynamic attach/detach of drivers
In reply to Ben WalkerThere 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 <[email protected]>
---
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(-)
lib/librte_eal/common/eal_common_dev.c +2 -2
@@ -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))
lib/librte_eal/common/eal_common_pci.c +29 -80
153 lines not on this page · raw message
lib/librte_eal/common/include/rte_pci.h +12 -10
@@ -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.
Signature · 1 line
2.9.3
Re: [PATCH v3 1/3] pci: rte_eal_pci_scan now handles removal of PCI devices
In reply to Ben WalkerQuoted 5 lines · benjamin.walker at intel.com (Ben Walker)
rte_eal_pci_scancan 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/
PATCH v2 5/7 pci: Move driver registration above pci scan
In reply to Ben WalkerThe 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 <[email protected]>
--- lib/librte_eal/common/include/rte_pci.h | 56 ++++++++++++++++----------------- 1 file changed, 28 insertions(+), 28 deletions(-)
lib/librte_eal/common/include/rte_pci.h +28 -28
@@ -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
Signature · 1 line
2.7.4
PATCH v2 6/7 pci: Combine rte_eal_pci_scan and rte_eal_pci_probe
In reply to Ben WalkerTwo 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 <[email protected]>
--- 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(-)
app/test/test_pci.c +1 -1
@@ -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);
lib/librte_eal/bsdapp/eal/eal.c +0 -3
@@ -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 )
lib/librte_eal/bsdapp/eal/eal_pci.c +1 -16
@@ -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 ; -}
lib/librte_eal/bsdapp/eal/rte_eal_version.map +0 -1
@@ -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;
lib/librte_eal/common/eal_common_pci.c +6 -0
@@ -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 ;
lib/librte_eal/common/eal_private.h +5 -9
@@ -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.
lib/librte_eal/common/include/rte_pci.h +5 -12
@@ -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.
lib/librte_eal/linuxapp/eal/eal.c +0 -3
@@ -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" );
lib/librte_eal/linuxapp/eal/eal_pci.c +1 -17
@@ -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 ; -}
lib/librte_eal/linuxapp/eal/rte_eal_version.map +0 -1
@@ -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;
Signature · 1 line
2.7.4
Re: [PATCH v2 6/7] pci: Combine rte_eal_pci_scan and rte_eal_pci_probe
Quoted 9 lines · Ben Walker
Two functions is both confusing and unnecessary. Previously,
rte_eal_pci_scanpopulated an internal list of devices by scanning sysfs. Then,rte_eal_pci_probewould match registered drivers to that internal list. These are not really useful operations to perform separately, though, so simplify the api down to justrte_eal_pci_probewhich 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
Quoted 99 lines
Signed-off-by Ben Walker <[email protected]>--- 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(-)app/test/test_pci.c +1
-1@@ -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);lib/librte_eal/bsdapp/eal/eal.c +0
-3@@ -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 )lib/librte_eal/bsdapp/eal/eal_pci.c +1
-16@@ -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 ; -}lib/librte_eal/bsdapp/eal/rte_eal_version.map +0
-1@@ -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;lib/librte_eal/common/eal_common_pci.c +1
-0@@ -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);
lib/librte_eal/common/eal_common_pci.c rte_eal_pci_probe(void) · line 431
Quoted 8 lines
lib/librte_eal/common/eal_common_pci.c +5
-0@@ -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.
Quoted 121 lines
lib/librte_eal/common/eal_common_pci.c +0
-0if (rte_eal_devargs_type_count (RTE_DEVTYPE_WHITELISTED_PCI ) ==0 ) probe_all =1 ;lib/librte_eal/common/eal_private.h +5
-9@@ -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. lib/librte_eal/common/include/rte_pci.h +5
-12@@ -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. lib/librte_eal/linuxapp/eal/eal.c +0
-3@@ -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" );lib/librte_eal/linuxapp/eal/eal_pci.c +1
-17@@ -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 ; -}lib/librte_eal/linuxapp/eal/rte_eal_version.map +0
-1@@ -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;
Signature · 2 lines
- Shreyansh
PATCH v2 7/7 pci: Clarify interfaces for dynamic attach/detach of drivers
In reply to Ben WalkerThere 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 <[email protected]>
--- 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(-)
lib/librte_eal/common/eal_common_dev.c +2 -2
@@ -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))
lib/librte_eal/common/eal_common_pci.c +29 -80
153 lines not on this page · raw message
lib/librte_eal/common/include/rte_pci.h +12 -10
@@ -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.
Signature · 1 line
2.7.4
Re: [PATCH v2 7/7] pci: Clarify interfaces for dynamic attach/detach of drivers
Quoted 18 lines · Ben Walker
There are now two functions -
rte_eal_pci_attach_driverandrte_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 <[email protected]>--- 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(-)lib/librte_eal/common/eal_common_dev.c +0
-0
lib/librte_eal/common/eal_common_dev.c int rte_eal_dev_attach(const char *name, const char *devargs) · line 116
Quoted 3 lines
lib/librte_eal/common/eal_common_dev.c +0
-0@@ -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?
Quoted 227 lines
lib/librte_eal/common/eal_common_dev.c +2
-2-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))lib/librte_eal/common/eal_common_pci.c +29
-80@@ -228,59 +228,36 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dreturn 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);lib/librte_eal/common/include/rte_pci.h +12
-10@@ -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.
Re: [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources.
In reply to Ben WalkerQuoted 19 lines · 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 <[email protected]>--- lib/librte_eal/common/eal_common_pci.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)lib/librte_eal/common/eal_common_pci.c +0
-0@@ -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 */
lib/librte_eal/common/eal_common_pci.c rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d · line 225
Quoted 10 lines
lib/librte_eal/common/eal_common_pci.c +4
-1ret = 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 <[email protected]>
Re: [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources.
2016-11-25 14:51, Shreyansh Jain:
Quoted 10 lines · Ben Walker
Quoted 7 lines
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 <[email protected]>Acked-by Shreyansh Jain <[email protected]>
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
Re: [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources.
2016-12-21 17:19, Thomas Monjalon:
Quoted 12 lines
2016-11-25 14:51, Shreyansh Jain:
Quoted 10 lines · Ben Walker
Quoted 7 lines
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 <[email protected]>Acked-by Shreyansh Jain <[email protected]>
Quoted 7 lines
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?
Re: [PATCH v2 1/7] pci: If a driver's probe function fails, unmap resources.
2017-01-04 18:39, Thomas Monjalon:
Quoted 16 lines
2016-12-21 17:19, Thomas Monjalon:
Quoted 15 lines
2016-11-25 14:51, Shreyansh Jain:
Quoted 10 lines · Ben Walker
Quoted 7 lines
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 <[email protected]>Acked-by Shreyansh Jain <[email protected]>The 3 first patches of this series seems good and easy to agree, except some minor nits.
Quoted 6 lines
Quoted 4 lines
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?
PATCH v3 1/3 pci: If a driver's probe function fails, unmap resources.
In reply to Ben WalkerIf 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 <[email protected]>
--- lib/librte_eal/common/eal_common_pci.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
lib/librte_eal/common/eal_common_pci.c +4 -1
@@ -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; }
Signature · 1 line
2.9.3
PATCH v3 2/3 pci: Separate detaching ethernet ports from PCI devices
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 <[email protected]>
--- 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(-)
doc/guides/prog_guide/port_hotplug_framework.rst +1 -1
@@ -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.
drivers/net/bnxt/bnxt_ethdev.c +2 -1
@@ -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 },
drivers/net/e1000/em_ethdev.c +2 -2
@@ -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, },
drivers/net/e1000/igb_ethdev.c +4 -3
@@ -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, },
drivers/net/fm10k/fm10k_ethdev.c +2 -2
@@ -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, },
drivers/net/i40e/i40e_ethdev.c +2 -2
@@ -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;
drivers/net/i40e/i40e_ethdev_vf.c +2 -1
@@ -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, },
drivers/net/ixgbe/ixgbe_ethdev.c +4 -3
@@ -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, },
drivers/net/nfp/nfp_net.c +2 -2
@@ -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, },
drivers/net/virtio/virtio_ethdev.c +4 -2
@@ -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, },
drivers/net/vmxnet3/vmxnet3_ethdev.c +2 -1
@@ -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, },
drivers/net/xenvirt/rte_eth_xenvirt.c +1 -1
@@ -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 ;
lib/librte_eal/common/include/rte_pci.h +0 -2
@@ -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.
lib/librte_ether/rte_ethdev.c +0 -2
@@ -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;
Signature · 1 line
2.9.3
PATCH v3 3/3 pci: Pass rte_pci_addr to functions instead of separate args
In reply to Ben WalkerInstead of passing domain, bus, devid, func, just pass an rte_pci_addr.
Signed-off-by Ben Walker <[email protected]>
--- lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-)
lib/librte_eal/linuxapp/eal/eal_pci.c +13 -19
@@ -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:
Signature · 1 line
2.9.3
Re: [PATCH v3 3/3] pci: Pass rte_pci_addr to functions instead of separate args
2017-01-11 10:10, Ben Walker:
Quoted 4 lines
Instead of passing domain, bus, devid, func, just pass an
rte_pci_addr.Signed-off-by Ben Walker <[email protected]>
Added v2 acks from Shreyansh and removed unnecessary empty lines as Shreyansh commented, and Series applied, thanks