On Fri, Oct 9, 2020 at 9:32 PM Anatoly Burakov
<anatoly.bura...@intel.com> wrote:
>
> Currently, it is not possible to check support for intrinsics that
> are platform-specific, cannot be abstracted in a generic way, or do not
> have support on all architectures. The CPUID flags can be used to some
> extent, but they are only defined for their platform, while intrinsics
> will be available to all code as they are in generic headers.
>
> This patch introduces infrastructure to check support for certain
> platform-specific intrinsics, and adds support for checking support for
> IA power management-related intrinsics for UMWAIT/UMONITOR and TPAUSE.
>
> Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
> ---
> .../arm/include/rte_power_intrinsics.h | 8 ++++++
> lib/librte_eal/arm/rte_cpuflags.c | 6 +++++
> lib/librte_eal/include/generic/rte_cpuflags.h | 26 +++++++++++++++++++
> .../include/generic/rte_power_intrinsics.h | 8 ++++++
> .../ppc/include/rte_power_intrinsics.h | 8 ++++++
> lib/librte_eal/ppc/rte_cpuflags.c | 6 +++++
> lib/librte_eal/rte_eal_version.map | 1 +
> .../x86/include/rte_power_intrinsics.h | 8 ++++++
> lib/librte_eal/x86/rte_cpuflags.c | 12 +++++++++
> 9 files changed, 83 insertions(+)
>
> diff --git a/lib/librte_eal/arm/include/rte_power_intrinsics.h
> b/lib/librte_eal/arm/include/rte_power_intrinsics.h
> index 4aad44a0b9..055ec5877a 100644
> --- a/lib/librte_eal/arm/include/rte_power_intrinsics.h
> +++ b/lib/librte_eal/arm/include/rte_power_intrinsics.h
> @@ -17,6 +17,10 @@ extern "C" {
> /**
> * This function is not supported on ARM.
> *
> + * @warning It is responsibility of the user to check if this function is
> + * supported at runtime using `rte_cpu_get_features()` API call. Failing
> to do
> + * so may result in an illegal CPU instruction error.
See below
> + *
> * @param p
> * Address to monitor for changes. Must be aligned on an 64-byte boundary.
> * @param expected_value
> @@ -43,6 +47,10 @@ static inline void rte_power_monitor(const volatile void
> *p,
> /**
> * This function is not supported on ARM.
> *
> + * @warning It is responsibility of the user to check if this function is
> + * supported at runtime using `rte_cpu_get_features()` API call. Failing
> to do
> + * so may result in an illegal CPU instruction error.
> + *
See below
This patch looks to me.
Since rte_power_monitor() API is public API, I think, only in the
generic header file, you need to have
these warnings and API documentation rather than repeating everywhere.
> * @param tsc_timestamp
> * Maximum TSC timestamp to wait for.
> *
> diff --git a/lib/librte_eal/arm/rte_cpuflags.c
> b/lib/librte_eal/arm/rte_cpuflags.c
> index caf3dc83a5..7eef11fa02 100644
> --- a/lib/librte_eal/arm/rte_cpuflags.c
> +++ b/lib/librte_eal/arm/rte_cpuflags.c
> @@ -138,3 +138,9 @@ rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
> return NULL;
> return rte_cpu_feature_table[feature].name;
> }
> +
> +void
> +rte_cpu_get_intrinsics_support(struct rte_cpu_intrinsics *intrinsics)
> +{
> + memset(intrinsics, 0, sizeof(*intrinsics));
> +}
> diff --git a/lib/librte_eal/include/generic/rte_cpuflags.h
> b/lib/librte_eal/include/generic/rte_cpuflags.h
> index 872f0ebe3e..28a5aecde8 100644
> --- a/lib/librte_eal/include/generic/rte_cpuflags.h
> +++ b/lib/librte_eal/include/generic/rte_cpuflags.h
> @@ -13,6 +13,32 @@
> #include "rte_common.h"
> #include <errno.h>
>
> +#include <rte_compat.h>
> +
> +/**
> + * Structure used to describe platform-specific intrinsics that may or may
> not
> + * be supported at runtime.
> + */
> +struct rte_cpu_intrinsics {
> + uint32_t power_monitor : 1;
> + /**< indicates support for rte_power_monitor function */
> + uint32_t power_pause : 1;
> + /**< indicates support for rte_power_pause function */
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Check CPU support for various intrinsics at runtime.
> + *
> + * @param intrinsics
> + * Pointer to a structure to be filled.
> + */
> +__rte_experimental
> +void
> +rte_cpu_get_intrinsics_support(struct rte_cpu_intrinsics *intrinsics);
> +
> /**
> * Enumeration of all CPU features supported
> */
> diff --git a/lib/librte_eal/include/generic/rte_power_intrinsics.h
> b/lib/librte_eal/include/generic/rte_power_intrinsics.h
> index e36c1f8976..218eda7e86 100644
> --- a/lib/librte_eal/include/generic/rte_power_intrinsics.h
> +++ b/lib/librte_eal/include/generic/rte_power_intrinsics.h
> @@ -26,6 +26,10 @@
> * checked against the expected value, and if they match, the entering of
> * optimized power state may be aborted.
> *
> + * @warning It is responsibility of the user to check if this function is
> + * supported at runtime using `rte_cpu_get_features()` API call. Failing
> to do
> + * so may result in an illegal CPU instruction error.
See above
> + *
> * @param p
> * Address to monitor for changes. Must be aligned on an 64-byte boundary.
> * @param expected_value
> @@ -49,6 +53,10 @@ static inline void rte_power_monitor(const volatile void
> *p,
> * Enter an architecture-defined optimized power state until a certain TSC
> * timestamp is reached.
> *
> + * @warning It is responsibility of the user to check if this function is
> + * supported at runtime using `rte_cpu_get_features()` API call. Failing
> to do
> + * so may result in an illegal CPU instruction error.
> + *
> * @param tsc_timestamp
> * Maximum TSC timestamp to wait for. Note that the wait behavior is
> * architecture-dependent.
> diff --git a/lib/librte_eal/ppc/include/rte_power_intrinsics.h
> b/lib/librte_eal/ppc/include/rte_power_intrinsics.h
> index 70fd7b094f..d63ad86849 100644
> --- a/lib/librte_eal/ppc/include/rte_power_intrinsics.h
> +++ b/lib/librte_eal/ppc/include/rte_power_intrinsics.h
> @@ -17,6 +17,10 @@ extern "C" {
> /**
> * This function is not supported on PPC64.
> *
> + * @warning It is responsibility of the user to check if this function is
> + * supported at runtime using `rte_cpu_get_features()` API call. Failing
> to do
> + * so may result in an illegal CPU instruction error.
> + *
> * @param p
> * Address to monitor for changes. Must be aligned on an 64-byte boundary.
> * @param expected_value
> @@ -43,6 +47,10 @@ static inline void rte_power_monitor(const volatile void
> *p,
> /**
> * This function is not supported on PPC64.
> *
> + * @warning It is responsibility of the user to check if this function is
> + * supported at runtime using `rte_cpu_get_features()` API call. Failing
> to do
> + * so may result in an illegal CPU instruction error.
> + *
> * @param tsc_timestamp
> * Maximum TSC timestamp to wait for.
> *
> diff --git a/lib/librte_eal/ppc/rte_cpuflags.c
> b/lib/librte_eal/ppc/rte_cpuflags.c
> index 3bb7563ce9..eee8234384 100644
> --- a/lib/librte_eal/ppc/rte_cpuflags.c
> +++ b/lib/librte_eal/ppc/rte_cpuflags.c
> @@ -108,3 +108,9 @@ rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
> return NULL;
> return rte_cpu_feature_table[feature].name;
> }
> +
> +void
> +rte_cpu_get_intrinsics_support(struct rte_cpu_intrinsics *intrinsics)
> +{
> + memset(intrinsics, 0, sizeof(*intrinsics));
> +}
> diff --git a/lib/librte_eal/rte_eal_version.map
> b/lib/librte_eal/rte_eal_version.map
> index a93dea9fe6..ed944f2bd4 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -400,6 +400,7 @@ EXPERIMENTAL {
> # added in 20.11
> __rte_eal_trace_generic_size_t;
> rte_service_lcore_may_be_active;
> + rte_cpu_get_intrinsics_support;
> };
>
> INTERNAL {
> diff --git a/lib/librte_eal/x86/include/rte_power_intrinsics.h
> b/lib/librte_eal/x86/include/rte_power_intrinsics.h
> index 8d579eaf64..3afc165a1f 100644
> --- a/lib/librte_eal/x86/include/rte_power_intrinsics.h
> +++ b/lib/librte_eal/x86/include/rte_power_intrinsics.h
> @@ -29,6 +29,10 @@ extern "C" {
> * For more information about usage of these instructions, please refer to
> * Intel(R) 64 and IA-32 Architectures Software Developer's Manual.
> *
> + * @warning It is responsibility of the user to check if this function is
> + * supported at runtime using `rte_cpu_get_features()` API call. Failing
> to do
> + * so may result in an illegal CPU instruction error.
> + *
> * @param p
> * Address to monitor for changes. Must be aligned on an 64-byte boundary.
> * @param expected_value
> @@ -80,6 +84,10 @@ static inline void rte_power_monitor(const volatile void
> *p,
> * information about usage of this instruction, please refer to Intel(R) 64
> and
> * IA-32 Architectures Software Developer's Manual.
> *
> + * @warning It is responsibility of the user to check if this function is
> + * supported at runtime using `rte_cpu_get_features()` API call. Failing
> to do
> + * so may result in an illegal CPU instruction error.
> + *
> * @param tsc_timestamp
> * Maximum TSC timestamp to wait for.
> *
> diff --git a/lib/librte_eal/x86/rte_cpuflags.c
> b/lib/librte_eal/x86/rte_cpuflags.c
> index 0325c4b93b..a96312ff7f 100644
> --- a/lib/librte_eal/x86/rte_cpuflags.c
> +++ b/lib/librte_eal/x86/rte_cpuflags.c
> @@ -7,6 +7,7 @@
> #include <stdio.h>
> #include <errno.h>
> #include <stdint.h>
> +#include <string.h>
>
> #include "rte_cpuid.h"
>
> @@ -179,3 +180,14 @@ rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
> return NULL;
> return rte_cpu_feature_table[feature].name;
> }
> +
> +void
> +rte_cpu_get_intrinsics_support(struct rte_cpu_intrinsics *intrinsics)
> +{
> + memset(intrinsics, 0, sizeof(*intrinsics));
> +
> + if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_WAITPKG)) {
> + intrinsics->power_monitor = 1;
> + intrinsics->power_pause = 1;
> + }
> +}
> --
> 2.17.1
