Re: [PATCH Part2 RFC v4 26/40] KVM: SVM: Add KVM_SEV_SNP_LAUNCH_FINISH command
From: Sean Christopherson <seanjc@google.com>
Date: 2021-07-16 20:18:46
Also in:
kvm, linux-crypto, linux-efi, linux-mm, lkml, platform-driver-x86
On Wed, Jul 07, 2021, Brijesh Singh wrote:
+ struct kvm_sev_snp_launch_finish {
+ __u64 id_block_uaddr;
+ __u64 id_auth_uaddr;
+ __u8 id_block_en;
+ __u8 auth_key_en;
+ __u8 host_data[32];Pad this one too?
+ }; + + +See SEV-SNP specification for further details on launch finish input parameters.
...
+ data->gctx_paddr = __psp_pa(sev->snp_context); + ret = sev_issue_cmd(kvm, SEV_CMD_SNP_LAUNCH_FINISH, data, &argp->error);
Shouldn't KVM unwind everything it did if LAUNCH_FINISH fails? And if that's not possible, take steps to make the VM unusable?
+ + kfree(id_auth); + +e_free_id_block: + kfree(id_block); + +e_free: + kfree(data); + + return ret; +} +
...
quoted hunk ↗ jump to hunk
@@ -2346,8 +2454,25 @@ void sev_free_vcpu(struct kvm_vcpu *vcpu) if (vcpu->arch.guest_state_protected) sev_flush_guest_memory(svm, svm->vmsa, PAGE_SIZE); + + /* + * If its an SNP guest, then VMSA was added in the RMP entry as a guest owned page. + * Transition the page to hyperivosr state before releasing it back to the system.
"hyperivosr" typo. And please wrap at 80 chars.
+ */
+ if (sev_snp_guest(vcpu->kvm)) {
+ struct rmpupdate e = {};
+ int rc;
+
+ rc = rmpupdate(virt_to_page(svm->vmsa), &e);So why does this not need to go through snp_page_reclaim()?
+ if (rc) {
+ pr_err("Failed to release SNP guest VMSA page (rc %d), leaking it\n", rc);
Seems like a WARN would be simpler. But the more I see the rmpupdate(..., {0})
pattern, the more I believe that nuking an RMP entry needs a dedicated helper.
+ goto skip_vmsa_free;