linux-kernel - Re: [PATCH v3 8/9] mailbox: mediatek: Add CMDQ secure mailbox driver

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 26 Dec 2023 08:58:45 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: "jassisinghbrar@...il.com" <jassisinghbrar@...il.com>,
	"matthias.bgg@...il.com" <matthias.bgg@...il.com>,
	Jason-JH Lin (林睿祥) <Jason-JH.Lin@...iatek.com>,
	"angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>, "robh+dt@...nel.org"
	<robh+dt@...nel.org>, "krzysztof.kozlowski+dt@...aro.org"
	<krzysztof.kozlowski+dt@...aro.org>, "chunkuang.hu@...nel.org"
	<chunkuang.hu@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	Singo Chang (張興國) <Singo.Chang@...iatek.com>,
	Johnson Wang (王聖鑫) <Johnson.Wang@...iatek.com>,
	"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Jason-ch Chen (陳建豪)
	<Jason-ch.Chen@...iatek.com>, Shawn Sung (宋孝謙)
	<Shawn.Sung@...iatek.com>, "mchehab@...nel.org" <mchehab@...nel.org>,
	Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>,
	Project_Global_Chrome_Upstream_Group
	<Project_Global_Chrome_Upstream_Group@...iatek.com>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 8/9] mailbox: mediatek: Add CMDQ secure mailbox driver

Hi, Jason:

On Fri, 2023-12-22 at 12:52 +0800, Jason-JH.Lin wrote:
> To support secure video path feature, GCE have to read/write
> registgers
> in the secure world. GCE will enable the secure access permission to
> the
> HW who wants to access the secure content buffer.
> 
> Add CMDQ secure mailbox driver to make CMDQ client user is able to
> sending their HW settings to the secure world. So that GCE can
> execute
> all instructions to configure HW in the secure world.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
> ---

[snip]

> +
> +static int cmdq_sec_fill_iwc_msg(struct cmdq_sec_context *context,
> +				 struct cmdq_sec_task *sec_task, u32
> thrd_idx)
> +{
> +	struct iwc_cmdq_message_t *iwc_msg = NULL;
> +	struct cmdq_sec_data *data = (struct cmdq_sec_data *)sec_task-
> >task.pkt->sec_data;
> +	u32 size = 0, offset = 0, *instr;
> +
> +	iwc_msg = (struct iwc_cmdq_message_t *)context->iwc_msg;
> +
> +	if (sec_task->task.pkt->cmd_buf_size + 4 * CMDQ_INST_SIZE >
> CMDQ_TZ_CMD_BLOCK_SIZE) {
> +		pr_err("sec_task:%p size:%zu > %u",
> +		       sec_task, sec_task->task.pkt->cmd_buf_size,
> CMDQ_TZ_CMD_BLOCK_SIZE);
> +		return -EFAULT;
> +	}
> +
> +	if (thrd_idx == CMDQ_INVALID_THREAD) {
> +		iwc_msg->command.cmd_size = 0;
> +		iwc_msg->command.metadata.addr_list_length = 0;
> +		return -EINVAL;
> +	}
> +
> +	iwc_msg->command.thread = thrd_idx;
> +	iwc_msg->command.scenario = sec_task->scenario;
> +	iwc_msg->command.eng_flag = sec_task->engine_flag;
> +	size = sec_task->task.pkt->cmd_buf_size;
> +	memcpy(iwc_msg->command.va_base + offset, sec_task->task.pkt-
> >va_base, size);

I think it's not necessary to copy from normal command buffer to normal
command buffer. Just

iwc_msg->command.va_base = sec_task->task.pkt->va_base;

You have a secure command buffer for secure gce to execute command. If
you want to modify command buffer, copy from normal command buffer to
secure command buffer, and modify the secure command buffer.

Regards,
CK


> +	iwc_msg->command.cmd_size += size;
> +	offset += size / 4;
> +
> +	instr = &iwc_msg->command.va_base[iwc_msg->command.cmd_size / 4
> - 4];
> +	if ((u64)*instr == CMDQ_EOC_CMD)
> +		instr[0] = 0;
> +	else
> +		pr_err("%s %d: find EOC failed: %#x %#x",
> +		       __func__, __LINE__, instr[1], instr[0]);
> +
> +	iwc_msg->command.wait_cookie = sec_task->wait_cookie;
> +	iwc_msg->command.reset_exec = sec_task->reset_exec;
> +
> +	if (data->addr_metadata_cnt) {
> +		iwc_msg->command.metadata.addr_list_length = data-
> >addr_metadata_cnt;
> +		memcpy(iwc_msg->command.metadata.addr_list,
> +		       (u32 *)(unsigned long)data->addr_metadatas,
> +		       data->addr_metadata_cnt * sizeof(struct
> iwc_cmdq_addr_metadata_t));
> +	}
> +
> +	iwc_msg->command.metadata.engs_need_dapc = data-
> >engs_need_dapc;
> +	iwc_msg->command.metadata.engs_need_sec_port = data-
> >engs_need_sec_port;
> +	iwc_msg->command.normal_task_handle = (unsigned long)sec_task-
> >task.pkt;
> +
> +	return 0;
> +}
> +