[PATCH 0/3] clean up interface between KVM and psp

[PATCH 0/3] clean up interface between KVM and psp

[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

[PATCH 1/3] KVM: SVM: move sev_decommission to psp driver

[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

[PATCH 2/3] KVM: SVM: move sev_bind_asid to psp

[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

[PATCH 3/3] KVM: SVM: move sev_unbind_asid and DF_FLUSH logic into psp

[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

Re: [PATCH 0/3] clean up interface between KVM and psp

[Paolo Bonzini]

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

Re: [PATCH 0/3] clean up interface between KVM and psp

[Mingwei Zhang]

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
>

Re: [PATCH 0/3] clean up interface between KVM and psp

[Brijesh Singh]

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

Re: [PATCH 0/3] clean up interface between KVM and psp

[Mingwei Zhang]

>
> 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