RE: [PATCH 2/4] drm/amdgpu: refactoring mailbox to fix TDR handshake bugs

[Liu, Monk]

>Can you do that ? looks to me the MACRO in amdgpu only works for DW 
> aligned register names, so I'm afraid we cannot do that gracefully  

[Pixel] some readable macros can help us easily understand which register field 
is accessed.

[ml] I can try it 

>[Pixel]generally, CMPL is not handled by rcv_msg anymore but in peek_msg. we 
>can remove the logics here.

Yeah, we can remove it 

/Monk


-Original Message-
From: Ding, Pixel 
Sent: 2018年3月7日 17:22
To: Liu, Monk <monk@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/4] drm/amdgpu: refactoring mailbox to fix TDR handshake 
bugs

Hi Monk,

See comments inline.
— 
Sincerely Yours,
Pixel


On 07/03/2018, 4:58 PM, "Liu, Monk" <monk@amd.com> wrote:

HI Pixel

The patch better not to be split, otherwise the stress TDR test still fail 
since there are couple issues and some of them mixed together 

For your concerns:
> Any VF ACK could lead missing msg if timing is bad while hypervisor send 
msg quite frequently and guest send ack frequently (suck like the case 
FLR_NOTIFY followed by READY_TO_ACCESS_GPU immediately). Is it correct? If so, 
maybe we need further protection in GIM.

We had a lot of fixings in GIM side along with this patch to make stress 
TDR test finally passed, you can watch the history, anyway GIM side is not 
related with this patch

[Pixel]Per talked offline, understand your point here.

> it’s not clear to understand the last 2 lines of code here to set 
RCV_MSG_ACK bit: 0x10 to +1 byte offset. Can we define some macros to make it 
clear?
Can you do that ? looks to me the MACRO in amdgpu only works for DW aligned 
register names, so I'm afraid we cannot do that gracefully  

[Pixel] some readable macros can help us easily understand which register field 
is accessed.

> Can we also set valid bit for IDH_FLR_NOTIFICATION_CMPL event in GIM? By 
now guest use peek_msg to catch this event, so I think it’s OK to set valid for 
it. Then the code here would be clear without CMPL condition. We can also use 
peek_ack to ack CMPL if it’s necessary. any problem?

I don't understand your point 
[Pixel]generally, CMPL is not handled by rcv_msg anymore but in peek_msg. we 
can remove the logics here.



-Original Message-
From: Ding, Pixel 
Sent: 2018年3月7日 16:31
To: Liu, Monk <monk@amd.com>; amd-gfx@lists.freedesktop.org
    Subject: Re: [PATCH 2/4] drm/amdgpu: refactoring mailbox to fix TDR 
handshake bugs

Hi Monk,

It’s a long patch which could split into 6 pieces at most… anyway I got 
some questions inline to have a better understanding.

— 
Sincerely Yours,
Pixel


On 06/03/2018, 11:29 AM, "amd-gfx on behalf of Monk Liu" 
<amd-gfx-boun...@lists.freedesktop.org on behalf of monk@amd.com> wrote:

this patch actually refactor mailbox implmentations, and
all below changes are needed together to fix all those mailbox
handshake issues exposured by heavey TDR test.

1)refactor all mailbox functions based on byte accessing for mb_control
reason is to avoid touching non-related bits when writing trn/rcv part 
of
mailbox_control, this way some incorrect INTR sent to hypervisor
side could be avoided, and it fixes couple handshake bug.

2)trans_msg function re-impled: put a invalid
logic before transmitting message to make sure the ACK bit is in
a clear status, otherwise there is chance that ACK asserted already
before transmitting message and lead to fake ACK polling.
(hypervisor side have some tricks to workaround ACK bit being corrupted
by VF FLR which hase an side effects that may make guest side ACK bit
asserted wrongly), and clear TRANS_MSG words after message transferred.

3)for mailbox_flr_work, it is also re-worked: it takes the mutex lock
first if invoked, to block gpu recover's participate too early while
hypervisor side is doing VF FLR. (hypervisor sends FLR_NOTIFY to guest
before doing VF FLR and sentds FLR_COMPLETE after VF FLR done, and
the FLR_NOTIFY will trigger interrupt to guest which lead to
mailbox_flr_work being invoked)

This can avoid the issue that mailbox trans msg being cleared by its VF 
FLR.

4)for mailbox_rcv_irq IRQ routine, it should only peek msg and schedule
mailbox_flr_work, instead of ACK to hypervisor itself, because 
FLR_NOTIFY
msg sent from hypervisor side doesn't need VF's ACK (this is because
VF's ACK would lead to hypervisor clear its trans_valid/msg, and this
would cause handshake bug if trans_valid/msg is cleared not due to
correct VF ACK but from a wrong VF ACK like this "FLR_NOTIFY"

Re: [PATCH 2/4] drm/amdgpu: refactoring mailbox to fix TDR handshake bugs

[Ding, Pixel]

Hi Monk,

See comments inline.
— 
Sincerely Yours,
Pixel


On 07/03/2018, 4:58 PM, "Liu, Monk" <monk@amd.com> wrote:

HI Pixel

The patch better not to be split, otherwise the stress TDR test still fail 
since there are couple issues and some of them mixed together 

For your concerns:
> Any VF ACK could lead missing msg if timing is bad while hypervisor send 
msg quite frequently and guest send ack frequently (suck like the case 
FLR_NOTIFY followed by READY_TO_ACCESS_GPU immediately). Is it correct? If so, 
maybe we need further protection in GIM.

We had a lot of fixings in GIM side along with this patch to make stress 
TDR test finally passed, you can watch the history, anyway GIM side is not 
related with this patch

[Pixel]Per talked offline, understand your point here.

> it’s not clear to understand the last 2 lines of code here to set 
RCV_MSG_ACK bit: 0x10 to +1 byte offset. Can we define some macros to make it 
clear?
Can you do that ? looks to me the MACRO in amdgpu only works for DW aligned 
register names, so I'm afraid we cannot do that gracefully  

[Pixel] some readable macros can help us easily understand which register field 
is accessed.

> Can we also set valid bit for IDH_FLR_NOTIFICATION_CMPL event in GIM? By 
now guest use peek_msg to catch this event, so I think it’s OK to set valid for 
it. Then the code here would be clear without CMPL condition. We can also use 
peek_ack to ack CMPL if it’s necessary. any problem?

I don't understand your point 
[Pixel]generally, CMPL is not handled by rcv_msg anymore but in peek_msg. we 
can remove the logics here.



-Original Message-
From: Ding, Pixel 
Sent: 2018年3月7日 16:31
To: Liu, Monk <monk@amd.com>; amd-gfx@lists.freedesktop.org
    Subject: Re: [PATCH 2/4] drm/amdgpu: refactoring mailbox to fix TDR 
handshake bugs

Hi Monk,

It’s a long patch which could split into 6 pieces at most… anyway I got 
some questions inline to have a better understanding.

— 
Sincerely Yours,
Pixel


On 06/03/2018, 11:29 AM, "amd-gfx on behalf of Monk Liu" 
<amd-gfx-boun...@lists.freedesktop.org on behalf of monk@amd.com> wrote:

this patch actually refactor mailbox implmentations, and
all below changes are needed together to fix all those mailbox
handshake issues exposured by heavey TDR test.

1)refactor all mailbox functions based on byte accessing for mb_control
reason is to avoid touching non-related bits when writing trn/rcv part 
of
mailbox_control, this way some incorrect INTR sent to hypervisor
side could be avoided, and it fixes couple handshake bug.

2)trans_msg function re-impled: put a invalid
logic before transmitting message to make sure the ACK bit is in
a clear status, otherwise there is chance that ACK asserted already
before transmitting message and lead to fake ACK polling.
(hypervisor side have some tricks to workaround ACK bit being corrupted
by VF FLR which hase an side effects that may make guest side ACK bit
asserted wrongly), and clear TRANS_MSG words after message transferred.

3)for mailbox_flr_work, it is also re-worked: it takes the mutex lock
first if invoked, to block gpu recover's participate too early while
hypervisor side is doing VF FLR. (hypervisor sends FLR_NOTIFY to guest
before doing VF FLR and sentds FLR_COMPLETE after VF FLR done, and
the FLR_NOTIFY will trigger interrupt to guest which lead to
mailbox_flr_work being invoked)

This can avoid the issue that mailbox trans msg being cleared by its VF 
FLR.

4)for mailbox_rcv_irq IRQ routine, it should only peek msg and schedule
mailbox_flr_work, instead of ACK to hypervisor itself, because 
FLR_NOTIFY
msg sent from hypervisor side doesn't need VF's ACK (this is because
VF's ACK would lead to hypervisor clear its trans_valid/msg, and this
would cause handshake bug if trans_valid/msg is cleared not due to
correct VF ACK but from a wrong VF ACK like this "FLR_NOTIFY" one)

Any VF ACK could lead missing msg if timing is bad while hypervisor send 
msg quite frequently and guest send ack frequently (suck like the case 
FLR_NOTIFY followed by READY_TO_ACCESS_GPU immediately). Is it correct? If so, 
maybe we need further protection in GIM.

This fixed handshake bug that sometimes GUEST always couldn't receive
"READY_TO_ACCESS_GPU" msg from hypervisor.

5)seperate polling time limite accordingly:
POLL ACK cost no more than 500ms
POLL MSG cost no more than 12000ms
POLL FLR finish cost no more than 500ms

6)

RE: [PATCH 2/4] drm/amdgpu: refactoring mailbox to fix TDR handshake bugs

[Liu, Monk]

HI Pixel

The patch better not to be split, otherwise the stress TDR test still fail 
since there are couple issues and some of them mixed together 

For your concerns:
> Any VF ACK could lead missing msg if timing is bad while hypervisor send msg 
> quite frequently and guest send ack frequently (suck like the case FLR_NOTIFY 
> followed by READY_TO_ACCESS_GPU immediately). Is it correct? If so, maybe we 
> need further protection in GIM.

We had a lot of fixings in GIM side along with this patch to make stress TDR 
test finally passed, you can watch the history, anyway GIM side is not related 
with this patch


> it’s not clear to understand the last 2 lines of code here to set RCV_MSG_ACK 
> bit: 0x10 to +1 byte offset. Can we define some macros to make it clear?
Can you do that ? looks to me the MACRO in amdgpu only works for DW aligned 
register names, so I'm afraid we cannot do that gracefully  


> Can we also set valid bit for IDH_FLR_NOTIFICATION_CMPL event in GIM? By now 
> guest use peek_msg to catch this event, so I think it’s OK to set valid for 
> it. Then the code here would be clear without CMPL condition. We can also use 
> peek_ack to ack CMPL if it’s necessary. any problem?

I don't understand your point 




-Original Message-
From: Ding, Pixel 
Sent: 2018年3月7日 16:31
To: Liu, Monk <monk@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/4] drm/amdgpu: refactoring mailbox to fix TDR handshake 
bugs

Hi Monk,

It’s a long patch which could split into 6 pieces at most… anyway I got some 
questions inline to have a better understanding.

— 
Sincerely Yours,
Pixel


On 06/03/2018, 11:29 AM, "amd-gfx on behalf of Monk Liu" 
<amd-gfx-boun...@lists.freedesktop.org on behalf of monk@amd.com> wrote:

this patch actually refactor mailbox implmentations, and
all below changes are needed together to fix all those mailbox
handshake issues exposured by heavey TDR test.

1)refactor all mailbox functions based on byte accessing for mb_control
reason is to avoid touching non-related bits when writing trn/rcv part of
mailbox_control, this way some incorrect INTR sent to hypervisor
side could be avoided, and it fixes couple handshake bug.

2)trans_msg function re-impled: put a invalid
logic before transmitting message to make sure the ACK bit is in
a clear status, otherwise there is chance that ACK asserted already
before transmitting message and lead to fake ACK polling.
(hypervisor side have some tricks to workaround ACK bit being corrupted
by VF FLR which hase an side effects that may make guest side ACK bit
asserted wrongly), and clear TRANS_MSG words after message transferred.

3)for mailbox_flr_work, it is also re-worked: it takes the mutex lock
first if invoked, to block gpu recover's participate too early while
hypervisor side is doing VF FLR. (hypervisor sends FLR_NOTIFY to guest
before doing VF FLR and sentds FLR_COMPLETE after VF FLR done, and
the FLR_NOTIFY will trigger interrupt to guest which lead to
mailbox_flr_work being invoked)

This can avoid the issue that mailbox trans msg being cleared by its VF FLR.

4)for mailbox_rcv_irq IRQ routine, it should only peek msg and schedule
mailbox_flr_work, instead of ACK to hypervisor itself, because FLR_NOTIFY
msg sent from hypervisor side doesn't need VF's ACK (this is because
VF's ACK would lead to hypervisor clear its trans_valid/msg, and this
would cause handshake bug if trans_valid/msg is cleared not due to
correct VF ACK but from a wrong VF ACK like this "FLR_NOTIFY" one)

Any VF ACK could lead missing msg if timing is bad while hypervisor send msg 
quite frequently and guest send ack frequently (suck like the case FLR_NOTIFY 
followed by READY_TO_ACCESS_GPU immediately). Is it correct? If so, maybe we 
need further protection in GIM.

This fixed handshake bug that sometimes GUEST always couldn't receive
"READY_TO_ACCESS_GPU" msg from hypervisor.

5)seperate polling time limite accordingly:
POLL ACK cost no more than 500ms
POLL MSG cost no more than 12000ms
POLL FLR finish cost no more than 500ms

6) we still need to set adev into in_gpu_reset mode after we received
FLR_NOTIFY from host side, this can prevent innocent app wrongly succesed
to open amdgpu dri device.

FLR_NOFITY is received due to an IDLE hang detected from hypervisor side
which indicating GPU is already die in this VF.

Change-Id: I17df8b4490a5b53a1cc2bd6c8f9bc3ee14c23f1a
Signed-off-by: Monk Liu <monk@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 200 
++
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h |   4 +-
 2 files changed, 111 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_a

Re: [PATCH 2/4] drm/amdgpu: refactoring mailbox to fix TDR handshake bugs

[Ding, Pixel]

Hi Monk,

It’s a long patch which could split into 6 pieces at most… anyway I got some 
questions inline to have a better understanding.

— 
Sincerely Yours,
Pixel


On 06/03/2018, 11:29 AM, "amd-gfx on behalf of Monk Liu" 
 wrote:

this patch actually refactor mailbox implmentations, and
all below changes are needed together to fix all those mailbox
handshake issues exposured by heavey TDR test.

1)refactor all mailbox functions based on byte accessing for mb_control
reason is to avoid touching non-related bits when writing trn/rcv part of
mailbox_control, this way some incorrect INTR sent to hypervisor
side could be avoided, and it fixes couple handshake bug.

2)trans_msg function re-impled: put a invalid
logic before transmitting message to make sure the ACK bit is in
a clear status, otherwise there is chance that ACK asserted already
before transmitting message and lead to fake ACK polling.
(hypervisor side have some tricks to workaround ACK bit being corrupted
by VF FLR which hase an side effects that may make guest side ACK bit
asserted wrongly), and clear TRANS_MSG words after message transferred.

3)for mailbox_flr_work, it is also re-worked: it takes the mutex lock
first if invoked, to block gpu recover's participate too early while
hypervisor side is doing VF FLR. (hypervisor sends FLR_NOTIFY to guest
before doing VF FLR and sentds FLR_COMPLETE after VF FLR done, and
the FLR_NOTIFY will trigger interrupt to guest which lead to
mailbox_flr_work being invoked)

This can avoid the issue that mailbox trans msg being cleared by its VF FLR.

4)for mailbox_rcv_irq IRQ routine, it should only peek msg and schedule
mailbox_flr_work, instead of ACK to hypervisor itself, because FLR_NOTIFY
msg sent from hypervisor side doesn't need VF's ACK (this is because
VF's ACK would lead to hypervisor clear its trans_valid/msg, and this
would cause handshake bug if trans_valid/msg is cleared not due to
correct VF ACK but from a wrong VF ACK like this "FLR_NOTIFY" one)

Any VF ACK could lead missing msg if timing is bad while hypervisor send msg 
quite frequently and guest send ack frequently (suck like the case FLR_NOTIFY 
followed by READY_TO_ACCESS_GPU immediately). Is it correct? If so, maybe we 
need further protection in GIM.

This fixed handshake bug that sometimes GUEST always couldn't receive
"READY_TO_ACCESS_GPU" msg from hypervisor.

5)seperate polling time limite accordingly:
POLL ACK cost no more than 500ms
POLL MSG cost no more than 12000ms
POLL FLR finish cost no more than 500ms

6) we still need to set adev into in_gpu_reset mode after we received
FLR_NOTIFY from host side, this can prevent innocent app wrongly succesed
to open amdgpu dri device.

FLR_NOFITY is received due to an IDLE hang detected from hypervisor side
which indicating GPU is already die in this VF.

Change-Id: I17df8b4490a5b53a1cc2bd6c8f9bc3ee14c23f1a
Signed-off-by: Monk Liu 
---
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 200 
++
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h |   4 +-
 2 files changed, 111 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c 
b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
index 271452d..8d09380 100644
--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
@@ -33,56 +33,42 @@
 
 static void xgpu_ai_mailbox_send_ack(struct amdgpu_device *adev)
 {
-   u32 reg;
-   int timeout = AI_MAILBOX_TIMEDOUT;
-   u32 mask = REG_FIELD_MASK(BIF_BX_PF0_MAILBOX_CONTROL, RCV_MSG_VALID);
-
-   reg = RREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0,
-mmBIF_BX_PF0_MAILBOX_CONTROL));
-   reg = REG_SET_FIELD(reg, BIF_BX_PF0_MAILBOX_CONTROL, RCV_MSG_ACK, 1);
-   WREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0,
-  mmBIF_BX_PF0_MAILBOX_CONTROL), reg);
-
-   /*Wait for RCV_MSG_VALID to be 0*/
-   reg = RREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0,
-mmBIF_BX_PF0_MAILBOX_CONTROL));
-   while (reg & mask) {
-   if (timeout <= 0) {
-   pr_err("RCV_MSG_VALID is not cleared\n");
-   break;
-   }
-   mdelay(1);
-   timeout -=1;
-
-   reg = RREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0,
-
mmBIF_BX_PF0_MAILBOX_CONTROL));
-   }
  + const u32 offset = SOC15_REG_OFFSET(NBIO, 0, 
mmBIF_BX_PF0_MAILBOX_CONTROL) * 4 + 1;
  + WREG8(offset, 2);

it’s not clear to understand the last 2 lines of code here to set RCV_MSG_ACK 
bit: 0x10 to +1 

[PATCH 2/4] drm/amdgpu: refactoring mailbox to fix TDR handshake bugs

[Monk Liu]

this patch actually refactor mailbox implmentations, and
all below changes are needed together to fix all those mailbox
handshake issues exposured by heavey TDR test.

1)refactor all mailbox functions based on byte accessing for mb_control
reason is to avoid touching non-related bits when writing trn/rcv part of
mailbox_control, this way some incorrect INTR sent to hypervisor
side could be avoided, and it fixes couple handshake bug.

2)trans_msg function re-impled: put a invalid
logic before transmitting message to make sure the ACK bit is in
a clear status, otherwise there is chance that ACK asserted already
before transmitting message and lead to fake ACK polling.
(hypervisor side have some tricks to workaround ACK bit being corrupted
by VF FLR which hase an side effects that may make guest side ACK bit
asserted wrongly), and clear TRANS_MSG words after message transferred.

3)for mailbox_flr_work, it is also re-worked: it takes the mutex lock
first if invoked, to block gpu recover's participate too early while
hypervisor side is doing VF FLR. (hypervisor sends FLR_NOTIFY to guest
before doing VF FLR and sentds FLR_COMPLETE after VF FLR done, and
the FLR_NOTIFY will trigger interrupt to guest which lead to
mailbox_flr_work being invoked)

This can avoid the issue that mailbox trans msg being cleared by its VF FLR.

4)for mailbox_rcv_irq IRQ routine, it should only peek msg and schedule
mailbox_flr_work, instead of ACK to hypervisor itself, because FLR_NOTIFY
msg sent from hypervisor side doesn't need VF's ACK (this is because
VF's ACK would lead to hypervisor clear its trans_valid/msg, and this
would cause handshake bug if trans_valid/msg is cleared not due to
correct VF ACK but from a wrong VF ACK like this "FLR_NOTIFY" one)

This fixed handshake bug that sometimes GUEST always couldn't receive
"READY_TO_ACCESS_GPU" msg from hypervisor.

5)seperate polling time limite accordingly:
POLL ACK cost no more than 500ms
POLL MSG cost no more than 12000ms
POLL FLR finish cost no more than 500ms

6) we still need to set adev into in_gpu_reset mode after we received
FLR_NOTIFY from host side, this can prevent innocent app wrongly succesed
to open amdgpu dri device.

FLR_NOFITY is received due to an IDLE hang detected from hypervisor side
which indicating GPU is already die in this VF.

Change-Id: I17df8b4490a5b53a1cc2bd6c8f9bc3ee14c23f1a
Signed-off-by: Monk Liu 
---
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c | 200 ++
 drivers/gpu/drm/amd/amdgpu/mxgpu_ai.h |   4 +-
 2 files changed, 111 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c 
b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
index 271452d..8d09380 100644
--- a/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
+++ b/drivers/gpu/drm/amd/amdgpu/mxgpu_ai.c
@@ -33,56 +33,42 @@
 
 static void xgpu_ai_mailbox_send_ack(struct amdgpu_device *adev)
 {
-   u32 reg;
-   int timeout = AI_MAILBOX_TIMEDOUT;
-   u32 mask = REG_FIELD_MASK(BIF_BX_PF0_MAILBOX_CONTROL, RCV_MSG_VALID);
-
-   reg = RREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0,
-mmBIF_BX_PF0_MAILBOX_CONTROL));
-   reg = REG_SET_FIELD(reg, BIF_BX_PF0_MAILBOX_CONTROL, RCV_MSG_ACK, 1);
-   WREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0,
-  mmBIF_BX_PF0_MAILBOX_CONTROL), reg);
-
-   /*Wait for RCV_MSG_VALID to be 0*/
-   reg = RREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0,
-mmBIF_BX_PF0_MAILBOX_CONTROL));
-   while (reg & mask) {
-   if (timeout <= 0) {
-   pr_err("RCV_MSG_VALID is not cleared\n");
-   break;
-   }
-   mdelay(1);
-   timeout -=1;
-
-   reg = RREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0,
-
mmBIF_BX_PF0_MAILBOX_CONTROL));
-   }
+   const u32 offset = SOC15_REG_OFFSET(NBIO, 0, 
mmBIF_BX_PF0_MAILBOX_CONTROL) * 4 + 1;
+   WREG8(offset, 2);
 }
 
 static void xgpu_ai_mailbox_set_valid(struct amdgpu_device *adev, bool val)
 {
-   u32 reg;
+   const u32 offset = SOC15_REG_OFFSET(NBIO, 0, 
mmBIF_BX_PF0_MAILBOX_CONTROL) * 4;
+   WREG8(offset, val ? 1 : 0);
+}
 
-   reg = RREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0,
-mmBIF_BX_PF0_MAILBOX_CONTROL));
-   reg = REG_SET_FIELD(reg, BIF_BX_PF0_MAILBOX_CONTROL,
-   TRN_MSG_VALID, val ? 1 : 0);
-   WREG32_NO_KIQ(SOC15_REG_OFFSET(NBIO, 0, mmBIF_BX_PF0_MAILBOX_CONTROL),
- reg);
+/*
+ * this peek_msg could *only* be called in IRQ routine becuase in IRQ routine
+ * RCV_MSG_VALID filed of BIF_BX_PF0_MAILBOX_CONTROL must already be set to 1
+ * by host.
+ *
+ * if called no in IRQ routine, this peek_msg cannot guaranteed to return the
+ * correct value since it doesn't return the RCV_DW0 under the case that
+ *