On Tue, Apr 10, 2018 at 09:16:55PM +0200, Paul Semel wrote:
> this file is introduce to be able to implement an inter domain
> communication protocol over xenstore. For synchronization purpose, we do
> really want to be able to "control" time
>
> common/time.c: since_boot_time gets the time in nanoseconds from the
> moment the VM has booted
>
> Signed-off-by: Paul Semel <phen...@amazon.de>
> ---
>
> Notes:
> v4:
> - moved rdtsc to arch/x86/include/arch/lib.h
> - added a rdtsc_ordered implementation to serialize rdtsc
> - simplified since_boot_time function
> - still need to have Andrew's scale_delta version
>
> arch/x86/include/arch/lib.h | 18 ++++++++++
> build/files.mk | 1 +
> common/time.c | 81
> +++++++++++++++++++++++++++++++++++++++++++++
> include/xtf/time.h | 24 ++++++++++++++
> 4 files changed, 124 insertions(+)
> create mode 100644 common/time.c
> create mode 100644 include/xtf/time.h
>
> diff --git a/arch/x86/include/arch/lib.h b/arch/x86/include/arch/lib.h
> index 0045902..510cdb1 100644
> --- a/arch/x86/include/arch/lib.h
> +++ b/arch/x86/include/arch/lib.h
> @@ -6,6 +6,7 @@
> #include <xen/arch-x86/xen.h>
> #include <arch/desc.h>
> #include <arch/msr.h>
> +#include <arch/barrier.h>
This include list is sorted alphabetically.
>
> static inline void cpuid(uint32_t leaf,
> uint32_t *eax, uint32_t *ebx,
> @@ -374,6 +375,23 @@ static inline void write_xcr0(uint64_t xcr0)
> xsetbv(0, xcr0);
> }
>
> +static inline uint64_t rdtsc(void)
> +{
> + uint32_t lo, hi;
> +
> + asm volatile("rdtsc": "=a"(lo), "=d"(hi));
> +
> + return ((uint64_t)hi << 32) | lo;
> +}
> +
> +static inline uint64_t rdtsc_ordered(void)
> +{
> + rmb();
> + mb();
> +
> + return rdtsc();
> +}
I would likely just add a single function like:
static inline uint64_t rdtsc_ordered(void)
{
uint32_t lo, hi;
asm volatile("lfence; mfence; rdtsc" : "=a" (lo), "=d" (hi));
return ((uint64_t)hi << 32) | lo;
}
Because there's no external caller of rdtsc, but I will leave that to
Andrew to decide. In any case this should work fine.
> +
> #endif /* XTF_X86_LIB_H */
>
> /*
> diff --git a/build/files.mk b/build/files.mk
> index 46b42d6..55ed1ca 100644
> --- a/build/files.mk
> +++ b/build/files.mk
> @@ -16,6 +16,7 @@ obj-perarch += $(ROOT)/common/libc/vsnprintf.o
> obj-perarch += $(ROOT)/common/report.o
> obj-perarch += $(ROOT)/common/setup.o
> obj-perarch += $(ROOT)/common/xenbus.o
> +obj-perarch += $(ROOT)/common/time.o
>
> obj-perenv += $(ROOT)/arch/x86/decode.o
> obj-perenv += $(ROOT)/arch/x86/desc.o
> diff --git a/common/time.c b/common/time.c
> new file mode 100644
> index 0000000..79abc7e
> --- /dev/null
> +++ b/common/time.c
> @@ -0,0 +1,81 @@
> +#include <xtf/types.h>
> +#include <xtf/traps.h>
> +#include <xtf/time.h>
Alphabetic order please.
> +#include <arch/barrier.h>
> +#include <arch/lib.h>
> +
> +/* This function was taken from mini-os source code */
> +/* It returns ((delta << shift) * mul_frac) >> 32 */
Comment has wrong style, please check CODING_STYLE.
> +static inline uint64_t scale_delta(uint64_t delta, uint32_t mul_frac, int
> shift)
> +{
> + uint64_t product;
> +#ifdef __i386__
> + uint32_t tmp1, tmp2;
> +#endif
> +
> + if ( shift < 0 )
> + delta >>= -shift;
> + else
> + delta <<= shift;
> +
> +#ifdef __i386__
> + __asm__ (
> + "mul %5 ; "
> + "mov %4,%%eax ; "
> + "mov %%edx,%4 ; "
> + "mul %5 ; "
> + "add %4,%%eax ; "
> + "xor %5,%5 ; "
> + "adc %5,%%edx ; "
> + : "=A" (product), "=r" (tmp1), "=r" (tmp2)
> + : "a" ((uint32_t)delta), "1" ((uint32_t)(delta >> 32)), "2"
> (mul_frac) );
This line is too long.
> +#else
> + __asm__ (
> + "mul %%rdx ; shrd $32,%%rdx,%%rax"
> + : "=a" (product) : "0" (delta), "d" ((uint64_t)mul_frac) );
Not sure whether you need to add a ': "d"' clobber here, since the d
register is used but it's not in the list of output operands.
> +#endif
> +
> + return product;
> +}
> +
> +
> +uint64_t since_boot_time(void)
> +{
> + uint32_t ver1, ver2;
> + uint64_t tsc_timestamp, system_time, tsc;
> + uint32_t tsc_to_system_mul;
> + int8_t tsc_shift;
> +
> + do
> + {
> + ver1 = ACCESS_ONCE(shared_info.vcpu_info[0].time.version);
> + smp_rmb();
> +
> + system_time = shared_info.vcpu_info[0].time.system_time;
> + tsc_timestamp = shared_info.vcpu_info[0].time.tsc_timestamp;
> + tsc_to_system_mul = shared_info.vcpu_info[0].time.tsc_to_system_mul;
> + tsc_shift = shared_info.vcpu_info[0].time.tsc_shift;
> + tsc = rdtsc_ordered();
> + smp_rmb();
I don't think you need the barrier here if you use rdtsc_ordered, but
I would like confirmation from someone else.
> +
> + ver2 = ACCESS_ONCE(shared_info.vcpu_info[0].time.version);
> + } while ( ver2 & 1 || ver1 != ver2 );
> +
> +
> + system_time += scale_delta(tsc - tsc_timestamp,
> + tsc_to_system_mul,
> + tsc_shift);
> +
> + return system_time;
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/include/xtf/time.h b/include/xtf/time.h
> new file mode 100644
> index 0000000..8180e07
> --- /dev/null
> +++ b/include/xtf/time.h
> @@ -0,0 +1,24 @@
> +/**
> + * @file include/xtf/time.h
> + *
> + * Time management
> + */
> +#ifndef XTF_TIME_H
> +# define XTF_TIME_H
> +
> +#include <xtf/types.h>
> +
> +/* Time from boot in nanoseconds */
> +uint64_t since_boot_time(void);
since_boot_time looks like a weird name, I would probably name this
hypervisor_uptime or system_time, but I'm not specially thrilled
anyway.
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
