From patchwork Sun Sep 25 09:00:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xiao Wang X-Patchwork-Id: 16096 X-Patchwork-Delegate: bruce.richardson@intel.com Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id BF94291A0; Sun, 25 Sep 2016 11:02:04 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 2B52B5323 for ; Sun, 25 Sep 2016 11:01:53 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP; 25 Sep 2016 02:01:53 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos; i="5.30,392,1470726000"; d="scan'208"; a="1045607516" Received: from shvmail01.sh.intel.com ([10.239.29.42]) by fmsmga001.fm.intel.com with ESMTP; 25 Sep 2016 02:01:52 -0700 Received: from shecgisg004.sh.intel.com (shecgisg004.sh.intel.com [10.239.29.89]) by shvmail01.sh.intel.com with ESMTP id u8P91oXs019156; Sun, 25 Sep 2016 17:01:50 +0800 Received: from shecgisg004.sh.intel.com (localhost [127.0.0.1]) by shecgisg004.sh.intel.com (8.13.6/8.13.6/SuSE Linux 0.8) with ESMTP id u8P91k0S006178; Sun, 25 Sep 2016 17:01:48 +0800 Received: (from xiaowan1@localhost) by shecgisg004.sh.intel.com (8.13.6/8.13.6/Submit) id u8P91kFk006174; Sun, 25 Sep 2016 17:01:46 +0800 From: Xiao Wang To: wenzhuo.lu@intel.com Cc: dev@dpdk.org, ferruh.yigit@intel.com, Xiao Wang Date: Sun, 25 Sep 2016 17:00:12 +0800 Message-Id: <1474794017-5896-36-git-send-email-xiao.w.wang@intel.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1474794017-5896-1-git-send-email-xiao.w.wang@intel.com> References: <1472312902-16963-2-git-send-email-xiao.w.wang@intel.com> <1474794017-5896-1-git-send-email-xiao.w.wang@intel.com> Subject: [dpdk-dev] [PATCH v2 35/40] net/ixgbe/base: hold semaphore for shadow RAM access X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" The semaphore is not being held for complete shadow RAM accesses 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 and it now gets both semaphore bits at once instead of nesting them. The new arrangement is able to get both the management interface and the EEPROM semaphores at the same time instead of separately. Signed-off-by: Xiao Wang --- v2: - Removed some whitespace changes to make this patch clean. All minor cleanups were put into 39/40 patch. --- drivers/net/ixgbe/base/ixgbe_common.c | 95 +++++++++++++++++++++++------------ drivers/net/ixgbe/base/ixgbe_common.h | 1 + drivers/net/ixgbe/base/ixgbe_x550.c | 57 ++++++--------------- drivers/net/ixgbe/base/ixgbe_x550.h | 2 - 4 files changed, 80 insertions(+), 75 deletions(-) diff --git a/drivers/net/ixgbe/base/ixgbe_common.c b/drivers/net/ixgbe/base/ixgbe_common.c index 4c95f81..0eb2166 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 0545f85..cd04237 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 0cc7a3f..6f4dfd9 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 c7253f0..36b36f6 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);