Patchwork [16/17] target-i386: set custom 'tsc-frequency' without intermediate x86_def_t

login
register
mail settings
Submitter Igor Mammedov
Date Jan. 11, 2013, 2:10 a.m.
Message ID <1357870231-26762-17-git-send-email-imammedo@redhat.com>
Download mbox | patch
Permalink /patch/211201/
State New
Headers show

Comments

Igor Mammedov - Jan. 11, 2013, 2:10 a.m.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
  make patch independent of visit_type_freq()
---
 target-i386/cpu.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
Eduardo Habkost - Jan. 14, 2013, 3:20 p.m.
On Fri, Jan 11, 2013 at 03:10:30AM +0100, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

Will this be converted to a simple qdict_put() of the raw string value,
to be parsed using visit_type_freq() (or something equivalent), in the
future? It would be interesting to keep the ability to use
"tsc-frequency=1GHz" on device_add/-device.


> ---
> v2:
>   make patch independent of visit_type_freq()
> ---
>  target-i386/cpu.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index ec27cf6..c3e1792 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1371,6 +1371,7 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features,
>              } else if (!strcmp(featurestr, "tsc_freq")) {
>                  int64_t tsc_freq;
>                  char *err;
> +                QString *s;
>  
>                  tsc_freq = strtosz_suffix_unit(val, &err,
>                                                 STRTOSZ_DEFSUFFIX_B, 1000);
> @@ -1378,7 +1379,9 @@ static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features,
>                      fprintf(stderr, "bad numerical value %s\n", val);
>                      goto error;
>                  }
> -                x86_cpu_def->tsc_khz = tsc_freq / 1000;
> +                s = qstring_new();
> +                qstring_append_int(s, tsc_freq);
> +                qdict_put(*props, "tsc-frequency", s);
>              } else if (!strcmp(featurestr, "hv_spinlocks")) {
>                  char *err;
>                  numvalue = strtoul(val, &err, 0);
> -- 
> 1.7.1
> 
>
Igor Mammedov - Jan. 14, 2013, 3:49 p.m.
On Mon, 14 Jan 2013 13:20:44 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> On Fri, Jan 11, 2013 at 03:10:30AM +0100, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Will this be converted to a simple qdict_put() of the raw string value,
> to be parsed using visit_type_freq() (or something equivalent), in the
> future? It would be interesting to keep the ability to use
> "tsc-frequency=1GHz" on device_add/-device.
Anthony was opposed to idea of special visitor. 
This patch allow us to move forward and in easily convert to visitor
later if there will be agreement to adding new visitor.

On the other hand we could leave it as it done here and enforce users to
provide numeric value for "tsc-frequency" in via  device_add interface
while maintaining compatibility for legacy 'tsc_freq'.

>
> 
> > ---
> > v2:
> >   make patch independent of visit_type_freq()
> > ---
> >  target-i386/cpu.c |    5 ++++-
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index ec27cf6..c3e1792 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -1371,6 +1371,7 @@ static int cpu_x86_parse_featurestr(x86_def_t
> > *x86_cpu_def, char *features, } else if (!strcmp(featurestr, "tsc_freq"))
> > { int64_t tsc_freq;
> >                  char *err;
> > +                QString *s;
> >  
> >                  tsc_freq = strtosz_suffix_unit(val, &err,
> >                                                 STRTOSZ_DEFSUFFIX_B,
> > 1000); @@ -1378,7 +1379,9 @@ static int
> > cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features,
> > fprintf(stderr, "bad numerical value %s\n", val); goto error;
> >                  }
> > -                x86_cpu_def->tsc_khz = tsc_freq / 1000;
> > +                s = qstring_new();
> > +                qstring_append_int(s, tsc_freq);
> > +                qdict_put(*props, "tsc-frequency", s);
> >              } else if (!strcmp(featurestr, "hv_spinlocks")) {
> >                  char *err;
> >                  numvalue = strtoul(val, &err, 0);
> > -- 
> > 1.7.1
> > 
> > 
>
Eduardo Habkost - Jan. 14, 2013, 4:10 p.m.
On Mon, Jan 14, 2013 at 04:49:23PM +0100, Igor Mammedov wrote:
> On Mon, 14 Jan 2013 13:20:44 -0200
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > On Fri, Jan 11, 2013 at 03:10:30AM +0100, Igor Mammedov wrote:
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > 
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > 
> > Will this be converted to a simple qdict_put() of the raw string value,
> > to be parsed using visit_type_freq() (or something equivalent), in the
> > future? It would be interesting to keep the ability to use
> > "tsc-frequency=1GHz" on device_add/-device.
> Anthony was opposed to idea of special visitor. 
> This patch allow us to move forward and in easily convert to visitor
> later if there will be agreement to adding new visitor.
> 
> On the other hand we could leave it as it done here and enforce users to
> provide numeric value for "tsc-frequency" in via  device_add interface
> while maintaining compatibility for legacy 'tsc_freq'.

I am starting to think that just using an integer without any special
parsing for "tsc-frequencey" on device_add/-device would be much
simpler.

(so ignore where I said "it would be interesting to keep the ability"
above).

> 
> >
> > 
> > > ---
> > > v2:
> > >   make patch independent of visit_type_freq()
> > > ---
> > >  target-i386/cpu.c |    5 ++++-
> > >  1 files changed, 4 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > > index ec27cf6..c3e1792 100644
> > > --- a/target-i386/cpu.c
> > > +++ b/target-i386/cpu.c
> > > @@ -1371,6 +1371,7 @@ static int cpu_x86_parse_featurestr(x86_def_t
> > > *x86_cpu_def, char *features, } else if (!strcmp(featurestr, "tsc_freq"))
> > > { int64_t tsc_freq;
> > >                  char *err;
> > > +                QString *s;
> > >  
> > >                  tsc_freq = strtosz_suffix_unit(val, &err,
> > >                                                 STRTOSZ_DEFSUFFIX_B,
> > > 1000); @@ -1378,7 +1379,9 @@ static int
> > > cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features,
> > > fprintf(stderr, "bad numerical value %s\n", val); goto error;
> > >                  }
> > > -                x86_cpu_def->tsc_khz = tsc_freq / 1000;
> > > +                s = qstring_new();
> > > +                qstring_append_int(s, tsc_freq);
> > > +                qdict_put(*props, "tsc-frequency", s);
> > >              } else if (!strcmp(featurestr, "hv_spinlocks")) {
> > >                  char *err;
> > >                  numvalue = strtoul(val, &err, 0);
> > > -- 
> > > 1.7.1
> > > 
> > > 
> > 
>

Patch

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index ec27cf6..c3e1792 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1371,6 +1371,7 @@  static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features,
             } else if (!strcmp(featurestr, "tsc_freq")) {
                 int64_t tsc_freq;
                 char *err;
+                QString *s;
 
                 tsc_freq = strtosz_suffix_unit(val, &err,
                                                STRTOSZ_DEFSUFFIX_B, 1000);
@@ -1378,7 +1379,9 @@  static int cpu_x86_parse_featurestr(x86_def_t *x86_cpu_def, char *features,
                     fprintf(stderr, "bad numerical value %s\n", val);
                     goto error;
                 }
-                x86_cpu_def->tsc_khz = tsc_freq / 1000;
+                s = qstring_new();
+                qstring_append_int(s, tsc_freq);
+                qdict_put(*props, "tsc-frequency", s);
             } else if (!strcmp(featurestr, "hv_spinlocks")) {
                 char *err;
                 numvalue = strtoul(val, &err, 0);