[PATCH v2 03/13] net/ena/base: fix missing barriers in PHC timestamp read

[PATCH v2 03/13] net/ena/base: fix missing barriers in PHC timestamp read

[Shai Brandes]

The PHC read path in ena_com_phc_get_timestamp() polls the shared
error_bound and timestamp fields for a matching req_id to detect
that the device has published a fresh reading. Both completion
checks read the payload without a barrier after observing req_id.

On weakly-ordered architectures the CPU may load the payload fields
before the req_id load is observed as updated, so a stale timestamp
or error bound can be returned while the read is reported as valid.

Add dma_rmb() after both req_id completion checks to order the
req_id load against the subsequent payload loads.

Fixes: a14be4dd8693 ("net/ena/base: add PHC error statistics")
Cc: stable@dpdk.org

Signed-off-by: Shai Brandes <shaibran@amazon.com>
---
 drivers/net/ena/base/ena_com.c | 50 +++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ena/base/ena_com.c b/drivers/net/ena/base/ena_com.c
index 737626bcfa..49818c4bd0 100644
--- a/drivers/net/ena/base/ena_com.c
+++ b/drivers/net/ena/base/ena_com.c
@@ -2037,27 +2037,34 @@ int ena_com_phc_get_timestamp(struct ena_com_dev *ena_dev, u64 *timestamp)
 				    "PHC get time request 0x%x failed (device error)\n",
 				    phc->req_id);
 			phc->stats.phc_err_dv++;
-		} else if (resp->error_flags & ENA_PHC_ERROR_FLAGS) {
-			/* Device updated req_id during blocking time but got
-			 * a PHC error, this occurs if device:
-			 * - exceeded the get time request limit
-			 * - received an invalid timestamp
-			 * - received an excessively high error bound
-			 * - received an invalid error bound
-			 */
-			ena_trc_err(ena_dev,
-				    "PHC get time request 0x%x failed (error 0x%x)\n",
-				    phc->req_id,
-				    resp->error_flags);
-			phc->stats.phc_err_ts += !!(resp->error_flags &
-				ENA_ADMIN_PHC_ERROR_FLAG_TIMESTAMP);
-			phc->stats.phc_err_eb += !!(resp->error_flags &
-				ENA_ADMIN_PHC_ERROR_FLAG_ERROR_BOUND);
 		} else {
-			/* Device updated req_id during blocking time
-			 * with valid timestamp and error bound
+			/* Ensure PHC payload is read after req_id update
+			 * is observed
 			 */
-			phc->stats.phc_exp++;
+			dma_rmb();
+
+			if (resp->error_flags & ENA_PHC_ERROR_FLAGS) {
+				/* Device updated req_id during blocking time
+				 * but got a PHC error, this occurs if device:
+				 * - exceeded the get time request limit
+				 * - received an invalid timestamp
+				 * - received an excessively high error bound
+				 * - received an invalid error bound
+				 */
+				ena_trc_err(ena_dev,
+					    "PHC get time request 0x%x failed (error 0x%x)\n",
+					    phc->req_id,
+					    resp->error_flags);
+				phc->stats.phc_err_ts += !!(resp->error_flags &
+					ENA_ADMIN_PHC_ERROR_FLAG_TIMESTAMP);
+				phc->stats.phc_err_eb += !!(resp->error_flags &
+					ENA_ADMIN_PHC_ERROR_FLAG_ERROR_BOUND);
+			} else {
+				/* Device updated req_id during blocking time
+				 * with valid timestamp and error bound
+				 */
+				phc->stats.phc_exp++;
+			}
 		}
 	}
 
@@ -2100,6 +2107,11 @@ int ena_com_phc_get_timestamp(struct ena_com_dev *ena_dev, u64 *timestamp)
 			continue;
 		}
 
+		/* Ensure PHC payload (timestamp, error_bound, error_flags)
+		 * is read after req_id update is observed
+		 */
+		dma_rmb();
+
 		/* req_id was updated by the device which indicates that
 		 * PHC timestamp, error_bound and error_flags are updated too,
 		 * checking errors before retrieving timestamp and
-- 
2.17.1