Patchwork [qom-cpu,3/7] target-i386: Move xlevel/xlevel2 in struct x86_def_t

login
register
mail settings
Submitter Eduardo Habkost
Date April 15, 2013, 7:25 p.m.
Message ID <1366053940-9401-4-git-send-email-ehabkost@redhat.com>
Download mbox | patch
Permalink /patch/236698/
State New
Headers show

Comments

Eduardo Habkost - April 15, 2013, 7:25 p.m.
As the *_features fields are going to be replaced with a features array,
the xlevel & xlevel2 fields won't be close to the corresponding
*_features fields anymore.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Igor Mammedov - April 16, 2013, 2:32 p.m.
On Mon, 15 Apr 2013 16:25:36 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> As the *_features fields are going to be replaced with a features array,
> the xlevel & xlevel2 fields won't be close to the corresponding
> *_features fields anymore.

patch has nothing to do with mentioned *_features.

Ii could be better to merge it with 1/7 and commit message could be changed
to something like:

target-i386: cleanup: group together level, xlevel, xlevel2 fields

Consolidate level, xlevel, xlevel2 fields in x86_def_t and CPUX86State.

Otherwise looks good.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index e2302d8..732cafd 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -349,6 +349,8 @@ static void add_flagname_to_bitmaps(const char
> *flagname, typedef struct x86_def_t {
>      const char *name;
>      uint32_t level;
> +    uint32_t xlevel;
> +    uint32_t xlevel2;
>      /* vendor is zero-terminated, 12 character ASCII string */
>      char vendor[CPUID_VENDOR_SZ + 1];
>      int family;
> @@ -356,11 +358,9 @@ typedef struct x86_def_t {
>      int stepping;
>      uint32_t features, ext_features, ext2_features, ext3_features;
>      uint32_t kvm_features, svm_features;
> -    uint32_t xlevel;
>      char model_id[48];
>      /* Store the results of Centaur's CPUID instructions */
>      uint32_t ext4_features;
> -    uint32_t xlevel2;
>      /* The feature bits on CPUID[EAX=7,ECX=0].EBX */
>      uint32_t cpuid_7_0_ebx_features;
>  } x86_def_t;
Eduardo Habkost - April 16, 2013, 2:41 p.m.
On Tue, Apr 16, 2013 at 04:32:03PM +0200, Igor Mammedov wrote:
> On Mon, 15 Apr 2013 16:25:36 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
> 
> > As the *_features fields are going to be replaced with a features array,
> > the xlevel & xlevel2 fields won't be close to the corresponding
> > *_features fields anymore.
> 
> patch has nothing to do with mentioned *_features.

The patch is not directly related to them, but my motivation for the
patch has to do with the *_features fields. I wouldn't send this patch
if I were not going to introduce the array.

> 
> Ii could be better to merge it with 1/7 and commit message could be changed
> to something like:
> 
> target-i386: cleanup: group together level, xlevel, xlevel2 fields
> 
> Consolidate level, xlevel, xlevel2 fields in x86_def_t and CPUX86State.

Merging them and using the description you suggest sounds good to me.

Andreas, what do you want me to do? Should I merge and resubmit? Do you
want to merge before applying? Do you want to keep them separated?

> 
> Otherwise looks good.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  target-i386/cpu.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index e2302d8..732cafd 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -349,6 +349,8 @@ static void add_flagname_to_bitmaps(const char
> > *flagname, typedef struct x86_def_t {
> >      const char *name;
> >      uint32_t level;
> > +    uint32_t xlevel;
> > +    uint32_t xlevel2;
> >      /* vendor is zero-terminated, 12 character ASCII string */
> >      char vendor[CPUID_VENDOR_SZ + 1];
> >      int family;
> > @@ -356,11 +358,9 @@ typedef struct x86_def_t {
> >      int stepping;
> >      uint32_t features, ext_features, ext2_features, ext3_features;
> >      uint32_t kvm_features, svm_features;
> > -    uint32_t xlevel;
> >      char model_id[48];
> >      /* Store the results of Centaur's CPUID instructions */
> >      uint32_t ext4_features;
> > -    uint32_t xlevel2;
> >      /* The feature bits on CPUID[EAX=7,ECX=0].EBX */
> >      uint32_t cpuid_7_0_ebx_features;
> >  } x86_def_t;
>

Patch

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e2302d8..732cafd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -349,6 +349,8 @@  static void add_flagname_to_bitmaps(const char *flagname,
 typedef struct x86_def_t {
     const char *name;
     uint32_t level;
+    uint32_t xlevel;
+    uint32_t xlevel2;
     /* vendor is zero-terminated, 12 character ASCII string */
     char vendor[CPUID_VENDOR_SZ + 1];
     int family;
@@ -356,11 +358,9 @@  typedef struct x86_def_t {
     int stepping;
     uint32_t features, ext_features, ext2_features, ext3_features;
     uint32_t kvm_features, svm_features;
-    uint32_t xlevel;
     char model_id[48];
     /* Store the results of Centaur's CPUID instructions */
     uint32_t ext4_features;
-    uint32_t xlevel2;
     /* The feature bits on CPUID[EAX=7,ECX=0].EBX */
     uint32_t cpuid_7_0_ebx_features;
 } x86_def_t;