[dpdk-dev] [PATCH v5 03/10] eal: add intrinsics support check infrastructure

[Jerin Jacob]

On Fri, Oct 9, 2020 at 9:32 PM Anatoly Burakov
<anatoly.burakov at 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.burakov at 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