From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LSdr9-00037S-Kk for qemu-devel@nongnu.org; Thu, 29 Jan 2009 15:51:23 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LSdr6-00035q-Tx for qemu-devel@nongnu.org; Thu, 29 Jan 2009 15:51:23 -0500 Received: from [199.232.76.173] (port=55087 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LSdr6-00035m-Oc for qemu-devel@nongnu.org; Thu, 29 Jan 2009 15:51:20 -0500 Received: from e35.co.us.ibm.com ([32.97.110.153]:52462) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LSdr6-0003x9-6y for qemu-devel@nongnu.org; Thu, 29 Jan 2009 15:51:20 -0500 Received: from d03relay02.boulder.ibm.com (d03relay02.boulder.ibm.com [9.17.195.227]) by e35.co.us.ibm.com (8.13.1/8.13.1) with ESMTP id n0TKmFra020806 for ; Thu, 29 Jan 2009 13:48:15 -0700 Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id n0TKpDEl176218 for ; Thu, 29 Jan 2009 13:51:14 -0700 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n0TKpDdd017656 for ; Thu, 29 Jan 2009 13:51:13 -0700 Message-ID: <498216B0.9030400@us.ibm.com> Date: Thu, 29 Jan 2009 14:50:56 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1233249569-16686-1-git-send-email-glommer@redhat.com> <1233249569-16686-2-git-send-email-glommer@redhat.com> <1233249569-16686-3-git-send-email-glommer@redhat.com> <1233249569-16686-4-git-send-email-glommer@redhat.com> In-Reply-To: <1233249569-16686-4-git-send-email-glommer@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 3/4] mask out forbidden cpuid features with kvm ioctl Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Glauber Costa Cc: qemu-devel@nongnu.org Glauber Costa wrote: > KVM has a (so far unused) ioctl to inform userspace > about supported cpuid bits in the current host. > > The lack of this kind of checking can lead to bugs in which > a cpuid bit is exposed but not supported by the host kernel > (an example is fedora bug https://bugzilla.redhat.com/show_bug.cgi?id=481274) > > The current host_cpuid code is replaced by this general check. > > Signed-off-by: Glauber Costa > So what do we do about live migration? Does KVM mask a set of bits unconditionally? Before, we were masking unconditionally which should insulate migration reasonable well. Regards, Anthony Liguori > --- > target-i386/helper.c | 89 ++++++++++++++++++++++--------------------------- > 1 files changed, 40 insertions(+), 49 deletions(-) > > diff --git a/target-i386/helper.c b/target-i386/helper.c > index a28ab93..1410002 100644 > --- a/target-i386/helper.c > +++ b/target-i386/helper.c > @@ -1370,37 +1370,33 @@ static void breakpoint_handler(CPUState *env) > } > #endif /* !CONFIG_USER_ONLY */ > > -static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx, > +static void host_cpuid(CPUState *env, uint32_t function, uint32_t *eax, uint32_t *ebx, > uint32_t *ecx, uint32_t *edx) > { > #if defined(CONFIG_KVM) > - uint32_t vec[4]; > + static struct { > + struct kvm_cpuid2 cpuid; > + struct kvm_cpuid_entry2 entries[100]; > + } __attribute__((packed)) cpuid_base; > + int i; > > -#ifdef __x86_64__ > - asm volatile("cpuid" > - : "=a"(vec[0]), "=b"(vec[1]), > - "=c"(vec[2]), "=d"(vec[3]) > - : "0"(function) : "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), "S"(vec) > - : "memory", "cc"); > -#endif > + if (cpuid_base.cpuid.nent == 0) { > + cpuid_base.cpuid.nent = 100; > + kvm_ioctl(env->kvm_state, KVM_GET_SUPPORTED_CPUID, &cpuid_base); > + } > > - if (eax) > - *eax = vec[0]; > - if (ebx) > - *ebx = vec[1]; > - if (ecx) > - *ecx = vec[2]; > - if (edx) > - *edx = vec[3]; > + for (i = 0; i < cpuid_base.cpuid.nent; i++) { > + if (cpuid_base.entries[i].function == function) { > + if (eax) > + *eax = cpuid_base.entries[i].eax; > + if (ebx) > + *ebx = cpuid_base.entries[i].ebx; > + if (ecx) > + *ecx = cpuid_base.entries[i].ecx; > + if (edx) > + *edx = cpuid_base.entries[i].edx; > + } > + } > #endif > } > > @@ -1429,7 +1425,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, > * actuall cpu, and say goodbye to migration between different vendors > * is you use compatibility mode. */ > if (kvm_enabled()) > - host_cpuid(0, NULL, ebx, ecx, edx); > + host_cpuid(env, 0, NULL, ebx, ecx, edx); > break; > case 1: > *eax = env->cpuid_version; > @@ -1437,9 +1433,18 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, > *ecx = env->cpuid_ext_features; > *edx = env->cpuid_features; > > - /* "Hypervisor present" bit required for Microsoft SVVP */ > - if (kvm_enabled()) > + if (kvm_enabled()) { > + uint32_t h_eax; > + uint32_t h_ecx = ~0U; > + uint32_t h_edx = ~0U; > + > + host_cpuid(env, 1, &h_eax, NULL, &h_ecx, &h_edx); > + > + *ecx &= h_ecx; > + *edx &= h_edx; > + /* "Hypervisor present" bit required for Microsoft SVVP */ > *ecx |= (1 << 31); > + } > break; > case 2: > /* cache info: needed for Pentium Pro compatibility */ > @@ -1519,28 +1524,14 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, > *edx = env->cpuid_ext2_features; > > if (kvm_enabled()) { > - uint32_t h_eax, h_edx; > - > - host_cpuid(0x80000001, &h_eax, NULL, NULL, &h_edx); > - > - /* disable CPU features that the host does not support */ > - > - /* long mode */ > - if ((h_edx & 0x20000000) == 0 /* || !lm_capable_kernel */) > - *edx &= ~0x20000000; > - /* syscall */ > - if ((h_edx & 0x00000800) == 0) > - *edx &= ~0x00000800; > - /* nx */ > - if ((h_edx & 0x00100000) == 0) > - *edx &= ~0x00100000; > + uint32_t h_eax; > + uint32_t h_ecx = ~0U; > + uint32_t h_edx = ~0U; > > - /* disable CPU features that KVM cannot support */ > + host_cpuid(env, 0x80000001, &h_eax, NULL, &h_ecx, &h_edx); > > - /* svm */ > - *ecx &= ~4UL; > - /* 3dnow */ > - *edx &= ~0xc0000000; > + *ecx &= h_ecx; > + *edx &= h_edx; > } > break; > case 0x80000002: >