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

On Wed, 2023-12-27 at 06:13 +0000, Jason-JH Lin (林睿祥) wrote:

Hi CK,

Thanks for the reviews.

On Tue, 2023-12-26 at 06:46 +0000, CK Hu (胡俊光) wrote:

quoted

Hi, Jason:

On Fri, 2023-12-22 at 12:52 +0800, Jason-JH.Lin wrote:

quoted

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 <redacted>
---

[snip]

quoted

+
+static int cmdq_sec_session_init(struct cmdq_sec_context
*context)
+{
+	int err = 0;
+
+	if (context->state >= IWC_SES_OPENED) {
+		pr_debug("session opened:%u", context->state);
+		return 0;
+	}
+
+	switch (context->state) {
+	case IWC_INIT:
+		err = cmdq_sec_init_context(&context->tee_ctx);
+		if (err)
+			return err;
+		context->state = IWC_CONTEXT_INITED;
+	fallthrough;
+	case IWC_CONTEXT_INITED:
+		if (context->iwc_msg) {
+			pr_err("iwcMessage not NULL:%p", context-

quoted

iwc_msg);

+			return -EINVAL;
+		}
+
+		err = cmdq_sec_allocate_wsm(&context->tee_ctx,
&context->iwc_msg,
+					    sizeof(struct
iwc_cmdq_message_t));
+		if (err)
+			return err;
+
+		context->state = IWC_WSM_ALLOCATED;
+	fallthrough;

Squash cmdq_sec_session_init() into cmdq_sec_mbox_of_xlate() and
drop
the context->state.

Regards,
CK

cmdq_sec_session_init() is called in the first
cmdq_sec_task_submit().
It means we don't need to connect a session to TEE or allocate share
memory if no one submit secure task.

On the other hand, optee may load slower than cmdq secure driver. If
we
move it to xlate secure session may init fail.

For driver probe dependency, you could refer to [1]. I would like
resource allocation and free to be symmetric. So maybe allocate/free in
probe/remove or starup/shutdown. If the secure driver is probed, I
think it would finally allocate secure resource. So later allocation
has no much benefit and make code more complicated. 

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/mediatek?h=next-20231222&id=c8048dd0b07df68724805254b9e994d99e9a7af4

Regards,
CK

Regards,
Jason-JH.Lin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel