linux-kernel - Re: [PATCH] drm/etnaviv: Fix flush sequence logic

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <05552657cac9395b83d454e2ede05271d4c15abc.camel@pengutronix.de>
Date: Wed, 07 May 2025 14:07:22 +0200
From: Lucas Stach <l.stach@...gutronix.de>
To: Tomeu Vizoso <tomeu@...euvizoso.net>, linux-kernel@...r.kernel.org
Cc: Russell King <linux+etnaviv@...linux.org.uk>, Christian Gmeiner
 <christian.gmeiner@...il.com>, David Airlie <airlied@...il.com>, Simona
 Vetter <simona@...ll.ch>, etnaviv@...ts.freedesktop.org, 
 dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] drm/etnaviv: Fix flush sequence logic

Am Mittwoch, dem 07.05.2025 um 13:21 +0200 schrieb Tomeu Vizoso:
> We should be comparing the last submitted sequence number with that of
> the address space we may be switching to.
> 
> And we should be using the latter as the last submitted sequence number
> afterwards.
> 
> Signed-off-by: Tomeu Vizoso <tomeu@...euvizoso.net>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_buffer.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> index b13a17276d07..865b07b14b38 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_buffer.c
> @@ -347,7 +347,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state,
>  	u32 link_target, link_dwords;
>  	bool switch_context = gpu->exec_state != exec_state;
>  	bool switch_mmu_context = gpu->mmu_context != mmu_context;
> -	unsigned int new_flush_seq = READ_ONCE(gpu->mmu_context->flush_seq);
> +	unsigned int new_flush_seq = READ_ONCE(mmu_context->flush_seq);
>  	bool need_flush = switch_mmu_context || gpu->flush_seq != new_flush_seq;
>  	bool has_blt = !!(gpu->identity.minor_features5 &
>  			  chipMinorFeatures5_BLT_ENGINE);
> @@ -399,6 +399,7 @@ void etnaviv_buffer_queue(struct etnaviv_gpu *gpu, u32 exec_state,
>  			struct etnaviv_iommu_context *old_context = gpu->mmu_context;
>  
>  			gpu->mmu_context = etnaviv_iommu_context_get(mmu_context);
> +			gpu->flush_seq = new_flush_seq;

As I said in IRC: this should not be necessary. If we switch to a new
MMU context, need_flush will also be set. The code under the need_flush
condition already includes this exact line.

Regards,
Lucas