[dpdk-dev] [PATCH v17 03/11] eal: change API of power intrinsics

[Anatoly Burakov <anatoly.burakov@intel.com>]

Instead of passing around pointers and integers, collect everything
into struct. This makes API design around these intrinsics much easier.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---

Notes:
    v16:
    - Add error handling

 drivers/event/dlb/dlb.c                       | 10 ++--
 drivers/event/dlb2/dlb2.c                     | 10 ++--
 lib/librte_eal/arm/rte_power_intrinsics.c     | 20 +++-----
 .../include/generic/rte_power_intrinsics.h    | 50 ++++++++-----------
 lib/librte_eal/ppc/rte_power_intrinsics.c     | 20 +++-----
 lib/librte_eal/x86/rte_power_intrinsics.c     | 42 +++++++++-------
 6 files changed, 70 insertions(+), 82 deletions(-)

diff --git a/drivers/event/dlb/dlb.c b/drivers/event/dlb/dlb.c
index 0c95c4793d..d2f2026291 100644
--- a/drivers/event/dlb/dlb.c
+++ b/drivers/event/dlb/dlb.c
@@ -3161,6 +3161,7 @@ dlb_dequeue_wait(struct dlb_eventdev *dlb,
 		/* Interrupts not supported by PF PMD */
 		return 1;
 	} else if (dlb->umwait_allowed) {
+		struct rte_power_monitor_cond pmc;
 		volatile struct dlb_dequeue_qe *cq_base;
 		union {
 			uint64_t raw_qe[2];
@@ -3181,9 +3182,12 @@ dlb_dequeue_wait(struct dlb_eventdev *dlb,
 		else
 			expected_value = 0;
 
-		rte_power_monitor(monitor_addr, expected_value,
-				  qe_mask.raw_qe[1], timeout + start_ticks,
-				  sizeof(uint64_t));
+		pmc.addr = monitor_addr;
+		pmc.val = expected_value;
+		pmc.mask = qe_mask.raw_qe[1];
+		pmc.data_sz = sizeof(uint64_t);
+
+		rte_power_monitor(&pmc, timeout + start_ticks);
 
 		DLB_INC_STAT(ev_port->stats.traffic.rx_umonitor_umwait, 1);
 	} else {
diff --git a/drivers/event/dlb2/dlb2.c b/drivers/event/dlb2/dlb2.c
index 86724863f2..c9a8a02278 100644
--- a/drivers/event/dlb2/dlb2.c
+++ b/drivers/event/dlb2/dlb2.c
@@ -2870,6 +2870,7 @@ dlb2_dequeue_wait(struct dlb2_eventdev *dlb2,
 	if (elapsed_ticks >= timeout) {
 		return 1;
 	} else if (dlb2->umwait_allowed) {
+		struct rte_power_monitor_cond pmc;
 		volatile struct dlb2_dequeue_qe *cq_base;
 		union {
 			uint64_t raw_qe[2];
@@ -2890,9 +2891,12 @@ dlb2_dequeue_wait(struct dlb2_eventdev *dlb2,
 		else
 			expected_value = 0;
 
-		rte_power_monitor(monitor_addr, expected_value,
-				  qe_mask.raw_qe[1], timeout + start_ticks,
-				  sizeof(uint64_t));
+		pmc.addr = monitor_addr;
+		pmc.val = expected_value;
+		pmc.mask = qe_mask.raw_qe[1];
+		pmc.data_sz = sizeof(uint64_t);
+
+		rte_power_monitor(&pmc, timeout + start_ticks);
 
 		DLB2_INC_STAT(ev_port->stats.traffic.rx_umonitor_umwait, 1);
 	} else {
diff --git a/lib/librte_eal/arm/rte_power_intrinsics.c b/lib/librte_eal/arm/rte_power_intrinsics.c
index 7e7552fa8a..5f1caaf25b 100644
--- a/lib/librte_eal/arm/rte_power_intrinsics.c
+++ b/lib/librte_eal/arm/rte_power_intrinsics.c
@@ -8,15 +8,11 @@
  * This function is not supported on ARM.
  */
 int
-rte_power_monitor(const volatile void *p, const uint64_t expected_value,
-		const uint64_t value_mask, const uint64_t tsc_timestamp,
-		const uint8_t data_sz)
+rte_power_monitor(const struct rte_power_monitor_cond *pmc,
+		const uint64_t tsc_timestamp)
 {
-	RTE_SET_USED(p);
-	RTE_SET_USED(expected_value);
-	RTE_SET_USED(value_mask);
+	RTE_SET_USED(pmc);
 	RTE_SET_USED(tsc_timestamp);
-	RTE_SET_USED(data_sz);
 
 	return -ENOTSUP;
 }
@@ -25,16 +21,12 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
  * This function is not supported on ARM.
  */
 int
-rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
-		const uint64_t value_mask, const uint64_t tsc_timestamp,
-		const uint8_t data_sz, rte_spinlock_t *lck)
+rte_power_monitor_sync(const struct rte_power_monitor_cond *pmc,
+		const uint64_t tsc_timestamp, rte_spinlock_t *lck)
 {
-	RTE_SET_USED(p);
-	RTE_SET_USED(expected_value);
-	RTE_SET_USED(value_mask);
+	RTE_SET_USED(pmc);
 	RTE_SET_USED(tsc_timestamp);
 	RTE_SET_USED(lck);
-	RTE_SET_USED(data_sz);
 
 	return -ENOTSUP;
 }
diff --git a/lib/librte_eal/include/generic/rte_power_intrinsics.h b/lib/librte_eal/include/generic/rte_power_intrinsics.h
index 37e4ec0414..3ad53068d5 100644
--- a/lib/librte_eal/include/generic/rte_power_intrinsics.h
+++ b/lib/librte_eal/include/generic/rte_power_intrinsics.h
@@ -18,6 +18,18 @@
  * which are architecture-dependent.
  */
 
+struct rte_power_monitor_cond {
+	volatile void *addr;  /**< Address to monitor for changes */
+	uint64_t val;         /**< Before attempting the monitoring, the address
+	                       *   may be read and compared against this value.
+	                       **/
+	uint64_t mask;   /**< 64-bit mask to extract current value from addr */
+	uint8_t data_sz; /**< Data size (in bytes) that will be used to compare
+	                  *   expected value with the memory address. Can be 1,
+	                  *   2, 4, or 8. Supplying any other value will lead to
+	                  *   undefined result. */
+};
+
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice
@@ -35,20 +47,11 @@
  * @warning It is responsibility of the user to check if this function is
  *   supported at runtime using `rte_cpu_get_intrinsics_support()` API call.
  *
- * @param p
- *   Address to monitor for changes.
- * @param expected_value
- *   Before attempting the monitoring, the `p` address may be read and compared
- *   against this value. If `value_mask` is zero, this step will be skipped.
- * @param value_mask
- *   The 64-bit mask to use to extract current value from `p`.
+ * @param pmc
+ *   The monitoring condition structure.
  * @param tsc_timestamp
  *   Maximum TSC timestamp to wait for. Note that the wait behavior is
  *   architecture-dependent.
- * @param data_sz
- *   Data size (in bytes) that will be used to compare expected value with the
- *   memory address. Can be 1, 2, 4 or 8. Supplying any other value will lead
- *   to undefined result.
  *
  * @return
  *   0 on success
@@ -56,10 +59,8 @@
  *   -ENOTSUP if unsupported
  */
 __rte_experimental
-int rte_power_monitor(const volatile void *p,
-		const uint64_t expected_value, const uint64_t value_mask,
-		const uint64_t tsc_timestamp, const uint8_t data_sz);
-
+int rte_power_monitor(const struct rte_power_monitor_cond *pmc,
+		const uint64_t tsc_timestamp);
 /**
  * @warning
  * @b EXPERIMENTAL: this API may change without prior notice
@@ -80,20 +81,11 @@ int rte_power_monitor(const volatile void *p,
  * @warning It is responsibility of the user to check if this function is
  *   supported at runtime using `rte_cpu_get_intrinsics_support()` API call.
  *
- * @param p
- *   Address to monitor for changes.
- * @param expected_value
- *   Before attempting the monitoring, the `p` address may be read and compared
- *   against this value. If `value_mask` is zero, this step will be skipped.
- * @param value_mask
- *   The 64-bit mask to use to extract current value from `p`.
+ * @param pmc
+ *   The monitoring condition structure.
  * @param tsc_timestamp
  *   Maximum TSC timestamp to wait for. Note that the wait behavior is
  *   architecture-dependent.
- * @param data_sz
- *   Data size (in bytes) that will be used to compare expected value with the
- *   memory address. Can be 1, 2, 4 or 8. Supplying any other value will lead
- *   to undefined result.
  * @param lck
  *   A spinlock that must be locked before entering the function, will be
  *   unlocked while the CPU is sleeping, and will be locked again once the CPU
@@ -105,10 +97,8 @@ int rte_power_monitor(const volatile void *p,
  *   -ENOTSUP if unsupported
  */
 __rte_experimental
-int rte_power_monitor_sync(const volatile void *p,
-		const uint64_t expected_value, const uint64_t value_mask,
-		const uint64_t tsc_timestamp, const uint8_t data_sz,
-		rte_spinlock_t *lck);
+int rte_power_monitor_sync(const struct rte_power_monitor_cond *pmc,
+		const uint64_t tsc_timestamp, rte_spinlock_t *lck);
 
 /**
  * @warning
diff --git a/lib/librte_eal/ppc/rte_power_intrinsics.c b/lib/librte_eal/ppc/rte_power_intrinsics.c
index 929e0611b0..5e5a1fff5a 100644
--- a/lib/librte_eal/ppc/rte_power_intrinsics.c
+++ b/lib/librte_eal/ppc/rte_power_intrinsics.c
@@ -8,15 +8,11 @@
  * This function is not supported on PPC64.
  */
 int
-rte_power_monitor(const volatile void *p, const uint64_t expected_value,
-		const uint64_t value_mask, const uint64_t tsc_timestamp,
-		const uint8_t data_sz)
+rte_power_monitor(const struct rte_power_monitor_cond *pmc,
+		const uint64_t tsc_timestamp)
 {
-	RTE_SET_USED(p);
-	RTE_SET_USED(expected_value);
-	RTE_SET_USED(value_mask);
+	RTE_SET_USED(pmc);
 	RTE_SET_USED(tsc_timestamp);
-	RTE_SET_USED(data_sz);
 
 	return -ENOTSUP;
 }
@@ -25,16 +21,12 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
  * This function is not supported on PPC64.
  */
 int
-rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
-		const uint64_t value_mask, const uint64_t tsc_timestamp,
-		const uint8_t data_sz, rte_spinlock_t *lck)
+rte_power_monitor_sync(const struct rte_power_monitor_cond *pmc,
+		const uint64_t tsc_timestamp, rte_spinlock_t *lck)
 {
-	RTE_SET_USED(p);
-	RTE_SET_USED(expected_value);
-	RTE_SET_USED(value_mask);
+	RTE_SET_USED(pmc);
 	RTE_SET_USED(tsc_timestamp);
 	RTE_SET_USED(lck);
-	RTE_SET_USED(data_sz);
 
 	return -ENOTSUP;
 }
diff --git a/lib/librte_eal/x86/rte_power_intrinsics.c b/lib/librte_eal/x86/rte_power_intrinsics.c
index 2a38440bec..6be5c8b9f1 100644
--- a/lib/librte_eal/x86/rte_power_intrinsics.c
+++ b/lib/librte_eal/x86/rte_power_intrinsics.c
@@ -46,9 +46,8 @@ __check_val_size(const uint8_t sz)
  * Intel(R) 64 and IA-32 Architectures Software Developer's Manual.
  */
 int
-rte_power_monitor(const volatile void *p, const uint64_t expected_value,
-		const uint64_t value_mask, const uint64_t tsc_timestamp,
-		const uint8_t data_sz)
+rte_power_monitor(const struct rte_power_monitor_cond *pmc,
+		const uint64_t tsc_timestamp)
 {
 	const uint32_t tsc_l = (uint32_t)tsc_timestamp;
 	const uint32_t tsc_h = (uint32_t)(tsc_timestamp >> 32);
@@ -57,7 +56,10 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
 	if (!wait_supported)
 		return -ENOTSUP;
 
-	if (__check_val_size(data_sz) < 0)
+	if (pmc == NULL)
+		return -EINVAL;
+
+	if (__check_val_size(pmc->data_sz) < 0)
 		return -EINVAL;
 
 	/*
@@ -68,14 +70,15 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
 	/* set address for UMONITOR */
 	asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf7;"
 			:
-			: "D"(p));
+			: "D"(pmc->addr));
 
-	if (value_mask) {
-		const uint64_t cur_value = __get_umwait_val(p, data_sz);
-		const uint64_t masked = cur_value & value_mask;
+	if (pmc->mask) {
+		const uint64_t cur_value = __get_umwait_val(
+				pmc->addr, pmc->data_sz);
+		const uint64_t masked = cur_value & pmc->mask;
 
 		/* if the masked value is already matching, abort */
-		if (masked == expected_value)
+		if (masked == pmc->val)
 			return 0;
 	}
 	/* execute UMWAIT */
@@ -93,9 +96,8 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
  * Intel(R) 64 and IA-32 Architectures Software Developer's Manual.
  */
 int
-rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
-		const uint64_t value_mask, const uint64_t tsc_timestamp,
-		const uint8_t data_sz, rte_spinlock_t *lck)
+rte_power_monitor_sync(const struct rte_power_monitor_cond *pmc,
+		const uint64_t tsc_timestamp, rte_spinlock_t *lck)
 {
 	const uint32_t tsc_l = (uint32_t)tsc_timestamp;
 	const uint32_t tsc_h = (uint32_t)(tsc_timestamp >> 32);
@@ -104,7 +106,10 @@ rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
 	if (!wait_supported)
 		return -ENOTSUP;
 
-	if (__check_val_size(data_sz) < 0)
+	if (pmc == NULL || lck == NULL)
+		return -EINVAL;
+
+	if (__check_val_size(pmc->data_sz) < 0)
 		return -EINVAL;
 
 	/*
@@ -115,14 +120,15 @@ rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
 	/* set address for UMONITOR */
 	asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf7;"
 			:
-			: "D"(p));
+			: "D"(pmc->addr));
 
-	if (value_mask) {
-		const uint64_t cur_value = __get_umwait_val(p, data_sz);
-		const uint64_t masked = cur_value & value_mask;
+	if (pmc->mask) {
+		const uint64_t cur_value = __get_umwait_val(
+				pmc->addr, pmc->data_sz);
+		const uint64_t masked = cur_value & pmc->mask;
 
 		/* if the masked value is already matching, abort */
-		if (masked == expected_value)
+		if (masked == pmc->val)
 			return 0;
 	}
 	rte_spinlock_unlock(lck);
-- 
2.25.1