[0/4] KVM: SVM: improve NMI window singlestep

Commit Message

Paolo Bonzini June 15, 2017, 12:03 p.m. UTC

On 15/06/2017 13:20, Ladi Prosek wrote:
> NMI window singlestep kind of works with these patches because it's meant
> only for short sequences (I believe that the original intention was to
> step over an IRET but I doubt it's that simple anymore)

Yes, it was meant to step over an IRET or an interrupt shadow.

One extra case that may cause NMI singlestep these days is GIF=0, but 
that is also solved easily:

Paolo

> so we can get
> away with half-butting it. In particular, it's unlikely that the guest
> would set the TRAP flag while the NMI window is closed. Properly handling
> KVM_GUESTDBG_SINGLESTEP would likely involve intercepting PUSHF & POPF,
> clearing the TRAP flag from the stack on interrupt entry, and possibly more.

Comments

Ladi Prosek June 15, 2017, 12:10 p.m. UTC | #1

On Thu, Jun 15, 2017 at 2:03 PM, Paolo Bonzini <[e-mail]> wrote:
>
>
> On 15/06/2017 13:20, Ladi Prosek wrote:
>> NMI window singlestep kind of works with these patches because it's meant
>> only for short sequences (I believe that the original intention was to
>> step over an IRET but I doubt it's that simple anymore)
>
> Yes, it was meant to step over an IRET or an interrupt shadow.
>
> One extra case that may cause NMI singlestep these days is GIF=0, but
> that is also solved easily:
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index d1efe2c62b3f..15a2f7f8e539 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -4622,6 +4622,9 @@ static void enable_nmi_window(struct kvm_vcpu *vcpu)
>         if ((svm->vcpu.arch.hflags & (HF_NMI_MASK | HF_IRET_MASK))
>             == HF_NMI_MASK)
>                 return; /* IRET will cause a vm exit */
> +       if ((svm->vcpu.arch.hflags & (HF_NMI_MASK | HF_GIF_MASK))
> +           == HF_NMI_MASK)
> +               return; /* STGI will cause a vm exit */
>
> so you could include this change in your series.

Will do, thanks!

> Paolo
>
>> so we can get
>> away with half-butting it. In particular, it's unlikely that the guest
>> would set the TRAP flag while the NMI window is closed. Properly handling
>> KVM_GUESTDBG_SINGLESTEP would likely involve intercepting PUSHF & POPF,
>> clearing the TRAP flag from the stack on interrupt entry, and possibly more.

Patch

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index d1efe2c62b3f..15a2f7f8e539 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4622,6 +4622,9 @@  static void enable_nmi_window(struct kvm_vcpu *vcpu)
 	if ((svm->vcpu.arch.hflags & (HF_NMI_MASK | HF_IRET_MASK))
 	    == HF_NMI_MASK)
 		return; /* IRET will cause a vm exit */
+	if ((svm->vcpu.arch.hflags & (HF_NMI_MASK | HF_GIF_MASK))
+	    == HF_NMI_MASK)
+		return; /* STGI will cause a vm exit */

so you could include this change in your series.