Re: [PATCH 3/3] KVM: x86: correct mwait and monitor emulation

[Eric Northup]

On Wed, Jun 18, 2014 at 7:19 AM, Nadav Amit <namit@xxxxxxxxxxxxxxxxx> wrote:
> mwait and monitor are currently handled as nop. Considering this behavior, they
> should still be handled correctly, i.e., check execution conditions and generate
> exceptions when required. mwait and monitor may also be executed in real-mode
> and are not handled in that case.  This patch performs the emulation of
> monitor-mwait according to Intel SDM (other than checking whether interrupt can
> be used as a break event).
>
> Signed-off-by: Nadav Amit <namit@xxxxxxxxxxxxxxxxx>
> ---
>  arch/x86/kvm/emulate.c | 41 +++++++++++++++++++++++++++++++++++++++--
>  arch/x86/kvm/svm.c     | 22 ++--------------------
>  arch/x86/kvm/vmx.c     | 27 +++++++++++----------------
>  3 files changed, 52 insertions(+), 38 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index ef7a5a0..424b58d 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -3344,6 +3344,43 @@ static int em_bswap(struct x86_emulate_ctxt *ctxt)
>         return X86EMUL_CONTINUE;
>  }
>
> +static int em_monitor(struct x86_emulate_ctxt *ctxt)
> +{
> +       int rc;
> +       struct segmented_address addr;
> +       u64 rcx = reg_read(ctxt, VCPU_REGS_RCX);
> +       u64 rax = reg_read(ctxt, VCPU_REGS_RAX);
> +       u8 byte;

I'd request:

u32 ebx, ecx, edx, eax = 1;
ctxt->opt->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx);
if (!(ecx & FFL(MWAIT)))
        return emulate_ud(ctxt);

and also in em_mwait.

> +
> +       if (ctxt->mode != X86EMUL_MODE_PROT64)
> +               rcx = (u32)rcx;
> +       if (rcx != 0)
> +               return emulate_gp(ctxt, 0);
> +
> +       addr.seg = seg_override(ctxt);
> +       addr.ea = ctxt->ad_bytes == 8 ? rax : (u32)rax;
> +       rc = segmented_read(ctxt, addr, &byte, 1);
> +       if (rc != X86EMUL_CONTINUE)
> +               return rc;
> +
> +       printk_once(KERN_WARNING "kvm: MONITOR instruction emulated as NOP!\n");
> +       return X86EMUL_CONTINUE;
> +}
> +
> +static int em_mwait(struct x86_emulate_ctxt *ctxt)
> +{
> +       u64 rcx = reg_read(ctxt, VCPU_REGS_RCX);
> +
> +       if (ctxt->mode != X86EMUL_MODE_PROT64)
> +               rcx = (u32)rcx;
> +       if ((rcx & ~1UL) != 0)
> +               return emulate_gp(ctxt, 0);
> +
> +       /* Accepting interrupt as break event regardless to cpuid */
> +       printk_once(KERN_WARNING "kvm: MWAIT instruction emulated as NOP!\n");
> +       return X86EMUL_CONTINUE;
> +}
> +
>  static bool valid_cr(int nr)
>  {
>         switch (nr) {
> @@ -3557,8 +3594,8 @@ static int check_perm_out(struct x86_emulate_ctxt *ctxt)
>                 F2bv(((_f) & ~Lock) | DstAcc | SrcImm, _e)
>
>  static const struct opcode group7_rm1[] = {
> -       DI(SrcNone | Priv, monitor),
> -       DI(SrcNone | Priv, mwait),
> +       II(SrcNone | Priv | NoBigReal | UDOnPriv, em_monitor, monitor),
> +       II(SrcNone | Priv | NoBigReal | UDOnPriv, em_mwait, mwait),
>         N, N, N, N, N, N,
>  };
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index ec8366c..a524e04 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -3274,24 +3274,6 @@ static int pause_interception(struct vcpu_svm *svm)
>         return 1;
>  }
>
> -static int nop_interception(struct vcpu_svm *svm)
> -{
> -       skip_emulated_instruction(&(svm->vcpu));
> -       return 1;
> -}
> -
> -static int monitor_interception(struct vcpu_svm *svm)
> -{
> -       printk_once(KERN_WARNING "kvm: MONITOR instruction emulated as NOP!\n");
> -       return nop_interception(svm);
> -}
> -
> -static int mwait_interception(struct vcpu_svm *svm)
> -{
> -       printk_once(KERN_WARNING "kvm: MWAIT instruction emulated as NOP!\n");
> -       return nop_interception(svm);
> -}
> -
>  static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
>         [SVM_EXIT_READ_CR0]                     = cr_interception,
>         [SVM_EXIT_READ_CR3]                     = cr_interception,
> @@ -3349,8 +3331,8 @@ static int (*const svm_exit_handlers[])(struct vcpu_svm *svm) = {
>         [SVM_EXIT_CLGI]                         = clgi_interception,
>         [SVM_EXIT_SKINIT]                       = skinit_interception,
>         [SVM_EXIT_WBINVD]                       = emulate_on_interception,
> -       [SVM_EXIT_MONITOR]                      = monitor_interception,
> -       [SVM_EXIT_MWAIT]                        = mwait_interception,
> +       [SVM_EXIT_MONITOR]                      = emulate_on_interception,
> +       [SVM_EXIT_MWAIT]                        = emulate_on_interception,
>         [SVM_EXIT_XSETBV]                       = xsetbv_interception,
>         [SVM_EXIT_NPF]                          = pf_interception,
>  };
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 801332e..7023e71 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5672,22 +5672,17 @@ static int handle_pause(struct kvm_vcpu *vcpu)
>         return 1;
>  }
>
> -static int handle_nop(struct kvm_vcpu *vcpu)
> +static int handle_emulate(struct kvm_vcpu *vcpu)
>  {
> -       skip_emulated_instruction(vcpu);
> -       return 1;
> -}
> +       int err = emulate_instruction(vcpu, 0);
>
> -static int handle_mwait(struct kvm_vcpu *vcpu)
> -{
> -       printk_once(KERN_WARNING "kvm: MWAIT instruction emulated as NOP!\n");
> -       return handle_nop(vcpu);
> -}
> -
> -static int handle_monitor(struct kvm_vcpu *vcpu)
> -{
> -       printk_once(KERN_WARNING "kvm: MONITOR instruction emulated as NOP!\n");
> -       return handle_nop(vcpu);
> +       if (err != EMULATE_DONE) {
> +               vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> +               vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
> +               vcpu->run->internal.ndata = 0;
> +               return 0;
> +       }
> +       return 1;
>  }
>
>  /*
> @@ -6651,8 +6646,8 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>         [EXIT_REASON_EPT_VIOLATION]           = handle_ept_violation,
>         [EXIT_REASON_EPT_MISCONFIG]           = handle_ept_misconfig,
>         [EXIT_REASON_PAUSE_INSTRUCTION]       = handle_pause,
> -       [EXIT_REASON_MWAIT_INSTRUCTION]       = handle_mwait,
> -       [EXIT_REASON_MONITOR_INSTRUCTION]     = handle_monitor,
> +       [EXIT_REASON_MWAIT_INSTRUCTION]       = handle_emulate,
> +       [EXIT_REASON_MONITOR_INSTRUCTION]     = handle_emulate,
>         [EXIT_REASON_INVEPT]                  = handle_invept,
>  };
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/