LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/3] clean up interface between KVM and psp
@ 2021-08-16 20:24 Mingwei Zhang
  2021-08-16 20:24 ` [PATCH 1/3] KVM: SVM: move sev_decommission to psp driver Mingwei Zhang
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Mingwei Zhang @ 2021-08-16 20:24 UTC (permalink / raw)
  To: Paolo Bonzini, Brijesh Singh, Tom Lendacky, John Allen
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-crypto, linux-kernel, Alper Gun,
	Borislav Petkov, David Rienjes, Marc Orr, Peter Gonda,
	Vipin Sharma, Mingwei Zhang

This patch set is trying to help make the interface between KVM and psp
cleaner and simpler. In particular, the patches do the following
improvements:
 - avoid the requirement of psp data structures for some psp APIs.
 - hide error handling within psp API, eg., using sev_decommission.
 - hide the serialization requirement between DF_FLUSH and DEACTIVATE.

Mingwei Zhang (3):
  KVM: SVM: move sev_decommission to psp driver
  KVM: SVM: move sev_bind_asid to psp
  KVM: SVM: move sev_unbind_asid and DF_FLUSH logic into psp

 arch/x86/kvm/svm/sev.c       | 69 ++++--------------------------------
 drivers/crypto/ccp/sev-dev.c | 57 +++++++++++++++++++++++++++--
 include/linux/psp-sev.h      | 44 ++++++++++++++++++++---
 3 files changed, 102 insertions(+), 68 deletions(-)

--
2.33.0.rc1.237.g0d66db33f3-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] KVM: SVM: move sev_decommission to psp driver
  2021-08-16 20:24 [PATCH 0/3] clean up interface between KVM and psp Mingwei Zhang
@ 2021-08-16 20:24 ` Mingwei Zhang
  2021-08-16 20:24 ` [PATCH 2/3] KVM: SVM: move sev_bind_asid to psp Mingwei Zhang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Mingwei Zhang @ 2021-08-16 20:24 UTC (permalink / raw)
  To: Paolo Bonzini, Brijesh Singh, Tom Lendacky, John Allen
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-crypto, linux-kernel, Alper Gun,
	Borislav Petkov, David Rienjes, Marc Orr, Peter Gonda,
	Vipin Sharma, Mingwei Zhang

ccp/sev-dev.c is part of the software layer in psp that allows KVM to
manage SEV/ES/SNP enabled VMs. Among the APIs exposed in sev-dev, many of
them requires caller (KVM) to understand psp specific data structures. This
often ends up with the fact that KVM has to create its own 'wrapper' API to
make it easy to use. The following is the pattern:

kvm_func(unsigned int handle)
{
	psp_data_structure data;

	data.handle = handle;
	psp_func(&data, NULL);
}

psp_func(psp_data_structure *data, int *error)
{
	sev_do_cmd(data, error);
}

struct psp_data_structure {
	u32 handle;
};

sev_decommission is one example following the above pattern. Since KVM is
the only user for this API and 'handle' is the only data that is meaningful
to KVM, simplify the interface by putting the code from kvm function
sev_decommission into the psp function sev_guest_decomssion.

Cc: Alper Gun <alpergun@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: David Rienjes <rientjes@google.com>
Cc: Marc Orr <marcorr@google.com>
Cc: John Allen <john.allen@amd.com>
Cc: Peter Gonda <pgonda@google.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Vipin Sharma <vipinsh@google.com>

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Mingwei Zhang <mizhang@google.com>
---
 arch/x86/kvm/svm/sev.c       | 15 ++-------------
 drivers/crypto/ccp/sev-dev.c |  8 +++++++-
 include/linux/psp-sev.h      |  7 ++++---
 3 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 75e0b21ad07c..6a1faf28d973 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -198,17 +198,6 @@ static void sev_asid_free(struct kvm_sev_info *sev)
 	sev->misc_cg = NULL;
 }
 
-static void sev_decommission(unsigned int handle)
-{
-	struct sev_data_decommission decommission;
-
-	if (!handle)
-		return;
-
-	decommission.handle = handle;
-	sev_guest_decommission(&decommission, NULL);
-}
-
 static void sev_unbind_asid(struct kvm *kvm, unsigned int handle)
 {
 	struct sev_data_deactivate deactivate;
@@ -223,7 +212,7 @@ static void sev_unbind_asid(struct kvm *kvm, unsigned int handle)
 	sev_guest_deactivate(&deactivate, NULL);
 	up_read(&sev_deactivate_lock);
 
-	sev_decommission(handle);
+	sev_guest_decommission(handle, NULL);
 }
 
 static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
@@ -349,7 +338,7 @@ static int sev_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	/* Bind ASID to this guest */
 	ret = sev_bind_asid(kvm, start.handle, error);
 	if (ret) {
-		sev_decommission(start.handle);
+		sev_guest_decommission(start.handle, NULL);
 		goto e_free_session;
 	}
 
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 91808402e0bf..ab9c2c49d612 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -903,8 +903,14 @@ int sev_guest_activate(struct sev_data_activate *data, int *error)
 }
 EXPORT_SYMBOL_GPL(sev_guest_activate);
 
-int sev_guest_decommission(struct sev_data_decommission *data, int *error)
+int sev_guest_decommission(unsigned int handle, int *error)
 {
+	struct sev_data_decommission decommission;
+
+	if (!handle)
+		return -EINVAL;
+
+	decommission.handle = handle;
 	return sev_do_cmd(SEV_CMD_DECOMMISSION, data, error);
 }
 EXPORT_SYMBOL_GPL(sev_guest_decommission);
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index d48a7192e881..6c0f2f451c89 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -612,17 +612,18 @@ int sev_guest_df_flush(int *error);
 /**
  * sev_guest_decommission - perform SEV DECOMMISSION command
  *
- * @decommission: sev_data_decommission structure to be processed
+ * @handle: sev_data_decommission structure to be processed
  * @sev_ret: sev command return code
  *
  * Returns:
  * 0 if the sev successfully processed the command
+ * -%EINVAL    if handle is NULL
  * -%ENODEV    if the sev device is not available
  * -%ENOTSUPP  if the sev does not support SEV
  * -%ETIMEDOUT if the sev command timed out
  * -%EIO       if the sev returned a non-zero return code
  */
-int sev_guest_decommission(struct sev_data_decommission *data, int *error);
+int sev_guest_decommission(unsigned int handle, int *error);
 
 void *psp_copy_user_blob(u64 uaddr, u32 len);
 
@@ -637,7 +638,7 @@ static inline int
 sev_guest_deactivate(struct sev_data_deactivate *data, int *error) { return -ENODEV; }
 
 static inline int
-sev_guest_decommission(struct sev_data_decommission *data, int *error) { return -ENODEV; }
+sev_guest_decommission(unsigned int handle, int *error) { return -ENODEV; }
 
 static inline int
 sev_guest_activate(struct sev_data_activate *data, int *error) { return -ENODEV; }
-- 
2.33.0.rc1.237.g0d66db33f3-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] KVM: SVM: move sev_bind_asid to psp
  2021-08-16 20:24 [PATCH 0/3] clean up interface between KVM and psp Mingwei Zhang
  2021-08-16 20:24 ` [PATCH 1/3] KVM: SVM: move sev_decommission to psp driver Mingwei Zhang
@ 2021-08-16 20:24 ` Mingwei Zhang
  2021-08-16 20:24 ` [PATCH 3/3] KVM: SVM: move sev_unbind_asid and DF_FLUSH logic into psp Mingwei Zhang
  2021-08-17  8:54 ` [PATCH 0/3] clean up interface between KVM and psp Paolo Bonzini
  3 siblings, 0 replies; 8+ messages in thread
From: Mingwei Zhang @ 2021-08-16 20:24 UTC (permalink / raw)
  To: Paolo Bonzini, Brijesh Singh, Tom Lendacky, John Allen
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-crypto, linux-kernel, Alper Gun,
	Borislav Petkov, David Rienjes, Marc Orr, Peter Gonda,
	Vipin Sharma, Mingwei Zhang

ccp/sev-dev.c is the software layer in psp that allows KVM to manage
SEV/ES/SNP enabled VMs. Since psp API provides only primitive sev command
invocation, KVM has to do extra processing that are specific only to psp
with KVM level wrapper function.

sev_bind_asid is such a KVM function that literally wraps around
sev_guest_activate in psp with extra steps like psp data structure creation
and error processing: invoking sev_guest_decommission on activation
failure.

Adding sev_guest_decommission is essentially required on all sev_bin_asid
call sites. This is error prone and in fact the upstream code in KVM still
have an issue on sev_receive_start where sev_guest_decommission is missing.

Since sev_bind_asid code logic is purely psp specific, putting it into psp
layer should make it more robust, since KVM code does not have to worry
about error handling of asid binding failure.

So replace the KVM pointer in sev_bind_asid with primitive arguments: asid
and handle; slightly change the name to sev_guest_bind_asid make it
consistent with other psp APIs; add the error handling code inside
sev_guest_bind_asid and; put it into the sev-dev.c.

Cc: Alper Gun <alpergun@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: David Rienjes <rientjes@google.com>
Cc: Marc Orr <marcorr@google.com>
Cc: John Allen <john.allen@amd.com>
Cc: Peter Gonda <pgonda@google.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Vipin Sharma <vipinsh@google.com>

Fixes: af43cbbf954b ("KVM: SVM: Add support for KVM_SEV_RECEIVE_START command")
Signed-off-by: Mingwei Zhang <mizhang@google.com>
---
 arch/x86/kvm/svm/sev.c       | 23 ++++-------------------
 drivers/crypto/ccp/sev-dev.c | 15 +++++++++++++++
 include/linux/psp-sev.h      | 19 +++++++++++++++++++
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 6a1faf28d973..2a674acb22ce 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -252,20 +252,6 @@ static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
-static int sev_bind_asid(struct kvm *kvm, unsigned int handle, int *error)
-{
-	struct sev_data_activate activate;
-	int asid = sev_get_asid(kvm);
-	int ret;
-
-	/* activate ASID on the given handle */
-	activate.handle = handle;
-	activate.asid   = asid;
-	ret = sev_guest_activate(&activate, error);
-
-	return ret;
-}
-
 static int __sev_issue_cmd(int fd, int id, void *data, int *error)
 {
 	struct fd f;
@@ -336,11 +322,9 @@ static int sev_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
 		goto e_free_session;
 
 	/* Bind ASID to this guest */
-	ret = sev_bind_asid(kvm, start.handle, error);
-	if (ret) {
-		sev_guest_decommission(start.handle, NULL);
+	ret = sev_guest_bind_asid(sev_get_asid(kvm), start.handle, error);
+	if (ret)
 		goto e_free_session;
-	}
 
 	/* return handle to userspace */
 	params.handle = start.handle;
@@ -1385,7 +1369,8 @@ static int sev_receive_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
 		goto e_free_session;
 
 	/* Bind ASID to this guest */
-	ret = sev_bind_asid(kvm, start.handle, error);
+	ret = sev_guest_bind_asid(sev_get_asid(kvm), start.handle, error);
+
 	if (ret)
 		goto e_free_session;
 
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index ab9c2c49d612..ef58f007030e 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -903,6 +903,21 @@ int sev_guest_activate(struct sev_data_activate *data, int *error)
 }
 EXPORT_SYMBOL_GPL(sev_guest_activate);
 
+int sev_guest_bind_asid(int asid, unsigned int handle, int *error)
+{
+	struct sev_data_activate activate;
+	int ret;
+
+	/* activate ASID on the given handle */
+	activate.handle = handle;
+	activate.asid   = asid;
+	ret = sev_guest_activate(&activate, error);
+	if (ret)
+		sev_guest_decommission(handle, NULL);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(sev_guest_bind_asid);
+
 int sev_guest_decommission(unsigned int handle, int *error)
 {
 	struct sev_data_decommission decommission;
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index 6c0f2f451c89..be50446ff3f1 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -595,6 +595,22 @@ int sev_guest_deactivate(struct sev_data_deactivate *data, int *error);
  */
 int sev_guest_activate(struct sev_data_activate *data, int *error);
 
+/**
+ * sev_guest_bind_asid - bind an ASID with VM and does decommission on failure
+ *
+ * @asid: current ASID of the VM
+ * @handle: handle of the VM to retrieve status
+ * @sev_ret: sev command return code
+ *
+ * Returns:
+ * 0 if the sev successfully processed the command
+ * -%ENODEV    if the sev device is not available
+ * -%ENOTSUPP  if the sev does not support SEV
+ * -%ETIMEDOUT if the sev command timed out
+ * -%EIO       if the sev returned a non-zero return code
+ */
+int sev_guest_bind_asid(int asid, unsigned int handle, int *error);
+
 /**
  * sev_guest_df_flush - perform SEV DF_FLUSH command
  *
@@ -643,6 +659,9 @@ sev_guest_decommission(unsigned int handle, int *error) { return -ENODEV; }
 static inline int
 sev_guest_activate(struct sev_data_activate *data, int *error) { return -ENODEV; }
 
+static inline int
+sev_guest_bind_asid(int asid, unsigned int handle, int *error) { return -ENODEV; }
+
 static inline int sev_guest_df_flush(int *error) { return -ENODEV; }
 
 static inline int
-- 
2.33.0.rc1.237.g0d66db33f3-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/3] KVM: SVM: move sev_unbind_asid and DF_FLUSH logic into psp
  2021-08-16 20:24 [PATCH 0/3] clean up interface between KVM and psp Mingwei Zhang
  2021-08-16 20:24 ` [PATCH 1/3] KVM: SVM: move sev_decommission to psp driver Mingwei Zhang
  2021-08-16 20:24 ` [PATCH 2/3] KVM: SVM: move sev_bind_asid to psp Mingwei Zhang
@ 2021-08-16 20:24 ` Mingwei Zhang
  2021-08-17  8:54 ` [PATCH 0/3] clean up interface between KVM and psp Paolo Bonzini
  3 siblings, 0 replies; 8+ messages in thread
From: Mingwei Zhang @ 2021-08-16 20:24 UTC (permalink / raw)
  To: Paolo Bonzini, Brijesh Singh, Tom Lendacky, John Allen
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-crypto, linux-kernel, Alper Gun,
	Borislav Petkov, David Rienjes, Marc Orr, Peter Gonda,
	Vipin Sharma, Mingwei Zhang

In KVM SEV code, sev_unbind_asid and sev_guest_df_flush needs to be
serialized because DEACTIVATE command in PSP may clear the WBINVD indicator
and cause DF_FLUSH to fail.

This is a PSP level detail that is not necessary to expose to KVM. So put
both functions as well as the RWSEM into the sev-dev.c.

Cc: Alper Gun <alpergun@google.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: David Rienjes <rientjes@google.com>
Cc: Marc Orr <marcorr@google.com>
Cc: John Allen <john.allen@amd.com>
Cc: Peter Gonda <pgonda@google.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Vipin Sharma <vipinsh@google.com>

Signed-off-by: Mingwei Zhang <mizhang@google.com>
---
 arch/x86/kvm/svm/sev.c       | 35 +++--------------------------------
 drivers/crypto/ccp/sev-dev.c | 34 +++++++++++++++++++++++++++++++++-
 include/linux/psp-sev.h      | 19 ++++++++++++++++++-
 3 files changed, 54 insertions(+), 34 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 2a674acb22ce..ecf9da718d21 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -57,7 +57,6 @@ module_param_named(sev_es, sev_es_enabled, bool, 0444);
 #endif /* CONFIG_KVM_AMD_SEV */
 
 static u8 sev_enc_bit;
-static DECLARE_RWSEM(sev_deactivate_lock);
 static DEFINE_MUTEX(sev_bitmap_lock);
 unsigned int max_sev_asid;
 static unsigned int min_sev_asid;
@@ -84,20 +83,9 @@ static int sev_flush_asids(int min_asid, int max_asid)
 	if (asid > max_asid)
 		return -EBUSY;
 
-	/*
-	 * DEACTIVATE will clear the WBINVD indicator causing DF_FLUSH to fail,
-	 * so it must be guarded.
-	 */
-	down_write(&sev_deactivate_lock);
-
-	wbinvd_on_all_cpus();
 	ret = sev_guest_df_flush(&error);
-
-	up_write(&sev_deactivate_lock);
-
 	if (ret)
 		pr_err("SEV: DF_FLUSH failed, ret=%d, error=%#x\n", ret, error);
-
 	return ret;
 }
 
@@ -198,23 +186,6 @@ static void sev_asid_free(struct kvm_sev_info *sev)
 	sev->misc_cg = NULL;
 }
 
-static void sev_unbind_asid(struct kvm *kvm, unsigned int handle)
-{
-	struct sev_data_deactivate deactivate;
-
-	if (!handle)
-		return;
-
-	deactivate.handle = handle;
-
-	/* Guard DEACTIVATE against WBINVD/DF_FLUSH used in ASID recycling */
-	down_read(&sev_deactivate_lock);
-	sev_guest_deactivate(&deactivate, NULL);
-	up_read(&sev_deactivate_lock);
-
-	sev_guest_decommission(handle, NULL);
-}
-
 static int sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp)
 {
 	struct kvm_sev_info *sev = &to_kvm_svm(kvm)->sev_info;
@@ -329,7 +300,7 @@ static int sev_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	/* return handle to userspace */
 	params.handle = start.handle;
 	if (copy_to_user((void __user *)(uintptr_t)argp->data, &params, sizeof(params))) {
-		sev_unbind_asid(kvm, start.handle);
+		sev_guest_unbind_asid(start.handle);
 		ret = -EFAULT;
 		goto e_free_session;
 	}
@@ -1378,7 +1349,7 @@ static int sev_receive_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	if (copy_to_user((void __user *)(uintptr_t)argp->data,
 			 &params, sizeof(struct kvm_sev_receive_start))) {
 		ret = -EFAULT;
-		sev_unbind_asid(kvm, start.handle);
+		sev_guest_unbind_asid(start.handle);
 		goto e_free_session;
 	}
 
@@ -1789,7 +1760,7 @@ void sev_vm_destroy(struct kvm *kvm)
 
 	mutex_unlock(&kvm->lock);
 
-	sev_unbind_asid(kvm, sev->handle);
+	sev_guest_unbind_asid(sev->handle);
 	sev_asid_free(sev);
 }
 
diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index ef58f007030e..7d53cd954f80 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -33,6 +33,7 @@
 #define SEV_FW_NAME_SIZE	64
 
 static DEFINE_MUTEX(sev_cmd_mutex);
+static DECLARE_RWSEM(sev_deactivate_lock);
 static struct sev_misc_dev *misc_dev;
 
 static int psp_cmd_timeout = 100;
@@ -932,10 +933,41 @@ EXPORT_SYMBOL_GPL(sev_guest_decommission);
 
 int sev_guest_df_flush(int *error)
 {
-	return sev_do_cmd(SEV_CMD_DF_FLUSH, NULL, error);
+	int ret;
+	/*
+	 * DEACTIVATE will clear the WBINVD indicator causing DF_FLUSH to fail,
+	 * so it must be guarded.
+	 */
+	down_write(&sev_deactivate_lock);
+
+	wbinvd_on_all_cpus();
+
+	ret = sev_do_cmd(SEV_CMD_DF_FLUSH, NULL, error);
+
+	up_write(&sev_deactivate_lock);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(sev_guest_df_flush);
 
+void sev_guest_unbind_asid(unsigned int handle)
+{
+	struct sev_data_deactivate deactivate;
+
+	if (!handle)
+		return;
+
+	deactivate.handle = handle;
+
+	/* Guard DEACTIVATE against WBINVD/DF_FLUSH used in ASID recycling */
+	down_read(&sev_deactivate_lock);
+	sev_guest_deactivate(&deactivate, NULL);
+	up_read(&sev_deactivate_lock);
+
+	sev_guest_decommission(handle, NULL);
+}
+EXPORT_SYMBOL_GPL(sev_guest_unbind_asid);
+
 static void sev_exit(struct kref *ref)
 {
 	misc_deregister(&misc_dev->misc);
diff --git a/include/linux/psp-sev.h b/include/linux/psp-sev.h
index be50446ff3f1..09447bce9665 100644
--- a/include/linux/psp-sev.h
+++ b/include/linux/psp-sev.h
@@ -580,6 +580,20 @@ int sev_issue_cmd_external_user(struct file *filep, unsigned int id,
  */
 int sev_guest_deactivate(struct sev_data_deactivate *data, int *error);
 
+/**
+ * sev_guest_unbind_asid - perform SEV DEACTIVATE command with lock held
+ *
+ * @handle: handle of the VM to deactivate
+ *
+ * Returns:
+ * 0 if the sev successfully processed the command
+ * -%ENODEV    if the sev device is not available
+ * -%ENOTSUPP  if the sev does not support SEV
+ * -%ETIMEDOUT if the sev command timed out
+ * -%EIO       if the sev returned a non-zero return code
+ */
+int sev_guest_unbind_asid(unsigned int handle);
+
 /**
  * sev_guest_activate - perform SEV ACTIVATE command
  *
@@ -612,7 +626,7 @@ int sev_guest_activate(struct sev_data_activate *data, int *error);
 int sev_guest_bind_asid(int asid, unsigned int handle, int *error);
 
 /**
- * sev_guest_df_flush - perform SEV DF_FLUSH command
+ * sev_guest_df_flush - perform SEV DF_FLUSH command with lock held
  *
  * @sev_ret: sev command return code
  *
@@ -656,6 +670,9 @@ sev_guest_deactivate(struct sev_data_deactivate *data, int *error) { return -ENO
 static inline int
 sev_guest_decommission(unsigned int handle, int *error) { return -ENODEV; }
 
+static inline int
+sev_guest_unbind_asid(unsigned int handle) { return -ENODEV; }
+
 static inline int
 sev_guest_activate(struct sev_data_activate *data, int *error) { return -ENODEV; }
 
-- 
2.33.0.rc1.237.g0d66db33f3-goog


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] clean up interface between KVM and psp
  2021-08-16 20:24 [PATCH 0/3] clean up interface between KVM and psp Mingwei Zhang
                   ` (2 preceding siblings ...)
  2021-08-16 20:24 ` [PATCH 3/3] KVM: SVM: move sev_unbind_asid and DF_FLUSH logic into psp Mingwei Zhang
@ 2021-08-17  8:54 ` Paolo Bonzini
  2021-08-17 18:08   ` Mingwei Zhang
  3 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2021-08-17  8:54 UTC (permalink / raw)
  To: Mingwei Zhang, Brijesh Singh, Tom Lendacky, John Allen
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-crypto, linux-kernel, Alper Gun,
	Borislav Petkov, David Rienjes, Marc Orr, Peter Gonda,
	Vipin Sharma

On 16/08/21 22:24, Mingwei Zhang wrote:
> This patch set is trying to help make the interface between KVM and psp
> cleaner and simpler. In particular, the patches do the following
> improvements:
>   - avoid the requirement of psp data structures for some psp APIs.
>   - hide error handling within psp API, eg., using sev_decommission.
>   - hide the serialization requirement between DF_FLUSH and DEACTIVATE.
> 
> Mingwei Zhang (3):
>    KVM: SVM: move sev_decommission to psp driver
>    KVM: SVM: move sev_bind_asid to psp
>    KVM: SVM: move sev_unbind_asid and DF_FLUSH logic into psp

No objections apart from the build failure on patch 1.  However, it's up 
to Tom whether they prefer this logic in KVM or the PSP driver.

Paolo


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] clean up interface between KVM and psp
  2021-08-17  8:54 ` [PATCH 0/3] clean up interface between KVM and psp Paolo Bonzini
@ 2021-08-17 18:08   ` Mingwei Zhang
  2021-08-17 19:54     ` Brijesh Singh
  0 siblings, 1 reply; 8+ messages in thread
From: Mingwei Zhang @ 2021-08-17 18:08 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Brijesh Singh, Tom Lendacky, John Allen, Sean Christopherson,
	Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-crypto, LKML, Alper Gun, Borislav Petkov, David Rienjes,
	Marc Orr, Peter Gonda, Vipin Sharma

Hi Paolo,

Thanks for the prompt reply. I will update the code and will be
waiting for Tom and other AMD folks' feedback.

Thanks. Regards
-Mingwei

On Tue, Aug 17, 2021 at 1:54 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 16/08/21 22:24, Mingwei Zhang wrote:
> > This patch set is trying to help make the interface between KVM and psp
> > cleaner and simpler. In particular, the patches do the following
> > improvements:
> >   - avoid the requirement of psp data structures for some psp APIs.
> >   - hide error handling within psp API, eg., using sev_decommission.
> >   - hide the serialization requirement between DF_FLUSH and DEACTIVATE.
> >
> > Mingwei Zhang (3):
> >    KVM: SVM: move sev_decommission to psp driver
> >    KVM: SVM: move sev_bind_asid to psp
> >    KVM: SVM: move sev_unbind_asid and DF_FLUSH logic into psp
>
> No objections apart from the build failure on patch 1.  However, it's up
> to Tom whether they prefer this logic in KVM or the PSP driver.
>
> Paolo
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] clean up interface between KVM and psp
  2021-08-17 18:08   ` Mingwei Zhang
@ 2021-08-17 19:54     ` Brijesh Singh
  2021-08-18  5:34       ` Mingwei Zhang
  0 siblings, 1 reply; 8+ messages in thread
From: Brijesh Singh @ 2021-08-17 19:54 UTC (permalink / raw)
  To: Mingwei Zhang, Paolo Bonzini
  Cc: brijesh.singh, Tom Lendacky, John Allen, Sean Christopherson,
	Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-crypto, LKML, Alper Gun, Borislav Petkov, David Rienjes,
	Marc Orr, Peter Gonda, Vipin Sharma



On 8/17/21 1:08 PM, Mingwei Zhang wrote:
> Hi Paolo,
> 
> Thanks for the prompt reply. I will update the code and will be
> waiting for Tom and other AMD folks' feedback.
> 
> Thanks. Regards
> -Mingwei
> 
> On Tue, Aug 17, 2021 at 1:54 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 16/08/21 22:24, Mingwei Zhang wrote:
>>> This patch set is trying to help make the interface between KVM and psp
>>> cleaner and simpler. In particular, the patches do the following
>>> improvements:
>>>    - avoid the requirement of psp data structures for some psp APIs.
>>>    - hide error handling within psp API, eg., using sev_decommission.
>>>    - hide the serialization requirement between DF_FLUSH and DEACTIVATE.
>>>
>>> Mingwei Zhang (3):
>>>     KVM: SVM: move sev_decommission to psp driver
>>>     KVM: SVM: move sev_bind_asid to psp
>>>     KVM: SVM: move sev_unbind_asid and DF_FLUSH logic into psp
>>
>> No objections apart from the build failure on patch 1.  However, it's up
>> to Tom whether they prefer this logic in KVM or the PSP driver.
>>

I have no objection to move those functions in SEV drv.

With build fix

Acked-by: Brijesh Singh <brijesh.singh@amd.com>


Just for the context, SEV API commands are divided in two sets:

1. commands to provision the host (such as PDH_GEN, CSR, CERT_EXPORT, 
CERT_IMPORT ...)
2. commands to manage the guest (such as LAUNCH_START, LAUNCH_UPDATE ...)

I was trying to keep all the guest management commands functions within 
KVM because no other driver needs it. Having said that, we made 
exception for the decommission and activate so we can cleanup the 
firmware resource in non-process context.

thanks

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] clean up interface between KVM and psp
  2021-08-17 19:54     ` Brijesh Singh
@ 2021-08-18  5:34       ` Mingwei Zhang
  0 siblings, 0 replies; 8+ messages in thread
From: Mingwei Zhang @ 2021-08-18  5:34 UTC (permalink / raw)
  To: Brijesh Singh
  Cc: Paolo Bonzini, Tom Lendacky, John Allen, Sean Christopherson,
	Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-crypto, LKML, Alper Gun, Borislav Petkov, David Rienjes,
	Marc Orr, Peter Gonda, Vipin Sharma

>
> I have no objection to move those functions in SEV drv.
>
> With build fix
>
> Acked-by: Brijesh Singh <brijesh.singh@amd.com>
>
Thanks for the ack. Will fix all build issues in the next version.

> I was trying to keep all the guest management commands functions within
> KVM because no other driver needs it. Having said that, we made
> exception for the decommission and activate so we can cleanup the
> firmware resource in non-process context.
>
Yes, ACTIVATE  / DECOMMISSION is one case that illustrates the need to
care about their internal relationship. And there is another case,
which is the serialization requirement between DF_FLUSH and
DEACTIVATE. This requires KVM to maintain an extra RWSEM. So I feel
that it would be good to hide these details away from KVM even if KVM
is the only user.

Thanks.
-Mingwei

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-08-18  5:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-16 20:24 [PATCH 0/3] clean up interface between KVM and psp Mingwei Zhang
2021-08-16 20:24 ` [PATCH 1/3] KVM: SVM: move sev_decommission to psp driver Mingwei Zhang
2021-08-16 20:24 ` [PATCH 2/3] KVM: SVM: move sev_bind_asid to psp Mingwei Zhang
2021-08-16 20:24 ` [PATCH 3/3] KVM: SVM: move sev_unbind_asid and DF_FLUSH logic into psp Mingwei Zhang
2021-08-17  8:54 ` [PATCH 0/3] clean up interface between KVM and psp Paolo Bonzini
2021-08-17 18:08   ` Mingwei Zhang
2021-08-17 19:54     ` Brijesh Singh
2021-08-18  5:34       ` Mingwei Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).