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@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Cc: "Radim Krčmář" <rkrcmar@xxxxxxxxxx>
> Cc: Joerg Roedel <joro@xxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxx>
> Cc: Tom Lendacky <thomas.lendacky@xxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: kvm@xxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
> ---
> 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.
--
