Messages in this thread | | | Date | Fri, 27 Oct 2017 22:24:21 +0200 | From | Borislav Petkov <> | Subject | Re: [Part2 PATCH v6 25/38] KVM: SVM: Add KVM_SEV_INIT command |
| |
On Thu, Oct 19, 2017 at 09:34:00PM -0500, Brijesh Singh wrote: > The command initializes the SEV platform context and allocates a new ASID > for this guest from the SEV ASID pool. The firmware must be initialized > before we issue any guest launch commands to create a new memory encryption > context. > > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: "Radim Krčmář" <rkrcmar@redhat.com> > Cc: Joerg Roedel <joro@8bytes.org> > Cc: Borislav Petkov <bp@suse.de> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: x86@kernel.org > Cc: kvm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > arch/x86/include/asm/kvm_host.h | 7 +++ > arch/x86/kvm/svm.c | 132 +++++++++++++++++++++++++++++++++++++++- > 2 files changed, 138 insertions(+), 1 deletion(-)
...
> @@ -1167,10 +1187,18 @@ static __init int svm_hardware_setup(void) > return r; > } > > +static __exit void sev_hardware_unsetup(void) > +{ > + kfree(sev_asid_bitmap); > +} > + > static __exit void svm_hardware_unsetup(void) > { > int cpu; > > + if (svm_sev_enabled()) > + sev_hardware_unsetup();
Move that svm_sev_enabled() check into the function.
> + > for_each_possible_cpu(cpu) > svm_cpu_uninit(cpu); > > @@ -1361,6 +1389,9 @@ static void init_vmcb(struct vcpu_svm *svm) > svm->vmcb->control.int_ctl |= V_GIF_ENABLE_MASK; > } > > + if (sev_guest(svm->vcpu.kvm)) > + svm->vmcb->control.nested_ctl |= SVM_NESTED_CTL_SEV_ENABLE; > + > mark_all_dirty(svm->vmcb); > > enable_gif(svm); > @@ -1443,6 +1474,28 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu) > return 0; > } > > +static void sev_asid_free(struct kvm *kvm) > +{ > + struct kvm_sev_info *sev = &kvm->arch.sev_info; > + int pos, asid; > + > + if (!svm_sev_enabled()) > + return;
You're already checking !sev_guest() below - no need to do the svm_sev_enabled() check again.
> + > + asid = sev->asid; > + pos = asid - 1; > + clear_bit(pos, sev_asid_bitmap); > +} > + > +static void sev_vm_destroy(struct kvm *kvm) > +{ > + if (!sev_guest(kvm)) > + return; > + > + sev_platform_shutdown(NULL); > + sev_asid_free(kvm); > +} > + > static void avic_vm_destroy(struct kvm *kvm) > { > unsigned long flags; > @@ -1461,6 +1514,12 @@ static void avic_vm_destroy(struct kvm *kvm) > spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
> } > > +static void svm_vm_destroy(struct kvm *kvm) > +{ > + avic_vm_destroy(kvm); > + sev_vm_destroy(kvm); > +} > + > static int avic_vm_init(struct kvm *kvm) > { > unsigned long flags; > @@ -5427,6 +5486,75 @@ static void svm_setup_mce(struct kvm_vcpu *vcpu) > vcpu->arch.mcg_cap &= 0x1ff; > } > > +static int sev_asid_new(void) > +{ > + int pos; > + > + if (!svm_sev_enabled()) > + return -EINVAL;
You already checked that in svm_mem_enc_op() - no need to do it again.
> + pos = find_first_zero_bit(sev_asid_bitmap, max_sev_asid); > + if (pos >= max_sev_asid) > + return -EBUSY; > + > + set_bit(pos, sev_asid_bitmap); > + return pos + 1; > +} > + > +static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp) > +{ > + struct kvm_sev_info *sev = &kvm->arch.sev_info; > + int asid, ret; > + > + ret = sev_platform_init(NULL, &argp->error); > + if (ret) > + return ret; > + > + ret = -EBUSY; > + asid = sev_asid_new();
This operation is cheaper so do it first and then sev_platform_init() so that in case sev_asid_new() fails, you don't "toggle" the PSP unnecessarily.
> + if (asid < 0) > + goto e_shutdown; > + > + sev->active = true; > + sev->asid = asid; > + > + return 0; > + > +e_shutdown: > + sev_platform_shutdown(&argp->error); > + return ret; > +} > +
-- Regards/Gruss, Boris.
ECO tip #101: Trim your mails when you reply. --
| |