Re: [dpdk-dev] [PATCH v3 1/6] eal: add power management intrinsics

[Jerin Jacob <jerinjacobk@gmail.com>]

On Fri, Sep 4, 2020 at 3:49 PM Liang Ma <liang.j.ma@intel.com> wrote:
>
> 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.
>
> Signed-off-by: Liang Ma <liang.j.ma@intel.com>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>


> +
> +#include "generic/rte_power_intrinsics.h"
> +
> +/**
> + * 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, or a certain TSC timestamp is reached.
> + *
> + * 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.
> + *
> + * This function uses UMONITOR/UMWAIT instructions. For more information about
> + * their usage, please refer to Intel(R) 64 and IA-32 Architectures Software
> + * Developer's Manual.
[snip]
> + */
> +static inline int rte_power_monitor(const volatile void *p,
> +               const uint64_t expected_value, const uint64_t value_mask,
> +               const uint32_t state, const uint64_t tsc_timestamp)

IMO, We must introduce some arch feature-capability _get_ scheme to tell
the consumer of this API is only supported on x86. Probably as functions[1]
or macro flags scheme and have a stub for the other architectures as the
API marked as generic ie rte_power_* not rte_x86_..

This will help the consumer to create workers based on the instruction features
which can NOT be abstracted as a generic feature across the architectures.


[1]
struct rte_arch_inst_feat {
        uint32_t power_monitor      : 1;  /**< Power monitor */
...
}

void rte_arch_inst_feat_get(struct rte_arch_inst_feat *feat);