From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: Current KVM head crashes on startup
Date: Wed, 18 Feb 2009 08:49:33 +0000
Message-ID: <499BCB9D.6070005@redhat.com>
References: <499AF81E.7080406@egenera.com> <20090218075126.GB12284@amit-x200.pnq.redhat.com> <20090218081602.GC12284@amit-x200.pnq.redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Cc: kvm@vger.kernel.org, Anthony Liguori <anthony@codemonkey.ws>
To: Amit Shah <amit.shah@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx2.redhat.com ([66.187.237.31]:40361 "EHLO mx2.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1750971AbZBRIyZ (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 18 Feb 2009 03:54:25 -0500
In-Reply-To: <20090218081602.GC12284@amit-x200.pnq.redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.