Re: [PATCH v2] kvm: x86: emulate monitor and mwait instructions as nop

Gabriel L. Somlo
2014-06-04T19:43:37Z

On Wed, Jun 04, 2014 at 10:12:39PM +0300, Nadav Amit wrote:
> Regardless to the whole discussion of what the guest is informed about, I think it might be better to implement mwait and monitor correctly according to the spec and let the instructions to be fully emulated.
> Both mwait and monitor may encounter exceptions (#GP, #PF, regardless of #UD), so this behaviour should be correct.
> If you want me, I?ll send my version which looks something like:
>
> 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;
>
> rc = check_mwait_supported(ctxt);
> if (rc != X86EMUL_CONTINUE)
> return rc;
>
> 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;
>
> return X86EMUL_CONTINUE;
> }
>
> static int em_mwait(struct x86_emulate_ctxt *ctxt)
> {
> u64 rcx = reg_read(ctxt, VCPU_REGS_RCX);
> int rc = check_mwait_supported(ctxt);
> if (rc != X86EMUL_CONTINUE)
> return rc;
> if (ctxt->mode != X86EMUL_MODE_PROT64)
> rcx = (u32)rcx;
>
> if ((rcx & ~(u64)1) != 0)
> return emulate_gp(ctxt, 0);
>
> if (rcx & 1) {
> /* Interrupt as break event */
> u32 ebx, ecx, edx, eax;
> eax = 5;
> ecx = 0;
> ctxt->ops->get_cpuid(ctxt, &eax, &ebx, &ecx, &edx);
> if (!(ecx & 1))
> return emulate_gp(ctxt, 0);
> }
> return X86EMUL_CONTINUE;
> }

I'd be curious how you're dealing with the "hidden" CPU state which
tells MWAIT to sleep until someone or something writes to the
monitored memory area set up by a corresponding MONITOR instruction.

Thanks,
--Gabriel