From: Jack Lange Date: Thu, 17 Nov 2011 03:42:04 +0000 (-0500) Subject: Fix to MSR save/restore handling to avoid VMX ABORT errors X-Git-Url: http://v3vee.org/palacios/gitweb/gitweb.cgi?p=palacios.git;a=commitdiff_plain;h=88648ddca6003a438826e7d86e28a2ba7b5bfcac Fix to MSR save/restore handling to avoid VMX ABORT errors --- diff --git a/palacios/include/palacios/vmx.h b/palacios/include/palacios/vmx.h index 06e4df2..4384b13 100644 --- a/palacios/include/palacios/vmx.h +++ b/palacios/include/palacios/vmx.h @@ -188,6 +188,29 @@ struct vmcs_host_state { }; +struct vmcs_msr_save_area { + union { + struct vmcs_msr_entry guest_msrs[4]; + struct { + struct vmcs_msr_entry guest_star; + struct vmcs_msr_entry guest_lstar; + struct vmcs_msr_entry guest_fmask; + struct vmcs_msr_entry guest_kern_gs; + } __attribute__((packed)); + } __attribute__((packed)); + + union { + struct vmcs_msr_entry host_msrs[4]; + struct { + struct vmcs_msr_entry host_star; + struct vmcs_msr_entry host_lstar; + struct vmcs_msr_entry host_fmask; + struct vmcs_msr_entry host_kern_gs; + } __attribute__((packed)); + } __attribute__((packed)); + +} __attribute__((packed)); + struct vmx_data { vmx_state_t state; @@ -210,7 +233,8 @@ struct vmx_data { struct vmx_exception_bitmap excp_bmap; - void * msr_area; + addr_t msr_area_paddr; + struct vmcs_msr_save_area * msr_area; }; int v3_is_vmx_capable(); diff --git a/palacios/src/palacios/vmcs.c b/palacios/src/palacios/vmcs.c index 1de8e6a..8fb4080 100644 --- a/palacios/src/palacios/vmcs.c +++ b/palacios/src/palacios/vmcs.c @@ -489,8 +489,16 @@ int v3_update_vmcs_host_state(struct guest_info * info) { // save STAR, LSTAR, FMASK, KERNEL_GS_BASE MSRs in MSR load/store area + { + struct vmx_data * vmx_state = (struct vmx_data *)info->vmm_data; + struct vmcs_msr_save_area * msr_entries = vmx_state->msr_area; + v3_get_msr(IA32_STAR_MSR, &(msr_entries->host_star.hi), &(msr_entries->host_star.lo)); + v3_get_msr(IA32_LSTAR_MSR, &(msr_entries->host_lstar.hi), &(msr_entries->host_lstar.lo)); + v3_get_msr(IA32_FMASK_MSR, &(msr_entries->host_fmask.hi), &(msr_entries->host_fmask.lo)); + v3_get_msr(IA32_KERN_GS_BASE_MSR, &(msr_entries->host_kern_gs.hi), &(msr_entries->host_kern_gs.lo)); + } diff --git a/palacios/src/palacios/vmx.c b/palacios/src/palacios/vmx.c index 5746e93..90f8e52 100644 --- a/palacios/src/palacios/vmx.c +++ b/palacios/src/palacios/vmx.c @@ -349,12 +349,10 @@ static int init_vmcs_bios(struct guest_info * core, struct vmx_data * vmx_state) // save STAR, LSTAR, FMASK, KERNEL_GS_BASE MSRs in MSR load/store area { - int msr_ret = 0; - struct vmcs_msr_entry * exit_store_msrs = NULL; - struct vmcs_msr_entry * exit_load_msrs = NULL; - struct vmcs_msr_entry * entry_load_msrs = NULL;; + struct vmcs_msr_save_area * msr_entries = NULL; int max_msrs = (hw_info.misc_info.max_msr_cache_size + 1) * 4; + int msr_ret = 0; V3_Print("Setting up MSR load/store areas (max_msr_count=%d)\n", max_msrs); @@ -363,58 +361,60 @@ static int init_vmcs_bios(struct guest_info * core, struct vmx_data * vmx_state) return -1; } - vmx_state->msr_area = V3_VAddr(V3_AllocPages(1)); - - if (vmx_state->msr_area == NULL) { + vmx_state->msr_area_paddr = (addr_t)V3_AllocPages(1); + + if (vmx_state->msr_area_paddr == (addr_t)NULL) { PrintError("could not allocate msr load/store area\n"); return -1; } + msr_entries = (struct vmcs_msr_save_area *)V3_VAddr((void *)(vmx_state->msr_area_paddr)); + vmx_state->msr_area = msr_entries; // cache in vmx_info + + memset(msr_entries, 0, PAGE_SIZE); + + msr_entries->guest_star.index = IA32_STAR_MSR; + msr_entries->guest_lstar.index = IA32_LSTAR_MSR; + msr_entries->guest_fmask.index = IA32_FMASK_MSR; + msr_entries->guest_kern_gs.index = IA32_KERN_GS_BASE_MSR; + + msr_entries->host_star.index = IA32_STAR_MSR; + msr_entries->host_lstar.index = IA32_LSTAR_MSR; + msr_entries->host_fmask.index = IA32_FMASK_MSR; + msr_entries->host_kern_gs.index = IA32_KERN_GS_BASE_MSR; + msr_ret |= check_vmcs_write(VMCS_EXIT_MSR_STORE_CNT, 4); msr_ret |= check_vmcs_write(VMCS_EXIT_MSR_LOAD_CNT, 4); msr_ret |= check_vmcs_write(VMCS_ENTRY_MSR_LOAD_CNT, 4); - - - exit_store_msrs = (struct vmcs_msr_entry *)(vmx_state->msr_area); - exit_load_msrs = (struct vmcs_msr_entry *)(vmx_state->msr_area + (sizeof(struct vmcs_msr_entry) * 4)); - entry_load_msrs = (struct vmcs_msr_entry *)(vmx_state->msr_area + (sizeof(struct vmcs_msr_entry) * 8)); + msr_ret |= check_vmcs_write(VMCS_EXIT_MSR_STORE_ADDR, (addr_t)V3_PAddr(msr_entries->guest_msrs)); + msr_ret |= check_vmcs_write(VMCS_ENTRY_MSR_LOAD_ADDR, (addr_t)V3_PAddr(msr_entries->guest_msrs)); + msr_ret |= check_vmcs_write(VMCS_EXIT_MSR_LOAD_ADDR, (addr_t)V3_PAddr(msr_entries->host_msrs)); - exit_store_msrs[0].index = IA32_STAR_MSR; - exit_store_msrs[1].index = IA32_LSTAR_MSR; - exit_store_msrs[2].index = IA32_FMASK_MSR; - exit_store_msrs[3].index = IA32_KERN_GS_BASE_MSR; - - memcpy(exit_store_msrs, exit_load_msrs, sizeof(struct vmcs_msr_entry) * 4); - memcpy(exit_store_msrs, entry_load_msrs, sizeof(struct vmcs_msr_entry) * 4); - - v3_get_msr(IA32_STAR_MSR, &(exit_load_msrs[0].hi), &(exit_load_msrs[0].lo)); - v3_get_msr(IA32_LSTAR_MSR, &(exit_load_msrs[1].hi), &(exit_load_msrs[1].lo)); - v3_get_msr(IA32_FMASK_MSR, &(exit_load_msrs[2].hi), &(exit_load_msrs[2].lo)); - v3_get_msr(IA32_KERN_GS_BASE_MSR, &(exit_load_msrs[3].hi), &(exit_load_msrs[3].lo)); + msr_ret |= v3_hook_msr(core->vm_info, IA32_STAR_MSR, NULL, NULL, NULL); + msr_ret |= v3_hook_msr(core->vm_info, IA32_LSTAR_MSR, NULL, NULL, NULL); + msr_ret |= v3_hook_msr(core->vm_info, IA32_FMASK_MSR, NULL, NULL, NULL); + msr_ret |= v3_hook_msr(core->vm_info, IA32_KERN_GS_BASE_MSR, NULL, NULL, NULL); - msr_ret |= check_vmcs_write(VMCS_EXIT_MSR_STORE_ADDR, (addr_t)V3_PAddr(exit_store_msrs)); - msr_ret |= check_vmcs_write(VMCS_EXIT_MSR_LOAD_ADDR, (addr_t)V3_PAddr(exit_load_msrs)); - msr_ret |= check_vmcs_write(VMCS_ENTRY_MSR_LOAD_ADDR, (addr_t)V3_PAddr(entry_load_msrs)); + // IMPORTANT: These MSRs appear to be cached by the hardware.... + msr_ret |= v3_hook_msr(core->vm_info, SYSENTER_CS_MSR, NULL, NULL, NULL); + msr_ret |= v3_hook_msr(core->vm_info, SYSENTER_ESP_MSR, NULL, NULL, NULL); + msr_ret |= v3_hook_msr(core->vm_info, SYSENTER_EIP_MSR, NULL, NULL, NULL); - v3_hook_msr(core->vm_info, IA32_STAR_MSR, NULL, NULL, NULL); - v3_hook_msr(core->vm_info, IA32_LSTAR_MSR, NULL, NULL, NULL); - v3_hook_msr(core->vm_info, IA32_FMASK_MSR, NULL, NULL, NULL); - v3_hook_msr(core->vm_info, IA32_KERN_GS_BASE_MSR, NULL, NULL, NULL); + msr_ret |= v3_hook_msr(core->vm_info, FS_BASE_MSR, NULL, NULL, NULL); + msr_ret |= v3_hook_msr(core->vm_info, GS_BASE_MSR, NULL, NULL, NULL); - // IMPORTANT: These SYSCALL MSRs are currently not handled by hardware or cached - // We should really emulate these ourselves, or ideally include them in the MSR store area if there is room - v3_hook_msr(core->vm_info, IA32_CSTAR_MSR, NULL, NULL, NULL); - v3_hook_msr(core->vm_info, SYSENTER_CS_MSR, NULL, NULL, NULL); - v3_hook_msr(core->vm_info, SYSENTER_ESP_MSR, NULL, NULL, NULL); - v3_hook_msr(core->vm_info, SYSENTER_EIP_MSR, NULL, NULL, NULL); - - v3_hook_msr(core->vm_info, FS_BASE_MSR, NULL, NULL, NULL); - v3_hook_msr(core->vm_info, GS_BASE_MSR, NULL, NULL, NULL); - + // Not sure what to do about this... Does not appear to be an explicit hardware cache version... + msr_ret |= v3_hook_msr(core->vm_info, IA32_CSTAR_MSR, NULL, NULL, NULL); + + if (msr_ret != 0) { + PrintError("Error configuring MSR save/restore area\n"); + return -1; + } + } @@ -451,10 +451,12 @@ static int init_vmcs_bios(struct guest_info * core, struct vmx_data * vmx_state) return -1; } + /* if (v3_update_vmcs_host_state(core)) { PrintError("Could not write host state\n"); return -1; } + */ // reenable global interrupts for vm state initialization now // that the vm state is initialized. If another VM kicks us off,