[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v4 1/7] introduce time managment in xtf
On 04/13/2018 02:05 PM, Roger Pau Monné 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 <phentex@xxxxxxxxx>
---
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.
I think you're right for this one. What do you think about it @Andrew ?
+
#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;
+}
+
Actually, I'm not sure that I have to make that much changes to this function,
as @Andrew wanted to use another version of it as far as I recall.
+
+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.
Sure. since_boot_time was really awful..
Thanks,
--
Paul Semel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
