Skip to content

Commit

Permalink
iavf: Fix kernel BUG in free_msi_irqs
Browse files Browse the repository at this point in the history
Fix driver not freeing VF's traffic irqs, prior to calling
pci_disable_msix in iavf_remove.
There were possible 2 erroneous states in which, iavf_close would
not be called.
One erroneous state is fixed by allowing netdev to register, when state
is already running. It was possible for VF adapter to enter state loop
from running to resetting, where iavf_open would subsequently fail.
If user would then unload driver/remove VF pci, iavf_close would not be
called, as the netdev was not registered, leaving traffic pcis still
allocated.
Fixed this by breaking loop, allowing netdev to open device when adapter
state is __IAVF_RUNNING and it is not explicitily downed.
Other possiblity is entering to iavf_remove from __IAVF_RESETTING state,
where iavf_close would not free irqs, but just return 0.
Fixed this by checking for last adapter state and then removing irqs.

Kernel panic:
[ 2773.628585] kernel BUG at drivers/pci/msi.c:375!
...
[ 2773.631567] RIP: 0010:free_msi_irqs+0x180/0x1b0
...
[ 2773.640939] Call Trace:
[ 2773.641572]  pci_disable_msix+0xf7/0x120
[ 2773.642224]  iavf_reset_interrupt_capability.part.41+0x15/0x30 [iavf]
[ 2773.642897]  iavf_remove+0x12e/0x500 [iavf]
[ 2773.643578]  pci_device_remove+0x3b/0xc0
[ 2773.644266]  device_release_driver_internal+0x103/0x1f0
[ 2773.644948]  pci_stop_bus_device+0x69/0x90
[ 2773.645576]  pci_stop_and_remove_bus_device+0xe/0x20
[ 2773.646215]  pci_iov_remove_virtfn+0xba/0x120
[ 2773.646862]  sriov_disable+0x2f/0xe0
[ 2773.647531]  ice_free_vfs+0x2f8/0x350 [ice]
[ 2773.648207]  ice_sriov_configure+0x94/0x960 [ice]
[ 2773.648883]  ? _kstrtoull+0x3b/0x90
[ 2773.649560]  sriov_numvfs_store+0x10a/0x190
[ 2773.650249]  kernfs_fop_write+0x116/0x190
[ 2773.650948]  vfs_write+0xa5/0x1a0
[ 2773.651651]  ksys_write+0x4f/0xb0
[ 2773.652358]  do_syscall_64+0x5b/0x1a0
[ 2773.653075]  entry_SYSCALL_64_after_hwframe+0x65/0xca

Fixes: 22ead37 ("i40evf: Add longer wait after remove module")
Signed-off-by: Przemyslaw Patynowski <przemyslawx.patynowski@intel.com>
Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
Tested-by: Konrad Jankowski <konrad0.jankowski@intel.com>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
  • Loading branch information
Kaaame authored and anguy11 committed Oct 29, 2021
1 parent 247aa00 commit 605ca7c
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 0 deletions.
36 changes: 36 additions & 0 deletions drivers/net/ethernet/intel/iavf/iavf.h
Original file line number Diff line number Diff line change
Expand Up @@ -394,13 +394,49 @@ struct iavf_device {
extern char iavf_driver_name[];
extern struct workqueue_struct *iavf_wq;

static inline const char *iavf_state_str(enum iavf_state_t state)
{
switch (state) {
case __IAVF_STARTUP:
return "__IAVF_STARTUP";
case __IAVF_REMOVE:
return "__IAVF_REMOVE";
case __IAVF_INIT_VERSION_CHECK:
return "__IAVF_INIT_VERSION_CHECK";
case __IAVF_INIT_GET_RESOURCES:
return "__IAVF_INIT_GET_RESOURCES";
case __IAVF_INIT_SW:
return "__IAVF_INIT_SW";
case __IAVF_INIT_FAILED:
return "__IAVF_INIT_FAILED";
case __IAVF_RESETTING:
return "__IAVF_RESETTING";
case __IAVF_COMM_FAILED:
return "__IAVF_COMM_FAILED";
case __IAVF_DOWN:
return "__IAVF_DOWN";
case __IAVF_DOWN_PENDING:
return "__IAVF_DOWN_PENDING";
case __IAVF_TESTING:
return "__IAVF_TESTING";
case __IAVF_RUNNING:
return "__IAVF_RUNNING";
default:
return "__IAVF_UNKNOWN_STATE";
}
}

static inline void iavf_change_state(struct iavf_adapter *adapter,
enum iavf_state_t state)
{
if (adapter->state != state) {
adapter->last_state = adapter->state;
adapter->state = state;
}
dev_dbg(&adapter->pdev->dev,
"state transition from:%s to:%s\n",
iavf_state_str(adapter->last_state),
iavf_state_str(adapter->state));
}

int iavf_up(struct iavf_adapter *adapter);
Expand Down
20 changes: 20 additions & 0 deletions drivers/net/ethernet/intel/iavf/iavf_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -3280,6 +3280,13 @@ static int iavf_open(struct net_device *netdev)
goto err_unlock;
}

if (adapter->state == __IAVF_RUNNING &&
!test_bit(__IAVF_VSI_DOWN, adapter->vsi.state)) {
dev_dbg(&adapter->pdev->dev, "VF is already open.\n");
err = 0;
goto err_unlock;
}

/* allocate transmit descriptors */
err = iavf_setup_all_tx_resources(adapter);
if (err)
Expand Down Expand Up @@ -3915,6 +3922,7 @@ static int __maybe_unused iavf_resume(struct device *dev_d)
static void iavf_remove(struct pci_dev *pdev)
{
struct iavf_adapter *adapter = iavf_pdev_to_adapter(pdev);
enum iavf_state_t prev_state = adapter->last_state;
struct net_device *netdev = adapter->netdev;
struct iavf_fdir_fltr *fdir, *fdirtmp;
struct iavf_vlan_filter *vlf, *vlftmp;
Expand Down Expand Up @@ -3953,10 +3961,22 @@ static void iavf_remove(struct pci_dev *pdev)
iavf_change_state(adapter, __IAVF_REMOVE);
adapter->aq_required = 0;
adapter->flags &= ~IAVF_FLAG_REINIT_ITR_NEEDED;

iavf_free_all_tx_resources(adapter);
iavf_free_all_rx_resources(adapter);
iavf_misc_irq_disable(adapter);
iavf_free_misc_irq(adapter);

/* In case we enter iavf_remove from erroneous state, free traffic irqs
* here, so as to not cause a kernel crash, when calling
* iavf_reset_interrupt_capability.
*/
if ((adapter->last_state == __IAVF_RESETTING &&
prev_state != __IAVF_DOWN) ||
(adapter->last_state == __IAVF_RUNNING &&
!(netdev->flags & IFF_UP)))
iavf_free_traffic_irqs(adapter);

iavf_reset_interrupt_capability(adapter);
iavf_free_q_vectors(adapter);

Expand Down

0 comments on commit 605ca7c

Please sign in to comment.