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]

Message-ID: <1b21f5bfa561b049193742804de42a1c03500a73.camel@mediatek.com>
Date: Fri, 29 Dec 2023 01:27:00 +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_session_send(struct cmdq_sec_context *context,
> +				 struct cmdq_sec_task *sec_task, const
> u32 iwc_cmd,
> +				 const u32 thrd_idx, struct cmdq_sec
> *cmdq)
> +{
> +	int err = 0;
> +	u64 cost;
> +	struct iwc_cmdq_message_t *iwc_msg = NULL;
> +
> +	iwc_msg = (struct iwc_cmdq_message_t *)context->iwc_msg;
> +
> +	memset(iwc_msg, 0, sizeof(*iwc_msg));
> +	iwc_msg->cmd = iwc_cmd;
> +	iwc_msg->cmdq_id = cmdq->pdata->hwid;
> +	iwc_msg->command.thread = thrd_idx;
> +
> +	switch (iwc_cmd) {
> +	case CMD_CMDQ_IWC_SUBMIT_TASK:
> +		err = cmdq_sec_fill_iwc_msg(context, sec_task,
> thrd_idx);
> +		if (err)
> +			return err;
> +		break;
> +	case CMD_CMDQ_IWC_CANCEL_TASK:
> +		iwc_msg->cancel_task.wait_cookie = sec_task-
> >wait_cookie;
> +		iwc_msg->cancel_task.thread = thrd_idx;
> +		break;
> +	case CMD_CMDQ_IWC_PATH_RES_ALLOCATE:
> +		if (!cmdq->shared_mem || !cmdq->shared_mem->va) {
> +			dev_err(cmdq->mbox.dev, "%s %d: shared_mem is
> NULL", __func__, __LINE__);
> +			return -EFAULT;
> +		}
> +		iwc_msg->path_resource.size = cmdq->shared_mem->size;
> +		iwc_msg->path_resource.share_memoy_pa = cmdq-
> >shared_mem->pa;
> +		iwc_msg->path_resource.use_normal_irq = 1;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	cmdq->sec_invoke = sched_clock();
> +	dev_dbg(cmdq->mbox.dev, "%s execute cmdq:%p sec_task:%p
> command:%u thread:%u cookie:%d",
> +		__func__, cmdq, sec_task, iwc_cmd, thrd_idx,
> +		sec_task ? sec_task->wait_cookie : -1);
> +
> +	/* send message */
> +	err = cmdq_sec_execute_session(&context->tee_ctx, iwc_cmd,
> CMDQ_TIMEOUT_DEFAULT);
> +
> +	cmdq->sec_done = sched_clock();
> +	cost = div_u64(cmdq->sec_done - cmdq->sec_invoke, 1000000);
> +	if (cost >= CMDQ_TIMEOUT_DEFAULT)

Maybe for some client driver, 1 ms is too long, and for some client
driver 1 second is not long. So I think the timeout detection should be
done by client driver. And the execution time depend on the command
buffer generated by client driver, so only client driver has the
ability to debug the command buffer it generated. So it's not necessary
to detect timeout in cmdq driver.

Regards,
CK

> +		dev_err(cmdq->mbox.dev, "%s execute timeout cmdq:%p
> sec_task:%p cost:%lluus",
> +			__func__, cmdq, sec_task, cost);
> +	else
> +		dev_dbg(cmdq->mbox.dev, "%s execute done cmdq:%p
> sec_task:%p cost:%lluus",
> +			__func__, cmdq, sec_task, cost);
> +
> +	if (err)
> +		return err;
> +
> +	context->state = IWC_SES_ON_TRANSACTED;
> +	return 0;
> +}
> +