Commit 369327fa authored by Ben Hutchings's avatar Ben Hutchings

sfc: Fix race in completion handling

When we poll for MCDI request completion, we don't hold the interface
lock while setting the response fields in struct efx_mcdi_iface.
Signed-off-by: default avatarBen Hutchings <bhutchings@solarflare.com>
parent df2cd8af
...@@ -112,16 +112,6 @@ static void efx_mcdi_copyin(struct efx_nic *efx, unsigned cmd, ...@@ -112,16 +112,6 @@ static void efx_mcdi_copyin(struct efx_nic *efx, unsigned cmd,
efx->type->mcdi_request(efx, hdr, hdr_len, inbuf, inlen); efx->type->mcdi_request(efx, hdr, hdr_len, inbuf, inlen);
} }
static void
efx_mcdi_copyout(struct efx_nic *efx, efx_dword_t *outbuf, size_t outlen)
{
struct efx_mcdi_iface *mcdi = efx_mcdi(efx);
BUG_ON(atomic_read(&mcdi->state) == MCDI_STATE_QUIESCENT);
efx->type->mcdi_read_response(efx, outbuf, mcdi->resp_hdr_len, outlen);
}
static int efx_mcdi_errno(unsigned int mcdi_err) static int efx_mcdi_errno(unsigned int mcdi_err)
{ {
switch (mcdi_err) { switch (mcdi_err) {
...@@ -200,9 +190,11 @@ static int efx_mcdi_poll(struct efx_nic *efx) ...@@ -200,9 +190,11 @@ static int efx_mcdi_poll(struct efx_nic *efx)
/* Check for a reboot atomically with respect to efx_mcdi_copyout() */ /* Check for a reboot atomically with respect to efx_mcdi_copyout() */
rc = efx_mcdi_poll_reboot(efx); rc = efx_mcdi_poll_reboot(efx);
if (rc) { if (rc) {
spin_lock_bh(&mcdi->iface_lock);
mcdi->resprc = rc; mcdi->resprc = rc;
mcdi->resp_hdr_len = 0; mcdi->resp_hdr_len = 0;
mcdi->resp_data_len = 0; mcdi->resp_data_len = 0;
spin_unlock_bh(&mcdi->iface_lock);
return 0; return 0;
} }
...@@ -231,7 +223,9 @@ static int efx_mcdi_poll(struct efx_nic *efx) ...@@ -231,7 +223,9 @@ static int efx_mcdi_poll(struct efx_nic *efx)
return -ETIMEDOUT; return -ETIMEDOUT;
} }
spin_lock_bh(&mcdi->iface_lock);
efx_mcdi_read_response_header(efx); efx_mcdi_read_response_header(efx);
spin_unlock_bh(&mcdi->iface_lock);
/* Return rc=0 like wait_event_timeout() */ /* Return rc=0 like wait_event_timeout() */
return 0; return 0;
...@@ -419,7 +413,7 @@ int efx_mcdi_rpc_finish(struct efx_nic *efx, unsigned cmd, size_t inlen, ...@@ -419,7 +413,7 @@ int efx_mcdi_rpc_finish(struct efx_nic *efx, unsigned cmd, size_t inlen,
"MC command 0x%x inlen %d mode %d timed out\n", "MC command 0x%x inlen %d mode %d timed out\n",
cmd, (int)inlen, mcdi->mode); cmd, (int)inlen, mcdi->mode);
} else { } else {
size_t resplen; size_t hdr_len, data_len;
/* At the very least we need a memory barrier here to ensure /* At the very least we need a memory barrier here to ensure
* we pick up changes from efx_mcdi_ev_cpl(). Protect against * we pick up changes from efx_mcdi_ev_cpl(). Protect against
...@@ -427,16 +421,17 @@ int efx_mcdi_rpc_finish(struct efx_nic *efx, unsigned cmd, size_t inlen, ...@@ -427,16 +421,17 @@ int efx_mcdi_rpc_finish(struct efx_nic *efx, unsigned cmd, size_t inlen,
* acquiring the iface_lock. */ * acquiring the iface_lock. */
spin_lock_bh(&mcdi->iface_lock); spin_lock_bh(&mcdi->iface_lock);
rc = mcdi->resprc; rc = mcdi->resprc;
resplen = mcdi->resp_data_len; hdr_len = mcdi->resp_hdr_len;
data_len = mcdi->resp_data_len;
spin_unlock_bh(&mcdi->iface_lock); spin_unlock_bh(&mcdi->iface_lock);
BUG_ON(rc > 0); BUG_ON(rc > 0);
if (rc == 0) { if (rc == 0) {
efx_mcdi_copyout(efx, outbuf, efx->type->mcdi_read_response(efx, outbuf, hdr_len,
min(outlen, mcdi->resp_data_len)); min(outlen, data_len));
if (outlen_actual != NULL) if (outlen_actual != NULL)
*outlen_actual = resplen; *outlen_actual = data_len;
} else if (cmd == MC_CMD_REBOOT && rc == -EIO) } else if (cmd == MC_CMD_REBOOT && rc == -EIO)
; /* Don't reset if MC_CMD_REBOOT returns EIO */ ; /* Don't reset if MC_CMD_REBOOT returns EIO */
else if (rc == -EIO || rc == -EINTR) { else if (rc == -EIO || rc == -EINTR) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment