On 14-Oct-20 6:48 PM, Ananyev, Konstantin wrote:
From: Liang Ma <liang.j...@intel.com>
Add two new power management intrinsics, and provide an implementation
in eal/x86 based on UMONITOR/UMWAIT instructions. The instructions
are implemented as raw byte opcodes because there is not yet widespread
compiler support for these instructions.
The power management instructions provide an architecture-specific
function to either wait until a specified TSC timestamp is reached, or
optionally wait until either a TSC timestamp is reached or a memory
location is written to. The monitor function also provides an optional
comparison, to avoid sleeping when the expected write has already
happened, and no more writes are expected.
For more details, please refer to Intel(R) 64 and IA-32 Architectures
Software Developer's Manual, Volume 2.
Signed-off-by: Liang Ma <liang.j...@intel.com>
Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
Acked-by: David Christensen <d...@linux.vnet.ibm.com>
---
Notes:
v6:
- Add spinlock-enabled version to allow pthread-wait-like
constructs with umwait
- Clarify comments
- Added experimental tags to intrinsics
- Added endianness support
v5:
- Removed return values
- Simplified intrinsics and hardcoded C0.2 state
- Added other arch stubs
lib/librte_eal/arm/include/meson.build | 1 +
.../arm/include/rte_power_intrinsics.h | 58 ++++++++
.../include/generic/rte_power_intrinsics.h | 111 +++++++++++++++
lib/librte_eal/include/meson.build | 1 +
lib/librte_eal/ppc/include/meson.build | 1 +
.../ppc/include/rte_power_intrinsics.h | 58 ++++++++
lib/librte_eal/x86/include/meson.build | 1 +
.../x86/include/rte_power_intrinsics.h | 132 ++++++++++++++++++
8 files changed, 363 insertions(+)
create mode 100644 lib/librte_eal/arm/include/rte_power_intrinsics.h
create mode 100644 lib/librte_eal/include/generic/rte_power_intrinsics.h
create mode 100644 lib/librte_eal/ppc/include/rte_power_intrinsics.h
create mode 100644 lib/librte_eal/x86/include/rte_power_intrinsics.h
diff --git a/lib/librte_eal/arm/include/meson.build
b/lib/librte_eal/arm/include/meson.build
index 73b750a18f..c6a9f70d73 100644
--- a/lib/librte_eal/arm/include/meson.build
+++ b/lib/librte_eal/arm/include/meson.build
@@ -20,6 +20,7 @@ arch_headers = files(
'rte_pause_32.h',
'rte_pause_64.h',
'rte_pause.h',
+'rte_power_intrinsics.h',
'rte_prefetch_32.h',
'rte_prefetch_64.h',
'rte_prefetch.h',
diff --git a/lib/librte_eal/arm/include/rte_power_intrinsics.h
b/lib/librte_eal/arm/include/rte_power_intrinsics.h
new file mode 100644
index 0000000000..b04ba10c76
--- /dev/null
+++ b/lib/librte_eal/arm/include/rte_power_intrinsics.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Intel Corporation
+ */
+
+#ifndef _RTE_POWER_INTRINSIC_ARM_H_
+#define _RTE_POWER_INTRINSIC_ARM_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <rte_common.h>
+
+#include "generic/rte_power_intrinsics.h"
+
+/**
+ * This function is not supported on ARM.
+ */
Here and in other places - please follow dpdk coding convention
for function definitions, i.e:
static inline void
rte_power_monitor(...
Yep, will do.
+static inline void 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_SET_USED(p);
+RTE_SET_USED(expected_value);
+RTE_SET_USED(value_mask);
+RTE_SET_USED(tsc_timestamp);
+RTE_SET_USED(data_sz);
+}
You can probably put NOP implementations of these rte_powe_* functions
into generic/rte_power_intrinsics.h.
So, wouldn't need to duplicate them for every non-supported arch.
Same as it was done for rte_wait_until_equal_*().
Will look into it.
+ *
+ * This file define APIs for advanced power management,
+ * which are architecture-dependent.
+ */
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Monitor specific address for changes. This will cause the CPU to enter an
+ * architecture-defined optimized power state until either the specified
+ * memory address is written to, a certain TSC timestamp is reached, or other
+ * reasons cause the CPU to wake up.
+ *
+ * Additionally, an `expected` 64-bit value and 64-bit mask are provided. If
+ * mask is non-zero, the current value pointed to by the `p` pointer will be
+ * checked against the expected value, and if they match, the entering of
+ * optimized power state may be aborted.
+ *
+ * @param p
+ * Address to monitor for changes. Must be aligned on an 64-byte boundary.
Is 64B alignment really needed?
I'm not 100% sure to be honest, but it's there just in case. I can
remove it.
'rte_prefetch.h',
+'rte_power_intrinsics.h',
'rte_pause.h',
'rte_rtm.h',
'rte_rwlock.h',
diff --git a/lib/librte_eal/x86/include/rte_power_intrinsics.h
b/lib/librte_eal/x86/include/rte_power_intrinsics.h
new file mode 100644
index 0000000000..9ac8e6eef6
--- /dev/null
+++ b/lib/librte_eal/x86/include/rte_power_intrinsics.h
@@ -0,0 +1,132 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Intel Corporation
+ */
+
+#ifndef _RTE_POWER_INTRINSIC_X86_64_H_
+#define _RTE_POWER_INTRINSIC_X86_64_H_
Why '_64_H'?
My understanding was these ops are supported 32-bit mode too.
Yeah, artifact of early implementation. Will fix.
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <rte_common.h>
+
+#include "generic/rte_power_intrinsics.h"
+
+static inline uint64_t __get_umwait_val(const volatile void *p,
+const uint8_t sz)
+{
+switch (sz) {
+case 1:
Just as a nit:
case sizeof(type_x):
return *(const volatile type_x *)p;
Thanks, will fix.
+return *(const volatile uint8_t *)p;
+case 2:
+return *(const volatile uint16_t *)p;
+case 4:
+return *(const volatile uint32_t *)p;
+case 8:
+return *(const volatile uint64_t *)p;
+default:
+/* this is an intrinsic, so we can't have any error handling */
RTE_ASSERT(0); ?
Great idea, will add.
--
Thanks,
Anatoly
