[qom-cpu,5/9] target-i386: Add ECX information to FeatureWordInfo

FEAT_7_0_EBX uses ECX as input, so we have to take that into account
when reporting feature word values.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target-i386/cpu.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Am 22.04.2013 21:00, schrieb Eduardo Habkost:
> FEAT_7_0_EBX uses ECX as input, so we have to take that into account
> when reporting feature word values.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 110ef98..314931e 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -151,8 +151,10 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
>  
>  typedef struct FeatureWordInfo {
>      const char **feat_names;
> -    uint32_t cpuid_eax; /* Input EAX for CPUID */
> -    int cpuid_reg;      /* R_* register constant */
> +    uint32_t cpuid_eax;   /* Input EAX for CPUID */

> +    bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */

Why do we need this needs_ field here? eax and reg just seem to be
reindented and the comment reworded for reg.

It just seems to be passed through to has_cpuid_input_ecx, which I
neither see defined nor checked - that data structure already existed
elsewhere as such?

Andreas

> +    uint32_t cpuid_ecx;   /* Input ECX value for CPUID */
> +    int cpuid_reg;        /* output register (R_* constant) */
>  } FeatureWordInfo;
>  
>  static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> @@ -186,7 +188,9 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>      },
>      [FEAT_7_0_EBX] = {
>          .feat_names = cpuid_7_0_ebx_feature_name,
> -        .cpuid_eax = 7, .cpuid_reg = R_EBX,
> +        .cpuid_eax = 7,
> +        .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> +        .cpuid_reg = R_EBX,
>      },
>  };
>  
>

On Fri, May 03, 2013 at 05:16:46PM +0200, Andreas Färber wrote:
> Am 22.04.2013 21:00, schrieb Eduardo Habkost:
> > FEAT_7_0_EBX uses ECX as input, so we have to take that into account
> > when reporting feature word values.
> > 
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  target-i386/cpu.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index 110ef98..314931e 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -151,8 +151,10 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
> >  
> >  typedef struct FeatureWordInfo {
> >      const char **feat_names;
> > -    uint32_t cpuid_eax; /* Input EAX for CPUID */
> > -    int cpuid_reg;      /* R_* register constant */
> > +    uint32_t cpuid_eax;   /* Input EAX for CPUID */
> 
> > +    bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */
> 
> Why do we need this needs_ field here? eax and reg just seem to be
> reindented and the comment reworded for reg.

We need it so we know if ECX is used as input on that CPUID leaf, in
other words: so we know if "cpuid-input-ecx" needs to be set on
X86CPUFeatureWordInfo or not.

"cpuid-input-ecx" is optional because most CPUID leafs don't depend on
input ECX value, only EAX. cpuid_needs_ecx has a similar meaning to
KVM_CPUID_FLAG_SIGNIFCANT_INDEX on kvm_cpuid_entry2.flags.

> 
> It just seems to be passed through to has_cpuid_input_ecx, which I
> neither see defined nor checked - that data structure already existed
> elsewhere as such?

X86CPUFeatureWordInfo is generated from the QAPI schema.

> 
> Andreas
> 
> > +    uint32_t cpuid_ecx;   /* Input ECX value for CPUID */
> > +    int cpuid_reg;        /* output register (R_* constant) */
> >  } FeatureWordInfo;
> >  
> >  static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> > @@ -186,7 +188,9 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
> >      },
> >      [FEAT_7_0_EBX] = {
> >          .feat_names = cpuid_7_0_ebx_feature_name,
> > -        .cpuid_eax = 7, .cpuid_reg = R_EBX,
> > +        .cpuid_eax = 7,
> > +        .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> > +        .cpuid_reg = R_EBX,
> >      },
> >  };
> >  
> > 
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Am 03.05.2013 17:54, schrieb Eduardo Habkost:
> On Fri, May 03, 2013 at 05:16:46PM +0200, Andreas Färber wrote:
>> Am 22.04.2013 21:00, schrieb Eduardo Habkost:
>>> FEAT_7_0_EBX uses ECX as input, so we have to take that into account
>>> when reporting feature word values.
>>>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Thanks for the explanations, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas