VMM: clean up IO emulation.
authorRonald G. Minnich <rminnich@gmail.com>
Fri, 3 Jun 2016 20:45:31 +0000 (13:45 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Sat, 4 Jun 2016 13:10:47 +0000 (09:10 -0400)
The io emulation code was written early in the game and was not correct
in several ways.

The big change is that we DO note that an IO address is not supported
but DO NOT consider it an error. The hardware does not, and we should
not. We do log a message to stderr that there was IO to an unsupported
address, but people commonly do such IO operations to implement, e.g.,
timing loops. It's perfectly legal to do IO to nonexistant config space
registers, as well, so it makes no sense to check those either. We can't
assume that an IO to a bogus address or config space register is not
intended or mistaken.

There is only one error return from the io() function: IO instructions
we did not handle because we could not interpret them. In that case, the
function really has no option but to return false. In all other cases,
it should return true.

As a result, all the configread/write functions become void, io() itself
now returns a bool, and handle_io returns what io returns.

We should consider changing regp() so that writes "to air" go to a
different place than reads "from air", as one might argue that reads
"from air" should return all 1s. In real hardware, not even this
reasonable rule always applies, so it's hard to make a strong argument
either way.

Change-Id: Ifec56655528f748183a35663c8ef20b8bc486b35
Signed-off-by: Ronald G. Minnich <rminnich@gmail.com>
Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
user/vmm/include/vmm/vmm.h
user/vmm/io.c
user/vmm/vmexit.c

index 971f2d0..8dedb28 100644 (file)
@@ -34,7 +34,7 @@ struct virtual_machine {
 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);
-int io(struct guest_thread *vm_thread);
+bool io(struct guest_thread *vm_thread);
 void showstatus(FILE *f, struct guest_thread *vm_thread);
 int msrio(struct guest_thread *vm_thread, struct vmm_gpcore_init *gpci,
           uint32_t opcode);
index 9e83057..f602c98 100644 (file)
@@ -53,23 +53,21 @@ void regp(uint32_t **reg)
        //printf("-->regp *reg 0x%lx\n", **reg);
 }
 
-static uint32_t configaddr(uint32_t val)
+static void configaddr(uint32_t val)
 {
        printf("%s 0x%lx\n", __func__, val);
        cf8 = val;
-       return 0;
 }
 
-static uint32_t configread32(uint32_t edx, uint64_t *reg)
+static void configread32(uint32_t edx, uint64_t *reg)
 {
        uint32_t *r = &cf8;
        regp(&r);
        *reg = *r;
        printf("%s: 0x%lx 0x%lx, 0x%lx 0x%lx\n", __func__, cf8, edx, r, *reg);
-       return 0;
 }
 
-static uint32_t configread16(uint32_t edx, uint64_t *reg)
+static void configread16(uint32_t edx, uint64_t *reg)
 {
        uint64_t val;
        int which = ((edx&2)>>1) * 16;
@@ -77,10 +75,9 @@ static uint32_t configread16(uint32_t edx, uint64_t *reg)
        val >>= which;
        *reg = val;
        printf("%s: 0x%lx, 0x%lx 0x%lx\n", __func__, edx, val, *reg);
-       return 0;
 }
 
-static uint32_t configread8(uint32_t edx, uint64_t *reg)
+static void configread8(uint32_t edx, uint64_t *reg)
 {
        uint64_t val;
        int which = (edx&3) * 8;
@@ -88,28 +85,24 @@ static uint32_t configread8(uint32_t edx, uint64_t *reg)
        val >>= which;
        *reg = val;
        printf("%s: 0x%lx, 0x%lx 0x%lx\n", __func__, edx, val, *reg);
-       return 0;
 }
 
-static int configwrite32(uint32_t addr, uint32_t val)
+static void configwrite32(uint32_t addr, uint32_t val)
 {
        uint32_t *r = &cf8;
        regp(&r);
        *r = val;
        printf("%s 0x%lx 0x%lx\n", __func__, addr, val);
-       return 0;
 }
 
-static int configwrite16(uint32_t addr, uint16_t val)
+static void configwrite16(uint32_t addr, uint16_t val)
 {
        printf("%s 0x%lx 0x%lx\n", __func__, addr, val);
-       return 0;
 }
 
-static int configwrite8(uint32_t addr, uint8_t val)
+static void configwrite8(uint32_t addr, uint8_t val)
 {
        printf("%s 0x%lx 0x%lx\n", __func__, addr, val);
-       return 0;
 }
 
 /* this is very minimal. It needs to move to vmm/io.c but we don't
@@ -119,7 +112,7 @@ static int configwrite8(uint32_t addr, uint8_t val)
  * It would have been nice had intel encoded the IO exit info as nicely as they
  * encoded, some of the other exits.
  */
-int io(struct guest_thread *vm_thread)
+bool io(struct guest_thread *vm_thread)
 {
 
        /* Get a pointer to the memory at %rip. This is quite messy and part of the
@@ -147,14 +140,31 @@ int io(struct guest_thread *vm_thread)
                /* out at %edx */
                if (edx == 0xcf8) {
                        //printf("Set cf8 ");
-                       return configaddr(vm_tf->tf_rax);
+                       configaddr(vm_tf->tf_rax);
+                       return true;
                }
                if (edx == 0xcfc) {
                        //printf("Set cfc ");
-                       return configwrite32(edx, vm_tf->tf_rax);
+                       configwrite32(edx, vm_tf->tf_rax);
+                       return true;
                }
+               /* While it is perfectly legal to do IO operations to
+                * nonexistant places, we print a warning here as it
+                * might also indicate a problem.  In practice these
+                * types of IOs happens less frequently, and whether
+                * they are bad or not is not always easy to decide.
+                * Simple example: for about the first 10 years Linux
+                * used to outb 0x98 to port 0x80 while idle. We
+                * wouldn't want to call that an error, but that kind
+                * of thing is a bad practice we ought to know about,
+                * because it can cause chipset errors and result in
+                * other non-obvious failures (in one case, breaking
+                * BIOS reflash operations).  Plus, true story, it
+                * confused people into thinking we were running
+                * Windows 98, not Linux.
+                */
                printf("(out rax, edx): unhandled IO address dx @%p is 0x%x\n", ip8, edx);
-               return -1;
+               return true;
        }
        // out %al, %dx
        if (*ip8 == 0xee) {
@@ -162,36 +172,49 @@ int io(struct guest_thread *vm_thread)
                /* out al %edx */
                if (edx == 0xcfb) { // special!
                        printf("Just ignore the damned cfb write\n");
-                       return 0;
+                       return true;
                }
                if ((edx&~3) == 0xcfc) {
                        //printf("ignoring write to cfc ");
-                       return 0;
+                       return true;
                }
+               /* Another case where we print a message but it's not an error. */
                printf("out al, dx: unhandled IO address dx @%p is 0x%x\n", ip8, edx);
-               return -1;
+               return true;
        }
        if (*ip8 == 0xec) {
                vm_tf->tf_rip += 1;
                //printf("configread8 ");
-               return configread8(edx, &vm_tf->tf_rax);
+               configread8(edx, &vm_tf->tf_rax);
+               return true;
        }
        if (*ip8 == 0xed) {
                vm_tf->tf_rip += 1;
                if (edx == 0xcf8) {
                        //printf("read cf8 0x%lx\n", v->regs.tf_rax);
                        vm_tf->tf_rax = cf8;
-                       return 0;
+                       return true;
                }
                //printf("configread32 ");
-               return configread32(edx, &vm_tf->tf_rax);
+               configread32(edx, &vm_tf->tf_rax);
+               return true;
        }
        if (*ip16 == 0xed66) {
                vm_tf->tf_rip += 2;
                //printf("configread16 ");
-               return configread16(edx, &vm_tf->tf_rax);
+               configread16(edx, &vm_tf->tf_rax);
+               return true;
        }
+
+       /* This is, so far, the only case in which we indicate
+        * failure: we can't even decode the instruction. We've
+        * implemented the common cases above, and recently this
+        * failure has been seen only when the RIP is set to some
+        * bizarre value and we start fetching instructions from
+        * (e.g.) the middle of a page table. PTEs look like IO
+        * instructions to the CPU.
+        */
        printf("unknown IO %p %x %x\n", ip8, *ip8, *ip16);
-       return -1;
+       return false;
 }
 
index f1fb83f..cefecdd 100644 (file)
@@ -74,8 +74,7 @@ static bool handle_vmcall(struct guest_thread *gth)
 
 static bool handle_io(struct guest_thread *gth)
 {
-       io(gth);
-       return TRUE;
+       return io(gth);
 }
 
 static bool handle_msr(struct guest_thread *gth)