Hi Christian,
On Tue, 2024-05-21 at 20:36 +0200, Christian König wrote:
> Am 20.05.24 um 09:58 schrieb Yong Wu (吴勇):
> > On Thu, 2024-05-16 at 10:17 +0200, Christian König wrote:
> > >
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > Am 15.05.24 um 13:23 schrieb Yong Wu:
> > > > Introduce a FLAG for the restricted memory which means the
> > > > memory
> > >
> > > is
> > > > protected by TEE or hypervisor, then it's inaccessiable for
> > > > kernel.
> > > >
> > > > Currently we don't use sg_dma_unmark_restricted, thus this
> > >
> > > interface
> > > > has not been added.
> > >
> > > Why should that be part of the scatterlist? It doesn't seem to
> > > affect
> > > any of it's functionality.
> > >
> > > As far as I can see the scatterlist shouldn't be the transport of
> > > this
> > > kind of information.
> >
> > Thanks for the review. I will remove this.
> >
> > In our user scenario, DRM will import these buffers and check if
> > this
> > is a restricted buffer. If yes, it will use secure GCE takes over.
> >
> > If this judgment is not suitable to be placed in scatterlist. I
> > don't
> > know if it is ok to limit this inside dma-buf. Adding such an
> > interface:
> >
> > static bool dma_buf_is_restricted(struct dma_buf *dmabuf)
> > {
> > return !strncmp(dmabuf->exp_name, "restricted", 10);
> > }
>
> No, usually stuff like that doesn't belong into DMA buf either.
>
> Question here really is who controls the security status of the
> memory
> backing the buffer?
>
> In other words who tells the exporter that it should allocate and
> fill a
> buffer with encrypted data?
>
> If that is userspace then that is part of the format information and
> it
> is also userspace who should tell the importer that it needs to work
> with encrypted data.
>
> The kernel is intentionally not involved in stuff like that.
>
Here is the expected protected content buffer flow in DRM:
1) userspace allocates a dma-buf FD from the "restricted_mtk_cma" by
DMA_HEAP_IOCTL_ALLOC.
2) userspace imports that dma-buf into the device using prime for the
drm_file.
3) userspace uses the already implemented driver import code for the
special cases of protected content buffer.
In the step 3), we need to verify the dma-buf is allocated from
"restricted_mtk_cma", but there is no way to pass the secure flag or
private data from userspace to the import interface in DRM driver.
So I can only verify it like this now:
struct drm_gem_object *mtk_gem_prime_import_sg_table(struct drm_device
*dev, struct dma_buf_attachment *attach, struct sg_table *sg)
{
struct mtk_gem_obj *mtk_gem;
/* check if the entries in the sg_table are contiguous */
if (drm_prime_get_contiguous_size(sg) < attach->dmabuf->size) {
DRM_ERROR("sg_table is not contiguous");
return ERR_PTR(-EINVAL);
}
mtk_gem = mtk_gem_init(dev, attach->dmabuf->size);
if (IS_ERR(mtk_gem))
return ERR_CAST(mtk_gem);
+ mtk_gem->secure = (!strncmp(attach->dmabuf->exp_name, "restricted",
10));
mtk_gem->dma_addr = sg_dma_address(sg->sgl);
mtk_gem->size = attach->dmabuf->size;
mtk_gem->sg = sg;
return &mtk_gem->base;
}
I think I have the same problem as the ECC_FLAG mention in:
https://lore.kernel.org/linux-media/20240515-dma-buf-ecc-heap-v1-0-54cbbd049511@xxxxxxxxxx/
I think it would be better to have the user configurable private
information in dma-buf, so all the drivers who have the same
requirement can get their private information from dma-buf directly and
no need to change or add the interface.
What's your opinion in this point?
Regards,
Jason-JH.Lin
> Regards,
> Christian.
