| Message ID | 20170509112034.23351-2-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, May 09, 2017 at 12:20:33PM +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. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Probably worth noting in the commit message that the patch changes the results of cpu_x86_cpuid() when (index > env->cpuid_xlevel2), and that it won't have any guest-visible effect because no CPUID[0xC0000001] feature is supported by TCG, and KVM code will never call cpu_x86_cpuid() with (index > env->cpuid_xlevel2). Reviewed-by: Eduardo Habkost <ehabkost@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 >
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) {
Change the nested if statements into a flat format, to make it clearer what validation / capping is being performed on different CPUID index values. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- target/i386/cpu.c | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-)