VMM: Fix vmm_struct_init() off-by-one
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 3 Aug 2016 17:13:57 +0000 (10:13 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 3 Aug 2016 17:35:01 +0000 (10:35 -0700)
We were returning and setting nr_guest_pcores to one less than the
number of cores we set up.

I also got rid of using 'i' outside the loop, which was slightly
confusing due to the other loop using its own 'i'.  I also cleaned up
some old comments that referred to the pre-waserror error handling
style.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/arch/x86/vmm/intel/vmx.c
kern/arch/x86/vmm/vmm.c

index 7ca1c67..485bb27 100644 (file)
@@ -802,7 +802,7 @@ construct_eptp(physaddr_t root_hpa)
 
 /* Helper: some fields of the VMCS need a physical page address, e.g. the VAPIC
  * page.  We have the user address.  This converts the user to phys addr and
- * sets that up in the VMCS.  Returns 0 on success, -1 o/w. */
+ * sets that up in the VMCS.  Throws on error. */
 static void vmcs_set_pgaddr(struct proc *p, void *u_addr,
                             unsigned long field, char *what)
 {
@@ -830,7 +830,7 @@ static void vmcs_set_pgaddr(struct proc *p, void *u_addr,
 
 /**
  * vmx_setup_initial_guest_state - configures the initial state of guest
- * registers and the VMCS.  Returns 0 on success, -1 o/w.
+ * registers and the VMCS.  Throws on error.
  */
 static void vmx_setup_initial_guest_state(struct proc *p,
                                           struct vmm_gpcore_init *gpci)
@@ -945,8 +945,7 @@ static void vmx_setup_initial_guest_state(struct proc *p,
        vmcs_writel(EOI_EXIT_BITMAP3, 0);
        vmcs_writel(EOI_EXIT_BITMAP3_HIGH, 0);
 
-       /* Initialize parts based on the users info.  If one of them fails, we'll do
-        * the others but then error out. */
+       /* Initialize parts based on the users info. */
        vmcs_set_pgaddr(p, gpci->posted_irq_desc, POSTED_INTR_DESC_ADDR,
                        "posted_irq_desc");
        vmcs_set_pgaddr(p, gpci->vapic_addr, VIRTUAL_APIC_PAGE_ADDR,
index 0781544..072f320 100644 (file)
@@ -66,13 +66,12 @@ void vmm_pcpu_init(void)
 }
 
 /* Initializes a process to run virtual machine contexts, returning the number
- * initialized, optionally setting errno */
+ * initialized, throwing on error. */
 int vmm_struct_init(struct proc *p, unsigned int nr_guest_pcores,
                     struct vmm_gpcore_init *u_gpcis, int flags)
 {
        ERRSTACK(1);
        struct vmm *vmm = &p->vmm;
-       unsigned int i;
        struct vmm_gpcore_init gpci;
 
        if (flags & ~VMM_ALL_FLAGS)
@@ -98,17 +97,17 @@ int vmm_struct_init(struct proc *p, unsigned int nr_guest_pcores,
        if (!vmm->guest_pcores)
                error(ENOMEM, "Allocation of vmm->guest_pcores failed");
 
-       for (i = 0; i < nr_guest_pcores; i++) {
+       for (int i = 0; i < nr_guest_pcores; i++) {
                if (copy_from_user(&gpci, &u_gpcis[i], sizeof(struct vmm_gpcore_init)))
                        error(EINVAL, "Bad pointer %p for gps", u_gpcis);
                vmm->guest_pcores[i] = create_guest_pcore(p, &gpci);
-               vmm->nr_guest_pcores = i;
+               vmm->nr_guest_pcores = i + 1;
        }
        for (int i = 0; i < VMM_VMEXIT_NR_TYPES; i++)
                vmm->vmexits[i] = 0;
        qunlock(&vmm->qlock);
        poperror();
-       return i;
+       return vmm->nr_guest_pcores;
 }
 
 /* Has no concurrency protection - only call this when you know you have the