diff mbox

do not keep interrupt window closed by sti in real mode

Message ID 1239161017-7398-1-git-send-email-glommer@redhat.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Glauber Costa April 8, 2009, 3:23 a.m. UTC
While in real mode, sti does not block interrupts from the subsequent
instruction. This is stated at Intel SDM Volume 2b, page 4-432

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>
---
 arch/x86/kvm/vmx.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

Comments

H. Peter Anvin April 8, 2009, 4:14 a.m. UTC | #1
Glauber Costa wrote:
> While in real mode, sti does not block interrupts from the subsequent
> instruction. This is stated at Intel SDM Volume 2b, page 4-432

I don't see how you're getting that idea from the STI documentation --
and I am quite sure that that is not the case.  Quite on the contrary.
The only differences between protected mode and real mode has to do with
the handling of VIF when CPL=3 (this rather naturally falls out if one
considers CPL=0 in real mode).

The text is:

"If protected-mode virtual interrupts are not enabled, STI sets the
interrupt flag (IF) in the EFLAGS register. After the IF flag is set,
the processor begins responding to external, maskable interrupts after
the next instruction is executed. The delayed effect of this instruction
is provided to allow interrupts to be enabled just before returning from
a procedure (or subroutine). For instance, if an STI instruction is
followed by an RET instruction, the RET instruction is allowed to
execute before external interrupts are recognized1. If the STI
instruction is followed by a CLI instruction (which clears the IF flag),
the effect of the STI instruction is negated."

Obviously, in real mode, "protected-mode virtual interrupts" are not
enabled, as is also confirmed by Table 4-5.

	-hpa
Avi Kivity April 8, 2009, 5:45 a.m. UTC | #2
H. Peter Anvin wrote:
> Glauber Costa wrote:
>   
>> While in real mode, sti does not block interrupts from the subsequent
>> instruction. This is stated at Intel SDM Volume 2b, page 4-432
>>     
>
> I don't see how you're getting that idea from the STI documentation --
> and I am quite sure that that is not the case.  Quite on the contrary.
> The only differences between protected mode and real mode has to do with
> the handling of VIF when CPL=3 (this rather naturally falls out if one
> considers CPL=0 in real mode).
>   

I'm guessing the problem is due to the second instruction.  We don't 
clear the 'blocked by interrupt shadow' flag when we emulate, which 
extends interrupt shadow by one more instruction.  If the instruction 
sequence is 'sti hlt' we end in an inconsistent state.
H. Peter Anvin April 8, 2009, 6:25 a.m. UTC | #3
Avi Kivity wrote:
> 
> I'm guessing the problem is due to the second instruction.  We don't
> clear the 'blocked by interrupt shadow' flag when we emulate, which
> extends interrupt shadow by one more instruction.  If the instruction
> sequence is 'sti hlt' we end in an inconsistent state.
> 

Ah, and since we're in real mode, we have to emulate everything (at
least on some hardware), right?  So we really do need to clear the
interrupt shadow bit in the interpreter... I don't see a way around that.

Otherwise not just STI but MOV SS shadows will break, and in real mode
MOV SS shadow is crucial.

	-hpa
Avi Kivity April 8, 2009, 8:16 a.m. UTC | #4
H. Peter Anvin wrote:
> Avi Kivity wrote:
>   
>> I'm guessing the problem is due to the second instruction.  We don't
>> clear the 'blocked by interrupt shadow' flag when we emulate, which
>> extends interrupt shadow by one more instruction.  If the instruction
>> sequence is 'sti hlt' we end in an inconsistent state.
>>
>>     
>
> Ah, and since we're in real mode, we have to emulate everything (at
> least on some hardware), right?  

Well, not everything.  We use vm86 mode in the guest to emulate real 
mode.  Of course that doesn't support all instructions, so we emulate 
these.  Unfortunately it also doesn't support big real mode.

> So we really do need to clear the
> interrupt shadow bit in the interpreter... I don't see a way around that.
>   

Yes.

> Otherwise not just STI but MOV SS shadows will break, and in real mode
> MOV SS shadow is crucial.
>   

'mov ss' executes natively.
Glauber Costa April 8, 2009, 2:55 p.m. UTC | #5
On Tue, Apr 07, 2009 at 09:14:58PM -0700, H. Peter Anvin wrote:
> Glauber Costa wrote:
> > While in real mode, sti does not block interrupts from the subsequent
> > instruction. This is stated at Intel SDM Volume 2b, page 4-432
> 
> I don't see how you're getting that idea from the STI documentation --
> and I am quite sure that that is not the case.  Quite on the contrary.
> The only differences between protected mode and real mode has to do with
> the handling of VIF when CPL=3 (this rather naturally falls out if one
> considers CPL=0 in real mode).
> 
> The text is:
> 
> "If protected-mode virtual interrupts are not enabled, STI sets the
> interrupt flag (IF) in the EFLAGS register. After the IF flag is set,
> the processor begins responding to external, maskable interrupts after
> the next instruction is executed. The delayed effect of this instruction
> is provided to allow interrupts to be enabled just before returning from
> a procedure (or subroutine). For instance, if an STI instruction is
> followed by an RET instruction, the RET instruction is allowed to
> execute before external interrupts are recognized1. If the STI
> instruction is followed by a CLI instruction (which clears the IF flag),
> the effect of the STI instruction is negated."
> 
> Obviously, in real mode, "protected-mode virtual interrupts" are not
> enabled, as is also confirmed by Table 4-5.

I get the idea from the pseudocode in sti description.
It says:
IF PE = 0 (* Executing in real-address mode *)
    THEN
        IF <- 1; (* Set Interrupt Flag *)
    ELSE (* Executing in protected mode or virtual-8086 mode *)

There is no mention to any other activity besides setting the if flag.
Also, sti is used extensively in many places like the linux kernel for the
guest, and it works just fine in kvm. So I was led to believe that real mode
in fact behaving differently.

I'll take a look at avi's suggestion.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
H. Peter Anvin April 8, 2009, 4:11 p.m. UTC | #6
Glauber Costa wrote:
> 
> I get the idea from the pseudocode in sti description.
> It says:
> IF PE = 0 (* Executing in real-address mode *)
>     THEN
>         IF <- 1; (* Set Interrupt Flag *)
>     ELSE (* Executing in protected mode or virtual-8086 mode *)
> 
> There is no mention to any other activity besides setting the if flag.

But the same is true for the protected mode side of the instruction 
description!

> Also, sti is used extensively in many places like the linux kernel for the
> guest, and it works just fine in kvm. So I was led to believe that real mode
> in fact behaving differently.

The difference is that at least under current Intel VT, VT only handles 
protected mode -- the real mode runs purely in the interpreter.

> I'll take a look at avi's suggestion.

	-hpa
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index c6997c0..51e0b8a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2490,18 +2490,19 @@  static void vmx_inject_nmi(struct kvm_vcpu *vcpu)
 static void vmx_update_window_states(struct kvm_vcpu *vcpu)
 {
 	u32 guest_intr = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO);
+	int rmode = vcpu->arch.rmode.active;
 
 	vcpu->arch.nmi_window_open =
-		!(guest_intr & (GUEST_INTR_STATE_STI |
-				GUEST_INTR_STATE_MOV_SS |
+		(rmode || !(guest_intr & GUEST_INTR_STATE_STI)) &&
+		!(guest_intr & (GUEST_INTR_STATE_MOV_SS |
 				GUEST_INTR_STATE_NMI));
 	if (!cpu_has_virtual_nmis() && to_vmx(vcpu)->soft_vnmi_blocked)
 		vcpu->arch.nmi_window_open = 0;
 
 	vcpu->arch.interrupt_window_open =
 		((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
-		 !(guest_intr & (GUEST_INTR_STATE_STI |
-				 GUEST_INTR_STATE_MOV_SS)));
+		(rmode || !(guest_intr & GUEST_INTR_STATE_STI)) &&
+		 !(guest_intr & GUEST_INTR_STATE_MOV_SS));
 }
 
 static int vmx_interrupt_allowed(struct kvm_vcpu *vcpu)