Eduardo Habkost <ehabk...@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 <ehabk...@redhat.com>
> ---
> Cc: Markus Armbruster <arm...@redhat.com>
> Cc: brill...@viatech.com.cn <brill...@viatech.com.cn>
> Cc: Marcelo Tosatti <mtosa...@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 {
Trivia: brought to my attention by when Coverity voiced suspicions on
the old code we still have in RHEL-6:
if (index & 0x80000000) {
if (index > env->cpuid_xlevel)
index = env->cpuid_level;
} else {
if (index > env->cpuid_level)
index = env->cpuid_level;
}
Looks just like a pasto, doesn't it? But it's correct! Commit b3baa152
silently "fixed" the pasto. This patch reverts that mistake. Thanks
Eduardo for adding the explanatory comment.
Reviewed-by: Markus Armbruster <arm...@redhat.com>
