From patchwork Thu Jan 14 14:46:05 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Burakov, Anatoly" X-Patchwork-Id: 86627 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 39560A0A02; Thu, 14 Jan 2021 15:46:46 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EA90614130D; Thu, 14 Jan 2021 15:46:26 +0100 (CET) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 6687B14130B for ; Thu, 14 Jan 2021 15:46:25 +0100 (CET) IronPort-SDR: hkUoUTPyqpxBU3yA0KE8neBx4Szpq8TqTq+wsbKJNxXyc28njURnmudg7vuRcEnYW37n+r5j/J EVQHQ0BoKQPA== X-IronPort-AV: E=McAfee;i="6000,8403,9863"; a="174870251" X-IronPort-AV: E=Sophos;i="5.79,347,1602572400"; d="scan'208";a="174870251" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Jan 2021 06:46:25 -0800 IronPort-SDR: BBR4iomTkSE7fTToWEqgqTbFm9vDz/ooR6bP1kp9L25AFEW3d1zJdii3wcI0F3gcEqStiy/y2w 8efV3Yvcxvgw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.79,347,1602572400"; d="scan'208";a="465271307" Received: from silpixa00399498.ir.intel.com (HELO silpixa00399498.ger.corp.intel.com) ([10.237.222.179]) by fmsmga001.fm.intel.com with ESMTP; 14 Jan 2021 06:46:22 -0800 From: Anatoly Burakov To: dev@dpdk.org Cc: Timothy McDaniel , Jan Viktorin , Ruifeng Wang , Jerin Jacob , David Christensen , Bruce Richardson , Konstantin Ananyev , thomas@monjalon.net, david.hunt@intel.com, chris.macnamara@intel.com Date: Thu, 14 Jan 2021 14:46:05 +0000 Message-Id: X-Mailer: git-send-email 2.25.1 In-Reply-To: References: MIME-Version: 1.0 Subject: [dpdk-dev] [PATCH v17 03/11] eal: change API of power intrinsics X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 Acked-by: Konstantin Ananyev --- 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);