diff mbox

[2/2] deal with interrupt shadow state for emulated instruction

Message ID 1242141230-22514-3-git-send-email-glommer@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Glauber Costa May 12, 2009, 3:13 p.m. UTC
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(-)

Comments

Avi Kivity May 12, 2009, 6:14 p.m. UTC | #1
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.
diff mbox

Patch

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. */