From 5ccf78fdd9a2728d6cd88d1b2c1f800c00589a77 Mon Sep 17 00:00:00 2001 From: "kfraser@localhost.localdomain" Date: Fri, 1 Dec 2006 09:48:18 +0000 Subject: [PATCH 1/8] [HVM][VMX] Fix for CR8 acceleration on 64bit guest. For current CR8 acceleration, we do not call update_tpr_threshold() at every VMEXIT. But at some situations, we cannot inject guest interrupts in time. And at some critical time, it will bring up a blue screen to 64bit Windows guest. Now, we select to call update_tpr_threshold() at very VMEXIT time. It's safe, and we do not see clear performance downgrade so far. Signed-off-by: Xiaohui Xin xiaohui.xin@intel.com --- xen/arch/x86/hvm/vlapic.c | 9 --------- xen/arch/x86/hvm/vmx/io.c | 8 +------- xen/arch/x86/hvm/vmx/vmx.c | 1 - xen/include/asm-x86/hvm/vlapic.h | 1 - 4 files changed, 1 insertion(+), 18 deletions(-) diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 1c6bef005e..60e6171c47 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -119,19 +119,16 @@ static int vlapic_find_highest_vector(u32 *bitmap) static int vlapic_test_and_set_irr(int vector, struct vlapic *vlapic) { - vlapic->flush_tpr_threshold = 1; return vlapic_test_and_set_vector(vector, vlapic->regs + APIC_IRR); } static void vlapic_set_irr(int vector, struct vlapic *vlapic) { - vlapic->flush_tpr_threshold = 1; vlapic_set_vector(vector, vlapic->regs + APIC_IRR); } static void vlapic_clear_irr(int vector, struct vlapic *vlapic) { - vlapic->flush_tpr_threshold = 1; vlapic_clear_vector(vector, vlapic->regs + APIC_IRR); } @@ -634,7 +631,6 @@ static void vlapic_write(struct vcpu *v, unsigned long address, { case APIC_TASKPRI: vlapic_set_reg(vlapic, APIC_TASKPRI, val & 0xff); - vlapic->flush_tpr_threshold = 1; break; case APIC_EOI: @@ -667,10 +663,7 @@ static void vlapic_write(struct vcpu *v, unsigned long address, } } else - { vlapic->disabled &= ~VLAPIC_SW_DISABLED; - vlapic->flush_tpr_threshold = 1; - } break; case APIC_ESR: @@ -925,8 +918,6 @@ static int vlapic_reset(struct vlapic *vlapic) vlapic_set_reg(vlapic, APIC_SPIV, 0xff); vlapic->disabled |= VLAPIC_SW_DISABLED; - vlapic->flush_tpr_threshold = 1; - return 1; } diff --git a/xen/arch/x86/hvm/vmx/io.c b/xen/arch/x86/hvm/vmx/io.c index 2f3cf623b1..662a6697a0 100644 --- a/xen/arch/x86/hvm/vmx/io.c +++ b/xen/arch/x86/hvm/vmx/io.c @@ -71,10 +71,6 @@ static void update_tpr_threshold(struct vlapic *vlapic) { int max_irr, tpr; - /* Clear the work-to-do flag /then/ do the work. */ - vlapic->flush_tpr_threshold = 0; - mb(); - if ( !vlapic_enabled(vlapic) || ((max_irr = vlapic_find_highest_irr(vlapic)) == -1) ) { @@ -95,7 +91,6 @@ asmlinkage void vmx_intr_assist(void) int highest_vector; unsigned long eflags; struct vcpu *v = current; - struct vlapic *vlapic = vcpu_vlapic(v); struct hvm_domain *plat=&v->domain->arch.hvm_domain; struct periodic_time *pt = &plat->pl_time.periodic_tm; unsigned int idtv_info_field; @@ -110,8 +105,7 @@ asmlinkage void vmx_intr_assist(void) hvm_set_callback_irq_level(); - if ( vlapic->flush_tpr_threshold ) - update_tpr_threshold(vlapic); + update_tpr_threshold(vcpu_vlapic(v)); has_ext_irq = cpu_has_pending_irq(v); diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 7d226e6ba1..58cca4bea4 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -2500,7 +2500,6 @@ asmlinkage void vmx_vmexit_handler(struct cpu_user_regs *regs) break; case EXIT_REASON_TPR_BELOW_THRESHOLD: - vcpu_vlapic(v)->flush_tpr_threshold = 1; break; default: diff --git a/xen/include/asm-x86/hvm/vlapic.h b/xen/include/asm-x86/hvm/vlapic.h index 003d93b389..2053fc56ec 100644 --- a/xen/include/asm-x86/hvm/vlapic.h +++ b/xen/include/asm-x86/hvm/vlapic.h @@ -54,7 +54,6 @@ struct vlapic { uint32_t timer_divisor; struct timer vlapic_timer; int timer_pending_count; - int flush_tpr_threshold; s_time_t timer_last_update; struct page_info *regs_page; void *regs; From f5bf5c912030f39d802f972c13b23e4afcf6a49b Mon Sep 17 00:00:00 2001 From: "kfraser@localhost.localdomain" Date: Fri, 1 Dec 2006 09:51:13 +0000 Subject: [PATCH 2/8] [HVM][VMX] Rename io.c -> intr.c. Signed-off-by: Keir Fraser --- xen/arch/x86/hvm/vmx/Makefile | 2 +- xen/arch/x86/hvm/vmx/{io.c => intr.c} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename xen/arch/x86/hvm/vmx/{io.c => intr.c} (100%) diff --git a/xen/arch/x86/hvm/vmx/Makefile b/xen/arch/x86/hvm/vmx/Makefile index 3eede43031..ba31793c2f 100644 --- a/xen/arch/x86/hvm/vmx/Makefile +++ b/xen/arch/x86/hvm/vmx/Makefile @@ -1,6 +1,6 @@ subdir-$(x86_32) += x86_32 subdir-$(x86_64) += x86_64 -obj-y += io.o +obj-y += intr.o obj-y += vmcs.o obj-y += vmx.o diff --git a/xen/arch/x86/hvm/vmx/io.c b/xen/arch/x86/hvm/vmx/intr.c similarity index 100% rename from xen/arch/x86/hvm/vmx/io.c rename to xen/arch/x86/hvm/vmx/intr.c From fefaf21f9d1376009766e62424d76bc112f320e6 Mon Sep 17 00:00:00 2001 From: "kfraser@localhost.localdomain" Date: Fri, 1 Dec 2006 09:59:56 +0000 Subject: [PATCH 3/8] [XENOPROF] Fix oprofile for AMD SVM guests. Signed-off-by: Keir Fraser --- xen/arch/x86/oprofile/op_model_athlon.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/xen/arch/x86/oprofile/op_model_athlon.c b/xen/arch/x86/oprofile/op_model_athlon.c index 1d8f9668f4..ec99be48b0 100644 --- a/xen/arch/x86/oprofile/op_model_athlon.c +++ b/xen/arch/x86/oprofile/op_model_athlon.c @@ -113,14 +113,15 @@ static int athlon_check_ctrs(unsigned int const cpu, unsigned long eip = regs->eip; int mode = 0; struct vcpu *v = current; - struct cpu_user_regs tmp_regs; + struct cpu_user_regs *guest_regs = guest_cpu_user_regs(); if (!guest_mode(regs) && (regs->eip == (unsigned long)svm_stgi_label)) { /* SVM guest was running when NMI occurred */ - hvm_store_cpu_guest_regs(v, &tmp_regs, NULL); - eip = tmp_regs.eip; - mode = xenoprofile_get_mode(v, &tmp_regs); + ASSERT(is_hvm_vcpu(v)); + hvm_store_cpu_guest_regs(v, guest_regs, NULL); + eip = guest_regs->eip; + mode = xenoprofile_get_mode(v, guest_regs); } else { eip = regs->eip; mode = xenoprofile_get_mode(v, regs); From 515db7a43abd0a872878c7408c1eb136e299d9fa Mon Sep 17 00:00:00 2001 From: Alastair Tse Date: Fri, 1 Dec 2006 10:47:57 +0000 Subject: [PATCH 4/8] [XEND] Fix missing serial and localtime args in device model for HVM Signed-off-by: Alastair Tse --- tools/python/xen/xend/XendConfig.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/python/xen/xend/XendConfig.py b/tools/python/xen/xend/XendConfig.py index 3d79c657c9..3bb0ba4cd9 100644 --- a/tools/python/xen/xend/XendConfig.py +++ b/tools/python/xen/xend/XendConfig.py @@ -186,19 +186,21 @@ LEGACY_IMAGE_HVM_CFG = [ ('pae', int), ('apic', int), ('acpi', int), - ('serial', str), ] LEGACY_IMAGE_HVM_DEVICES_CFG = [ + ('acpi', int), ('boot', str), ('fda', str), ('fdb', str), - ('soundhw', str), ('isa', str), - ('vcpus', int), - ('acpi', int), + ('keymap', str), + ('localtime', str), + ('serial', str), + ('soundhw', str), ('usb', str), - ('usbdevice', str), + ('usbdevice', str), + ('vcpus', int), ] From e4022aef8e9076a5ffe70abfd2fff6fee5c99512 Mon Sep 17 00:00:00 2001 From: "kfraser@localhost.localdomain" Date: Fri, 1 Dec 2006 11:04:27 +0000 Subject: [PATCH 5/8] [HVM] Add canonical address checks and generally clean up. Based on a patch from Jan Beulich Signed-off-by: Keir Fraser --- xen/arch/x86/hvm/svm/emulate.c | 11 --- xen/arch/x86/hvm/svm/svm.c | 122 ++++++++++++++--------- xen/arch/x86/hvm/vmx/vmx.c | 154 ++++++++++++++++++------------ xen/arch/x86/mm/shadow/common.c | 11 ++- xen/include/asm-x86/hvm/hvm.h | 4 + xen/include/asm-x86/x86_32/page.h | 2 + xen/include/asm-x86/x86_64/page.h | 2 + 7 files changed, 187 insertions(+), 119 deletions(-) diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c index c0ea001c82..7966bb9da2 100644 --- a/xen/arch/x86/hvm/svm/emulate.c +++ b/xen/arch/x86/hvm/svm/emulate.c @@ -128,17 +128,6 @@ static inline unsigned long DECODE_GPR_VALUE(struct vmcb_struct *vmcb, return (unsigned long) -1; \ } -#if 0 -/* - * hv_is_canonical - checks if the given address is canonical - */ -static inline u64 hv_is_canonical(u64 addr) -{ - u64 bits = addr & (u64)0xffff800000000000; - return (u64)((bits == (u64)0xffff800000000000) || (bits == (u64)0x0)); -} -#endif - #define modrm operand [0] #define sib operand [1] diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 9c5393f5f5..b95be55b80 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -269,13 +269,11 @@ static int svm_long_mode_enabled(struct vcpu *v) return test_bit(SVM_CPU_STATE_LMA_ENABLED, &v->arch.hvm_svm.cpu_state); } -#define IS_CANO_ADDRESS(add) 1 - static inline int long_mode_do_msr_read(struct cpu_user_regs *regs) { u64 msr_content = 0; - struct vcpu *vc = current; - struct vmcb_struct *vmcb = vc->arch.hvm_svm.vmcb; + struct vcpu *v = current; + struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; switch ((u32)regs->ecx) { @@ -284,17 +282,25 @@ static inline int long_mode_do_msr_read(struct cpu_user_regs *regs) msr_content &= ~EFER_SVME; break; +#ifdef __x86_64__ case MSR_FS_BASE: msr_content = vmcb->fs.base; - break; + goto check_long_mode; case MSR_GS_BASE: msr_content = vmcb->gs.base; - break; + goto check_long_mode; case MSR_SHADOW_GS_BASE: msr_content = vmcb->kerngsbase; + check_long_mode: + if ( !svm_long_mode_enabled(v) ) + { + svm_inject_exception(v, TRAP_gp_fault, 1, 0); + return 0; + } break; +#endif case MSR_STAR: msr_content = vmcb->star; @@ -326,25 +332,25 @@ static inline int long_mode_do_msr_read(struct cpu_user_regs *regs) static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) { u64 msr_content = (u32)regs->eax | ((u64)regs->edx << 32); + u32 ecx = regs->ecx; struct vcpu *v = current; struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; HVM_DBG_LOG(DBG_LEVEL_1, "msr %x msr_content %"PRIx64"\n", - (u32)regs->ecx, msr_content); + ecx, msr_content); - switch ( (u32)regs->ecx ) + switch ( ecx ) { case MSR_EFER: -#ifdef __x86_64__ /* offending reserved bit will cause #GP */ if ( msr_content & ~(EFER_LME | EFER_LMA | EFER_NX | EFER_SCE) ) { - printk("Trying to set reserved bit in EFER: %"PRIx64"\n", - msr_content); - svm_inject_exception(v, TRAP_gp_fault, 1, 0); - return 0; + gdprintk(XENLOG_WARNING, "Trying to set reserved bit in " + "EFER: %"PRIx64"\n", msr_content); + goto gp_fault; } +#ifdef __x86_64__ /* LME: 0 -> 1 */ if ( msr_content & EFER_LME && !test_bit(SVM_CPU_STATE_LME_ENABLED, &v->arch.hvm_svm.cpu_state)) @@ -353,10 +359,9 @@ static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) !test_bit(SVM_CPU_STATE_PAE_ENABLED, &v->arch.hvm_svm.cpu_state) ) { - printk("Trying to set LME bit when " - "in paging mode or PAE bit is not set\n"); - svm_inject_exception(v, TRAP_gp_fault, 1, 0); - return 0; + gdprintk(XENLOG_WARNING, "Trying to set LME bit when " + "in paging mode or PAE bit is not set\n"); + goto gp_fault; } set_bit(SVM_CPU_STATE_LME_ENABLED, &v->arch.hvm_svm.cpu_state); } @@ -371,37 +376,38 @@ static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) vmcb->efer = msr_content | EFER_SVME; break; +#ifdef __x86_64__ case MSR_FS_BASE: case MSR_GS_BASE: - if ( !svm_long_mode_enabled(v) ) - goto exit_and_crash; - - if (!IS_CANO_ADDRESS(msr_content)) - { - HVM_DBG_LOG(DBG_LEVEL_1, "Not cano address of msr write\n"); - svm_inject_exception(v, TRAP_gp_fault, 1, 0); - } - - if (regs->ecx == MSR_FS_BASE) - vmcb->fs.base = msr_content; - else - vmcb->gs.base = msr_content; - break; - case MSR_SHADOW_GS_BASE: - vmcb->kerngsbase = msr_content; + if ( !svm_long_mode_enabled(v) ) + goto gp_fault; + + if ( !is_canonical_address(msr_content) ) + goto uncanonical_address; + + if ( ecx == MSR_FS_BASE ) + vmcb->fs.base = msr_content; + else if ( ecx == MSR_GS_BASE ) + vmcb->gs.base = msr_content; + else + vmcb->kerngsbase = msr_content; break; +#endif case MSR_STAR: vmcb->star = msr_content; break; case MSR_LSTAR: - vmcb->lstar = msr_content; - break; - case MSR_CSTAR: - vmcb->cstar = msr_content; + if ( !is_canonical_address(msr_content) ) + goto uncanonical_address; + + if ( ecx == MSR_LSTAR ) + vmcb->lstar = msr_content; + else + vmcb->cstar = msr_content; break; case MSR_SYSCALL_MASK: @@ -414,10 +420,11 @@ static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) return 1; - exit_and_crash: - gdprintk(XENLOG_ERR, "Fatal error writing MSR %lx\n", (long)regs->ecx); - domain_crash(v->domain); - return 1; /* handled */ + uncanonical_address: + HVM_DBG_LOG(DBG_LEVEL_1, "Not cano address of msr write %x\n", ecx); + gp_fault: + svm_inject_exception(v, TRAP_gp_fault, 1, 0); + return 0; } @@ -1272,7 +1279,7 @@ static inline int svm_get_io_address( #endif /* d field of cs.attr is 1 for 32-bit, 0 for 16 or 64 bit. - * l field combined with EFER_LMA -> longmode says whether it's 16 or 64 bit. + * l field combined with EFER_LMA says whether it's 16 or 64 bit. */ asize = (long_mode)?64:((vmcb->cs.attr.fields.db)?32:16); @@ -1383,8 +1390,35 @@ static inline int svm_get_io_address( *addr += seg->base; } - else if (seg == &vmcb->fs || seg == &vmcb->gs) - *addr += seg->base; +#ifdef __x86_64__ + else + { + if (seg == &vmcb->fs || seg == &vmcb->gs) + *addr += seg->base; + + if (!is_canonical_address(*addr) || + !is_canonical_address(*addr + size - 1)) + { + svm_inject_exception(v, TRAP_gp_fault, 1, 0); + return 0; + } + if (*count > (1UL << 48) / size) + *count = (1UL << 48) / size; + if (!(regs->eflags & EF_DF)) + { + if (*addr + *count * size - 1 < *addr || + !is_canonical_address(*addr + *count * size - 1)) + *count = (*addr & ~((1UL << 48) - 1)) / size; + } + else + { + if ((*count - 1) * size > *addr || + !is_canonical_address(*addr + (*count - 1) * size)) + *count = (*addr & ~((1UL << 48) - 1)) / size + 1; + } + ASSERT(*count); + } +#endif return 1; } diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 58cca4bea4..556ea7b79f 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -95,13 +95,7 @@ static void vmx_save_host_msrs(void) rdmsrl(msr_index[i], host_msr_state->msrs[i]); } -#define CASE_READ_MSR(address) \ - case MSR_ ## address: \ - msr_content = guest_msr_state->msrs[VMX_INDEX_MSR_ ## address]; \ - break - -#define CASE_WRITE_MSR(address) \ - case MSR_ ## address: \ +#define WRITE_MSR(address) \ guest_msr_state->msrs[VMX_INDEX_MSR_ ## address] = msr_content; \ if ( !test_bit(VMX_INDEX_MSR_ ## address, &guest_msr_state->flags) )\ set_bit(VMX_INDEX_MSR_ ## address, &guest_msr_state->flags); \ @@ -109,7 +103,6 @@ static void vmx_save_host_msrs(void) set_bit(VMX_INDEX_MSR_ ## address, &host_msr_state->flags); \ break -#define IS_CANO_ADDRESS(add) 1 static inline int long_mode_do_msr_read(struct cpu_user_regs *regs) { u64 msr_content = 0; @@ -123,27 +116,38 @@ static inline int long_mode_do_msr_read(struct cpu_user_regs *regs) break; case MSR_FS_BASE: - if ( !(vmx_long_mode_enabled(v)) ) - goto exit_and_crash; - msr_content = __vmread(GUEST_FS_BASE); - break; + goto check_long_mode; case MSR_GS_BASE: - if ( !(vmx_long_mode_enabled(v)) ) - goto exit_and_crash; - msr_content = __vmread(GUEST_GS_BASE); - break; + goto check_long_mode; case MSR_SHADOW_GS_BASE: msr_content = guest_msr_state->shadow_gs; + check_long_mode: + if ( !(vmx_long_mode_enabled(v)) ) + { + vmx_inject_hw_exception(v, TRAP_gp_fault, 0); + return 0; + } break; - CASE_READ_MSR(STAR); - CASE_READ_MSR(LSTAR); - CASE_READ_MSR(CSTAR); - CASE_READ_MSR(SYSCALL_MASK); + case MSR_STAR: + msr_content = guest_msr_state->msrs[VMX_INDEX_MSR_STAR]; + break; + + case MSR_LSTAR: + msr_content = guest_msr_state->msrs[VMX_INDEX_MSR_LSTAR]; + break; + + case MSR_CSTAR: + msr_content = guest_msr_state->msrs[VMX_INDEX_MSR_CSTAR]; + break; + + case MSR_SYSCALL_MASK: + msr_content = guest_msr_state->msrs[VMX_INDEX_MSR_SYSCALL_MASK]; + break; default: return 0; @@ -155,32 +159,28 @@ static inline int long_mode_do_msr_read(struct cpu_user_regs *regs) regs->edx = (u32)(msr_content >> 32); return 1; - - exit_and_crash: - gdprintk(XENLOG_ERR, "Fatal error reading MSR %lx\n", (long)regs->ecx); - domain_crash(v->domain); - return 1; /* handled */ } static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) { u64 msr_content = (u32)regs->eax | ((u64)regs->edx << 32); + u32 ecx = regs->ecx; struct vcpu *v = current; struct vmx_msr_state *guest_msr_state = &v->arch.hvm_vmx.msr_state; struct vmx_msr_state *host_msr_state = &this_cpu(host_msr_state); HVM_DBG_LOG(DBG_LEVEL_1, "msr 0x%x msr_content 0x%"PRIx64"\n", - (u32)regs->ecx, msr_content); + ecx, msr_content); - switch ( (u32)regs->ecx ) { + switch ( ecx ) + { case MSR_EFER: /* offending reserved bit will cause #GP */ if ( msr_content & ~(EFER_LME | EFER_LMA | EFER_NX | EFER_SCE) ) { - printk("Trying to set reserved bit in EFER: %"PRIx64"\n", - msr_content); - vmx_inject_hw_exception(v, TRAP_gp_fault, 0); - return 0; + gdprintk(XENLOG_WARNING, "Trying to set reserved bit in " + "EFER: %"PRIx64"\n", msr_content); + goto gp_fault; } if ( (msr_content & EFER_LME) @@ -188,9 +188,9 @@ static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) { if ( unlikely(vmx_paging_enabled(v)) ) { - printk("Trying to set EFER.LME with paging enabled\n"); - vmx_inject_hw_exception(v, TRAP_gp_fault, 0); - return 0; + gdprintk(XENLOG_WARNING, + "Trying to set EFER.LME with paging enabled\n"); + goto gp_fault; } } else if ( !(msr_content & EFER_LME) @@ -198,9 +198,9 @@ static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) { if ( unlikely(vmx_paging_enabled(v)) ) { - printk("Trying to clear EFER.LME with paging enabled\n"); - vmx_inject_hw_exception(v, TRAP_gp_fault, 0); - return 0; + gdprintk(XENLOG_WARNING, + "Trying to clear EFER.LME with paging enabled\n"); + goto gp_fault; } } @@ -209,35 +209,40 @@ static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) case MSR_FS_BASE: case MSR_GS_BASE: + case MSR_SHADOW_GS_BASE: if ( !vmx_long_mode_enabled(v) ) - goto exit_and_crash; + goto gp_fault; - if ( !IS_CANO_ADDRESS(msr_content) ) + if ( !is_canonical_address(msr_content) ) + goto uncanonical_address; + + if ( ecx == MSR_FS_BASE ) + __vmwrite(GUEST_FS_BASE, msr_content); + else if ( ecx == MSR_GS_BASE ) + __vmwrite(GUEST_GS_BASE, msr_content); + else { - HVM_DBG_LOG(DBG_LEVEL_1, "Not cano address of msr write\n"); - vmx_inject_hw_exception(v, TRAP_gp_fault, 0); - return 0; + v->arch.hvm_vmx.msr_state.shadow_gs = msr_content; + wrmsrl(MSR_SHADOW_GS_BASE, msr_content); } - if ( regs->ecx == MSR_FS_BASE ) - __vmwrite(GUEST_FS_BASE, msr_content); - else - __vmwrite(GUEST_GS_BASE, msr_content); - break; - case MSR_SHADOW_GS_BASE: - if ( !(vmx_long_mode_enabled(v)) ) - goto exit_and_crash; + case MSR_STAR: + WRITE_MSR(STAR); - v->arch.hvm_vmx.msr_state.shadow_gs = msr_content; - wrmsrl(MSR_SHADOW_GS_BASE, msr_content); - break; + case MSR_LSTAR: + if ( !is_canonical_address(msr_content) ) + goto uncanonical_address; + WRITE_MSR(LSTAR); - CASE_WRITE_MSR(STAR); - CASE_WRITE_MSR(LSTAR); - CASE_WRITE_MSR(CSTAR); - CASE_WRITE_MSR(SYSCALL_MASK); + case MSR_CSTAR: + if ( !is_canonical_address(msr_content) ) + goto uncanonical_address; + WRITE_MSR(CSTAR); + + case MSR_SYSCALL_MASK: + WRITE_MSR(SYSCALL_MASK); default: return 0; @@ -245,10 +250,11 @@ static inline int long_mode_do_msr_write(struct cpu_user_regs *regs) return 1; - exit_and_crash: - gdprintk(XENLOG_ERR, "Fatal error writing MSR %lx\n", (long)regs->ecx); - domain_crash(v->domain); - return 1; /* handled */ + uncanonical_address: + HVM_DBG_LOG(DBG_LEVEL_1, "Not cano address of msr write %x\n", ecx); + gp_fault: + vmx_inject_hw_exception(v, TRAP_gp_fault, 0); + return 0; } /* @@ -1283,6 +1289,32 @@ static void vmx_io_instruction(unsigned long exit_qualification, ASSERT(count); } } +#ifdef __x86_64__ + else + { + if ( !is_canonical_address(addr) || + !is_canonical_address(addr + size - 1) ) + { + vmx_inject_hw_exception(current, TRAP_gp_fault, 0); + return; + } + if ( count > (1UL << 48) / size ) + count = (1UL << 48) / size; + if ( !(regs->eflags & EF_DF) ) + { + if ( addr + count * size - 1 < addr || + !is_canonical_address(addr + count * size - 1) ) + count = (addr & ~((1UL << 48) - 1)) / size; + } + else + { + if ( (count - 1) * size > addr || + !is_canonical_address(addr + (count - 1) * size) ) + count = (addr & ~((1UL << 48) - 1)) / size + 1; + } + ASSERT(count); + } +#endif /* * Handle string pio instructions that cross pages or that diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c index d71704cbd2..450dcd8ba0 100644 --- a/xen/arch/x86/mm/shadow/common.c +++ b/xen/arch/x86/mm/shadow/common.c @@ -120,12 +120,17 @@ static int hvm_translate_linear_addr( */ addr = (uint32_t)(addr + dreg.base); } - else if ( (seg == x86_seg_fs) || (seg == x86_seg_gs) ) + else { /* - * LONG MODE: FS and GS add a segment base. + * LONG MODE: FS and GS add segment base. Addresses must be canonical. */ - addr += dreg.base; + + if ( (seg == x86_seg_fs) || (seg == x86_seg_gs) ) + addr += dreg.base; + + if ( !is_canonical_address(addr) ) + goto gpf; } *paddr = addr; diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index aafeecf3df..1d2cc8bfdc 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -157,11 +157,15 @@ hvm_paging_enabled(struct vcpu *v) return hvm_funcs.paging_enabled(v); } +#ifdef __x86_64__ static inline int hvm_long_mode_enabled(struct vcpu *v) { return hvm_funcs.long_mode_enabled(v); } +#else +#define hvm_long_mode_enabled(v) 0 +#endif static inline int hvm_pae_enabled(struct vcpu *v) diff --git a/xen/include/asm-x86/x86_32/page.h b/xen/include/asm-x86/x86_32/page.h index 228445a62d..74e36a1a40 100644 --- a/xen/include/asm-x86/x86_32/page.h +++ b/xen/include/asm-x86/x86_32/page.h @@ -7,6 +7,8 @@ #define VADDR_BITS 32 #define VADDR_MASK (~0UL) +#define is_canonical_address(x) 1 + #include #ifdef CONFIG_X86_PAE # include diff --git a/xen/include/asm-x86/x86_64/page.h b/xen/include/asm-x86/x86_64/page.h index 4e0f3675ce..e52097d73e 100644 --- a/xen/include/asm-x86/x86_64/page.h +++ b/xen/include/asm-x86/x86_64/page.h @@ -24,6 +24,8 @@ #define PADDR_MASK ((1UL << PADDR_BITS)-1) #define VADDR_MASK ((1UL << VADDR_BITS)-1) +#define is_canonical_address(x) (((long)(x) >> 47) == ((long)(x) >> 63)) + #ifndef __ASSEMBLY__ #include From bd8806fb71aebdab6289dd42cab8e84301d060ff Mon Sep 17 00:00:00 2001 From: Alastair Tse Date: Fri, 1 Dec 2006 11:07:22 +0000 Subject: [PATCH 6/8] [XEND] More fixes for HVM device configuration parsing Removed duplicated acpi option, added missing stdvga option. Do not output empty configurations in image sxp. Remove bug with vcpus passing in device model. Signed-off-by: Alastair Tse --- tools/python/xen/xend/XendConfig.py | 8 ++++---- tools/python/xen/xend/image.py | 5 ++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/python/xen/xend/XendConfig.py b/tools/python/xen/xend/XendConfig.py index 3bb0ba4cd9..846aebda02 100644 --- a/tools/python/xen/xend/XendConfig.py +++ b/tools/python/xen/xend/XendConfig.py @@ -185,7 +185,6 @@ LEGACY_IMAGE_HVM_CFG = [ ('vncconsole', int), ('pae', int), ('apic', int), - ('acpi', int), ] LEGACY_IMAGE_HVM_DEVICES_CFG = [ @@ -196,7 +195,8 @@ LEGACY_IMAGE_HVM_DEVICES_CFG = [ ('isa', str), ('keymap', str), ('localtime', str), - ('serial', str), + ('serial', str), + ('stdvga', int), ('soundhw', str), ('usb', str), ('usbdevice', str), @@ -987,12 +987,12 @@ class XendConfig(dict): if 'hvm' in self['image']: for arg, conv in LEGACY_IMAGE_HVM_CFG: - if self['image']['hvm'].has_key(arg): + if self['image']['hvm'].get(arg): image.append([arg, self['image']['hvm'][arg]]) if 'hvm' in self['image'] and 'devices' in self['image']['hvm']: for arg, conv in LEGACY_IMAGE_HVM_DEVICES_CFG: - if self['image']['hvm']['devices'].has_key(arg): + if self['image']['hvm']['devices'].get(arg): image.append([arg, self['image']['hvm']['devices'][arg]]) diff --git a/tools/python/xen/xend/image.py b/tools/python/xen/xend/image.py index 31194c7a73..8d2aab1836 100644 --- a/tools/python/xen/xend/image.py +++ b/tools/python/xen/xend/image.py @@ -270,8 +270,9 @@ class HVMImageHandler(ImageHandler): self.dmargs += self.configVNC(imageConfig) self.pae = imageConfig['hvm'].get('pae', 0) - self.acpi = imageConfig['hvm'].get('acpi', 0) self.apic = imageConfig['hvm'].get('apic', 0) + self.acpi = imageConfig['hvm']['devices'].get('acpi', 0) + def buildDomain(self): store_evtchn = self.vm.getStorePort() @@ -310,8 +311,6 @@ class HVMImageHandler(ImageHandler): for a in dmargs: v = hvmDeviceConfig.get(a) - if a == 'vcpus': - v = hvmDeviceConfig.get('vcpus_number') # python doesn't allow '-' in variable names if a == 'stdvga': a = 'std-vga' From eb1eb71059074995a758aac068aec0aa0905016d Mon Sep 17 00:00:00 2001 From: Ewan Mellor Date: Fri, 1 Dec 2006 11:32:32 +0000 Subject: [PATCH 7/8] Tidy up the creation of directories that Xend needs. This avoids potential races in this creation. Signed-off-by: Ewan Mellor --- tools/python/xen/util/mkdir.py | 44 +++++++++++++++++++ tools/python/xen/util/xmlrpclib2.py | 13 +++--- tools/python/xen/web/unix.py | 12 +++-- tools/python/xen/xend/XendDomain.py | 37 ++++++---------- tools/python/xen/xend/XendLogging.py | 5 ++- .../python/xen/xend/XendStorageRepository.py | 9 ++-- tools/python/xen/xend/server/SrvDaemon.py | 4 +- 7 files changed, 77 insertions(+), 47 deletions(-) create mode 100644 tools/python/xen/util/mkdir.py diff --git a/tools/python/xen/util/mkdir.py b/tools/python/xen/util/mkdir.py new file mode 100644 index 0000000000..121e498a23 --- /dev/null +++ b/tools/python/xen/util/mkdir.py @@ -0,0 +1,44 @@ +#============================================================================ +# This library is free software; you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library; if not, write to the Free Software +# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +#============================================================================ +# Copyright (c) 2006 XenSource Inc. +#============================================================================ + +import errno +import os +import os.path +import stat + + +def parents(dir, perms, enforcePermissions = False): + """ + Ensure that the given directory exists, creating it if necessary, but not + complaining if it's already there. + + @param dir The directory name. + @param perms One of the stat.S_ constants. + @param enforcePermissions Enforce our ownership and the given permissions, + even if the directory pre-existed with different ones. + """ + # Catch the exception here, rather than checking for the directory's + # existence first, to avoid races. + try: + os.makedirs(dir, perms) + except OSError, exn: + if exn.args[0] != errno.EEXIST or not os.path.isdir(dir): + raise + if enforcePermissions: + os.chown(dir, os.geteuid(), os.getegid()) + os.chmod(dir, stat.S_IRWXU) diff --git a/tools/python/xen/util/xmlrpclib2.py b/tools/python/xen/util/xmlrpclib2.py index 3a3973f838..efb29d7b0e 100644 --- a/tools/python/xen/util/xmlrpclib2.py +++ b/tools/python/xen/util/xmlrpclib2.py @@ -30,6 +30,8 @@ from SimpleXMLRPCServer import SimpleXMLRPCServer, SimpleXMLRPCRequestHandler import SocketServer import xmlrpclib, socket, os, stat +import mkdir + from xen.web import connection from xen.xend.XendLogging import log @@ -234,14 +236,9 @@ class UnixXMLRPCServer(TCPXMLRPCServer): address_family = socket.AF_UNIX def __init__(self, addr, allowed, logRequests = 1): - parent = os.path.dirname(addr) - if os.path.exists(parent): - os.chown(parent, os.geteuid(), os.getegid()) - os.chmod(parent, stat.S_IRWXU) - if self.allow_reuse_address and os.path.exists(addr): - os.unlink(addr) - else: - os.makedirs(parent, stat.S_IRWXU) + mkdir.parents(os.path.dirname(addr), stat.S_IRWXU, True) + if self.allow_reuse_address and os.path.exists(addr): + os.unlink(addr) TCPXMLRPCServer.__init__(self, addr, allowed, UnixXMLRPCRequestHandler, logRequests) diff --git a/tools/python/xen/web/unix.py b/tools/python/xen/web/unix.py index 8ec715c12f..12b6e9694e 100644 --- a/tools/python/xen/web/unix.py +++ b/tools/python/xen/web/unix.py @@ -22,6 +22,8 @@ import os.path import socket import stat +from xen.util import mkdir + import connection @@ -30,13 +32,9 @@ def bind(path): created such that only the current user may access it.""" parent = os.path.dirname(path) - if os.path.exists(parent): - os.chown(parent, os.geteuid(), os.getegid()) - os.chmod(parent, stat.S_IRWXU) - if os.path.exists(path): - os.unlink(path) - else: - os.makedirs(parent, stat.S_IRWXU) + mkdir.parents(parent, stat.S_IRWXU, True) + if os.path.exists(path): + os.unlink(path) sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) sock.bind(path) diff --git a/tools/python/xen/xend/XendDomain.py b/tools/python/xen/xend/XendDomain.py index 9a4d112e95..bb14e70667 100644 --- a/tools/python/xen/xend/XendDomain.py +++ b/tools/python/xen/xend/XendDomain.py @@ -23,6 +23,7 @@ """ import os +import stat import shutil import socket import threading @@ -44,7 +45,7 @@ from xen.xend.XendDevices import XendDevices from xen.xend.xenstore.xstransact import xstransact from xen.xend.xenstore.xswatch import xswatch -from xen.util import security +from xen.util import mkdir, security from xen.xend import uuid xc = xen.lowlevel.xc.xc() @@ -99,11 +100,7 @@ class XendDomain: """Singleton initialisation function.""" dom_path = self._managed_path() - try: - os.stat(dom_path) - except OSError: - log.info("Making %s", dom_path) - os.makedirs(dom_path, 0755) + mkdir.parents(dom_path, stat.S_IRWXU) xstransact.Mkdir(XS_VMROOT) xstransact.SetPermissions(XS_VMROOT, {'dom': DOM0_ID}) @@ -271,25 +268,17 @@ class XendDomain: domains_dir = self._managed_path() dom_uuid = dominfo.get_uuid() domain_config_dir = self._managed_path(dom_uuid) - - # make sure the domain dir exists - if not os.path.exists(domains_dir): - os.makedirs(domains_dir, 0755) - elif not os.path.isdir(domains_dir): - log.error("xend_domain_dir is not a directory.") - raise XendError("Unable to save managed configuration " - "because %s is not a directory." % - domains_dir) - - if not os.path.exists(domain_config_dir): + + def make_or_raise(path): try: - os.makedirs(domain_config_dir, 0755) - except IOError: - log.exception("Failed to create directory: %s" % - domain_config_dir) - raise XendError("Failed to create directory: %s" % - domain_config_dir) - + mkdir.parents(path, stat.S_IRWXU) + except: + log.exception("%s could not be created." % path) + raise XendError("%s could not be created." % path) + + make_or_raise(domains_dir) + make_or_raise(domain_config_dir) + try: sxp_cache_file = open(self._managed_config_path(dom_uuid),'w') prettyprint(dominfo.sxpr(), sxp_cache_file, width = 78) diff --git a/tools/python/xen/xend/XendLogging.py b/tools/python/xen/xend/XendLogging.py index a3d6de86d1..32cd13c5db 100644 --- a/tools/python/xen/xend/XendLogging.py +++ b/tools/python/xen/xend/XendLogging.py @@ -16,13 +16,15 @@ # Copyright (C) 2005, 2006 XenSource Ltd. #============================================================================ - +import os +import stat import tempfile import types import logging import logging.handlers import fcntl +from xen.util import mkdir from xen.xend.server import params @@ -80,6 +82,7 @@ def init(filename, level): global logfilename def openFileHandler(fname): + mkdir.parents(os.path.dirname(fname), stat.S_IRWXU) return XendRotatingFileHandler(fname, mode = 'a', maxBytes = MAX_BYTES, backupCount = BACKUP_COUNT) diff --git a/tools/python/xen/xend/XendStorageRepository.py b/tools/python/xen/xend/XendStorageRepository.py index d906f5b5fd..a16a293191 100644 --- a/tools/python/xen/xend/XendStorageRepository.py +++ b/tools/python/xen/xend/XendStorageRepository.py @@ -19,10 +19,12 @@ # The default QCOW Xen API Storage Repository # -import os import commands +import os +import stat import threading +from xen.util import mkdir from xen.xend import uuid from xen.xend.XendError import XendError from xen.xend.XendVDI import * @@ -98,10 +100,7 @@ class XendStorageRepository: """ self.lock.acquire() try: - # create directory if /var/lib/xend/storage does not exist - if not os.path.exists(XEND_STORAGE_DIR): - os.makedirs(XEND_STORAGE_DIR) - os.chmod(XEND_STORAGE_DIR, 0700) + mkdir.parents(XEND_STORAGE_DIR, stat.S_IRWXU) # scan the directory and populate self.images total_used = 0 diff --git a/tools/python/xen/xend/server/SrvDaemon.py b/tools/python/xen/xend/server/SrvDaemon.py index 7bfa466dff..35c64869d4 100644 --- a/tools/python/xen/xend/server/SrvDaemon.py +++ b/tools/python/xen/xend/server/SrvDaemon.py @@ -21,6 +21,7 @@ import xen.lowlevel.xc from xen.xend.XendLogging import log from xen.xend import osdep +from xen.util import mkdir import relocate import SrvServer @@ -108,8 +109,7 @@ class Daemon: # so _before_ we close stderr. try: parent = os.path.dirname(XEND_DEBUG_LOG) - if not os.path.exists(parent): - os.makedirs(parent, stat.S_IRWXU) + mkdir.parents(parent, stat.S_IRWXU) fd = os.open(XEND_DEBUG_LOG, os.O_WRONLY|os.O_CREAT|os.O_APPEND) except Exception, exn: print >>sys.stderr, exn From 43bcc64d9992847e69c9b65c290d5bf6d01f1753 Mon Sep 17 00:00:00 2001 From: "kfraser@localhost.localdomain" Date: Fri, 1 Dec 2006 11:54:53 +0000 Subject: [PATCH 8/8] [XEN] Quieten down HVM debug output. Signed-off-by: Keir Fraser --- xen/arch/x86/hvm/vlapic.c | 2 +- xen/arch/x86/x86_emulate.c | 40 +++++++++++--------------------------- 2 files changed, 12 insertions(+), 30 deletions(-) diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 60e6171c47..a0304deb37 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -723,7 +723,7 @@ static void vlapic_write(struct vcpu *v, unsigned long address, break; default: - gdprintk(XENLOG_WARNING, + gdprintk(XENLOG_DEBUG, "Local APIC Write to read-only register 0x%x\n", offset); break; } diff --git a/xen/arch/x86/x86_emulate.c b/xen/arch/x86/x86_emulate.c index 555655f83a..e5f6a6b503 100644 --- a/xen/arch/x86/x86_emulate.c +++ b/xen/arch/x86/x86_emulate.c @@ -7,16 +7,14 @@ */ #ifndef __XEN__ -#include +#include #include #include -#define dprintf(_f, _a...) printf( _f , ## _a ) #else #include #include #include #include -#define dprintf(_f, _a...) gdprintk(XENLOG_WARNING, _f , ## _a ) #undef cmpxchg #endif #include @@ -440,27 +438,6 @@ decode_register( return p; } -static void -dump_instr( - struct x86_emulate_ctxt *ctxt, - struct x86_emulate_ops *ops) -{ -#ifdef __XEN__ - int i; - unsigned long x, eip = ctxt->regs->eip; - - dprintf("Instr:"); - for ( i = 0; i < 16; i++, eip++ ) - { - if ( ops->read(x86_seg_cs, eip, &x, 1, ctxt) != 0 ) - printk(" ??"); - else - printk(" %02x", (uint8_t)x); - } - printk("\n"); -#endif -} - int x86_emulate_memop( struct x86_emulate_ctxt *ctxt, @@ -579,10 +556,7 @@ x86_emulate_memop( modrm_rm = modrm & 0x07; if ( modrm_mod == 3 ) - { - dprintf("Cannot parse ModRM.mod == 3.\n"); goto cannot_emulate; - } if ( ad_bytes == 2 ) { @@ -1206,7 +1180,15 @@ x86_emulate_memop( goto writeback; cannot_emulate: - dprintf("Cannot emulate %02x\n", b); - dump_instr(ctxt, ops); +#ifdef __XEN__ + gdprintk(XENLOG_DEBUG, "Instr:"); + for ( ea_off = ctxt->regs->eip; ea_off < _regs.eip; ea_off++ ) + { + unsigned long x; + ops->read(x86_seg_cs, ea_off, &x, 1, ctxt); + printk(" %02x", (uint8_t)x); + } + printk("\n"); +#endif return -1; }