Blob Blame History Raw
From: Michael Chan <michael.chan@broadcom.com>
Date: Sat, 31 Mar 2018 13:54:15 -0400
Subject: bnxt_en: Improve valid bit checking in firmware response message.
Patch-mainline: v4.17-rc1
Git-commit: 845adfe40c2a75e67ddae6639fc2b987338b7983
References: bsc#1086282 FATE#324873

When firmware sends a DMA response to the driver, the last byte of the
message will be set to 1 to indicate that the whole response is valid.
The driver waits for the message to be valid before reading the message.

The firmware spec allows these response messages to increase in
length by adding new fields to the end of these messages.  The
older spec's valid location may become a new field in a newer
spec.  To guarantee compatibility, the driver should zero the valid
byte before interpreting the entire message so that any new fields not
implemented by the older spec will be read as zero.

For messages that are forwarded to VFs, we need to set the length
and re-instate the valid bit so the VF will see the valid response.

Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Acked-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c       |   21 ++++++++++++++++-----
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c |    2 ++
 2 files changed, 18 insertions(+), 5 deletions(-)

--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -3397,7 +3397,8 @@ static int bnxt_hwrm_do_send_msg(struct
 	int i, intr_process, rc, tmo_count;
 	struct input *req = msg;
 	u32 *data = msg;
-	__le32 *resp_len, *valid;
+	__le32 *resp_len;
+	u8 *valid;
 	u16 cp_ring_id, len = 0;
 	struct hwrm_err_output *resp = bp->hwrm_cmd_resp_addr;
 	u16 max_req_len = BNXT_HWRM_MAX_REQ_LEN;
@@ -3449,6 +3450,7 @@ static int bnxt_hwrm_do_send_msg(struct
 
 	i = 0;
 	tmo_count = timeout * 40;
+	resp_len = bp->hwrm_cmd_resp_addr + HWRM_RESP_LEN_OFFSET;
 	if (intr_process) {
 		/* Wait until hwrm response cmpl interrupt is processed */
 		while (bp->hwrm_intr_seq_id != HWRM_SEQ_ID_INVALID &&
@@ -3461,9 +3463,11 @@ static int bnxt_hwrm_do_send_msg(struct
 				   le16_to_cpu(req->req_type));
 			return -1;
 		}
+		len = (le32_to_cpu(*resp_len) & HWRM_RESP_LEN_MASK) >>
+		      HWRM_RESP_LEN_SFT;
+		valid = bp->hwrm_cmd_resp_addr + len - 1;
 	} else {
 		/* Check if response len is updated */
-		resp_len = bp->hwrm_cmd_resp_addr + HWRM_RESP_LEN_OFFSET;
 		for (i = 0; i < tmo_count; i++) {
 			len = (le32_to_cpu(*resp_len) & HWRM_RESP_LEN_MASK) >>
 			      HWRM_RESP_LEN_SFT;
@@ -3479,10 +3483,12 @@ static int bnxt_hwrm_do_send_msg(struct
 			return -1;
 		}
 
-		/* Last word of resp contains valid bit */
-		valid = bp->hwrm_cmd_resp_addr + len - 4;
+		/* Last byte of resp contains valid bit */
+		valid = bp->hwrm_cmd_resp_addr + len - 1;
 		for (i = 0; i < 5; i++) {
-			if (le32_to_cpu(*valid) & HWRM_RESP_VALID_MASK)
+			/* make sure we read from updated DMA memory */
+			dma_rmb();
+			if (*valid)
 				break;
 			udelay(1);
 		}
@@ -3495,6 +3501,11 @@ static int bnxt_hwrm_do_send_msg(struct
 		}
 	}
 
+	/* Zero valid bit for compatibility.  Valid bit in an older spec
+	 * may become a new field in a newer spec.  We must make sure that
+	 * a new field not implemented by old spec will read zero.
+	 */
+	*valid = 0;
 	rc = le16_to_cpu(resp->error_code);
 	if (rc && !silent)
 		netdev_err(bp->dev, "hwrm req_type 0x%x seq id 0x%x error 0x%x\n",
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
@@ -974,7 +974,9 @@ static int bnxt_vf_set_link(struct bnxt
 		memcpy(&phy_qcfg_resp, &bp->link_info.phy_qcfg_resp,
 		       sizeof(phy_qcfg_resp));
 		mutex_unlock(&bp->hwrm_cmd_lock);
+		phy_qcfg_resp.resp_len = cpu_to_le16(sizeof(phy_qcfg_resp));
 		phy_qcfg_resp.seq_id = phy_qcfg_req->seq_id;
+		phy_qcfg_resp.valid = 1;
 
 		if (vf->flags & BNXT_VF_LINK_UP) {
 			/* if physical link is down, force link up on VF */