Re: [PATCH v5 2/9] drm/mediatek: Add secure buffer control flow to mtk_drm_gem - Jason-JH Lin (林睿祥)

From: "Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>
To: "Shawn Sung (宋孝謙)" <Shawn.Sung@mediatek.com>,
	"mripard@kernel.org" <mripard@kernel.org>
Cc: "sumit.semwal@linaro.org" <sumit.semwal@linaro.org>,
	"linux-mediatek@lists.infradead.org"
	<linux-mediatek@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linaro-mm-sig@lists.linaro.org" <linaro-mm-sig@lists.linaro.org>,
	"christian.koenig@amd.com" <christian.koenig@amd.com>,
	"chunkuang.hu@kernel.org" <chunkuang.hu@kernel.org>,
	"tzimmermann@suse.de" <tzimmermann@suse.de>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	"daniel@ffwll.ch" <daniel@ffwll.ch>,
	"p.zabel@pengutronix.de" <p.zabel@pengutronix.de>,
	"maarten.lankhorst@linux.intel.com"
	<maarten.lankhorst@linux.intel.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"airlied@gmail.com" <airlied@gmail.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
	"angelogioacchino.delregno@collabora.com"
	<angelogioacchino.delregno@collabora.com>
Subject: Re: [PATCH v5 2/9] drm/mediatek: Add secure buffer control flow to mtk_drm_gem
Date: Tue, 28 May 2024 07:06:40 +0000	[thread overview]
Message-ID: <5ed64ef3eb237cece6efd3e83477fcde93d38a17.camel@mediatek.com> (raw)
In-Reply-To: <20240415-guppy-of-perpetual-current-3a7974@houat>

Hi Maxime,

[snip]

I'm sorry for losing your previous comment mail.
I finally found a way to import this mail back so I can reply to you.

> > -	mtk_gem = mtk_gem_create(dev, args->size, false);
> > +	if (args->flags & DRM_MTK_GEM_CREATE_ENCRYPTED)
> > +		mtk_gem = mtk_gem_create_from_heap(dev, "mtk_svp_cma",
> > args->size);
> 
> That heap doesn't exist upstream either. Also, I'm wondering if it's
> the
> right solution there.
> 

Yes, I found that its name changed to "restricted_mtk_cma" in the
latest patch: 
https://patchwork.kernel.org/project/linux-mediatek/patch/20240515112308.10171-10-yong.wu@mediatek.com/

> From what I can tell, you want to allow to create encrypted buffers
> from
> the TEE. Why do we need this as a DRM ioctl at all? A heap seems like
> the perfect solution to do so, and then you just have to import it
> into
> DRM.
> 

OK, I'll try to change the userspace's ioctl from
DRM_IOCTL_MTK_GEM_CREATE to DMA_HEAP_IOCTL_ALLOC to get the buffer fd,
then import to DRM.

> I'm also not entirely sure that not having a SG list is enough to
> consider the buffer secure. Wouldn't a buffer allocated without a
> kernel
> mapping also be in that situation?
> 

I have confirmed to Yong.Wu that secure buffer also have sg list, so
the secure checking method "!sg_page(sg->sgl)" will be deprecated.

Regards,
Jason-JH.Lin

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