Hi CK,

Thanks for the reviews.

On Tue, 2023-10-24 at 07:42 +0000, CK Hu (胡俊光) wrote:

Hi, Jason:

On Mon, 2023-10-23 at 12:45 +0800, Jason-JH.Lin wrote:

quoted

To add secure flow support for mediatek-drm, each crtc have to
create a secure cmdq mailbox channel. Then cmdq packets with
display HW configuration will be sent to secure cmdq mailbox
channel
and configured in the secure world.

Each crtc have to use secure cmdq interface to configure some
secure
settings for display HW before sending cmdq packets to secure cmdq
mailbox channel.

If any of fb get from current drm_atomic_state is secure, then crtc
will switch to the secure flow to configure display HW.
If all fbs are not secure in current drm_atomic_state, then crtc
will
switch to the normal flow.

Signed-off-by: Jason-JH.Lin <redacted>
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c  | 272
++++++++++++++++++++++-
 drivers/gpu/drm/mediatek/mtk_drm_crtc.h  |   1 +
 drivers/gpu/drm/mediatek/mtk_drm_plane.c |   7 +
 3 files changed, 269 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c

b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index b6fa4ad2f94d..6c2cf339b923 100644

--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c

@@ -56,6 +56,11 @@ struct mtk_drm_crtc {
 	u32				cmdq_event;
 	u32				cmdq_vblank_cnt;
 	wait_queue_head_t		cb_blocking_queue;
+
+	struct cmdq_client		sec_cmdq_client;
+	struct cmdq_pkt			sec_cmdq_handle;
+	bool				sec_cmdq_working;
+	wait_queue_head_t		sec_cb_blocking_queue;
 #endif
 
 	struct device			*mmsys_dev;

@@ -67,6 +72,7 @@ struct mtk_drm_crtc {
 	/* lock for display hardware access */
 	struct mutex			hw_lock;
 	bool				config_updating;
+	bool				sec_on;
 };
 
 struct mtk_crtc_state {

@@ -109,6 +115,154 @@ static void mtk_drm_finish_page_flip(struct

mtk_drm_crtc *mtk_crtc)
 	}
 }
 
+void mtk_crtc_disable_secure_state(struct drm_crtc *crtc)
+{
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+	enum cmdq_sec_scenario sec_scn = CMDQ_MAX_SEC_COUNT;
+	int i;
+	struct mtk_ddp_comp *ddp_first_comp;
+	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
+	u64 sec_engine = 0; /* for hw engine write output secure fb */
+	u64 sec_port = 0; /* for larb port read input secure fb */
+
+	mutex_lock(&mtk_crtc->hw_lock);
+
+	if (!mtk_crtc->sec_cmdq_client.chan) {
+		pr_err("crtc-%d secure mbox channel is NULL\n",
drm_crtc_index(crtc));
+		goto err;
+	}
+
+	if (!mtk_crtc->sec_on) {
+		pr_debug("crtc-%d is already disabled!\n",
drm_crtc_index(crtc));
+		goto err;
+	}
+
+	mbox_flush(mtk_crtc->sec_cmdq_client.chan, 0);
+	mtk_crtc->sec_cmdq_handle.cmd_buf_size = 0;
+
+	if (mtk_crtc->sec_cmdq_handle.sec_data) {
+		struct cmdq_sec_data *sec_data;
+
+		sec_data = mtk_crtc->sec_cmdq_handle.sec_data;
+		sec_data->addrMetadataCount = 0;
+		sec_data->addrMetadatas = (uintptr_t)NULL;
+	}
+
+	/*
+	 * Secure path only support DL mode, so we just wait
+	 * the first path frame done here
+	 */
+	cmdq_pkt_wfe(&mtk_crtc->sec_cmdq_handle, mtk_crtc->cmdq_event,
false);
+
+	ddp_first_comp = mtk_crtc->ddp_comp[0];
+	for (i = 0; i < mtk_crtc->layer_nr; i++) {
+		struct drm_plane *plane = &mtk_crtc->planes[i];
+
+		sec_port |=
mtk_ddp_comp_layer_get_sec_port(ddp_first_comp, i);
+
+		/* make sure secure layer off before switching secure
state */
+		if (!mtk_plane_fb_is_secure(plane->state->fb)) {
+			struct mtk_plane_state *plane_state =
to_mtk_plane_state(plane->state);
+
+			plane_state->pending.enable = false;
+			mtk_ddp_comp_layer_config(ddp_first_comp, i,
plane_state,
+						  &mtk_crtc-

quoted

sec_cmdq_handle);

You disable layer here and disable secure path in
cmdq_sec_pkt_set_data() later. But this is real world and could be
hacked by hacker. If hacker do not disable layer here but disable
secure path in cmdq_sec_pkt_set_data() later, the hardware would keep
reading secure buffer and path is not secure? That means video could
output to HDMI without HDCP?

Disabling secure path by cmdq_sec_pkt_set_data() will also switch the
larb used by OVL to non-secure identity. So even if the secure layer is
enabled, OVL can not access secure DRAM with non-secure larb.

And it will cause a IOMMU translation fault when non-secure larb access
the address of secure DRAM.

Regards,
Jason-JH.Lin

Regards,
CK 

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