Re: [Qemu-devel] [PATCH] target-i386: CPUID: return highest basic leaf if eax > cpuid_xlevel

[Anthony Liguori <anthony@codemonkey.ws>]

Eduardo Habkost <ehabkost@redhat.com> writes:

> This fixes a subtle bug. A bug that probably won't cause trouble for any
> existing OS, but a bug anyway:
>
> Intel SDM Volume 2, CPUID Instruction states:
>
>> Two types of information are returned: basic and extended function
>> information. If a value entered for CPUID.EAX is higher than the maximum
>> input value for basic or extended function for that processor then the
>> data for the highest basic information leaf is returned. For example,
>> using the Intel Core i7 processor, the following is true:
>>
>>   CPUID.EAX = 05H (* Returns MONITOR/MWAIT leaf. *)
>>   CPUID.EAX = 0AH (* Returns Architectural Performance Monitoring leaf. *)
>>   CPUID.EAX = 0BH (* Returns Extended Topology Enumeration leaf. *)
>>   CPUID.EAX = 0CH (* INVALID: Returns the same information as CPUID.EAX = 0BH. *)
>>   CPUID.EAX = 80000008H (* Returns linear/physical address size data. *)
>>   CPUID.EAX = 8000000AH (* INVALID: Returns same information as CPUID.EAX = 0BH. *)
>
> AMD's CPUID Specification, on the other hand, is less specific:
>
>> The CPUID instruction supports two sets or ranges of functions,
>> standard and extended.
>>
>> • The smallest function number of the standard function range is
>>   Fn0000_0000. The largest function num- ber of the standard function
>>   range, for a particular implementation, is returned in CPUID
>>   Fn0000_0000_EAX.
>>
>> • The smallest function number of the extended function range is
>>   Fn8000_0000. The largest function num- ber of the extended function
>>   range, for a particular implementation, is returned in CPUID
>>   Fn8000_0000_EAX.
>>
>> Functions that are neither standard nor extended are undefined and
>> should not be relied upon.
>
> QEMU's behavior matched Intel's specification before, but this was
> changed by commit b3baa152aaef1905876670590275c2dd0bbb088c. This patch
> restores the behavior documented by Intel when cpuid_xlevel2 is 0.
>
> The existing behavior when cpuid_xlevel2 is set (falling back to
> level=cpuid_xlevel) is being kept, as I couldn't find any public
> documentation on the CPUID 0xC0000000 function range on Centaur CPUs.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Applied. Thanks.

Regards,

Anthony Liguori

> ---
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: brillywu@viatech.com.cn <brillywu@viatech.com.cn>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> ---
>  target-i386/cpu.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 1837f5a..3cd1cee 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1648,7 +1648,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>                      index = env->cpuid_xlevel;
>                  }
>              } else {
> -                index =  env->cpuid_xlevel;
> +                /* 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;
>              }
>          }
>      } else {
> -- 
> 1.7.11.7