It's not clear if SNP guests will need any other SNP-specific userspace
exit types in the future, and if they do, it's not clear that they would
necessary be related to VMGEXIT events or something else entirely.
So, rather than trying to anticipate future use-cases and have a single
union structure to manage the associated parameters, just use a common
KVM_EXIT_SNP_* prefix, but otherwise treat these as separate events, and
go ahead and convert the only VMGEXIT type currently defined,
KVM_USER_VMGEXIT_REQ_CERTS, over to KVM_EXIT_SNP_REQ_CERTS.
Also, formally document that kvm_run->immediate_exit is guaranteed to
handle userspace IO completion callbacks before returning to userspace
with -EINTR, and use this mechanism to allow userspace to use this
mechanism as a means to know when an attestation request is complete and
the KVM_EXIT_SNP_REQ_CERTS event is fully-finished, allowing userspace
to at that point handle any certificate synchronization cleanup like
releasing file locks/etc.
Suggested-by: Sean Christopherson <seanjc@xxxxxxxxxx>
Signed-off-by: Michael Roth <michael.roth@xxxxxxx>
---
Hi Sean,
Here's an attempt to address your concerns regarding
KVM_EXIT_VMGEXIT/KVM_USER_VMGEXIT_REQ_CERTS. Please let me know what
you think.
The main gist of it is that it leverages kvm->immediate_edit rather than
a flag in the kvm_run exit struct to receive notification when the
attestation has been completed and the certificate is no longer needed.
Additionally it drops the confusing KVM_EXIT_VMGEXIT naming in favor of
the KVM_EXIT_SNP_* prefix, and rather than introduce infrastructure and
a union to handle other SNP-specific types in the future, it simply
defines KVM_EXIT_SNP_REQ_CERTS as a one-off event so we are free to
handle future SNP-specific/SNP-related userspace exits however makes
sense for future cases.
Thanks,
Mike
Documentation/virt/kvm/api.rst | 64 +++++++++++-----------------------
arch/x86/kvm/svm/sev.c | 28 +++------------
include/uapi/linux/kvm.h | 31 ++++++++--------
3 files changed, 43 insertions(+), 80 deletions(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 6ab8b5b7c64e..ea05c16f3438 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6381,6 +6381,11 @@ to avoid usage of KVM_SET_SIGNAL_MASK, which has worse scalability.
Rather than blocking the signal outside KVM_RUN, userspace can set up
a signal handler that sets run->immediate_exit to a non-zero value.
+Also note that any KVM_EXIT_* events that have associated completion
+callbacks that KVM needs to process when KVM_RUN is called will be
+processed *before* exiting again to userspace with -EINTR as a result
+of run->immediate_exit.
+
This field is ignored if KVM_CAP_IMMEDIATE_EXIT is not available.
::
@@ -7069,50 +7074,24 @@ values in kvm_run even if the corresponding bit in kvm_dirty_regs is not set.
::
- /* KVM_EXIT_VMGEXIT */
- struct kvm_user_vmgexit {
- #define KVM_USER_VMGEXIT_REQ_CERTS 1
- __u32 type; /* KVM_USER_VMGEXIT_* type */
- union {
- struct {
- __u64 data_gpa;
- __u64 data_npages;
- #define KVM_USER_VMGEXIT_REQ_CERTS_ERROR_INVALID_LEN 1
- #define KVM_USER_VMGEXIT_REQ_CERTS_ERROR_BUSY 2
- #define KVM_USER_VMGEXIT_REQ_CERTS_ERROR_GENERIC (1 << 31)
- __u32 ret;
- #define KVM_USER_VMGEXIT_REQ_CERTS_FLAGS_NOTIFY_DONE BIT(0)
- __u8 flags;
- #define KVM_USER_VMGEXIT_REQ_CERTS_STATUS_PENDING 0
- #define KVM_USER_VMGEXIT_REQ_CERTS_STATUS_DONE 1
- __u8 status;
- } req_certs;
- };
- };
-
-
-If exit reason is KVM_EXIT_VMGEXIT then it indicates that an SEV-SNP guest
-has issued a VMGEXIT instruction (as documented by the AMD Architecture
-Programmer's Manual (APM)) to the hypervisor that needs to be serviced by
-userspace. These are generally handled by the host kernel, but in some
-cases some aspects of handling a VMGEXIT are done in userspace.
-
-A kvm_user_vmgexit structure is defined to encapsulate the data to be
-sent to or returned by userspace. The type field defines the specific type
-of exit that needs to be serviced, and that type is used as a discriminator
-to determine which union type should be used for input/output.
-
-KVM_USER_VMGEXIT_REQ_CERTS
---------------------------
+ /* KVM_EXIT_SNP_REQ_CERTS */
+ struct {
+ __u64 data_gpa;
+ __u64 data_npages;
+ #define KVM_EXIT_SNP_REQ_CERTS_ERROR_INVALID_LEN 1
+ #define KVM_EXIT_SNP_REQ_CERTS_ERROR_BUSY 2
+ #define KVM_EXIT_SNP_REQ_CERTS_ERROR_GENERIC (1 << 31)
+ __u32 ret;
+ } snp_req_certs;
When an SEV-SNP issues a guest request for an attestation report, it has the
option of issuing it in the form an *extended* guest request when a
certificate blob is returned alongside the attestation report so the guest
can validate the endorsement key used by SNP firmware to sign the report.
These certificates are managed by userspace and are requested via
-KVM_EXIT_VMGEXITs using the KVM_USER_VMGEXIT_REQ_CERTS type.
+KVM_EXIT_SNP exits using the KVM_EXIT_SNP_REQ_CERTS type.
-For the KVM_USER_VMGEXIT_REQ_CERTS type, the req_certs union type
+For the KVM_EXIT_SNP_REQ_CERTS type, the req_certs union type
is used. The kernel will supply in 'data_gpa' the value the guest supplies
via the RAX field of the GHCB when issuing extended guest requests.
'data_npages' will similarly contain the value the guest supplies in RBX
@@ -7139,12 +7118,11 @@ this is for the VMM to obtain a shared or exclusive lock on the path the
certificate blob file resides at before reading it and returning it to KVM,
and that it continues to hold the lock until the attestation request is
actually sent to firmware. To facilitate this, the VMM can set the
-KVM_USER_VMGEXIT_REQ_CERTS_FLAGS_NOTIFY_DONE flag before returning the
-certificate blob, in which case another KVM_EXIT_VMGEXIT of type
-KVM_USER_VMGEXIT_REQ_CERTS will be sent to userspace with
-KVM_USER_VMGEXIT_REQ_CERTS_STATUS_DONE being set in the status field to
-indicate the request is fully-completed and that any associated locks can be
-released.
+run->immediate_exit flag before returning the certificate blob, in which
+case KVM is guaranteed to complete the issuing of all pending IO completion
+callbacks before exiting to userspace with EINTR. At this point userspace
+can release any locks it may have taken when the KVM_EXIT_SNP_REQ_CERTS exit
+was originally received.
Tools/libraries that perform updates to SNP firmware TCB values or endorsement
keys (e.g. firmware interfaces such as SNP_COMMIT, SNP_SET_CONFIG, or
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 6cf665c410b2..e6318bbd8a6a 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4006,21 +4006,14 @@ static int snp_complete_ext_guest_req(struct kvm_vcpu *vcpu)
sev_ret_code fw_err = 0;
int vmm_ret;
- vmm_ret = vcpu->run->vmgexit.req_certs.ret;
+ vmm_ret = vcpu->run->snp_req_certs.ret;
if (vmm_ret) {
if (vmm_ret == SNP_GUEST_VMM_ERR_INVALID_LEN)
vcpu->arch.regs[VCPU_REGS_RBX] =
- vcpu->run->vmgexit.req_certs.data_npages;
+ vcpu->run->snp_req_certs.data_npages;
goto out;
}
- /*
- * The request was completed on the previous completion callback and
- * this completion is only for the STATUS_DONE userspace notification.
- */
- if (vcpu->run->vmgexit.req_certs.status == KVM_USER_VMGEXIT_REQ_CERTS_STATUS_DONE)
- goto out_resume;
-
control = &svm->vmcb->control;
if (__snp_handle_guest_req(kvm, control->exit_info_1,
@@ -4029,14 +4022,6 @@ static int snp_complete_ext_guest_req(struct kvm_vcpu *vcpu)
out:
ghcb_set_sw_exit_info_2(svm->sev_es.ghcb, SNP_GUEST_ERR(vmm_ret, fw_err));
-
- if (vcpu->run->vmgexit.req_certs.flags & KVM_USER_VMGEXIT_REQ_CERTS_FLAGS_NOTIFY_DONE) {
- vcpu->run->vmgexit.req_certs.status = KVM_USER_VMGEXIT_REQ_CERTS_STATUS_DONE;
- vcpu->run->vmgexit.req_certs.flags = 0;
- return 0; /* notify userspace of completion */
- }
-
-out_resume:
return 1; /* resume guest */
}
@@ -4060,12 +4045,9 @@ static int snp_begin_ext_guest_req(struct kvm_vcpu *vcpu)
* Grab the certificates from userspace so that can be bundled with
* attestation/guest requests.
*/
- vcpu->run->exit_reason = KVM_EXIT_VMGEXIT;
- vcpu->run->vmgexit.type = KVM_USER_VMGEXIT_REQ_CERTS;
- vcpu->run->vmgexit.req_certs.data_gpa = data_gpa;
- vcpu->run->vmgexit.req_certs.data_npages = data_npages;
- vcpu->run->vmgexit.req_certs.flags = 0;
- vcpu->run->vmgexit.req_certs.status = KVM_USER_VMGEXIT_REQ_CERTS_STATUS_PENDING;
+ vcpu->run->exit_reason = KVM_EXIT_SNP_REQ_CERTS;
+ vcpu->run->snp_req_certs.data_gpa = data_gpa;
+ vcpu->run->snp_req_certs.data_npages = data_npages;
vcpu->arch.complete_userspace_io = snp_complete_ext_guest_req;
return 0; /* forward request to userspace */
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 106367d87189..8ebfc91dc967 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -135,22 +135,17 @@ struct kvm_xen_exit {
} u;
};
-struct kvm_user_vmgexit {
-#define KVM_USER_VMGEXIT_REQ_CERTS 1
- __u32 type; /* KVM_USER_VMGEXIT_* type */
+struct kvm_exit_snp {
+#define KVM_EXIT_SNP_REQ_CERTS 1
+ __u32 type; /* KVM_EXIT_SNP_* type */
union {
struct {
__u64 data_gpa;
__u64 data_npages;
-#define KVM_USER_VMGEXIT_REQ_CERTS_ERROR_INVALID_LEN 1
-#define KVM_USER_VMGEXIT_REQ_CERTS_ERROR_BUSY 2
-#define KVM_USER_VMGEXIT_REQ_CERTS_ERROR_GENERIC (1 << 31)
+#define KVM_EXIT_SNP_REQ_CERTS_ERROR_INVALID_LEN 1
+#define KVM_EXIT_SNP_REQ_CERTS_ERROR_BUSY 2
+#define KVM_EXIT_SNP_REQ_CERTS_ERROR_GENERIC (1 << 31)
__u32 ret;
-#define KVM_USER_VMGEXIT_REQ_CERTS_FLAGS_NOTIFY_DONE BIT(0)
- __u8 flags;
-#define KVM_USER_VMGEXIT_REQ_CERTS_STATUS_PENDING 0
-#define KVM_USER_VMGEXIT_REQ_CERTS_STATUS_DONE 1
- __u8 status;
} req_certs;
};
};
@@ -198,7 +193,7 @@ struct kvm_user_vmgexit {
#define KVM_EXIT_NOTIFY 37
#define KVM_EXIT_LOONGARCH_IOCSR 38
#define KVM_EXIT_MEMORY_FAULT 39
-#define KVM_EXIT_VMGEXIT 40
+#define KVM_EXIT_SNP_REQUEST_CERTS 40
/* For KVM_EXIT_INTERNAL_ERROR */
/* Emulate instruction failed. */
@@ -454,8 +449,16 @@ struct kvm_run {
__u64 gpa;
__u64 size;
} memory_fault;
- /* KVM_EXIT_VMGEXIT */
- struct kvm_user_vmgexit vmgexit;
+ /* KVM_EXIT_SNP_REQ_CERTS */
+ struct {
+ __u64 data_gpa;
+ __u64 data_npages;
+#define KVM_EXIT_SNP_REQ_CERTS_ERROR_INVALID_LEN 1
+#define KVM_EXIT_SNP_REQ_CERTS_ERROR_BUSY 2
+#define KVM_EXIT_SNP_REQ_CERTS_ERROR_GENERIC (1 << 31)
+ __u32 ret;
+ } snp_req_certs;
+
/* Fix the size of the union. */
char padding[256];
};
--
2.25.1
