Re: [Qemu-devel] [PATCH 2/3] qemu-x86: Add tsc_freq option to -cpu
On 07/19/2011 05:14 PM, Joerg Roedel wrote: On Tue, Jul 19, 2011 at 04:55:53PM +0300, Avi Kivity wrote: > On 07/19/2011 04:54 PM, Avi Kivity wrote: >> On 07/19/2011 04:30 PM, Joerg Roedel wrote: >>> Hmm, I planned to do the VMSTATE thing in a follow-on patch-set. The >>> plan is to read the VCPU tsc_freq at guest start time on !tsc-scale >>> hosts and migrate it over so that the destination host can set the >>> tsc-freq if it supports tsc-scaling. >> >> This can be done by a management tool if desired. >> > > Although, if we do this unconditionally (that is, also for tsc-scale > hosts) then we get stable tsc even without supplying a tsc frequency > argument... need to think about this. It has the advantage that it "just works", without the need to extend management tools and the like. And it makes migration more transparent to the guests. Yes, exactly. The flip side is that automagic stuff is sometimes unexpected and leads to breakage. I'm not sure what the right thing is. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 2/3] qemu-x86: Add tsc_freq option to -cpu
On Tue, Jul 19, 2011 at 04:55:53PM +0300, Avi Kivity wrote: > On 07/19/2011 04:54 PM, Avi Kivity wrote: >> On 07/19/2011 04:30 PM, Joerg Roedel wrote: >>> Hmm, I planned to do the VMSTATE thing in a follow-on patch-set. The >>> plan is to read the VCPU tsc_freq at guest start time on !tsc-scale >>> hosts and migrate it over so that the destination host can set the >>> tsc-freq if it supports tsc-scaling. >> >> This can be done by a management tool if desired. >> > > Although, if we do this unconditionally (that is, also for tsc-scale > hosts) then we get stable tsc even without supplying a tsc frequency > argument... need to think about this. It has the advantage that it "just works", without the need to extend management tools and the like. And it makes migration more transparent to the guests. Joerg
Re: [Qemu-devel] [PATCH 2/3] qemu-x86: Add tsc_freq option to -cpu
On 07/19/2011 04:54 PM, Avi Kivity wrote: On 07/19/2011 04:30 PM, Joerg Roedel wrote: > > (although we've been talking about starting a naked qemu and pushing all > of the configuration from the source). Hmm, I planned to do the VMSTATE thing in a follow-on patch-set. The plan is to read the VCPU tsc_freq at guest start time on !tsc-scale hosts and migrate it over so that the destination host can set the tsc-freq if it supports tsc-scaling. This can be done by a management tool if desired. Although, if we do this unconditionally (that is, also for tsc-scale hosts) then we get stable tsc even without supplying a tsc frequency argument... need to think about this. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 2/3] qemu-x86: Add tsc_freq option to -cpu
On 07/19/2011 04:30 PM, Joerg Roedel wrote: > > (although we've been talking about starting a naked qemu and pushing all > of the configuration from the source). Hmm, I planned to do the VMSTATE thing in a follow-on patch-set. The plan is to read the VCPU tsc_freq at guest start time on !tsc-scale hosts and migrate it over so that the destination host can set the tsc-freq if it supports tsc-scaling. This can be done by a management tool if desired. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 2/3] qemu-x86: Add tsc_freq option to -cpu
On Tue, Jul 19, 2011 at 03:20:37PM +0300, Avi Kivity wrote:
> On 07/19/2011 02:46 PM, Marcelo Tosatti wrote:
>> On Thu, Jul 07, 2011 at 04:13:12PM +0200, Joerg Roedel wrote:
>> > To let the user configure the desired tsc frequency for the
>> > guest if running in KVM.
>> >
>> > Signed-off-by: Joerg Roedel
>> > ---
>> > target-i386/cpu.h |1 +
>> > target-i386/cpuid.c | 13 +
>> > 2 files changed, 14 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/target-i386/cpu.h b/target-i386/cpu.h
>> > index cdf68ff..399e124 100644
>> > --- a/target-i386/cpu.h
>> > +++ b/target-i386/cpu.h
>> > @@ -743,6 +743,7 @@ typedef struct CPUX86State {
>> > uint32_t cpuid_kvm_features;
>> > uint32_t cpuid_svm_features;
>> > bool tsc_valid;
>> > +int tsc_khz;
>>
>> This should be saved/restore in migration data (missing VMSTATE entry).
>
> Why? It's static data. Traditionally we only migrate runtime data.
>
> (although we've been talking about starting a naked qemu and pushing all
> of the configuration from the source).
Hmm, I planned to do the VMSTATE thing in a follow-on patch-set. The
plan is to read the VCPU tsc_freq at guest start time on !tsc-scale
hosts and migrate it over so that the destination host can set the
tsc-freq if it supports tsc-scaling.
Joerg
Re: [Qemu-devel] [PATCH 2/3] qemu-x86: Add tsc_freq option to -cpu
On Tue, Jul 19, 2011 at 03:20:37PM +0300, Avi Kivity wrote:
> On 07/19/2011 02:46 PM, Marcelo Tosatti wrote:
> >On Thu, Jul 07, 2011 at 04:13:12PM +0200, Joerg Roedel wrote:
> >> To let the user configure the desired tsc frequency for the
> >> guest if running in KVM.
> >>
> >> Signed-off-by: Joerg Roedel
> >> ---
> >> target-i386/cpu.h |1 +
> >> target-i386/cpuid.c | 13 +
> >> 2 files changed, 14 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> >> index cdf68ff..399e124 100644
> >> --- a/target-i386/cpu.h
> >> +++ b/target-i386/cpu.h
> >> @@ -743,6 +743,7 @@ typedef struct CPUX86State {
> >> uint32_t cpuid_kvm_features;
> >> uint32_t cpuid_svm_features;
> >> bool tsc_valid;
> >> +int tsc_khz;
> >
> >This should be saved/restore in migration data (missing VMSTATE entry).
>
> Why? It's static data. Traditionally we only migrate runtime data.
>
> (although we've been talking about starting a naked qemu and pushing
> all of the configuration from the source).
Right.
Re: [Qemu-devel] [PATCH 2/3] qemu-x86: Add tsc_freq option to -cpu
On 07/19/2011 02:46 PM, Marcelo Tosatti wrote:
On Thu, Jul 07, 2011 at 04:13:12PM +0200, Joerg Roedel wrote:
> To let the user configure the desired tsc frequency for the
> guest if running in KVM.
>
> Signed-off-by: Joerg Roedel
> ---
> target-i386/cpu.h |1 +
> target-i386/cpuid.c | 13 +
> 2 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index cdf68ff..399e124 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -743,6 +743,7 @@ typedef struct CPUX86State {
> uint32_t cpuid_kvm_features;
> uint32_t cpuid_svm_features;
> bool tsc_valid;
> +int tsc_khz;
This should be saved/restore in migration data (missing VMSTATE entry).
Why? It's static data. Traditionally we only migrate runtime data.
(although we've been talking about starting a naked qemu and pushing all
of the configuration from the source).
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 2/3] qemu-x86: Add tsc_freq option to -cpu
On Thu, Jul 07, 2011 at 04:13:12PM +0200, Joerg Roedel wrote:
> To let the user configure the desired tsc frequency for the
> guest if running in KVM.
>
> Signed-off-by: Joerg Roedel
> ---
> target-i386/cpu.h |1 +
> target-i386/cpuid.c | 13 +
> 2 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index cdf68ff..399e124 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -743,6 +743,7 @@ typedef struct CPUX86State {
> uint32_t cpuid_kvm_features;
> uint32_t cpuid_svm_features;
> bool tsc_valid;
> +int tsc_khz;
This should be saved/restore in migration data (missing VMSTATE entry).
> /* in order to simplify APIC support, we leave this pointer to the
> user */
> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> index e1ae3af..89e9623 100644
> --- a/target-i386/cpuid.c
> +++ b/target-i386/cpuid.c
> @@ -224,6 +224,7 @@ typedef struct x86_def_t {
> int family;
> int model;
> int stepping;
> +int tsc_khz;
> uint32_t features, ext_features, ext2_features, ext3_features;
> uint32_t kvm_features, svm_features;
> uint32_t xlevel;
> @@ -704,6 +705,17 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def,
> const char *cpu_model)
> } else if (!strcmp(featurestr, "model_id")) {
> pstrcpy(x86_cpu_def->model_id, sizeof(x86_cpu_def->model_id),
> val);
> +} else if (!strcmp(featurestr, "tsc_freq")) {
> +int64_t tsc_freq;
> +char *err;
> +
> +tsc_freq = strtosz_suffix_unit(val, &err,
> + STRTOSZ_DEFSUFFIX_B, 1000);
> +if (!*val || *err) {
> +fprintf(stderr, "bad numerical value %s\n", val);
> +goto error;
> +}
> +x86_cpu_def->tsc_khz = tsc_freq / 1000;
> } else {
> fprintf(stderr, "unrecognized feature %s\n", featurestr);
> goto error;
> @@ -872,6 +884,7 @@ int cpu_x86_register (CPUX86State *env, const char
> *cpu_model)
> env->cpuid_svm_features = def->svm_features;
> env->cpuid_ext4_features = def->ext4_features;
> env->cpuid_xlevel2 = def->xlevel2;
> +env->tsc_khz = def->tsc_khz;
> if (!kvm_enabled()) {
> env->cpuid_features &= TCG_FEATURES;
> env->cpuid_ext_features &= TCG_EXT_FEATURES;
> --
> 1.7.4.1
>
>
> --
> 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
