<!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> Am 25.06.24 um 13:02 schrieb Jason-JH Lin (林睿祥): <blockquote type="cite" cite="mid:1050c44512374031d1349b5dced228d0efc3fbde.camel@mediatek.com"> <pre>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.</pre> </blockquote> What is so special on that case? <blockquote type="cite" cite="mid:1050c44512374031d1349b5dced228d0efc3fbde.camel@mediatek.com"> <pre> 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.</pre> </blockquote> Why do you need to verify that? <blockquote type="cite" cite="mid:1050c44512374031d1349b5dced228d0efc3fbde.camel@mediatek.com"> <pre> 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; }</pre> </blockquote> Complete NAK from my side to that approach. Importing of a DMA-buf should be independent of the exporter. What you could do is to provide the secure buffer from a device and not a device heap. <blockquote type="cite" cite="mid:1050c44512374031d1349b5dced228d0efc3fbde.camel@mediatek.com"> <pre>I think I have the same problem as the ECC_FLAG mention in: <a class="moz-txt-link-freetext" href="https://lore.kernel.org/linux-media/20240515-dma-buf-ecc-heap-v1-0-54cbbd049511@kernel.org/">https://lore.kernel.org/linux-media/20240515-dma-buf-ecc-heap-v1-0-54cbbd049511@kernel.org/</a> 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?</pre> </blockquote> Well of hand I don't see the need for that. What happens if you get a non-secure buffer imported in your secure device? Regards, Christian. <blockquote type="cite" cite="mid:1050c44512374031d1349b5dced228d0efc3fbde.camel@mediatek.com"> <pre> Regards, Jason-JH.Lin > Regards, > Christian. </pre> <pre>************* MEDIATEK Confidentiality Notice ******************** The information contained in this e-mail message (including any attachments) may be confidential, proprietary, privileged, or otherwise exempt from disclosure under applicable laws. It is intended to be conveyed only to the designated recipient(s). Any use, dissemination, distribution, printing, retaining or copying of this e-mail (including its attachments) by unintended recipient(s) is strictly prohibited and may be unlawful. If you are not an intended recipient of this e-mail, or believe that you have received this e-mail in error, please notify the sender immediately (by replying to this e-mail), delete any and all copies of this e-mail (including any attachments) from your system, and do not disclose the content of this e-mail to any other person. Thank you! </pre>   </blockquote> </body> </html>