19/06/2024 08:45, Wathsala Vithanage:
> --- a/lib/eal/arm/include/rte_cpuflags_64.h
> +++ b/lib/eal/arm/include/rte_cpuflags_64.h
> @@ -36,6 +36,7 @@ enum rte_cpu_flag_t {
> RTE_CPUFLAG_SVEF64MM,
> RTE_CPUFLAG_SVEBF16,
> RTE_CPUFLAG_AARCH64,
> + RTE_CPUFLAG_WFXT,
> };
It may be useful to add comments explaining each flag.
May be a separate patch in this series?
> - * Copyright(c) 2019 Arm Limited
> + * Copyright(c) 2024 Arm Limited
No, it's wrong to remove initial date,
and no, you don't need to update dates at all.
> -#ifdef RTE_WAIT_UNTIL_EQUAL_ARCH_DEFINED
Why removing this #ifdef?
> -/* Send a local event to quit WFE. */
> +/* Send a local event to quit WFE/WFxT. */
> #define __RTE_ARM_SEVL() { asm volatile("sevl" : : : "memory"); }
>
> -/* Send a global event to quit WFE for all cores. */
> +/* Send a global event to quit WFE/WFxT for all cores. */
> #define __RTE_ARM_SEV() { asm volatile("sev" : : : "memory"); }
>
> /* Put processor into low power WFE(Wait For Event) state. */
> #define __RTE_ARM_WFE() { asm volatile("wfe" : : : "memory"); }
>
> +/* Put processor into low power WFET (WFE with Timeout) state. */
> +#ifdef RTE_ARM_FEATURE_WFXT
> +#define __RTE_ARM_WFET(t) { \
> + asm volatile("wfet %x[to]" \
> + : \
> + : [to] "r" (t) \
> + : "memory"); \
> + }
Is there any intrinsic function available?
[...]
> --- a/lib/eal/arm/rte_cpuflags.c
> +++ b/lib/eal/arm/rte_cpuflags.c
> @@ -115,6 +115,7 @@ const struct feature_entry rte_cpu_feature_table[] = {
> FEAT_DEF(SVEF32MM, REG_HWCAP2, 10)
> FEAT_DEF(SVEF64MM, REG_HWCAP2, 11)
> FEAT_DEF(SVEBF16, REG_HWCAP2, 12)
> + FEAT_DEF(WFXT, REG_HWCAP2, 31)
> FEAT_DEF(AARCH64, REG_PLATFORM, 0)
Are you sure of alignment? (looks wrong in my email client)
[...]
> rte_cpu_get_intrinsics_support(struct rte_cpu_intrinsics *intrinsics)
> {
> memset(intrinsics, 0, sizeof(*intrinsics));
> -#ifdef RTE_ARM_USE_WFE
> intrinsics->power_monitor = 1;
> -#endif
Why removing this #ifdef?
> +uint8_t wfet_en;
It should be made static probably.
This variable will be unused in some cases, needs #ifdef.
> +
> +RTE_INIT(rte_power_intrinsics_init)
> +{
> +#ifdef RTE_ARCH_64
> + if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_WFXT))
> + wfet_en = 1;
> +#endif
> +}
> +
> +/**
> + * This function uses WFE/WFET instruction to make lcore suspend
> * execution on ARM.
> - * Note that timestamp based timeout is not supported yet.
> */
> int
> rte_power_monitor(const struct rte_power_monitor_cond *pmc,
> const uint64_t tsc_timestamp)
> {
> - RTE_SET_USED(tsc_timestamp);
> -
> -#ifdef RTE_ARM_USE_WFE
> +#ifdef RTE_ARCH_64
It looks wrong.
If RTE_ARM_USE_WFE is disabled, you should not call __RTE_ARM_WFE().
> const unsigned int lcore_id = rte_lcore_id();
> uint64_t cur_value;
>
> @@ -33,28 +44,30 @@ rte_power_monitor(const struct rte_power_monitor_cond
> *pmc,
>
> switch (pmc->size) {
> case sizeof(uint8_t):
> - __RTE_ARM_LOAD_EXC_8(pmc->addr, cur_value,
> rte_memory_order_relaxed)
> - __RTE_ARM_WFE()
> + __RTE_ARM_LOAD_EXC_8(pmc->addr, cur_value,
> rte_memory_order_relaxed);
