In order to get more accurate the cntvct_el0 reading, SW must invoke isb and arch_counter_enforce_ordering. Reference of linux kernel: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/ linux.git/tree/arch/arm64/include/asm/arch_timer.h?h=v5.5#n220 Fixes: ccad39ea0712 ("eal/arm: add cpu cycle operations for ARMv8") Cc: stable@dpdk.org Reviewed-by: Jerin Jacob <jerinjacobk@gmail.com> Reviewed-by: Gavin Hu <gavin.hu@arm.com> Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com> --- .../common/include/arch/arm/rte_cycles_64.h | 26 +++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/lib/librte_eal/common/include/arch/arm/rte_cycles_64.h b/lib/librte_eal/common/include/arch/arm/rte_cycles_64.h index 68e7c7338..6b0df5b0a 100644 --- a/lib/librte_eal/common/include/arch/arm/rte_cycles_64.h +++ b/lib/librte_eal/common/include/arch/arm/rte_cycles_64.h @@ -59,11 +59,33 @@ rte_rdtsc(void) } #endif +static inline void +isb(void) +{ + asm volatile("isb" : : : "memory"); +} + +static inline void +__rte_arm64_cntvct_el0_enforce_ordering(uint64_t val) +{ + uint64_t tmp; + + asm volatile( + " eor %0, %1, %1\n" + " add %0, sp, %0\n" + " ldr xzr, [%0]" + : "=r" (tmp) : "r" (val)); +} + static inline uint64_t rte_rdtsc_precise(void) { - rte_mb(); - return rte_rdtsc(); + uint64_t tsc; + + isb(); + tsc = rte_rdtsc(); + __rte_arm64_cntvct_el0_enforce_ordering(tsc); + return tsc; } static inline uint64_t -- 2.24.1.windows.2
On Wed, Mar 11, 2020 at 8:24 AM Linhaifeng <haifeng.lin@huawei.com> wrote: > > In order to get more accurate the cntvct_el0 reading, > SW must invoke isb and arch_counter_enforce_ordering. > > Reference of linux kernel: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/ > linux.git/tree/arch/arm64/include/asm/arch_timer.h?h=v5.5#n220 > > Fixes: ccad39ea0712 ("eal/arm: add cpu cycle operations for ARMv8") > Cc: stable@dpdk.org > > Reviewed-by: Jerin Jacob <jerinjacobk@gmail.com> > Reviewed-by: Gavin Hu <gavin.hu@arm.com> > Signed-off-by: Haifeng Lin <haifeng.lin@huawei.com> > --- > .../common/include/arch/arm/rte_cycles_64.h | 26 +++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/lib/librte_eal/common/include/arch/arm/rte_cycles_64.h b/lib/librte_eal/common/include/arch/arm/rte_cycles_64.h > index 68e7c7338..6b0df5b0a 100644 > --- a/lib/librte_eal/common/include/arch/arm/rte_cycles_64.h > +++ b/lib/librte_eal/common/include/arch/arm/rte_cycles_64.h > @@ -59,11 +59,33 @@ rte_rdtsc(void) > } > #endif > > +static inline void > +isb(void) > +{ > + asm volatile("isb" : : : "memory"); > +} NAK. Don't export badly named stuff like this. > + > +static inline void > +__rte_arm64_cntvct_el0_enforce_ordering(uint64_t val) > +{ > + uint64_t tmp; > + > + asm volatile( > + " eor %0, %1, %1\n" > + " add %0, sp, %0\n" > + " ldr xzr, [%0]" > + : "=r" (tmp) : "r" (val)); > +} > + I can see no point in exporting this. Please move this inline of rte_rdtsc_precise(). If one day, ARM needs this elsewhere, we can reevaluate and introduce a helper, but I don't see this atm. > static inline uint64_t > rte_rdtsc_precise(void) > { > - rte_mb(); > - return rte_rdtsc(); > + uint64_t tsc; > + > + isb(); > + tsc = rte_rdtsc(); > + __rte_arm64_cntvct_el0_enforce_ordering(tsc); > + return tsc; > } > > static inline uint64_t > -- > 2.24.1.windows.2 > -- David Marchand
> > +static inline void > > +isb(void) > > +{ > > + asm volatile("isb" : : : "memory"); } > > NAK. > > Don't export badly named stuff like this. > Just use asm volatile("isb" : : : "memory") in rte_rdtsc_precise or which file I should use to define this maco > > + > > +static inline void > > +__rte_arm64_cntvct_el0_enforce_ordering(uint64_t val) { > > + uint64_t tmp; > > + > > + asm volatile( > > + " eor %0, %1, %1\n" > > + " add %0, sp, %0\n" > > + " ldr xzr, [%0]" > > + : "=r" (tmp) : "r" (val)); > > +} > > + > > I can see no point in exporting this. > Please move this inline of rte_rdtsc_precise(). > If one day, ARM needs this elsewhere, we can reevaluate and introduce a > helper, but I don't see this atm. > Ok. I will remove it. > > -- > David Marchand