From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Google-Smtp-Source: AH8x226yyM+LqrWKNXFJ+JbuXGNUNju5BkimRf+3yNIYz4o57eeYfJ8IWdwzFCIwyWO2O0unJYz2 ARC-Seal: i=1; a=rsa-sha256; t=1517223312; cv=none; d=google.com; s=arc-20160816; b=CfjJay5O4ML9fb9/TCeTzlH6eh9Rbo7I9c12vLWY9i1kwpuI4Sl/O622YwtfD7An5O 8MElUFcKNdIF2sjNSPHWczNA70hJwJyKdLOn2c4U8nk1ItseeWODlvkpjzZXLj+T401e fLIGrFFaMFmNf1XTjIfMNrKlOIjHZiLn+GtU8fam1a8MAF39+PvOnex/KiAgROFlowJ+ LkTGOqQDr2i/c+iZjJkuB+Bo1LimF1H3Db9qhS1MZiq1y0oWWgRzv8gbUNFgSqs8KRlu 1HqauKRWxqAj/cd4zErjiSGXNdbXde0D6mttNd/OHxGY7ft2CO6aC4LYxOuBPA994sYl iNEA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :arc-authentication-results; bh=ODKFidiUFOCuM7OUKYJPmNSTI6k31J4JACTm+rcauvw=; b=eTQRsNNni+UU3eMMkbes3D2isZsK8BTgGgpGBFnz94OTKSP4HmszzooPmzpAYJ+Emh Nngbf+BL27jSOewpBXtKw//I3mynXZo4Slr3825pbmURMu4j/g3lPQ2sL1ugLdlBkZSX T4ZwokjI0cLlRHaUU/UcGCq+AlM+tKhM38phgZPacoxD4fv0kFGKFH6PuQUl/Xld31N7 4Qa6+4mZ+0c/qjXG7BlEZibYPLa1BEhO0iup9Igd62KtZbFQHtMTTuWqZKy30ljJdG+6 oUmG0sLWiq1UCVCYi7PeJHgaG0cNUZal59K8+lc3Z7KW+Ui3SY/vkWIDI/W5vCdvQYqc 7QAw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of pbonzini@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=pbonzini@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Authentication-Results: mx.google.com; spf=pass (google.com: domain of pbonzini@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=pbonzini@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Subject: Re: [PATCH] x86: vmx: Allow direct access to MSR_IA32_SPEC_CTRL To: David Woodhouse , KarimAllah Ahmed , Liran Alon Cc: konrad.wilk@oracle.com, luto@kernel.org, tglx@linutronix.de, torvalds@linux-foundation.org, gregkh@linuxfoundation.org, asit.k.mallick@intel.com, dave.hansen@intel.com, karahmed@amazon.de, jun.nakajima@intel.com, dan.j.williams@intel.com, ashok.raj@intel.com, daniel.kiper@oracle.com, arjan.van.de.ven@intel.com, tim.c.chen@linux.intel.com, linux-kernel@vger.kernel.org, ak@linux.intel.com, kvm@vger.kernel.org, aarcange@redhat.com References: <6b9a1ec2-5ebd-4624-a825-3f31db5cefb5@default> <1517215563.6624.118.camel@infradead.org> <8bed4a5a-afc6-1569-d9bf-a3e1103e92f8@amazon.com> <1517222264.6624.131.camel@infradead.org> From: Paolo Bonzini Message-ID: <23d496d7-37f0-166d-9b3a-7cf731d8244f@redhat.com> Date: Mon, 29 Jan 2018 11:55:02 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1517222264.6624.131.camel@infradead.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: =?utf-8?q?1590865729989295970?= X-GMAIL-MSGID: =?utf-8?q?1590923952420371480?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On 29/01/2018 11:37, David Woodhouse wrote: > On Mon, 2018-01-29 at 10:43 +0100, KarimAllah Ahmed wrote: >> On 01/29/2018 09:46 AM, David Woodhouse wrote: >>> Reading the code and comparing with the SDM, I can't see where we're >>> ever setting VM_EXIT_MSR_STORE_{ADDR,COUNT} except in the nested >>> case... >> Hmmm ... you are probably right! I think all users of this interface >> always trap + update save area and never passthrough the MSR. That is >> why only LOAD is needed *so far*. >> >> Okay, let me sort this out in v3 then. > > I'm starting to think a variant of Ashok's patch might actually be the > simpler approach, and not "premature optimisation". Especially if we > need to support the !cpu_has_vmx_msr_bitmaps() case? That case is awfully slow anyway, it doesn't matter, but the direct-access flag would simply be always 0 if you have no MSR bitmaps. > Start with vmx->spec_ctrl set to zero. When first touched, make it > passthrough (but not atomically switched) and set a flag (e.g. > "spec_ctrl_live") which triggers the 'restore_branch_speculation' and > 'save_and_restrict_branch_speculation' behaviours. Except don't use > those macros. Those can look something like > >  /* If this vCPU has touched SPEC_CTRL then restore its value if needed */ >  if (vmx->spec_ctrl_live && vmx->spec_ctrl) >      wrmsrl(MSR_IA32_SPEC_CTRL, vmx->spec_ctrl); >  /* vmentry is serialising on affected CPUs, so the conditional branch is safe */ > > > ... and, respectively, ... > > /* If this vCPU has touched SPEC_CTRL then save its value and ensure we have zero */ > if (vmx->spec_ctrl_live) { > rdmsrl(MSR_IA32_SPEC_CTRL, vmx->spec_ctrl); > if (vmx->spec_ctrl) > wrmsrl(MSR_IA32_SPEC_CTRL, 0); > } > > Perhaps we can ditch the separate 'spec_ctrl_live' flag and check the > pass-through MSR bitmap directly, in the case that it exists?  Probably a cache miss, or even a TLB miss if you're unlucky, so the separate flag is okay. Paolo