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

Nadav Amit
2014-06-04T19:12:47Z

[-- Attachment #1: Type: text/plain, Size: 2375 bytes --]

On Jun 4, 2014, at 7:34 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 04/06/2014 16:44, Alexander Graf ha scritto:
>>
>>
>>> Obviously, if you really like the current behavior better you can
>>> always reject whatever patch I'll come up with, but I'd like to at
>>> least try and see what it would look like :)
>>
>> I think it's perfectly fine to leave mwait always implemented as NOP -
>> it's valid behavior.
>>
>> As for the CPUID exposure, that should be a pure QEMU thing. If
>> overriding CPUID bits the kernel mask tells us doesn't work today, we
>> should just make it possible :).
>
> That should be the purpose of KVM_GET_EMULATED_CPUID, so MWAIT could be added in __do_cpuid_ent_emulated. However, the corresponding QEMU patches were never included. Borislav, can you refresh them?

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;
}

[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 495 bytes --]