[PATCH v5 0/3] Time-related fixes for migration

* [PATCH v5 0/3] Time-related fixes for migration
@ 2014-04-16 22:59 Boris Ostrovsky
  2014-04-16 22:59 ` [PATCH v5 1/3] x86: Use native RDTSC(P) execution when guest and host frequencies are the same Boris Ostrovsky
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Boris Ostrovsky @ 2014-04-16 22:59 UTC (permalink / raw)
  To: xen-devel
  Cc: kevin.tian, suravee.suthikulpanit, eddie.dong, jbeulich,
	jun.nakajima, boris.ostrovsky

Version 5:
 * Fixed checks in tsc_set_info to make sure that PV works.

   The fix also covers PVH case although after having a quick look at PVH
   support wrt time/TSC it's pretty clear that more work needs to be done.
   For example, PVH doesn't appear to touch TSC offsets in VMCB, it uses
   hosts TSC value as is. It also doesn't initialise guest time (which is
   possibly why TSC emulation doesn't work). All these issues will have to
   be addressed separately.

 * Syntax cleanup

Version 4:
 While testing on AMD processors I realized that TSC scaling has a number of
 problems, some of which need to use the same interfaces as I added for
 migration bugs. I therefore decided to fold TSC scaling fixes into this series.

 Three patches:
 1 Revert the change introduced by 4aab59a3 where we stop intercepting RDTSC(P)
   if TSC scaling is supported. This was done in the wrong place resulting in guest
   running without intercepts but with vtsc on. This patch also allows us to
   continue running in tsc native mode after migration if frequency stays the same
   (which is what docs/misc/tscmode.txt implies also)
 2 It looks like TSC scaling enablement logic was inverted: we should be using it
   when running in tsc native mode, which is not what happens now. We also need to
   do some work to synchronize TSCs during initial boot when TSC scaling is on.
 3 The remainder of the original patch to cover TSC synchronization after
   migration.

 During v3 review Jan requested that I drop the at_tsc argument to 
 hvm_set_guest_tsc_fixed() since it should always be possible to safely examine
 chkpt_tsc (now called sync_tsc). This is no longer the case because we use this
 variable also during boot and so I left the interface as it was in V3 (and dropped
 arch_hvm_save_done() where sync_tsc is cleared since it doesn't add anything)

Version 3:
* Only the second patch is submitted (the first one has been applied)
* More thorough AMD support (work around rdtscll() in svm_set_tsc_offset())

Version 2:
* Avoid setting parameters from config file twice
* Use out-of-line definition of get_s_time()
* Update rdtscll macro definition to avoid namespace clashes
* Syntax cleanup

Two patches to address issues we discovered during migration testing.

* The first patch loads HVM parameters from configuration file during restore.
To fix the actual problem that we saw only timer_mode needed to be restored but
it seems to me that other parameters are needed as well since at least some of
them are used "at runtime".

The bug can be demonstrated with a Solaris guest but I haven't been able to
trigger it with Linux. Possibly because Solaris's gethrtime() routine is a
fast trap to kernel's hrtimer which does some tricks to account for missed
ticks during interrupts.

* The second patch keeps TSCs synchronized across VPCUs after save/restore.
Currently TSC values diverge after migration because during both save and restore
we calculate them separately for each VCPU and base each calculation on
newly-read host's TSC.

The problem can be easily demonstrated with this program for a 2-VCPU guest 
(I am assuming here invariant TSC so, for example, tsc_mode="always_emulate" (*)):

int
main(int argc, char* argv[])
{

  unsigned long long h = 0LL;
  int proc = 0;
  cpu_set_t set;

  for(;;) {
    unsigned long long n = __native_read_tsc();
    if(h && n < h)
        printf("prev 0x%llx cur 0x%llx\n", h, n);
    CPU_ZERO(&set);
    proc = (proc + 1) & 1;
    CPU_SET(proc, &set);
    if (sched_setaffinity(0, sizeof(cpu_set_t), &set)) {
        perror("sched_setaffinity");
        exit(1);
    }

    h = n;
  }
}

(*) Which brings up another observation: when we are in default tsc_mode we
start off with vtsc=0 and thus clear TSC_Invariant bit in guest's CPUID.
After migration vtsc is 1 and TSC_Invariant bit is set. So the guest may observe
different values of CPUID. Which technically reflects the fact that TSC became
"safe" but I think potentially may be problematic to some guests.

Boris Ostrovsky (3):
  x86: Use native RDTSC(P) execution when guest and host frequencies
    are the same
  x86/svm: Enable TSC scaling
  x86/HVM: Use fixed TSC value when saving or restoring domain

 xen/arch/x86/hvm/hvm.c           |   23 +++++++++++++-----
 xen/arch/x86/hvm/save.c          |    6 +++++
 xen/arch/x86/hvm/svm/svm.c       |   23 +++++++++++-------
 xen/arch/x86/hvm/vmx/vmx.c       |    7 +++--
 xen/arch/x86/hvm/vmx/vvmx.c      |    4 +-
 xen/arch/x86/hvm/vpt.c           |   16 ++++++++-----
 xen/arch/x86/time.c              |   46 ++++++++++++++++++++++++++++++++-----
 xen/include/asm-x86/hvm/domain.h |    6 +++++
 xen/include/asm-x86/hvm/hvm.h    |   11 +++++---
 xen/include/asm-x86/msr.h        |    6 ++--
 xen/include/xen/time.h           |    1 +
 11 files changed, 108 insertions(+), 41 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread