Skip to content

Commit

Permalink
Merge branch 'ibmvnic-Harden-device-commands-and-queries'
Browse files Browse the repository at this point in the history 
Thomas Falcon says:

====================
ibmvnic: Harden device commands and queries

This patch series fixes some shortcomings with the current
VNIC device command implementation. The first patch fixes
the initialization of driver completion structures used
for device commands. Additionally, all waits for device
commands are bounded with a timeout in the event that the
device does not respond or becomes inoperable. Finally,
serialize queries to retain the integrity of device return
codes.

Changes in v2:

 - included header comment for ibmvnic_wait_for_completion
 - removed open-coded loop in patch 3/4, suggested by Jakub
 - ibmvnic_wait_for_completion accepts timeout value in milliseconds
   instead of jiffies
 - timeout calculations cleaned up and completed before wait loop
 - included missing mutex_destroy calls, suggested by Jakub
 - included comment before mutex declaration
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Nov 26, 2019
2 parents fb82238 + ff25dcb commit e94a5d1
Show file tree
Hide file tree
Showing 2 changed files with 167 additions and 27 deletions.
192 changes: 165 additions & 27 deletions drivers/net/ethernet/ibm/ibmvnic.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,40 @@ static long h_reg_sub_crq(unsigned long unit_address, unsigned long token,
return rc;
}

/**
* ibmvnic_wait_for_completion - Check device state and wait for completion
* @adapter: private device data
* @comp_done: completion structure to wait for
* @timeout: time to wait in milliseconds
*
* Wait for a completion signal or until the timeout limit is reached
* while checking that the device is still active.
*/
static int ibmvnic_wait_for_completion(struct ibmvnic_adapter *adapter,
struct completion *comp_done,
unsigned long timeout)
{
struct net_device *netdev;
unsigned long div_timeout;
u8 retry;

netdev = adapter->netdev;
retry = 5;
div_timeout = msecs_to_jiffies(timeout / retry);
while (true) {
if (!adapter->crq.active) {
netdev_err(netdev, "Device down!\n");
return -ENODEV;
}
if (retry--)
break;
if (wait_for_completion_timeout(comp_done, div_timeout))
return 0;
}
netdev_err(netdev, "Operation timed out.\n");
return -ETIMEDOUT;
}

static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
struct ibmvnic_long_term_buff *ltb, int size)
{
Expand All @@ -176,21 +210,35 @@ static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
ltb->map_id = adapter->map_id;
adapter->map_id++;

init_completion(&adapter->fw_done);
mutex_lock(&adapter->fw_lock);
adapter->fw_done_rc = 0;
reinit_completion(&adapter->fw_done);
rc = send_request_map(adapter, ltb->addr,
ltb->size, ltb->map_id);
if (rc) {
dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
mutex_unlock(&adapter->fw_lock);
return rc;
}

rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
if (rc) {
dev_err(dev,
"Long term map request aborted or timed out,rc = %d\n",
rc);
dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
mutex_unlock(&adapter->fw_lock);
return rc;
}
wait_for_completion(&adapter->fw_done);

if (adapter->fw_done_rc) {
dev_err(dev, "Couldn't map long term buffer,rc = %d\n",
adapter->fw_done_rc);
dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
mutex_unlock(&adapter->fw_lock);
return -1;
}
mutex_unlock(&adapter->fw_lock);
return 0;
}

Expand All @@ -211,22 +259,37 @@ static void free_long_term_buff(struct ibmvnic_adapter *adapter,
static int reset_long_term_buff(struct ibmvnic_adapter *adapter,
struct ibmvnic_long_term_buff *ltb)
{
struct device *dev = &adapter->vdev->dev;
int rc;

memset(ltb->buff, 0, ltb->size);

init_completion(&adapter->fw_done);
mutex_lock(&adapter->fw_lock);
adapter->fw_done_rc = 0;

reinit_completion(&adapter->fw_done);
rc = send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
if (rc)
if (rc) {
mutex_unlock(&adapter->fw_lock);
return rc;
}

rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
if (rc) {
dev_info(dev,
"Reset failed, long term map request timed out or aborted\n");
mutex_unlock(&adapter->fw_lock);
return rc;
wait_for_completion(&adapter->fw_done);
}

if (adapter->fw_done_rc) {
dev_info(&adapter->vdev->dev,
dev_info(dev,
"Reset failed, attempting to free and reallocate buffer\n");
free_long_term_buff(adapter, ltb);
mutex_unlock(&adapter->fw_lock);
return alloc_long_term_buff(adapter, ltb, ltb->size);
}
mutex_unlock(&adapter->fw_lock);
return 0;
}

Expand Down Expand Up @@ -943,13 +1006,25 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
if (adapter->vpd->buff)
len = adapter->vpd->len;

init_completion(&adapter->fw_done);
mutex_lock(&adapter->fw_lock);
adapter->fw_done_rc = 0;
reinit_completion(&adapter->fw_done);

crq.get_vpd_size.first = IBMVNIC_CRQ_CMD;
crq.get_vpd_size.cmd = GET_VPD_SIZE;
rc = ibmvnic_send_crq(adapter, &crq);
if (rc)
if (rc) {
mutex_unlock(&adapter->fw_lock);
return rc;
wait_for_completion(&adapter->fw_done);
}

rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
if (rc) {
dev_err(dev, "Could not retrieve VPD size, rc = %d\n", rc);
mutex_unlock(&adapter->fw_lock);
return rc;
}
mutex_unlock(&adapter->fw_lock);

if (!adapter->vpd->len)
return -ENODATA;
Expand All @@ -976,7 +1051,10 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
return -ENOMEM;
}

mutex_lock(&adapter->fw_lock);
adapter->fw_done_rc = 0;
reinit_completion(&adapter->fw_done);

crq.get_vpd.first = IBMVNIC_CRQ_CMD;
crq.get_vpd.cmd = GET_VPD;
crq.get_vpd.ioba = cpu_to_be32(adapter->vpd->dma_addr);
Expand All @@ -985,10 +1063,20 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
if (rc) {
kfree(adapter->vpd->buff);
adapter->vpd->buff = NULL;
mutex_unlock(&adapter->fw_lock);
return rc;
}

rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
if (rc) {
dev_err(dev, "Unable to retrieve VPD, rc = %d\n", rc);
kfree(adapter->vpd->buff);
adapter->vpd->buff = NULL;
mutex_unlock(&adapter->fw_lock);
return rc;
}
wait_for_completion(&adapter->fw_done);

mutex_unlock(&adapter->fw_lock);
return 0;
}

Expand Down Expand Up @@ -1689,20 +1777,25 @@ static int __ibmvnic_set_mac(struct net_device *netdev, u8 *dev_addr)
crq.change_mac_addr.cmd = CHANGE_MAC_ADDR;
ether_addr_copy(&crq.change_mac_addr.mac_addr[0], dev_addr);

init_completion(&adapter->fw_done);
mutex_lock(&adapter->fw_lock);
adapter->fw_done_rc = 0;
reinit_completion(&adapter->fw_done);

rc = ibmvnic_send_crq(adapter, &crq);
if (rc) {
rc = -EIO;
mutex_unlock(&adapter->fw_lock);
goto err;
}

wait_for_completion(&adapter->fw_done);
rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
/* netdev->dev_addr is changed in handle_change_mac_rsp function */
if (adapter->fw_done_rc) {
if (rc || adapter->fw_done_rc) {
rc = -EIO;
mutex_unlock(&adapter->fw_lock);
goto err;
}

mutex_unlock(&adapter->fw_lock);
return 0;
err:
ether_addr_copy(adapter->mac_addr, netdev->dev_addr);
Expand Down Expand Up @@ -2316,12 +2409,19 @@ static int wait_for_reset(struct ibmvnic_adapter *adapter)
adapter->fallback.rx_entries = adapter->req_rx_add_entries_per_subcrq;
adapter->fallback.tx_entries = adapter->req_tx_entries_per_subcrq;

init_completion(&adapter->reset_done);
reinit_completion(&adapter->reset_done);
adapter->wait_for_reset = true;
rc = ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM);
if (rc)
return rc;
wait_for_completion(&adapter->reset_done);

if (rc) {
ret = rc;
goto out;
}
rc = ibmvnic_wait_for_completion(adapter, &adapter->reset_done, 60000);
if (rc) {
ret = -ENODEV;
goto out;
}

ret = 0;
if (adapter->reset_done_rc) {
Expand All @@ -2332,13 +2432,21 @@ static int wait_for_reset(struct ibmvnic_adapter *adapter)
adapter->desired.rx_entries = adapter->fallback.rx_entries;
adapter->desired.tx_entries = adapter->fallback.tx_entries;

init_completion(&adapter->reset_done);
reinit_completion(&adapter->reset_done);
adapter->wait_for_reset = true;
rc = ibmvnic_reset(adapter, VNIC_RESET_CHANGE_PARAM);
if (rc)
return ret;
wait_for_completion(&adapter->reset_done);
if (rc) {
ret = rc;
goto out;
}
rc = ibmvnic_wait_for_completion(adapter, &adapter->reset_done,
60000);
if (rc) {
ret = -ENODEV;
goto out;
}
}
out:
adapter->wait_for_reset = false;

return ret;
Expand Down Expand Up @@ -2603,11 +2711,13 @@ static void ibmvnic_get_ethtool_stats(struct net_device *dev,
cpu_to_be32(sizeof(struct ibmvnic_statistics));

/* Wait for data to be written */
init_completion(&adapter->stats_done);
reinit_completion(&adapter->stats_done);
rc = ibmvnic_send_crq(adapter, &crq);
if (rc)
return;
wait_for_completion(&adapter->stats_done);
rc = ibmvnic_wait_for_completion(adapter, &adapter->stats_done, 10000);
if (rc)
return;

for (i = 0; i < ARRAY_SIZE(ibmvnic_stats); i++)
data[i] = be64_to_cpu(IBMVNIC_GET_STAT(adapter,
Expand Down Expand Up @@ -4408,11 +4518,24 @@ static int send_query_phys_parms(struct ibmvnic_adapter *adapter)
memset(&crq, 0, sizeof(crq));
crq.query_phys_parms.first = IBMVNIC_CRQ_CMD;
crq.query_phys_parms.cmd = QUERY_PHYS_PARMS;
init_completion(&adapter->fw_done);

mutex_lock(&adapter->fw_lock);
adapter->fw_done_rc = 0;
reinit_completion(&adapter->fw_done);

rc = ibmvnic_send_crq(adapter, &crq);
if (rc)
if (rc) {
mutex_unlock(&adapter->fw_lock);
return rc;
wait_for_completion(&adapter->fw_done);
}

rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
if (rc) {
mutex_unlock(&adapter->fw_lock);
return rc;
}

mutex_unlock(&adapter->fw_lock);
return adapter->fw_done_rc ? -EIO : 0;
}

Expand Down Expand Up @@ -4505,6 +4628,15 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
case IBMVNIC_CRQ_XPORT_EVENT:
netif_carrier_off(netdev);
adapter->crq.active = false;
/* terminate any thread waiting for a response
* from the device
*/
if (!completion_done(&adapter->fw_done)) {
adapter->fw_done_rc = -EIO;
complete(&adapter->fw_done);
}
if (!completion_done(&adapter->stats_done))
complete(&adapter->stats_done);
if (test_bit(0, &adapter->resetting))
adapter->force_reset_recovery = true;
if (gen_crq->cmd == IBMVNIC_PARTITION_MIGRATED) {
Expand Down Expand Up @@ -4959,7 +5091,11 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
__ibmvnic_delayed_reset);
INIT_LIST_HEAD(&adapter->rwi_list);
spin_lock_init(&adapter->rwi_lock);
mutex_init(&adapter->fw_lock);
init_completion(&adapter->init_done);
init_completion(&adapter->fw_done);
init_completion(&adapter->reset_done);
init_completion(&adapter->stats_done);
clear_bit(0, &adapter->resetting);

do {
Expand Down Expand Up @@ -5017,6 +5153,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
ibmvnic_init_fail:
release_sub_crqs(adapter, 1);
release_crq_queue(adapter);
mutex_destroy(&adapter->fw_lock);
free_netdev(netdev);

return rc;
Expand All @@ -5041,6 +5178,7 @@ static int ibmvnic_remove(struct vio_dev *dev)
adapter->state = VNIC_REMOVED;

rtnl_unlock();
mutex_destroy(&adapter->fw_lock);
device_remove_file(&dev->dev, &dev_attr_failover);
free_netdev(netdev);
dev_set_drvdata(&dev->dev, NULL);
Expand Down
2 changes: 2 additions & 0 deletions drivers/net/ethernet/ibm/ibmvnic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,8 @@ struct ibmvnic_adapter {
int init_done_rc;

struct completion fw_done;
/* Used for serialization of device commands */
struct mutex fw_lock;
int fw_done_rc;

struct completion reset_done;
Expand Down

0 comments on commit e94a5d1

Please sign in to comment.