From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O3cGV-0004cE-0o for qemu-devel@nongnu.org; Sun, 18 Apr 2010 17:42:55 -0400 Received: from [140.186.70.92] (port=49349 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O3cGT-0004bx-Oq for qemu-devel@nongnu.org; Sun, 18 Apr 2010 17:42:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O3cGR-0004Ud-DH for qemu-devel@nongnu.org; Sun, 18 Apr 2010 17:42:53 -0400 Received: from hall.aurel32.net ([88.191.82.174]:50117) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O3cGR-0004UV-72 for qemu-devel@nongnu.org; Sun, 18 Apr 2010 17:42:51 -0400 Date: Sun, 18 Apr 2010 23:42:47 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] Re: [PATCH][STABLE] fix CPUID vendor override Message-ID: <20100418214247.GD12023@hall.aurel32.net> References: <1271013662-7640-1-git-send-email-andre.przywara@amd.com> <4BC2238C.2070009@redhat.com> <4BC227D4.9060700@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <4BC227D4.9060700@amd.com> Sender: Aurelien Jarno List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andre Przywara Cc: Avi Kivity , qemu-devel@nongnu.org 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" -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net