[RFC PATCH] x86 emulator: emulate CPUID instruction

[RFC PATCH] x86 emulator: emulate CPUID instruction

[Guillaume Thouvenin]

This patch emulates CPUID instruction. It should work but when I'm
testing it in kvm-userspace, the cpuid instruction seems to have no
effect (I mean eax == ebx == ecx == edx == 0x0). Should
kvm_cpuid_emulate() be modified? Is cpuid instruction correctly
implemented?

Regards,

Signed-off-by: Guillaume Thouvenin <guillaume.thouvenin@ext.bull.net>
---
 arch/x86/kvm/x86_emulate.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
index d174db7..4d1821c 100644
--- a/arch/x86/kvm/x86_emulate.c
+++ b/arch/x86/kvm/x86_emulate.c
@@ -237,7 +237,7 @@ static u32 twobyte_table[256] = {
 	/* 0x90 - 0x9F */
 	0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
 	/* 0xA0 - 0xA7 */
-	0, 0, 0, DstMem | SrcReg | ModRM | BitOp,
+	0, 0, ImplicitOps, DstMem | SrcReg | ModRM | BitOp,
 	DstMem | SrcReg | Src2ImmByte | ModRM,
 	DstMem | SrcReg | Src2CL | ModRM, 0, 0,
 	/* 0xA8 - 0xAF */
@@ -2035,6 +2035,9 @@ twobyte_insn:
 		c->dst.type = OP_NONE;
 		break;
 	}
+	case 0xa2:		/* cpuid */
+		kvm_emulate_cpuid(ctxt->vcpu);
+		break;
 	case 0xa3:
 	      bt:		/* bt */
 		c->dst.type = OP_NONE;
-- 
1.6.0.4.623.g171d7

Re: [RFC PATCH] x86 emulator: emulate CPUID instruction

[Avi Kivity]

Guillaume Thouvenin wrote:
> This patch emulates CPUID instruction. It should work but when I'm
> testing it in kvm-userspace, the cpuid instruction seems to have no
> effect (I mean eax == ebx == ecx == edx == 0x0). Should
> kvm_cpuid_emulate() be modified? Is cpuid instruction correctly
> implemented?
>
>   

It doesn't have any effect because writeback() writes back the cached 
registers back into vcpu->arch.regs, overwriting kvm_cpuid_emulate()'s 
effect.

You need to first modify kvm_cpuid_emulate() to accept the registers as 
in/out parameters, and pass the emulator's registers which can be later 
written back.

An alternative implementation can modify the emulator not to work on a 
temporarary copy, instead something like this:

decode():
   copy registers to backup store

execute():
   start executing
   if fail:
        copy registers from backup store to main state
 
The later approach is more invasive, but has the advantage of speeding 
up the emulator (since usually emulation will succeed, so we don't need 
to writeback)

-- 
error compiling committee.c: too many arguments to function