Re: [Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-18 Thread Eduardo Habkost
On Wed, Jan 18, 2017 at 09:55:43AM -0200, Marcelo Tosatti wrote:
> On Tue, Jan 10, 2017 at 05:36:48PM +0100, Paolo Bonzini wrote:
> > 
> > 
> > On 05/01/2017 11:48, Marcelo Tosatti wrote:
> > >> Host A has TSC scaling, host B doesn't have TSC scaling. We want
> > >> to be able to start the VM on host A, and migrate to B. In this
> > >> case, the only possible solution is to use B's frequency when
> > >> starting the VM. The QEMU process doesn't have enough information
> > >> to make that decision.
> > > That is a good point. But again, its a special case and 
> > > should be supported by -cpu xxx,tsc-frequency=.
> > 
> > I don't think this is a scenario that can work reliably.  The computed
> > TSC frequency may vary by 0.5% or so on every boot (e.g. you may get
> > 2497000 kHz or 2511000 kHz for a 2.5 GHz TSC).  You can start the VM on
> > host A, reboot host B, and then you'll be unable to migrate.
> > 
> > Paolo
> 
> Including an acceptable error in the comparison seems to be
> acceptable to work around that case.

How much error is acceptable when exposing the invtsc flag to the
guest?

-- 
Eduardo



Re: [Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-18 Thread Marcelo Tosatti
On Tue, Jan 10, 2017 at 05:36:48PM +0100, Paolo Bonzini wrote:
> 
> 
> On 05/01/2017 11:48, Marcelo Tosatti wrote:
> >> Host A has TSC scaling, host B doesn't have TSC scaling. We want
> >> to be able to start the VM on host A, and migrate to B. In this
> >> case, the only possible solution is to use B's frequency when
> >> starting the VM. The QEMU process doesn't have enough information
> >> to make that decision.
> > That is a good point. But again, its a special case and 
> > should be supported by -cpu xxx,tsc-frequency=.
> 
> I don't think this is a scenario that can work reliably.  The computed
> TSC frequency may vary by 0.5% or so on every boot (e.g. you may get
> 2497000 kHz or 2511000 kHz for a 2.5 GHz TSC).  You can start the VM on
> host A, reboot host B, and then you'll be unable to migrate.
> 
> Paolo

Including an acceptable error in the comparison seems to be acceptable to work 
around that
case.





Re: [Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-11 Thread Eduardo Habkost
On Tue, Jan 10, 2017 at 05:36:48PM +0100, Paolo Bonzini wrote:
> 
> 
> On 05/01/2017 11:48, Marcelo Tosatti wrote:
> >> Host A has TSC scaling, host B doesn't have TSC scaling. We want
> >> to be able to start the VM on host A, and migrate to B. In this
> >> case, the only possible solution is to use B's frequency when
> >> starting the VM. The QEMU process doesn't have enough information
> >> to make that decision.
> > That is a good point. But again, its a special case and 
> > should be supported by -cpu xxx,tsc-frequency=.
> 
> I don't think this is a scenario that can work reliably.  The computed
> TSC frequency may vary by 0.5% or so on every boot (e.g. you may get
> 2497000 kHz or 2511000 kHz for a 2.5 GHz TSC).  You can start the VM on
> host A, reboot host B, and then you'll be unable to migrate.

Right, so it means invtsc migration without TSC scaling will be
possible in practice only if we tolerate a small variance on TSC
frequency on migration. The question is: should we? Can we
tolerate a 0.5% variance on TSC frequency and still expose invtsc
to the guest?

-- 
Eduardo



Re: [Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-10 Thread Paolo Bonzini


On 05/01/2017 11:48, Marcelo Tosatti wrote:
>> Host A has TSC scaling, host B doesn't have TSC scaling. We want
>> to be able to start the VM on host A, and migrate to B. In this
>> case, the only possible solution is to use B's frequency when
>> starting the VM. The QEMU process doesn't have enough information
>> to make that decision.
> That is a good point. But again, its a special case and 
> should be supported by -cpu xxx,tsc-frequency=.

I don't think this is a scenario that can work reliably.  The computed
TSC frequency may vary by 0.5% or so on every boot (e.g. you may get
2497000 kHz or 2511000 kHz for a 2.5 GHz TSC).  You can start the VM on
host A, reboot host B, and then you'll be unable to migrate.

Paolo



Re: [Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-06 Thread Marcelo Tosatti
On Thu, Jan 05, 2017 at 10:19:50AM -0200, Eduardo Habkost wrote:
> On Thu, Jan 05, 2017 at 08:48:32AM -0200, Marcelo Tosatti wrote:
> > On Wed, Jan 04, 2017 at 11:36:31PM -0200, Eduardo Habkost wrote:
> > > On Wed, Jan 04, 2017 at 08:26:27PM -0200, Marcelo Tosatti wrote:
> > > > On Wed, Jan 04, 2017 at 05:59:17PM -0200, Eduardo Habkost wrote:
> > > > > 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 
> > > > > [...]
> > > > > > > > @@ -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;
> > > > 
> > > > Well not really: the TSC frequency where the guest starts 
> > > > is the frequency the guest software expects.
> > > > So it does depend on host capabilities.
> > > 
> > > Could you explain where this expectation comes from? I can see
> > > multiple cases where choosing the TSC frequency where the VM
> > > starts is not the best option.
> > 
> > 1. Boot guest.
> > 2. Calibrate TSC.
> > 3. Use delay() with TSC calibration above, or
> > use TSC to measure the passage of time (TSC clock 
> > in userspace).
> 
> If TSC scaling is available, using a different frequency should
> be safe, shouldn't it? Otherwise, migrating with TSC scaling
> wouldn't be safe either.

Yes, but if you don't have TSC scaling, you have to boot with the
host frequency.

> Anyway: I don't disagree the starting host frequency is a good
> default. It is. But I don't think it's the best option on all
> cases.

Agree.

> > 
> > > I am considering two possible scenarios below. You probably have
> > > another scenario in mind, so it would be useful if you could
> > > describe it so we can consider possible solutions.
> > > 
> > > 
> > > Scenario 1:
> > > 
> > > You have two hosts: A and B, both with TSC scaling. They have
> > > different frequencies. The VM can be started on either one of
> > > them, and can be migrated to either one depending on the policy
> > > of management software.
> > > 
> > > Maybe B is a back

Re: [Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-05 Thread Eduardo Habkost
On Thu, Jan 05, 2017 at 08:48:32AM -0200, Marcelo Tosatti wrote:
> On Wed, Jan 04, 2017 at 11:36:31PM -0200, Eduardo Habkost wrote:
> > On Wed, Jan 04, 2017 at 08:26:27PM -0200, Marcelo Tosatti wrote:
> > > On Wed, Jan 04, 2017 at 05:59:17PM -0200, Eduardo Habkost wrote:
> > > > 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 
> > > > [...]
> > > > > > > @@ -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;
> > > 
> > > Well not really: the TSC frequency where the guest starts 
> > > is the frequency the guest software expects.
> > > So it does depend on host capabilities.
> > 
> > Could you explain where this expectation comes from? I can see
> > multiple cases where choosing the TSC frequency where the VM
> > starts is not the best option.
> 
> 1. Boot guest.
> 2. Calibrate TSC.
> 3. Use delay() with TSC calibration above, or
> use TSC to measure the passage of time (TSC clock 
> in userspace).

If TSC scaling is available, using a different frequency should
be safe, shouldn't it? Otherwise, migrating with TSC scaling
wouldn't be safe either.

Anyway: I don't disagree the starting host frequency is a good
default. It is. But I don't think it's the best option on all
cases.

> 
> > I am considering two possible scenarios below. You probably have
> > another scenario in mind, so it would be useful if you could
> > describe it so we can consider possible solutions.
> > 
> > 
> > Scenario 1:
> > 
> > You have two hosts: A and B, both with TSC scaling. They have
> > different frequencies. The VM can be started on either one of
> > them, and can be migrated to either one depending on the policy
> > of management software.
> > 
> > Maybe B is a backup host, the VM is expected to run most times on
> > host A, and we want to use the TSC frequency from host A every
> > time. Maybe it's the opposite and we want to use the frequency of
> > B. Maybe we want to use the lowest frequency of both, maybe the
> > highest one. We (QEMU developers) can recommend policy to libvirt
> > developers, but a given QEMU process doesn't have information to
> > decide what's be

Re: [Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-05 Thread Marcelo Tosatti
On Thu, Jan 05, 2017 at 08:48:32AM -0200, Marcelo Tosatti wrote:
> > Note that even if we follow your suggestion and implement an
> > alternative version of patch 4/4 to cover your use case, I will
> > strongly recommend libvirt developers to support configuring TSC
> > frequency if they want to support invtsc + migration without
> > surprising/unpredictable restrictions on migratability.
> 
> Well, alright. If you make the TSC frequency of the host
> available to mgmt software as you describe, and write the steps mgmt
> software should take, i'm good.
> 
> Thanks for the help Eduardo.
> 

I'll be happy to not have to add a weird script to 

1. Query the host TSC frequency.
2. Pass that to -cpu xxx,tsc-frequency=ZZZ

To my migration scripts. 

Probably easy to make that the default behaviour...




Re: [Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-05 Thread Marcelo Tosatti
On Wed, Jan 04, 2017 at 11:36:31PM -0200, Eduardo Habkost wrote:
> On Wed, Jan 04, 2017 at 08:26:27PM -0200, Marcelo Tosatti wrote:
> > On Wed, Jan 04, 2017 at 05:59:17PM -0200, Eduardo Habkost wrote:
> > > 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 
> > > [...]
> > > > > > @@ -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;
> > 
> > Well not really: the TSC frequency where the guest starts 
> > is the frequency the guest software expects.
> > So it does depend on host capabilities.
> 
> Could you explain where this expectation comes from? I can see
> multiple cases where choosing the TSC frequency where the VM
> starts is not the best option.

1. Boot guest.
2. Calibrate TSC.
3. Use delay() with TSC calibration above, or
use TSC to measure the passage of time (TSC clock 
in userspace).

> I am considering two possible scenarios below. You probably have
> another scenario in mind, so it would be useful if you could
> describe it so we can consider possible solutions.
> 
> 
> Scenario 1:
> 
> You have two hosts: A and B, both with TSC scaling. They have
> different frequencies. The VM can be started on either one of
> them, and can be migrated to either one depending on the policy
> of management software.
> 
> Maybe B is a backup host, the VM is expected to run most times on
> host A, and we want to use the TSC frequency from host A every
> time. Maybe it's the opposite and we want to use the frequency of
> B. Maybe we want to use the lowest frequency of both, maybe the
> highest one. We (QEMU developers) can recommend policy to libvirt
> developers, but a given QEMU process doesn't have information to
> decide what's best.

I can't see any practical scenario here, you will always want
to start with TSC frequency of the host where the VM was started.

If i am mistaken, please describe a practical case.

(If a practical scenario comes up, and there is a use-case
for setting the TSC frequency on startup, lets say 
a Windows VM which fails to boot if the TSC frequency
is too high, then it should be supported... But the
only known scenario to me, applying to 99.999% of cases, 
is to expose the TSC frequency where the guest booted at).

> Scenario 2:
> 
> Host A has TSC scaling, 

Re: [Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-04 Thread Eduardo Habkost
On Wed, Jan 04, 2017 at 08:26:27PM -0200, Marcelo Tosatti wrote:
> On Wed, Jan 04, 2017 at 05:59:17PM -0200, Eduardo Habkost wrote:
> > 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 
> > [...]
> > > > > @@ -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;
> 
> Well not really: the TSC frequency where the guest starts 
> is the frequency the guest software expects.
> So it does depend on host capabilities.

Could you explain where this expectation comes from? I can see
multiple cases where choosing the TSC frequency where the VM
starts is not the best option.

I am considering two possible scenarios below. You probably have
another scenario in mind, so it would be useful if you could
describe it so we can consider possible solutions.


Scenario 1:

You have two hosts: A and B, both with TSC scaling. They have
different frequencies. The VM can be started on either one of
them, and can be migrated to either one depending on the policy
of management software.

Maybe B is a backup host, the VM is expected to run most times on
host A, and we want to use the TSC frequency from host A every
time. Maybe it's the opposite and we want to use the frequency of
B. Maybe we want to use the lowest frequency of both, maybe the
highest one. We (QEMU developers) can recommend policy to libvirt
developers, but a given QEMU process doesn't have information to
decide what's best.


Scenario 2:

Host A has TSC scaling, host B doesn't have TSC scaling. We want
to be able to start the VM on host A, and migrate to B. In this
case, the only possible solution is to use B's frequency when
starting the VM. The QEMU process doesn't have enough information
to make that decision.


> 
> > 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.
> 
> Well, just check the frequency.

What do you mean by "just check the frequency"? What exactly
should management software do?

Whatever management software do, if you just use the source host
frequency you can get into a situation where you can start a VM
on host A and migrate it to B, but can't start the VM on host B
and migrate it to A. This would be confusing for users, and
probably break assumpt

Re: [Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-04 Thread Marcelo Tosatti
On Wed, Jan 04, 2017 at 05:59:17PM -0200, Eduardo Habkost wrote:
> 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 
> [...]
> > > > @@ -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;

Well not really: the TSC frequency where the guest starts 
is the frequency the guest software expects.
So it does depend 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.

Well, just check the frequency.

> 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.

I don't like that for the following reasons:

* It moves low level complexity from QEMU/KVM to libvirt 
   (libvirt has to call KVM_GET_TSC_KHZ from a vcpu thread).
* It makes it difficult to run QEMU manually (i use QEMU
   manually all the time).
* It requires patching libvirt.

In my experience things work better when the functionality is
not split across libvirt/qemu.

Can't this be fixed in QEMU? Just check that destination host supports
TSC scaling before migration (or that KVM_GET_TSC_KHZ return value
matches on source and destination).




[Qemu-devel] TSC frequency configuration & invtsc migration (was Re: [PATCH 4/4] kvm: Allow migration with invtsc)

2017-01-04 Thread Eduardo Habkost
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 
[...]
> > > @@ -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