Re: [Qemu-devel] [PATCH v3 2/3] target-i386: calculate vcpu's TSC rate to be migrated

[Eduardo Habkost]

On Fri, Nov 06, 2015 at 10:32:24AM +0800, address@hidden wrote:
> On 11/05/15 14:05, Eduardo Habkost wrote:
> > On Thu, Nov 05, 2015 at 09:30:51AM +0800, Haozhong Zhang wrote:
> > > On 11/04/15 19:42, Eduardo Habkost wrote:
[...]
> > > > > +        env->tsc_khz_saved = r;
> > > > > +    }
> > > > 
> > > > Why do you need a separate tsc_khz_saved field, and don't simply use
> > > > tsc_khz? It would have the additional feature of letting QMP clients
> > > > query the current TSC rate by asking for the tsc-freq property on CPU
> > > > objects.
> > > >
> > > 
> > > It's to avoid overriding env->tsc_khz on the destination in the
> > > migration. I can change this line to
> > >              env->tsc_khz = env->tsc_khz_saved = r;
> > 
> > You are already avoiding overriding env->tsc_khz, because you use
> > KVM_GET_TSC_KHZ only if tsc_khz is not set yet. I still don't see why
> > you need a tsc_khz_saved field that requires duplicating the SET_TSC_KHZ
> > code, if you could just do this:
> > 
> >     if (!env->tsc_khz) {
> >         env->tsc_khz = kvm_vcpu_ioctl(cs, KVM_GET_TSC_KHZ);
> >     }
> >
> 
> Consider an example that we migrate a VM from machine A to machine B
> and then to machine C, and QEMU on machine B is launched with the cpu
> option 'tsc-freq' (i.e. env->tsc_khz on B is non-zero at the
> beginning):
>  1) In the migration from B to C, the user-specified TSC frequency by
>     'tsc-freq' on B is expected to be migrated to C. That is, the
>     value of env->tsc_khz on B is migrated.
>  2) If TSC frequency is migrated through env->tsc_khz, then
>     env->tsc_khz on B will be overrode in the migration from A to B
>     before kvm_arch_setup_tsc_khz(). If the guest TSC frequency is
>     different than the user-specified TSC frequency on B, the
>     expectation in 1) will not be satisfied anymore.

Setting tsc-freq on B when tsc-freq was not used on A is invalid usage.
This is not different from changing the CPU model and adding or removing
CPU flags when migrating, which is also incorrect. The command-line
parameters defining the VM must be the same when you migrate.

-- 
Eduardo