igb: do not drop PF mailbox lock after read of VF message

When the PF receives a mailbox message from the VF, it grabs the mailbox
lock, reads the VF message from the mailbox, ACKs the message and drops
the lock.

While the PF is performing the action for the VF message, nothing
prevents another VF message from being posted to the mailbox.  The
current code handles this condition by just dropping any new VF messages
without processing them.  This results in a mailbox timeout in the VM
for posted messages waiting for an ACK, and the VF is reset by the
igbvf_watchdog_task in the VM.

Given the right sequence of VF messages and mailbox timeouts, this
condition can go on ad infinitum.

Modify the PF mailbox read method to take an 'unlock' argument that
optionally leaves the mailbox locked by the PF after reading the VF
message.  This ensures another VF message is not posted to the mailbox
until after the PF has completed processing the VF message and written
its reply.

Signed-off-by: Greg Edwards <gedwards@ddn.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

diff --git a/drivers/net/ethernet/intel/igb/e1000_hw.h b/drivers/net/ethernet/intel/igb/e1000_hw.h
index 6076f258a0a5a118e17f9ccbe00c2bff7015b6ef..6ea9f702ba0f317d5cb7214c461950742d9035b0 100644 (file)
--- a/drivers/net/ethernet/intel/igb/e1000_hw.h
+++ b/drivers/net/ethernet/intel/igb/e1000_hw.h
@@ -491,7 +491,8 @@ struct e1000_fc_info {
 
 struct e1000_mbx_operations {
        s32 (*init_params)(struct e1000_hw *hw);
-       s32 (*read)(struct e1000_hw *hw, u32 *msg, u16 size, u16 mbx_id);
+       s32 (*read)(struct e1000_hw *hw, u32 *msg, u16 size, u16 mbx_id,
+                   bool unlock);
        s32 (*write)(struct e1000_hw *hw, u32 *msg, u16 size, u16 mbx_id);
        s32 (*read_posted)(struct e1000_hw *hw, u32 *msg, u16 size, u16 mbx_id);
        s32 (*write_posted)(struct e1000_hw *hw, u32 *msg, u16 size,

diff --git a/drivers/net/ethernet/intel/igb/e1000_mbx.c b/drivers/net/ethernet/intel/igb/e1000_mbx.c
index 6aa44723507bf3e42d35de6aa2bf7b5a948ba142..bffd58f7b2a1d8e1e49850f37dbf97916a876772 100644 (file)
--- a/drivers/net/ethernet/intel/igb/e1000_mbx.c
+++ b/drivers/net/ethernet/intel/igb/e1000_mbx.c
@@ -32,7 +32,8 @@
  *
  *  returns SUCCESS if it successfully read message from buffer
  **/
-s32 igb_read_mbx(struct e1000_hw *hw, u32 *msg, u16 size, u16 mbx_id)
+s32 igb_read_mbx(struct e1000_hw *hw, u32 *msg, u16 size, u16 mbx_id,
+                bool unlock)
 {
        struct e1000_mbx_info *mbx = &hw->mbx;
        s32 ret_val = -E1000_ERR_MBX;
@@ -42,7 +43,7 @@ s32 igb_read_mbx(struct e1000_hw *hw, u32 *msg, u16 size, u16 mbx_id)
                size = mbx->size;
 
        if (mbx->ops.read)
-               ret_val = mbx->ops.read(hw, msg, size, mbx_id);
+               ret_val = mbx->ops.read(hw, msg, size, mbx_id, unlock);
 
        return ret_val;
 }
@@ -222,7 +223,7 @@ static s32 igb_read_posted_mbx(struct e1000_hw *hw, u32 *msg, u16 size,
        ret_val = igb_poll_for_msg(hw, mbx_id);
 
        if (!ret_val)
-               ret_val = mbx->ops.read(hw, msg, size, mbx_id);
+               ret_val = mbx->ops.read(hw, msg, size, mbx_id, true);
 out:
        return ret_val;
 }
@@ -423,13 +424,14 @@ out_no_write:
  *  @msg: The message buffer
  *  @size: Length of buffer
  *  @vf_number: the VF index
+ *  @unlock: unlock the mailbox when done?
  *
  *  This function copies a message from the mailbox buffer to the caller's
  *  memory buffer.  The presumption is that the caller knows that there was
  *  a message due to a VF request so no polling for message is needed.
  **/
 static s32 igb_read_mbx_pf(struct e1000_hw *hw, u32 *msg, u16 size,
-                          u16 vf_number)
+                          u16 vf_number, bool unlock)
 {
        s32 ret_val;
        u16 i;
@@ -443,8 +445,12 @@ static s32 igb_read_mbx_pf(struct e1000_hw *hw, u32 *msg, u16 size,
        for (i = 0; i < size; i++)
                msg[i] = array_rd32(E1000_VMBMEM(vf_number), i);
 
-       /* Acknowledge the message and release buffer */
-       wr32(E1000_P2VMAILBOX(vf_number), E1000_P2VMAILBOX_ACK);
+       /* Acknowledge the message and release mailbox lock (or not) */
+       if (unlock)
+               wr32(E1000_P2VMAILBOX(vf_number), E1000_P2VMAILBOX_ACK);
+       else
+               wr32(E1000_P2VMAILBOX(vf_number),
+                    E1000_P2VMAILBOX_ACK | E1000_P2VMAILBOX_PFU);
 
        /* update stats */
        hw->mbx.stats.msgs_rx++;

diff --git a/drivers/net/ethernet/intel/igb/e1000_mbx.h b/drivers/net/ethernet/intel/igb/e1000_mbx.h
index a98c5dc60afd4ceb05228fb28cc0ca549a108941..a62b08e1572e440fb15afd49b06a315ed753f943 100644 (file)
--- a/drivers/net/ethernet/intel/igb/e1000_mbx.h
+++ b/drivers/net/ethernet/intel/igb/e1000_mbx.h
@@ -67,7 +67,8 @@
 
 #define E1000_PF_CONTROL_MSG   0x0100 /* PF control message */
 
-s32 igb_read_mbx(struct e1000_hw *hw, u32 *msg, u16 size, u16 mbx_id);
+s32 igb_read_mbx(struct e1000_hw *hw, u32 *msg, u16 size, u16 mbx_id,
+                bool unlock);
 s32 igb_write_mbx(struct e1000_hw *hw, u32 *msg, u16 size, u16 mbx_id);
 s32 igb_check_for_msg(struct e1000_hw *hw, u16 mbx_id);
 s32 igb_check_for_ack(struct e1000_hw *hw, u16 mbx_id);

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 1a99164d5d11c896fa0a0921f22a07e37f9fa787..fd4a46b03cc8317f44f0c4d3ee5141dd05858244 100644 (file)
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6675,32 +6675,33 @@ static void igb_rcv_msg_from_vf(struct igb_adapter *adapter, u32 vf)
        struct vf_data_storage *vf_data = &adapter->vf_data[vf];
        s32 retval;
 
-       retval = igb_read_mbx(hw, msgbuf, E1000_VFMAILBOX_SIZE, vf);
+       retval = igb_read_mbx(hw, msgbuf, E1000_VFMAILBOX_SIZE, vf, false);
 
        if (retval) {
                /* if receive failed revoke VF CTS stats and restart init */
                dev_err(&pdev->dev, "Error receiving message from VF\n");
                vf_data->flags &= ~IGB_VF_FLAG_CTS;
                if (!time_after(jiffies, vf_data->last_nack + (2 * HZ)))
-                       return;
+                       goto unlock;
                goto out;
        }
 
        /* this is a message we already processed, do nothing */
        if (msgbuf[0] & (E1000_VT_MSGTYPE_ACK | E1000_VT_MSGTYPE_NACK))
-               return;
+               goto unlock;
 
        /* until the vf completes a reset it should not be
         * allowed to start any configuration.
         */
        if (msgbuf[0] == E1000_VF_RESET) {
+               /* unlocks mailbox */
                igb_vf_reset_msg(adapter, vf);
                return;
        }
 
        if (!(vf_data->flags & IGB_VF_FLAG_CTS)) {
                if (!time_after(jiffies, vf_data->last_nack + (2 * HZ)))
-                       return;
+                       goto unlock;
                retval = -1;
                goto out;
        }
@@ -6741,7 +6742,12 @@ out:
        else
                msgbuf[0] |= E1000_VT_MSGTYPE_ACK;
 
+       /* unlocks mailbox */
        igb_write_mbx(hw, msgbuf, 1, vf);
+       return;
+
+unlock:
+       igb_unlock_mbx(hw, vf);
 }
 
 static void igb_msg_task(struct igb_adapter *adapter)