Change the nested if statements into a flat format, to make
it clearer what validation / capping is being performed on
different CPUID index values.

NB this changes behaviour when "index > env->cpuid_xlevel2".
This won't have any guest-visible effect because no there is
no CPUID[0xC0000001] feature supported by TCG, and KVM code
will never call cpu_x86_cpuid() with such an index value.

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 target/i386/cpu.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 13c0985..8cb4af4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2626,28 +2626,23 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
     X86CPU *cpu = x86_env_get_cpu(env);
     CPUState *cs = CPU(cpu);
     uint32_t pkg_offset;
+    uint32_t limit;
 
-    /* test if maximum index reached */
-    if (index & 0x80000000) {
-        if (index > env->cpuid_xlevel) {
-            if (env->cpuid_xlevel2 > 0) {
-                /* Handle the Centaur's CPUID instruction. */
-                if (index > env->cpuid_xlevel2) {
-                    index = env->cpuid_xlevel2;
-                } else if (index < 0xC0000000) {
-                    index = env->cpuid_xlevel;
-                }
-            } else {
-                /* Intel documentation states that invalid EAX input will
-                 * return the same information as EAX=cpuid_level
-                 * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
-                 */
-                index =  env->cpuid_level;
-            }
-        }
+    /* Calculate & apply limits for different index ranges */
+    if (index >= 0xC0000000) {
+        limit = env->cpuid_xlevel2;
+    } else if (index >= 0x80000000) {
+        limit = env->cpuid_xlevel;
     } else {
-        if (index > env->cpuid_level)
-            index = env->cpuid_level;
+        limit = env->cpuid_level;
+    }
+
+    if (index > limit) {
+        /* Intel documentation states that invalid EAX input will
+         * return the same information as EAX=cpuid_level
+         * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
+         */
+        index = env->cpuid_level;
     }
 
     switch(index) {
-- 
2.9.3

On Tue, May 09, 2017 at 02:27:35PM +0100, Daniel P. Berrange wrote:
> Change the nested if statements into a flat format, to make
> it clearer what validation / capping is being performed on
> different CPUID index values.
> 
> NB this changes behaviour when "index > env->cpuid_xlevel2".
> This won't have any guest-visible effect because no there is
> no CPUID[0xC0000001]

Nit: When applying, maybe the maintainer could fix the typo:

"because no there is no" -> "because there is no"

> feature supported by TCG, and KVM code
> will never call cpu_x86_cpuid() with such an index value.
> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  target/i386/cpu.c | 35 +++++++++++++++--------------------
>  1 file changed, 15 insertions(+), 20 deletions(-)
> 

[...]

-- 
/kashyap

On Fri, May 19, 2017 at 04:58:23PM +0200, Kashyap Chamarthy wrote:
> On Tue, May 09, 2017 at 02:27:35PM +0100, Daniel P. Berrange wrote:
> > Change the nested if statements into a flat format, to make
> > it clearer what validation / capping is being performed on
> > different CPUID index values.
> > 
> > NB this changes behaviour when "index > env->cpuid_xlevel2".
> > This won't have any guest-visible effect because no there is
> > no CPUID[0xC0000001]
> 
> Nit: When applying, maybe the maintainer could fix the typo:
> 
> "because no there is no" -> "because there is no"

This was already merged, unfortunately.  Thanks for noting that,
anyway.

-- 
Eduardo