[dpdk-dev] [PATCH v13 02/11] eal: avoid invalid API usage in power intrinsics

[Ananyev, Konstantin]

> Currently, the API documentation mandates that if the user wants to use
> the power management intrinsics, they need to call the
> `rte_cpu_get_intrinsics_support` API and check support for specific
> intrinsics.
> 
> However, if the user does not do that, it is possible to get illegal
> instruction error because we're using raw instruction opcodes, which may
> or may not be supported at runtime.
> 
> Now that we have everything in a C file, we can check for support at
> startup and prevent the user from possibly encountering illegal
> instruction errors.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
> ---
>  .../include/generic/rte_power_intrinsics.h    |  3 --
>  lib/librte_eal/x86/rte_power_intrinsics.c     | 31 +++++++++++++++++--
>  2 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/librte_eal/include/generic/rte_power_intrinsics.h b/lib/librte_eal/include/generic/rte_power_intrinsics.h
> index 67977bd511..ffa72f7578 100644
> --- a/lib/librte_eal/include/generic/rte_power_intrinsics.h
> +++ b/lib/librte_eal/include/generic/rte_power_intrinsics.h
> @@ -34,7 +34,6 @@
>   *
>   * @warning It is responsibility of the user to check if this function is
>   *   supported at runtime using `rte_cpu_get_intrinsics_support()` API call.
> - *   Failing to do so may result in an illegal CPU instruction error.
>   *
>   * @param p
>   *   Address to monitor for changes.
> @@ -75,7 +74,6 @@ void 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.
> - *   Failing to do so may result in an illegal CPU instruction error.
>   *
>   * @param p
>   *   Address to monitor for changes.
> @@ -111,7 +109,6 @@ void rte_power_monitor_sync(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.
> - *   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
> diff --git a/lib/librte_eal/x86/rte_power_intrinsics.c b/lib/librte_eal/x86/rte_power_intrinsics.c
> index 34c5fd9c3e..b48a54ec7f 100644
> --- a/lib/librte_eal/x86/rte_power_intrinsics.c
> +++ b/lib/librte_eal/x86/rte_power_intrinsics.c
> @@ -4,6 +4,8 @@
> 
>  #include "rte_power_intrinsics.h"
> 
> +static uint8_t wait_supported;
> +
>  static inline uint64_t
>  __get_umwait_val(const volatile void *p, const uint8_t sz)
>  {
> @@ -35,6 +37,11 @@ rte_power_monitor(const volatile void *p, const uint64_t expected_value,
>  {
>  	const uint32_t tsc_l = (uint32_t)tsc_timestamp;
>  	const uint32_t tsc_h = (uint32_t)(tsc_timestamp >> 32);
> +
> +	/* prevent user from running this instruction if it's not supported */
> +	if (!wait_supported)
> +		return;
> +
>  	/*
>  	 * we're using raw byte codes for now as only the newest compiler
>  	 * versions support this instruction natively.
> @@ -72,6 +79,11 @@ rte_power_monitor_sync(const volatile void *p, const uint64_t expected_value,
>  {
>  	const uint32_t tsc_l = (uint32_t)tsc_timestamp;
>  	const uint32_t tsc_h = (uint32_t)(tsc_timestamp >> 32);
> +
> +	/* prevent user from running this instruction if it's not supported */
> +	if (!wait_supported)
> +		return;
> +
>  	/*
>  	 * we're using raw byte codes for now as only the newest compiler
>  	 * versions support this instruction natively.
> @@ -112,9 +124,22 @@ rte_power_pause(const uint64_t tsc_timestamp)
>  	const uint32_t tsc_l = (uint32_t)tsc_timestamp;
>  	const uint32_t tsc_h = (uint32_t)(tsc_timestamp >> 32);
> 
> +	/* prevent user from running this instruction if it's not supported */
> +	if (!wait_supported)
> +		return;
> +
>  	/* execute TPAUSE */
>  	asm volatile(".byte 0x66, 0x0f, 0xae, 0xf7;"
> -			: /* ignore rflags */
> -			: "D"(0), /* enter C0.2 */
> -			"a"(tsc_l), "d"(tsc_h));
> +		: /* ignore rflags */
> +		: "D"(0), /* enter C0.2 */
> +		  "a"(tsc_l), "d"(tsc_h));
> +}
> +
> +RTE_INIT(rte_power_intrinsics_init) {
> +	struct rte_cpu_intrinsics i;
> +
> +	rte_cpu_get_intrinsics_support(&i);
> +
> +	if (i.power_monitor && i.power_pause)
> +		wait_supported = 1;
>  }
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev at intel.com>

> 2.25.1