From patchwork Wed May 7 18:10:59 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Gabriel L. Somlo" X-Patchwork-Id: 4131081 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 6C09BBFF02 for ; Wed, 7 May 2014 18:11:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7493720212 for ; Wed, 7 May 2014 18:11:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5C592201FB for ; Wed, 7 May 2014 18:11:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751901AbaEGSLG (ORCPT ); Wed, 7 May 2014 14:11:06 -0400 Received: from mail-qc0-f179.google.com ([209.85.216.179]:33693 "EHLO mail-qc0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752438AbaEGSLB (ORCPT ); Wed, 7 May 2014 14:11:01 -0400 Received: by mail-qc0-f179.google.com with SMTP id x3so1513123qcv.38 for ; Wed, 07 May 2014 11:11:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:mime-version:content-type :content-disposition:user-agent; bh=Zhz0ELoEII3sG/8byokSG8X6qg//CY+CZsx7GDqlnAg=; b=slMvKtY9vJDRPlXTwa7npE0Tkg1L1LDLkhviJo4zXajvBCXyzWxFgQ4TEr8K9U/7j3 D4GJVziNk7Vrgn8kZ2ZWFibkERfMoNChBfTYU7iR1j6zLj41ehcxTfTHXxPA5kCDXS1e 8X5g0h/uxn9ltmaxssk7LV3ina6z39KAg45JrvvHM3sGSKH5gOB0UTWr8gwuiSA27Wba cSVlqUGHzWZKbiykVoTOfoufVKiOy4g+FRLElQRRyjDJjFSzXv9CYbSYHuRkKSu+lBEA 1+CFMeUifnu0ZsYbOsMOSwDkDZdWOVIpZbualuzg5pTUFqjY1kHzzzL5NhXU66+geXaT PlYA== X-Received: by 10.140.30.161 with SMTP id d30mr62237696qgd.62.1399486261125; Wed, 07 May 2014 11:11:01 -0700 (PDT) Received: from ERROL.INI.CMU.EDU (ERROL.INI.CMU.EDU. [128.2.16.43]) by mx.google.com with ESMTPSA id a9sm30277776qaa.26.2014.05.07.11.11.00 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 07 May 2014 11:11:00 -0700 (PDT) Date: Wed, 7 May 2014 14:10:59 -0400 From: "Gabriel L. Somlo" To: kvm@vger.kernel.org Cc: mst@redhat.com, pbonzini@redhat.com, afaerber@suse.de, agraf@suse.de Subject: [PATCH] kvm: x86: emulate monitor and mwait instructions as nop Message-ID: <20140507181056.GD1655@ERROL.INI.CMU.EDU> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Treat monitor and mwait instructions as nop, which is architecturally correct (but inefficient) behavior. We do this to prevent misbehaving guests (e.g. OS X <= 10.7) from receiving invalid opcode faults after failing to check for monitor/mwait availability via cpuid. Since mwait-based idle loops relying on these nop-emulated instructions would keep the host CPU pegged at 100%, do NOT advertise their presence via cpuid, preventing compliant guests from ever using them inadvertently. Signed-off-by: Gabriel L. Somlo --- On Wed, May 07, 2014 at 05:30:47PM +0200, Paolo Bonzini wrote: > Il 07/05/2014 17:05, Michael S. Tsirkin ha scritto: > >>> 2. Emulate monitor and mwait as "nop", but continue to claim they are > >>> not supported via CPUID. That's the patch you cited. Not sure > >>> though whether that sort of "undocumented" functionality would be > >>> OK with the KVM crowd, though :) > >I'd go for this one. It seems unlikely a guest wants to get > >an exception intentionally. > >Paolo? > > That's okay, but please add a printk_once the first time mwait is called. OK, here's a first pass at an official submission. I have two questions: 1. I can't test svm.c (on AMD). As such, I'm not sure the skip_emulated_instruction() call in my own version of nop_interception() is necessary. If not, I could probably just call the already existing nop_on_interception() (line 1926 or thereabouts in svm.c), which just returns returns 1 without skipping anything. 2. I get "defined but not used" warnings on invalid_op_interception() (svm.c) and handle_invalid_op() (vmx.c). Apparently monitor/mwait are currently the only VM exit reasons which lead to an "invalid opcode" exception. Should my patch just nuke those functions (so that if anyone needs them in the future they'd have to re-add them), or comment them out, or call them after the "return 1;" statement in the monitor/mwait functions to shut up gcc, or ??? :) Thanks much, Gabriel arch/x86/kvm/cpuid.c | 2 ++ arch/x86/kvm/svm.c | 22 ++++++++++++++++++++-- arch/x86/kvm/vmx.c | 22 ++++++++++++++++++++-- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index f47a104..d094fc6 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -283,6 +283,8 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, 0 /* Reserved */ | f_lm | F(3DNOWEXT) | F(3DNOW); /* cpuid 1.ecx */ const u32 kvm_supported_word4_x86_features = + /* NOTE: MONITOR (and MWAIT) are emulated as NOP, + * but *not* advertised to guests via CPUID ! */ F(XMM3) | F(PCLMULQDQ) | 0 /* DTES64, MONITOR */ | 0 /* DS-CPL, VMX, SMX, EST */ | 0 /* TM2 */ | F(SSSE3) | 0 /* CNXT-ID */ | 0 /* Reserved */ | diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 7f4f9c2..1976488 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -3287,6 +3287,24 @@ 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, @@ -3344,8 +3362,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] = invalid_op_interception, - [SVM_EXIT_MWAIT] = invalid_op_interception, + [SVM_EXIT_MONITOR] = monitor_interception, + [SVM_EXIT_MWAIT] = mwait_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 33e8c02..060b384 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -5669,6 +5669,24 @@ static int handle_pause(struct kvm_vcpu *vcpu) return 1; } +static int handle_nop(struct kvm_vcpu *vcpu) +{ + skip_emulated_instruction(vcpu); + return 1; +} + +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); +} + static int handle_invalid_op(struct kvm_vcpu *vcpu) { kvm_queue_exception(vcpu, UD_VECTOR); @@ -6571,8 +6589,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_invalid_op, - [EXIT_REASON_MONITOR_INSTRUCTION] = handle_invalid_op, + [EXIT_REASON_MWAIT_INSTRUCTION] = handle_mwait, + [EXIT_REASON_MONITOR_INSTRUCTION] = handle_monitor, [EXIT_REASON_INVEPT] = handle_invept, };