Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751626AbdIMPh7 (ORCPT ); Wed, 13 Sep 2017 11:37:59 -0400 Received: from mx2.suse.de ([195.135.220.15]:54493 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751062AbdIMPh6 (ORCPT ); Wed, 13 Sep 2017 11:37:58 -0400 Date: Wed, 13 Sep 2017 17:37:46 +0200 From: Borislav Petkov To: Brijesh Singh Cc: linux-kernel@vger.kernel.org, x86@kernel.org, kvm@vger.kernel.org, Thomas Gleixner , Joerg Roedel , "Michael S . Tsirkin" , Paolo Bonzini , =?utf-8?B?XCJSYWRpbSBLcsSNbcOhxZlcIg==?= , Tom Lendacky Subject: Re: [RFC Part2 PATCH v3 14/26] KVM: SVM: VMRUN should use assosiated ASID when SEV is enabled Message-ID: <20170913153746.lq74vpr465d4pjaq@pd.tnic> References: <20170724200303.12197-1-brijesh.singh@amd.com> <20170724200303.12197-15-brijesh.singh@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170724200303.12197-15-brijesh.singh@amd.com> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3303 Lines: 127 On Mon, Jul 24, 2017 at 03:02:51PM -0500, Brijesh Singh wrote: > SEV hardware uses ASIDs to associate memory encryption key with the > guest VMs. During the guest creation time, we use SEV_CMD_ACTIVATE "VM" > command to bind a particular ASID to the guest. Lets make sure that > VMCB is programmed with the binded ASID before a VMRUN. "the VMCB" binded? you mean "bound" > > Signed-off-by: Brijesh Singh > --- > arch/x86/kvm/svm.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 49 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index e99a572..72f7c27 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -213,6 +213,9 @@ struct vcpu_svm { > */ > struct list_head ir_list; > spinlock_t ir_list_lock; > + > + /* which host cpu was used for running this vcpu */ s/cpu/CPU/ > + unsigned int last_cpuid; ... and since it is a CPU, then "last_cpu" I guess. > }; > > /* > @@ -573,6 +576,8 @@ struct svm_cpu_data { > struct kvm_ldttss_desc *tss_desc; > > struct page *save_area; > + > + struct vmcb **sev_vmcbs; /* index = sev_asid, value = vmcb pointer */ Put that comment above it. > static DEFINE_PER_CPU(struct svm_cpu_data *, svm_data); > @@ -886,6 +891,7 @@ static void svm_cpu_uninit(int cpu) > return; > > per_cpu(svm_data, raw_smp_processor_id()) = NULL; > + kfree(sd->sev_vmcbs); > __free_page(sd->save_area); > kfree(sd); > } > @@ -904,6 +910,14 @@ static int svm_cpu_init(int cpu) > if (!sd->save_area) > goto err_1; > > + if (svm_sev_enabled()) { > + sd->sev_vmcbs = kmalloc((max_sev_asid + 1) * sizeof(void *), > + GFP_KERNEL); You can let that line stick out. > + r = -ENOMEM; That assignment usually is done before the call. I know, this function does it differently but you can fix it up while you're touching it. > + if (!sd->sev_vmcbs) > + goto err_1; > + } > + > per_cpu(svm_data, cpu) = sd; > > return 0; > @@ -4442,12 +4456,40 @@ static void reload_tss(struct kvm_vcpu *vcpu) > load_TR_desc(); > } > > +static void pre_sev_run(struct vcpu_svm *svm) > +{ > + int cpu = raw_smp_processor_id(); > + int asid = sev_get_asid(svm->vcpu.kvm); > + struct svm_cpu_data *sd = per_cpu(svm_data, cpu); > + > + /* Assign the asid allocated with this SEV guest */ > + svm->vmcb->control.asid = asid; > + > + /* > + * Flush guest TLB: > + * > + * 1) when different VMCB for the same ASID is to be run on the same host CPU. > + * 2) or this VMCB was executed on different host cpu in previous VMRUNs. s/cpu/CPU/ > + */ > + if (sd->sev_vmcbs[asid] == svm->vmcb && > + svm->last_cpuid == cpu) > + return; > + > + svm->last_cpuid = cpu; > + sd->sev_vmcbs[asid] = svm->vmcb; > + svm->vmcb->control.tlb_ctl = TLB_CONTROL_FLUSH_ASID; > + mark_dirty(svm->vmcb, VMCB_ASID); > +} > + > static void pre_svm_run(struct vcpu_svm *svm) > { > int cpu = raw_smp_processor_id(); > > struct svm_cpu_data *sd = per_cpu(svm_data, cpu); > > + if (sev_guest(svm->vcpu.kvm)) > + return pre_sev_run(svm); Just pass @cpu here so that you don't have to do raw_smp_processor_id() again above. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --