From: Avi Kivity <avi@redhat.com>
To: kvm@vger.kernel.org, Marcelo Tosatti <mtosatti@redhat.com>
Subject: [PATCH 02/15] KVM: x86 emulator: use correct type for memory address in operands
Date: Sun, 1 Aug 2010 17:23:20 +0300 [thread overview]
Message-ID: <1280672613-14026-3-git-send-email-avi@redhat.com> (raw)
In-Reply-To: <1280672613-14026-1-git-send-email-avi@redhat.com>
Currently we use a void pointer for memory addresses. That's wrong since
these are guest virtual addresses which are not directly dereferencable by
the host.
Use the correct type, unsigned long.
Signed-off-by: Avi Kivity <avi@redhat.com>
---
arch/x86/include/asm/kvm_emulate.h | 6 ++-
arch/x86/kvm/emulate.c | 117 +++++++++++++++++------------------
2 files changed, 62 insertions(+), 61 deletions(-)
diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
index ecb2653..bbf0e81 100644
--- a/arch/x86/include/asm/kvm_emulate.h
+++ b/arch/x86/include/asm/kvm_emulate.h
@@ -152,7 +152,11 @@ struct x86_emulate_ops {
struct operand {
enum { OP_REG, OP_MEM, OP_IMM, OP_NONE } type;
unsigned int bytes;
- unsigned long orig_val, *ptr;
+ unsigned long orig_val;
+ union {
+ unsigned long *reg;
+ unsigned long mem;
+ } addr;
union {
unsigned long val;
char valptr[sizeof(unsigned long) + 2];
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2ed6c67..61d728d 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -489,7 +489,7 @@ static void *decode_register(u8 modrm_reg, unsigned long *regs,
static int read_descriptor(struct x86_emulate_ctxt *ctxt,
struct x86_emulate_ops *ops,
- void *ptr,
+ ulong addr,
u16 *size, unsigned long *address, int op_bytes)
{
int rc;
@@ -497,12 +497,10 @@ static int read_descriptor(struct x86_emulate_ctxt *ctxt,
if (op_bytes == 2)
op_bytes = 3;
*address = 0;
- rc = ops->read_std((unsigned long)ptr, (unsigned long *)size, 2,
- ctxt->vcpu, NULL);
+ rc = ops->read_std(addr, (unsigned long *)size, 2, ctxt->vcpu, NULL);
if (rc != X86EMUL_CONTINUE)
return rc;
- rc = ops->read_std((unsigned long)ptr + 2, address, op_bytes,
- ctxt->vcpu, NULL);
+ rc = ops->read_std(addr + 2, address, op_bytes, ctxt->vcpu, NULL);
return rc;
}
@@ -552,21 +550,21 @@ static void decode_register_operand(struct operand *op,
reg = (c->b & 7) | ((c->rex_prefix & 1) << 3);
op->type = OP_REG;
if ((c->d & ByteOp) && !inhibit_bytereg) {
- op->ptr = decode_register(reg, c->regs, highbyte_regs);
- op->val = *(u8 *)op->ptr;
+ op->addr.reg = decode_register(reg, c->regs, highbyte_regs);
+ op->val = *(u8 *)op->addr.reg;
op->bytes = 1;
} else {
- op->ptr = decode_register(reg, c->regs, 0);
+ op->addr.reg = decode_register(reg, c->regs, 0);
op->bytes = c->op_bytes;
switch (op->bytes) {
case 2:
- op->val = *(u16 *)op->ptr;
+ op->val = *(u16 *)op->addr.reg;
break;
case 4:
- op->val = *(u32 *)op->ptr;
+ op->val = *(u32 *)op->addr.reg;
break;
case 8:
- op->val = *(u64 *) op->ptr;
+ op->val = *(u64 *) op->addr.reg;
break;
}
}
@@ -976,23 +974,23 @@ static inline int writeback(struct x86_emulate_ctxt *ctxt,
*/
switch (c->dst.bytes) {
case 1:
- *(u8 *)c->dst.ptr = (u8)c->dst.val;
+ *(u8 *)c->dst.addr.reg = (u8)c->dst.val;
break;
case 2:
- *(u16 *)c->dst.ptr = (u16)c->dst.val;
+ *(u16 *)c->dst.addr.reg = (u16)c->dst.val;
break;
case 4:
- *c->dst.ptr = (u32)c->dst.val;
+ *c->dst.addr.reg = (u32)c->dst.val;
break; /* 64b: zero-ext */
case 8:
- *c->dst.ptr = c->dst.val;
+ *c->dst.addr.reg = c->dst.val;
break;
}
break;
case OP_MEM:
if (c->lock_prefix)
rc = ops->cmpxchg_emulated(
- (unsigned long)c->dst.ptr,
+ c->dst.addr.mem,
&c->dst.orig_val,
&c->dst.val,
c->dst.bytes,
@@ -1000,14 +998,13 @@ static inline int writeback(struct x86_emulate_ctxt *ctxt,
ctxt->vcpu);
else
rc = ops->write_emulated(
- (unsigned long)c->dst.ptr,
+ c->dst.addr.mem,
&c->dst.val,
c->dst.bytes,
&err,
ctxt->vcpu);
if (rc == X86EMUL_PROPAGATE_FAULT)
- emulate_pf(ctxt,
- (unsigned long)c->dst.ptr, err);
+ emulate_pf(ctxt, c->dst.addr.mem, err);
if (rc != X86EMUL_CONTINUE)
return rc;
break;
@@ -1029,8 +1026,8 @@ static inline void emulate_push(struct x86_emulate_ctxt *ctxt,
c->dst.bytes = c->op_bytes;
c->dst.val = c->src.val;
register_address_increment(c, &c->regs[VCPU_REGS_RSP], -c->op_bytes);
- c->dst.ptr = (void *) register_address(c, ss_base(ctxt, ops),
- c->regs[VCPU_REGS_RSP]);
+ c->dst.addr.mem = register_address(c, ss_base(ctxt, ops),
+ c->regs[VCPU_REGS_RSP]);
}
static int emulate_pop(struct x86_emulate_ctxt *ctxt,
@@ -2014,7 +2011,7 @@ static void string_addr_inc(struct x86_emulate_ctxt *ctxt, unsigned long base,
int df = (ctxt->eflags & EFLG_DF) ? -1 : 1;
register_address_increment(c, &c->regs[reg], df * op->bytes);
- op->ptr = (unsigned long *)register_address(c, base, c->regs[reg]);
+ op->addr.mem = register_address(c, base, c->regs[reg]);
}
static int em_push(struct x86_emulate_ctxt *ctxt)
@@ -2451,17 +2448,17 @@ done_prefixes:
if ((c->d & ModRM) && c->modrm_mod == 3) {
c->src.type = OP_REG;
c->src.val = c->modrm_val;
- c->src.ptr = c->modrm_ptr;
+ c->src.addr.reg = c->modrm_ptr;
break;
}
c->src.type = OP_MEM;
- c->src.ptr = (unsigned long *)c->modrm_ea;
+ c->src.addr.mem = c->modrm_ea;
c->src.val = 0;
break;
case SrcImm:
case SrcImmU:
c->src.type = OP_IMM;
- c->src.ptr = (unsigned long *)c->eip;
+ c->src.addr.mem = c->eip;
c->src.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
if (c->src.bytes == 8)
c->src.bytes = 4;
@@ -2494,7 +2491,7 @@ done_prefixes:
case SrcImmByte:
case SrcImmUByte:
c->src.type = OP_IMM;
- c->src.ptr = (unsigned long *)c->eip;
+ c->src.addr.mem = c->eip;
c->src.bytes = 1;
if ((c->d & SrcMask) == SrcImmByte)
c->src.val = insn_fetch(s8, 1, c->eip);
@@ -2504,19 +2501,19 @@ done_prefixes:
case SrcAcc:
c->src.type = OP_REG;
c->src.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
- c->src.ptr = &c->regs[VCPU_REGS_RAX];
+ c->src.addr.reg = &c->regs[VCPU_REGS_RAX];
switch (c->src.bytes) {
case 1:
- c->src.val = *(u8 *)c->src.ptr;
+ c->src.val = *(u8 *)c->src.addr.reg;
break;
case 2:
- c->src.val = *(u16 *)c->src.ptr;
+ c->src.val = *(u16 *)c->src.addr.reg;
break;
case 4:
- c->src.val = *(u32 *)c->src.ptr;
+ c->src.val = *(u32 *)c->src.addr.reg;
break;
case 8:
- c->src.val = *(u64 *)c->src.ptr;
+ c->src.val = *(u64 *)c->src.addr.reg;
break;
}
break;
@@ -2527,20 +2524,20 @@ done_prefixes:
case SrcSI:
c->src.type = OP_MEM;
c->src.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
- c->src.ptr = (unsigned long *)
+ c->src.addr.mem =
register_address(c, seg_override_base(ctxt, ops, c),
c->regs[VCPU_REGS_RSI]);
c->src.val = 0;
break;
case SrcImmFAddr:
c->src.type = OP_IMM;
- c->src.ptr = (unsigned long *)c->eip;
+ c->src.addr.mem = c->eip;
c->src.bytes = c->op_bytes + 2;
insn_fetch_arr(c->src.valptr, c->src.bytes, c->eip);
break;
case SrcMemFAddr:
c->src.type = OP_MEM;
- c->src.ptr = (unsigned long *)c->modrm_ea;
+ c->src.addr.mem = c->modrm_ea;
c->src.bytes = c->op_bytes + 2;
break;
}
@@ -2558,7 +2555,7 @@ done_prefixes:
break;
case Src2ImmByte:
c->src2.type = OP_IMM;
- c->src2.ptr = (unsigned long *)c->eip;
+ c->src2.addr.mem = c->eip;
c->src2.bytes = 1;
c->src2.val = insn_fetch(u8, 1, c->eip);
break;
@@ -2583,11 +2580,11 @@ done_prefixes:
c->dst.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
c->dst.type = OP_REG;
c->dst.val = c->dst.orig_val = c->modrm_val;
- c->dst.ptr = c->modrm_ptr;
+ c->dst.addr.reg = c->modrm_ptr;
break;
}
c->dst.type = OP_MEM;
- c->dst.ptr = (unsigned long *)c->modrm_ea;
+ c->dst.addr.mem = c->modrm_ea;
if ((c->d & DstMask) == DstMem64)
c->dst.bytes = 8;
else
@@ -2596,26 +2593,26 @@ done_prefixes:
if (c->d & BitOp) {
unsigned long mask = ~(c->dst.bytes * 8 - 1);
- c->dst.ptr = (void *)c->dst.ptr +
+ c->dst.addr.mem = c->dst.addr.mem +
(c->src.val & mask) / 8;
}
break;
case DstAcc:
c->dst.type = OP_REG;
c->dst.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
- c->dst.ptr = &c->regs[VCPU_REGS_RAX];
+ c->dst.addr.reg = &c->regs[VCPU_REGS_RAX];
switch (c->dst.bytes) {
case 1:
- c->dst.val = *(u8 *)c->dst.ptr;
+ c->dst.val = *(u8 *)c->dst.addr.reg;
break;
case 2:
- c->dst.val = *(u16 *)c->dst.ptr;
+ c->dst.val = *(u16 *)c->dst.addr.reg;
break;
case 4:
- c->dst.val = *(u32 *)c->dst.ptr;
+ c->dst.val = *(u32 *)c->dst.addr.reg;
break;
case 8:
- c->dst.val = *(u64 *)c->dst.ptr;
+ c->dst.val = *(u64 *)c->dst.addr.reg;
break;
}
c->dst.orig_val = c->dst.val;
@@ -2623,7 +2620,7 @@ done_prefixes:
case DstDI:
c->dst.type = OP_MEM;
c->dst.bytes = (c->d & ByteOp) ? 1 : c->op_bytes;
- c->dst.ptr = (unsigned long *)
+ c->dst.addr.mem =
register_address(c, es_base(ctxt, ops),
c->regs[VCPU_REGS_RDI]);
c->dst.val = 0;
@@ -2691,7 +2688,7 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
}
if (c->src.type == OP_MEM) {
- rc = read_emulated(ctxt, ops, (unsigned long)c->src.ptr,
+ rc = read_emulated(ctxt, ops, c->src.addr.mem,
c->src.valptr, c->src.bytes);
if (rc != X86EMUL_CONTINUE)
goto done;
@@ -2699,7 +2696,7 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
}
if (c->src2.type == OP_MEM) {
- rc = read_emulated(ctxt, ops, (unsigned long)c->src2.ptr,
+ rc = read_emulated(ctxt, ops, c->src2.addr.mem,
&c->src2.val, c->src2.bytes);
if (rc != X86EMUL_CONTINUE)
goto done;
@@ -2711,7 +2708,7 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt)
if ((c->dst.type == OP_MEM) && !(c->d & Mov)) {
/* optimisation - avoid slow emulated read if Mov */
- rc = read_emulated(ctxt, ops, (unsigned long)c->dst.ptr,
+ rc = read_emulated(ctxt, ops, c->dst.addr.mem,
&c->dst.val, c->dst.bytes);
if (rc != X86EMUL_CONTINUE)
goto done;
@@ -2875,16 +2872,16 @@ special_insn:
/* Write back the register source. */
switch (c->dst.bytes) {
case 1:
- *(u8 *) c->src.ptr = (u8) c->dst.val;
+ *(u8 *) c->src.addr.reg = (u8) c->dst.val;
break;
case 2:
- *(u16 *) c->src.ptr = (u16) c->dst.val;
+ *(u16 *) c->src.addr.reg = (u16) c->dst.val;
break;
case 4:
- *c->src.ptr = (u32) c->dst.val;
+ *c->src.addr.reg = (u32) c->dst.val;
break; /* 64b reg: zero-extend */
case 8:
- *c->src.ptr = c->dst.val;
+ *c->src.addr.reg = c->dst.val;
break;
}
/*
@@ -2931,15 +2928,15 @@ special_insn:
goto done;
break;
case 0x90: /* nop / xchg r8,rax */
- if (c->dst.ptr == (unsigned long *)&c->regs[VCPU_REGS_RAX]) {
+ if (c->dst.addr.reg == &c->regs[VCPU_REGS_RAX]) {
c->dst.type = OP_NONE; /* nop */
break;
}
case 0x91 ... 0x97: /* xchg reg,rax */
c->src.type = OP_REG;
c->src.bytes = c->op_bytes;
- c->src.ptr = (unsigned long *) &c->regs[VCPU_REGS_RAX];
- c->src.val = *(c->src.ptr);
+ c->src.addr.reg = &c->regs[VCPU_REGS_RAX];
+ c->src.val = *(c->src.addr.reg);
goto xchg;
case 0x9c: /* pushf */
c->src.val = (unsigned long) ctxt->eflags;
@@ -2947,7 +2944,7 @@ special_insn:
break;
case 0x9d: /* popf */
c->dst.type = OP_REG;
- c->dst.ptr = (unsigned long *) &ctxt->eflags;
+ c->dst.addr.reg = &ctxt->eflags;
c->dst.bytes = c->op_bytes;
rc = emulate_popf(ctxt, ops, &c->dst.val, c->op_bytes);
if (rc != X86EMUL_CONTINUE)
@@ -2958,7 +2955,7 @@ special_insn:
goto mov;
case 0xa6 ... 0xa7: /* cmps */
c->dst.type = OP_NONE; /* Disable writeback. */
- DPRINTF("cmps: mem1=0x%p mem2=0x%p\n", c->src.ptr, c->dst.ptr);
+ DPRINTF("cmps: mem1=0x%p mem2=0x%p\n", c->src.addr.mem, c->dst.addr.mem);
goto cmp;
case 0xa8 ... 0xa9: /* test ax, imm */
goto test;
@@ -2977,7 +2974,7 @@ special_insn:
break;
case 0xc3: /* ret */
c->dst.type = OP_REG;
- c->dst.ptr = &c->eip;
+ c->dst.addr.reg = &c->eip;
c->dst.bytes = c->op_bytes;
goto pop_instruction;
case 0xc6 ... 0xc7: /* mov (sole member of Grp11) */
@@ -3179,7 +3176,7 @@ twobyte_insn:
c->dst.type = OP_NONE;
break;
case 2: /* lgdt */
- rc = read_descriptor(ctxt, ops, c->src.ptr,
+ rc = read_descriptor(ctxt, ops, c->src.addr.mem,
&size, &address, c->op_bytes);
if (rc != X86EMUL_CONTINUE)
goto done;
@@ -3199,7 +3196,7 @@ twobyte_insn:
goto cannot_emulate;
}
} else {
- rc = read_descriptor(ctxt, ops, c->src.ptr,
+ rc = read_descriptor(ctxt, ops, c->src.addr.mem,
&size, &address,
c->op_bytes);
if (rc != X86EMUL_CONTINUE)
@@ -3394,7 +3391,7 @@ twobyte_insn:
} else {
/* Failure: write the value we saw to EAX. */
c->dst.type = OP_REG;
- c->dst.ptr = (unsigned long *)&c->regs[VCPU_REGS_RAX];
+ c->dst.addr.reg = (unsigned long *)&c->regs[VCPU_REGS_RAX];
}
break;
case 0xb3:
--
1.7.1
next prev parent reply other threads:[~2010-08-01 14:23 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-01 14:23 [PATCH 00/15] More emulator cleanups Avi Kivity
2010-08-01 14:23 ` [PATCH 01/15] KVM: x86 emulator: push segment override out of decode_modrm() Avi Kivity
2010-08-01 14:23 ` Avi Kivity [this message]
2010-08-01 14:23 ` [PATCH 03/15] KVM: x86 emulator: simplify xchg decode tables Avi Kivity
2010-08-01 14:23 ` [PATCH 04/15] KVM: x86 emulator: use SrcAcc to simplify xchg decoding Avi Kivity
2010-08-01 14:23 ` [PATCH 05/15] KVM: x86 emulator: put register operand fetch into a function Avi Kivity
2010-08-01 14:23 ` [PATCH 06/15] KVM: x86 emulator: drop use_modrm_ea Avi Kivity
2010-08-01 14:23 ` [PATCH 07/15] KVM: x86 emulator: simplify REX.W check Avi Kivity
2010-08-01 14:23 ` [PATCH 08/15] KVM: x86 emulator: introduce Force64 for forcing operand size to 64 bits Avi Kivity
2010-08-01 14:23 ` [PATCH 09/15] KVM: x86 emulator: mark mov cr and mov dr as 64-bit instructions in long mode Avi Kivity
2010-08-02 7:37 ` Avi Kivity
2010-08-01 14:23 ` [PATCH 10/15] KVM: x86 emulator: use struct operand for mov reg,cr and mov cr,reg for reg op Avi Kivity
2010-08-01 14:23 ` [PATCH 11/15] KVM: x86 emulator: use struct operand for mov reg,dr and mov dr,reg " Avi Kivity
2010-08-01 14:23 ` [PATCH 12/15] KVM: x86 emulator: add NoAccess flag for memory instructions that skip access Avi Kivity
2010-08-01 14:23 ` [PATCH 13/15] KVM: x86 emulator: switch LEA to use SrcMem decoding Avi Kivity
2010-08-01 14:23 ` [PATCH 14/15] KVM: x86 emulator: change invlpg emulation to use src.mem.addr Avi Kivity
2010-08-01 14:23 ` [PATCH 15/15] KVM: x86 emulator: Decode memory operands directly into a 'struct operand' Avi Kivity
2010-08-02 7:28 ` Paolo Bonzini
2010-08-02 7:36 ` Avi Kivity
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1280672613-14026-3-git-send-email-avi@redhat.com \
--to=avi@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=mtosatti@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox