Эх сурвалжийг харах

qcacmn: Add memory barrier to avoid inconsistent reg write

The delayed register write enqueue fills a queue element
with the required data which can be dequeued in a workqueue
running on a different CPU. Since these operations are not
lock protected, there can be stale value access when memory
write has not been flushed to the actual address.

Using write memory barrier before setting the valid flag for
a queue element will make sure that the dequeuing worker
thread will always see the updated values if the element valid
flag is set and thereby avoid any race condition.

Change-Id: I81b0735f0fb39599095ad309157020c691e25a0b
CRs-Fixed: 2665576
Rakesh Pillai 5 жил өмнө
parent
commit
94ff74fcf9

+ 24 - 11
hal/wifi3.0/hal_srng.c

@@ -378,12 +378,14 @@ static inline bool hal_is_reg_write_tput_level_high(struct hal_soc *hal)
  * @hal: hal_soc pointer
  * @q_elem: pointer to hal regiter write queue element
  *
- * Return: None
+ * Return: The value which was written to the address
  */
-static void hal_process_reg_write_q_elem(struct hal_soc *hal,
-					 struct hal_reg_write_q_elem *q_elem)
+static uint32_t
+hal_process_reg_write_q_elem(struct hal_soc *hal,
+			     struct hal_reg_write_q_elem *q_elem)
 {
 	struct hal_srng *srng = q_elem->srng;
+	uint32_t write_val;
 
 	SRNG_LOCK(&srng->lock);
 
@@ -395,14 +397,19 @@ static void hal_process_reg_write_q_elem(struct hal_soc *hal,
 		hal_write_address_32_mb(hal,
 					srng->u.src_ring.hp_addr,
 					srng->u.src_ring.hp, false);
+		write_val = srng->u.src_ring.hp;
 	} else {
 		q_elem->dequeue_val = srng->u.dst_ring.tp;
 		hal_write_address_32_mb(hal,
 					srng->u.dst_ring.tp_addr,
 					srng->u.dst_ring.tp, false);
+		write_val = srng->u.dst_ring.tp;
 	}
 
+	q_elem->valid = 0;
 	SRNG_UNLOCK(&srng->lock);
+
+	return write_val;
 }
 
 /**
@@ -437,10 +444,12 @@ static inline void hal_reg_write_fill_sched_delay_hist(struct hal_soc *hal,
  */
 static void hal_reg_write_work(void *arg)
 {
-	int32_t q_depth;
+	int32_t q_depth, write_val;
 	struct hal_soc *hal = arg;
 	struct hal_reg_write_q_elem *q_elem;
 	uint64_t delta_us;
+	uint8_t ring_id;
+	uint32_t *addr;
 
 	q_elem = &hal->reg_write_queue[(hal->read_idx)];
 
@@ -458,6 +467,8 @@ static void hal_reg_write_work(void *arg)
 
 	while (q_elem->valid) {
 		q_elem->dequeue_time = qdf_get_log_timestamp();
+		ring_id = q_elem->srng->ring_id;
+		addr = q_elem->addr;
 		delta_us = qdf_log_timestamp_to_usecs(q_elem->dequeue_time -
 						      q_elem->enqueue_time);
 		hal_reg_write_fill_sched_delay_hist(hal, delta_us);
@@ -465,15 +476,10 @@ static void hal_reg_write_work(void *arg)
 		hal->stats.wstats.dequeues++;
 		qdf_atomic_dec(&hal->stats.wstats.q_depth);
 
-		hal_process_reg_write_q_elem(hal, q_elem);
+		write_val = hal_process_reg_write_q_elem(hal, q_elem);
 		hal_verbose_debug("read_idx %u srng 0x%x, addr 0x%pK dequeue_val %u sched delay %llu us",
-				  hal->read_idx,
-				  q_elem->srng->ring_id,
-				  q_elem->addr,
-				  q_elem->dequeue_val,
-				  delta_us);
+				  hal->read_idx, ring_id, addr, write_val, delta_us);
 
-		q_elem->valid = 0;
 		hal->read_idx = (hal->read_idx + 1) &
 					(HAL_REG_WRITE_QUEUE_LEN - 1);
 		q_elem = &hal->reg_write_queue[(hal->read_idx)];
@@ -544,6 +550,13 @@ static void hal_reg_write_enqueue(struct hal_soc *hal_soc,
 	q_elem->enqueue_val = value;
 	q_elem->enqueue_time = qdf_get_log_timestamp();
 
+	/*
+	 * Before the valid flag is set to true, all the other
+	 * fields in the q_elem needs to be updated in memory.
+	 * Else there is a chance that the dequeuing worker thread
+	 * might read stale entries and process incorrect srng.
+	 */
+	qdf_wmb();
 	q_elem->valid = true;
 
 	srng->reg_write_in_progress  = true;

+ 5 - 0
qdf/inc/qdf_util.h

@@ -46,6 +46,11 @@ typedef __qdf_wait_queue_head_t qdf_wait_queue_head_t;
  */
 #define qdf_likely(_expr)       __qdf_likely(_expr)
 
+/**
+ * qdf_wmb - write memory barrier.
+ */
+#define qdf_wmb()                 __qdf_wmb()
+
 /**
  * qdf_mb - read + write memory barrier.
  */