* [PATCH 1/2] KVM: x86 emulator: put register operand write back to a function
@ 2010-08-12 13:38 Wei Yongjun
2010-08-12 13:41 ` [PATCH 2/2] KVM: x86 emulator: add XADD instruction emulation Wei Yongjun
2010-08-15 11:30 ` [PATCH 1/2] KVM: x86 emulator: put register operand write back to a function Avi Kivity
0 siblings, 2 replies; 7+ messages in thread
From: Wei Yongjun @ 2010-08-12 13:38 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
Introduce function write_register_operand() to write back the
register operand.
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
arch/x86/kvm/emulate.c | 53 +++++++++++++++++++----------------------------
1 files changed, 22 insertions(+), 31 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index c476a67..8bf80a9 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1020,6 +1020,26 @@ exception:
return X86EMUL_PROPAGATE_FAULT;
}
+static void write_register_operand(struct operand *op, unsigned long val,
+ unsigned int bytes)
+{
+ /* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
+ switch (bytes) {
+ case 1:
+ *(u8 *)op->addr.reg = (u8)val;
+ break;
+ case 2:
+ *(u16 *)op->addr.reg = (u16)val;
+ break;
+ case 4:
+ *op->addr.reg = (u32)val;
+ break; /* 64b: zero-extend */
+ case 8:
+ *op->addr.reg = val;
+ break;
+ }
+}
+
static inline int writeback(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops)
{
@@ -1029,23 +1049,7 @@ static inline int writeback(struct x86_emulate_ctxt *ctxt,
switch (c->dst.type) {
case OP_REG:
- /* The 4-byte case *is* correct:
- * in 64-bit mode we zero-extend.
- */
- switch (c->dst.bytes) {
- case 1:
- *(u8 *)c->dst.addr.reg = (u8)c->dst.val;
- break;
- case 2:
- *(u16 *)c->dst.addr.reg = (u16)c->dst.val;
- break;
- case 4:
- *c->dst.addr.reg = (u32)c->dst.val;
- break; /* 64b: zero-ext */
- case 8:
- *c->dst.addr.reg = c->dst.val;
- break;
- }
+ write_register_operand(&c->dst, c->dst.val, c->dst.bytes);
break;
case OP_MEM:
if (c->lock_prefix)
@@ -2971,20 +2975,7 @@ special_insn:
case 0x86 ... 0x87: /* xchg */
xchg:
/* Write back the register source. */
- switch (c->dst.bytes) {
- case 1:
- *(u8 *) c->src.addr.reg = (u8) c->dst.val;
- break;
- case 2:
- *(u16 *) c->src.addr.reg = (u16) c->dst.val;
- break;
- case 4:
- *c->src.addr.reg = (u32) c->dst.val;
- break; /* 64b reg: zero-extend */
- case 8:
- *c->src.addr.reg = c->dst.val;
- break;
- }
+ write_register_operand(&c->src, c->dst.val, c->dst.bytes);
/*
* Write back the memory destination with implicit LOCK
* prefix.
--
1.7.0.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] KVM: x86 emulator: add XADD instruction emulation
2010-08-12 13:38 [PATCH 1/2] KVM: x86 emulator: put register operand write back to a function Wei Yongjun
@ 2010-08-12 13:41 ` Wei Yongjun
2010-08-12 15:34 ` Paolo Bonzini
2010-08-15 11:30 ` [PATCH 1/2] KVM: x86 emulator: put register operand write back to a function Avi Kivity
1 sibling, 1 reply; 7+ messages in thread
From: Wei Yongjun @ 2010-08-12 13:41 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm
Add XADD instruction emulation (opcode 0x0f 0xc0~0xc1)
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
arch/x86/kvm/emulate.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 8bf80a9..7c47e37 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2387,7 +2387,8 @@ static struct opcode twobyte_table[256] = {
D(DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
D(ByteOp | DstReg | SrcMem | ModRM | Mov), D(DstReg | SrcMem16 | ModRM | Mov),
/* 0xC0 - 0xCF */
- N, N, N, D(DstMem | SrcReg | ModRM | Mov),
+ D(ByteOp | DstMem | SrcReg | ModRM), D(DstMem | SrcReg | ModRM),
+ N, D(DstMem | SrcReg | ModRM | Mov),
N, N, N, GD(0, &group9),
N, N, N, N, N, N, N, N,
/* 0xD0 - 0xDF */
@@ -3532,6 +3533,12 @@ twobyte_insn:
c->dst.val = (c->d & ByteOp) ? (s8) c->src.val :
(s16) c->src.val;
break;
+ case 0xc0 ... 0xc1: /* xadd */
+ /* Write back the register source. */
+ write_register_operand(&c->src, c->dst.val, c->dst.bytes);
+ /* Write back the memory destination with implicit LOCK prefix. */
+ c->lock_prefix = 1;
+ goto add;
case 0xc3: /* movnti */
c->dst.bytes = c->op_bytes;
c->dst.val = (c->op_bytes == 4) ? (u32) c->src.val :
--
1.7.0.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] KVM: x86 emulator: add XADD instruction emulation
2010-08-12 13:41 ` [PATCH 2/2] KVM: x86 emulator: add XADD instruction emulation Wei Yongjun
@ 2010-08-12 15:34 ` Paolo Bonzini
2010-08-13 5:13 ` [PATCH 2/2 v2] " Wei Yongjun
0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2010-08-12 15:34 UTC (permalink / raw)
To: Wei Yongjun; +Cc: Avi Kivity, kvm
On 08/12/2010 09:41 AM, Wei Yongjun wrote:
> + case 0xc0 ... 0xc1: /* xadd */
> + /* Write back the register source. */
> + write_register_operand(&c->src, c->dst.val, c->dst.bytes);
> + /* Write back the memory destination with implicit LOCK prefix. */
> + c->lock_prefix = 1;
It's not a major performance problem, but xadd does _not_ have an
implicit LOCK prefix.
Paolo
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 v2] KVM: x86 emulator: add XADD instruction emulation
2010-08-12 15:34 ` Paolo Bonzini
@ 2010-08-13 5:13 ` Wei Yongjun
2010-08-13 5:32 ` [PATCH 2/2 v3] " Wei Yongjun
0 siblings, 1 reply; 7+ messages in thread
From: Wei Yongjun @ 2010-08-13 5:13 UTC (permalink / raw)
To: Paolo Bonzini, Avi Kivity; +Cc: kvm
Add XADD instruction emulation (opcode 0x0f 0xc0~0xc1)
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
v1 -> v2: remove implicit LOCK prefix
---
arch/x86/kvm/emulate.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 8bf80a9..279547a 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2387,7 +2387,8 @@ static struct opcode twobyte_table[256] = {
D(DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
D(ByteOp | DstReg | SrcMem | ModRM | Mov), D(DstReg | SrcMem16 | ModRM | Mov),
/* 0xC0 - 0xCF */
- N, N, N, D(DstMem | SrcReg | ModRM | Mov),
+ D(ByteOp | DstMem | SrcReg | ModRM), D(DstMem | SrcReg | ModRM),
+ N, D(DstMem | SrcReg | ModRM | Mov),
N, N, N, GD(0, &group9),
N, N, N, N, N, N, N, N,
/* 0xD0 - 0xDF */
@@ -3532,6 +3533,10 @@ twobyte_insn:
c->dst.val = (c->d & ByteOp) ? (s8) c->src.val :
(s16) c->src.val;
break;
+ case 0xc0 ... 0xc1: /* xadd */
+ /* Write back the register source. */
+ write_register_operand(&c->src, c->dst.val, c->dst.bytes);
+ goto add;
case 0xc3: /* movnti */
c->dst.bytes = c->op_bytes;
c->dst.val = (c->op_bytes == 4) ? (u32) c->src.val :
--
1.7.0.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2 v3] KVM: x86 emulator: add XADD instruction emulation
2010-08-13 5:13 ` [PATCH 2/2 v2] " Wei Yongjun
@ 2010-08-13 5:32 ` Wei Yongjun
0 siblings, 0 replies; 7+ messages in thread
From: Wei Yongjun @ 2010-08-13 5:32 UTC (permalink / raw)
To: Paolo Bonzini, Avi Kivity; +Cc: kvm
Add XADD instruction emulation (opcode 0x0f 0xc0~0xc1)
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
v2 -> v3: add Lock prefix to decode
---
arch/x86/kvm/emulate.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 8bf80a9..e091718 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -2387,7 +2387,8 @@ static struct opcode twobyte_table[256] = {
D(DstReg | SrcMem | ModRM), D(DstReg | SrcMem | ModRM),
D(ByteOp | DstReg | SrcMem | ModRM | Mov), D(DstReg | SrcMem16 | ModRM | Mov),
/* 0xC0 - 0xCF */
- N, N, N, D(DstMem | SrcReg | ModRM | Mov),
+ D(ByteOp | DstMem | SrcReg | ModRM | Lock), D(DstMem | SrcReg | ModRM | Lock),
+ N, D(DstMem | SrcReg | ModRM | Mov),
N, N, N, GD(0, &group9),
N, N, N, N, N, N, N, N,
/* 0xD0 - 0xDF */
@@ -3532,6 +3533,10 @@ twobyte_insn:
c->dst.val = (c->d & ByteOp) ? (s8) c->src.val :
(s16) c->src.val;
break;
+ case 0xc0 ... 0xc1: /* xadd */
+ /* Write back the register source. */
+ write_register_operand(&c->src, c->dst.val, c->dst.bytes);
+ goto add;
case 0xc3: /* movnti */
c->dst.bytes = c->op_bytes;
c->dst.val = (c->op_bytes == 4) ? (u32) c->src.val :
--
1.7.0.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] KVM: x86 emulator: put register operand write back to a function
2010-08-12 13:38 [PATCH 1/2] KVM: x86 emulator: put register operand write back to a function Wei Yongjun
2010-08-12 13:41 ` [PATCH 2/2] KVM: x86 emulator: add XADD instruction emulation Wei Yongjun
@ 2010-08-15 11:30 ` Avi Kivity
[not found] ` <4C689B56.4000405@cn.fujitsu.com>
1 sibling, 1 reply; 7+ messages in thread
From: Avi Kivity @ 2010-08-15 11:30 UTC (permalink / raw)
To: Wei Yongjun; +Cc: kvm
On 08/12/2010 04:38 PM, Wei Yongjun wrote:
> Introduce function write_register_operand() to write back the
> register operand.
>
>
>
> +static void write_register_operand(struct operand *op, unsigned long val,
> + unsigned int bytes)
> +{
> + /* The 4-byte case *is* correct: in 64-bit mode we zero-extend. */
> + switch (bytes) {
> + case 1:
> + *(u8 *)op->addr.reg = (u8)val;
> + break;
> + case 2:
> + *(u16 *)op->addr.reg = (u16)val;
> + break;
> + case 4:
> + *op->addr.reg = (u32)val;
> + break; /* 64b: zero-extend */
> + case 8:
> + *op->addr.reg = val;
> + break;
> + }
> +}
It's cleaner to take val and bytes from struct operand, and do the
assignment from the callers, no?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] KVM: x86 emulator: put register operand write back to a function
[not found] ` <4C689B56.4000405@cn.fujitsu.com>
@ 2010-08-16 8:55 ` Avi Kivity
0 siblings, 0 replies; 7+ messages in thread
From: Avi Kivity @ 2010-08-16 8:55 UTC (permalink / raw)
To: Wei Yongjun; +Cc: kvm
On 08/16/2010 04:58 AM, Wei Yongjun wrote:
>
>> It's cleaner to take val and bytes from struct operand, and do the
>> assignment from the callers, no?
>>
> take val and bytes from struct operand may have other issue, when we
> writeback
> the source register, we need do the assignment from the caller, and then
> change
> the val back before write src val to dst val. Such as xadd:
> c->src.val = c->dst.val;
> write_register_operand(&c->src);
> c->src.val = c->src.orig_val;
> goto add;
Or avoid the 'goto add'. XADD is not ADD.
write_register_operand(struct operand *) is easy to understand. With the
two additional arguments it becomes confusing since it uses some parts
of the operand but ignores others.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-08-16 8:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-12 13:38 [PATCH 1/2] KVM: x86 emulator: put register operand write back to a function Wei Yongjun
2010-08-12 13:41 ` [PATCH 2/2] KVM: x86 emulator: add XADD instruction emulation Wei Yongjun
2010-08-12 15:34 ` Paolo Bonzini
2010-08-13 5:13 ` [PATCH 2/2 v2] " Wei Yongjun
2010-08-13 5:32 ` [PATCH 2/2 v3] " Wei Yongjun
2010-08-15 11:30 ` [PATCH 1/2] KVM: x86 emulator: put register operand write back to a function Avi Kivity
[not found] ` <4C689B56.4000405@cn.fujitsu.com>
2010-08-16 8:55 ` Avi Kivity
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.