Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1001dc37 authored by Mitch Williams's avatar Mitch Williams Committed by Jeff Kirsher
Browse files

i40e: don't overload fields



Overloading the msg_size field in the arq_event_info struct is just a
bad idea. It leads to repeated bugs when the structure is used in a
loop, since the input value (buffer size) is overwritten by the output
value (actual message length).

Fix this by splitting the field into two and renaming to indicate the
actual function of each field.

Since the arq_event struct has now changed, we need to change the drivers
to support this. Note that we no longer need to initialize the buffer size
each time we go through a loop as this value is no longer destroyed by
arq processing.

In the process, we also fix a bug in i40evf_verify_api_ver where the
buffer size was not correctly reinitialized each time through the loop.

Change-ID: Ic7f9633cdd6f871f93e698dfb095e29c696f5581
Signed-off-by: default avatarMitch Williams <mitch.a.williams@intel.com>
Acked-by: default avatarShannon Nelson <shannon.nelson@intel.com>
Acked-by: default avatarAshish Shah <ashish.n.shah@intel.com>
Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
parent 688ff32d
Loading
Loading
Loading
Loading
+3 −3
Original line number Original line Diff line number Diff line
@@ -980,10 +980,10 @@ i40e_status i40e_clean_arq_element(struct i40e_hw *hw,


	e->desc = *desc;
	e->desc = *desc;
	datalen = le16_to_cpu(desc->datalen);
	datalen = le16_to_cpu(desc->datalen);
	e->msg_size = min(datalen, e->msg_size);
	e->msg_len = min(datalen, e->buf_len);
	if (e->msg_buf != NULL && (e->msg_size != 0))
	if (e->msg_buf != NULL && (e->msg_len != 0))
		memcpy(e->msg_buf, hw->aq.arq.r.arq_bi[desc_idx].va,
		memcpy(e->msg_buf, hw->aq.arq.r.arq_bi[desc_idx].va,
		       e->msg_size);
		       e->msg_len);


	i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE, "AQRX: desc and buffer:\n");
	i40e_debug(hw, I40E_DEBUG_AQ_MESSAGE, "AQRX: desc and buffer:\n");
	i40e_debug_aq(hw, I40E_DEBUG_AQ_COMMAND, (void *)desc, e->msg_buf,
	i40e_debug_aq(hw, I40E_DEBUG_AQ_COMMAND, (void *)desc, e->msg_buf,
+2 −1
Original line number Original line Diff line number Diff line
@@ -76,7 +76,8 @@ struct i40e_asq_cmd_details {
/* ARQ event information */
/* ARQ event information */
struct i40e_arq_event_info {
struct i40e_arq_event_info {
	struct i40e_aq_desc desc;
	struct i40e_aq_desc desc;
	u16 msg_size;
	u16 msg_len;
	u16 buf_len;
	u8 *msg_buf;
	u8 *msg_buf;
};
};


+3 −4
Original line number Original line Diff line number Diff line
@@ -5750,13 +5750,12 @@ static void i40e_clean_adminq_subtask(struct i40e_pf *pf)
	if (oldval != val)
	if (oldval != val)
		wr32(&pf->hw, pf->hw.aq.asq.len, val);
		wr32(&pf->hw, pf->hw.aq.asq.len, val);


	event.msg_size = I40E_MAX_AQ_BUF_SIZE;
	event.buf_len = I40E_MAX_AQ_BUF_SIZE;
	event.msg_buf = kzalloc(event.msg_size, GFP_KERNEL);
	event.msg_buf = kzalloc(event.buf_len, GFP_KERNEL);
	if (!event.msg_buf)
	if (!event.msg_buf)
		return;
		return;


	do {
	do {
		event.msg_size = I40E_MAX_AQ_BUF_SIZE; /* reinit each time */
		ret = i40e_clean_arq_element(hw, &event, &pending);
		ret = i40e_clean_arq_element(hw, &event, &pending);
		if (ret == I40E_ERR_ADMIN_QUEUE_NO_WORK)
		if (ret == I40E_ERR_ADMIN_QUEUE_NO_WORK)
			break;
			break;
@@ -5777,7 +5776,7 @@ static void i40e_clean_adminq_subtask(struct i40e_pf *pf)
					le32_to_cpu(event.desc.cookie_high),
					le32_to_cpu(event.desc.cookie_high),
					le32_to_cpu(event.desc.cookie_low),
					le32_to_cpu(event.desc.cookie_low),
					event.msg_buf,
					event.msg_buf,
					event.msg_size);
					event.msg_len);
			break;
			break;
		case i40e_aqc_opc_lldp_update_mib:
		case i40e_aqc_opc_lldp_update_mib:
			dev_dbg(&pf->pdev->dev, "ARQ: Update LLDP MIB event received\n");
			dev_dbg(&pf->pdev->dev, "ARQ: Update LLDP MIB event received\n");
+3 −3
Original line number Original line Diff line number Diff line
@@ -929,10 +929,10 @@ i40e_status i40evf_clean_arq_element(struct i40e_hw *hw,


	e->desc = *desc;
	e->desc = *desc;
	datalen = le16_to_cpu(desc->datalen);
	datalen = le16_to_cpu(desc->datalen);
	e->msg_size = min(datalen, e->msg_size);
	e->msg_len = min(datalen, e->buf_len);
	if (e->msg_buf != NULL && (e->msg_size != 0))
	if (e->msg_buf != NULL && (e->msg_len != 0))
		memcpy(e->msg_buf, hw->aq.arq.r.arq_bi[desc_idx].va,
		memcpy(e->msg_buf, hw->aq.arq.r.arq_bi[desc_idx].va,
		       e->msg_size);
		       e->msg_len);


	if (i40e_is_nvm_update_op(&e->desc))
	if (i40e_is_nvm_update_op(&e->desc))
		hw->aq.nvm_busy = false;
		hw->aq.nvm_busy = false;
+2 −1
Original line number Original line Diff line number Diff line
@@ -76,7 +76,8 @@ struct i40e_asq_cmd_details {
/* ARQ event information */
/* ARQ event information */
struct i40e_arq_event_info {
struct i40e_arq_event_info {
	struct i40e_aq_desc desc;
	struct i40e_aq_desc desc;
	u16 msg_size;
	u16 msg_len;
	u16 buf_len;
	u8 *msg_buf;
	u8 *msg_buf;
};
};


Loading