Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] drm/vc4: dma_free_wc() used with wrong device in vc4_bo_purge()
--

commit abd61016c96bce404abb5e31405d8c24a7acfb12
Author: Daniel Drake <[email protected]>

    drm/vc4: Set DRM DMA device directly from HVS and V3D
    
    vc4 uses of_dma_configure() during bind to set the DMA configuration of
    the drm device by borrowing the configuration of one of its component
    devices.

> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index e715147d091ca..305770c87dcf0 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
[ ... ]
> @@ -1662,6 +1663,19 @@ static int vc4_hvs_bind(struct device *dev, struct 
> device *master, void *data)
>               hvs->regset.nregs = ARRAY_SIZE(vc4_hvs_regs);
>       }
>  
> +     if (vc4->gen >= VC4_GEN_6_C) {
> +             ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
> +             if (ret)
> +                     return ret;
> +     }
> +
> +     /*
> +      * Use the HVS as the DRM device's DMA controller for buffer
> +      * allocations if one has not already been configured.
> +      */
> +     if (drm_dev_dma_dev(drm) == drm->dev)
> +             drm_dev_set_dma_dev(drm, dev);

[Severity: High]
Will this change lead to a DMA API regression in vc4_bo_purge()?

Looking at drivers/gpu/drm/vc4/vc4_bo.c, when a DMA allocation fails under
memory pressure, vc4_bo_userspace_cache_purge() calls vc4_bo_purge() to
reclaim memory from BOs marked as DONTNEED.

In vc4_bo_purge(), the memory is freed using the virtual aggregate device
instead of the newly configured DMA device:

static void vc4_bo_purge(struct drm_gem_object *obj)
{
        ...
        dma_free_wc(dev->dev, obj->size, bo->base.vaddr, bo->base.dma_addr);
        ...
}

Since buffer objects are allocated using drm_gem_dma_create(), which now
uses the properly configured drm_dev_dma_dev(drm) device, bypassing GEM
helpers and explicitly calling dma_free_wc(dev->dev, ...) breaks the DMA
API contract.

This contract requires memory to be freed with the exact same device pointer
used for allocation. Using a different device guarantees IOMMU unmap failures,
as the aggregate device no longer has a matching DMA/IOMMU configuration.

Should vc4_bo_purge() be updated to use drm_dev_dma_dev(dev) instead of
dev->dev?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to