linux-kernel - Re: [PATCH v3] drm/etnaviv: Request pages from DMA32 zone on addressing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7a6ffbb773784dee0ea3ee87e563ac4e4f7c9c90.camel@pengutronix.de>
Date: Tue, 01 Oct 2024 14:17:31 +0200
From: Lucas Stach <l.stach@...gutronix.de>
To: Xiaolei Wang <xiaolei.wang@...driver.com>, sui.jingfeng@...ux.dev, 
 linux+etnaviv@...linux.org.uk, christian.gmeiner@...il.com,
 airlied@...il.com,  daniel@...ll.ch
Cc: etnaviv@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] drm/etnaviv: Request pages from DMA32 zone on
 addressing_limited

Hi Xiaolei,

Am Dienstag, dem 03.09.2024 um 10:08 +0800 schrieb Xiaolei Wang:
> Remove __GFP_HIGHMEM when requesting a page from DMA32 zone,
> and since all vivante GPUs in the system will share the same
> DMA constraints, move the check of whether to get a page from
> DMA32 to etnaviv_bind().
> 
> Fixes: b72af445cd38 ("drm/etnaviv: request pages from DMA32 zone when needed")
> Suggested-by: Sui Jingfeng <sui.jingfeng@...ux.dev>
> Signed-off-by: Xiaolei Wang <xiaolei.wang@...driver.com>
> ---
> 
> change log
> 
> v1:
>   https://patchwork.kernel.org/project/dri-devel/patch/20240806104733.2018783-1-xiaolei.wang@windriver.com/
> 
> v2:
>   Modify the issue of not retaining GFP_USER in v1 and update the commit log.
> 
> v3:
>   Use "priv->shm_gfp_mask = GFP_USER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;"
> instead of
>   "priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;"

I don't understand this part of the changes in the new version. Why
should we drop the HIGHMEM bit always and not only in the case where
dma addressing is limited? This seems overly restrictive.

Regards,
Lucas

> and move the check of whether to get a page from DMA32 to etnaviv_bind().
> 
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 10 +++++++++-
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c |  8 --------
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6500f3999c5f..8cb2c3ec8e5d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -536,7 +536,15 @@ static int etnaviv_bind(struct device *dev)
>  	mutex_init(&priv->gem_lock);
>  	INIT_LIST_HEAD(&priv->gem_list);
>  	priv->num_gpus = 0;
> -	priv->shm_gfp_mask = GFP_HIGHUSER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
> +	priv->shm_gfp_mask = GFP_USER | __GFP_RETRY_MAYFAIL | __GFP_NOWARN;
> +
> +	/*
> +	 * If the GPU is part of a system with DMA addressing limitations,
> +	 * request pages for our SHM backend buffers from the DMA32 zone to
> +	 * hopefully avoid performance killing SWIOTLB bounce buffering.
> +	 */
> +	if (dma_addressing_limited(dev))
> +		priv->shm_gfp_mask |= GFP_DMA32;
>  
>  	priv->cmdbuf_suballoc = etnaviv_cmdbuf_suballoc_new(drm->dev);
>  	if (IS_ERR(priv->cmdbuf_suballoc)) {
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 7c7f97793ddd..5e753dd42f72 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -839,14 +839,6 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
>  	if (ret)
>  		goto fail;
>  
> -	/*
> -	 * If the GPU is part of a system with DMA addressing limitations,
> -	 * request pages for our SHM backend buffers from the DMA32 zone to
> -	 * hopefully avoid performance killing SWIOTLB bounce buffering.
> -	 */
> -	if (dma_addressing_limited(gpu->dev))
> -		priv->shm_gfp_mask |= GFP_DMA32;
> -
>  	/* Create buffer: */
>  	ret = etnaviv_cmdbuf_init(priv->cmdbuf_suballoc, &gpu->buffer,
>  				  PAGE_SIZE);