lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Thu, 19 Jun 2014 13:34:44 +0200
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	Eric Northup <digitaleric@...gle.com>,
	Nadav Amit <namit@...technion.ac.il>
CC:	gleb@...nel.org, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	the arch/x86 maintainers <x86@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	KVM <kvm@...r.kernel.org>, joro@...tes.org
Subject: Re: [PATCH 3/3] KVM: x86: correct mwait and monitor emulation

Il 18/06/2014 19:59, Eric Northup ha scritto:
> On Wed, Jun 18, 2014 at 7:19 AM, Nadav Amit <namit@...technion.ac.il> 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@...technion.ac.il>
>> ---
>>  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.

Ignoring the fact that this should never be true 
(KVM_GET_SUPPORTED_CPUID never reports the MWAIT bit), why should 
MONITOR and MWAIT be special?  We do not do this kind of check for SSE 
or AVX instructions.

An alternative is to record the address that was being waited on, and 
invoke PLE (kvm_vcpu_on_spin) if the current address matches the last 
one.  A VMEXIT + emulation takes a couple thousand cycles, which is the 
same order of magnitude as the PLE window.

Even if there is a workaround, I don't think reverting the patch is 
necessary.  The patch was there for a fringe case anyway (recent 
versions of Mac OS X get CPUID right), so I don't think the availability 
of a work around changes the assessment of how ugly/useful MONITOR/MWAIT is.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists