* [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
[parent not found: <4C689B56.4000405@cn.fujitsu.com>]
* 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).