vmm: reimplement the x86 instruction decoder test origin/test
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 23 Oct 2019 22:56:05 +0000 (18:56 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 23 Oct 2019 23:06:29 +0000 (19:06 -0400)
Our old decoder had a bunch of issues.  Whenever we get a new version of
Linux, we tend to have new instructions that need decoding.  The old
decoder was hard to extend, and it was also hiding a bunch of bugs.

Here are some of the problems the old decoder had:
- It assumed every operation was a load or store.  Including cmp (which
does not change registers/memory) and add (which does change them, but
only after adding).
- It did not set rflags
- It did not zero-extend 32-bit wide register results
- *word was busted.  At one point, we did word++ when we meant to
advance by a byte.
- etc.

The code was pretty 'swirly' too, where you'd have similar processing
repeated all over the place, like the REX checks.

To fix that, I added a 'decode' struct, to pass along values that we
determined, such as address size, operand size, rex bits, etc.

Best of all, we weren't computing the size correctly, since we didn't
really do the modrm handling right.  Here's the case:

81 7d 00 5f 4d 50 5f    cmp    DWORD PTR [rbp+0x0],0x5f504d5f

That was being treated like it is only 4 bytes long, instead of 7.
Whoops!

However, it didn't crash, even though we set RIP to be part way (4
bytes) into the instruction!  Why?  well, those extra three bytes that
are just arbitrary numbers in the immediate32 part of the instruction
(which we end up running) decodes too!

0:  4d 50                   rex.WRB push r8
2:  5f                      pop    rdi

It pushes and pops, essentially clobbering rdi.  The Linux guest ends up
resetting rdi later, so no one noticed.

Had it been another value for the immed, we'd execute that too.  It
might blow up, and we'd notice.  But this one silently executed and
silently trashed a register.

To fix that, I needed better mod/rm+sib handling.  We still get away
with using GPA instead of decoding modrm+sib and translating through the
guest's page tables.  Ron's comment still applies.  =)

To handle the emulation of instructions, I had our callers pass us the
'access()' function.  So we can handle read-modify-write instructions,
like add.  Those didn't need to change too much, though I yanked out
destreg, which was just debug clutter.

I could have broken the commit up a little bit, but there wasn't a lot
of value in it, since the whole thing needed to be overhauled.

Note that the APIC_ACCESS and WRITE exits never happen.  That might have
been the case ever since we started using the x2APIC for the guest.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
user/vmm/apic.c
user/vmm/decode.c
user/vmm/include/vmm/vmm.h
user/vmm/ioapic.c
user/vmm/vmexit.c
user/vmm/vmx.c

index 78a9930..bc164c2 100644 (file)
@@ -168,8 +168,8 @@ static void apic_write(uint64_t offset, uint32_t value)
 
 }
 
-int __apic_access(struct guest_thread *vm_thread, uint64_t gpa, int destreg,
-                  uint64_t *regp, int store)
+int __apic_access(struct guest_thread *gth, uint64_t gpa, uint64_t *regp,
+                 size_t size, bool store)
 {
        uint32_t offset = gpa & 0xfffff;
        /* basic sanity tests. */
@@ -192,12 +192,8 @@ int __apic_access(struct guest_thread *vm_thread, uint64_t gpa, int destreg,
 
        if (store) {
                apic_write(offset, *regp);
-               DPRINTF("Write: mov %s to %s @%p val %p\n", regname(destreg),
-                       apicregs[offset].name, gpa, *regp);
        } else {
                *regp = apic_read(offset);
-               DPRINTF("Read: Set %s from %s @%p to %p\n", regname(destreg),
-                       apicregs[offset].name, gpa, *regp);
        }
        return 0;
 }
index d0a0dff..f61de37 100644 (file)
@@ -3,7 +3,7 @@
  *
  * This file is part of Akaros.
  *
- * Akarosn is free software: you can redistribute it and/or modify
+ * Akaros is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation, version 2 of the License.
  *
  *
  * See COPYING.LESSER for details on the GNU Lesser General Public License.
  * See COPYING for details on the GNU General Public License.
+ *
+ *
+ * A few notes:
+ * - We only emulate memory operations, so we don't need to worry about decoding
+ *   the target addresses or whatnot.  We could, just for sanity reasons, though
+ *   the registers (via modrm/sib) are in the guest virtual address space.  We
+ *   operate in the guest physical space.  Having the GPA from the fault makes
+ *   this all easier.
+ * - Just like with fetch_insn(), since we use the GPA, we assume that the
+ *   target of our memory access is also contiguous physically.  The guest could
+ *   have two virtual pages, one mapped to something that triggers an EPT fault
+ *   and the other doesn't.  The upper part of that access will go to the
+ *   adjacent physical page (e.g. a virtio region), and not to the actual
+ *   destination that the guest had mapped.  Buyer beware.  I'm less concerned
+ *   about this than I am with instructions.
+ * - To emulate instructions that set rflags, like add and cmp, I just execute
+ *   the instruction with inline asm and pop rflags.  Let the hardware do it.
+ * - The inline asm often uses %2,%1 and not %1,%2 for the args to e.g. cmp.
+ *   The good book is in Intel syntax.  Code AT&T.  It's easier to read the args
+ *   as if they are in the book, and just switch the ASM args like that.
+ * - add and cmp (80 /0) have a rex, and the Good Book says to add a
+ *   sign-extended imm8 to r/m8.  Extended to what?  I skipped that, and treated
+ *   it like a regular imm8.  The rex should apply for register selection still.
  */
 
 #include <parlib/stdio.h>
@@ -47,289 +70,726 @@ int debug_decode = 0;
        } \
        while (0)
 
-static char *modrmreg[] = {"rax", "rcx", "rdx", "rbx", "rsp", "rbp", "rsi", "rdi"};
-
-// Since we at most have to decode less than half of each instruction, I'm
-// trying to be dumb here.
-// Fortunately, for me, that's not hard.
-// I'm trying to avoid the whole Big Fun of full instruction decode, and in most
-// of these cases we only have to know register, address, operation size, and
-// instruction length.
-// The ugly messiness of the SIB and all that are not yet needed. Maybe they
-// never will be.
-
-// Target size -- 1, 2, 4, or 8 bytes. We have yet to see 64 bytes.
-// TODO: if we ever see it, test the prefix. Since this only supports the low
-// 1M, that's not likely.
-static int target(void *insn, int *store)
+enum x86_register {
+       X86_RAX = 0,
+       X86_RCX = 1,
+       X86_RDX = 2,
+       X86_RBX = 3,
+       X86_RSP = 4,
+       X86_RBP = 5,
+       X86_RSI = 6,
+       X86_RDI = 7,
+       X86_R8  = 8,
+       X86_R9  = 9,
+       X86_R10 = 10,
+       X86_R11 = 11,
+       X86_R12 = 12,
+       X86_R13 = 13,
+       X86_R14 = 14,
+       X86_R15 = 15,
+};
+
+static const char * const reg_names[] = {
+       "rax", "rcx", "rdx", "rbx", "rsp", "rbp", "rsi", "rdi",
+       "r8",  "r9",  "r10", "r11", "r12", "r13", "r14", "r15"
+};
+
+static const char *regname(uint8_t reg)
 {
-       *store = 0;
-       int s = -1;
-       uint8_t *byte = insn;
-       uint16_t *word = insn;
-
-       if (*byte == 0x66) {
-               s = target(insn+1,store);
-               // flip the sense of s.
-               s = s == 4 ? 2 : 4;
-               return s;
-       }
-       if (*byte == 0x44) {
-               byte++;
-               word++;
+       return reg_names[reg];
+}
+
+/* Helper: points to the reg in the VMTF */
+static uint64_t *reg_to_vmtf_reg(struct vm_trapframe *vm_tf, int reg)
+{
+       switch (reg) {
+       case 0:
+               return &vm_tf->tf_rax;
+       case 1:
+               return &vm_tf->tf_rcx;
+       case 2:
+               return &vm_tf->tf_rdx;
+       case 3:
+               return &vm_tf->tf_rbx;
+       case 4:
+               return &vm_tf->tf_rsp;
+       case 5:
+               return &vm_tf->tf_rbp;
+       case 6:
+               return &vm_tf->tf_rsi;
+       case 7:
+               return &vm_tf->tf_rdi;
+       case 8:
+               return &vm_tf->tf_r8;
+       case 9:
+               return &vm_tf->tf_r9;
+       case 10:
+               return &vm_tf->tf_r10;
+       case 11:
+               return &vm_tf->tf_r11;
+       case 12:
+               return &vm_tf->tf_r12;
+       case 13:
+               return &vm_tf->tf_r13;
+       case 14:
+               return &vm_tf->tf_r14;
+       case 15:
+               return &vm_tf->tf_r15;
        }
-       switch(*byte) {
-       case 0x3a:
-       case 0x8a:
-       case 0x88:
-               s = 1;
-               break;
-       case 0x89:
-       case 0x8b:
-               // TODO: To really know, for sure, that this is 32 bit, we'd
-               // likely have to check the segment descriptor for the guest's
-               // current code segment in it's GDT. The D flag (bit 22)
-               // determines whether the instruction is using 32 or 16-bit
-               // operand size. I'm just going to assume the flag is set
-               // (meaning 32 bit operands) for now, in order to make virtio
-               // work. But really we should check if we want to know for sure.
-               // Note that this hack (changing the below line) only applies to
-               // mov instructions.
-               //
-               //       And I think there's also a prefix you can use to switch
-               //       the instruction to 16-bit addressing (address-size
-               //       override prefix?)
-               s = 4;
+       panic("Unknown reg %d\n", reg);
+}
+
+struct x86_decode {
+       uint8_t prefix_sz;
+       uint8_t opcode_sz;
+       uint8_t modrm_sib_sz;
+       uint8_t imm_sz;
+       uint8_t operand_bytes;
+       uint8_t address_bytes;
+       bool has_modrm;
+       bool is_store;
+       bool rex_r;
+       bool rex_x;
+       bool rex_b;
+};
+
+/* We decode instructions in response to memory faults, so most every
+ * instruction will have modrm. */
+#define X86_DECODE_64_DEFAULT { \
+       .prefix_sz = 0, \
+       .opcode_sz = 1, \
+       .modrm_sib_sz = 0, \
+       .imm_sz = 0, \
+       .operand_bytes = 4, \
+       .address_bytes = 8, \
+       .has_modrm = true, \
+       .is_store = false, \
+       .rex_r = false, \
+       .rex_x = false, \
+       .rex_b = false, \
+}
+
+static void print_decoded_insn(uint8_t *insn, struct x86_decode *d);
+static void run_decode_hokey_tests(void);
+
+static int decode_prefix(uint8_t *insn, struct x86_decode *d)
+{
+       uint8_t *p = insn;
+
+       for (;; p++) {
+               /* Operand-size override prefix */
+               if (p[0] == 0x66) {
+                       /* Ignore 0x66 if REX.W changed us to 8 bytes (64).
+                        * Though we should only see 0x66 before REX.W.
+                        *
+                        * If this was handling 32 bit code but with cs.d clear
+                        * (default 16), 66 should set us to 4 bytes. */
+                       if (d->operand_bytes == 4)
+                               d->operand_bytes = 2;
+                       continue;
+               }
+               /* Address-size override prefix */
+               if (p[0] == 0x67) {
+                       d->address_bytes = 4;
+                       continue;
+               }
+               /* REX.* */
+               if ((p[0] & 0xf0) == 0x40) {
+                       if (p[0] & 0x08)
+                               d->operand_bytes = 8;
+                       if (p[0] & 0x04)
+                               d->rex_r = true;
+                       if (p[0] & 0x02)
+                               d->rex_x = true;
+                       if (p[0] & 0x01)
+                               d->rex_b = true;
+                       continue;
+               }
                break;
-       case 0x81:
-               s = 4;
+       }
+       d->prefix_sz = p - insn;
+       return 0;
+}
+
+static uint8_t *get_modrm(uint8_t *insn, struct x86_decode *d)
+{
+       if (!d->has_modrm)
+               return NULL;
+       return insn + d->prefix_sz + d->opcode_sz;
+}
+
+static uint8_t modrm_sib_bytes_16(int mod, int rm, struct x86_decode *d)
+{
+       uint8_t ret = 1;        /* counting the mod/rm byte itself */
+
+       switch (mod) {
+       case 0:
+               if (rm == 6)
+                       ret += 2; /* disp16 */
                break;
-       case 0x0f:
-               switch (*word) {
-                       case 0xb70f:
-                               s = 2;
-                               break;
-                       default:
-                               fprintf(stderr,
-                                       "can't get size of %02x/%04x @ %p\n",
-                                       *byte, *word, byte);
-                               return -1;
-               }
+       case 1:
+               ret += 1; /* disp8 */
+               if (rm == 4)
+                       ret += 1; /* SIB */
                break;
-       case 0x41:
-               /* VEX byte for modrm field */
-               switch (*word) {
-                       case 0x8a41:
-                               s = 1;
-                               break;
-                       default:
-                               fprintf(stderr, "unparsed vex instruction %02x/%04x @ %p\n",
-                                       *byte, *word, byte);
-                               return -1;
-               }
+       case 2:
+               ret += 2; /* disp16 */
                break;
-       default:
-               fprintf(stderr, "can't get size of %02x @ %p\n", *byte, byte);
-               fprintf(stderr, "can't get WORD of %04x @ %p\n", *word, word);
-               return -1;
+       case 3:
                break;
        }
+       return ret;
+}
 
-       switch(*byte) {
-       case 0x0f:
-       case 0x41:
+static uint8_t modrm_sib_bytes_32(int mod, int rm, struct x86_decode *d)
+{
+       uint8_t ret = 1;        /* counting the mod/rm byte itself */
+
+       switch (mod) {
+       case 0:
+               if (rm == 4)
+                       ret += 1; /* SIB */
+               else if (rm == 5)
+                       ret += 4; /* disp32 */
                break;
-       case 0x3a:
-       case 0x8a:
-       case 0x88:
-       case 0x89:
-       case 0x8b:
-       case 0x81:
-               *store = !(*byte & 2);
+       case 1:
+               ret += 1; /* disp8 */
+               if (rm == 4)
+                       ret += 1; /* SIB */
                break;
-       default:
-               fprintf(stderr, "%s: Can't happen. rip is: %p\n", __func__,
-                       byte);
+       case 2:
+               ret += 4; /* disp32 */
+               if (rm == 4) /* SIB */
+                       ret += 1;
+               break;
+       case 3:
                break;
        }
-       return s;
+       return ret;
 }
 
-char *regname(uint8_t reg)
+/* Returns the number of bytes in the instruction due to things encoded by
+ * mod/rm, such as displacements (disp32) or the SIB byte, including the mod/rm
+ * byte itself. */
+static uint8_t modrm_sib_bytes(uint8_t *insn, struct x86_decode *d)
 {
-       return modrmreg[reg];
+       uint8_t *modrm = get_modrm(insn, d);
+       int mod, rm;
+
+       if (!modrm)
+               return 0;
+       mod = *modrm >> 6;
+       rm = *modrm & 0x7;
+
+       switch (d->address_bytes) {
+       case 2:
+               /* We should never see this, but was easy enough to code */
+               fprintf(stderr, "decode: %s had %u address bytes!\n", __func__,
+                       d->address_bytes);
+               return modrm_sib_bytes_16(mod, rm, d);
+       case 4:
+       case 8:
+               return modrm_sib_bytes_32(mod, rm, d);
+       default:
+               panic("decode: %s had %u address bytes!\n", __func__,
+                       d->address_bytes);
+               return 0;
+       }
+}
+
+static uint8_t modrm_get_reg_16(uint8_t reg, struct x86_decode *d)
+{
+       return reg;
+}
+
+static uint8_t modrm_get_reg_32(uint8_t reg, struct x86_decode *d)
+{
+       return reg + (d->rex_r ? 8 : 0);
+}
+
+static uint64_t get_imm(uint8_t *insn, struct x86_decode *d)
+{
+       uint8_t *imm = insn + d->prefix_sz + d->opcode_sz + d->modrm_sib_sz;
+       uint64_t ret = 0;
+
+       switch (d->imm_sz) {
+       case 8:
+               ret |= (uint64_t)imm[7] << (8 * 7);
+               ret |= (uint64_t)imm[6] << (8 * 6);
+               ret |= (uint64_t)imm[5] << (8 * 5);
+               ret |= (uint64_t)imm[4] << (8 * 4);
+               /* fall-through */
+       case 4:
+               ret |= (uint64_t)imm[3] << (8 * 3);
+               ret |= (uint64_t)imm[2] << (8 * 2);
+               /* fall-through */
+       case 2:
+               ret |= (uint64_t)imm[1] << (8 * 1);
+               /* fall-through */
+       case 1:
+               ret |= (uint64_t)imm[0] << (8 * 0);
+               break;
+       default:
+               panic("Bad IMM size %u", d->imm_sz);
+       }
+       return ret;
 }
 
-static int insize(void *rip)
+/* This is the three-bit (or more with REX) register used by opcodes that have
+ * /r.  The first argument for opcodes is in the modrm part, e.g. [eax]+disp8.
+ * We don't need to parse that, since we know the faulting GPA. */
+static uint8_t modrm_get_reg(uint8_t *insn, struct x86_decode *d)
 {
-       uint8_t *rip_gpa = rip;
-       int advance = 3;
-       int extra = 0;
-       if (rip_gpa[0] == 0x44) {
-               extra = 1;
-               rip_gpa++;
+       uint8_t *modrm = get_modrm(insn, d);
+       uint8_t reg;
+
+       if (!modrm) {
+               fprintf(stderr, "%s called with no modrm!\n, insn: %p\n",
+                       __func__, insn);
+               hexdump(stderr, insn, 15);
+               panic("Continuing could corrupt registers");
        }
+       reg = (*modrm >> 3) & 7;
 
-       /* return 3 to handle this specific instruction case. We don't want this
-        * to turn into a fully fledged decode.
-        * This specific instruction is an extended move using r9. It uses the
-        * VEX byte to extend the register bits. */
-       if (rip_gpa[0] == 0x41 && rip_gpa[1] == 0x8a && rip_gpa[2] == 0x01)
-               return 3;
-       /* the dreaded mod/rm byte. */
-       int mod = rip_gpa[1] >> 6;
-       int rm = rip_gpa[1] & 7;
-
-       switch (rip_gpa[0]) {
+       switch (d->address_bytes) {
+       case 2:
+               fprintf(stderr, "decode: %s had %u address bytes!\n", __func__,
+                       d->address_bytes);
+               return modrm_get_reg_16(reg, d);
+       case 4:
+       case 8:
+               return modrm_get_reg_32(reg, d);
        default:
-               fprintf(stderr, "BUG! %s got 0x%x\n", __func__, rip_gpa[0]);
-       case 0x0f:
+               panic("decode: %s had %u address bytes!\n", __func__,
+                       d->address_bytes);
+       }
+}
+
+/* Decodes the actual opcode, storing things we care about in d.
+ * -1 on error (for opcodes we haven't coded up), 0 success.
+ *
+ * Sets operand_bytes, various sizes, is_store, etc. */
+static int decode_opcode(uint8_t *insn, struct x86_decode *d)
+{
+       uint8_t *opcode = insn + d->prefix_sz;
+
+       /* If we don't set anything, we're using the defaults:  1 byte opcode,
+        * has_modrm, operand_bytes determined by the prefix/64-bit mode, etc */
+       switch (opcode[0]) {
+       case 0x80:
+               switch (modrm_get_reg(insn, d)) {
+               case 0: // add
+               case 7: // cmp
+                       break;
+               default:
+                       goto unknown;
+               }
+               d->imm_sz = 1;
+               d->operand_bytes = 1;
                break;
        case 0x81:
-               advance = 6 + extra;
-               break;
-       case 0x3a:
-       case 0x8a:
-       case 0x88:
-       case 0x89:
-       case 0x8b:
-               switch (mod) {
-               case 0:
-                       advance = 2 + (rm == 4) + extra;
-                       break;
-               case 1:
-                       advance = 3 + (rm == 4) + extra;
+               switch (modrm_get_reg(insn, d)) {
+               case 0: // add
+               case 7: // cmp
                        break;
-               case 2:
-                       advance = 6 + (rm == 4) + extra;
+               default:
+                       goto unknown;
+               }
+               d->imm_sz = d->address_bytes == 2 ? 2 : 4;
+               break;
+       case 0x3a:      // cmp /r
+               d->operand_bytes = 1;
+               break;
+       case 0x88:      // mov
+       case 0x8a:      // mov
+               d->operand_bytes = 1;
+               /* Instructions that could be loads or stores differ in bit 2.
+                * e.g.  88 (store, bit 2 unset) vs 8a (load, bit 2 set). */
+               d->is_store = !(opcode[0] & 2);
+               break;
+       case 0x89:      // mov
+       case 0x8b:      // mov
+               d->is_store = !(opcode[0] & 2);
+               break;
+       case 0x0f:
+               d->opcode_sz = 2;
+               switch (opcode[1]) {
+               case 0xb7:      // movzw
+                       d->operand_bytes = 2;
                        break;
-               case 3:
-                       advance = 2 + extra;
+               case 0xb6:      // movzb
+                       d->operand_bytes = 1;
                        break;
+               default:
+                       goto unknown;
                }
                break;
+       default:
+       unknown:
+               fprintf(stderr, "unknown decode %02x %02x %02x@ %p\n",
+                       opcode[0], opcode[1], opcode[2], opcode);
+               return -1;
        }
-       return advance;
+
+       d->modrm_sib_sz = modrm_sib_bytes(insn, d);
+       return 0;
 }
 
-// This is a very limited function. It's only here to manage virtio-mmio and low
-// memory pointer loads. I am hoping it won't grow with time. The intent is that
-// we enter it with and EPT fault from a region that is deliberately left
-// unbacked by any memory.
-// We return enough info to let you emulate the operation if you want. Because
-// we have the failing physical address (gpa) the decode is far simpler because
-// we only need to find the register, how many bytes to move, and how big the
-// instruction is. I thought about bringing in emulate.c from kvm from xen,
-// but it has way more stuff than we need.
-// gpa is a pointer to the gpa.
-// int is the reg index which we can use for printing info.
-// regp points to the register in hw_trapframe from which
-// to load or store a result.
-int decode(struct guest_thread *vm_thread, uint64_t *gpa, uint8_t *destreg,
-           uint64_t **regp, int *store, int *size, int *advance)
+static void set_rflags_status_bits(uint64_t *rfl, uint64_t new)
 {
-       struct vm_trapframe *vm_tf = &(vm_thread->uthread.u_ctx.tf.vm_tf);
-       uint8_t *rip_gpa = NULL;
-
-       DPRINTF("v is %p\n", vm_tf);
-
-       // Duh, which way did he go George? Which way did he go?
-       // First hit on Google gets you there!
-       // This is the guest physical address of the access.
-       // This is nice, because if we ever go with more complete
-       // instruction decode, knowing this gpa reduces our work:
-       // we don't have to find the source address in registers,
-       // only the register holding or receiving the value.
-       *gpa = vm_tf->tf_guest_pa;
-       DPRINTF("gpa is %p\n", *gpa);
-
-       DPRINTF("rip is %p\n", vm_tf->tf_rip);
-
-       if (rippa(vm_thread, (uint64_t *)&rip_gpa))
-               return VM_PAGE_FAULT;
-       DPRINTF("rip_gpa is %p\n", rip_gpa);
-
-       // fail fast. If we can't get the size we're done.
-       *size = target(rip_gpa, store);
-       DPRINTF("store is %d\n", *store);
-       if (*size < 0)
-               return -1;
-
-       *advance = insize(rip_gpa);
-
-       uint16_t ins = *(uint16_t *)(rip_gpa +
-           ((rip_gpa[0] == 0x44) || (rip_gpa[0] == 0x0f) || (rip_gpa[0] ==
-                                                             0x41)));
+       *rfl &= ~FL_STATUS;
+       new &= FL_STATUS;
+       *rfl |= new;
+}
 
-       DPRINTF("ins is %04x\n", ins);
+static int add_8081(struct guest_thread *gth, uint8_t *insn,
+                   struct x86_decode *d, emu_mem_access access,
+                   uint64_t gpa)
+{
+       struct vm_trapframe *vm_tf = gth_to_vmtf(gth);
+       int ret;
+       uint64_t scratch = 0;
+       int8_t imm8, scr8;
+       int16_t imm16, scr16;
+       int32_t imm32, scr32;
+       int64_t imm64, scr64;
+       unsigned long rflags;
 
-       *destreg = (ins>>11) & 7;
-       *destreg += 8 * (rip_gpa[0] == 0x44);
-       // Our primitive approach wins big here.
-       // We don't have to decode the register or the offset used
-       // in the computation; that was done by the CPU and is the gpa.
-       // All we need to know is which destination or source register it is.
-       switch (*destreg) {
-       case 0:
-               *regp = &vm_tf->tf_rax;
-               break;
+       ret = access(gth, gpa, &scratch, d->operand_bytes, false);
+       if (ret < 0)
+               return ret;
+       switch (d->operand_bytes) {
        case 1:
-               *regp = &vm_tf->tf_rcx;
+               imm8 = get_imm(insn, d);
+               /* scratch is input and output, but you can't cast it in an
+                * output operand */
+               scr8 = scratch;
+               asm volatile ("addb %2,%1; pushfq; popq %0"
+                             : "=r"(rflags),
+                               "+r"(scr8)
+                             : "r"(imm8)
+                             : "cc");
+               scratch = scr8;
                break;
        case 2:
-               *regp = &vm_tf->tf_rdx;
-               break;
-       case 3:
-               *regp = &vm_tf->tf_rbx;
+               imm16 = get_imm(insn, d);
+               scr16 = scratch;
+               asm volatile ("addw %2,%1; pushfq; popq %0"
+                             : "=r"(rflags),
+                               "+r"(scr16)
+                             : "r"(imm16)
+                             : "cc");
+               scratch = scr16;
                break;
        case 4:
-               *regp = &vm_tf->tf_rsp; // uh, right.
-               break;
-       case 5:
-               *regp = &vm_tf->tf_rbp;
-               break;
-       case 6:
-               *regp = &vm_tf->tf_rsi;
-               break;
-       case 7:
-               *regp = &vm_tf->tf_rdi;
+               imm32 = get_imm(insn, d);
+               scr32 = scratch;
+               asm volatile ("addl %2,%1; pushfq; popq %0"
+                             : "=r"(rflags),
+                               "+r"(scr32)
+                             : "r"(imm32)
+                             : "cc");
+               scratch = scr32;
                break;
        case 8:
-               *regp = &vm_tf->tf_r8;
-               break;
-       case 9:
-               *regp = &vm_tf->tf_r9;
-               break;
-       case 10:
-               *regp = &vm_tf->tf_r10;
-               break;
-       case 11:
-               *regp = &vm_tf->tf_r11;
+               imm32 = get_imm(insn, d);
+               scr64 = scratch;
+               asm volatile ("addq %2,%1; pushfq; popq %0"
+                             : "=r"(rflags),
+                               "+r"(scr64)
+                             : "r"((int64_t)imm32)
+                             : "cc");
+               scratch = scr64;
                break;
-       case 12:
-               *regp = &vm_tf->tf_r12;
+       }
+       set_rflags_status_bits(&vm_tf->tf_rflags, rflags);
+       return access(gth, gpa, &scratch, d->operand_bytes, true);
+}
+
+static int cmp_8081(struct guest_thread *gth, uint8_t *insn,
+                   struct x86_decode *d, emu_mem_access access,
+                   uint64_t gpa)
+{
+       struct vm_trapframe *vm_tf = gth_to_vmtf(gth);
+       int ret;
+       uint64_t scratch = 0;
+       int8_t imm8;
+       int16_t imm16;
+       int32_t imm32;
+       int64_t imm64;
+       unsigned long rflags;
+
+       ret = access(gth, gpa, &scratch, d->operand_bytes, false);
+       if (ret < 0)
+               return ret;
+       switch (d->operand_bytes) {
+       case 1:
+               imm8 = get_imm(insn, d);
+               asm volatile ("cmpb %2,%1; pushfq; popq %0"
+                             : "=r"(rflags)
+                             : "r"((int8_t)scratch),
+                               "r"(imm8)
+                             : "cc");
                break;
-       case 13:
-               *regp = &vm_tf->tf_r13;
+       case 2:
+               imm16 = get_imm(insn, d);
+               asm volatile ("cmpw %2,%1; pushfq; popq %0"
+                             : "=r"(rflags)
+                             : "r"((int16_t)scratch),
+                               "r"(imm16)
+                             : "cc");
                break;
-       case 14:
-               *regp = &vm_tf->tf_r14;
+       case 4:
+               imm32 = get_imm(insn, d);
+               asm volatile ("cmpl %2,%1; pushfq; popq %0"
+                             : "=r"(rflags)
+                             : "r"((int32_t)scratch),
+                               "r"(imm32)
+                             : "cc");
                break;
-       case 15:
-               *regp = &vm_tf->tf_r15;
+       case 8:
+               imm32 = get_imm(insn, d);
+               asm volatile ("cmpq %2,%1; pushfq; popq %0"
+                             : "=r"(rflags)
+                             : "r"((int64_t)scratch),
+                               "r"((int64_t)imm32)
+                             : "cc");
                break;
        }
-       /* Handle movz{b,w}X.  Zero the destination. */
-       if ((rip_gpa[0] == 0x0f) && (rip_gpa[1] == 0xb6)) {
-               /* movzb.
-                * TODO: figure out if the destination size is 16 or 32 bits.
-                * Linux doesn't call this yet, so it's not urgent. */
+       set_rflags_status_bits(&vm_tf->tf_rflags, rflags);
+       return 0;
+}
+
+static int cmp_3a(struct guest_thread *gth, uint8_t *insn,
+                 struct x86_decode *d, emu_mem_access access,
+                 uint64_t gpa)
+{
+       struct vm_trapframe *vm_tf = gth_to_vmtf(gth);
+       int ret;
+       uint64_t scratch = 0;
+       int8_t reg8;
+       unsigned long rflags;
+       int mod_reg = modrm_get_reg(insn, d);
+
+       assert(d->operand_bytes == 1);
+       ret = access(gth, gpa, &scratch, 1, false);
+       if (ret < 0)
+               return ret;
+       reg8 = (int8_t)*reg_to_vmtf_reg(vm_tf, mod_reg);
+       asm volatile ("cmpb %2,%1; pushfq; popq %0"
+                     : "=r"(rflags)
+                     : "r"(reg8),
+                       "r"((int8_t)scratch)
+                     : "cc");
+       set_rflags_status_bits(&vm_tf->tf_rflags, rflags);
+       return 0;
+}
+
+static int execute_op(struct guest_thread *gth, uint8_t *insn,
+                     struct x86_decode *d, emu_mem_access access,
+                     uint64_t gpa)
+{
+       struct vm_trapframe *vm_tf = gth_to_vmtf(gth);
+       uint8_t *opcode = insn + d->prefix_sz;
+       int ret, mod_reg;
+
+       switch (opcode[0]) {
+       case 0x80:
+       case 0x81:
+               switch (modrm_get_reg(insn, d)) {
+               case 0: // add
+                       return add_8081(gth, insn, d, access, gpa);
+               case 7: // cmp
+                       return cmp_8081(gth, insn, d, access, gpa);
+               }
+               goto unknown;
+       case 0x3a:      // cmp
+               return cmp_3a(gth, insn, d, access, gpa);
+       case 0x89:      // mov
+       case 0x8b:      // mov
+       case 0x8a:      // mov
+       case 0x88:      // mov
+               mod_reg = modrm_get_reg(insn, d);
+               ret = access(gth, gpa, reg_to_vmtf_reg(vm_tf, mod_reg),
+                            d->operand_bytes, d->is_store);
+               /* We set a register's value.  For 32-bit operands, we need to
+                * zero the upper 32 bits. */
+               if (!ret && !d->is_store && d->operand_bytes == 4)
+                       *reg_to_vmtf_reg(vm_tf, mod_reg) &= 0xffffffff;
+               return ret;
+       case 0x0f:
+               switch (opcode[1]) {
+               case 0xb7:      // movzw
+               case 0xb6:      // movzb
+                       mod_reg = modrm_get_reg(insn, d);
+                       *reg_to_vmtf_reg(vm_tf, mod_reg) = 0;
+                       return access(gth, gpa, reg_to_vmtf_reg(vm_tf, mod_reg),
+                                     d->operand_bytes, d->is_store);
+               }
+               goto unknown;
+       default:
+       unknown:
+               fprintf(stderr, "unknown execute %02x %02x %02x@ %p\n",
+                       opcode[0], opcode[1], opcode[2], opcode);
+               return -1;
+       }
+}
+
+static int decode_inst_size(uint8_t *insn, struct x86_decode *d)
+{
+       return d->prefix_sz + d->opcode_sz + d->modrm_sib_sz +
+              + d->imm_sz;
+}
+
+/* Emulates a memory operation that faulted/vmexited.  Despite the file name,
+ * this is x86-specific, so we only have at most one address involved.  We have
+ * at least one address involved, since it is a memory operation.
+ *
+ * The main thing our caller provides is a function pointer for accessing
+ * memory.  The address is gpa, the register (which doesn't have to be a real
+ * register in a VMTF) involved for the source/destination (based on 'store').
+ */
+int emulate_mem_insn(struct guest_thread *gth, uint8_t *insn,
+                    emu_mem_access access, int *advance)
+{
+       struct x86_decode d[1] = {X86_DECODE_64_DEFAULT};
+       uintptr_t gpa;
+
+       // Duh, which way did he go George? Which way did he go?
+       // First hit on Google gets you there!
+       // This is the guest physical address of the access.
+       // This is nice, because if we ever go with more complete
+       // instruction decode, knowing this gpa reduces our work:
+       // we don't have to find the source address in registers,
+       // only the register holding or receiving the value.
+       gpa = gth_to_vmtf(gth)->tf_guest_pa;
+       if (decode_prefix(insn, d) < 0)
                return -1;
+       if (decode_opcode(insn, d) < 0)
+               return -1;
+       if (execute_op(gth, insn, d, access, gpa) < 0)
+               return -1;
+       *advance = decode_inst_size(insn, d);
+
+       if (debug_decode) {
+               /* This is racy - multiple decoded threads on different cores
+                * will clutter the output. */
+               fprintf(stderr, "gpa %p", gpa);
+               print_decoded_insn(insn, d);
        }
-       if ((rip_gpa[0] == 0x0f) && (rip_gpa[1] == 0xb7)) {
-               /* movzwl.  Destination is 32 bits, unless we had the rex prefix
-                * */
-               **regp &= ~((1ULL << 32) - 1);
+       return 0;
+}
+
+/* Debugging */
+
+static void print_decoded_insn(uint8_t *insn, struct x86_decode *d)
+{
+       uint8_t inst_sz = decode_inst_size(insn, d);
+
+       fprintf(stderr,
+               "oprnd_bs %d, addr_bs %d, reg %2d, imm 0x%08llx, inst_size %2d:",
+               d->operand_bytes, d->address_bytes,
+               get_modrm(insn, d) ? modrm_get_reg(insn, d) : -1,
+               d->imm_sz ? get_imm(insn, d) : 0xdeadbeef, inst_sz);
+       for (int i = 0; i < inst_sz; i++)
+               fprintf(stderr, " %02x", insn[i]);
+       fprintf(stderr, "\n");
+}
+
+static int tst_mem_access(struct guest_thread *gth, uintptr_t gpa,
+                         unsigned long *regp, size_t size, bool store)
+{
+       if (store) {
+               switch (size) {
+               case 1:
+                       *(uint8_t*)gpa = *(uint8_t*)regp;
+                       break;
+               case 2:
+                       *(uint16_t*)gpa = *(uint16_t*)regp;
+                       break;
+               case 4:
+                       *(uint32_t*)gpa = *(uint32_t*)regp;
+                       break;
+               case 8:
+                       *(uint64_t*)gpa = *(uint64_t*)regp;
+                       break;
+               }
+       } else {
+               switch (size) {
+               case 1:
+                       *(uint8_t*)regp = *(uint8_t*)gpa;
+                       break;
+               case 2:
+                       *(uint16_t*)regp = *(uint16_t*)gpa;
+                       break;
+               case 4:
+                       *(uint32_t*)regp = *(uint32_t*)gpa;
+                       break;
+               case 8:
+                       *(uint64_t*)regp = *(uint64_t*)gpa;
+                       break;
+               }
        }
        return 0;
 }
+
+/* Far from exhaustive, and you need to call it manually.  It actually caught a
+ * bug though. */
+static void run_decode_hokey_tests(void)
+{
+       struct guest_thread gth[1] = {0};
+       struct vm_trapframe *vm_tf = gth_to_vmtf(gth);
+       uint8_t insn[VMM_MAX_INSN_SZ];
+       uint8_t ram[16];
+       int inst_size, ret;
+
+       vm_tf->tf_guest_pa = (uint64_t)ram;
+
+       // movw [rax],ecx (rax ignored, we use the GPA)
+       memcpy(insn, "\x89\x08", 2);
+       memset(ram, 0, sizeof(ram));
+       vm_tf->tf_rcx = 0x1234;
+       ret = emulate_mem_insn(gth, insn, tst_mem_access, &inst_size);
+       assert(!ret);
+       assert(inst_size = 2);
+       assert(*(uint16_t*)ram == 0x1234);
+
+       // add8 /0 [rbx], -1
+       memcpy(insn, "\x80\x03\xff", 3);
+       memset(ram, 0, sizeof(ram));
+       ram[0] = 1;
+       ret = emulate_mem_insn(gth, insn, tst_mem_access, &inst_size);
+       assert(!ret);
+       assert(inst_size == 3);
+       assert(ram[0] == 0);
+       assert(vm_tf->tf_rflags & FL_ZF);
+       assert(!(vm_tf->tf_rflags & FL_SF));
+
+       // REX.W add /0, r/m64/imm32: 84: SIB+disp32 /0. (-1 sign extend)
+       memcpy(insn, "\x48\x81\x84\x00\x00\x00\x00\x00\xff\xff\xff\xff", 12);
+       memset(ram, 0, sizeof(ram));
+       ram[0] = 2;
+       ret = emulate_mem_insn(gth, insn, tst_mem_access, &inst_size);
+       assert(!ret);
+       assert(inst_size == 12);
+       assert(*(uint64_t*)ram == 1);
+       assert(!(vm_tf->tf_rflags & FL_ZF));
+       assert(!(vm_tf->tf_rflags & FL_SF));
+
+       // REX.R movzw, 14: SIB, reg rdx -> r10
+       memcpy(insn, "\x44\x0f\xb7\x14\x00", 5);
+       memset(ram, 0, sizeof(ram));
+       ram[0] = 0x12;
+       vm_tf->tf_r10 = 0xffffffffffffffff;
+       ret = emulate_mem_insn(gth, insn, tst_mem_access, &inst_size);
+       assert(!ret);
+       assert(inst_size == 5);
+       assert(vm_tf->tf_r10 == 0x12);
+
+       fprintf(stderr, "Hokey decode tests passed\n");
+       return;
+
+       /* Helpful debuggers for the debugger */
+       fprintf(stderr, "ram %x %x %x %x %x %x %x %x\n",
+               ram[0], ram[1], ram[2], ram[3], ram[4], ram[5], ram[6], ram[7]);
+}
index 5a8d91f..447a8f3 100644 (file)
@@ -87,20 +87,25 @@ struct elf_aux {
        unsigned long v[2];
 };
 
-char *regname(uint8_t reg);
-int decode(struct guest_thread *vm_thread, uint64_t *gpa, uint8_t *destreg,
-           uint64_t **regp, int *store, int *size, int *advance);
+/* TODO: x86-specific */
+#define VMM_MAX_INSN_SZ 15
+typedef int (*emu_mem_access)(struct guest_thread *gth, uintptr_t gpa,
+                             unsigned long *regp, size_t size,
+                             bool store);
+int emulate_mem_insn(struct guest_thread *gth, uint8_t *insn,
+                    emu_mem_access access, int *advance);
 int io(struct guest_thread *vm_thread);
 void showstatus(FILE *f, struct guest_thread *vm_thread);
 int gva2gpa(struct guest_thread *vm_thread, uint64_t va, uint64_t *pa);
 int rippa(struct guest_thread *vm_thread, uint64_t *pa);
+int fetch_insn(struct guest_thread *gth, uint8_t *insn);
 int msrio(struct guest_thread *vm_thread, struct vmm_gpcore_init *gpci,
           uint32_t opcode);
-int do_ioapic(struct guest_thread *vm_thread, uint64_t gpa,
-              int destreg, uint64_t *regp, int store);
+int do_ioapic(struct guest_thread *vm_thread, uint64_t gpa, uint64_t *regp,
+             bool store);
 bool handle_vmexit(struct guest_thread *gth);
-int __apic_access(struct guest_thread *vm_thread, uint64_t gpa, int destreg,
-                  uint64_t *regp, int store);
+int __apic_access(struct guest_thread *gth, uint64_t gpa, uint64_t *regp,
+                 size_t size, bool store);
 int vmm_interrupt_guest(struct virtual_machine *vm, unsigned int gpcoreid,
                         unsigned int vector);
 uintptr_t load_elf(char *filename, uint64_t offset, uint64_t *highest,
index 9111d0c..0b35f63 100644 (file)
@@ -141,14 +141,14 @@ static void ioapic_write(struct guest_thread *vm_thread, int ix,
 
 }
 
-int do_ioapic(struct guest_thread *vm_thread, uint64_t gpa, int destreg,
-              uint64_t *regp, int store)
+int do_ioapic(struct guest_thread *vm_thread, uint64_t gpa,
+             uint64_t *regp, bool store)
 {
        /* TODO(ganshun): compute an index for the ioapic array. */
        int ix = 0;
        uint32_t offset = gpa & 0xfffff;
        /* basic sanity tests. */
-       DPRINTF("%s: %p 0x%x %p %s\n", __func__, (void *)gpa, destreg, regp,
+       DPRINTF("%s: %p %p %s\n", __func__, (void *)gpa, regp,
                store ? "write" : "read");
 
        if ((offset != 0) && (offset != 0x10)) {
index 1153895..ca83fc9 100644 (file)
@@ -122,40 +122,17 @@ static bool handle_cpuid(struct guest_thread *gth)
        return true;
 }
 
-static bool handle_ept_fault(struct guest_thread *gth)
+static int ept_mem_access(struct guest_thread *gth, uintptr_t gpa,
+                         unsigned long *regp, size_t size, bool store)
 {
        struct vm_trapframe *vm_tf = gth_to_vmtf(gth);
        struct virtual_machine *vm = gth_to_vm(gth);
-       uint64_t gpa, *regp;
-       uint8_t regx;
-       int store, size;
-       int advance;
-       int ret;
 
-       if (vm_tf->tf_flags & VMCTX_FL_EPT_VMR_BACKED) {
-               ret = ros_syscall(SYS_populate_va, vm_tf->tf_guest_pa, 1, 0, 0,
-                                 0, 0);
-               if (ret <= 0)
-                       panic("[user] handle_ept_fault: populate_va failed: ret = %d\n",
-                             ret);
-               return TRUE;
-       }
-       ret = decode(gth, &gpa, &regx, &regp, &store, &size, &advance);
-
-       if (ret < 0)
-               return FALSE;
-       if (ret == VM_PAGE_FAULT) {
-               /* We were unable to translate RIP due to an ept fault */
-               vm_tf->tf_trap_inject = VM_TRAP_VALID
-                                     | VM_TRAP_ERROR_CODE
-                                     | VM_TRAP_HARDWARE
-                                     | HW_TRAP_PAGE_FAULT;
-               return TRUE;
-       }
-
-       assert(size >= 0);
        /* TODO use helpers for some of these addr checks.  the fee/fec ones
-        * might be wrong too. */
+        * might be wrong too.
+        *
+        * Also consider adding checks to make sure the entire access (gpa +
+        * size) is within the page / segment / region. */
        for (int i = 0; i < VIRTIO_MMIO_MAX_NUM_DEV; i++) {
                if (vm->virtio_mmio_devices[i] == NULL)
                        continue;
@@ -163,17 +140,19 @@ static bool handle_ept_fault(struct guest_thread *gth)
                        continue;
                /* TODO: can the guest cause us to spawn off infinite threads?
                 */
+               /* TODO: regp often gets cast in virtio_mmio_wr, but not always.
+                * We probably don't need this assert or the u32* cast below. */
+               assert(size <= 4);
                if (store)
                        virtio_mmio_wr(vm, vm->virtio_mmio_devices[i], gpa,
                                       size, (uint32_t *)regp);
                else
                        *regp = virtio_mmio_rd(vm, vm->virtio_mmio_devices[i],
                                               gpa, size);
-               vm_tf->tf_rip += advance;
-               return TRUE;
+               return 0;
        }
        if (PG_ADDR(gpa) == 0xfec00000) {
-               do_ioapic(gth, gpa, regx, regp, store);
+               do_ioapic(gth, gpa, regp, store);
        } else if (PG_ADDR(gpa) == 0) {
                memmove(regp, &vm->low4k[gpa], size);
        } else {
@@ -184,10 +163,51 @@ static bool handle_ept_fault(struct guest_thread *gth)
                showstatus(stderr, gth);
                /* Just fill the whole register for now. */
                *regp = (uint64_t) -1;
-               return FALSE;
+               return -1;
        }
+       return 0;
+}
+
+
+static bool handle_ept_fault(struct guest_thread *gth)
+{
+       struct vm_trapframe *vm_tf = gth_to_vmtf(gth);
+       struct virtual_machine *vm = gth_to_vm(gth);
+       uint64_t gpa, *regp;
+       uint8_t regx;
+       bool store;
+       int size;
+       int advance;
+       int ret;
+       uint8_t insn[VMM_MAX_INSN_SZ];
+
+       if (vm_tf->tf_flags & VMCTX_FL_EPT_VMR_BACKED) {
+               ret = ros_syscall(SYS_populate_va, vm_tf->tf_guest_pa, 1, 0, 0,
+                                 0, 0);
+               if (ret <= 0)
+                       panic("[user] handle_ept_fault: populate_va failed: ret = %d\n",
+                             ret);
+               return TRUE;
+       }
+       /* TODO: consider pushing the PF stuff into fetch, since rippa has a
+        * bunch of callers.  Though we'll still need to know to return true to
+        * restart the guest, instead of killing them with e.g. a failed emu. */
+       if (fetch_insn(gth, insn)) {
+               /* We were unable to translate RIP due to an ept fault */
+               vm_tf->tf_trap_inject = VM_TRAP_VALID
+                                     | VM_TRAP_ERROR_CODE
+                                     | VM_TRAP_HARDWARE
+                                     | HW_TRAP_PAGE_FAULT;
+               return true;
+       }
+       ret = emulate_mem_insn(gth, insn, ept_mem_access, &advance);
+       if (ret < 0) {
+               fprintf(stderr, "emulate failed!\n");
+               return false;
+       }
+
        vm_tf->tf_rip += advance;
-       return TRUE;
+       return true;
 }
 
 static bool handle_vmcall_printc(struct guest_thread *gth)
@@ -353,13 +373,20 @@ static bool handle_apic_access(struct guest_thread *gth)
 {
        uint64_t gpa, *regp;
        uint8_t regx;
-       int store, size;
+       bool store;
+       int size;
        int advance;
+       uint8_t insn[VMM_MAX_INSN_SZ];
        struct vm_trapframe *vm_tf = gth_to_vmtf(gth);
 
-       if (decode(gth, &gpa, &regx, &regp, &store, &size, &advance))
-               return FALSE;
-       if (__apic_access(gth, gpa, regx, regp, store))
+       if (fetch_insn(gth, insn)) {
+               vm_tf->tf_trap_inject = VM_TRAP_VALID
+                                     | VM_TRAP_ERROR_CODE
+                                     | VM_TRAP_HARDWARE
+                                     | HW_TRAP_PAGE_FAULT;
+               return true;
+       }
+       if (emulate_mem_insn(gth, insn, __apic_access, &advance))
                return FALSE;
        vm_tf->tf_rip += advance;
        return TRUE;
index 1d450f8..eb3ae1c 100644 (file)
@@ -80,3 +80,16 @@ int rippa(struct guest_thread *vm_thread, uint64_t *pa)
        assert(vm_thread != NULL);
        return gva2gpa(vm_thread, gth_to_vmtf(vm_thread)->tf_rip, pa);
 }
+
+int fetch_insn(struct guest_thread *gth, uint8_t *insn)
+{
+       uint64_t rip_gpa;
+
+       /* TODO: this will break if an instruction crosses a page boundary where
+        * the adjacent pages do not map to guest-physically contiguous pages.
+        * All callers of rippa() have this problem. */
+       if (rippa(gth, &rip_gpa))
+               return -1;
+       memcpy(insn, (void*)rip_gpa, VMM_MAX_INSN_SZ);
+       return 0;
+}