From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: ARC-Seal: i=1; a=rsa-sha256; t=1516375540; cv=none; d=google.com; s=arc-20160816; b=pj1OkIVsSBDsh4s4cXji2r+ypYMvwtfW6X96YyXVAg2J5VL6veqIxRqDXZI6P2/lG8 8xOUnXUvHG047QpL0UgkL09ICkcdJXy7sDzWAT6L3l6BonC9ZZ55T+IC0VTvRiGi25Zs RmvkyL5B2Q654sh4UjCFjCUDTNvMxvJx8inRwTuqw1UBUcTinKd3s8SCSJH5o/RTkJkT PKhsrKEGI9ohJo3x9ZOm2ZdY0H9tyyDg2Yb4cbhR47az68MTOmL1AHyw+SAzeAM0DNie KrYDnPjZHhAr62VMlH89wch8VWDOmqTrRdp3Q1bIWqzF1rEFzDiEikuK/GH4LPOVxdV0 YR9A== 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:references:cc:to:from:subject:sender :dkim-signature:arc-authentication-results; bh=OMO7k+NGlpp39Ik+oTH4IMb+R4CEGM1wGDjLTMjDgDM=; b=o3d8EQVKQe1CvrYDE2IE2JeGDvHrnJC7sFvCB/yIpwPj6nXXHsU2Vsoi3PDj13JPzV GdnlK/nY6fWUOqR9DcL2qfpJpHEq7XfVsn8mNk55QF+umes5Sb03T37Ttvflsf6GeC4g u8Ies9zYn6g/vqR+K764cJMrM86rhUDh2Lh71RCdl3SR02Is+tSY14ffpIHGoR/4/XDl d3LpGugQr42MGe9DJS3TakoK1F9nSr9RhJTXNb1dJjT/D+eOS6AXQ0UV6V0KMg65oeCu qJPmVUMBo+DiChm7v8tpcc6E2LKHN/3RwgbCPj20uhkDRabzpWUkJO1Rxq4pnktGONf/ x8Nw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=S8fvEkzN; spf=pass (google.com: domain of paolo.bonzini@gmail.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=paolo.bonzini@gmail.com; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=S8fvEkzN; spf=pass (google.com: domain of paolo.bonzini@gmail.com designates 209.85.220.65 as permitted sender) smtp.mailfrom=paolo.bonzini@gmail.com; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com X-Google-Smtp-Source: ACJfBosaSV+N/7Q+uQIXhifAGwLxQRZiISRoE2UuquS5JSkVNIa0qzW+LdulYJT82JVZbbXYb8VA3g== Sender: Paolo Bonzini Subject: Re: [PATCH 34/35] x86/kvm: Add IBPB support From: Paolo Bonzini To: Peter Zijlstra , David Woodhouse , Thomas Gleixner , Josh Poimboeuf Cc: linux-kernel@vger.kernel.org, Dave Hansen , Ashok Raj , Tim Chen , Andy Lutomirski , Linus Torvalds , Greg KH , Andrea Arcangeli , Andi Kleen , Arjan Van De Ven , Dan Williams , Jun Nakajima , Asit Mallick , Jason Baron , David Woodhouse References: <20180118134800.711245485@infradead.org> <20180118140153.498071980@infradead.org> Message-ID: Date: Fri, 19 Jan 2018 16:25:35 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-LABELS: =?utf-8?b?IlxcSW1wb3J0YW50Ig==?= X-GMAIL-THRID: =?utf-8?q?1589944833614136394?= X-GMAIL-MSGID: =?utf-8?q?1590034998915255339?= X-Mailing-List: linux-kernel@vger.kernel.org List-ID: On 18/01/2018 16:32, Paolo Bonzini wrote: > On 18/01/2018 14:48, Peter Zijlstra wrote: >> From: Ashok Raj >> >> Add MSR passthrough for MSR_IA32_PRED_CMD and place branch predictor >> barriers on switching between VMs to avoid inter VM specte-v2 attacks. >> >> [peterz: rebase and changelog rewrite] >> >> Cc: Asit Mallick >> Cc: Dave Hansen >> Cc: Arjan Van De Ven >> Cc: Tim Chen >> Cc: Linus Torvalds >> Cc: Andrea Arcangeli >> Cc: Andi Kleen >> Cc: Thomas Gleixner >> Cc: Dan Williams >> Cc: Jun Nakajima >> Cc: Andy Lutomirski >> Cc: Greg KH >> Cc: David Woodhouse >> Cc: Paolo Bonzini >> Signed-off-by: Ashok Raj >> Signed-off-by: Peter Zijlstra (Intel) >> --- >> arch/x86/kvm/svm.c | 8 ++++++++ >> arch/x86/kvm/vmx.c | 8 ++++++++ >> 2 files changed, 16 insertions(+) > > This patch is missing the AMD-specific CPUID bit. > > In addition, it is not bisectable because guests will see the SPEC_CTRL CPUID > bit after patch 32 even if PRED_CMD is not supported yet. > > The simplest solutions are: > > 1) add the indirect_branch_prediction_barrier() calls first, and squash > everything else including the AMD CPUID bit into a single patch. > > 2) place the IBPB in this series, and only add stop/restart_indirect_branch_ > speculation() to the vmexit and vmentry paths. We will do the guest enabling > in kvm.git after this is ready, it should only take a week and we'll ensure > it is backportable to 4.14 and 4.15. > > 3) same as (2) except we'll send the guest enabling through TIP. > > Paolo > >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -252,6 +252,7 @@ static const struct svm_direct_access_ms >> { .index = MSR_SYSCALL_MASK, .always = true }, >> #endif >> { .index = MSR_IA32_SPEC_CTRL, .always = true }, >> + { .index = MSR_IA32_PRED_CMD, .always = true }, >> { .index = MSR_IA32_LASTBRANCHFROMIP, .always = false }, >> { .index = MSR_IA32_LASTBRANCHTOIP, .always = false }, >> { .index = MSR_IA32_LASTINTFROMIP, .always = false }, >> @@ -532,6 +533,7 @@ struct svm_cpu_data { >> struct kvm_ldttss_desc *tss_desc; >> >> struct page *save_area; >> + struct vmcb *current_vmcb; >> }; >> >> static DEFINE_PER_CPU(struct svm_cpu_data *, svm_data); >> @@ -1709,11 +1711,13 @@ static void svm_free_vcpu(struct kvm_vcp >> __free_pages(virt_to_page(svm->nested.msrpm), MSRPM_ALLOC_ORDER); >> kvm_vcpu_uninit(vcpu); >> kmem_cache_free(kvm_vcpu_cache, svm); >> + indirect_branch_prediction_barrier(); >> } >> >> static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu) >> { >> struct vcpu_svm *svm = to_svm(vcpu); >> + struct svm_cpu_data *sd = per_cpu(svm_data, cpu); >> int i; >> >> if (unlikely(cpu != vcpu->cpu)) { >> @@ -1742,6 +1746,10 @@ static void svm_vcpu_load(struct kvm_vcp >> if (static_cpu_has(X86_FEATURE_RDTSCP)) >> wrmsrl(MSR_TSC_AUX, svm->tsc_aux); >> >> + if (sd->current_vmcb != svm->vmcb) { >> + sd->current_vmcb = svm->vmcb; >> + indirect_branch_prediction_barrier(); >> + } >> avic_vcpu_load(vcpu, cpu); >> } >> >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -2280,6 +2280,7 @@ static void vmx_vcpu_load(struct kvm_vcp >> if (per_cpu(current_vmcs, cpu) != vmx->loaded_vmcs->vmcs) { >> per_cpu(current_vmcs, cpu) = vmx->loaded_vmcs->vmcs; >> vmcs_load(vmx->loaded_vmcs->vmcs); >> + indirect_branch_prediction_barrier(); >> } >> >> if (!already_loaded) { >> @@ -3837,6 +3838,11 @@ static void free_loaded_vmcs(struct load >> free_vmcs(loaded_vmcs->vmcs); >> loaded_vmcs->vmcs = NULL; >> WARN_ON(loaded_vmcs->shadow_vmcs != NULL); >> + /* >> + * The VMCS could be recycled, causing a false negative in vmx_vcpu_load >> + * block speculative execution. >> + */ >> + indirect_branch_prediction_barrier(); > > This IBPB is not needed, as loaded_vmcs->NULL is now NULL and there will be a > barrier the next time vmx_vcpu_load is called on this CPU. Without retpolines, KVM userspace is not protected from the guest poisoning the BTB, because there is no IBRS-barrier on the vmexit path. So there are two more IBPBs that are needed if retpolines are enabled: 1) in kvm_sched_out 2) at the end of vcpu_run Thanks, Paolo