linux-kernel - Re: [PATCH RESEND, v6 6/8] 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] [day] [month] [year] [list]

Date: Mon, 17 Jun 2024 07:18:21 +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>,
	"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>, "robh+dt@...nel.org"
	<robh+dt@...nel.org>, "krzysztof.kozlowski+dt@...aro.org"
	<krzysztof.kozlowski+dt@...aro.org>,
	"angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Singo Chang (張興國) <Singo.Chang@...iatek.com>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	Jason-ch Chen (陳建豪)
	<Jason-ch.Chen@...iatek.com>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, Shawn Sung (宋孝謙)
	<Shawn.Sung@...iatek.com>, Nancy Lin (林欣螢)
	<Nancy.Lin@...iatek.com>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.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 RESEND, v6 6/8] mailbox: mediatek: Add CMDQ secure mailbox
 driver

Hi, Jason:

On Sun, 2024-05-26 at 22:44 +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.
> 
> TODO:
> 1. Squash cmdq_sec_task_exec_work() into cmdq_sec_mbox_send_data().
> 2. Call into TEE to query cookie instead of using shared memory in
>    cmdq_sec_get_cookie().
> 3. Register shared memory as command buffer instead of copying normal
>    command buffer to IWC shared memory.
> 4. Use SOFTDEP to make cmdq_sec_probe later than OPTEE loaded and then
>    move cmdq_sec_session_init into cmdq_sec_probe().
> 5. Remove timeout detection in cmdq_sec_session_send().
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
> Signed-off-by: Hsiao Chien Sung <shawn.sung@...iatek.com>
> ---

[snip]

> +static int cmdq_sec_irq_notify_start(struct cmdq_sec *cmdq)
> +{
> +	int err;
> +	dma_addr_t dma_addr;
> +	u64 *inst = NULL;
> +
> +	if (cmdq->notify_run)
> +		return 0;
> +
> +	cmdq->notify_clt.dev = cmdq->pdata->mbox->dev;
> +	cmdq->notify_clt.rx_callback = cmdq_sec_irq_notify_callback;
> +	cmdq->notify_clt.tx_block = false;
> +	cmdq->notify_clt.knows_txdone = true;
> +	cmdq->notify_chan = mbox_request_channel(&cmdq->notify_clt, 0);

When playing secure video, video decoder driver would have interrupt both in normal world and secure world [1].
I guess cmdq driver could also have both interrupt in normal world and secure world.
If so, it's not necessary to create a channel to wait SW token.

[1] https://patchwork.kernel.org/project/linux-mediatek/patch/20240516122102.16379-20-yunfei.dong@mediatek.com/

Regards,
CK

> +	if (IS_ERR(cmdq->notify_chan)) {
> +		dev_err(&cmdq->dev, "failed to request channel\n");
> +		return -ENODEV;
> +	}
> +
> +	cmdq->clt_pkt.va_base = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (!cmdq->clt_pkt.va_base)
> +		return -ENOMEM;
> +
> +	cmdq->clt_pkt.buf_size = PAGE_SIZE;
> +
> +	dma_addr = dma_map_single(cmdq->pdata->mbox->dev, cmdq->clt_pkt.va_base,
> +				  cmdq->clt_pkt.buf_size, DMA_TO_DEVICE);
> +	if (dma_mapping_error(cmdq->pdata->mbox->dev, dma_addr)) {
> +		dev_err(cmdq->pdata->mbox->dev, "dma map failed, size=%lu\n", PAGE_SIZE);
> +		kfree(cmdq->clt_pkt.va_base);
> +		return -ENOMEM;
> +	}
> +	cmdq->clt_pkt.pa_base = dma_addr;
> +
> +	INIT_WORK(&cmdq->irq_notify_work, cmdq_sec_irq_notify_work);
> +
> +	/* generate irq notify loop command */
> +	inst = (u64 *)cmdq->clt_pkt.va_base;
> +	*inst = CMDQ_WFE_CMD(cmdq->pdata->cmdq_event);
> +	inst++;
> +	*inst = CMDQ_EOC_CMD;
> +	inst++;
> +	*inst = CMDQ_JUMP_CMD(cmdq->clt_pkt.pa_base, cmdq->pdata->shift);
> +	inst++;
> +	cmdq->clt_pkt.cmd_buf_size += CMDQ_INST_SIZE * 3;
> +	cmdq->clt_pkt.loop = true;
> +
> +	dma_sync_single_for_device(cmdq->pdata->mbox->dev,
> +				   cmdq->clt_pkt.pa_base,
> +				   cmdq->clt_pkt.cmd_buf_size,
> +				   DMA_TO_DEVICE);
> +	err = mbox_send_message(cmdq->notify_chan, &cmdq->clt_pkt);
> +	mbox_client_txdone(cmdq->notify_chan, 0);
> +	if (err < 0) {
> +		dev_err(&cmdq->dev, "%s failed:%d", __func__, err);
> +		dma_unmap_single(cmdq->pdata->mbox->dev, cmdq->clt_pkt.pa_base,
> +				 cmdq->clt_pkt.buf_size, DMA_TO_DEVICE);
> +		kfree(cmdq->clt_pkt.va_base);
> +		mbox_free_channel(cmdq->notify_chan);
> +
> +		return err;
> +	}
> +
> +	cmdq->notify_run = true;
> +	dev_dbg(&cmdq->dev, "%s success!", __func__);
> +
> +	return 0;
> +}
> +