On 3/20/24 09:39, Michael Roth wrote:
From: Brijesh Singh <brijesh.singh@xxxxxxx>
The SNP_LAUNCH_START is called first to create a cryptographic launch
context within the firmware.
Signed-off-by: Brijesh Singh <brijesh.singh@xxxxxxx>
Signed-off-by: Michael Roth <michael.roth@xxxxxxx>
---
target/i386/sev.c | 42 +++++++++++++++++++++++++++++++++++++++-
target/i386/trace-events | 1 +
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 3b4dbc63b1..9f63a41f08 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -39,6 +39,7 @@
#include "confidential-guest.h"
#include "hw/i386/pc.h"
#include "exec/address-spaces.h"
+#include "qemu/queue.h"
OBJECT_DECLARE_SIMPLE_TYPE(SevCommonState, SEV_COMMON)
OBJECT_DECLARE_SIMPLE_TYPE(SevGuestState, SEV_GUEST)
@@ -106,6 +107,16 @@ struct SevSnpGuestState {
#define DEFAULT_SEV_DEVICE "/dev/sev"
#define DEFAULT_SEV_SNP_POLICY 0x30000
+typedef struct SevLaunchUpdateData {
+ QTAILQ_ENTRY(SevLaunchUpdateData) next;
+ hwaddr gpa;
+ void *hva;
+ uint64_t len;
+ int type;
+} SevLaunchUpdateData;
+
+static QTAILQ_HEAD(, SevLaunchUpdateData) launch_update;
+
#define SEV_INFO_BLOCK_GUID "00f771de-1a7e-4fcb-890e-68c77e2fb44e"
typedef struct __attribute__((__packed__)) SevInfoBlock {
/* SEV-ES Reset Vector Address */
@@ -668,6 +679,30 @@ sev_read_file_base64(const char *filename, guchar **data, gsize *len)
return 0;
}
+static int
+sev_snp_launch_start(SevSnpGuestState *sev_snp_guest)
+{
+ int fw_error, rc;
+ SevCommonState *sev_common = SEV_COMMON(sev_snp_guest);
+ struct kvm_sev_snp_launch_start *start = &sev_snp_guest->kvm_start_conf;
+
+ trace_kvm_sev_snp_launch_start(start->policy, sev_snp_guest->guest_visible_workarounds);
+
+ rc = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_START,
+ start, &fw_error);
+ if (rc < 0) {
+ error_report("%s: SNP_LAUNCH_START ret=%d fw_error=%d '%s'",
+ __func__, rc, fw_error, fw_error_to_str(fw_error));
+ return 1;
+ }
+
+ QTAILQ_INIT(&launch_update);
+
+ sev_set_guest_state(sev_common, SEV_STATE_LAUNCH_UPDATE);
+
+ return 0;
+}
+
static int
sev_launch_start(SevGuestState *sev_guest)
{
@@ -1007,7 +1042,12 @@ static int sev_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
goto err;
}
- ret = sev_launch_start(SEV_GUEST(sev_common));
+ if (sev_snp_enabled()) {
+ ret = sev_snp_launch_start(SEV_SNP_GUEST(sev_common));
+ } else {
+ ret = sev_launch_start(SEV_GUEST(sev_common));
+ }
Instead of an "if", this should be a method in sev-common. Likewise for
launch_finish in the next patch.
Also, patch 47 should introduce an "int (*launch_update_data)(hwaddr
gpa, uint8_t *ptr, uint64_t len)" method whose implementation is either
the existing sev_launch_update_data() for sev-guest, or a wrapper around
snp_launch_update_data() (to add KVM_SEV_SNP_PAGE_TYPE_NORMAL) for
sev-snp-guest.
In general, the only uses of sev_snp_enabled() should be in
sev_add_kernel_loader_hashes() and kvm_handle_vmgexit_ext_req(). I
would not be that strict for the QMP and HMP functions, but if you want
to make those methods of sev-common I wouldn't complain.
Paolo
if (ret) {
error_setg(errp, "%s: failed to create encryption context", __func__);
goto err;
diff --git a/target/i386/trace-events b/target/i386/trace-events
index 2cd8726eeb..cb26d8a925 100644
--- a/target/i386/trace-events
+++ b/target/i386/trace-events
@@ -11,3 +11,4 @@ kvm_sev_launch_measurement(const char *value) "data %s"
kvm_sev_launch_finish(void) ""
kvm_sev_launch_secret(uint64_t hpa, uint64_t hva, uint64_t secret, int len) "hpa 0x%" PRIx64 " hva 0x%" PRIx64 " data 0x%" PRIx64 " len %d"
kvm_sev_attestation_report(const char *mnonce, const char *data) "mnonce %s data %s"
+kvm_sev_snp_launch_start(uint64_t policy, char *gosvw) "policy 0x%" PRIx64 " gosvw %s"
