| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <SJ0PR18MB390077BA38838F7BDA2C57C5A1E89@SJ0PR18MB3900.namprd18.prod.outlook.com>
Date: Thu, 22 Dec 2022 09:38:41 +0000
From: Alok Prasad <palok@...vell.com>
To: "csander@...estorage.com" <csander@...estorage.com>,
Ariel Elior <aelior@...vell.com>,
Manish Chopra <manishc@...vell.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC: "joern@...estorage.com" <joern@...estorage.com>,
"csander@...estorage.com" <csander@...estorage.com>
Subject: RE: [PATCH] qed: allow sleep in qed_mcp_trace_dump()
> -----Original Message-----
> From: Caleb Sander <csander@...estorage.com>
> Sent: Saturday, December 17, 2022 7:56 PM
> To: Ariel Elior <aelior@...vell.com>; Manish Chopra
> <manishc@...vell.com>; netdev@...r.kernel.org
> Cc: Joern Engel <joern@...estorage.com>; Caleb Sander
> <csander@...estorage.com>
> Subject: [EXT] [PATCH] qed: allow sleep in qed_mcp_trace_dump()
>
> External Email
>
> ----------------------------------------------------------------------
> By default, qed_mcp_cmd_and_union() waits for 10us at a time in a loop
> that can run 500K times, so calls to qed_mcp_nvm_rd_cmd() may block
> the current thread for over 5s.
> We observed thread scheduling delays of over 700ms in production, with
> stacktraces pointing to this code as the culprit.
>
> qed_mcp_trace_dump() is called from ethtool, so sleeping is permitted.
> It already can sleep in qed_mcp_halt(), which calls qed_mcp_cmd().
> Add a "can sleep" parameter to qed_find_nvram_image() and
> qed_nvram_read() so they can sleep during qed_mcp_trace_dump().
> qed_mcp_trace_get_meta_info() and qed_mcp_trace_read_meta(), called
> only by qed_mcp_trace_dump(), allow these functions to sleep.
> It's not clear to me that the other caller
> (qed_grc_dump_mcp_hw_dump()) can sleep, so it keeps b_can_sleep set to false.
>
> Signed-off-by: Caleb Sander <csander@...estorage.com>
> ---
> drivers/net/ethernet/qlogic/qed/qed_debug.c | 28 +++++++++++++++-----
> -
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_debug.c
> b/drivers/net/ethernet/qlogic/qed/qed_debug.c
> index 86ecb080b153..cdcead614e9f 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_debug.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_debug.c
> @@ -1830,11 +1830,12 @@ static void qed_grc_clear_all_prty(struct
> qed_hwfn *p_hwfn,
> /* Finds the meta data image in NVRAM */ static enum dbg_status
> qed_find_nvram_image(struct qed_hwfn *p_hwfn,
> struct qed_ptt *p_ptt,
> u32 image_type,
> u32 *nvram_offset_bytes,
> - u32 *nvram_size_bytes)
> + u32 *nvram_size_bytes,
> + bool b_can_sleep)
> {
> u32 ret_mcp_resp, ret_mcp_param, ret_txn_size;
> struct mcp_file_att file_att;
> int nvm_result;
>
> @@ -1844,11 +1845,12 @@ static enum dbg_status
> qed_find_nvram_image(struct qed_hwfn *p_hwfn,
>
> DRV_MSG_CODE_NVM_GET_FILE_ATT,
> image_type,
> &ret_mcp_resp,
> &ret_mcp_param,
> &ret_txn_size,
> - (u32 *)&file_att, false);
> + (u32 *)&file_att,
> + b_can_sleep);
>
> /* Check response */
> if (nvm_result || (ret_mcp_resp & FW_MSG_CODE_MASK) !=
> FW_MSG_CODE_NVM_OK)
> return DBG_STATUS_NVRAM_GET_IMAGE_FAILED;
> @@ -1871,11 +1873,13 @@ static enum dbg_status
> qed_find_nvram_image(struct qed_hwfn *p_hwfn,
>
> /* Reads data from NVRAM */
> static enum dbg_status qed_nvram_read(struct qed_hwfn *p_hwfn,
> struct qed_ptt *p_ptt,
> u32 nvram_offset_bytes,
> - u32 nvram_size_bytes, u32 *ret_buf)
> + u32 nvram_size_bytes,
> + u32 *ret_buf,
> + bool b_can_sleep)
> {
> u32 ret_mcp_resp, ret_mcp_param, ret_read_size, bytes_to_copy;
> s32 bytes_left = nvram_size_bytes;
> u32 read_offset = 0, param = 0;
>
> @@ -1897,11 +1901,11 @@ static enum dbg_status qed_nvram_read(struct
> qed_hwfn *p_hwfn,
> if (qed_mcp_nvm_rd_cmd(p_hwfn, p_ptt,
> DRV_MSG_CODE_NVM_READ_NVRAM, param,
> &ret_mcp_resp,
> &ret_mcp_param, &ret_read_size,
> (u32 *)((u8 *)ret_buf + read_offset),
> - false))
> + b_can_sleep))
> return DBG_STATUS_NVRAM_READ_FAILED;
>
> /* Check response */
> if ((ret_mcp_resp & FW_MSG_CODE_MASK) !=
> FW_MSG_CODE_NVM_OK)
> return DBG_STATUS_NVRAM_READ_FAILED; @@ -3378,11 +3382,12 @@
> static u32 qed_grc_dump_mcp_hw_dump(struct qed_hwfn *p_hwfn,
> /* Read HW dump image from NVRAM */
> status = qed_find_nvram_image(p_hwfn,
> p_ptt,
> NVM_TYPE_HW_DUMP_OUT,
> &hw_dump_offset_bytes,
> - &hw_dump_size_bytes);
> + &hw_dump_size_bytes,
> + false);
> if (status != DBG_STATUS_OK)
> return 0;
>
> hw_dump_size_dwords =
> BYTES_TO_DWORDS(hw_dump_size_bytes);
>
> @@ -3395,11 +3400,13 @@ static u32 qed_grc_dump_mcp_hw_dump(struct
> qed_hwfn *p_hwfn,
> /* Read MCP HW dump image into dump buffer */
> if (dump && hw_dump_size_dwords) {
> status = qed_nvram_read(p_hwfn,
> p_ptt,
> hw_dump_offset_bytes,
> - hw_dump_size_bytes, dump_buf +
> offset);
> + hw_dump_size_bytes,
> + dump_buf + offset,
> + false);
> if (status != DBG_STATUS_OK) {
> DP_NOTICE(p_hwfn,
> "Failed to read MCP HW Dump image from NVRAM\n");
> return 0;
> }
> @@ -4121,11 +4128,13 @@ static enum dbg_status
> qed_mcp_trace_get_meta_info(struct qed_hwfn *p_hwfn,
> (*running_bundle_id ==
> DIR_ID_1) ? NVM_TYPE_MFW_TRACE1 :
> NVM_TYPE_MFW_TRACE2;
> return qed_find_nvram_image(p_hwfn,
> p_ptt,
> nvram_image_type,
> - trace_meta_offset, trace_meta_size);
> + trace_meta_offset,
> + trace_meta_size,
> + true);
> }
>
> /* Reads the MCP Trace meta data from NVRAM into the specified buffer
> */ static enum dbg_status qed_mcp_trace_read_meta(struct qed_hwfn
> *p_hwfn,
> struct qed_ptt *p_ptt,
> @@ -4137,11 +4146,14 @@ static enum dbg_status
> qed_mcp_trace_read_meta(struct qed_hwfn *p_hwfn,
> u32 signature;
>
> /* Read meta data from NVRAM */
> status = qed_nvram_read(p_hwfn,
> p_ptt,
> - nvram_offset_in_bytes, size_in_bytes, buf);
> + nvram_offset_in_bytes,
> + size_in_bytes,
> + buf,
> + true);
> if (status != DBG_STATUS_OK)
> return status;
>
> /* Extract and check first signature */
> signature = qed_read_unaligned_dword(byte_buf);
> --
> 2.25.1
Thanks,
Acked-by: Alok Prasad <palok@...vell.com>
Powered by blists - more mailing lists