[dpdk-dev,v6] e1000: igb and em1000 PCI Port Hotplug changes

  This patch depends on the Port Hotplug Framework.
It implements the eth_dev_uninit functions for rte_em_pmd,
rte_igb_pmd and rte_igbvf_pmd.

Changes in v6:
Set nb_rx_queues and nb_tx_queues to 0 in uninit functions.
Rebase to latest code.

Changes in V5:
Moved adapter stopped boolean to struct e1000_adapter.
Rebase to latest code.

Changes in V4:
Release rx and tx queues in eth_igbvf_dev_uninit.

Changes in V3:
Add igb_adapter_stopped and em_adapter_stopped booleans.
Release rx and tx queues in eth_igb_dev_uninit.

Changes in V2:
Call dev_close() from  dev_uninit() functions.
Remove input parameter checking from dev_uninit() functions.

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 drivers/net/e1000/e1000_ethdev.h |    8 ++-
 drivers/net/e1000/em_ethdev.c    |   59 ++++++++++++++++++-
 drivers/net/e1000/igb_ethdev.c   |  119 +++++++++++++++++++++++++++++++++++++-
 drivers/net/e1000/igb_pf.c       |   22 +++++++
 4 files changed, 203 insertions(+), 5 deletions(-)
  

Hi Bernard

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Monday, June 22, 2015 6:44 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin; Iremonger, Bernard
> Subject: [PATCH v6] e1000: igb and em1000 PCI Port Hotplug changes
> 
> This patch depends on the Port Hotplug Framework.
> It implements the eth_dev_uninit functions for rte_em_pmd, rte_igb_pmd and
> rte_igbvf_pmd.
Would it be better to split this patch into smaller patches in a patch set as you did
for i40e hotplug?

Regards,
Helin

> 
> Changes in v6:
> Set nb_rx_queues and nb_tx_queues to 0 in uninit functions.
> Rebase to latest code.
> 
> Changes in V5:
> Moved adapter stopped boolean to struct e1000_adapter.
> Rebase to latest code.
> 
> Changes in V4:
> Release rx and tx queues in eth_igbvf_dev_uninit.
> 
> Changes in V3:
> Add igb_adapter_stopped and em_adapter_stopped booleans.
> Release rx and tx queues in eth_igb_dev_uninit.
> 
> Changes in V2:
> Call dev_close() from  dev_uninit() functions.
> Remove input parameter checking from dev_uninit() functions.
> 
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  drivers/net/e1000/e1000_ethdev.h |    8 ++-
>  drivers/net/e1000/em_ethdev.c    |   59 ++++++++++++++++++-
>  drivers/net/e1000/igb_ethdev.c   |  119
> +++++++++++++++++++++++++++++++++++++-
>  drivers/net/e1000/igb_pf.c       |   22 +++++++
>  4 files changed, 203 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/e1000/e1000_ethdev.h
> b/drivers/net/e1000/e1000_ethdev.h
> index c451faa..ee8b872 100644
> --- a/drivers/net/e1000/e1000_ethdev.h
> +++ b/drivers/net/e1000/e1000_ethdev.h
> @@ -1,7 +1,7 @@
>  /*-
>   *   BSD LICENSE
>   *
> - *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
>   *   All rights reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
> @@ -229,8 +229,12 @@ struct e1000_adapter {
>  	struct e1000_vfta       shadow_vfta;
>  	struct e1000_vf_info    *vfdata;
>  	struct e1000_filter_info filter;
> +	bool stopped;
>  };
> 
> +#define E1000_DEV_PRIVATE(adapter) \
> +	((struct e1000_adapter *)adapter)
> +
>  #define E1000_DEV_PRIVATE_TO_HW(adapter) \
>  	(&((struct e1000_adapter *)adapter)->hw)
> 
> @@ -337,4 +341,6 @@ uint16_t eth_em_recv_pkts(void *rx_queue, struct
> rte_mbuf **rx_pkts,  uint16_t eth_em_recv_scattered_pkts(void *rx_queue,
> struct rte_mbuf **rx_pkts,
>  		uint16_t nb_pkts);
> 
> +void igb_pf_host_uninit(struct rte_eth_dev *dev);
> +
>  #endif /* _E1000_ETHDEV_H_ */
> diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
> index a306c55..b496c39 100644
> --- a/drivers/net/e1000/em_ethdev.c
> +++ b/drivers/net/e1000/em_ethdev.c
> @@ -224,6 +224,8 @@ static int
>  eth_em_dev_init(struct rte_eth_dev *eth_dev)  {
>  	struct rte_pci_device *pci_dev;
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(eth_dev->data->dev_private);
>  	struct e1000_hw *hw =
>  		E1000_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
>  	struct e1000_vfta * shadow_vfta =
> @@ -246,6 +248,7 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev)
> 
>  	hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
>  	hw->device_id = pci_dev->id.device_id;
> +	adapter->stopped = 0;
> 
>  	/* For ICH8 support we'll need to map the flash memory BAR */
> 
> @@ -285,13 +288,60 @@ eth_em_dev_init(struct rte_eth_dev *eth_dev)
>  	return (0);
>  }
> 
> +static int
> +eth_em_dev_uninit(struct rte_eth_dev *eth_dev) {
> +	struct rte_pci_device *pci_dev;
> +	unsigned i;
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(eth_dev->data->dev_private);
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return -EPERM;
> +
> +	pci_dev = eth_dev->pci_dev;
> +
> +	if (adapter->stopped == 0)
> +		eth_em_close(eth_dev);
> +
> +	eth_dev->dev_ops = NULL;
> +	eth_dev->rx_pkt_burst = NULL;
> +	eth_dev->tx_pkt_burst = NULL;
> +
> +	rte_free(eth_dev->data->mac_addrs);
> +	eth_dev->data->mac_addrs = NULL;
> +
> +	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
> +		eth_em_rx_queue_release(eth_dev->data->rx_queues[i]);
> +		eth_dev->data->rx_queues[i] = NULL;
> +	}
> +	eth_dev->data->nb_rx_queues = 0;
> +
> +	for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
> +		eth_em_tx_queue_release(eth_dev->data->tx_queues[i]);
> +		eth_dev->data->tx_queues[i] = NULL;
> +	}
> +	eth_dev->data->nb_tx_queues = 0;
> +
> +	/* disable uio intr before callback unregister */
> +	rte_intr_disable(&(pci_dev->intr_handle));
> +	rte_intr_callback_unregister(&(pci_dev->intr_handle),
> +		eth_em_interrupt_handler, (void *)eth_dev);
> +
> +	return 0;
> +}
> +
>  static struct eth_driver rte_em_pmd = {
>  	.pci_drv = {
>  		.name = "rte_em_pmd",
>  		.id_table = pci_id_em_map,
> -		.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> RTE_PCI_DRV_INTR_LSC,
> +		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC
> |
> +			RTE_PCI_DRV_DETACHABLE,
>  	},
>  	.eth_dev_init = eth_em_dev_init,
> +	.eth_dev_uninit = eth_em_dev_uninit,
>  	.dev_private_size = sizeof(struct e1000_adapter),  };
> 
> @@ -451,6 +501,8 @@ em_set_pba(struct e1000_hw *hw)  static int
> eth_em_start(struct rte_eth_dev *dev)  {
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(dev->data->dev_private);
>  	struct e1000_hw *hw =
>  		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	int ret, mask;
> @@ -570,6 +622,8 @@ eth_em_start(struct rte_eth_dev *dev)
>  		}
>  	}
> 
> +	adapter->stopped = 0;
> +
>  	PMD_INIT_LOG(DEBUG, "<<");
> 
>  	return (0);
> @@ -613,8 +667,11 @@ static void
>  eth_em_close(struct rte_eth_dev *dev)
>  {
>  	struct e1000_hw *hw =
> E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(dev->data->dev_private);
> 
>  	eth_em_stop(dev);
> +	adapter->stopped = 1;
>  	e1000_phy_hw_reset(hw);
>  	em_release_manageability(hw);
>  	em_hw_control_release(hw);
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index 24c7510..16acdc9 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -465,9 +465,12 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
>  	struct e1000_hw *hw =
>  		E1000_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
>  	struct e1000_vfta * shadow_vfta =
> -			E1000_DEV_PRIVATE_TO_VFTA(eth_dev->data->dev_private);
> +		E1000_DEV_PRIVATE_TO_VFTA(eth_dev->data->dev_private);
>  	struct e1000_filter_info *filter_info =
>  		E1000_DEV_PRIVATE_TO_FILTER_INFO(eth_dev->data->dev_private);
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(eth_dev->data->dev_private);
> +
>  	uint32_t ctrl_ext;
> 
>  	pci_dev = eth_dev->pci_dev;
> @@ -570,6 +573,7 @@ eth_igb_dev_init(struct rte_eth_dev *eth_dev)
>  		goto err_late;
>  	}
>  	hw->mac.get_link_status = 1;
> +	adapter->stopped = 0;
> 
>  	/* Indicate SOL/IDER usage */
>  	if (e1000_check_reset_block(hw) < 0) { @@ -614,6 +618,59 @@ err_late:
>  	return (error);
>  }
> 
> +static int
> +eth_igb_dev_uninit(struct rte_eth_dev *eth_dev) {
> +	struct rte_pci_device *pci_dev;
> +	struct e1000_hw *hw;
> +	unsigned i;
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(eth_dev->data->dev_private);
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return -EPERM;
> +
> +	hw = E1000_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
> +	pci_dev = eth_dev->pci_dev;
> +
> +	if (adapter->stopped == 0)
> +		eth_igb_close(eth_dev);
> +
> +	eth_dev->dev_ops = NULL;
> +	eth_dev->rx_pkt_burst = NULL;
> +	eth_dev->tx_pkt_burst = NULL;
> +
> +	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
> +		eth_igb_rx_queue_release(eth_dev->data->rx_queues[i]);
> +		eth_dev->data->rx_queues[i] = NULL;
> +	}
> +	eth_dev->data->nb_rx_queues = 0;
> +
> +	for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
> +		eth_igb_tx_queue_release(eth_dev->data->tx_queues[i]);
> +		eth_dev->data->tx_queues[i] = NULL;
> +	}
> +	eth_dev->data->nb_tx_queues = 0;
> +
> +	/* Reset any pending lock */
> +	igb_reset_swfw_lock(hw);
> +
> +	rte_free(eth_dev->data->mac_addrs);
> +	eth_dev->data->mac_addrs = NULL;
> +
> +	/* uninitialize PF if max_vfs not zero */
> +	igb_pf_host_uninit(eth_dev);
> +
> +	/* disable uio intr before callback unregister */
> +	rte_intr_disable(&(pci_dev->intr_handle));
> +	rte_intr_callback_unregister(&(pci_dev->intr_handle),
> +		eth_igb_interrupt_handler, (void *)eth_dev);
> +
> +	return 0;
> +}
> +
>  /*
>   * Virtual Function device init
>   */
> @@ -621,6 +678,8 @@ static int
>  eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)  {
>  	struct rte_pci_device *pci_dev;
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(eth_dev->data->dev_private);
>  	struct e1000_hw *hw =
>  		E1000_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
>  	int diag;
> @@ -645,6 +704,7 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
>  	hw->device_id = pci_dev->id.device_id;
>  	hw->vendor_id = pci_dev->id.vendor_id;
>  	hw->hw_addr = (void *)pci_dev->mem_resource[0].addr;
> +	adapter->stopped = 0;
> 
>  	/* Initialize the shared code (base driver) */
>  	diag = e1000_setup_init_funcs(hw, TRUE); @@ -685,13 +745,52 @@
> eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
>  	return 0;
>  }
> 
> +static int
> +eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev) {
> +	unsigned i;
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(eth_dev->data->dev_private);
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> +		return -EPERM;
> +
> +	if (adapter->stopped == 0)
> +		igbvf_dev_close(eth_dev);
> +
> +	eth_dev->dev_ops = NULL;
> +	eth_dev->rx_pkt_burst = NULL;
> +	eth_dev->tx_pkt_burst = NULL;
> +
> +	for (i = 0; i < eth_dev->data->nb_rx_queues; i++) {
> +		eth_igb_rx_queue_release(eth_dev->data->rx_queues[i]);
> +		eth_dev->data->rx_queues[i] = NULL;
> +	}
> +	eth_dev->data->nb_rx_queues = 0;
> +
> +	for (i = 0; i < eth_dev->data->nb_tx_queues; i++) {
> +		eth_igb_tx_queue_release(eth_dev->data->tx_queues[i]);
> +		eth_dev->data->tx_queues[i] = NULL;
> +	}
> +	eth_dev->data->nb_tx_queues = 0;
> +
> +	rte_free(eth_dev->data->mac_addrs);
> +	eth_dev->data->mac_addrs = NULL;
> +
> +	return 0;
> +}
> +
>  static struct eth_driver rte_igb_pmd = {
>  	.pci_drv = {
>  		.name = "rte_igb_pmd",
>  		.id_table = pci_id_igb_map,
> -		.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> RTE_PCI_DRV_INTR_LSC,
> +		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC
> |
> +			RTE_PCI_DRV_DETACHABLE,
>  	},
>  	.eth_dev_init = eth_igb_dev_init,
> +	.eth_dev_uninit = eth_igb_dev_uninit,
>  	.dev_private_size = sizeof(struct e1000_adapter),  };
> 
> @@ -702,9 +801,10 @@ static struct eth_driver rte_igbvf_pmd = {
>  	.pci_drv = {
>  		.name = "rte_igbvf_pmd",
>  		.id_table = pci_id_igbvf_map,
> -		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
> +		.drv_flags = RTE_PCI_DRV_NEED_MAPPING |
> RTE_PCI_DRV_DETACHABLE,
>  	},
>  	.eth_dev_init = eth_igbvf_dev_init,
> +	.eth_dev_uninit = eth_igbvf_dev_uninit,
>  	.dev_private_size = sizeof(struct e1000_adapter),  };
> 
> @@ -758,6 +858,8 @@ eth_igb_start(struct rte_eth_dev *dev)  {
>  	struct e1000_hw *hw =
>  		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(dev->data->dev_private);
>  	int ret, i, mask;
>  	uint32_t ctrl_ext;
> 
> @@ -786,6 +888,7 @@ eth_igb_start(struct rte_eth_dev *dev)
>  		PMD_INIT_LOG(ERR, "Unable to initialize the hardware");
>  		return (-EIO);
>  	}
> +	adapter->stopped = 0;
> 
>  	E1000_WRITE_REG(hw, E1000_VET, ETHER_TYPE_VLAN << 16 |
> ETHER_TYPE_VLAN);
> 
> @@ -992,9 +1095,13 @@ static void
>  eth_igb_close(struct rte_eth_dev *dev)
>  {
>  	struct e1000_hw *hw =
> E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(dev->data->dev_private);
>  	struct rte_eth_link link;
> 
>  	eth_igb_stop(dev);
> +	adapter->stopped = 1;
> +
>  	e1000_phy_hw_reset(hw);
>  	igb_release_manageability(hw);
>  	igb_hw_control_release(hw);
> @@ -2228,11 +2335,14 @@ igbvf_dev_start(struct rte_eth_dev *dev)  {
>  	struct e1000_hw *hw =
>  		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(dev->data->dev_private);
>  	int ret;
> 
>  	PMD_INIT_FUNC_TRACE();
> 
>  	hw->mac.ops.reset_hw(hw);
> +	adapter->stopped = 0;
> 
>  	/* Set all vfta */
>  	igbvf_set_vfta_all(dev,1);
> @@ -2270,12 +2380,15 @@ static void
>  igbvf_dev_close(struct rte_eth_dev *dev)  {
>  	struct e1000_hw *hw =
> E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct e1000_adapter *adapter =
> +		E1000_DEV_PRIVATE(dev->data->dev_private);
> 
>  	PMD_INIT_FUNC_TRACE();
> 
>  	e1000_reset_hw(hw);
> 
>  	igbvf_dev_stop(dev);
> +	adapter->stopped = 1;
>  }
> 
>  static int igbvf_set_vfta(struct e1000_hw *hw, uint16_t vid, bool on) diff --git
> a/drivers/net/e1000/igb_pf.c b/drivers/net/e1000/igb_pf.c index
> 6a4d210..26c2960 100644
> --- a/drivers/net/e1000/igb_pf.c
> +++ b/drivers/net/e1000/igb_pf.c
> @@ -127,6 +127,28 @@ void igb_pf_host_init(struct rte_eth_dev *eth_dev)
>  	return;
>  }
> 
> +void igb_pf_host_uninit(struct rte_eth_dev *dev) {
> +	struct e1000_vf_info **vfinfo;
> +	uint16_t vf_num;
> +
> +	PMD_INIT_FUNC_TRACE();
> +
> +	vfinfo = E1000_DEV_PRIVATE_TO_P_VFDATA(dev->data->dev_private);
> +
> +	RTE_ETH_DEV_SRIOV(dev).active = 0;
> +	RTE_ETH_DEV_SRIOV(dev).nb_q_per_pool = 0;
> +	RTE_ETH_DEV_SRIOV(dev).def_vmdq_idx = 0;
> +	RTE_ETH_DEV_SRIOV(dev).def_pool_q_idx = 0;
> +
> +	vf_num = dev_num_vf(dev);
> +	if (vf_num == 0)
> +		return;
> +
> +	rte_free(*vfinfo);
> +	*vfinfo = NULL;
> +}
> +
>  #define E1000_RAH_POOLSEL_SHIFT    (18)
>  int igb_pf_host_configure(struct rte_eth_dev *eth_dev)  {
> --
> 1.7.4.1
  

> -----Original Message-----
> From: Zhang, Helin
> Sent: Thursday, June 25, 2015 3:34 AM
> To: Iremonger, Bernard; dev@dpdk.org
> Subject: RE: [PATCH v6] e1000: igb and em1000 PCI Port Hotplug changes
> 
> Hi Bernard
> 
> > -----Original Message-----
> > From: Iremonger, Bernard
> > Sent: Monday, June 22, 2015 6:44 PM
> > To: dev@dpdk.org
> > Cc: Zhang, Helin; Iremonger, Bernard
> > Subject: [PATCH v6] e1000: igb and em1000 PCI Port Hotplug changes
> >
> > This patch depends on the Port Hotplug Framework.
> > It implements the eth_dev_uninit functions for rte_em_pmd,
> rte_igb_pmd
> > and rte_igbvf_pmd.
> Would it be better to split this patch into smaller patches in a patch set as you
> did for i40e hotplug?
> 
> Regards,
> Helin

Hi Helin,
I don't think there is anything to be gained by splitting up this patch.
All the changes are hotplug related.

In the case of the i40e, there are five patches which are unrelated to hotplug 
which resolve issues encountered during development.
 
Regards,

Bernard

<snip>
  

> -----Original Message-----
> From: Iremonger, Bernard
> Sent: Monday, June 22, 2015 6:44 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin; Iremonger, Bernard
> Subject: [PATCH v6] e1000: igb and em1000 PCI Port Hotplug changes
> 
> This patch depends on the Port Hotplug Framework.
> It implements the eth_dev_uninit functions for rte_em_pmd, rte_igb_pmd and
> rte_igbvf_pmd.
> 
> Changes in v6:
> Set nb_rx_queues and nb_tx_queues to 0 in uninit functions.
> Rebase to latest code.
> 
> Changes in V5:
> Moved adapter stopped boolean to struct e1000_adapter.
> Rebase to latest code.
> 
> Changes in V4:
> Release rx and tx queues in eth_igbvf_dev_uninit.
> 
> Changes in V3:
> Add igb_adapter_stopped and em_adapter_stopped booleans.
> Release rx and tx queues in eth_igb_dev_uninit.
> 
> Changes in V2:
> Call dev_close() from  dev_uninit() functions.
> Remove input parameter checking from dev_uninit() functions.
> 
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>