[PATCH 0/2] drm/amdgpu: fix MES queue init wptr reset on atomic64 carriers

[PATCH 0/2] drm/amdgpu: fix MES queue init wptr reset on atomic64 carriers

[Runyu Xiao]

This series fixes two MES queue-init reset paths that still clear
ring->wptr_cpu_addr with a plain 32-bit store even though the same
carrier is accessed with atomic64_set()/atomic64_read() and
support_64bit_ptrs is enabled.

This is not just a missing atomic annotation. The write pointer carrier
is used as a shared 64-bit value, so a plain *ring->wptr_cpu_addr = 0
only clears the low 32 bits. A later atomic64_read() can then observe
stale high 32 bits instead of a real zeroed reset state.

Use atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0) in both reset
paths so the full 64-bit MES wptr is cleared with the same access
family as the existing readers and writers.

This issue was first flagged by our static analysis tool while scanning
for 64-bit atomic write pointer carriers mixed with plain reset stores,
then manually audited on Linux v6.18.21.

It was further confirmed with a QEMU no-device stand-in that preserved
the same access contract: atomic64_set() write, plain low-32 reset, and
atomic64_read() readback. In that stand-in, a buggy reset turned
0x12345678abcdeff0 into 0x1234567800000000, while replacing the reset
with atomic64_set(..., 0) read back 0x0.

Build-tested by compiling mes_v11_0.o and mes_v12_0.o.

No AMDGPU hardware was available for end-to-end runtime testing.

Runyu Xiao (2):
  drm/amdgpu/mes11: fix queue init wptr reset
  drm/amdgpu/mes12: fix queue init wptr reset

 drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/2] drm/amdgpu/mes11: fix queue init wptr reset

[Runyu Xiao]

mes_v11_0_queue_init() resets ring->wptr_cpu_addr with a plain 32-bit
store in the reset/suspend path even though the same carrier is
accessed with atomic64_set()/atomic64_read() and support_64bit_ptrs is
enabled.

This is not just a missing atomic annotation. The MES queue write
pointer is a shared 64-bit carrier, and *ring->wptr_cpu_addr = 0 only
clears the low 32 bits. A later atomic64_read() can then observe stale
high 32 bits instead of a real zeroed reset state.

Use atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0) so the reset
path updates the full 64-bit wptr with the same access family as the
existing readers and writers.

Build-tested by compiling mes_v11_0.o.

No AMDGPU hardware was available for end-to-end runtime testing.

Fixes: d81d75c99936 ("drm/amdgpu/gfx11: enable kiq to map mes ring")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
index a926a3307..e2f762c2e 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
@@ -1308,7 +1308,7 @@ static int mes_v11_0_queue_init(struct amdgpu_device *adev,
 
 	if ((pipe == AMDGPU_MES_SCHED_PIPE) &&
 	    (amdgpu_in_reset(adev) || adev->in_suspend)) {
-		*(ring->wptr_cpu_addr) = 0;
+		atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0);
 		*(ring->rptr_cpu_addr) = 0;
 		amdgpu_ring_clear_ring(ring);
 	}
-- 
2.34.1

[PATCH 2/2] drm/amdgpu/mes12: fix queue init wptr reset

[Runyu Xiao]

mes_v12_0_queue_init() resets ring->wptr_cpu_addr with a plain 32-bit
store in the reset/suspend path even though the same carrier is
accessed with atomic64_set()/atomic64_read() and support_64bit_ptrs is
enabled.

This is not just a missing atomic annotation. The MES queue write
pointer is a shared 64-bit carrier, and *ring->wptr_cpu_addr = 0 only
clears the low 32 bits. A later atomic64_read() can then observe stale
high 32 bits instead of a real zeroed reset state.

Use atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0) so the reset
path updates the full 64-bit wptr with the same access family as the
existing readers and writers.

Build-tested by compiling mes_v12_0.o.

No AMDGPU hardware was available for end-to-end runtime testing.

Fixes: 785f0f9fe742 ("drm/amdgpu: Add mes v12_0 ip block support (v4)")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/gpu/drm/amd/amdgpu/mes_v12_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
index 023c7345e..22ed7bb51 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_v12_0.c
@@ -1476,7 +1476,7 @@ static int mes_v12_0_queue_init(struct amdgpu_device *adev,
 
 	if ((adev->enable_uni_mes || pipe == AMDGPU_MES_SCHED_PIPE) &&
 	    (amdgpu_in_reset(adev) || adev->in_suspend)) {
-		*(ring->wptr_cpu_addr) = 0;
+		atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0);
 		*(ring->rptr_cpu_addr) = 0;
 		amdgpu_ring_clear_ring(ring);
 	}
-- 
2.34.1

Re: [PATCH 1/2] drm/amdgpu/mes11: fix queue init wptr reset

[Christian König]

On 6/2/26 07:03, Runyu Xiao wrote:
> mes_v11_0_queue_init() resets ring->wptr_cpu_addr with a plain 32-bit
> store in the reset/suspend path even though the same carrier is
> accessed with atomic64_set()/atomic64_read() and support_64bit_ptrs is
> enabled.
> 
> This is not just a missing atomic annotation. The MES queue write
> pointer is a shared 64-bit carrier, and *ring->wptr_cpu_addr = 0 only
> clears the low 32 bits. A later atomic64_read() can then observe stale
> high 32 bits instead of a real zeroed reset state.
> 
> Use atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0) so the reset
> path updates the full 64-bit wptr with the same access family as the
> existing readers and writers.
> 
> Build-tested by compiling mes_v11_0.o.
> 
> No AMDGPU hardware was available for end-to-end runtime testing.

Clear NAK.

The atomic64_t cast hack is just something we did for older generations and is not something which is necessary not should be done here.

What could be possible is that we need to use amdgpu_ring_set_wptr() here to correctly distinct between queues with 32bit and 64bit wptrs.

Regards,
Christian.

> 
> Fixes: d81d75c99936 ("drm/amdgpu/gfx11: enable kiq to map mes ring")
> Cc: stable@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
>  drivers/gpu/drm/amd/amdgpu/mes_v11_0.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> index a926a3307..e2f762c2e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v11_0.c
> @@ -1308,7 +1308,7 @@ static int mes_v11_0_queue_init(struct amdgpu_device *adev,
> 
>         if ((pipe == AMDGPU_MES_SCHED_PIPE) &&
>             (amdgpu_in_reset(adev) || adev->in_suspend)) {
> -               *(ring->wptr_cpu_addr) = 0;
> +               atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0);
>                 *(ring->rptr_cpu_addr) = 0;
>                 amdgpu_ring_clear_ring(ring);
>         }
> --
> 2.34.1

Re: [PATCH 1/2] drm/amdgpu/mes11: fix queue init wptr reset

[Christian König]

On 6/2/26 12:53, 肖润宇 wrote:
> 	
> Some people who received this message don't often get email from 220255722@seu.edu.cn. Learn why this is important <https://aka.ms/LearnAboutSenderIdentification>
> 	
> 
> Hi Christian,
> 
> Thanks, understood.
> 
> To make sure I rework this in the right direction: would you expect
> this reset path to do
> 
>   ring->wptr = 0;
>   amdgpu_ring_set_wptr(ring);
> 
> instead of writing wptr_cpu_addr directly?
> 
> I am asking because amdgpu_ring_set_wptr() also updates the doorbell,
> so I want to confirm that this is the intended sequence for the
> reset/suspend case here.

Yeah I was wondering the same thing.

I think the correct approach would be to make both rptr_cpu_addr and wptr_cpu_addr void* in the amdgpu_ring.h structure instead of u32* and then cast that to either (u64*) or (u32*) depending on the ring type.

The atomic64_t hack should really be removed.

BTW Reading the rptr is wrong on multiple instances as well and should probably be fixed in the same patch set.

Regards,
Christian.

> 
> Thanks,
> Runyu
> 
> On Tue, Jun 2, 2026 at 11:49:05AM +0200, Christian König wrote:
>> Clear NAK.
>>
>> The atomic64_t cast hack is just something we did for older
>> generations and is not something which is necessary nor should
>> be done here.
>>
>> What could be possible is that we need to use amdgpu_ring_set_wptr()
>> here to correctly distinguish between queues with 32bit and 64bit
>> wptrs.
> 

Re:Re: [PATCH 1/2] drm/amdgpu/mes11: fix queue init wptr reset

[Runyu Xiao]

Hi Christian,


Thanks, that makes sense.


I will rework this around typed rptr/wptr_cpu_addr accesses instead of
the atomic64_t cast, and I will include the related rptr fixes in the
same patch set.


Thanks,
Runyu
>> 	
>> 
>> Hi Christian,
>> 
>> Thanks, understood.
>> 
>> To make sure I rework this in the right direction: would you expect
>> this reset path to do
>> 
>>   ring->wptr = 0;
>>   amdgpu_ring_set_wptr(ring);
>> 
>> instead of writing wptr_cpu_addr directly?
>> 
>> I am asking because amdgpu_ring_set_wptr() also updates the doorbell,
>> so I want to confirm that this is the intended sequence for the
>> reset/suspend case here.
>
>Yeah I was wondering the same thing.
>
>I think the correct approach would be to make both rptr_cpu_addr and wptr_cpu_addr void* in the amdgpu_ring.h structure instead of u32* and then cast that to either (u64*) or (u32*) depending on the ring type.
>
>The atomic64_t hack should really be removed.
>
>BTW Reading the rptr is wrong on multiple instances as well and should probably be fixed in the same patch set.
>
>Regards,
>Christian.
>
>> 
>> Thanks,
>> Runyu
>> 
>> On Tue, Jun 2, 2026 at 11:49:05AM +0200, Christian König wrote:
>>> Clear NAK.
>>>
>>> The atomic64_t cast hack is just something we did for older
>>> generations and is not something which is necessary nor should
>>> be done here.
>>>
>>> What could be possible is that we need to use amdgpu_ring_set_wptr()
>>> here to correctly distinguish between queues with 32bit and 64bit
>>> wptrs.
>> 
>
>

>> 	
>> 
>> Hi Christian,
>> 
>> Thanks, understood.
>> 
>> To make sure I rework this in the right direction: would you expect
>> this reset path to do
>> 
>>   ring->wptr = 0;
>>   amdgpu_ring_set_wptr(ring);
>> 
>> instead of writing wptr_cpu_addr directly?
>> 
>> I am asking because amdgpu_ring_set_wptr() also updates the doorbell,
>> so I want to confirm that this is the intended sequence for the
>> reset/suspend case here.
>
>Yeah I was wondering the same thing.
>
>I think the correct approach would be to make both rptr_cpu_addr and wptr_cpu_addr void* in the amdgpu_ring.h structure instead of u32* and then cast that to either (u64*) or (u32*) depending on the ring type.
>
>The atomic64_t hack should really be removed.
>
>BTW Reading the rptr is wrong on multiple instances as well and should probably be fixed in the same patch set.
>
>Regards,
>Christian.
>
>> 
>> Thanks,
>> Runyu
>> 
>> On Tue, Jun 2, 2026 at 11:49:05AM +0200, Christian König wrote:
>>> Clear NAK.
>>>
>>> The atomic64_t cast hack is just something we did for older
>>> generations and is not something which is necessary nor should
>>> be done here.
>>>
>>> What could be possible is that we need to use amdgpu_ring_set_wptr()
>>> here to correctly distinguish between queues with 32bit and 64bit
>>> wptrs.
>> 
>
>