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

[Andre Przywara]

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))

Regards,
Andre.

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