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<andre.przyw...@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))
>
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
aurel...@aurel32.net http://www.aurel32.net
