On 10/27/17 3:24 PM, Borislav Petkov wrote: ... >> + >> 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. Sure, I will remove the check. > >> + >> 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. Agreed, I will remove it. >> +{ >> + int pos; >> + >> + if (!svm_sev_enabled()) >> + return -EINVAL; > You already checked that in svm_mem_enc_op() - no need to do it again. Agreed, I will remove it > >> + 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. Sure, I will change the order.