| 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
| ||
|
Date: Wed, 6 Sep 2023 09:33:15 +0000
From: Ming Qian <ming.qian@....com>
To: Hans Verkuil <hverkuil-cisco@...all.nl>,
"mchehab@...nel.org" <mchehab@...nel.org>
CC: "shawnguo@...nel.org" <shawnguo@...nel.org>,
"robh+dt@...nel.org" <robh+dt@...nel.org>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"kernel@...gutronix.de" <kernel@...gutronix.de>,
"festevam@...il.com" <festevam@...il.com>,
dl-linux-imx <linux-imx@....com>,
"X.H. Bao" <xiahong.bao@....com>, Eagle Zhou <eagle.zhou@....com>,
Tao Jiang <tao.jiang_2@....com>,
"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: RE: [EXT] Re: [PATCH v2] media: amphion: handle firmware debug
message
>> decoder firmware may notify host some debug message, it can help
>> analyze the state of the firmware in case of error
>>
>> Fixes: 9f599f351e86 ("media: amphion: add vpu core driver")
>> Signed-off-by: Ming Qian <ming.qian@....com>
>> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
>> ---
>> v2
>> - force to terminate string message data
>> drivers/media/platform/amphion/vpu_defs.h | 1 +
>> drivers/media/platform/amphion/vpu_helpers.c | 1 +
>> drivers/media/platform/amphion/vpu_malone.c | 1 +
>> drivers/media/platform/amphion/vpu_msgs.c | 9 +++++++++
>> drivers/media/platform/amphion/vpu_rpc.c | 3 ++-
>> 5 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/amphion/vpu_defs.h
>> b/drivers/media/platform/amphion/vpu_defs.h
>> index 667637eedb5d..7320852668d6 100644
>> --- a/drivers/media/platform/amphion/vpu_defs.h
>> +++ b/drivers/media/platform/amphion/vpu_defs.h
>> @@ -71,6 +71,7 @@ enum {
>> VPU_MSG_ID_TIMESTAMP_INFO,
>> VPU_MSG_ID_FIRMWARE_XCPT,
>> VPU_MSG_ID_PIC_SKIPPED,
>> + VPU_MSG_ID_DBG_MSG,
>> };
>>
>> enum VPU_ENC_MEMORY_RESOURSE {
>> diff --git a/drivers/media/platform/amphion/vpu_helpers.c
>> b/drivers/media/platform/amphion/vpu_helpers.c
>> index af3b336e5dc3..d12310af9ebc 100644
>> --- a/drivers/media/platform/amphion/vpu_helpers.c
>> +++ b/drivers/media/platform/amphion/vpu_helpers.c
>> @@ -489,6 +489,7 @@ const char *vpu_id_name(u32 id)
>> case VPU_MSG_ID_UNSUPPORTED: return "unsupported";
>> case VPU_MSG_ID_FIRMWARE_XCPT: return "exception";
>> case VPU_MSG_ID_PIC_SKIPPED: return "skipped";
>> + case VPU_MSG_ID_DBG_MSG: return "debug msg";
>> }
>> return "<unknown>";
>> }
>> diff --git a/drivers/media/platform/amphion/vpu_malone.c
>> b/drivers/media/platform/amphion/vpu_malone.c
>> index f771661980c0..d3425de7bccd 100644
>> --- a/drivers/media/platform/amphion/vpu_malone.c
>> +++ b/drivers/media/platform/amphion/vpu_malone.c
>> @@ -745,6 +745,7 @@ static struct vpu_pair malone_msgs[] = {
>> {VPU_MSG_ID_UNSUPPORTED,
>VID_API_EVENT_UNSUPPORTED_STREAM},
>> {VPU_MSG_ID_FIRMWARE_XCPT, VID_API_EVENT_FIRMWARE_XCPT},
>> {VPU_MSG_ID_PIC_SKIPPED, VID_API_EVENT_PIC_SKIPPED},
>> + {VPU_MSG_ID_DBG_MSG, VID_API_EVENT_DBG_MSG_DEC},
>> };
>>
>> static void vpu_malone_pack_fs_alloc(struct vpu_rpc_event *pkt, diff
>> --git a/drivers/media/platform/amphion/vpu_msgs.c
>> b/drivers/media/platform/amphion/vpu_msgs.c
>> index d0ead051f7d1..999efeaebfd5 100644
>> --- a/drivers/media/platform/amphion/vpu_msgs.c
>> +++ b/drivers/media/platform/amphion/vpu_msgs.c
>> @@ -180,6 +180,14 @@ static void vpu_session_handle_pic_skipped(struct
>vpu_inst *inst, struct vpu_rpc
>> vpu_inst_unlock(inst);
>> }
>>
>> +static void vpu_session_handle_dbg_msg(struct vpu_inst *inst, struct
>> +vpu_rpc_event *pkt) {
>> + char *str = (char *)pkt->data;
>> +
>> + if (strlen(str))
>
>This can be replaced by: 'if (*str)', it's faster than strlen.
>
>> + dev_info(inst->dev, "instance %d firmware dbg msg :
>> +%s\n", inst->id, str); }
>> +
>> static struct vpu_msg_handler handlers[] = {
>> {VPU_MSG_ID_START_DONE, vpu_session_handle_start_done},
>> {VPU_MSG_ID_STOP_DONE, vpu_session_handle_stop_done}, @@ -
>196,6
>> +204,7 @@ static struct vpu_msg_handler handlers[] = {
>> {VPU_MSG_ID_UNSUPPORTED, vpu_session_handle_error},
>> {VPU_MSG_ID_FIRMWARE_XCPT, vpu_session_handle_firmware_xcpt},
>> {VPU_MSG_ID_PIC_SKIPPED, vpu_session_handle_pic_skipped},
>> + {VPU_MSG_ID_DBG_MSG, vpu_session_handle_dbg_msg},
>> };
>>
>> static int vpu_session_handle_msg(struct vpu_inst *inst, struct
>> vpu_rpc_event *msg) diff --git
>> a/drivers/media/platform/amphion/vpu_rpc.c
>> b/drivers/media/platform/amphion/vpu_rpc.c
>> index f626a9f835e0..8234370afee5 100644
>> --- a/drivers/media/platform/amphion/vpu_rpc.c
>> +++ b/drivers/media/platform/amphion/vpu_rpc.c
>> @@ -144,7 +144,7 @@ static int vpu_rpc_receive_msg_buf(struct
>vpu_shared_addr *shared, struct vpu_rp
>> msg->hdr.num = (msgword >> 16) & 0xff;
>> msg->hdr.id = msgword & 0x3fff;
>>
>> - if (msg->hdr.num > ARRAY_SIZE(msg->data))
>> + if (msg->hdr.num >= ARRAY_SIZE(msg->data))
>
>Is this right? You are changing the max length just to be able to set the last
>byte to 0, which makes sense for VPU_MSG_ID_DBG_MSG but perhaps not for
>others.
>
>I think it would be better to leave this code as-is and instead set the last byte
>to 0 in vpu_session_handle_dbg_msg(). That's where it is actually needed.
>
>Or am I missing something here?
>
>Regards,
>
> Hans
It will be fixed in v3
>
>> return -EINVAL;
>>
>> for (i = 0; i < msg->hdr.num; i++) { @@ -156,6 +156,7 @@ static
>> int vpu_rpc_receive_msg_buf(struct vpu_shared_addr *shared, struct
>vpu_rp
>> data = shared->msg_mem_vir;
>> }
>> }
>> + msg->data[msg->hdr.num] = 0;
>>
>> /*update rptr after data is read*/
>> mb();
Powered by blists - more mailing lists