Re: [PATCH RFC 12/18] accel/qda: Add PRIME dma-buf import support

[Christian König]

On 2/23/26 20:09, Ekansh Gupta wrote:
> [Sie erhalten nicht häufig E-Mails von ekansh.gupta@xxxxxxxxxxxxxxxx. Weitere Informationen, warum dies wichtig ist, finden Sie unter https://aka.ms/LearnAboutSenderIdentification ]
> 
> Add PRIME dma-buf import support for QDA GEM buffer objects and integrate
> it with the existing per-process memory manager and IOMMU device model.
> 
> The implementation extends qda_gem_obj to represent imported dma-bufs,
> including dma_buf references, attachment state, scatter-gather tables
> and an imported DMA address used for DSP-facing book-keeping. The
> qda_gem_prime_import() path handles reimports of buffers originally
> exported by QDA as well as imports of external dma-bufs, attaching them
> to the assigned IOMMU device

That is usually an absolutely clear NO-GO for DMA-bufs. Where exactly in the code is that?

> and mapping them through the memory manager
> for DSP access. The GEM free path is updated to unmap and detach
> imported buffers while preserving the existing behaviour for locally
> allocated memory.
> 
> The PRIME fd-to-handle path is implemented in qda_prime_fd_to_handle(),
> which records the calling drm_file in a driver-private import context
> before invoking the core DRM helpers. The GEM import callback retrieves
> this context to ensure that an IOMMU device is assigned to the process
> and that imported buffers follow the same per-process IOMMU selection
> rules as natively allocated GEM objects.
> 
> This patch prepares the driver for interoperable buffer sharing between
> QDA and other dma-buf capable subsystems while keeping IOMMU mapping and
> lifetime handling consistent with the existing GEM allocation flow.
> 
> Signed-off-by: Ekansh Gupta <ekansh.gupta@xxxxxxxxxxxxxxxx>

...

> @@ -15,23 +16,29 @@ static int validate_gem_obj_for_mmap(struct qda_gem_obj *qda_gem_obj)
>                 qda_err(NULL, "Invalid GEM object size\n");
>                 return -EINVAL;
>         }
> -       if (!qda_gem_obj->iommu_dev || !qda_gem_obj->iommu_dev->dev) {
> -               qda_err(NULL, "Allocated buffer missing IOMMU device\n");
> -               return -EINVAL;
> -       }
> -       if (!qda_gem_obj->iommu_dev->dev) {
> -               qda_err(NULL, "Allocated buffer missing IOMMU device\n");
> -               return -EINVAL;
> -       }
> -       if (!qda_gem_obj->virt) {
> -               qda_err(NULL, "Allocated buffer missing virtual address\n");
> -               return -EINVAL;
> -       }
> -       if (qda_gem_obj->dma_addr == 0) {
> -               qda_err(NULL, "Allocated buffer missing DMA address\n");
> -               return -EINVAL;
> +       if (qda_gem_obj->is_imported) {

Absolutely clear NAK to that. Imported buffers *can't* be mmaped through the importer!

Userspace needs to mmap() them through the exporter.

If you absolutely have to map them through the importer for uAPI backward compatibility then there is dma_buf_mmap() for that, but this is clearly not the case here.

...
> +static int qda_memory_manager_map_imported(struct qda_memory_manager *mem_mgr,
> +                                          struct qda_gem_obj *gem_obj,
> +                                          struct qda_iommu_device *iommu_dev)
> +{
> +       struct scatterlist *sg;
> +       dma_addr_t dma_addr;
> +       int ret = 0;
> +
> +       if (!gem_obj->is_imported || !gem_obj->sgt || !iommu_dev) {
> +               qda_err(NULL, "Invalid parameters for imported buffer mapping\n");
> +               return -EINVAL;
> +       }
> +
> +       gem_obj->iommu_dev = iommu_dev;
> +
> +       sg = gem_obj->sgt->sgl;
> +       if (sg) {
> +               dma_addr = sg_dma_address(sg);
> +               dma_addr += ((u64)iommu_dev->sid << 32);
> +
> +               gem_obj->imported_dma_addr = dma_addr;

Well that looks like you are only using the first DMA address from the imported sgt. What about the others?

Regards,
Christian.