Re: [PATCH 11/14] media: medkatek: vcodec: covert secure fd to secure handle

quoted

Le mardi 12 septembre 2023 à 01:55 +0000, Yunfei Dong (董云飞) a écrit :
Hi Nicolas,

Thanks for your advice.
On Mon, 2023-09-11 at 11:47 -0400, Nicolas Dufresne wrote:
Hi,

Le lundi 11 septembre 2023 à 20:59 +0800, Yunfei Dong a écrit :
User driver will fill or parse data in optee-os with secure handle,
need to covert secure fd to secure handle in kernel.
A major rework of the wording is needed in this patchset, to fix the
obvious
typos like covert->convert, 
I will fix in next patch.
but also to stop calling dmabuf allocated from
secure heap, secure fd, 
Could you please help to explain it detail?
Whether you meaning I can't
call dma_buf_get/dma_buf_attach functions directly?
This is in regard to comments in the code. I'd suggest to use secure dmabuf fd
rather then just "secure fd", which is ambiguous.


its not precise enough to understand what this patch is
going to be about.
This patch is used to covert secure fd to secure handle.
User space will get a secure fd using dma interface to allocate secure
memory, but need to covert fd to handle in order to fill es
buffer(output queue) in optee-os. I will write the commit message
detail.
When you say "secure fd", it gives readers the impression you have introduced a
new concept of memory object in the linux kernel. In fact, these are dmabuf
object. My suggestion is to rework the accuracy of the naming and wording.

Best Regards,
Yunfei Dong
Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../vcodec/decoder/mtk_vcodec_dec_drv.c       |  1 +
 .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 54
++++++++++++++++++-
 drivers/media/v4l2-core/v4l2-ctrls-defs.c     |  5 ++
 include/uapi/linux/v4l2-controls.h            |  4 ++
 4 files changed, 62 insertions(+), 2 deletions(-)

diff --git
a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
.c
b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
.c
index 0a89ce452ac3..64e006820f43 100644
---
a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
.c
+++
b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv
.c
@@ -571,3 +571,4 @@ module_platform_driver(mtk_vcodec_dec_driver);
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Mediatek video codec V4L2 decoder driver");
+MODULE_IMPORT_NS(DMA_BUF);
diff --git
a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_sta
teless.c
b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_sta
teless.c
index 2ea517883a86..d2b09ce9f1cf 100644
---
a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_sta
teless.c
+++
b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_sta
teless.c
@@ -426,6 +426,46 @@ static int mtk_vcodec_get_pic_info(struct
mtk_vcodec_dec_ctx *ctx)
 	return ret;
 }
 
+static int mtk_dma_contig_get_secure_handle(struct
mtk_vcodec_dec_ctx *ctx, int fd)
+{
+	int secure_handle = 0;
+	struct dma_buf *buf;
+	struct dma_buf_attachment *dba;
+	struct sg_table *sgt;
+	struct device *dev = &ctx->dev->plat_dev->dev;
+
+	buf = dma_buf_get(fd);
+	if (IS_ERR(buf)) {
+		mtk_v4l2_vdec_err(ctx, "dma_buf_get fail fd:%d", fd);
+		return 0;
+	}
+
+	dba = dma_buf_attach(buf, dev);
+	if (IS_ERR(dba)) {
+		mtk_v4l2_vdec_err(ctx, "dma_buf_attach fail fd:%d",
fd);
+		goto err_attach;
+	}
+
+	sgt = dma_buf_map_attachment(dba, DMA_BIDIRECTIONAL);
+	if (IS_ERR(sgt)) {
+		mtk_v4l2_vdec_err(ctx, "dma_buf_map_attachment fail
fd:%d", fd);
+		goto err_map;
+	}
+	secure_handle = sg_dma_address(sgt->sgl);
+
+	dma_buf_unmap_attachment(dba, sgt, DMA_BIDIRECTIONAL);
+	dma_buf_detach(buf, dba);
+	dma_buf_put(buf);
+
+	return secure_handle;
+err_map:
+	dma_buf_detach(buf, dba);
+err_attach:
+	dma_buf_put(buf);
+
+	return 0;
+}
+
 static int mtk_vdec_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct mtk_vcodec_dec_ctx *ctx = ctrl_to_dec_ctx(ctrl);
@@ -436,7 +476,7 @@ static int mtk_vdec_s_ctrl(struct v4l2_ctrl
*ctrl)
 	struct v4l2_ctrl *hdr_ctrl;
 	const struct mtk_vcodec_dec_pdata *dec_pdata = ctx->dev-
vdec_pdata;
 	const struct mtk_video_fmt *fmt;
-	int i = 0, ret = 0;
+	int i = 0, ret = 0, sec_fd;
 
 	hdr_ctrl = ctrl;
 	if (!hdr_ctrl || !hdr_ctrl->p_new.p)
@@ -489,6 +529,12 @@ static int mtk_vdec_s_ctrl(struct v4l2_ctrl
*ctrl)
 			return -EINVAL;
 		}
 		break;
+	case V4L2_CID_MPEG_MTK_GET_SECURE_HANDLE:
+		sec_fd = ctrl->val;
+
+		ctrl->val = mtk_dma_contig_get_secure_handle(ctx, ctrl-
val);
+		mtk_v4l2_vdec_dbg(3, ctx, "get secure handle: %d =>
0x%x", sec_fd, ctrl->val);
+		break;
 	default:
 		mtk_v4l2_vdec_dbg(3, ctx, "Not supported to set ctrl
id: 0x%x\n", hdr_ctrl->id);
 		return ret;
@@ -525,8 +571,9 @@ static const struct v4l2_ctrl_ops
mtk_vcodec_dec_ctrl_ops = {
 static int mtk_vcodec_dec_ctrls_setup(struct mtk_vcodec_dec_ctx
*ctx)
 {
 	unsigned int i;
+	struct v4l2_ctrl *ctrl;
 
-	v4l2_ctrl_handler_init(&ctx->ctrl_hdl, NUM_CTRLS);
+	v4l2_ctrl_handler_init(&ctx->ctrl_hdl, NUM_CTRLS + 1);
 	if (ctx->ctrl_hdl.error) {
 		mtk_v4l2_vdec_err(ctx, "v4l2_ctrl_handler_init
failed\n");
 		return ctx->ctrl_hdl.error;
@@ -543,6 +590,9 @@ static int mtk_vcodec_dec_ctrls_setup(struct
mtk_vcodec_dec_ctx *ctx)
 		}
 	}
 
+	ctrl = v4l2_ctrl_new_std(&ctx->ctrl_hdl,
&mtk_vcodec_dec_ctrl_ops,
+				 V4L2_CID_MPEG_MTK_GET_SECURE_HANDLE,
0, 65535, 1, 0);
+
 	v4l2_ctrl_handler_setup(&ctx->ctrl_hdl);
 
 	return 0;
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index 8696eb1cdd61..d8cf01f76aab 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -1041,6 +1041,7 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_HEVC_SIZE_OF_LENGTH_FIELD:	return
"HEVC Size of Length Field";
 	case V4L2_CID_MPEG_VIDEO_REF_NUMBER_FOR_PFRAMES:	return
"Reference Frames for a P-Frame";
 	case V4L2_CID_MPEG_VIDEO_PREPEND_SPSPPS_TO_IDR:		ret
urn "Prepend SPS and PPS to IDR";
+	case V4L2_CID_MPEG_MTK_GET_SECURE_HANDLE:		return
"MediaTek Decoder get secure handle";
 
 	/* AV1 controls */
 	case V4L2_CID_MPEG_VIDEO_AV1_PROFILE:			ret
urn "AV1 Profile";
@@ -1437,6 +1438,10 @@ void v4l2_ctrl_fill(u32 id, const char
**name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_MPEG_VIDEO_VPX_NUM_REF_FRAMES:
 		*type = V4L2_CTRL_TYPE_INTEGER_MENU;
 		break;
+	case V4L2_CID_MPEG_MTK_GET_SECURE_HANDLE:
+		*type = V4L2_CTRL_TYPE_INTEGER;
+		*flags |= V4L2_CTRL_FLAG_WRITE_ONLY;
+		break;
 	case V4L2_CID_USER_CLASS:
 	case V4L2_CID_CAMERA_CLASS:
 	case V4L2_CID_CODEC_CLASS:
diff --git a/include/uapi/linux/v4l2-controls.h
b/include/uapi/linux/v4l2-controls.h
index c3604a0a3e30..7b3694985366 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -954,6 +954,10 @@ enum v4l2_mpeg_mfc51_video_force_frame_type {
 #define V4L2_CID_MPEG_MFC51_VIDEO_H264_ADAPTIVE_RC_STATIC		
(V4L2_CID_CODEC_MFC51_BASE+53)
 #define V4L2_CID_MPEG_MFC51_VIDEO_H264_NUM_REF_PIC_FOR_P		
(V4L2_CID_CODEC_MFC51_BASE+54)
 
+/*  MPEG-class control IDs specific to the MediaTek Decoder driver
as defined by V4L2 */
+#define V4L2_CID_MPEG_MTK_BASE			(V4L2_CTRL_CLAS
S_CODEC | 0x2000)
+#define V4L2_CID_MPEG_MTK_GET_SECURE_HANDLE	(V4L2_CID_MPEG_
MTK_BASE+8)
+
 /*  Camera class control IDs */
 
 #define V4L2_CID_CAMERA_CLASS_BASE	(V4L2_CTRL_CLASS_CAMERA |
0x900)


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

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help