From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,HK_RANDOM_FROM,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5451CC4320A for ; Tue, 3 Aug 2021 00:38:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 37B9060EFD for ; Tue, 3 Aug 2021 00:38:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233232AbhHCAic (ORCPT ); Mon, 2 Aug 2021 20:38:32 -0400 Received: from mga14.intel.com ([192.55.52.115]:59360 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232540AbhHCAia (ORCPT ); Mon, 2 Aug 2021 20:38:30 -0400 X-IronPort-AV: E=McAfee;i="6200,9189,10064"; a="213287155" X-IronPort-AV: E=Sophos;i="5.84,290,1620716400"; d="scan'208";a="213287155" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Aug 2021 17:38:19 -0700 X-IronPort-AV: E=Sophos;i="5.84,290,1620716400"; d="scan'208";a="510573535" Received: from xiaoyaol-mobl.ccr.corp.intel.com (HELO [10.249.175.54]) ([10.249.175.54]) by fmsmga003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Aug 2021 17:38:15 -0700 Subject: Re: [PATCH v2] KVM: VMX: Enable Notify VM exit To: Sean Christopherson Cc: Tao Xu , pbonzini@redhat.com, vkuznets@redhat.com, wanpengli@tencent.com, jmattson@google.com, joro@8bytes.org, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, hpa@zytor.com, x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org References: <20210525051204.1480610-1-tao3.xu@intel.com> From: Xiaoyao Li Message-ID: <080602dc-f998-ec13-ddf9-42902aa477de@intel.com> Date: Tue, 3 Aug 2021 08:38:13 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Firefox/78.0 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 8/2/2021 11:46 PM, Sean Christopherson wrote: > On Mon, Aug 02, 2021, Xiaoyao Li wrote: >> On 7/31/2021 4:41 AM, Sean Christopherson wrote: >>> On Tue, May 25, 2021, Tao Xu wrote: >>>> #endif /* __KVM_X86_VMX_CAPS_H */ >>>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c >>>> index 4bceb5ca3a89..c0ad01c88dac 100644 >>>> --- a/arch/x86/kvm/vmx/vmx.c >>>> +++ b/arch/x86/kvm/vmx/vmx.c >>>> @@ -205,6 +205,10 @@ module_param(ple_window_max, uint, 0444); >>>> int __read_mostly pt_mode = PT_MODE_SYSTEM; >>>> module_param(pt_mode, int, S_IRUGO); >>>> +/* Default is 0, less than 0 (for example, -1) disables notify window. */ >>>> +static int __read_mostly notify_window; >>> >>> I'm not sure I like the idea of trusting ucode to select an appropriate internal >>> threshold. Unless the internal threshold is architecturally defined to be at >>> least N nanoseconds or whatever, I think KVM should provide its own sane default. >>> E.g. it's not hard to imagine a scenario where a ucode patch gets rolled out that >>> adjusts the threshold and starts silently degrading guest performance. >> >> You mean when internal threshold gets smaller somehow, and cases >> false-positive that leads unexpected VM exit on normal instruction? In this >> case, we set increase the vmcs.notify_window in KVM. > > Not while VMs are running though. > >> I think there is no better to avoid this case if ucode changes internal >> threshold. Unless KVM's default notify_window is bigger enough. >> >>> Even if the internal threshold isn't architecturally constrained, it would be very, >>> very helpful if Intel could publish the per-uarch/stepping thresholds, e.g. to give >>> us a ballpark idea of how agressive KVM can be before it risks false positives. >> >> Even Intel publishes the internal threshold, we still need to provide a >> final best_value (internal + vmcs.notify_window). Then what's that value? > > The ideal value would be high enough to guarantee there are zero false positives, > yet low enough to prevent a malicious guest from causing instability in the host > by blocking events for an extended duration. The problem is that there's no > magic answer for the threshold at which a blocked event would lead to system > instability, and without at least a general idea of the internal value there's no > answer at all. > > IIRC, SGX instructions have a hard upper bound of 25k cycles before they have to > check for pending interrupts, e.g. it's why EINIT is interruptible. The 25k cycle > limit is likely a good starting point for the combined minimum. That's why I want > to know the internal minimum; if the internal minimum is _guaranteed_ to be >25k, > then KVM can be more aggressive with its default value. OK. I will go internally to see if we can publish the internal threshold. >> If we have an option for final best_value, then I think it's OK to just let >> vmcs.notify_window = best_value. Then the true final value is best_value + >> internal. >> - if it's a normal instruction, it should finish within best_value or >> best_value + internal. So it makes no difference. >> - if it's an instruction in malicious case, it won't go to next instruction >> whether wait for best_value or best_value + internal. > > ... > >>>> + >>>> vmcs_write32(PAGE_FAULT_ERROR_CODE_MASK, 0); >>>> vmcs_write32(PAGE_FAULT_ERROR_CODE_MATCH, 0); >>>> vmcs_write32(CR3_TARGET_COUNT, 0); /* 22.2.1 */ >>>> @@ -5642,6 +5653,31 @@ static int handle_bus_lock_vmexit(struct kvm_vcpu *vcpu) >>>> return 0; >>>> } >>>> +static int handle_notify(struct kvm_vcpu *vcpu) >>>> +{ >>>> + unsigned long exit_qual = vmx_get_exit_qual(vcpu); >>>> + >>>> + if (!(exit_qual & NOTIFY_VM_CONTEXT_INVALID)) { >>> >>> What does CONTEXT_INVALID mean? The ISE doesn't provide any information whatsoever. >> >> It means whether the VM context is corrupted and not valid in the VMCS. > > Well that's a bit terrifying. Under what conditions can the VM context become > corrupted? E.g. if the context can be corrupted by an inopportune NOTIFY exit, > then KVM needs to be ultra conservative as a false positive could be fatal to a > guest. > Short answer is no case will set the VM_CONTEXT_INVALID bit. VM_CONTEXT_INVALID is so fatal and IMHO it won't be set for any inopportune NOTIFY exit.