Re: [Qemu-devel] [PATCH v2 0/3] target-i386: save/restore vcpu's TSC rate during migration

[Eduardo Habkost]

On Fri, Oct 23, 2015 at 10:27:27AM +0800, Haozhong Zhang wrote:
> On Thu, Oct 22, 2015 at 04:45:21PM -0200, Eduardo Habkost wrote:
> > On Tue, Oct 20, 2015 at 03:22:51PM +0800, Haozhong Zhang wrote:
> > > This patchset enables QEMU to save/restore vcpu's TSC rate during the
> > > migration. When cooperating with KVM which supports TSC scaling, guest
> > > programs can observe a consistent guest TSC rate even though they are
> > > migrated among machines with different host TSC rates.
> > > 
> > > A pair of cpu options 'save-tsc-freq' and 'load-tsc-freq' are added to
> > > control the migration of vcpu's TSC rate.
> > 
> > The requirements and goals aren't clear to me. I see two possible use
> > cases, here:
> > 
> > 1) Best effort to keep TSC frequency constant if possible (but not
> >    aborting migration if not possible). This would be an interesting
> >    default, but a bit unpredictable.
> > 2) Strictly ensuring TSC frequency stays constant on migration (and
> >    aborting migration if not possible). This would be an useful feature,
> >    but can't be enabled by default unless both hosts have the same TSC
> >    frequency or support TSC scaling.
> > 
> > Which one(s) you are trying to implement?
> >
> 
> The former. I agree that it's unpredictable if setting vcpu's TSC
> frequency to the migrated value is enabled by default (but not in this
> patchset). The cpu option 'load-tsc-freq' is introduced to allow users
> to enable this behavior if they do know the underlying KVM and CPU
> support TSC scaling. In this way, I think the behavior is predictable
> as users do know what they are doing.

I'm confused. If load-tsc-freq doesn't abort when TSC scaling isn't
available (use case #1), why isn't it enabled by default? On the other
hand, if you expect the user to enable it only if the host supports TSC
scaling, why doesn't it abort if TSC scaling isn't available?

I mean, we can implement both use cases above this way:

1) If the user didn't ask for anything explicitly:
  * If the tsc-freq value is available in the migration stream, try to
    set it (but don't abort if it can't be set). (use case #1 above)
    * Rationale: it won't hurt to try to make the VM behave nicely if
      possible, without blocking migration if TSC scaling isn't
      available.
2) If the user asked for the TSC frequency to be enforced, set it and
  abort if it couldn't be set (use case #2 above). This could apply to
  both cases:
  2.1) If tsc-freq is explicitly set in the command-line.
    * Rationale: if the user asked for a specific frequency, we
      should do what was requested and not ignore errors silently.
  2.2) If tsc-freq is available in the migration stream, and the
    user asked explicitly for it to be enforced.
    * Rationale: the user is telling us that the incoming tsc-freq
      is important, so we shouldn't ignore it silently.
    * Open question: how should we name the new option?
      "load-tsc-freq" would be misleading because it won't be just about
      _loading_ tsc-freq (we would be loading it on use case #1, too),
      but about making sure it is enforced. "strict-tsc-freq"?
      "enforce-tsc-freq"?

We don't need to implement both #1 and #2 at the same time. But if you
just want to implement #1 first, I don't see the need for the
"load-tsc-freq" option.

On the migration source, we need another option or internal machine flag
for #1. I am not sure it should be an user-visible option. If
user-visible, I don't know how to name it. "save-tsc-freq" describes it
correctly, but it doesn't make its purpose very clear. Any suggestions?
It can also be implemented first as an internal machine class flag (set
in pc >= 2.5 only), and possibly become a user-visible option later.

> 
> > In other words, what is the right behavior when KVM_SET_TSC_KHZ fails or
> > KVM_CAP_TSC_CONTROL is not available? We can't answer that question if
> > the requirements and goals are not clear.
> >
> 
> If KVM_CAP_TSC_CONTROL is unavailable, QEMU and KVM will use the host
> TSC frequency as vcpu's TSC frequency.
> 
> If KVM_CAP_TSC_CONTROL is available and KVM_SET_TSC_KHZ fails, the
> setting of TSC frequency will fail and abort either the VM creation
> (this is the case for cpu option 'tsc-freq') or the migration.

I don't see why the lack of KVM_CAP_TSC_CONTROL and failure of
KVM_SET_TSC_KHZ should be treated differently. In both cases it means we
the TSC frequency can't be set.

I mean: if KVM_SET_TSC_KHZ is important enough for the user to make QEMU
abort, it should abort if KVM_CAP_TSC_CONTROL isn't even available. On
the other hand, if the user doesn't care about the lack of
KVM_CAP_TSC_CONTROL (meaning it isn't possible to call KVM_SET_TSC_KHZ
at all), I don't see why they would care if KVM_SET_TSC_KHZ failed.


> 
> > Once we know what exactly is the goal, we could enable the new mode with
> > a single option, instead of raw options to control migration stream
> > loading/saving.
> >
> 
> Saving vcpu's TSC frequency does not depend on TSC scaling but the
> loading does. And that is why I introduce two cpu options to control
> them separately.

I understand we probably need internal flags on the source and
destination side to control saving/loading/setting the tsc-freq. But I
am still trying to clarify what are the configuration semantics we need,
exactly, to properly evaluate both the new configuration interface and
its implementation.

-- 
Eduardo