bcmdhd: Fixed DHD wrongly updating flowring write pointer to the dongle

Problem: Dongle ASSERT is observed due to wrong write pointer update by DHD

Analysis:
In the firmware ASSERT is hitting due to wrong write pointer update

By adding logging in the FW, we can see
  Time           Rd          Wr
ts: 00181114 fl37=0060 fl37=0059
ts: 00181115 fl37=0187 fl37=0186
ts: 00181117 fl37=0286 fl37=0000

Suddenly the write pointer of flowring 37 becoming 0.

After this we added logging the RD/Wr pointers of flowrings and we could see RD pointer misbehaving

      timestamp          dma_wr-rd       local_wr-rd     phase   start   rbk_cnt
   7184157843337            63-96             94-96     0x0         0         0
   7184157849066            63-96             95-96     0x0         0         0
   7184158379274            95-128            96-128    0x0         0         0
   7184158386045            95-128            97-128    0x0         0         0
   7184158391774            95-128            98-96     0x0         0         0
   7184158398024            95-128            99-96     0x0         0         0
   7184158404274            95-128           100-96     0x0         0         0
   7184158409483            95-128           101-96     0x0         0         0
   7184158414691            95-128           102-96     0x0         0         0

At time 7184.158379274 sec, the ring->rd (local_rd) becoming 128 and suddenly changing back to 96.
Due to which, the host assumed that Ring is empty and started filling complete ring even though actual RD is at only 128.

We added Indices validation in the dhd, before reading/writing flworing indices, but that was not caught either.

Then we suspected if there are any race conditions where ring->rd is updated from different contexts without holding any locks.
We found that dhd_prot_alloc_ring_space() and dhd_prot_update_txflowring() functions update ring->rd from dma indices. However dhd_prot_update_txflowring() updates without holding any RING_LOCK.
As there is a clear race between these 2 function, the value of ring->rd cannot be determined at any given stage and will lead to inconsistent values.

Solution:
As the ring->rd is updated from dma indices inside dhd_prot_alloc_ring_space() when all items are written, there is no need to update the same from another context dhd_prot_update_txflowring().
Hence removing the updation of ring->rd from dhd_prot_update_txflowring()

Bug: 245471430
Test: BRCM SVT test is completed without regression with this change.

Change-Id: Ib52f84add51957b46eed827e2b2ccd257463e730
Signed-off-by: Sungjoon Park <sungjoon.park@broadcom.corp-partner.google.com>

1 files changed, 0 insertions, 4 deletions

diff --git a/dhd_msgbuf.c b/dhd_msgbuf.c
index 6768e4f..0900fb7 100644
--- a/dhd_msgbuf.c
+++ b/dhd_msgbuf.c
@@ -7240,10 +7240,6 @@ dhd_prot_update_txflowring(dhd_pub_t *dhd, uint16 flowid, void *msgring)
 		DHD_ERROR(("%s: NULL txflowring. exiting...\n",  __FUNCTION__));
 		return FALSE;
 	}
-	/* Update read pointer */
-	if (dhd->dma_d2h_ring_upd_support) {
-		ring->rd = dhd_prot_dma_indx_get(dhd, H2D_DMA_INDX_RD_UPD, ring->idx);
-	}
 
 	DHD_TRACE(("ringid %d flowid %d write %d read %d \n\n",
 		ring->idx, flowid, ring->wr, ring->rd));