Re: [Qemu-devel] [PATCH v3 1/3] target-i386: add a subsection for migrating vcpu's TSC rate

[Haozhong Zhang]

On 11/13/15 13:21, Eduardo Habkost wrote:
> On Fri, Nov 13, 2015 at 10:23:54AM +0800, Haozhong Zhang wrote:
> > On 11/11/15 22:27, Haozhong Zhang wrote:
> > > On 11/11/15 12:16, Eduardo Habkost wrote:
> > [...]
> > > > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > > > > index 2f8f396..858ed69 100644
> > > > > --- a/hw/i386/pc_q35.c
> > > > > +++ b/hw/i386/pc_q35.c
> > > > > @@ -385,6 +385,7 @@ static void 
> > > > > pc_q35_2_4_machine_options(MachineClass *m)
> > > > >  pc_q35_2_5_machine_options(m);
> > > > >  m->alias = NULL;
> > > > >  pcmc->broken_reserved_end = true;
> > > > > +pcmc->save_tsc_khz = false;
> > > > 
> > > > I had suggested the PCMachineClass field, but now I've been thinking:
> > > > all other fields related to tsc_khz are in X86CPU, so I believe this
> > > > belongs to X86CPU too. It could be a simple X86CPU property set by
> > > > PC_COMPAT_2_4.
> > > >
> > > 
> > > Reasonable, will update in the next version.
> > 
> > Or maybe no ...
> > 
> > I think there is still a problem to set a X86CPU property in
> > PC_COMPAT_2_4:
> > 
> > if I create a property for save_tsc_khz by adding
> >   DEFINE_PROP_BOOL("save-tsc-freq", X86CPU, save_tsc_khz, true)
> > in x86_cpu_properties and add
> >   {
> >   .driver   = TYPE_X86_CPU,
> >   .property = "save-tsc-freq",
> >   .value= "off",
> >   }
> > in PC_COMPAT_2_4, then "save-tsc-freq" will also become a
> > user-visible cpu option. But we agreed on keeping it as an
> > internal flag in the previous discussion.
> > 
> > Any other ways to set a property in PC_COMPAT_* while keeping that
> > property internal?
> 
> I don't think making it internal is a requirement. It just make
> things simpler because it allowed us to postpone decisions about
> the user-visible parts.
> 
> ...which seems to be a good reason to keep it on PCMachineClass
> by now, if you prefer it that way. The subsection code is already
> on machine.c and not on cpu.c, anyway.
>

Thanks, I'll keep it in PCMachineClass in the next version.

Haozhong
--
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 v3 1/3] target-i386: add a subsection for migrating vcpu's TSC rate

[Eduardo Habkost]

On Fri, Nov 13, 2015 at 10:23:54AM +0800, Haozhong Zhang wrote:
> On 11/11/15 22:27, Haozhong Zhang wrote:
> > On 11/11/15 12:16, Eduardo Habkost wrote:
> [...]
> > > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > > > index 2f8f396..858ed69 100644
> > > > --- a/hw/i386/pc_q35.c
> > > > +++ b/hw/i386/pc_q35.c
> > > > @@ -385,6 +385,7 @@ static void pc_q35_2_4_machine_options(MachineClass 
> > > > *m)
> > > >  pc_q35_2_5_machine_options(m);
> > > >  m->alias = NULL;
> > > >  pcmc->broken_reserved_end = true;
> > > > +pcmc->save_tsc_khz = false;
> > > 
> > > I had suggested the PCMachineClass field, but now I've been thinking:
> > > all other fields related to tsc_khz are in X86CPU, so I believe this
> > > belongs to X86CPU too. It could be a simple X86CPU property set by
> > > PC_COMPAT_2_4.
> > >
> > 
> > Reasonable, will update in the next version.
> 
> Or maybe no ...
> 
> I think there is still a problem to set a X86CPU property in
> PC_COMPAT_2_4:
> 
> if I create a property for save_tsc_khz by adding
>   DEFINE_PROP_BOOL("save-tsc-freq", X86CPU, save_tsc_khz, true)
> in x86_cpu_properties and add
>   {
>   .driver   = TYPE_X86_CPU,
>   .property = "save-tsc-freq",
>   .value= "off",
>   }
> in PC_COMPAT_2_4, then "save-tsc-freq" will also become a
> user-visible cpu option. But we agreed on keeping it as an
> internal flag in the previous discussion.
> 
> Any other ways to set a property in PC_COMPAT_* while keeping that
> property internal?

I don't think making it internal is a requirement. It just make
things simpler because it allowed us to postpone decisions about
the user-visible parts.

...which seems to be a good reason to keep it on PCMachineClass
by now, if you prefer it that way. The subsection code is already
on machine.c and not on cpu.c, anyway.

-- 
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 v3 1/3] target-i386: add a subsection for migrating vcpu's TSC rate

[Haozhong Zhang]

On 11/11/15 22:27, Haozhong Zhang wrote:
> On 11/11/15 12:16, Eduardo Habkost wrote:
[...]
> > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > > index 2f8f396..858ed69 100644
> > > --- a/hw/i386/pc_q35.c
> > > +++ b/hw/i386/pc_q35.c
> > > @@ -385,6 +385,7 @@ static void pc_q35_2_4_machine_options(MachineClass 
> > > *m)
> > >  pc_q35_2_5_machine_options(m);
> > >  m->alias = NULL;
> > >  pcmc->broken_reserved_end = true;
> > > +pcmc->save_tsc_khz = false;
> > 
> > I had suggested the PCMachineClass field, but now I've been thinking:
> > all other fields related to tsc_khz are in X86CPU, so I believe this
> > belongs to X86CPU too. It could be a simple X86CPU property set by
> > PC_COMPAT_2_4.
> >
> 
> Reasonable, will update in the next version.

Or maybe no ...

I think there is still a problem to set a X86CPU property in
PC_COMPAT_2_4:

if I create a property for save_tsc_khz by adding
  DEFINE_PROP_BOOL("save-tsc-freq", X86CPU, save_tsc_khz, true)
in x86_cpu_properties and add
  {
  .driver   = TYPE_X86_CPU,
  .property = "save-tsc-freq",
  .value= "off",
  }
in PC_COMPAT_2_4, then "save-tsc-freq" will also become a
user-visible cpu option. But we agreed on keeping it as an
internal flag in the previous discussion.

Any other ways to set a property in PC_COMPAT_* while keeping that
property internal?

Haozhong
--
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 v3 1/3] target-i386: add a subsection for migrating vcpu's TSC rate

[Eduardo Habkost]

On Mon, Nov 02, 2015 at 05:26:41PM +0800, Haozhong Zhang wrote:
> A new subsection 'vmstate_tsc_khz' is added to migrate vcpu's TSC
> rate. For the backwards compatibility, this subsection is not migrated
> on pc-*-2.4 and older machine types.
> 
> Signed-off-by: Haozhong Zhang 
> ---
>  hw/i386/pc.c  |  1 +
>  hw/i386/pc_piix.c |  1 +
>  hw/i386/pc_q35.c  |  1 +
>  include/hw/i386/pc.h  |  1 +
>  target-i386/cpu.h |  1 +
>  target-i386/machine.c | 21 +
>  6 files changed, 26 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0cb8afd..2f2fc93 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1952,6 +1952,7 @@ static void pc_machine_class_init(ObjectClass *oc, void 
> *data)
>  HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>  
>  pcmc->get_hotplug_handler = mc->get_hotplug_handler;
> +pcmc->save_tsc_khz = true;
>  mc->get_hotplug_handler = pc_get_hotpug_handler;
>  mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
>  mc->default_boot_order = "cad";
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 393dcc4..fc71321 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -487,6 +487,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m)
>  m->alias = NULL;
>  m->is_default = 0;
>  pcmc->broken_reserved_end = true;
> +pcmc->save_tsc_khz = false;
>  SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
>  }
>  
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 2f8f396..858ed69 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -385,6 +385,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
>  pc_q35_2_5_machine_options(m);
>  m->alias = NULL;
>  pcmc->broken_reserved_end = true;
> +pcmc->save_tsc_khz = false;

I had suggested the PCMachineClass field, but now I've been thinking:
all other fields related to tsc_khz are in X86CPU, so I believe this
belongs to X86CPU too. It could be a simple X86CPU property set by
PC_COMPAT_2_4.

-- 
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 v3 1/3] target-i386: add a subsection for migrating vcpu's TSC rate

[Haozhong Zhang]

On 11/11/15 12:16, Eduardo Habkost wrote:
> On Mon, Nov 02, 2015 at 05:26:41PM +0800, Haozhong Zhang wrote:
> > A new subsection 'vmstate_tsc_khz' is added to migrate vcpu's TSC
> > rate. For the backwards compatibility, this subsection is not migrated
> > on pc-*-2.4 and older machine types.
> > 
> > Signed-off-by: Haozhong Zhang 
> > ---
> >  hw/i386/pc.c  |  1 +
> >  hw/i386/pc_piix.c |  1 +
> >  hw/i386/pc_q35.c  |  1 +
> >  include/hw/i386/pc.h  |  1 +
> >  target-i386/cpu.h |  1 +
> >  target-i386/machine.c | 21 +
> >  6 files changed, 26 insertions(+)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 0cb8afd..2f2fc93 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1952,6 +1952,7 @@ static void pc_machine_class_init(ObjectClass *oc, 
> > void *data)
> >  HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> >  
> >  pcmc->get_hotplug_handler = mc->get_hotplug_handler;
> > +pcmc->save_tsc_khz = true;
> >  mc->get_hotplug_handler = pc_get_hotpug_handler;
> >  mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
> >  mc->default_boot_order = "cad";
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 393dcc4..fc71321 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -487,6 +487,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass 
> > *m)
> >  m->alias = NULL;
> >  m->is_default = 0;
> >  pcmc->broken_reserved_end = true;
> > +pcmc->save_tsc_khz = false;
> >  SET_MACHINE_COMPAT(m, PC_COMPAT_2_4);
> >  }
> >  
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 2f8f396..858ed69 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -385,6 +385,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m)
> >  pc_q35_2_5_machine_options(m);
> >  m->alias = NULL;
> >  pcmc->broken_reserved_end = true;
> > +pcmc->save_tsc_khz = false;
> 
> I had suggested the PCMachineClass field, but now I've been thinking:
> all other fields related to tsc_khz are in X86CPU, so I believe this
> belongs to X86CPU too. It could be a simple X86CPU property set by
> PC_COMPAT_2_4.
>

Reasonable, will update in the next version.

Thanks,
Haozhong

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