From d56214d30df99f8822e757d5d64be31f5e2f4dd5 Mon Sep 17 00:00:00 2001 From: Karthik Anantha Ram Date: Fri, 28 Apr 2023 15:29:30 -0700 Subject: [PATCH] msm: camera: icp: Debug queue updates Reduce size of the buffer to drain the dbg_q. Add a mutex for dbg_q, and in HFI read validate the input buffer size prior to copying the queue contents into the input buffer. CRs-Fixed: 3477543 Change-Id: I2043f3db6189ebfc8b8ead8db0266a83bc94b6a2 Signed-off-by: Karthik Anantha Ram --- drivers/cam_icp/fw_inc/hfi_intf.h | 9 ++- drivers/cam_icp/fw_inc/hfi_reg.h | 18 +++--- drivers/cam_icp/hfi.c | 62 ++++++++++++++----- .../icp_hw/icp_hw_mgr/cam_icp_hw_mgr.c | 6 +- .../icp_hw/icp_hw_mgr/cam_icp_hw_mgr.h | 16 +++-- 5 files changed, 79 insertions(+), 32 deletions(-) diff --git a/drivers/cam_icp/fw_inc/hfi_intf.h b/drivers/cam_icp/fw_inc/hfi_intf.h index 77d83bcc7e..79dfeb56c1 100644 --- a/drivers/cam_icp/fw_inc/hfi_intf.h +++ b/drivers/cam_icp/fw_inc/hfi_intf.h @@ -79,12 +79,14 @@ struct hfi_ops { * @msg_q: msg queue * @msg_q_state: msg queue state * @cmd_q_state: cmd queue state + * @dbg_q_state: dbg queue state */ struct hfi_mini_dump_info { uint32_t cmd_q[HFI_CMD_Q_MINI_DUMP_SIZE_IN_BYTES]; uint32_t msg_q[HFI_MSG_Q_MINI_DUMP_SIZE_IN_BYTES]; bool msg_q_state; bool cmd_q_state; + bool dbg_q_state; }; /** * hfi_write_cmd() - function for hfi write @@ -99,13 +101,15 @@ int hfi_write_cmd(int client_handle, void *cmd_ptr); * hfi_read_message() - function for hfi read * @client_handle: client handle * @pmsg: buffer to place read message for hfi queue - * @q_id: queue id + * @q_id: Queue to read from + * @buf_words_size: size in words of the input buffer * @words_read: total number of words read from the queue * returned as output to the caller * * Returns success(zero)/failure(non zero) */ -int hfi_read_message(int client_handle, uint32_t *pmsg, uint8_t q_id, uint32_t *words_read); +int hfi_read_message(int client_handle, uint32_t *pmsg, uint8_t q_id, + size_t buf_words_size, uint32_t *words_read); /** * hfi_init() - function initialize hfi after firmware download @@ -243,5 +247,4 @@ void cam_hfi_queue_dump(int client_handle, bool dump_queue_data); * @dst: memory destination */ void cam_hfi_mini_dump(int client_handle, struct hfi_mini_dump_info *dst); - #endif /* _HFI_INTF_H_ */ diff --git a/drivers/cam_icp/fw_inc/hfi_reg.h b/drivers/cam_icp/fw_inc/hfi_reg.h index e4d06a0217..1fbdd96bc2 100644 --- a/drivers/cam_icp/fw_inc/hfi_reg.h +++ b/drivers/cam_icp/fw_inc/hfi_reg.h @@ -274,15 +274,17 @@ struct hfi_qtbl { * @smem_size: Shared memory size * @uncachedheap_size: uncached heap size * @msgpacket_buf: message buffer - * @hfi_state: State machine for hfi * @cmd_q_lock: Lock for command queue - * @cmd_q_state: State of command queue - * @mutex msg_q_lock: Lock for message queue - * @msg_q_state: State of message queue + * @msg_q_lock: Lock for message queue + * @dbg_q_lock: Lock for debug queue + * @hfi_state: State machine for hfi * @priv: device private data * @dbg_lvl: debug level set to FW * @fw_version: firmware version * @client_name: hfi client's name + * @cmd_q_state: State of command queue + * @msg_q_state: State of message queue + * @dbg_q_state: State of debug queue */ struct hfi_info { struct hfi_mem_info map; @@ -290,15 +292,17 @@ struct hfi_info { uint32_t smem_size; uint32_t uncachedheap_size; uint32_t msgpacket_buf[ICP_HFI_MAX_MSG_SIZE_IN_WORDS]; - uint8_t hfi_state; struct mutex cmd_q_lock; - bool cmd_q_state; struct mutex msg_q_lock; - bool msg_q_state; + struct mutex dbg_q_lock; + uint8_t hfi_state; void *priv; u64 dbg_lvl; uint32_t fw_version; char client_name[HFI_CLIENT_NAME_LEN]; + bool cmd_q_state; + bool msg_q_state; + bool dbg_q_state; }; #endif /* _CAM_HFI_REG_H_ */ diff --git a/drivers/cam_icp/hfi.c b/drivers/cam_icp/hfi.c index 878a327008..819f4c62ae 100644 --- a/drivers/cam_icp/hfi.c +++ b/drivers/cam_icp/hfi.c @@ -147,6 +147,7 @@ void cam_hfi_mini_dump(int client_handle, struct hfi_mini_dump_info *dst) memcpy(dst->msg_q, dwords, ICP_CMD_Q_SIZE_IN_BYTES); dst->msg_q_state = hfi->msg_q_state; dst->cmd_q_state = hfi->cmd_q_state; + dst->dbg_q_state = hfi->dbg_q_state; } void cam_hfi_queue_dump(int client_handle, bool dump_queue_data) @@ -302,7 +303,7 @@ err: } int hfi_read_message(int client_handle, uint32_t *pmsg, uint8_t q_id, - uint32_t *words_read) + size_t buf_words_size, uint32_t *words_read) { struct hfi_info *hfi; struct hfi_qtbl *q_tbl_ptr; @@ -310,6 +311,7 @@ int hfi_read_message(int client_handle, uint32_t *pmsg, uint8_t q_id, uint32_t new_read_idx, size_in_words, word_diff, temp; uint32_t *read_q, *read_ptr, *write_ptr; uint32_t size_upper_bound = 0; + struct mutex *q_lock; int rc = 0; rc = hfi_get_client_info(client_handle, &hfi); @@ -325,13 +327,19 @@ int hfi_read_message(int client_handle, uint32_t *pmsg, uint8_t q_id, return -EINVAL; } - if (!((q_id == Q_MSG) || (q_id == Q_DBG))) { - CAM_ERR(CAM_HFI, "[%s] Invalid q :%u", - hfi->client_name, q_id); + switch (q_id) { + case Q_MSG: + q_lock = &hfi->msg_q_lock; + break; + case Q_DBG: + q_lock = &hfi->dbg_q_lock; + break; + default: + CAM_ERR(CAM_HFI, "Invalid q_id: %u for read", q_id); return -EINVAL; } - mutex_lock(&hfi->msg_q_lock); + mutex_lock(q_lock); if (hfi->hfi_state != HFI_READY || !hfi->msg_q_state) { CAM_ERR(CAM_HFI, "[%s] Invalid hfi state:%u msg q state: %u hfi hdl: %d", @@ -378,6 +386,14 @@ int hfi_read_message(int client_handle, uint32_t *pmsg, uint8_t q_id, goto err; } + if (size_in_words > buf_words_size) { + CAM_ERR(CAM_HFI, + "[%s] hdl: %d Size of buffer: %u is smaller than size to read from queue: %u", + hfi->client_name, client_handle, buf_words_size, size_in_words); + rc = -EIO; + goto err; + } + new_read_idx = q->qhdr_read_idx + size_in_words; if (new_read_idx < q->qhdr_q_size) { @@ -397,9 +413,10 @@ int hfi_read_message(int client_handle, uint32_t *pmsg, uint8_t q_id, */ wmb(); err: - mutex_unlock(&hfi->msg_q_lock); + mutex_unlock(q_lock); return rc; } + #endif /* #ifndef CONFIG_CAM_PRESIL */ int hfi_cmd_ubwc_config(int client_handle, uint32_t *ubwc_cfg) @@ -902,6 +919,7 @@ int cam_hfi_init(int client_handle, struct hfi_mem_info *hfi_mem, mutex_lock(&hfi->cmd_q_lock); mutex_lock(&hfi->msg_q_lock); + mutex_lock(&hfi->dbg_q_lock); hfi->hfi_state = HFI_INIT; memcpy(&hfi->map, hfi_mem, sizeof(hfi->map)); @@ -1109,18 +1127,21 @@ int cam_hfi_init(int client_handle, struct hfi_mem_info *hfi_mem, hfi->cmd_q_state = true; hfi->msg_q_state = true; + hfi->dbg_q_state = true; hfi->hfi_state = HFI_READY; hfi_irq_enable(hfi); - mutex_unlock(&hfi->cmd_q_lock); + mutex_unlock(&hfi->dbg_q_lock); mutex_unlock(&hfi->msg_q_lock); + mutex_unlock(&hfi->cmd_q_lock); return rc; regions_fail: - mutex_unlock(&hfi->cmd_q_lock); + mutex_unlock(&hfi->dbg_q_lock); mutex_unlock(&hfi->msg_q_lock); + mutex_unlock(&hfi->cmd_q_lock); return rc; } @@ -1145,11 +1166,14 @@ void cam_hfi_deinit(int client_handle) mutex_lock(&hfi->cmd_q_lock); mutex_lock(&hfi->msg_q_lock); + mutex_lock(&hfi->dbg_q_lock); hfi->hfi_state = HFI_DEINIT; hfi->cmd_q_state = false; hfi->msg_q_state = false; + hfi->dbg_q_state = false; + mutex_unlock(&hfi->dbg_q_lock); mutex_unlock(&hfi->cmd_q_lock); mutex_unlock(&hfi->msg_q_lock); @@ -1229,6 +1253,7 @@ int cam_hfi_register(int *client_handle, const char *client_name) mutex_init(&hfi->cmd_q_lock); mutex_init(&hfi->msg_q_lock); + mutex_init(&hfi->dbg_q_lock); return rc; @@ -1253,6 +1278,7 @@ int cam_hfi_unregister(int *client_handle) } mutex_lock(&g_hfi_lock); + mutex_destroy(&hfi->dbg_q_lock); mutex_destroy(&hfi->msg_q_lock); mutex_destroy(&hfi->cmd_q_lock); cam_free_clear((void *)hfi); @@ -1334,10 +1360,11 @@ int hfi_write_cmd(int client_handle, void *cmd_ptr) } int hfi_read_message(int client_handle, uint32_t *pmsg, uint8_t q_id, - uint32_t *words_read) + size_t buf_words_size, uint32_t *words_read) { struct hfi_info *hfi; int presil_rc = CAM_PRESIL_BLOCKED; + struct mutex *q_lock = NULL; int rc = 0; rc = hfi_get_client_info(client_handle, &hfi); @@ -1353,12 +1380,19 @@ int hfi_read_message(int client_handle, uint32_t *pmsg, uint8_t q_id, return -EINVAL; } - if (q_id > Q_DBG) { - CAM_ERR(CAM_HFI, "[%s] Invalid q :%u hdl: %d", - hfi->client_name, q_id, client_handle); + switch (q_id) { + case Q_MSG: + q_lock = &hfi->msg_q_lock; + break; + case Q_DBG: + q_lock = &hfi->dbg_q_lock; + break; + default: + CAM_ERR(CAM_HFI, "Invalid q_id: %u for read", q_id); return -EINVAL; } - mutex_lock(&hfi->msg_q_lock); + + mutex_lock(q_lock); memset(pmsg, 0x0, sizeof(uint32_t) * 256 /* ICP_MSG_BUF_SIZE */); *words_read = 0; @@ -1375,7 +1409,7 @@ int hfi_read_message(int client_handle, uint32_t *pmsg, uint8_t q_id, hfi->client_name, client_handle, presil_rc); } - mutex_unlock(&hfi->msg_q_lock); + mutex_unlock(q_lock); return rc; } #else diff --git a/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.c b/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.c index a668c2c9a8..8dbdb00f56 100644 --- a/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.c +++ b/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.c @@ -2971,7 +2971,8 @@ static void cam_icp_mgr_process_dbg_buf(struct cam_icp_hw_mgr *hw_mgr) char *dbg_buf; int rc = 0; - rc = hfi_read_message(hw_mgr->hfi_handle, hw_mgr->dbg_buf, Q_DBG, &read_len); + rc = hfi_read_message(hw_mgr->hfi_handle, hw_mgr->dbg_buf, Q_DBG, + ICP_DBG_BUF_SIZE_IN_WORDS, &read_len); if (rc) return; @@ -3101,7 +3102,8 @@ static int32_t cam_icp_mgr_process_msg(void *priv, void *data) task_data = data; hw_mgr = priv; - rc = hfi_read_message(hw_mgr->hfi_handle, hw_mgr->msg_buf, Q_MSG, &read_len); + rc = hfi_read_message(hw_mgr->hfi_handle, hw_mgr->msg_buf, Q_MSG, + ICP_MSG_BUF_SIZE_IN_WORDS, &read_len); if (rc) { CAM_DBG(CAM_ICP, "Unable to read msg q rc %d", rc); } else { diff --git a/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.h b/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.h index b93cb0af9b..f5a5d51b8e 100644 --- a/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.h +++ b/drivers/cam_icp/icp_hw/icp_hw_mgr/cam_icp_hw_mgr.h @@ -41,8 +41,10 @@ #define ICP_FRAME_PROCESS_SUCCESS 0 #define ICP_FRAME_PROCESS_FAILURE 1 -#define ICP_MSG_BUF_SIZE 256 -#define ICP_DBG_BUF_SIZE 102400 + +/* size of buffer to drain from msg/dbq queue */ +#define ICP_MSG_BUF_SIZE_IN_WORDS 512 +#define ICP_DBG_BUF_SIZE_IN_WORDS 25600 #define ICP_OVER_CLK_THRESHOLD 5 #define ICP_TWO_DEV_BW_SHARE_RATIO 2 @@ -425,8 +427,10 @@ struct cam_icp_hw_ctx_data { * @cmd_work: Work queue for hfi commands * @msg_work: Work queue for hfi messages * @timer_work: Work queue for timer watchdog - * @msg_buf: Buffer for message data from firmware - * @dbg_buf: Buffer for debug data from firmware + * @msg_buf: Drain Buffer for message data from firmware + * Buffer is an array of type __u32, total size + * would be sizeof(_u32) * queue_size + * @dbg_buf: Drain Buffer for debug data from firmware * @icp_complete: Completion info * @cmd_work_data: Pointer to command work queue task * @msg_work_data: Pointer to message work queue task @@ -479,8 +483,8 @@ struct cam_icp_hw_mgr { struct cam_req_mgr_core_workq *cmd_work; struct cam_req_mgr_core_workq *msg_work; struct cam_req_mgr_core_workq *timer_work; - uint32_t msg_buf[ICP_MSG_BUF_SIZE]; - uint32_t dbg_buf[ICP_DBG_BUF_SIZE]; + uint32_t msg_buf[ICP_MSG_BUF_SIZE_IN_WORDS]; + uint32_t dbg_buf[ICP_DBG_BUF_SIZE_IN_WORDS]; struct completion icp_complete; struct hfi_cmd_work_data *cmd_work_data; struct hfi_msg_work_data *msg_work_data;