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

2015-10-26 Thread haozhong . zhang
On Mon, Oct 26, 2015 at 04:41:22PM -0200, Eduardo Habkost wrote:
> On Mon, Oct 26, 2015 at 10:09:13AM +0800, haozhong.zh...@intel.com wrote:
> > On Fri, Oct 23, 2015 at 12:45:13PM -0200, Eduardo Habkost wrote:
> > > 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?
> > >
> > 
> > Sorry for the confusion. For user case #1, load-tsc-freq is really not
> > needed and the migrated TSC frequency should be set if possible
> > (ie. if TSC scaling is supported and KVM_SET_TSC_KHZ succeeds). If
> > setting TSC frequency fails, the migration will not be aborted.
> 
> Agreed.
> 
> > 
> > > 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.
> > >
> > 
> > Because the way I implements 'save-tsc-freq' in patch 1, it's exposed
> > to users. I'm not familiar the way to make a feature only available
> > for newer machine types. Could you provide some suggestions to hide
> > 

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

2015-10-26 Thread Eduardo Habkost
On Mon, Oct 26, 2015 at 10:09:13AM +0800, haozhong.zh...@intel.com wrote:
> On Fri, Oct 23, 2015 at 12:45:13PM -0200, Eduardo Habkost wrote:
> > 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?
> >
> 
> Sorry for the confusion. For user case #1, load-tsc-freq is really not
> needed and the migrated TSC frequency should be set if possible
> (ie. if TSC scaling is supported and KVM_SET_TSC_KHZ succeeds). If
> setting TSC frequency fails, the migration will not be aborted.

Agreed.

> 
> > 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.
> >
> 
> Because the way I implements 'save-tsc-freq' in patch 1, it's exposed
> to users. I'm not familiar the way to make a feature only available
> for newer machine types. Could you provide some suggestions to hide
> 'save-tsc-freq' from users?

You can make it an internal flag if you make it a PCMachineClass field,
set it to true on pc_machine_class_init(), and set it to false on
pc_*_2_4_machine_options().

I am unsure about the user-visible option. I am inclined towards making
it internal first because once we make it 

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

2015-10-25 Thread haozhong . zhang
On Fri, Oct 23, 2015 at 12:45:13PM -0200, Eduardo Habkost wrote:
> 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?
>

Sorry for the confusion. For user case #1, load-tsc-freq is really not
needed and the migrated TSC frequency should be set if possible
(ie. if TSC scaling is supported and KVM_SET_TSC_KHZ succeeds). If
setting TSC frequency fails, the migration will not be aborted.

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

Because the way I implements 'save-tsc-freq' in patch 1, it's exposed
to users. I'm not familiar the way to make a feature only available
for newer machine types. Could you provide some suggestions to hide
'save-tsc-freq' from users?

For the name, if we make the option internal only, could we still use
'save-tsc-freq' as it does mean saving the TSC frequency.

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

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

2015-10-23 Thread Eduardo Habkost
On Fri, Oct 23, 2015 at 08:35:20AM -0200, Marcelo Tosatti 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.
> 
> Only destination needs to support TSC scaling, to match the frequency
> of the incoming host.

True.

> 
> The KVM code for this feature has submitted or integrated? 
> 
> > Which one(s) you are trying to implement?
> > 
> > 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.
> > 
> > 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.
> 
> Windows and Linux guests have paravirt clocks and/or options to
> disable direct TSC usage for timekeeping purposes. So disabling
> migration seems overkill.

I assume that users who set TSC frequency explicitly in the VM config
care about it (otherwise they wouldn't be setting it explicitly) and
don't want it to change after migration.

-- 
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-10-23 Thread Haozhong Zhang
On Fri, Oct 23, 2015 at 08:35:20AM -0200, Marcelo Tosatti 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.
> 
> Only destination needs to support TSC scaling, to match the frequency
> of the incoming host.
>

Yes.

> The KVM code for this feature has submitted or integrated?

submitted and can be found at http://www.spinics.net/lists/kvm/msg122431.html

> 
> > Which one(s) you are trying to implement?
> > 
> > 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.
> > 
> > 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.
> 
> Windows and Linux guests have paravirt clocks and/or options to
> disable direct TSC usage for timekeeping purposes. So disabling
> migration seems overkill.
>

For KVM clock, guest users still need to know the host TSC (possibly
adjusted by scaling and offset) to know how long has passed since the
time provided by the PV clock. The KVM patch has adjusted KVM clock
for VMX TSC scaling so that it can be safely used across migration.

Haozhong

> > 
> > 
> > >  * By default, the migration of vcpu's TSC rate is enabled only on
> > >pc-*-2.5 and newer machine types. If the cpu option 'save-tsc-freq'
> > >is present, the vcpu's TSC rate will be migrated from older machine
> > >types as well.
> > >  * Another cpu option 'load-tsc-freq' controls whether the migrated
> > >vcpu's TSC rate is used. By default, QEMU will not use the migrated
> > >TSC rate if this option is not present. Otherwise, QEMU will use
> > >the migrated TSC rate and override the TSC rate given by the cpu
> > >option 'tsc-freq'.
> > > 
> > > Changes in v2:
> > >  * Add a pair of cpu options 'save-tsc-freq' and 'load-tsc-freq' to
> > >control the migration of vcpu's TSC rate.
> > >  * Move all logic of setting TSC rate to target-i386.
> > >  * Remove the duplicated TSC setup in kvm_arch_init_vcpu().
> > > 
> > > Haozhong Zhang (3):
> > >   target-i386: add a subsection for migrating vcpu's TSC rate
> > >   target-i386: calculate vcpu's TSC rate to be migrated
> > >   target-i386: load the migrated vcpu's TSC rate
> > > 
> > >  include/hw/i386/pc.h  |  5 +
> > >  target-i386/cpu.c |  2 ++
> > >  target-i386/cpu.h |  3 +++
> > >  target-i386/kvm.c | 61 
> > > +++
> > >  target-i386/machine.c | 19 
> > >  5 files changed, 81 insertions(+), 9 deletions(-)
> > > 
> > > -- 
> > > 2.4.8
> > > 
> > 
> > -- 
> > Eduardo
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-10-23 Thread Marcelo Tosatti
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.

Only destination needs to support TSC scaling, to match the frequency
of the incoming host.

The KVM code for this feature has submitted or integrated? 

> Which one(s) you are trying to implement?
> 
> 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.
> 
> 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.

Windows and Linux guests have paravirt clocks and/or options to
disable direct TSC usage for timekeeping purposes. So disabling
migration seems overkill.

> 
> 
> >  * By default, the migration of vcpu's TSC rate is enabled only on
> >pc-*-2.5 and newer machine types. If the cpu option 'save-tsc-freq'
> >is present, the vcpu's TSC rate will be migrated from older machine
> >types as well.
> >  * Another cpu option 'load-tsc-freq' controls whether the migrated
> >vcpu's TSC rate is used. By default, QEMU will not use the migrated
> >TSC rate if this option is not present. Otherwise, QEMU will use
> >the migrated TSC rate and override the TSC rate given by the cpu
> >option 'tsc-freq'.
> > 
> > Changes in v2:
> >  * Add a pair of cpu options 'save-tsc-freq' and 'load-tsc-freq' to
> >control the migration of vcpu's TSC rate.
> >  * Move all logic of setting TSC rate to target-i386.
> >  * Remove the duplicated TSC setup in kvm_arch_init_vcpu().
> > 
> > Haozhong Zhang (3):
> >   target-i386: add a subsection for migrating vcpu's TSC rate
> >   target-i386: calculate vcpu's TSC rate to be migrated
> >   target-i386: load the migrated vcpu's TSC rate
> > 
> >  include/hw/i386/pc.h  |  5 +
> >  target-i386/cpu.c |  2 ++
> >  target-i386/cpu.h |  3 +++
> >  target-i386/kvm.c | 61 
> > +++
> >  target-i386/machine.c | 19 
> >  5 files changed, 81 insertions(+), 9 deletions(-)
> > 
> > -- 
> > 2.4.8
> > 
> 
> -- 
> Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-10-23 Thread 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 

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

2015-10-22 Thread Eduardo Habkost
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?

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.

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.


>  * By default, the migration of vcpu's TSC rate is enabled only on
>pc-*-2.5 and newer machine types. If the cpu option 'save-tsc-freq'
>is present, the vcpu's TSC rate will be migrated from older machine
>types as well.
>  * Another cpu option 'load-tsc-freq' controls whether the migrated
>vcpu's TSC rate is used. By default, QEMU will not use the migrated
>TSC rate if this option is not present. Otherwise, QEMU will use
>the migrated TSC rate and override the TSC rate given by the cpu
>option 'tsc-freq'.
> 
> Changes in v2:
>  * Add a pair of cpu options 'save-tsc-freq' and 'load-tsc-freq' to
>control the migration of vcpu's TSC rate.
>  * Move all logic of setting TSC rate to target-i386.
>  * Remove the duplicated TSC setup in kvm_arch_init_vcpu().
> 
> Haozhong Zhang (3):
>   target-i386: add a subsection for migrating vcpu's TSC rate
>   target-i386: calculate vcpu's TSC rate to be migrated
>   target-i386: load the migrated vcpu's TSC rate
> 
>  include/hw/i386/pc.h  |  5 +
>  target-i386/cpu.c |  2 ++
>  target-i386/cpu.h |  3 +++
>  target-i386/kvm.c | 61 
> +++
>  target-i386/machine.c | 19 
>  5 files changed, 81 insertions(+), 9 deletions(-)
> 
> -- 
> 2.4.8
> 

-- 
Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-10-22 Thread Haozhong Zhang
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.

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

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

Haozhong

> 
> >  * By default, the migration of vcpu's TSC rate is enabled only on
> >pc-*-2.5 and newer machine types. If the cpu option 'save-tsc-freq'
> >is present, the vcpu's TSC rate will be migrated from older machine
> >types as well.
> >  * Another cpu option 'load-tsc-freq' controls whether the migrated
> >vcpu's TSC rate is used. By default, QEMU will not use the migrated
> >TSC rate if this option is not present. Otherwise, QEMU will use
> >the migrated TSC rate and override the TSC rate given by the cpu
> >option 'tsc-freq'.
> > 
> > Changes in v2:
> >  * Add a pair of cpu options 'save-tsc-freq' and 'load-tsc-freq' to
> >control the migration of vcpu's TSC rate.
> >  * Move all logic of setting TSC rate to target-i386.
> >  * Remove the duplicated TSC setup in kvm_arch_init_vcpu().
> > 
> > Haozhong Zhang (3):
> >   target-i386: add a subsection for migrating vcpu's TSC rate
> >   target-i386: calculate vcpu's TSC rate to be migrated
> >   target-i386: load the migrated vcpu's TSC rate
> > 
> >  include/hw/i386/pc.h  |  5 +
> >  target-i386/cpu.c |  2 ++
> >  target-i386/cpu.h |  3 +++
> >  target-i386/kvm.c | 61 
> > +++
> >  target-i386/machine.c | 19 
> >  5 files changed, 81 insertions(+), 9 deletions(-)
> > 
> > -- 
> > 2.4.8
> > 
> 
> -- 
> Eduardo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2015-10-20 Thread Haozhong Zhang
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.
 * By default, the migration of vcpu's TSC rate is enabled only on
   pc-*-2.5 and newer machine types. If the cpu option 'save-tsc-freq'
   is present, the vcpu's TSC rate will be migrated from older machine
   types as well.
 * Another cpu option 'load-tsc-freq' controls whether the migrated
   vcpu's TSC rate is used. By default, QEMU will not use the migrated
   TSC rate if this option is not present. Otherwise, QEMU will use
   the migrated TSC rate and override the TSC rate given by the cpu
   option 'tsc-freq'.

Changes in v2:
 * Add a pair of cpu options 'save-tsc-freq' and 'load-tsc-freq' to
   control the migration of vcpu's TSC rate.
 * Move all logic of setting TSC rate to target-i386.
 * Remove the duplicated TSC setup in kvm_arch_init_vcpu().

Haozhong Zhang (3):
  target-i386: add a subsection for migrating vcpu's TSC rate
  target-i386: calculate vcpu's TSC rate to be migrated
  target-i386: load the migrated vcpu's TSC rate

 include/hw/i386/pc.h  |  5 +
 target-i386/cpu.c |  2 ++
 target-i386/cpu.h |  3 +++
 target-i386/kvm.c | 61 +++
 target-i386/machine.c | 19 
 5 files changed, 81 insertions(+), 9 deletions(-)

-- 
2.4.8

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html