qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 1/2] i386: rewrite way CPUID index is validat


From: Eduardo Habkost
Subject: Re: [Qemu-devel] [PATCH v3 1/2] i386: rewrite way CPUID index is validated
Date: Tue, 9 May 2017 08:40:32 -0300
User-agent: Mutt/1.8.0 (2017-02-23)

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 <address@hidden>

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 <address@hidden>

> ---
>  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
> 

-- 
Eduardo



reply via email to

[Prev in Thread] Current Thread [Next in Thread]