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

[Andre Przywara <andre.przywara@amd.com>]

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<andre.przywara@amd.com>
>> ---
>>   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))

Regards,
Andre.

-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 488-3567-12