Re: [PATCH] drm/vmwgfx: Fix scatterlist unmapping

[Thomas Hellstrom <thellstrom@vmware.com>]

Hi, Robin,

Thanks for the patch. It was some time since I put together that code, 
but I remember hitting something similar to

https://www.linuxquestions.org/questions/linux-kernel-70/%27nents%27-argument-of-dma_unmap_sg-4175621964/

Even if it's clear from the documentation that orig_nents should be used.

/Thomas

On 04/13/2018 05:14 PM, Robin Murphy wrote:
> dma_unmap_sg() should be called with the same number of entries
> originally passed to dma_map_sg(), not the number it returned, which may
> be fewer. Admittedly this driver probably never runs on non-coherent
> architectures where getting that wrong could lead to data loss, but it's
> always good to be correct, and it's trivially easy to fix by just
> restoring the SG table state before the call instead of afterwards.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>
> Found by inspection while poking around TTM users.
>
>   drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
> index 21111fd091f9..971223d39469 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
> @@ -369,9 +369,9 @@ static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt *vmw_tt)
>   {
>   	struct device *dev = vmw_tt->dev_priv->dev->dev;
>   
> +	vmw_tt->sgt.nents = vmw_tt->sgt.orig_nents;
>   	dma_unmap_sg(dev, vmw_tt->sgt.sgl, vmw_tt->sgt.nents,
>   		DMA_BIDIRECTIONAL);
> -	vmw_tt->sgt.nents = vmw_tt->sgt.orig_nents;
>   }
>   
>   /**