On 2/24/2026 2:42 PM, Christian König wrote:
> On 2/23/26 20:09, Ekansh Gupta wrote:
>> [Sie erhalten nicht häufig E-Mails von [email protected].
>> 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?
dma_buf_attach* to comute-cb iommu devices are critical for DSPs to access the
buffer.
This is needed if the buffer is exported by anyone other than QDA(say system
heap). If this is not
the correct way, what should be the right way here? On the current fastrpc
driver also,
the DMABUF is getting attached with iommu device[1] due to the same requirement.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/misc/fastrpc.c#n779
>
>> 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 <[email protected]>
> ...
>
>> @@ -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.
>
> ...
Okay, the requirement is slightly different here. Any buffer which is not
allocated using the
QDA GEM interface needs to be attached to the iommu device for that particular
process to
enable DSP for the access. I should not call it `mmap` instead it should be
called importing the
buffer to a particular iommu context bank. With this definition, is it fine to
keep it this way? Or
should the dma_buf_attach* calls be moved to some other place?
>> +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?
I might have a proper appach for this now, will update in the next spin.
>
> Regards,
> Christian.
