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

["Zhang, Helin" <helin.zhang@intel.com>]

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