On 04/09/2018 04:35 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 <phen...@amazon.de>
---
This seems to be missing a list of changes between v2 and v3. Please
add such a list when posting new versions.
+uint64_t since_boot_time(void)
+{
+ uint64_t tsc;
+ uint32_t ver1, ver2;
+ uint64_t system_time;
+ uint64_t old_tsc;
+
+ do
+ {
+ do
+ {
+ ver1 = ACCESS_ONCE(shared_info.vcpu_info[0].time.version);
+ smp_rmb();
+ } while ( (ver1 & 1) == 1 );
+
+ system_time = ACCESS_ONCE(shared_info.vcpu_info[0].time.system_time);
+ old_tsc = ACCESS_ONCE(shared_info.vcpu_info[0].time.tsc_timestamp);
+ smp_rmb();
+ ver2 = ACCESS_ONCE(shared_info.vcpu_info[0].time.version);
+ smp_rmb();
+ } while ( ver1 != ver2 );
This is still overly complicated IMO, and you have not replied to my
question of whether doing the scale_delta below is OK.
About this scale_delta, we discussed with Andrew, and we are going to use
another version of the function as far as I remember. That's why I am not taking
care of it for the moment.
AFAICT uou _cannot_ access any of the vcpu_time_info fields without
checking for the version (in order to avoid reading inconsistent data
during an update), yet below you read tsc_to_system_mul and
tsc_shift.
I'm sorry, I am probably not getting your point here, because I am already
checking for the version. I was actually checking for the wc_version too in the
first version of those patches, but after chatting with Andrew, It appeared that
it was not necessary..
I've already pointed out the code at:
https://github.com/freebsd/freebsd/blob/master/sys/x86/x86/pvclock.c#L141
As a simpler reference implementation.
+
+ rdtsc(tsc);
+
+ system_time += scale_delta(tsc - old_tsc,
+
ACCESS_ONCE(shared_info.vcpu_info[0].time.tsc_to_system_mul),
+ ACCESS_ONCE(shared_info.vcpu_info[0].time.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..b88da63
--- /dev/null
+++ b/include/xtf/time.h
@@ -0,0 +1,31 @@
+/**
+ * @file include/xtf/time.h
+ *
+ * Time management
+ */
+#ifndef XTF_TIME_H
+# define XTF_TIME_H
+
+#include <xtf/types.h>
+
+#define rdtsc(tsc) {\
+ uint32_t lo, hi;\
+ __asm__ volatile("rdtsc": "=a"(lo), "=d"(hi));\
Please make sure you only send a new version after having fixed all
the comments, this is still missing the serialization requirements
mentioned in the review, and it's also the wrong file to place this
helper:
I am sorry, I was really convinced that this version didn't need revision
anymore (and I still don't see what I should change).
https://lists.xenproject.org/archives/html/xen-devel/2018-04/msg00546.html
Roger.
Thanks,
--
Paul
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel