From: Xiao Wang Date: Sun, 25 Sep 2016 09:00:12 +0000 (+0800) Subject: net/ixgbe/base: fix possible corruption of shadow RAM X-Git-Tag: spdx-start~5772 X-Git-Url: http://git.droids-corp.org/?a=commitdiff_plain;h=8709f63782e46a921ddeb86dac75c9af24121f7d;p=dpdk.git net/ixgbe/base: fix possible corruption of shadow RAM Currently, not all shadow RAM accesses are being done under the protection of a semaphore, which could result in corruption. Refactor the code so that it is possible to hold the semaphore around ixgbe_host_interface_command by introducing an unlocked form. This patch also eliminates the function ixgbe_read_ee_hostif_data_X550 in favor of the function ixgbe_read_ee_hostif_X550. The new arrangement is able to get both the management interface semaphore and the EEPROM semaphore at the same time instead of separately. Fixes: af75078fece3 ("first public release") Signed-off-by: Xiao Wang Acked-by: Wenzhuo Lu --- diff --git a/drivers/net/ixgbe/base/ixgbe_common.c b/drivers/net/ixgbe/base/ixgbe_common.c index 4c95f81d57..0eb216636e 100644 --- a/drivers/net/ixgbe/base/ixgbe_common.c +++ b/drivers/net/ixgbe/base/ixgbe_common.c @@ -4413,43 +4413,31 @@ u8 ixgbe_calculate_checksum(u8 *buffer, u32 length) } /** - * ixgbe_host_interface_command - Issue command to manageability block + * ixgbe_hic_unlocked - Issue command to manageability block unlocked * @hw: pointer to the HW structure - * @buffer: contains the command to write and where the return status will - * be placed + * @buffer: command to write and where the return status will be placed * @length: length of buffer, must be multiple of 4 bytes * @timeout: time in ms to wait for command completion - * @return_data: read and return data from the buffer (true) or not (false) - * Needed because FW structures are big endian and decoding of - * these fields can be 8 bit or 16 bit based on command. Decoding - * is not easily understood without making a table of commands. - * So we will leave this up to the caller to read back the data - * in these cases. * * Communicates with the manageability block. On success return IXGBE_SUCCESS * else returns semaphore error when encountering an error acquiring * semaphore or IXGBE_ERR_HOST_INTERFACE_COMMAND when command fails. + * + * This function assumes that the IXGBE_GSSR_SW_MNG_SM semaphore is held + * by the caller. **/ -s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, u32 *buffer, - u32 length, u32 timeout, bool return_data) +s32 ixgbe_hic_unlocked(struct ixgbe_hw *hw, u32 *buffer, u32 length, + u32 timeout) { - u32 hicr, i, bi, fwsts; - u32 hdr_size = sizeof(struct ixgbe_hic_hdr); - u16 buf_len; + u32 hicr, i, fwsts; u16 dword_len; - s32 status; - DEBUGFUNC("ixgbe_host_interface_command"); + DEBUGFUNC("ixgbe_hic_unlocked"); - if (length == 0 || length > IXGBE_HI_MAX_BLOCK_BYTE_LENGTH) { + if (!length || length > IXGBE_HI_MAX_BLOCK_BYTE_LENGTH) { DEBUGOUT1("Buffer length failure buffersize=%d.\n", length); return IXGBE_ERR_HOST_INTERFACE_COMMAND; } - /* Take management host interface semaphore */ - status = hw->mac.ops.acquire_swfw_sync(hw, IXGBE_GSSR_SW_MNG_SM); - - if (status) - return status; /* Set bit 9 of FWSTS clearing FW reset indication */ fwsts = IXGBE_READ_REG(hw, IXGBE_FWSTS); @@ -4457,17 +4445,15 @@ s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, u32 *buffer, /* Check that the host interface is enabled. */ hicr = IXGBE_READ_REG(hw, IXGBE_HICR); - if ((hicr & IXGBE_HICR_EN) == 0) { + if (!(hicr & IXGBE_HICR_EN)) { DEBUGOUT("IXGBE_HOST_EN bit disabled.\n"); - status = IXGBE_ERR_HOST_INTERFACE_COMMAND; - goto rel_out; + return IXGBE_ERR_HOST_INTERFACE_COMMAND; } /* Calculate length in DWORDs. We must be DWORD aligned */ - if ((length % (sizeof(u32))) != 0) { + if (length % sizeof(u32)) { DEBUGOUT("Buffer length failure, not aligned to dword"); - status = IXGBE_ERR_INVALID_ARGUMENT; - goto rel_out; + return IXGBE_ERR_INVALID_ARGUMENT; } dword_len = length >> 2; @@ -4490,14 +4476,59 @@ s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, u32 *buffer, } /* Check command completion */ - if ((timeout != 0 && i == timeout) || + if ((timeout && i == timeout) || !(IXGBE_READ_REG(hw, IXGBE_HICR) & IXGBE_HICR_SV)) { ERROR_REPORT1(IXGBE_ERROR_CAUTION, "Command has failed with no status valid.\n"); - status = IXGBE_ERR_HOST_INTERFACE_COMMAND; - goto rel_out; + return IXGBE_ERR_HOST_INTERFACE_COMMAND; } + return IXGBE_SUCCESS; +} + +/** + * ixgbe_host_interface_command - Issue command to manageability block + * @hw: pointer to the HW structure + * @buffer: contains the command to write and where the return status will + * be placed + * @length: length of buffer, must be multiple of 4 bytes + * @timeout: time in ms to wait for command completion + * @return_data: read and return data from the buffer (true) or not (false) + * Needed because FW structures are big endian and decoding of + * these fields can be 8 bit or 16 bit based on command. Decoding + * is not easily understood without making a table of commands. + * So we will leave this up to the caller to read back the data + * in these cases. + * + * Communicates with the manageability block. On success return IXGBE_SUCCESS + * else returns semaphore error when encountering an error acquiring + * semaphore or IXGBE_ERR_HOST_INTERFACE_COMMAND when command fails. + **/ +s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, u32 *buffer, + u32 length, u32 timeout, bool return_data) +{ + u32 hdr_size = sizeof(struct ixgbe_hic_hdr); + u16 dword_len; + u16 buf_len; + s32 status; + u32 bi; + + DEBUGFUNC("ixgbe_host_interface_command"); + + if (length == 0 || length > IXGBE_HI_MAX_BLOCK_BYTE_LENGTH) { + DEBUGOUT1("Buffer length failure buffersize=%d.\n", length); + return IXGBE_ERR_HOST_INTERFACE_COMMAND; + } + + /* Take management host interface semaphore */ + status = hw->mac.ops.acquire_swfw_sync(hw, IXGBE_GSSR_SW_MNG_SM); + if (status) + return status; + + status = ixgbe_hic_unlocked(hw, buffer, length, timeout); + if (status) + goto rel_out; + if (!return_data) goto rel_out; @@ -4512,7 +4543,7 @@ s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, u32 *buffer, /* If there is any thing in data position pull it in */ buf_len = ((struct ixgbe_hic_hdr *)buffer)->buf_len; - if (buf_len == 0) + if (!buf_len) goto rel_out; if (length < buf_len + hdr_size) { diff --git a/drivers/net/ixgbe/base/ixgbe_common.h b/drivers/net/ixgbe/base/ixgbe_common.h index 0545f85c1c..cd04237503 100644 --- a/drivers/net/ixgbe/base/ixgbe_common.h +++ b/drivers/net/ixgbe/base/ixgbe_common.h @@ -159,6 +159,7 @@ s32 ixgbe_set_fw_drv_ver_generic(struct ixgbe_hw *hw, u8 maj, u8 min, u8 ixgbe_calculate_checksum(u8 *buffer, u32 length); s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, u32 *buffer, u32 length, u32 timeout, bool return_data); +s32 ixgbe_hic_unlocked(struct ixgbe_hw *, u32 *buffer, u32 length, u32 timeout); void ixgbe_clear_tx_pending(struct ixgbe_hw *hw); diff --git a/drivers/net/ixgbe/base/ixgbe_x550.c b/drivers/net/ixgbe/base/ixgbe_x550.c index 0cc7a3f23b..6f4dfd9d95 100644 --- a/drivers/net/ixgbe/base/ixgbe_x550.c +++ b/drivers/net/ixgbe/base/ixgbe_x550.c @@ -3192,13 +3192,13 @@ s32 ixgbe_setup_phy_loopback_x550em(struct ixgbe_hw *hw) * * Reads a 16 bit word from the EEPROM using the hostif. **/ -s32 ixgbe_read_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset, - u16 *data) +s32 ixgbe_read_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, u16 *data) { - s32 status; + const u32 mask = IXGBE_GSSR_SW_MNG_SM | IXGBE_GSSR_EEP_SM; struct ixgbe_hic_read_shadow_ram buffer; + s32 status; - DEBUGFUNC("ixgbe_read_ee_hostif_data_X550"); + DEBUGFUNC("ixgbe_read_ee_hostif_X550"); buffer.hdr.req.cmd = FW_READ_SHADOW_RAM_CMD; buffer.hdr.req.buf_lenh = 0; buffer.hdr.req.buf_lenl = FW_READ_SHADOW_RAM_LEN; @@ -3209,42 +3209,18 @@ s32 ixgbe_read_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset, /* one word */ buffer.length = IXGBE_CPU_TO_BE16(sizeof(u16)); - status = ixgbe_host_interface_command(hw, (u32 *)&buffer, - sizeof(buffer), - IXGBE_HI_COMMAND_TIMEOUT, false); - + status = hw->mac.ops.acquire_swfw_sync(hw, mask); if (status) return status; - *data = (u16)IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG, - FW_NVM_DATA_OFFSET); - - return 0; -} - -/** - * ixgbe_read_ee_hostif_X550 - Read EEPROM word using a host interface command - * @hw: pointer to hardware structure - * @offset: offset of word in the EEPROM to read - * @data: word read from the EEPROM - * - * Reads a 16 bit word from the EEPROM using the hostif. - **/ -s32 ixgbe_read_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, - u16 *data) -{ - s32 status = IXGBE_SUCCESS; - - DEBUGFUNC("ixgbe_read_ee_hostif_X550"); - - if (hw->mac.ops.acquire_swfw_sync(hw, IXGBE_GSSR_EEP_SM) == - IXGBE_SUCCESS) { - status = ixgbe_read_ee_hostif_data_X550(hw, offset, data); - hw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_EEP_SM); - } else { - status = IXGBE_ERR_SWFW_SYNC; + status = ixgbe_hic_unlocked(hw, (u32 *)&buffer, sizeof(buffer), + IXGBE_HI_COMMAND_TIMEOUT); + if (!status) { + *data = (u16)IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG, + FW_NVM_DATA_OFFSET); } + hw->mac.ops.release_swfw_sync(hw, mask); return status; } @@ -3260,6 +3236,7 @@ s32 ixgbe_read_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw, u16 offset, u16 words, u16 *data) { + const u32 mask = IXGBE_GSSR_SW_MNG_SM | IXGBE_GSSR_EEP_SM; struct ixgbe_hic_read_shadow_ram buffer; u32 current_word = 0; u16 words_to_read; @@ -3269,7 +3246,7 @@ s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw, DEBUGFUNC("ixgbe_read_ee_hostif_buffer_X550"); /* Take semaphore for the entire operation. */ - status = hw->mac.ops.acquire_swfw_sync(hw, IXGBE_GSSR_EEP_SM); + status = hw->mac.ops.acquire_swfw_sync(hw, mask); if (status) { DEBUGOUT("EEPROM read buffer - semaphore failed\n"); return status; @@ -3289,10 +3266,8 @@ s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw, buffer.address = IXGBE_CPU_TO_BE32((offset + current_word) * 2); buffer.length = IXGBE_CPU_TO_BE16(words_to_read * 2); - status = ixgbe_host_interface_command(hw, (u32 *)&buffer, - sizeof(buffer), - IXGBE_HI_COMMAND_TIMEOUT, - false); + status = ixgbe_hic_unlocked(hw, (u32 *)&buffer, sizeof(buffer), + IXGBE_HI_COMMAND_TIMEOUT); if (status) { DEBUGOUT("Host interface command failed\n"); @@ -3317,7 +3292,7 @@ s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw, } out: - hw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_EEP_SM); + hw->mac.ops.release_swfw_sync(hw, mask); return status; } diff --git a/drivers/net/ixgbe/base/ixgbe_x550.h b/drivers/net/ixgbe/base/ixgbe_x550.h index c7253f0483..36b36f6d78 100644 --- a/drivers/net/ixgbe/base/ixgbe_x550.h +++ b/drivers/net/ixgbe/base/ixgbe_x550.h @@ -55,8 +55,6 @@ s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw, u16 offset, u16 words, u16 *data); s32 ixgbe_read_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, u16 *data); -s32 ixgbe_read_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset, - u16 *data); s32 ixgbe_write_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset, u16 data); s32 ixgbe_set_eee_X550(struct ixgbe_hw *hw, bool enable_eee);