diff mbox series

[v12,12/29] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_FINISH command

Message ID 20240329225835.400662-13-michael.roth@amd.com (mailing list archive)
State New
Headers show
Series Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support | expand

Commit Message

Michael Roth March 29, 2024, 10:58 p.m. UTC
Add a KVM_SEV_SNP_LAUNCH_FINISH command to finalize the cryptographic
launch digest and stores it as the measurement of the guest at launch
time. Also extend the existing SNP firmware data structures to support
enforcing the use of Version Loaded Endorsement Keys by guests as part
of this command.

While finalizing the launch flow, it also issues the LAUNCH_UPDATE SNP
firmware commands to encrypt/measure the initial VMSA pages for each
configured vCPU. This involves setting the RMP entries for those pages
to provide, so also add handling to clean up the RMP entries for these
pages whening free'ing vCPUs.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
Signed-off-by: Harald Hoyer <harald@profian.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
[mdr: always measure BSP first to get consistent launch measurements]
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
 .../virt/kvm/x86/amd-memory-encryption.rst    |  26 ++++
 arch/x86/include/uapi/asm/kvm.h               |  15 ++
 arch/x86/kvm/svm/sev.c                        | 137 ++++++++++++++++++
 include/linux/psp-sev.h                       |   4 +-
 4 files changed, 181 insertions(+), 1 deletion(-)

Comments

Michael Roth April 1, 2024, 11:17 p.m. UTC | #1
On Sat, Mar 30, 2024 at 09:41:30PM +0100, Paolo Bonzini wrote:
> On 3/29/24 23:58, Michael Roth wrote:
> > 
> > +		/* Handle boot vCPU first to ensure consistent measurement of initial state. */
> > +		if (!boot_vcpu_handled && vcpu->vcpu_id != 0)
> > +			continue;
> > +
> > +		if (boot_vcpu_handled && vcpu->vcpu_id == 0)
> > +			continue;
> 
> Why was this not necessary for KVM_SEV_LAUNCH_UPDATE_VMSA?  Do we need it
> now?

I tried to find the original discussion for more context, but can't seem to
locate it. But AIUI, there are cases where a VMM may create AP vCPUs earlier
than it does the BSP, in which case kvm_for_each_vcpu() might return an AP
as it's first entry and cause that VMSA to get measured before, leading
to a different measurement depending on the creation ordering.

Measuring the BSP first ensures consistent measurement, since the
initial AP contents are all identical so their ordering doesn't matter.

For SNP, it makes sense to take the more consistent approach right off
the bat. But for SEV-ES, it's possible that there are VMMs/userspaces
out there that have already accounted for this in their measurement
calculations, so it could cause issues if we should the behavior for all
SEV-ES. We could however limit the change to KVM_X86_SEV_ES_VM and
document that as part of KVM_SEV_INIT2, since there is similarly chance
for measurement changes their WRT to the new FPU/XSAVE sync'ing that was
added.


> 
> > +See SEV-SNP specification [snp-fw-abi]_ for SNP_LAUNCH_FINISH further details
> > +on launch finish input parameters.
> 
> See SNP_LAUNCH_FINISH in the SEV-SNP specification [snp-fw-abi]_ for further
> details on the input parameters in ``struct kvm_sev_snp_launch_finish``.

Will make similar changes for the others as well. Thanks!

-Mike

> 
> Paolo
> 
>
Paolo Bonzini April 3, 2024, 12:56 p.m. UTC | #2
On Tue, Apr 2, 2024 at 1:18 AM Michael Roth <michael.roth@amd.com> wrote:
>
> On Sat, Mar 30, 2024 at 09:41:30PM +0100, Paolo Bonzini wrote:
> > On 3/29/24 23:58, Michael Roth wrote:
> > >
> > > +           /* Handle boot vCPU first to ensure consistent measurement of initial state. */
> > > +           if (!boot_vcpu_handled && vcpu->vcpu_id != 0)
> > > +                   continue;
> > > +
> > > +           if (boot_vcpu_handled && vcpu->vcpu_id == 0)
> > > +                   continue;
> >
> > Why was this not necessary for KVM_SEV_LAUNCH_UPDATE_VMSA?  Do we need it
> > now?
>
> I tried to find the original discussion for more context, but can't seem to
> locate it. But AIUI, there are cases where a VMM may create AP vCPUs earlier
> than it does the BSP, in which case kvm_for_each_vcpu() might return an AP
> as it's first entry and cause that VMSA to get measured before, leading
> to a different measurement depending on the creation ordering.

I think that would be considered a bug in either the VMM or the
"thing" that computes the measurement.

If that hasn't been a problem for SEV-ES, I'd rather keep the code simple.

> We could however limit the change to KVM_X86_SEV_ES_VM and
> document that as part of KVM_SEV_INIT2, since there is similarly chance
> for measurement changes their WRT to the new FPU/XSAVE sync'ing that was
> added.

Hmm, I need to double check that the FPU/XSAVE syncing doesn't break
existing measurements, too.

Paolo
diff mbox series

Patch

diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
index 4268aa5c380e..a49e8cff9133 100644
--- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
@@ -517,6 +517,32 @@  where the allowed values for page_type are #define'd as::
 See the SEV-SNP spec [snp-fw-abi]_ for further details on how each page type is
 used/measured.
 
+20. KVM_SEV_SNP_LAUNCH_FINISH
+-----------------------------
+
+After completion of the SNP guest launch flow, the KVM_SEV_SNP_LAUNCH_FINISH
+command can be issued to make the guest ready for execution.
+
+Parameters (in): struct kvm_sev_snp_launch_finish
+
+Returns: 0 on success, -negative on error
+
+::
+
+        struct kvm_sev_snp_launch_finish {
+                __u64 id_block_uaddr;
+                __u64 id_auth_uaddr;
+                __u8 id_block_en;
+                __u8 auth_key_en;
+                __u8 vlek_required;
+                __u8 host_data[32];
+                __u8 pad[6];
+        };
+
+
+See SEV-SNP specification [snp-fw-abi]_ for SNP_LAUNCH_FINISH further details
+on launch finish input parameters.
+
 Device attribute API
 ====================
 
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 956eb548c08e..2b08fcbe039a 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -696,6 +696,7 @@  enum sev_cmd_id {
 	/* SNP-specific commands */
 	KVM_SEV_SNP_LAUNCH_START,
 	KVM_SEV_SNP_LAUNCH_UPDATE,
+	KVM_SEV_SNP_LAUNCH_FINISH,
 
 	KVM_SEV_NR_MAX,
 };
@@ -841,6 +842,20 @@  struct kvm_sev_snp_launch_update {
 	__u8 type;
 };
 
+#define KVM_SEV_SNP_ID_BLOCK_SIZE	96
+#define KVM_SEV_SNP_ID_AUTH_SIZE	4096
+#define KVM_SEV_SNP_FINISH_DATA_SIZE	32
+
+struct kvm_sev_snp_launch_finish {
+	__u64 id_block_uaddr;
+	__u64 id_auth_uaddr;
+	__u8 id_block_en;
+	__u8 auth_key_en;
+	__u8 vlek_required;
+	__u8 host_data[KVM_SEV_SNP_FINISH_DATA_SIZE];
+	__u8 pad[6];
+};
+
 #define KVM_X2APIC_API_USE_32BIT_IDS            (1ULL << 0)
 #define KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK  (1ULL << 1)
 
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index a8a8a285b4a4..3d6c030091c2 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -63,6 +63,8 @@  static u64 sev_supported_vmsa_features;
 #define SNP_POLICY_MASK_SMT		BIT_ULL(16)
 #define SNP_POLICY_MASK_SINGLE_SOCKET	BIT_ULL(20)
 
+#define INITIAL_VMSA_GPA 0xFFFFFFFFF000
+
 static u8 sev_enc_bit;
 static DECLARE_RWSEM(sev_deactivate_lock);
 static DEFINE_MUTEX(sev_bitmap_lock);
@@ -2283,6 +2285,125 @@  static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct sev_data_snp_launch_update data = {};
+	bool boot_vcpu_handled = false;
+	struct kvm_vcpu *vcpu;
+	unsigned long i;
+	int ret;
+
+	data.gctx_paddr = __psp_pa(sev->snp_context);
+	data.page_type = SNP_PAGE_TYPE_VMSA;
+
+handle_remaining_vcpus:
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		struct vcpu_svm *svm = to_svm(vcpu);
+		u64 pfn = __pa(svm->sev_es.vmsa) >> PAGE_SHIFT;
+
+		/* Handle boot vCPU first to ensure consistent measurement of initial state. */
+		if (!boot_vcpu_handled && vcpu->vcpu_id != 0)
+			continue;
+
+		if (boot_vcpu_handled && vcpu->vcpu_id == 0)
+			continue;
+
+		/* Perform some pre-encryption checks against the VMSA */
+		ret = sev_es_sync_vmsa(svm);
+		if (ret)
+			return ret;
+
+		/* Transition the VMSA page to a firmware state. */
+		ret = rmp_make_private(pfn, INITIAL_VMSA_GPA, PG_LEVEL_4K, sev->asid, true);
+		if (ret)
+			return ret;
+
+		/* Issue the SNP command to encrypt the VMSA */
+		data.address = __sme_pa(svm->sev_es.vmsa);
+		ret = __sev_issue_cmd(argp->sev_fd, SEV_CMD_SNP_LAUNCH_UPDATE,
+				      &data, &argp->error);
+		if (ret) {
+			snp_page_reclaim(pfn);
+			return ret;
+		}
+
+		svm->vcpu.arch.guest_state_protected = true;
+
+		if (!boot_vcpu_handled) {
+			boot_vcpu_handled = true;
+			goto handle_remaining_vcpus;
+		}
+	}
+
+	return 0;
+}
+
+static int snp_launch_finish(struct kvm *kvm, struct kvm_sev_cmd *argp)
+{
+	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
+	struct kvm_sev_snp_launch_finish params;
+	struct sev_data_snp_launch_finish *data;
+	void *id_block = NULL, *id_auth = NULL;
+	int ret;
+
+	if (!sev_snp_guest(kvm))
+		return -ENOTTY;
+
+	if (!sev->snp_context)
+		return -EINVAL;
+
+	if (copy_from_user(&params, u64_to_user_ptr(argp->data), sizeof(params)))
+		return -EFAULT;
+
+	/* Measure all vCPUs using LAUNCH_UPDATE before finalizing the launch flow. */
+	ret = snp_launch_update_vmsa(kvm, argp);
+	if (ret)
+		return ret;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL_ACCOUNT);
+	if (!data)
+		return -ENOMEM;
+
+	if (params.id_block_en) {
+		id_block = psp_copy_user_blob(params.id_block_uaddr, KVM_SEV_SNP_ID_BLOCK_SIZE);
+		if (IS_ERR(id_block)) {
+			ret = PTR_ERR(id_block);
+			goto e_free;
+		}
+
+		data->id_block_en = 1;
+		data->id_block_paddr = __sme_pa(id_block);
+
+		id_auth = psp_copy_user_blob(params.id_auth_uaddr, KVM_SEV_SNP_ID_AUTH_SIZE);
+		if (IS_ERR(id_auth)) {
+			ret = PTR_ERR(id_auth);
+			goto e_free_id_block;
+		}
+
+		data->id_auth_paddr = __sme_pa(id_auth);
+
+		if (params.auth_key_en)
+			data->auth_key_en = 1;
+	}
+
+	data->vcek_disabled = params.vlek_required;
+
+	memcpy(data->host_data, params.host_data, KVM_SEV_SNP_FINISH_DATA_SIZE);
+	data->gctx_paddr = __psp_pa(sev->snp_context);
+	ret = sev_issue_cmd(kvm, SEV_CMD_SNP_LAUNCH_FINISH, data, &argp->error);
+
+	kfree(id_auth);
+
+e_free_id_block:
+	kfree(id_block);
+
+e_free:
+	kfree(data);
+
+	return ret;
+}
+
 int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_sev_cmd sev_cmd;
@@ -2376,6 +2497,9 @@  int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
 	case KVM_SEV_SNP_LAUNCH_UPDATE:
 		r = snp_launch_update(kvm, &sev_cmd);
 		break;
+	case KVM_SEV_SNP_LAUNCH_FINISH:
+		r = snp_launch_finish(kvm, &sev_cmd);
+		break;
 	default:
 		r = -EINVAL;
 		goto out;
@@ -2866,11 +2990,24 @@  void sev_free_vcpu(struct kvm_vcpu *vcpu)
 
 	svm = to_svm(vcpu);
 
+	/*
+	 * If it's an SNP guest, then the VMSA was marked in the RMP table as
+	 * a guest-owned page. Transition the page to hypervisor state before
+	 * releasing it back to the system.
+	 */
+	if (sev_snp_guest(vcpu->kvm)) {
+		u64 pfn = __pa(svm->sev_es.vmsa) >> PAGE_SHIFT;
+
+		if (host_rmp_make_shared(pfn, PG_LEVEL_4K, true))
+			goto skip_vmsa_free;
+	}
+
 	if (vcpu->arch.guest_state_protected)
 		sev_flush_encrypted_page(vcpu, svm->sev_es.vmsa);
 
 	__free_page(virt_to_page(svm->sev_es.vmsa));
 
+skip_vmsa_free:
 	if (svm->sev_es.ghcb_sa_free)
 		kvfree(svm->sev_es.ghcb_sa);
 }
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 3705c2044fc0..903ddfea8585 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -658,6 +658,7 @@  struct sev_data_snp_launch_update {
  * @id_auth_paddr: system physical address of ID block authentication structure
  * @id_block_en: indicates whether ID block is present
  * @auth_key_en: indicates whether author key is present in authentication structure
+ * @vcek_disabled: indicates whether use of VCEK is allowed for attestation reports
  * @rsvd: reserved
  * @host_data: host-supplied data for guest, not interpreted by firmware
  */
@@ -667,7 +668,8 @@  struct sev_data_snp_launch_finish {
 	u64 id_auth_paddr;
 	u8 id_block_en:1;
 	u8 auth_key_en:1;
-	u64 rsvd:62;
+	u8 vcek_disabled:1;
+	u64 rsvd:61;
 	u8 host_data[32];
 } __packed;