Re: Current KVM head crashes on startup

[Avi Kivity <avi@redhat.com>]

Amit Shah wrote:
> On (Wed) Feb 18 2009 [13:21:26], Amit Shah wrote:
>   
>> On (Tue) Feb 17 2009 [12:47:10], Brian Kress wrote:
>>     
>>> When I try to run KVM built off the current head, it crashes with a  
>>> Segmentation fault.  KVM-84 does
>>> not.  Seems to be dealing with the CPUID changes:
>>>
>>>
>>>    0x081a5c70 in host_cpuid ()
>>>        at /home/kressb/kvm/src/qemu/target-i386/helper.c:1426
>>>    1426        asm volatile("pusha \n\t"
>>>       
>> This looks like some kind of stack corruption on 32-bit:
>>
>> 1472            if (kvm_enabled())
>> (gdb)
>> 1473                host_cpuid(0, 0, NULL, ebx, ecx, edx);
>> (gdb)
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x081a2d60 in host_cpuid (function=10, count=1231384169, eax=0x0, ebx=0xadfc1914,
>>     ecx=0xadfc1910, edx=0xadfc190c)
>>     at /home/amit/src/kvm-userspace/qemu/target-i386/helper.c:1426
>> 1426        asm volatile("pusha \n\t"
>>
>> I don't see this on 64-bit. Investigating.
>>     
>
> Avi, what's the reason for doing this in the host_cpuid code? As I see
> it, the first version should work for both 64-bit and 32-bit code.
>
> #ifdef __x86_64__
>     asm volatile("cpuid"
>                  : "=a"(vec[0]), "=b"(vec[1]),
>                    "=c"(vec[2]), "=d"(vec[3])
>                  : "0"(function), "c"(count) : "cc");
> #else
>     asm volatile("pusha \n\t"
>                  "cpuid \n\t"
>                  "mov %%eax, 0(%1) \n\t"
>                  "mov %%ebx, 4(%1) \n\t"
>                  "mov %%ecx, 8(%1) \n\t"
>                  "mov %%edx, 12(%1) \n\t"
>                  "popa"
>                  : : "a"(function), "c"(count), "S"(vec)
>                  : "memory", "cc");
> #endif
>   

The first version generates too much register pressure for some 
compilers on i386, leading to compilation failures.  The second version 
is surely wrong, though?  Counting from zero, the "vec" parameter would 
be %2, not %1.


(copied Anthony)

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.