Hi,

On Tue, Jun 11, 2024 at 09:13:03AM GMT, Jason-JH Lin (林睿祥) wrote:

Hi Maxime,

[snip]

...

...

...

...

...

---

TODO:

1. Drop MTK_DRM_IOCTL_GEM_CREATE and use DMA_HEAP_IOCTL_ALLOC

in userspace 2) DRM driver use secure mailbox channel to handle normal and secure flow 3) Implement setting mmsys routing table in the secure world series

I'm not sure what you mean here. Why are you trying to upstream something that still needs to be removed from your patch series?

Because their is too much patches need to be fixed in this series, so I list down the remaining TODO items and send to review for the other patches.

Sorry for the bothering, I'll drop this at the next version.

If you don't intend to use it, we just shouldn't add it. Removing the TODO item doesn't make sense, even more so if heaps should be the way you handle this.

Sorry for this misunderstanding.

I mean I'll remove the DRM_IOCTL_GEM_CREATE patch and then change user space calling DMA_HEAP_IOCTL_ALLOC to allocate buffer from secure heap.

I have changed user space to use DMA_HEAP_IOCTL_ALLOC to allocate secure buffer, but I still encounter the problem of determining whether the buffer is secure in mediatek-drm driver to add some secure configure for hardware.

As the comment in [1], dma driver won't provide API for use. [1]: https://patchwork.kernel.org/project/linux-mediatek/patch/20240515112308.101...

So I use name checking at [PATCH v6 3/7] like this currently:

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;

}

But I want to change this name checking to the information brought from user space. I tried to use arg->flags to append the secure flag in user space and call drmPrimeHandleToFD() to pass it to DRM driver, but it will be blocked by at the beginning of the drm_prime_handle_to_fd_ioctl().

I agree with you, it's something to discuss mostly with the dma-buf maintainers but it would be better to just set a flag on the dma-buf, and use that flag whenever necessary.

It might be related to the recent work I did to introduce allocation flags too: https://lore.kernel.org/linux-media/20240515-dma-buf-ecc-heap-v1-0-54cbbd049...

Maxime