* [PATCH 0/2] Deal with shadow interrupts after emulated instructions @ 2009-05-08 20:23 Glauber Costa 2009-05-08 20:23 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa 2009-05-10 8:15 ` [PATCH 0/2] Deal with shadow interrupts after emulated instructions Gleb Natapov 0 siblings, 2 replies; 9+ messages in thread From: Glauber Costa @ 2009-05-08 20:23 UTC (permalink / raw) To: kvm; +Cc: linux-kernel, avi Hi, This is the same patch you're all used to by now, but split in two per Avi request. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow 2009-05-08 20:23 [PATCH 0/2] Deal with shadow interrupts after emulated instructions Glauber Costa @ 2009-05-08 20:23 ` Glauber Costa 2009-05-08 20:23 ` [PATCH 2/2] deal with interrupt shadow state for emulated instruction Glauber Costa 2009-05-10 13:07 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Avi Kivity 2009-05-10 8:15 ` [PATCH 0/2] Deal with shadow interrupts after emulated instructions Gleb Natapov 1 sibling, 2 replies; 9+ messages in thread From: Glauber Costa @ 2009-05-08 20:23 UTC (permalink / raw) To: kvm; +Cc: linux-kernel, avi, H. Peter Anvin, Gleb Natapov This patch replaces drop_interrupt_shadow with the more general set_interrupt_shadow, that can either drop or raise it, depending on its parameter. Signed-off-by: Glauber Costa <glommer@redhat.com> CC: H. Peter Anvin <hpa@zytor.com> CC: Avi Kivity <avi@redhat.com> CC: Gleb Natapov <gleb@redhat.com> --- arch/x86/include/asm/kvm_host.h | 3 +- arch/x86/include/asm/kvm_x86_emulate.h | 3 ++ arch/x86/kvm/svm.c | 32 +++++++++++++---- arch/x86/kvm/vmx.c | 56 ++++++++++++++++++++++--------- arch/x86/kvm/x86.c | 2 +- 5 files changed, 70 insertions(+), 26 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 8e680c3..b63a3b2 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -510,6 +510,8 @@ struct kvm_x86_ops { void (*run)(struct kvm_vcpu *vcpu, struct kvm_run *run); int (*handle_exit)(struct kvm_run *run, struct kvm_vcpu *vcpu); void (*skip_emulated_instruction)(struct kvm_vcpu *vcpu); + void (*set_interrupt_shadow)(struct kvm_vcpu *vcpu, int mask); + u32 (*get_interrupt_shadow)(struct kvm_vcpu *vcpu); void (*patch_hypercall)(struct kvm_vcpu *vcpu, unsigned char *hypercall_addr); void (*set_irq)(struct kvm_vcpu *vcpu, int vec); @@ -521,7 +523,6 @@ struct kvm_x86_ops { void (*enable_nmi_window)(struct kvm_vcpu *vcpu); void (*enable_irq_window)(struct kvm_vcpu *vcpu); void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr); - void (*drop_interrupt_shadow)(struct kvm_vcpu *vcpu); int (*set_tss_addr)(struct kvm *kvm, unsigned int addr); int (*get_tdp_level)(void); u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio); diff --git a/arch/x86/include/asm/kvm_x86_emulate.h b/arch/x86/include/asm/kvm_x86_emulate.h index 6a15973..be40d6e 100644 --- a/arch/x86/include/asm/kvm_x86_emulate.h +++ b/arch/x86/include/asm/kvm_x86_emulate.h @@ -143,6 +143,9 @@ struct decode_cache { struct fetch_cache fetch; }; +#define X86_SHADOW_INT_MOV_SS 1 +#define X86_SHADOW_INT_STI 2 + struct x86_emulate_ctxt { /* Register state before/after emulation. */ struct kvm_vcpu *vcpu; diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index ef43a18..a9669ff 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -202,6 +202,27 @@ static int is_external_interrupt(u32 info) return info == (SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_INTR); } +static u32 svm_get_interrupt_shadow(struct kvm_vcpu *vcpu) +{ + struct vcpu_svm *svm = to_svm(vcpu); + u32 ret = 0; + + if (svm->vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) + ret |= (X86_SHADOW_INT_STI && X86_SHADOW_INT_MOV_SS); + return ret; +} + +static void svm_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) +{ + struct vcpu_svm *svm = to_svm(vcpu); + + if (mask == 0) + svm->vmcb->control.int_state &= ~SVM_INTERRUPT_SHADOW_MASK; + else + svm->vmcb->control.int_state |= SVM_INTERRUPT_SHADOW_MASK; + +} + static void skip_emulated_instruction(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); @@ -215,7 +236,7 @@ static void skip_emulated_instruction(struct kvm_vcpu *vcpu) __func__, kvm_rip_read(vcpu), svm->next_rip); kvm_rip_write(vcpu, svm->next_rip); - svm->vmcb->control.int_state &= ~SVM_INTERRUPT_SHADOW_MASK; + svm_set_interrupt_shadow(vcpu, 0); } static int has_svm(void) @@ -2229,12 +2250,6 @@ static void pre_svm_run(struct vcpu_svm *svm) new_asid(svm, svm_data); } -static void svm_drop_interrupt_shadow(struct kvm_vcpu *vcpu) -{ - struct vcpu_svm *svm = to_svm(vcpu); - svm->vmcb->control.int_state &= ~SVM_INTERRUPT_SHADOW_MASK; -} - static void svm_inject_nmi(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); @@ -2637,6 +2652,8 @@ static struct kvm_x86_ops svm_x86_ops = { .run = svm_vcpu_run, .handle_exit = handle_exit, .skip_emulated_instruction = skip_emulated_instruction, + .set_interrupt_shadow= svm_set_interrupt_shadow, + .get_interrupt_shadow = svm_get_interrupt_shadow, .patch_hypercall = svm_patch_hypercall, .set_irq = svm_set_irq, .set_nmi = svm_inject_nmi, @@ -2646,7 +2663,6 @@ static struct kvm_x86_ops svm_x86_ops = { .enable_nmi_window = enable_nmi_window, .enable_irq_window = enable_irq_window, .update_cr8_intercept = update_cr8_intercept, - .drop_interrupt_shadow = svm_drop_interrupt_shadow, .set_tss_addr = svm_set_tss_addr, .get_tdp_level = get_npt_level, diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index e8a5649..6c805c6 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -736,23 +736,52 @@ static void vmx_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags) vmcs_writel(GUEST_RFLAGS, rflags); } +static u32 vmx_get_interrupt_shadow(struct kvm_vcpu *vcpu) +{ + u32 interruptibility = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); + int ret = 0; + + if (interruptibility & GUEST_INTR_STATE_STI) + ret |= X86_SHADOW_INT_STI; + if (interruptibility & GUEST_INTR_STATE_MOV_SS) + ret |= X86_SHADOW_INT_MOV_SS; + + return ret; +} + +static void vmx_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) +{ + u32 interruptibility_old = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); + u32 interruptibility = interruptibility_old; + + switch (mask) { + case 0: + interruptibility &= ~((GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS)); + break; + case X86_SHADOW_INT_MOV_SS: + interruptibility |= GUEST_INTR_STATE_MOV_SS; + break; + case X86_SHADOW_INT_STI: + interruptibility |= GUEST_INTR_STATE_STI; + break; + default: + printk(KERN_ERR "Bogus mask for interrupt shadow!\n"); + } + + if ((interruptibility != interruptibility_old)) + vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, interruptibility); +} + static void skip_emulated_instruction(struct kvm_vcpu *vcpu) { unsigned long rip; - u32 interruptibility; rip = kvm_rip_read(vcpu); rip += vmcs_read32(VM_EXIT_INSTRUCTION_LEN); kvm_rip_write(vcpu, rip); - /* - * We emulated an instruction, so temporary interrupt blocking - * should be removed, if set. - */ - interruptibility = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); - if (interruptibility & 3) - vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, - interruptibility & ~3); + /* skipping an emulated instruction also counts */ + vmx_set_interrupt_shadow(vcpu, 0); } static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr, @@ -2400,12 +2429,6 @@ out: return ret; } -void vmx_drop_interrupt_shadow(struct kvm_vcpu *vcpu) -{ - vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO, - GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS); -} - static void enable_irq_window(struct kvm_vcpu *vcpu) { u32 cpu_based_vm_exec_control; @@ -3649,6 +3672,8 @@ static struct kvm_x86_ops vmx_x86_ops = { .run = vmx_vcpu_run, .handle_exit = vmx_handle_exit, .skip_emulated_instruction = skip_emulated_instruction, + .set_interrupt_shadow = vmx_set_interrupt_shadow, + .get_interrupt_shadow = vmx_get_interrupt_shadow, .patch_hypercall = vmx_patch_hypercall, .set_irq = vmx_inject_irq, .set_nmi = vmx_inject_nmi, @@ -3658,7 +3683,6 @@ static struct kvm_x86_ops vmx_x86_ops = { .enable_nmi_window = enable_nmi_window, .enable_irq_window = enable_irq_window, .update_cr8_intercept = update_cr8_intercept, - .drop_interrupt_shadow = vmx_drop_interrupt_shadow, .set_tss_addr = vmx_set_tss_addr, .get_tdp_level = get_ept_level, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2d7082c..3d8fcc5 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3161,7 +3161,7 @@ static void inject_pending_irq(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) kvm_run->request_interrupt_window; if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) - kvm_x86_ops->drop_interrupt_shadow(vcpu); + kvm_x86_ops->set_interrupt_shadow(vcpu, 0); inject_irq(vcpu); -- 1.5.6.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] deal with interrupt shadow state for emulated instruction 2009-05-08 20:23 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa @ 2009-05-08 20:23 ` Glauber Costa 2009-05-10 13:09 ` Avi Kivity 2009-05-10 13:07 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Avi Kivity 1 sibling, 1 reply; 9+ messages in thread From: Glauber Costa @ 2009-05-08 20:23 UTC (permalink / raw) To: kvm; +Cc: linux-kernel, avi, H. Peter Anvin, Gleb Natapov we currently unblock shadow interrupt state when we skip an instruction, but failing to do so when we actually emulate one. This blocks interrupts in key instruction blocks, in particular sti; hlt; sequences If the instruction emulated is an sti, we have to block shadow interrupts. The same goes for mov ss. pop ss also needs it, but we don't currently emulate it. Without this patch, I cannot boot gpxe option roms at vmx machines. This is described at https://bugzilla.redhat.com/show_bug.cgi?id=494469 Signed-off-by: Glauber Costa <glommer@redhat.com> CC: H. Peter Anvin <hpa@zytor.com> CC: Avi Kivity <avi@redhat.com> CC: Gleb Natapov <gleb@redhat.com> --- arch/x86/include/asm/kvm_x86_emulate.h | 3 +++ arch/x86/kvm/x86.c | 7 ++++++- arch/x86/kvm/x86_emulate.c | 21 ++++++++++++++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/kvm_x86_emulate.h b/arch/x86/include/asm/kvm_x86_emulate.h index be40d6e..b7ed2c4 100644 --- a/arch/x86/include/asm/kvm_x86_emulate.h +++ b/arch/x86/include/asm/kvm_x86_emulate.h @@ -155,6 +155,9 @@ struct x86_emulate_ctxt { int mode; u32 cs_base; + /* interruptibility state, as a result of execution of STI or MOV SS */ + int interruptibility; + /* decode cache */ struct decode_cache decode; }; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 3d8fcc5..c456aa5 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2362,7 +2362,7 @@ int emulate_instruction(struct kvm_vcpu *vcpu, u16 error_code, int emulation_type) { - int r; + int r, shadow_mask; struct decode_cache *c; kvm_clear_exception_queue(vcpu); @@ -2415,7 +2415,12 @@ int emulate_instruction(struct kvm_vcpu *vcpu, return EMULATE_DONE; } + vcpu->arch.emulate_ctxt.interruptibility = 0; r = x86_emulate_insn(&vcpu->arch.emulate_ctxt, &emulate_ops); + shadow_mask = vcpu->arch.emulate_ctxt.interruptibility; + + if (r == 0) + kvm_x86_ops->set_interrupt_shadow(vcpu, shadow_mask); if (vcpu->arch.pio.string) return EMULATE_DO_MMIO; diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c index d2664fc..1d042d4 100644 --- a/arch/x86/kvm/x86_emulate.c +++ b/arch/x86/kvm/x86_emulate.c @@ -1618,6 +1618,14 @@ special_insn: int err; sel = c->src.val; + if (c->modrm_reg == VCPU_SREG_SS) { + u32 int_shadow = + kvm_x86_ops->get_interrupt_shadow(ctxt->vcpu); + /* See sti emulation for an explanation of this */ + if (!(int_shadow & X86_SHADOW_INT_MOV_SS)) + ctxt->interruptibility = X86_SHADOW_INT_MOV_SS; + } + if (c->modrm_reg <= 5) { type_bits = (c->modrm_reg == 1) ? 9 : 1; err = kvm_load_segment_descriptor(ctxt->vcpu, sel, @@ -1846,10 +1854,21 @@ special_insn: ctxt->eflags &= ~X86_EFLAGS_IF; c->dst.type = OP_NONE; /* Disable writeback. */ break; - case 0xfb: /* sti */ + case 0xfb: { /* sti */ + u32 int_shadow = kvm_x86_ops->get_interrupt_shadow(ctxt->vcpu); + /* + * an sti; sti; sequence only disable interrupts for the first + * instruction. So, if the last instruction, be it emulated or + * not, left the system with the INT_STI flag enabled, it + * means that the last instruction is an sti. We should not + * leave the flag on in this case + */ + if (!(int_shadow & X86_SHADOW_INT_STI)) + ctxt->interruptibility = X86_SHADOW_INT_STI; ctxt->eflags |= X86_EFLAGS_IF; c->dst.type = OP_NONE; /* Disable writeback. */ break; + } case 0xfc: /* cld */ ctxt->eflags &= ~EFLG_DF; c->dst.type = OP_NONE; /* Disable writeback. */ -- 1.5.6.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] deal with interrupt shadow state for emulated instruction 2009-05-08 20:23 ` [PATCH 2/2] deal with interrupt shadow state for emulated instruction Glauber Costa @ 2009-05-10 13:09 ` Avi Kivity 0 siblings, 0 replies; 9+ messages in thread From: Avi Kivity @ 2009-05-10 13:09 UTC (permalink / raw) To: Glauber Costa; +Cc: kvm, linux-kernel, H. Peter Anvin, Gleb Natapov Glauber Costa wrote: > we currently unblock shadow interrupt state when we skip an instruction, > but failing to do so when we actually emulate one. This blocks interrupts > in key instruction blocks, in particular sti; hlt; sequences > > If the instruction emulated is an sti, we have to block shadow interrupts. > The same goes for mov ss. pop ss also needs it, but we don't currently > emulate it. > > Without this patch, I cannot boot gpxe option roms at vmx machines. > This is described at https://bugzilla.redhat.com/show_bug.cgi?id=494469 > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 3d8fcc5..c456aa5 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -2362,7 +2362,7 @@ int emulate_instruction(struct kvm_vcpu *vcpu, > u16 error_code, > int emulation_type) > { > - int r; > + int r, shadow_mask; > struct decode_cache *c; > > kvm_clear_exception_queue(vcpu); > @@ -2415,7 +2415,12 @@ int emulate_instruction(struct kvm_vcpu *vcpu, > return EMULATE_DONE; > } > > + vcpu->arch.emulate_ctxt.interruptibility = 0; > If the instruction is sti or mov ss, this is ouput only. Otherwise, it is an input/output parameter. To reduce confusion suggest moving this statement to x86_emulate_insn, so it can be an output-only parameter. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow 2009-05-08 20:23 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa 2009-05-08 20:23 ` [PATCH 2/2] deal with interrupt shadow state for emulated instruction Glauber Costa @ 2009-05-10 13:07 ` Avi Kivity 1 sibling, 0 replies; 9+ messages in thread From: Avi Kivity @ 2009-05-10 13:07 UTC (permalink / raw) To: Glauber Costa; +Cc: kvm, linux-kernel, H. Peter Anvin, Gleb Natapov Glauber Costa wrote: > This patch replaces drop_interrupt_shadow with the more > general set_interrupt_shadow, that can either drop or raise > it, depending on its parameter. > > void (*skip_emulated_instruction)(struct kvm_vcpu *vcpu); > + void (*set_interrupt_shadow)(struct kvm_vcpu *vcpu, int mask); > + u32 (*get_interrupt_shadow)(struct kvm_vcpu *vcpu); > Would be better to make these symmetric in their mask types. > +++ b/arch/x86/kvm/svm.c > @@ -202,6 +202,27 @@ static int is_external_interrupt(u32 info) > return info == (SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_INTR); > } > > +static u32 svm_get_interrupt_shadow(struct kvm_vcpu *vcpu) > +{ > + struct vcpu_svm *svm = to_svm(vcpu); > + u32 ret = 0; > + > + if (svm->vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) > + ret |= (X86_SHADOW_INT_STI && X86_SHADOW_INT_MOV_SS); > && -> |. Parentheses unnecessary. > { > struct vcpu_svm *svm = to_svm(vcpu); > @@ -2637,6 +2652,8 @@ static struct kvm_x86_ops svm_x86_ops = { > .run = svm_vcpu_run, > .handle_exit = handle_exit, > .skip_emulated_instruction = skip_emulated_instruction, > + .set_interrupt_shadow= svm_set_interrupt_shadow, > Missing space. > +static void vmx_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) > +{ > + u32 interruptibility_old = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); > + u32 interruptibility = interruptibility_old; > + > + switch (mask) { > + case 0: > + interruptibility &= ~((GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS)); > + break; > Need to clear this unconditionally, otherwise 'sti; mov ss' will set both flags. > + case X86_SHADOW_INT_MOV_SS: > + interruptibility |= GUEST_INTR_STATE_MOV_SS; > + break; > + case X86_SHADOW_INT_STI: > + interruptibility |= GUEST_INTR_STATE_STI; > + break; > + default: > + printk(KERN_ERR "Bogus mask for interrupt shadow!\n"); > + } > I suggest we deal with the case where both are set. One day we'll live migrate this state, and if we come from an AMD host, we might get this state. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Deal with shadow interrupts after emulated instructions 2009-05-08 20:23 [PATCH 0/2] Deal with shadow interrupts after emulated instructions Glauber Costa 2009-05-08 20:23 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa @ 2009-05-10 8:15 ` Gleb Natapov 1 sibling, 0 replies; 9+ messages in thread From: Gleb Natapov @ 2009-05-10 8:15 UTC (permalink / raw) To: Glauber Costa; +Cc: kvm, linux-kernel, avi On Fri, May 08, 2009 at 04:23:05PM -0400, Glauber Costa wrote: > Hi, > > This is the same patch you're all used to by now, but split in two per > Avi request. > > Acked-by: Gleb Natapov <gleb@redhat.com> -- Gleb. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/2] Deal with shadow interrupts after emulated instructions @ 2009-05-12 15:13 Glauber Costa 2009-05-12 15:13 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa 0 siblings, 1 reply; 9+ messages in thread From: Glauber Costa @ 2009-05-12 15:13 UTC (permalink / raw) To: kvm; +Cc: linux-kernel, avi Same as before, addressing avi's comments ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow 2009-05-12 15:13 Glauber Costa @ 2009-05-12 15:13 ` Glauber Costa 2009-05-12 15:13 ` [PATCH 2/2] deal with interrupt shadow state for emulated instruction Glauber Costa 0 siblings, 1 reply; 9+ messages in thread From: Glauber Costa @ 2009-05-12 15:13 UTC (permalink / raw) To: kvm; +Cc: linux-kernel, avi, H. Peter Anvin, Gleb Natapov This patch replaces drop_interrupt_shadow with the more general set_interrupt_shadow, that can either drop or raise it, depending on its parameter. Signed-off-by: Glauber Costa <glommer@redhat.com> CC: H. Peter Anvin <hpa@zytor.com> CC: Avi Kivity <avi@redhat.com> CC: Gleb Natapov <gleb@redhat.com> --- arch/x86/include/asm/kvm_host.h | 3 +- arch/x86/include/asm/kvm_x86_emulate.h | 3 ++ arch/x86/kvm/svm.c | 32 +++++++++++++++----- arch/x86/kvm/vmx.c | 49 +++++++++++++++++++++---------- arch/x86/kvm/x86.c | 2 +- 5 files changed, 63 insertions(+), 26 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 8e680c3..3d933cf 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -510,6 +510,8 @@ struct kvm_x86_ops { void (*run)(struct kvm_vcpu *vcpu, struct kvm_run *run); int (*handle_exit)(struct kvm_run *run, struct kvm_vcpu *vcpu); void (*skip_emulated_instruction)(struct kvm_vcpu *vcpu); + void (*set_interrupt_shadow)(struct kvm_vcpu *vcpu, int mask); + u32 (*get_interrupt_shadow)(struct kvm_vcpu *vcpu, int mask); void (*patch_hypercall)(struct kvm_vcpu *vcpu, unsigned char *hypercall_addr); void (*set_irq)(struct kvm_vcpu *vcpu, int vec); @@ -521,7 +523,6 @@ struct kvm_x86_ops { void (*enable_nmi_window)(struct kvm_vcpu *vcpu); void (*enable_irq_window)(struct kvm_vcpu *vcpu); void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr); - void (*drop_interrupt_shadow)(struct kvm_vcpu *vcpu); int (*set_tss_addr)(struct kvm *kvm, unsigned int addr); int (*get_tdp_level)(void); u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio); diff --git a/arch/x86/include/asm/kvm_x86_emulate.h b/arch/x86/include/asm/kvm_x86_emulate.h index 6a15973..be40d6e 100644 --- a/arch/x86/include/asm/kvm_x86_emulate.h +++ b/arch/x86/include/asm/kvm_x86_emulate.h @@ -143,6 +143,9 @@ struct decode_cache { struct fetch_cache fetch; }; +#define X86_SHADOW_INT_MOV_SS 1 +#define X86_SHADOW_INT_STI 2 + struct x86_emulate_ctxt { /* Register state before/after emulation. */ struct kvm_vcpu *vcpu; diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index ef43a18..ae29a95 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -202,6 +202,27 @@ static int is_external_interrupt(u32 info) return info == (SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_INTR); } +static u32 svm_get_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) +{ + struct vcpu_svm *svm = to_svm(vcpu); + u32 ret = 0; + + if (svm->vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) + ret |= X86_SHADOW_INT_STI && X86_SHADOW_INT_MOV_SS; + return ret & mask; +} + +static void svm_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) +{ + struct vcpu_svm *svm = to_svm(vcpu); + + if (mask == 0) + svm->vmcb->control.int_state &= ~SVM_INTERRUPT_SHADOW_MASK; + else + svm->vmcb->control.int_state |= SVM_INTERRUPT_SHADOW_MASK; + +} + static void skip_emulated_instruction(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); @@ -215,7 +236,7 @@ static void skip_emulated_instruction(struct kvm_vcpu *vcpu) __func__, kvm_rip_read(vcpu), svm->next_rip); kvm_rip_write(vcpu, svm->next_rip); - svm->vmcb->control.int_state &= ~SVM_INTERRUPT_SHADOW_MASK; + svm_set_interrupt_shadow(vcpu, 0); } static int has_svm(void) @@ -2229,12 +2250,6 @@ static void pre_svm_run(struct vcpu_svm *svm) new_asid(svm, svm_data); } -static void svm_drop_interrupt_shadow(struct kvm_vcpu *vcpu) -{ - struct vcpu_svm *svm = to_svm(vcpu); - svm->vmcb->control.int_state &= ~SVM_INTERRUPT_SHADOW_MASK; -} - static void svm_inject_nmi(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); @@ -2637,6 +2652,8 @@ static struct kvm_x86_ops svm_x86_ops = { .run = svm_vcpu_run, .handle_exit = handle_exit, .skip_emulated_instruction = skip_emulated_instruction, + .set_interrupt_shadow = svm_set_interrupt_shadow, + .get_interrupt_shadow = svm_get_interrupt_shadow, .patch_hypercall = svm_patch_hypercall, .set_irq = svm_set_irq, .set_nmi = svm_inject_nmi, @@ -2646,7 +2663,6 @@ static struct kvm_x86_ops svm_x86_ops = { .enable_nmi_window = enable_nmi_window, .enable_irq_window = enable_irq_window, .update_cr8_intercept = update_cr8_intercept, - .drop_interrupt_shadow = svm_drop_interrupt_shadow, .set_tss_addr = svm_set_tss_addr, .get_tdp_level = get_npt_level, diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index e8a5649..c28baf8 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -736,23 +736,45 @@ static void vmx_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags) vmcs_writel(GUEST_RFLAGS, rflags); } +static u32 vmx_get_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) +{ + u32 interruptibility = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); + int ret = 0; + + if (interruptibility & GUEST_INTR_STATE_STI) + ret |= X86_SHADOW_INT_STI; + if (interruptibility & GUEST_INTR_STATE_MOV_SS) + ret |= X86_SHADOW_INT_MOV_SS; + + return ret & mask; +} + +static void vmx_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) +{ + u32 interruptibility_old = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); + u32 interruptibility = interruptibility_old; + + interruptibility &= ~((GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS)); + + if (mask & X86_SHADOW_INT_MOV_SS) + interruptibility |= GUEST_INTR_STATE_MOV_SS; + if (mask & X86_SHADOW_INT_STI) + interruptibility |= GUEST_INTR_STATE_STI; + + if ((interruptibility != interruptibility_old)) + vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, interruptibility); +} + static void skip_emulated_instruction(struct kvm_vcpu *vcpu) { unsigned long rip; - u32 interruptibility; rip = kvm_rip_read(vcpu); rip += vmcs_read32(VM_EXIT_INSTRUCTION_LEN); kvm_rip_write(vcpu, rip); - /* - * We emulated an instruction, so temporary interrupt blocking - * should be removed, if set. - */ - interruptibility = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); - if (interruptibility & 3) - vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, - interruptibility & ~3); + /* skipping an emulated instruction also counts */ + vmx_set_interrupt_shadow(vcpu, 0); } static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr, @@ -2400,12 +2422,6 @@ out: return ret; } -void vmx_drop_interrupt_shadow(struct kvm_vcpu *vcpu) -{ - vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO, - GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS); -} - static void enable_irq_window(struct kvm_vcpu *vcpu) { u32 cpu_based_vm_exec_control; @@ -3649,6 +3665,8 @@ static struct kvm_x86_ops vmx_x86_ops = { .run = vmx_vcpu_run, .handle_exit = vmx_handle_exit, .skip_emulated_instruction = skip_emulated_instruction, + .set_interrupt_shadow = vmx_set_interrupt_shadow, + .get_interrupt_shadow = vmx_get_interrupt_shadow, .patch_hypercall = vmx_patch_hypercall, .set_irq = vmx_inject_irq, .set_nmi = vmx_inject_nmi, @@ -3658,7 +3676,6 @@ static struct kvm_x86_ops vmx_x86_ops = { .enable_nmi_window = enable_nmi_window, .enable_irq_window = enable_irq_window, .update_cr8_intercept = update_cr8_intercept, - .drop_interrupt_shadow = vmx_drop_interrupt_shadow, .set_tss_addr = vmx_set_tss_addr, .get_tdp_level = get_ept_level, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2d7082c..3d8fcc5 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3161,7 +3161,7 @@ static void inject_pending_irq(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) kvm_run->request_interrupt_window; if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) - kvm_x86_ops->drop_interrupt_shadow(vcpu); + kvm_x86_ops->set_interrupt_shadow(vcpu, 0); inject_irq(vcpu); -- 1.5.6.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] deal with interrupt shadow state for emulated instruction 2009-05-12 15:13 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa @ 2009-05-12 15:13 ` Glauber Costa 2009-05-12 18:14 ` Avi Kivity 0 siblings, 1 reply; 9+ messages in thread From: Glauber Costa @ 2009-05-12 15:13 UTC (permalink / raw) To: kvm; +Cc: linux-kernel, avi, H. Peter Anvin, Gleb Natapov we currently unblock shadow interrupt state when we skip an instruction, but failing to do so when we actually emulate one. This blocks interrupts in key instruction blocks, in particular sti; hlt; sequences If the instruction emulated is an sti, we have to block shadow interrupts. The same goes for mov ss. pop ss also needs it, but we don't currently emulate it. Without this patch, I cannot boot gpxe option roms at vmx machines. This is described at https://bugzilla.redhat.com/show_bug.cgi?id=494469 Signed-off-by: Glauber Costa <glommer@redhat.com> CC: H. Peter Anvin <hpa@zytor.com> CC: Avi Kivity <avi@redhat.com> CC: Gleb Natapov <gleb@redhat.com> --- arch/x86/include/asm/kvm_x86_emulate.h | 3 +++ arch/x86/kvm/x86.c | 6 +++++- arch/x86/kvm/x86_emulate.c | 26 +++++++++++++++++++++++++- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/kvm_x86_emulate.h b/arch/x86/include/asm/kvm_x86_emulate.h index be40d6e..b7ed2c4 100644 --- a/arch/x86/include/asm/kvm_x86_emulate.h +++ b/arch/x86/include/asm/kvm_x86_emulate.h @@ -155,6 +155,9 @@ struct x86_emulate_ctxt { int mode; u32 cs_base; + /* interruptibility state, as a result of execution of STI or MOV SS */ + int interruptibility; + /* decode cache */ struct decode_cache decode; }; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 3d8fcc5..b45baff 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2362,7 +2362,7 @@ int emulate_instruction(struct kvm_vcpu *vcpu, u16 error_code, int emulation_type) { - int r; + int r, shadow_mask; struct decode_cache *c; kvm_clear_exception_queue(vcpu); @@ -2416,6 +2416,10 @@ int emulate_instruction(struct kvm_vcpu *vcpu, } r = x86_emulate_insn(&vcpu->arch.emulate_ctxt, &emulate_ops); + shadow_mask = vcpu->arch.emulate_ctxt.interruptibility; + + if (r == 0) + kvm_x86_ops->set_interrupt_shadow(vcpu, shadow_mask); if (vcpu->arch.pio.string) return EMULATE_DO_MMIO; diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c index d2664fc..b847523 100644 --- a/arch/x86/kvm/x86_emulate.c +++ b/arch/x86/kvm/x86_emulate.c @@ -1372,6 +1372,8 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops) int io_dir_in; int rc = 0; + ctxt->interruptibility = 0; + /* Shadow copy of register state. Committed on successful emulation. * NOTE: we can copy them from vcpu as x86_decode_insn() doesn't * modify them. @@ -1618,6 +1620,15 @@ special_insn: int err; sel = c->src.val; + if (c->modrm_reg == VCPU_SREG_SS) { + u32 int_shadow = + kvm_x86_ops->get_interrupt_shadow(ctxt->vcpu, + X86_SHADOW_INT_MOV_SS); + /* See sti emulation for an explanation of this */ + if (!(int_shadow & X86_SHADOW_INT_MOV_SS)) + ctxt->interruptibility = X86_SHADOW_INT_MOV_SS; + } + if (c->modrm_reg <= 5) { type_bits = (c->modrm_reg == 1) ? 9 : 1; err = kvm_load_segment_descriptor(ctxt->vcpu, sel, @@ -1846,10 +1857,23 @@ special_insn: ctxt->eflags &= ~X86_EFLAGS_IF; c->dst.type = OP_NONE; /* Disable writeback. */ break; - case 0xfb: /* sti */ + case 0xfb: { /* sti */ + u32 int_shadow = + kvm_x86_ops->get_interrupt_shadow(ctxt->vcpu, + X86_SHADOW_INT_STI); + /* + * an sti; sti; sequence only disable interrupts for the first + * instruction. So, if the last instruction, be it emulated or + * not, left the system with the INT_STI flag enabled, it + * means that the last instruction is an sti. We should not + * leave the flag on in this case + */ + if (!(int_shadow & X86_SHADOW_INT_STI)) + ctxt->interruptibility = X86_SHADOW_INT_STI; ctxt->eflags |= X86_EFLAGS_IF; c->dst.type = OP_NONE; /* Disable writeback. */ break; + } case 0xfc: /* cld */ ctxt->eflags &= ~EFLG_DF; c->dst.type = OP_NONE; /* Disable writeback. */ -- 1.5.6.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] deal with interrupt shadow state for emulated instruction 2009-05-12 15:13 ` [PATCH 2/2] deal with interrupt shadow state for emulated instruction Glauber Costa @ 2009-05-12 18:14 ` Avi Kivity 0 siblings, 0 replies; 9+ messages in thread From: Avi Kivity @ 2009-05-12 18:14 UTC (permalink / raw) To: Glauber Costa; +Cc: kvm, linux-kernel, H. Peter Anvin, Gleb Natapov Glauber Costa wrote: > we currently unblock shadow interrupt state when we skip an instruction, > but failing to do so when we actually emulate one. This blocks interrupts > in key instruction blocks, in particular sti; hlt; sequences > > If the instruction emulated is an sti, we have to block shadow interrupts. > The same goes for mov ss. pop ss also needs it, but we don't currently > emulate it. > > Without this patch, I cannot boot gpxe option roms at vmx machines. > This is described at https://bugzilla.redhat.com/show_bug.cgi?id=494469 > > @@ -1618,6 +1620,15 @@ special_insn: > int err; > > sel = c->src.val; > + if (c->modrm_reg == VCPU_SREG_SS) { > + u32 int_shadow = > + kvm_x86_ops->get_interrupt_shadow(ctxt->vcpu, > + X86_SHADOW_INT_MOV_SS); > + /* See sti emulation for an explanation of this */ > + if (!(int_shadow & X86_SHADOW_INT_MOV_SS)) > + ctxt->interruptibility = X86_SHADOW_INT_MOV_SS; > + } > The indentation of the first statement here is annoying. Suggest a function toggle_interruptibility(ctxt, mask). Would eliminate the need for the comment forward reference as well. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 0/2] Deal with shadow interrupts after emulated instructions @ 2009-05-12 20:21 Glauber Costa 2009-05-12 20:21 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa 0 siblings, 1 reply; 9+ messages in thread From: Glauber Costa @ 2009-05-12 20:21 UTC (permalink / raw) To: kvm; +Cc: linux-kernel, avi Same story, more avi's comments merged. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow 2009-05-12 20:21 [PATCH 0/2] Deal with shadow interrupts after emulated instructions Glauber Costa @ 2009-05-12 20:21 ` Glauber Costa 2009-05-12 20:21 ` [PATCH 2/2] deal with interrupt shadow state for emulated instruction Glauber Costa 0 siblings, 1 reply; 9+ messages in thread From: Glauber Costa @ 2009-05-12 20:21 UTC (permalink / raw) To: kvm; +Cc: linux-kernel, avi, H. Peter Anvin, Gleb Natapov This patch replaces drop_interrupt_shadow with the more general set_interrupt_shadow, that can either drop or raise it, depending on its parameter. Signed-off-by: Glauber Costa <glommer@redhat.com> CC: H. Peter Anvin <hpa@zytor.com> CC: Avi Kivity <avi@redhat.com> CC: Gleb Natapov <gleb@redhat.com> --- arch/x86/include/asm/kvm_host.h | 3 +- arch/x86/include/asm/kvm_x86_emulate.h | 3 ++ arch/x86/kvm/svm.c | 32 +++++++++++++++----- arch/x86/kvm/vmx.c | 49 +++++++++++++++++++++---------- arch/x86/kvm/x86.c | 2 +- 5 files changed, 63 insertions(+), 26 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 8e680c3..3d933cf 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -510,6 +510,8 @@ struct kvm_x86_ops { void (*run)(struct kvm_vcpu *vcpu, struct kvm_run *run); int (*handle_exit)(struct kvm_run *run, struct kvm_vcpu *vcpu); void (*skip_emulated_instruction)(struct kvm_vcpu *vcpu); + void (*set_interrupt_shadow)(struct kvm_vcpu *vcpu, int mask); + u32 (*get_interrupt_shadow)(struct kvm_vcpu *vcpu, int mask); void (*patch_hypercall)(struct kvm_vcpu *vcpu, unsigned char *hypercall_addr); void (*set_irq)(struct kvm_vcpu *vcpu, int vec); @@ -521,7 +523,6 @@ struct kvm_x86_ops { void (*enable_nmi_window)(struct kvm_vcpu *vcpu); void (*enable_irq_window)(struct kvm_vcpu *vcpu); void (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr); - void (*drop_interrupt_shadow)(struct kvm_vcpu *vcpu); int (*set_tss_addr)(struct kvm *kvm, unsigned int addr); int (*get_tdp_level)(void); u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio); diff --git a/arch/x86/include/asm/kvm_x86_emulate.h b/arch/x86/include/asm/kvm_x86_emulate.h index 6a15973..be40d6e 100644 --- a/arch/x86/include/asm/kvm_x86_emulate.h +++ b/arch/x86/include/asm/kvm_x86_emulate.h @@ -143,6 +143,9 @@ struct decode_cache { struct fetch_cache fetch; }; +#define X86_SHADOW_INT_MOV_SS 1 +#define X86_SHADOW_INT_STI 2 + struct x86_emulate_ctxt { /* Register state before/after emulation. */ struct kvm_vcpu *vcpu; diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index ef43a18..f4d1bd9 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -202,6 +202,27 @@ static int is_external_interrupt(u32 info) return info == (SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_INTR); } +static u32 svm_get_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) +{ + struct vcpu_svm *svm = to_svm(vcpu); + u32 ret = 0; + + if (svm->vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) + ret |= X86_SHADOW_INT_STI | X86_SHADOW_INT_MOV_SS; + return ret & mask; +} + +static void svm_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) +{ + struct vcpu_svm *svm = to_svm(vcpu); + + if (mask == 0) + svm->vmcb->control.int_state &= ~SVM_INTERRUPT_SHADOW_MASK; + else + svm->vmcb->control.int_state |= SVM_INTERRUPT_SHADOW_MASK; + +} + static void skip_emulated_instruction(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); @@ -215,7 +236,7 @@ static void skip_emulated_instruction(struct kvm_vcpu *vcpu) __func__, kvm_rip_read(vcpu), svm->next_rip); kvm_rip_write(vcpu, svm->next_rip); - svm->vmcb->control.int_state &= ~SVM_INTERRUPT_SHADOW_MASK; + svm_set_interrupt_shadow(vcpu, 0); } static int has_svm(void) @@ -2229,12 +2250,6 @@ static void pre_svm_run(struct vcpu_svm *svm) new_asid(svm, svm_data); } -static void svm_drop_interrupt_shadow(struct kvm_vcpu *vcpu) -{ - struct vcpu_svm *svm = to_svm(vcpu); - svm->vmcb->control.int_state &= ~SVM_INTERRUPT_SHADOW_MASK; -} - static void svm_inject_nmi(struct kvm_vcpu *vcpu) { struct vcpu_svm *svm = to_svm(vcpu); @@ -2637,6 +2652,8 @@ static struct kvm_x86_ops svm_x86_ops = { .run = svm_vcpu_run, .handle_exit = handle_exit, .skip_emulated_instruction = skip_emulated_instruction, + .set_interrupt_shadow = svm_set_interrupt_shadow, + .get_interrupt_shadow = svm_get_interrupt_shadow, .patch_hypercall = svm_patch_hypercall, .set_irq = svm_set_irq, .set_nmi = svm_inject_nmi, @@ -2646,7 +2663,6 @@ static struct kvm_x86_ops svm_x86_ops = { .enable_nmi_window = enable_nmi_window, .enable_irq_window = enable_irq_window, .update_cr8_intercept = update_cr8_intercept, - .drop_interrupt_shadow = svm_drop_interrupt_shadow, .set_tss_addr = svm_set_tss_addr, .get_tdp_level = get_npt_level, diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index e8a5649..f3ab27b 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -736,23 +736,45 @@ static void vmx_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags) vmcs_writel(GUEST_RFLAGS, rflags); } +static u32 vmx_get_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) +{ + u32 interruptibility = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); + int ret = 0; + + if (interruptibility & GUEST_INTR_STATE_STI) + ret |= X86_SHADOW_INT_STI; + if (interruptibility & GUEST_INTR_STATE_MOV_SS) + ret |= X86_SHADOW_INT_MOV_SS; + + return ret & mask; +} + +static void vmx_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask) +{ + u32 interruptibility_old = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); + u32 interruptibility = interruptibility_old; + + interruptibility &= ~(GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS); + + if (mask & X86_SHADOW_INT_MOV_SS) + interruptibility |= GUEST_INTR_STATE_MOV_SS; + if (mask & X86_SHADOW_INT_STI) + interruptibility |= GUEST_INTR_STATE_STI; + + if ((interruptibility != interruptibility_old)) + vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, interruptibility); +} + static void skip_emulated_instruction(struct kvm_vcpu *vcpu) { unsigned long rip; - u32 interruptibility; rip = kvm_rip_read(vcpu); rip += vmcs_read32(VM_EXIT_INSTRUCTION_LEN); kvm_rip_write(vcpu, rip); - /* - * We emulated an instruction, so temporary interrupt blocking - * should be removed, if set. - */ - interruptibility = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO); - if (interruptibility & 3) - vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, - interruptibility & ~3); + /* skipping an emulated instruction also counts */ + vmx_set_interrupt_shadow(vcpu, 0); } static void vmx_queue_exception(struct kvm_vcpu *vcpu, unsigned nr, @@ -2400,12 +2422,6 @@ out: return ret; } -void vmx_drop_interrupt_shadow(struct kvm_vcpu *vcpu) -{ - vmcs_clear_bits(GUEST_INTERRUPTIBILITY_INFO, - GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS); -} - static void enable_irq_window(struct kvm_vcpu *vcpu) { u32 cpu_based_vm_exec_control; @@ -3649,6 +3665,8 @@ static struct kvm_x86_ops vmx_x86_ops = { .run = vmx_vcpu_run, .handle_exit = vmx_handle_exit, .skip_emulated_instruction = skip_emulated_instruction, + .set_interrupt_shadow = vmx_set_interrupt_shadow, + .get_interrupt_shadow = vmx_get_interrupt_shadow, .patch_hypercall = vmx_patch_hypercall, .set_irq = vmx_inject_irq, .set_nmi = vmx_inject_nmi, @@ -3658,7 +3676,6 @@ static struct kvm_x86_ops vmx_x86_ops = { .enable_nmi_window = enable_nmi_window, .enable_irq_window = enable_irq_window, .update_cr8_intercept = update_cr8_intercept, - .drop_interrupt_shadow = vmx_drop_interrupt_shadow, .set_tss_addr = vmx_set_tss_addr, .get_tdp_level = get_ept_level, diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2d7082c..3d8fcc5 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3161,7 +3161,7 @@ static void inject_pending_irq(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run) kvm_run->request_interrupt_window; if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP) - kvm_x86_ops->drop_interrupt_shadow(vcpu); + kvm_x86_ops->set_interrupt_shadow(vcpu, 0); inject_irq(vcpu); -- 1.5.6.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] deal with interrupt shadow state for emulated instruction 2009-05-12 20:21 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa @ 2009-05-12 20:21 ` Glauber Costa 0 siblings, 0 replies; 9+ messages in thread From: Glauber Costa @ 2009-05-12 20:21 UTC (permalink / raw) To: kvm; +Cc: linux-kernel, avi, H. Peter Anvin, Gleb Natapov we currently unblock shadow interrupt state when we skip an instruction, but failing to do so when we actually emulate one. This blocks interrupts in key instruction blocks, in particular sti; hlt; sequences If the instruction emulated is an sti, we have to block shadow interrupts. The same goes for mov ss. pop ss also needs it, but we don't currently emulate it. Without this patch, I cannot boot gpxe option roms at vmx machines. This is described at https://bugzilla.redhat.com/show_bug.cgi?id=494469 Signed-off-by: Glauber Costa <glommer@redhat.com> CC: H. Peter Anvin <hpa@zytor.com> CC: Avi Kivity <avi@redhat.com> CC: Gleb Natapov <gleb@redhat.com> --- arch/x86/include/asm/kvm_x86_emulate.h | 3 +++ arch/x86/kvm/x86.c | 6 +++++- arch/x86/kvm/x86_emulate.c | 20 ++++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletions(-) diff --git a/arch/x86/include/asm/kvm_x86_emulate.h b/arch/x86/include/asm/kvm_x86_emulate.h index be40d6e..b7ed2c4 100644 --- a/arch/x86/include/asm/kvm_x86_emulate.h +++ b/arch/x86/include/asm/kvm_x86_emulate.h @@ -155,6 +155,9 @@ struct x86_emulate_ctxt { int mode; u32 cs_base; + /* interruptibility state, as a result of execution of STI or MOV SS */ + int interruptibility; + /* decode cache */ struct decode_cache decode; }; diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 3d8fcc5..b45baff 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -2362,7 +2362,7 @@ int emulate_instruction(struct kvm_vcpu *vcpu, u16 error_code, int emulation_type) { - int r; + int r, shadow_mask; struct decode_cache *c; kvm_clear_exception_queue(vcpu); @@ -2416,6 +2416,10 @@ int emulate_instruction(struct kvm_vcpu *vcpu, } r = x86_emulate_insn(&vcpu->arch.emulate_ctxt, &emulate_ops); + shadow_mask = vcpu->arch.emulate_ctxt.interruptibility; + + if (r == 0) + kvm_x86_ops->set_interrupt_shadow(vcpu, shadow_mask); if (vcpu->arch.pio.string) return EMULATE_DO_MMIO; diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c index d2664fc..c1b6c23 100644 --- a/arch/x86/kvm/x86_emulate.c +++ b/arch/x86/kvm/x86_emulate.c @@ -1361,6 +1361,20 @@ static inline int writeback(struct x86_emulate_ctxt *ctxt, return 0; } +void toggle_interruptibility(struct x86_emulate_ctxt *ctxt, u32 mask) +{ + u32 int_shadow = kvm_x86_ops->get_interrupt_shadow(ctxt->vcpu, mask); + /* + * an sti; sti; sequence only disable interrupts for the first + * instruction. So, if the last instruction, be it emulated or + * not, left the system with the INT_STI flag enabled, it + * means that the last instruction is an sti. We should not + * leave the flag on in this case. The same goes for mov ss + */ + if (!(int_shadow & mask)) + ctxt->interruptibility = mask; +} + int x86_emulate_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops) { @@ -1372,6 +1386,8 @@ x86_emulate_insn(struct x86_emulate_ctxt *ctxt, struct x86_emulate_ops *ops) int io_dir_in; int rc = 0; + ctxt->interruptibility = 0; + /* Shadow copy of register state. Committed on successful emulation. * NOTE: we can copy them from vcpu as x86_decode_insn() doesn't * modify them. @@ -1618,6 +1634,9 @@ special_insn: int err; sel = c->src.val; + if (c->modrm_reg == VCPU_SREG_SS) + toggle_interruptibility(ctxt, X86_SHADOW_INT_MOV_SS); + if (c->modrm_reg <= 5) { type_bits = (c->modrm_reg == 1) ? 9 : 1; err = kvm_load_segment_descriptor(ctxt->vcpu, sel, @@ -1847,6 +1866,7 @@ special_insn: c->dst.type = OP_NONE; /* Disable writeback. */ break; case 0xfb: /* sti */ + toggle_interruptibility(ctxt, X86_SHADOW_INT_STI); ctxt->eflags |= X86_EFLAGS_IF; c->dst.type = OP_NONE; /* Disable writeback. */ break; -- 1.5.6.6 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-05-12 20:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-05-08 20:23 [PATCH 0/2] Deal with shadow interrupts after emulated instructions Glauber Costa 2009-05-08 20:23 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa 2009-05-08 20:23 ` [PATCH 2/2] deal with interrupt shadow state for emulated instruction Glauber Costa 2009-05-10 13:09 ` Avi Kivity 2009-05-10 13:07 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Avi Kivity 2009-05-10 8:15 ` [PATCH 0/2] Deal with shadow interrupts after emulated instructions Gleb Natapov -- strict thread matches above, loose matches on Subject: below -- 2009-05-12 15:13 Glauber Costa 2009-05-12 15:13 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa 2009-05-12 15:13 ` [PATCH 2/2] deal with interrupt shadow state for emulated instruction Glauber Costa 2009-05-12 18:14 ` Avi Kivity 2009-05-12 20:21 [PATCH 0/2] Deal with shadow interrupts after emulated instructions Glauber Costa 2009-05-12 20:21 ` [PATCH 1/2] replace drop_interrupt_shadow by set_interrupt_shadow Glauber Costa 2009-05-12 20:21 ` [PATCH 2/2] deal with interrupt shadow state for emulated instruction Glauber Costa
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.