v2: Remove duplicate cpuid functionality in kvm userspace code

v2: Remove duplicate cpuid functionality in kvm userspace code

[Amit Shah]

This supersedes my previous patch which just removed the host_cpuid function.

This patch cleans up the do_cpuid_ent function of duplicate code as well.

Amit

[PATCH] KVM: userspace: Remove duplicated functionality for cpuid processing

[Amit Shah]

host_cpuid is now available in target-i386/helper.c.
Remove the duplicated code now in kvm-specific code.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 qemu/qemu-kvm-x86.c |   70 ---------------------------------------------------
 1 files changed, 0 insertions(+), 70 deletions(-)

diff --git a/qemu/qemu-kvm-x86.c b/qemu/qemu-kvm-x86.c
index aa36be8..1bf86e1 100644
--- a/qemu/qemu-kvm-x86.c
+++ b/qemu/qemu-kvm-x86.c
@@ -451,39 +451,6 @@ void kvm_arch_save_regs(CPUState *env)
     }
 }
 
-static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t *ebx,
-		       uint32_t *ecx, uint32_t *edx)
-{
-    uint32_t vec[4];
-
-#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 (eax)
-	*eax = vec[0];
-    if (ebx)
-	*ebx = vec[1];
-    if (ecx)
-	*ecx = vec[2];
-    if (edx)
-	*edx = vec[3];
-}
-
-
 static void do_cpuid_ent(struct kvm_cpuid_entry *e, uint32_t function,
 			 CPUState *env)
 {
@@ -494,43 +461,6 @@ static void do_cpuid_ent(struct kvm_cpuid_entry *e, uint32_t function,
     e->ebx = env->regs[R_EBX];
     e->ecx = env->regs[R_ECX];
     e->edx = env->regs[R_EDX];
-    if (function == 0x80000001) {
-	uint32_t h_eax, h_edx;
-
-	host_cpuid(function, &h_eax, NULL, NULL, &h_edx);
-
-	// long mode
-	if ((h_edx & 0x20000000) == 0 || !lm_capable_kernel)
-	    e->edx &= ~0x20000000u;
-	// syscall
-	if ((h_edx & 0x00000800) == 0)
-	    e->edx &= ~0x00000800u;
-	// nx
-	if ((h_edx & 0x00100000) == 0)
-	    e->edx &= ~0x00100000u;
-	// svm
-	if (!kvm_nested && (e->ecx & 4))
-	    e->ecx &= ~4u;
-    }
-    // sysenter isn't supported on compatibility mode on AMD.  and syscall
-    // isn't supported in compatibility mode on Intel.  so advertise the
-    // actuall cpu, and say goodbye to migration between different vendors
-    // is you use compatibility mode.
-    if (function == 0) {
-	uint32_t bcd[3];
-
-	host_cpuid(0, NULL, &bcd[0], &bcd[1], &bcd[2]);
-	e->ebx = bcd[0];
-	e->ecx = bcd[1];
-	e->edx = bcd[2];
-    }
-    // "Hypervisor present" bit for Microsoft guests
-    if (function == 1)
-	e->ecx |= (1u << 31);
-
-    // 3dnow isn't properly emulated yet
-    if (function == 0x80000001)
-	e->edx &= ~0xc0000000;
 }
 
 struct kvm_para_features {
-- 
1.5.4.3

Re: [PATCH] KVM: userspace: Remove duplicated functionality for cpuid processing

[Alexander Graf]

Hi Amit,

On 26.12.2008, at 07:02, Amit Shah wrote:

> host_cpuid is now available in target-i386/helper.c.
> Remove the duplicated code now in kvm-specific code.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
> qemu/qemu-kvm-x86.c |   70  
> ---------------------------------------------------
> 1 files changed, 0 insertions(+), 70 deletions(-)
>
> diff --git a/qemu/qemu-kvm-x86.c b/qemu/qemu-kvm-x86.c
> index aa36be8..1bf86e1 100644
> --- a/qemu/qemu-kvm-x86.c
> +++ b/qemu/qemu-kvm-x86.c
> @@ -451,39 +451,6 @@ void kvm_arch_save_regs(CPUState *env)
>     }
> }
>
> -static void host_cpuid(uint32_t function, uint32_t *eax, uint32_t  
> *ebx,
> -		       uint32_t *ecx, uint32_t *edx)
> -{
> -    uint32_t vec[4];
> -
> -#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 (eax)
> -	*eax = vec[0];
> -    if (ebx)
> -	*ebx = vec[1];
> -    if (ecx)
> -	*ecx = vec[2];
> -    if (edx)
> -	*edx = vec[3];
> -}
> -
> -
> static void do_cpuid_ent(struct kvm_cpuid_entry *e, uint32_t function,
> 			 CPUState *env)
> {
> @@ -494,43 +461,6 @@ static void do_cpuid_ent(struct kvm_cpuid_entry  
> *e, uint32_t function,
>     e->ebx = env->regs[R_EBX];
>     e->ecx = env->regs[R_ECX];
>     e->edx = env->regs[R_EDX];

That looks a lot better, but I think we could easily do more!

do_cpuid_ent is only called twice like this:  
do_cpuid_ent(&cpuid_ent[cpuid_nent++], i, &copy);

We can replace that with:

e->eax = i;
struct kvm_cpuid_entry *e = &cpuid_ent[cpuid_nent++];
cpu_x86_cpuid(&copy, &e->eax, &e->ebx, &e->ecx, &e->edx);

The same could be done for qemu_kvm_cpuid_on_env. Then we can get rid  
of qemu-kvm-helper.c too :-).

Alex

Re: [PATCH] KVM: userspace: Remove duplicated functionality for cpuid processing

[Avi Kivity]

Amit Shah wrote:
> host_cpuid is now available in target-i386/helper.c.
> Remove the duplicated code now in kvm-specific code.
>
>   

Applied, thanks.

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