Re: [Qemu-devel] Re: [PATCH][STABLE] fix CPUID vendor override

[Aurelien Jarno]

On Sun, Apr 11, 2010 at 09:49:40PM +0200, Andre Przywara wrote:
> Avi Kivity wrote:
>> On 04/11/2010 10:21 PM, Andre Przywara wrote:
>>> the meaning of vendor_override is actually the opposite of how it
>>> is currently used :-(
>>> Fix it to allow KVM to export the non-native CPUID vendor if
>>> explicitly requested by the user.
>>>
>>> Signed-off-by: Andre Przywara<address@hidden>
>>> ---
>>>   target-i386/helper.c |    2 +-
>>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> I will send a refactoring patch including this fix for git HEAD later.
>>>
>>> Regards,
>>> Andre.
>>>
>>> diff --git a/target-i386/helper.c b/target-i386/helper.c
>>> index 9d7fec3..c17adc1 100644
>>> --- a/target-i386/helper.c
>>> +++ b/target-i386/helper.c
>>> @@ -1655,7 +1655,7 @@ static void get_cpuid_vendor(CPUX86State *env,  
>>> uint32_t *ebx,
>>>        * this if you want to use KVM's sysenter/syscall emulation
>>>        * in compatibility mode and when doing cross vendor migration
>>>        */
>>> -    if (kvm_enabled()&&  env->cpuid_vendor_override) {
>>> +    if (kvm_enabled()&&  ! env->cpuid_vendor_override) {
>>>           host_cpuid(0, 0, NULL, ebx, ecx, edx);
>>>       }
>>>   }
>>>    
>>
>> Why is the original code wrong?  I would say vendor_override means  
>> overriding the qemu-picked vendor ID in favour of the host cpuid.
> I meant it to mean: override the automatically chosen vendor ID (which  
> is the host ID in case of KVM). I think the reason for KVM to use the  
> host ID is valid, so I wanted to have an explicit override only if the  
> user says so.
> If you look at the code, you will see that it is initialized to 0 and  
> only set to 1 if one specifies an explicit vendor ID on the command line.
> Honestly I cannot say how this bug slipped through, I can only guess  
> that I tricked myself while making the final version of the patch  
> (lost-in-branches(TM))
>

While it clearly fixes the -cpu vendor option when kvm is enabled, it
also forces the vendor id to the one of the host. Try for example -cpu
qemu64. Before your patch the vendor id is "Authentic AMD" and after
your patch it is "Genuine Intel"

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net