From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O3dID-0005KW-HL for qemu-devel@nongnu.org; Sun, 18 Apr 2010 18:48:45 -0400 Received: from [140.186.70.92] (port=52432 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O3dIC-0005JY-1X for qemu-devel@nongnu.org; Sun, 18 Apr 2010 18:48:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O3dIA-0005Tp-PN for qemu-devel@nongnu.org; Sun, 18 Apr 2010 18:48:43 -0400 Received: from tx2ehsobe002.messaging.microsoft.com ([65.55.88.12]:54107 helo=TX2EHSOBE004.bigfish.com) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O3dIA-0005TA-KG for qemu-devel@nongnu.org; Sun, 18 Apr 2010 18:48:42 -0400 Message-ID: <4BCB8C42.7030100@amd.com> Date: Mon, 19 Apr 2010 00:48:34 +0200 From: Andre Przywara MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH][STABLE] fix CPUID vendor override References: <1271013662-7640-1-git-send-email-andre.przywara@amd.com> <4BC2238C.2070009@redhat.com> <4BC227D4.9060700@amd.com> <20100418214247.GD12023@hall.aurel32.net> In-Reply-To: <20100418214247.GD12023@hall.aurel32.net> Content-Type: text/plain; charset="ISO-8859-15"; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: Avi Kivity , qemu-devel@nongnu.org Aurelien Jarno wrote: > 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 >>>> --- >>>> 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" You are using the wrong CPU ;-) This is actually correct for KVM. (or are you seeing this with TCG?) To make sure we are actually talk about the same problem, the intended behavior is: With TCG: - always inject the configured vendor (either hard-coded, in config files or via ",vendor=" commandline) With KVM: - by default inject the host's vendor - if the user specifies ",vendor=" on the commandline, use this instead of the host's vendor - all pre-configured vendors (hard-coded, config file) are ignored The reason KVM is injecting the host's vendor is the following: There is a nasty difference between AMD and Intel chips in supporting syscall and sysenter in _compat_ mode (that is: running a 32bit binary while the OS is running 64bit). This mode is 99% compatible with the legacy protected mode, but AMD does not support sysenter here and Intel does not support syscall. So for instance the Linux kernel checks the vendor id and tells the userspace to use the supported instruction. On TCG we don't care as we properly emulate both(?), but on KVM the instruction is called natively. This leads to a crash (#UD exception) if the wrong vendor is used. So a long time ago the workaround of always propagating the host vendor was introduced. Later last year I introduced emulation of the missing instructions, so to overcome the former limitation (and allow for more hacking with KVM) I introduced the vendor override. Sadly my patch was broken (I used a slightly different version for development), that's why this fix. qemu-kvm had an additional fix to kind of work-around this bug. Hope that clarifies the issue, if you meant something else, tell me. Regards, Andre. -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany Tel: +49 351 488-3567-12