| Message ID | 20170509132736.10071-2-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
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(-) > [...]
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.
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) {