Am 28.02.2018 um 10:48 schrieb Lucas Stach:
> Hi Christian,
>
> Am Dienstag, den 27.02.2018, 12:49 +0100 schrieb Christian König:
>> Unpin the GEM object only after freeing the sg table.
> What is the race that is being fixed here? The SG table is private to
> the importer and the importer should hopefully only call map_detach if
> it is done with all operations using the SG table. Thus it shouldn't
> matter that the SG table might point to moved pages during execution of
> this function.
Exactly, it shouldn't matter. This is just a precaution.
When the device driver is buggy I want proper error messages from IOMMU
and not accessing pages which might already be reused for something else.
Regards,
Christian.
>
> Regards,
> Lucas
>
>> Signed-off-by: Christian König <christian.koenig at amd.com>
>> ---
>> drivers/gpu/drm/drm_prime.c | 32 ++++++++++++++++----------------
>> 1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_prime.c
>> b/drivers/gpu/drm/drm_prime.c
>> index e82a976f0fba..c38dacda6119 100644
>> --- a/drivers/gpu/drm/drm_prime.c
>> +++ b/drivers/gpu/drm/drm_prime.c
>> @@ -230,26 +230,26 @@ void drm_gem_map_detach(struct dma_buf
>> *dma_buf,
>> struct drm_prime_attachment *prime_attach = attach->priv;
>> struct drm_gem_object *obj = dma_buf->priv;
>> struct drm_device *dev = obj->dev;
>> - struct sg_table *sgt;
>>
>> - if (dev->driver->gem_prime_unpin)
>> - dev->driver->gem_prime_unpin(obj);
>> + if (prime_attach) {
>> + struct sg_table *sgt = prime_attach->sgt;
>>
>> - if (!prime_attach)
>> - return;
>> -
>> - sgt = prime_attach->sgt;
>> - if (sgt) {
>> - if (prime_attach->dir != DMA_NONE)
>> - dma_unmap_sg_attrs(attach->dev, sgt->sgl,
>> sgt->nents,
>> - prime_attach->dir,
>> - DMA_ATTR_SKIP_CPU_SYNC);
>> - sg_free_table(sgt);
>> + if (sgt) {
>> + if (prime_attach->dir != DMA_NONE)
>> + dma_unmap_sg_attrs(attach->dev, sgt-
>>> sgl,
>> + sgt->nents,
>> + prime_attach-
>>> dir,
>> + DMA_ATTR_SKIP_CPU
>> _SYNC);
>> + sg_free_table(sgt);
>> + }
>> +
>> + kfree(sgt);
>> + kfree(prime_attach);
>> + attach->priv = NULL;
>> }
>>
>> - kfree(sgt);
>> - kfree(prime_attach);
>> - attach->priv = NULL;
>> + if (dev->driver->gem_prime_unpin)
>> + dev->driver->gem_prime_unpin(obj);
>> }
>> EXPORT_SYMBOL(drm_gem_map_detach);
>>
