public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
From: Eduardo Habkost <ehabkost@redhat.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Cc: libvir-list@redhat.com, Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, kvm@vger.kernel.org,
	Haozhong Zhang <haozhong.zhang@intel.com>
Subject: [libvirt] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)
Date: Wed, 4 Jan 2017 17:59:17 -0200	[thread overview]
Message-ID: <20170104195917.GM3315@thinpad.lan.raisama.net> (raw)
In-Reply-To: <20170104133916.GG3315@thinpad.lan.raisama.net>

On Wed, Jan 04, 2017 at 11:39:16AM -0200, Eduardo Habkost wrote:
> On Wed, Jan 04, 2017 at 09:56:56AM -0200, Marcelo Tosatti wrote:
> > On Tue, Dec 27, 2016 at 05:21:20PM -0200, Eduardo Habkost wrote:
> > > Instead of blocking migration on the source when invtsc is
> > > enabled, rely on the migration destination to ensure there's no
> > > TSC frequency mismatch.
> > > 
> > > We can't allow migration unconditionally because we don't know if
> > > the destination is a QEMU version that is really going to ensure
> > > there's no TSC frequency mismatch. To ensure we are migrating to
> > > a destination that won't ignore SET_TSC_KHZ errors, allow invtsc
> > > migration only on pc-*-2.9 and newer.
> > > 
> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
[...]
> > > @@ -2655,12 +2656,14 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
> > >      }
> > >  
> > >      if (level == KVM_PUT_FULL_STATE) {
> > > -        /* We don't check for kvm_arch_set_tsc_khz() errors here,
> > > -         * because TSC frequency mismatch shouldn't abort migration,
> > > -         * unless the user explicitly asked for a more strict TSC
> > > -         * setting (e.g. using an explicit "tsc-freq" option).
> > > +        /* Migration TSC frequency mismatch is fatal only if we are
> > > +         * actually reporting Invariant TSC to the guest.
> > >           */
> > > -        kvm_arch_set_tsc_khz(cpu);
> > > +        ret = kvm_arch_set_tsc_khz(cpu);
> > > +        if ((x86_cpu->env.features[FEAT_8000_0007_EDX] & CPUID_APM_INVTSC) &&
> > > +            ret < 0) {
> > > +            return ret;
> > > +        }
> > >      }
> > 
> > Will the guest continue in the source in this case?
> > 
> > I think this is past the point where migration has been declared
> > successful. 
> > 
> > Otherwise looks good.
> 
> Good point. I will make additional tests and see if there's some
> other place where the kvm_arch_set_tsc_khz() call can be moved
> to.

So, if we solve this and do something on (for example) post_load,
we still have a problem: device state is migrated after RAM. This
means QEMU will check for TSC scaling and abort migration very
late.

We could solve that by manually registering a SaveVMHandler that
will send the TSC frequency on save_live_setup, so migration is
aborted earlier.

But: this sounds like just a complex hack to work around the real
problems:

1) TSC frequency is guest-visible, and anything that affects
   guest ABI should depend on the VM configuration, not on host
   capabilities;
2) Setting TSC frequency depending on the host will make
   migratability unpredictable for management software: the same
   VM config could be migratable to host A when started on host
   B, but not migratable to host A when started on host C.

I suggest we allow migration with invtsc if and only if
tsc-frequency is set explicitly by management software. In other
words, apply only patches 1/4 and 2/4 from this series. After
that, we will need libvirt support for configuring tsc-frequency.

-- 
Eduardo

  reply	other threads:[~2017-01-04 19:59 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-27 19:21 [PATCH 0/4] Allow migration with invtsc if there's no frequency mismatch Eduardo Habkost
2016-12-27 19:21 ` [PATCH 1/4] kvm: Simplify invtsc check Eduardo Habkost
2016-12-29 18:57   ` Marcelo Tosatti
2016-12-27 19:21 ` [PATCH 2/4] kvm: Allow invtsc migration if tsc-khz is set explicitly Eduardo Habkost
2017-01-04 11:44   ` Marcelo Tosatti
2017-01-04 11:57     ` Marcelo Tosatti
2017-01-04 13:40     ` Eduardo Habkost
2017-01-04 13:45       ` Marcelo Tosatti
2017-01-04 13:51         ` Eduardo Habkost
2016-12-27 19:21 ` [PATCH 3/4] pc: Add 2.9 machine-types Eduardo Habkost
2016-12-30 13:38   ` Igor Mammedov
2017-01-04 14:01   ` [Qemu-devel] " Laszlo Ersek
2017-01-04 14:20     ` Eduardo Habkost
2017-01-04 16:40       ` Laszlo Ersek
2017-01-04 17:46         ` [Qemu-devel] " Eduardo Habkost
2016-12-27 19:21 ` [PATCH 4/4] kvm: Allow migration with invtsc Eduardo Habkost
2017-01-04 11:56   ` Marcelo Tosatti
2017-01-04 13:39     ` Eduardo Habkost
2017-01-04 19:59       ` Eduardo Habkost [this message]
2017-01-04 22:26         ` TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc) Marcelo Tosatti
2017-01-05  1:36           ` Eduardo Habkost
2017-01-05 10:48             ` Marcelo Tosatti
2017-01-05 10:50               ` Marcelo Tosatti
2017-01-05 12:19               ` Eduardo Habkost
2017-01-05 12:33                 ` [libvirt] " Daniel P. Berrange
2017-01-05 12:48                   ` Eduardo Habkost
2017-01-05 13:00                     ` Daniel P. Berrange
2017-01-05 13:11                       ` Eduardo Habkost
2017-01-10 16:38                       ` Paolo Bonzini
2017-01-06 10:31                 ` Marcelo Tosatti
2017-01-08 15:49                   ` How to make dest host abort migration safely " Eduardo Habkost
2017-01-09 10:04                     ` [libvirt] " Dr. David Alan Gilbert
2017-01-08 20:28                   ` Exporting kvm_max_guest_tsc_khz to userspace " Eduardo Habkost
2017-01-09 14:58                     ` Paolo Bonzini
2017-01-11 13:26                       ` Eduardo Habkost
2017-01-11 14:06                         ` Paolo Bonzini
2017-01-10 16:36               ` TSC frequency configuration & invtsc migration " Paolo Bonzini
2017-01-11 11:58                 ` Eduardo Habkost
2017-01-18 11:55                 ` Marcelo Tosatti
2017-01-18 12:43                   ` Eduardo Habkost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170104195917.GM3315@thinpad.lan.raisama.net \
    --to=ehabkost@redhat.com \
    --cc=haozhong.zhang@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=libvir-list@redhat.com \
    --cc=mtosatti@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox