* [PATCH v3] SEV: QMP support for Inject-Launch-Secret
@ 2020-07-06 21:54 Tobin Feldman-Fitzthum
2020-07-06 22:00 ` tobin
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Tobin Feldman-Fitzthum @ 2020-07-06 21:54 UTC (permalink / raw)
To: qemu-devel
Cc: thomas.lendacky, jejb, tobin, dgilbert, Tobin Feldman-Fitzthum,
pbonzini, brijesh.singh
AMD SEV allows a guest owner to inject a secret blob
into the memory of a virtual machine. The secret is
encrypted with the SEV Transport Encryption Key and
integrity is guaranteed with the Transport Integrity
Key. Although QEMU faciliates the injection of the
launch secret, it cannot access the secret.
Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
---
include/monitor/monitor.h | 3 ++
include/sysemu/sev.h | 2 ++
monitor/misc.c | 8 ++---
qapi/misc-target.json | 18 +++++++++++
target/i386/monitor.c | 9 ++++++
target/i386/sev-stub.c | 5 +++
target/i386/sev.c | 66 +++++++++++++++++++++++++++++++++++++++
target/i386/trace-events | 1 +
8 files changed, 108 insertions(+), 4 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1018d754a6..bf049c5b00 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -4,6 +4,7 @@
#include "block/block.h"
#include "qapi/qapi-types-misc.h"
#include "qemu/readline.h"
+#include "include/exec/hwaddr.h"
extern __thread Monitor *cur_mon;
typedef struct MonitorHMP MonitorHMP;
@@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
int monitor_set_cpu(int cpu_index);
int monitor_get_cpu_index(void);
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp);
+
void monitor_read_command(MonitorHMP *mon, int show_prompt);
int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
void *opaque);
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 98c1ec8d38..b279b293e8 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -18,4 +18,6 @@
void *sev_guest_init(const char *id);
int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa);
#endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 89bb970b00..b9ec8ba410 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor *mon, const QDict *qdict)
memory_dump(mon, count, format, size, addr, 1);
}
-static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
+void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp)
{
MemoryRegionSection mrs = memory_region_find(get_system_memory(),
- addr, 1);
+ addr, size);
if (!mrs.mr) {
error_setg(errp, "No memory is mapped at address 0x%" HWADDR_PRIx, addr);
@@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict *qdict)
MemoryRegion *mr = NULL;
void *ptr;
- ptr = gpa2hva(&mr, addr, &local_err);
+ ptr = gpa2hva(&mr, addr, 1, &local_err);
if (local_err) {
error_report_err(local_err);
return;
@@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict *qdict)
void *ptr;
uint64_t physaddr;
- ptr = gpa2hva(&mr, addr, &local_err);
+ ptr = gpa2hva(&mr, addr, 1, &local_err);
if (local_err) {
error_report_err(local_err);
return;
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index dee3b45930..d145f916b3 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -200,6 +200,24 @@
{ 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
'if': 'defined(TARGET_I386)' }
+##
+# @sev-inject-launch-secret:
+#
+# This command injects a secret blob into memory of SEV guest.
+#
+# @packet-header: the launch secret packet header encoded in base64
+#
+# @secret: the launch secret data to be injected encoded in base64
+#
+# @gpa: the guest physical address where secret will be injected.
+#
+# Since: 5.1
+#
+##
+{ 'command': 'sev-inject-launch-secret',
+ 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' },
+ 'if': 'defined(TARGET_I386)' }
+
##
# @dump-skeys:
#
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 27ebfa3ad2..42bcfe6dc0 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
return data;
}
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+ const char *secret, uint64_t gpa,
+ Error **errp)
+{
+ if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
+ error_setg(errp, "SEV inject secret failed");
+ }
+}
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index e5ee13309c..fed4588185 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
{
return NULL;
}
+int sev_inject_launch_secret(const char *hdr, const char *secret,
+ uint64_t gpa)
+{
+ return 1;
+}
diff --git a/target/i386/sev.c b/target/i386/sev.c
index d273174ad3..cbeb8f2e02 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -28,6 +28,8 @@
#include "sysemu/runstate.h"
#include "trace.h"
#include "migration/blocker.h"
+#include "exec/address-spaces.h"
+#include "monitor/monitor.h"
#define TYPE_SEV_GUEST "sev-guest"
#define SEV_GUEST(obj) \
@@ -769,6 +771,70 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
return 0;
}
+int sev_inject_launch_secret(const char *packet_hdr,
+ const char *secret, uint64_t gpa)
+{
+ struct kvm_sev_launch_secret input;
+ guchar *data = NULL, *hdr = NULL;
+ int error, ret = 1;
+ void *hva;
+ gsize hdr_sz = 0, data_sz = 0;
+ Error *local_err = NULL;
+ MemoryRegion *mr = NULL;
+
+ /* secret can be inject only in this state */
+ if (!sev_check_state(sev_guest, SEV_STATE_LAUNCH_SECRET)) {
+ error_report("SEV: Not in correct state. (LSECRET) %x",
+ sev_guest->state);
+ return 1;
+ }
+
+ hdr = g_base64_decode(packet_hdr, &hdr_sz);
+ if (!hdr || !hdr_sz) {
+ error_report("SEV: Failed to decode sequence header");
+ return 1;
+ }
+
+ data = g_base64_decode(secret, &data_sz);
+ if (!data || !data_sz) {
+ error_report("SEV: Failed to decode data");
+ goto err;
+ }
+
+ hva = gpa2hva(&mr, gpa, data_sz, &local_err);
+ if (!hva) {
+ error_report("SEV: Failed to calculate guest address.");
+ goto err;
+ }
+
+ input.hdr_uaddr = (uint64_t)(unsigned long)hdr;
+ input.hdr_len = hdr_sz;
+
+ input.trans_uaddr = (uint64_t)(unsigned long)data;
+ input.trans_len = data_sz;
+
+ input.guest_uaddr = (uint64_t)(unsigned long)hva;
+ input.guest_len = data_sz;
+
+ trace_kvm_sev_launch_secret(gpa, input.guest_uaddr,
+ input.trans_uaddr, input.trans_len);
+
+ ret = sev_ioctl(sev_guest->sev_fd, KVM_SEV_LAUNCH_SECRET,
+ &input, &error);
+ if (ret) {
+ error_report("SEV: failed to inject secret ret=%d fw_error=%d '%s'",
+ ret, error, fw_error_to_str(error));
+ goto err;
+ }
+
+ ret = 0;
+
+err:
+ g_free(data);
+ g_free(hdr);
+ return ret;
+}
+
static void
sev_register_types(void)
{
diff --git a/target/i386/trace-events b/target/i386/trace-events
index 789c700d4a..9f299e94a2 100644
--- a/target/i386/trace-events
+++ b/target/i386/trace-events
@@ -15,3 +15,4 @@ kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session
kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIu64
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"
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret
2020-07-06 21:54 [PATCH v3] SEV: QMP support for Inject-Launch-Secret Tobin Feldman-Fitzthum
@ 2020-07-06 22:00 ` tobin
2020-09-21 19:16 ` Dr. David Alan Gilbert
2020-10-12 16:21 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 11+ messages in thread
From: tobin @ 2020-07-06 22:00 UTC (permalink / raw)
To: Tobin Feldman-Fitzthum
Cc: thomas.lendacky, jejb, tobin, qemu-devel, dgilbert, pbonzini,
brijesh.singh
On 2020-07-06 17:54, Tobin Feldman-Fitzthum wrote:
Not sure if v3 is necessary, but here it is.
Fixed the 32-bit issues and removed the checks
on header and secret length. I agree with Brijesh
that those are best left to the PSP, which
returns somewhat helpful errors if either are incorrect.
Having a check in QEMU might be a handy hint for
developers who are trying to formulate a valid
secret, but it is easy enough to find the requirements
in the spec. This way we do not need to worry about
the secret format changing in future versions.
> AMD SEV allows a guest owner to inject a secret blob
> into the memory of a virtual machine. The secret is
> encrypted with the SEV Transport Encryption Key and
> integrity is guaranteed with the Transport Integrity
> Key. Although QEMU faciliates the injection of the
> launch secret, it cannot access the secret.
>
> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
> ---
> include/monitor/monitor.h | 3 ++
> include/sysemu/sev.h | 2 ++
> monitor/misc.c | 8 ++---
> qapi/misc-target.json | 18 +++++++++++
> target/i386/monitor.c | 9 ++++++
> target/i386/sev-stub.c | 5 +++
> target/i386/sev.c | 66 +++++++++++++++++++++++++++++++++++++++
> target/i386/trace-events | 1 +
> 8 files changed, 108 insertions(+), 4 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 1018d754a6..bf049c5b00 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -4,6 +4,7 @@
> #include "block/block.h"
> #include "qapi/qapi-types-misc.h"
> #include "qemu/readline.h"
> +#include "include/exec/hwaddr.h"
>
> extern __thread Monitor *cur_mon;
> typedef struct MonitorHMP MonitorHMP;
> @@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
> int monitor_set_cpu(int cpu_index);
> int monitor_get_cpu_index(void);
>
> +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error
> **errp);
> +
> void monitor_read_command(MonitorHMP *mon, int show_prompt);
> int monitor_read_password(MonitorHMP *mon, ReadLineFunc
> *readline_func,
> void *opaque);
> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
> index 98c1ec8d38..b279b293e8 100644
> --- a/include/sysemu/sev.h
> +++ b/include/sysemu/sev.h
> @@ -18,4 +18,6 @@
>
> void *sev_guest_init(const char *id);
> int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
> +int sev_inject_launch_secret(const char *hdr, const char *secret,
> + uint64_t gpa);
> #endif
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 89bb970b00..b9ec8ba410 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor
> *mon, const QDict *qdict)
> memory_dump(mon, count, format, size, addr, 1);
> }
>
> -static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
> +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error
> **errp)
> {
> MemoryRegionSection mrs = memory_region_find(get_system_memory(),
> - addr, 1);
> + addr, size);
>
> if (!mrs.mr) {
> error_setg(errp, "No memory is mapped at address 0x%"
> HWADDR_PRIx, addr);
> @@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict
> *qdict)
> MemoryRegion *mr = NULL;
> void *ptr;
>
> - ptr = gpa2hva(&mr, addr, &local_err);
> + ptr = gpa2hva(&mr, addr, 1, &local_err);
> if (local_err) {
> error_report_err(local_err);
> return;
> @@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict
> *qdict)
> void *ptr;
> uint64_t physaddr;
>
> - ptr = gpa2hva(&mr, addr, &local_err);
> + ptr = gpa2hva(&mr, addr, 1, &local_err);
> if (local_err) {
> error_report_err(local_err);
> return;
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index dee3b45930..d145f916b3 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -200,6 +200,24 @@
> { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
> 'if': 'defined(TARGET_I386)' }
>
> +##
> +# @sev-inject-launch-secret:
> +#
> +# This command injects a secret blob into memory of SEV guest.
> +#
> +# @packet-header: the launch secret packet header encoded in base64
> +#
> +# @secret: the launch secret data to be injected encoded in base64
> +#
> +# @gpa: the guest physical address where secret will be injected.
> +#
> +# Since: 5.1
> +#
> +##
> +{ 'command': 'sev-inject-launch-secret',
> + 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64'
> },
> + 'if': 'defined(TARGET_I386)' }
> +
> ##
> # @dump-skeys:
> #
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index 27ebfa3ad2..42bcfe6dc0 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error
> **errp)
>
> return data;
> }
> +
> +void qmp_sev_inject_launch_secret(const char *packet_hdr,
> + const char *secret, uint64_t gpa,
> + Error **errp)
> +{
> + if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
> + error_setg(errp, "SEV inject secret failed");
> + }
> +}
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> index e5ee13309c..fed4588185 100644
> --- a/target/i386/sev-stub.c
> +++ b/target/i386/sev-stub.c
> @@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
> {
> return NULL;
> }
> +int sev_inject_launch_secret(const char *hdr, const char *secret,
> + uint64_t gpa)
> +{
> + return 1;
> +}
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index d273174ad3..cbeb8f2e02 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -28,6 +28,8 @@
> #include "sysemu/runstate.h"
> #include "trace.h"
> #include "migration/blocker.h"
> +#include "exec/address-spaces.h"
> +#include "monitor/monitor.h"
>
> #define TYPE_SEV_GUEST "sev-guest"
> #define SEV_GUEST(obj) \
> @@ -769,6 +771,70 @@ sev_encrypt_data(void *handle, uint8_t *ptr,
> uint64_t len)
> return 0;
> }
>
> +int sev_inject_launch_secret(const char *packet_hdr,
> + const char *secret, uint64_t gpa)
> +{
> + struct kvm_sev_launch_secret input;
> + guchar *data = NULL, *hdr = NULL;
> + int error, ret = 1;
> + void *hva;
> + gsize hdr_sz = 0, data_sz = 0;
> + Error *local_err = NULL;
> + MemoryRegion *mr = NULL;
> +
> + /* secret can be inject only in this state */
> + if (!sev_check_state(sev_guest, SEV_STATE_LAUNCH_SECRET)) {
> + error_report("SEV: Not in correct state. (LSECRET) %x",
> + sev_guest->state);
> + return 1;
> + }
> +
> + hdr = g_base64_decode(packet_hdr, &hdr_sz);
> + if (!hdr || !hdr_sz) {
> + error_report("SEV: Failed to decode sequence header");
> + return 1;
> + }
> +
> + data = g_base64_decode(secret, &data_sz);
> + if (!data || !data_sz) {
> + error_report("SEV: Failed to decode data");
> + goto err;
> + }
> +
> + hva = gpa2hva(&mr, gpa, data_sz, &local_err);
> + if (!hva) {
> + error_report("SEV: Failed to calculate guest address.");
> + goto err;
> + }
> +
> + input.hdr_uaddr = (uint64_t)(unsigned long)hdr;
> + input.hdr_len = hdr_sz;
> +
> + input.trans_uaddr = (uint64_t)(unsigned long)data;
> + input.trans_len = data_sz;
> +
> + input.guest_uaddr = (uint64_t)(unsigned long)hva;
> + input.guest_len = data_sz;
> +
> + trace_kvm_sev_launch_secret(gpa, input.guest_uaddr,
> + input.trans_uaddr, input.trans_len);
> +
> + ret = sev_ioctl(sev_guest->sev_fd, KVM_SEV_LAUNCH_SECRET,
> + &input, &error);
> + if (ret) {
> + error_report("SEV: failed to inject secret ret=%d fw_error=%d
> '%s'",
> + ret, error, fw_error_to_str(error));
> + goto err;
> + }
> +
> + ret = 0;
> +
> +err:
> + g_free(data);
> + g_free(hdr);
> + return ret;
> +}
> +
> static void
> sev_register_types(void)
> {
> diff --git a/target/i386/trace-events b/target/i386/trace-events
> index 789c700d4a..9f299e94a2 100644
> --- a/target/i386/trace-events
> +++ b/target/i386/trace-events
> @@ -15,3 +15,4 @@ kvm_sev_launch_start(int policy, void *session, void
> *pdh) "policy 0x%x session
> kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%"
> PRIu64
> 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"
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret
2020-07-06 21:54 [PATCH v3] SEV: QMP support for Inject-Launch-Secret Tobin Feldman-Fitzthum
2020-07-06 22:00 ` tobin
@ 2020-09-21 19:16 ` Dr. David Alan Gilbert
2020-09-21 20:33 ` Tobin Feldman-Fitzthum
2020-10-12 16:21 ` Dr. David Alan Gilbert
2 siblings, 1 reply; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2020-09-21 19:16 UTC (permalink / raw)
To: Tobin Feldman-Fitzthum
Cc: thomas.lendacky, brijesh.singh, jejb, tobin, qemu-devel, pbonzini
* Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
> AMD SEV allows a guest owner to inject a secret blob
> into the memory of a virtual machine. The secret is
> encrypted with the SEV Transport Encryption Key and
> integrity is guaranteed with the Transport Integrity
> Key. Although QEMU faciliates the injection of the
> launch secret, it cannot access the secret.
>
> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
Hi Tobin,
Did the ovmf stuff for agreeing the GUID for automating this ever
happen?
Dave
> ---
> include/monitor/monitor.h | 3 ++
> include/sysemu/sev.h | 2 ++
> monitor/misc.c | 8 ++---
> qapi/misc-target.json | 18 +++++++++++
> target/i386/monitor.c | 9 ++++++
> target/i386/sev-stub.c | 5 +++
> target/i386/sev.c | 66 +++++++++++++++++++++++++++++++++++++++
> target/i386/trace-events | 1 +
> 8 files changed, 108 insertions(+), 4 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 1018d754a6..bf049c5b00 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -4,6 +4,7 @@
> #include "block/block.h"
> #include "qapi/qapi-types-misc.h"
> #include "qemu/readline.h"
> +#include "include/exec/hwaddr.h"
>
> extern __thread Monitor *cur_mon;
> typedef struct MonitorHMP MonitorHMP;
> @@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
> int monitor_set_cpu(int cpu_index);
> int monitor_get_cpu_index(void);
>
> +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp);
> +
> void monitor_read_command(MonitorHMP *mon, int show_prompt);
> int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
> void *opaque);
> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
> index 98c1ec8d38..b279b293e8 100644
> --- a/include/sysemu/sev.h
> +++ b/include/sysemu/sev.h
> @@ -18,4 +18,6 @@
>
> void *sev_guest_init(const char *id);
> int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
> +int sev_inject_launch_secret(const char *hdr, const char *secret,
> + uint64_t gpa);
> #endif
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 89bb970b00..b9ec8ba410 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor *mon, const QDict *qdict)
> memory_dump(mon, count, format, size, addr, 1);
> }
>
> -static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
> +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp)
> {
> MemoryRegionSection mrs = memory_region_find(get_system_memory(),
> - addr, 1);
> + addr, size);
>
> if (!mrs.mr) {
> error_setg(errp, "No memory is mapped at address 0x%" HWADDR_PRIx, addr);
> @@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict *qdict)
> MemoryRegion *mr = NULL;
> void *ptr;
>
> - ptr = gpa2hva(&mr, addr, &local_err);
> + ptr = gpa2hva(&mr, addr, 1, &local_err);
> if (local_err) {
> error_report_err(local_err);
> return;
> @@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict *qdict)
> void *ptr;
> uint64_t physaddr;
>
> - ptr = gpa2hva(&mr, addr, &local_err);
> + ptr = gpa2hva(&mr, addr, 1, &local_err);
> if (local_err) {
> error_report_err(local_err);
> return;
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index dee3b45930..d145f916b3 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -200,6 +200,24 @@
> { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
> 'if': 'defined(TARGET_I386)' }
>
> +##
> +# @sev-inject-launch-secret:
> +#
> +# This command injects a secret blob into memory of SEV guest.
> +#
> +# @packet-header: the launch secret packet header encoded in base64
> +#
> +# @secret: the launch secret data to be injected encoded in base64
> +#
> +# @gpa: the guest physical address where secret will be injected.
> +#
> +# Since: 5.1
> +#
> +##
> +{ 'command': 'sev-inject-launch-secret',
> + 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' },
> + 'if': 'defined(TARGET_I386)' }
> +
> ##
> # @dump-skeys:
> #
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index 27ebfa3ad2..42bcfe6dc0 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
>
> return data;
> }
> +
> +void qmp_sev_inject_launch_secret(const char *packet_hdr,
> + const char *secret, uint64_t gpa,
> + Error **errp)
> +{
> + if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
> + error_setg(errp, "SEV inject secret failed");
> + }
> +}
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> index e5ee13309c..fed4588185 100644
> --- a/target/i386/sev-stub.c
> +++ b/target/i386/sev-stub.c
> @@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
> {
> return NULL;
> }
> +int sev_inject_launch_secret(const char *hdr, const char *secret,
> + uint64_t gpa)
> +{
> + return 1;
> +}
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index d273174ad3..cbeb8f2e02 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -28,6 +28,8 @@
> #include "sysemu/runstate.h"
> #include "trace.h"
> #include "migration/blocker.h"
> +#include "exec/address-spaces.h"
> +#include "monitor/monitor.h"
>
> #define TYPE_SEV_GUEST "sev-guest"
> #define SEV_GUEST(obj) \
> @@ -769,6 +771,70 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
> return 0;
> }
>
> +int sev_inject_launch_secret(const char *packet_hdr,
> + const char *secret, uint64_t gpa)
> +{
> + struct kvm_sev_launch_secret input;
> + guchar *data = NULL, *hdr = NULL;
> + int error, ret = 1;
> + void *hva;
> + gsize hdr_sz = 0, data_sz = 0;
> + Error *local_err = NULL;
> + MemoryRegion *mr = NULL;
> +
> + /* secret can be inject only in this state */
> + if (!sev_check_state(sev_guest, SEV_STATE_LAUNCH_SECRET)) {
> + error_report("SEV: Not in correct state. (LSECRET) %x",
> + sev_guest->state);
> + return 1;
> + }
> +
> + hdr = g_base64_decode(packet_hdr, &hdr_sz);
> + if (!hdr || !hdr_sz) {
> + error_report("SEV: Failed to decode sequence header");
> + return 1;
> + }
> +
> + data = g_base64_decode(secret, &data_sz);
> + if (!data || !data_sz) {
> + error_report("SEV: Failed to decode data");
> + goto err;
> + }
> +
> + hva = gpa2hva(&mr, gpa, data_sz, &local_err);
> + if (!hva) {
> + error_report("SEV: Failed to calculate guest address.");
> + goto err;
> + }
> +
> + input.hdr_uaddr = (uint64_t)(unsigned long)hdr;
> + input.hdr_len = hdr_sz;
> +
> + input.trans_uaddr = (uint64_t)(unsigned long)data;
> + input.trans_len = data_sz;
> +
> + input.guest_uaddr = (uint64_t)(unsigned long)hva;
> + input.guest_len = data_sz;
> +
> + trace_kvm_sev_launch_secret(gpa, input.guest_uaddr,
> + input.trans_uaddr, input.trans_len);
> +
> + ret = sev_ioctl(sev_guest->sev_fd, KVM_SEV_LAUNCH_SECRET,
> + &input, &error);
> + if (ret) {
> + error_report("SEV: failed to inject secret ret=%d fw_error=%d '%s'",
> + ret, error, fw_error_to_str(error));
> + goto err;
> + }
> +
> + ret = 0;
> +
> +err:
> + g_free(data);
> + g_free(hdr);
> + return ret;
> +}
> +
> static void
> sev_register_types(void)
> {
> diff --git a/target/i386/trace-events b/target/i386/trace-events
> index 789c700d4a..9f299e94a2 100644
> --- a/target/i386/trace-events
> +++ b/target/i386/trace-events
> @@ -15,3 +15,4 @@ kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session
> kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIu64
> 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"
> --
> 2.20.1 (Apple Git-117)
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret
2020-09-21 19:16 ` Dr. David Alan Gilbert
@ 2020-09-21 20:33 ` Tobin Feldman-Fitzthum
2020-09-21 22:14 ` Tom Lendacky
2020-10-12 15:57 ` Dr. David Alan Gilbert
0 siblings, 2 replies; 11+ messages in thread
From: Tobin Feldman-Fitzthum @ 2020-09-21 20:33 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: thomas.lendacky, brijesh.singh, jejb, tobin, qemu-devel, pbonzini
On 2020-09-21 15:16, Dr. David Alan Gilbert wrote:
> * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
>> AMD SEV allows a guest owner to inject a secret blob
>> into the memory of a virtual machine. The secret is
>> encrypted with the SEV Transport Encryption Key and
>> integrity is guaranteed with the Transport Integrity
>> Key. Although QEMU faciliates the injection of the
>> launch secret, it cannot access the secret.
>>
>> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
>
> Hi Tobin,
> Did the ovmf stuff for agreeing the GUID for automating this ever
> happen?
>
OVMF patches have not been upstreamed yet. I think we are planning
to do that relatively soon.
-Tobin
> Dave
>
>> ---
>> include/monitor/monitor.h | 3 ++
>> include/sysemu/sev.h | 2 ++
>> monitor/misc.c | 8 ++---
>> qapi/misc-target.json | 18 +++++++++++
>> target/i386/monitor.c | 9 ++++++
>> target/i386/sev-stub.c | 5 +++
>> target/i386/sev.c | 66
>> +++++++++++++++++++++++++++++++++++++++
>> target/i386/trace-events | 1 +
>> 8 files changed, 108 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
>> index 1018d754a6..bf049c5b00 100644
>> --- a/include/monitor/monitor.h
>> +++ b/include/monitor/monitor.h
>> @@ -4,6 +4,7 @@
>> #include "block/block.h"
>> #include "qapi/qapi-types-misc.h"
>> #include "qemu/readline.h"
>> +#include "include/exec/hwaddr.h"
>>
>> extern __thread Monitor *cur_mon;
>> typedef struct MonitorHMP MonitorHMP;
>> @@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
>> int monitor_set_cpu(int cpu_index);
>> int monitor_get_cpu_index(void);
>>
>> +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error
>> **errp);
>> +
>> void monitor_read_command(MonitorHMP *mon, int show_prompt);
>> int monitor_read_password(MonitorHMP *mon, ReadLineFunc
>> *readline_func,
>> void *opaque);
>> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
>> index 98c1ec8d38..b279b293e8 100644
>> --- a/include/sysemu/sev.h
>> +++ b/include/sysemu/sev.h
>> @@ -18,4 +18,6 @@
>>
>> void *sev_guest_init(const char *id);
>> int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
>> +int sev_inject_launch_secret(const char *hdr, const char *secret,
>> + uint64_t gpa);
>> #endif
>> diff --git a/monitor/misc.c b/monitor/misc.c
>> index 89bb970b00..b9ec8ba410 100644
>> --- a/monitor/misc.c
>> +++ b/monitor/misc.c
>> @@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor
>> *mon, const QDict *qdict)
>> memory_dump(mon, count, format, size, addr, 1);
>> }
>>
>> -static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
>> +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error
>> **errp)
>> {
>> MemoryRegionSection mrs = memory_region_find(get_system_memory(),
>> - addr, 1);
>> + addr, size);
>>
>> if (!mrs.mr) {
>> error_setg(errp, "No memory is mapped at address 0x%"
>> HWADDR_PRIx, addr);
>> @@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict
>> *qdict)
>> MemoryRegion *mr = NULL;
>> void *ptr;
>>
>> - ptr = gpa2hva(&mr, addr, &local_err);
>> + ptr = gpa2hva(&mr, addr, 1, &local_err);
>> if (local_err) {
>> error_report_err(local_err);
>> return;
>> @@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict
>> *qdict)
>> void *ptr;
>> uint64_t physaddr;
>>
>> - ptr = gpa2hva(&mr, addr, &local_err);
>> + ptr = gpa2hva(&mr, addr, 1, &local_err);
>> if (local_err) {
>> error_report_err(local_err);
>> return;
>> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>> index dee3b45930..d145f916b3 100644
>> --- a/qapi/misc-target.json
>> +++ b/qapi/misc-target.json
>> @@ -200,6 +200,24 @@
>> { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
>> 'if': 'defined(TARGET_I386)' }
>>
>> +##
>> +# @sev-inject-launch-secret:
>> +#
>> +# This command injects a secret blob into memory of SEV guest.
>> +#
>> +# @packet-header: the launch secret packet header encoded in base64
>> +#
>> +# @secret: the launch secret data to be injected encoded in base64
>> +#
>> +# @gpa: the guest physical address where secret will be injected.
>> +#
>> +# Since: 5.1
>> +#
>> +##
>> +{ 'command': 'sev-inject-launch-secret',
>> + 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64'
>> },
>> + 'if': 'defined(TARGET_I386)' }
>> +
>> ##
>> # @dump-skeys:
>> #
>> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
>> index 27ebfa3ad2..42bcfe6dc0 100644
>> --- a/target/i386/monitor.c
>> +++ b/target/i386/monitor.c
>> @@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error
>> **errp)
>>
>> return data;
>> }
>> +
>> +void qmp_sev_inject_launch_secret(const char *packet_hdr,
>> + const char *secret, uint64_t gpa,
>> + Error **errp)
>> +{
>> + if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
>> + error_setg(errp, "SEV inject secret failed");
>> + }
>> +}
>> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
>> index e5ee13309c..fed4588185 100644
>> --- a/target/i386/sev-stub.c
>> +++ b/target/i386/sev-stub.c
>> @@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
>> {
>> return NULL;
>> }
>> +int sev_inject_launch_secret(const char *hdr, const char *secret,
>> + uint64_t gpa)
>> +{
>> + return 1;
>> +}
>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>> index d273174ad3..cbeb8f2e02 100644
>> --- a/target/i386/sev.c
>> +++ b/target/i386/sev.c
>> @@ -28,6 +28,8 @@
>> #include "sysemu/runstate.h"
>> #include "trace.h"
>> #include "migration/blocker.h"
>> +#include "exec/address-spaces.h"
>> +#include "monitor/monitor.h"
>>
>> #define TYPE_SEV_GUEST "sev-guest"
>> #define SEV_GUEST(obj) \
>> @@ -769,6 +771,70 @@ sev_encrypt_data(void *handle, uint8_t *ptr,
>> uint64_t len)
>> return 0;
>> }
>>
>> +int sev_inject_launch_secret(const char *packet_hdr,
>> + const char *secret, uint64_t gpa)
>> +{
>> + struct kvm_sev_launch_secret input;
>> + guchar *data = NULL, *hdr = NULL;
>> + int error, ret = 1;
>> + void *hva;
>> + gsize hdr_sz = 0, data_sz = 0;
>> + Error *local_err = NULL;
>> + MemoryRegion *mr = NULL;
>> +
>> + /* secret can be inject only in this state */
>> + if (!sev_check_state(sev_guest, SEV_STATE_LAUNCH_SECRET)) {
>> + error_report("SEV: Not in correct state. (LSECRET) %x",
>> + sev_guest->state);
>> + return 1;
>> + }
>> +
>> + hdr = g_base64_decode(packet_hdr, &hdr_sz);
>> + if (!hdr || !hdr_sz) {
>> + error_report("SEV: Failed to decode sequence header");
>> + return 1;
>> + }
>> +
>> + data = g_base64_decode(secret, &data_sz);
>> + if (!data || !data_sz) {
>> + error_report("SEV: Failed to decode data");
>> + goto err;
>> + }
>> +
>> + hva = gpa2hva(&mr, gpa, data_sz, &local_err);
>> + if (!hva) {
>> + error_report("SEV: Failed to calculate guest address.");
>> + goto err;
>> + }
>> +
>> + input.hdr_uaddr = (uint64_t)(unsigned long)hdr;
>> + input.hdr_len = hdr_sz;
>> +
>> + input.trans_uaddr = (uint64_t)(unsigned long)data;
>> + input.trans_len = data_sz;
>> +
>> + input.guest_uaddr = (uint64_t)(unsigned long)hva;
>> + input.guest_len = data_sz;
>> +
>> + trace_kvm_sev_launch_secret(gpa, input.guest_uaddr,
>> + input.trans_uaddr, input.trans_len);
>> +
>> + ret = sev_ioctl(sev_guest->sev_fd, KVM_SEV_LAUNCH_SECRET,
>> + &input, &error);
>> + if (ret) {
>> + error_report("SEV: failed to inject secret ret=%d fw_error=%d
>> '%s'",
>> + ret, error, fw_error_to_str(error));
>> + goto err;
>> + }
>> +
>> + ret = 0;
>> +
>> +err:
>> + g_free(data);
>> + g_free(hdr);
>> + return ret;
>> +}
>> +
>> static void
>> sev_register_types(void)
>> {
>> diff --git a/target/i386/trace-events b/target/i386/trace-events
>> index 789c700d4a..9f299e94a2 100644
>> --- a/target/i386/trace-events
>> +++ b/target/i386/trace-events
>> @@ -15,3 +15,4 @@ kvm_sev_launch_start(int policy, void *session, void
>> *pdh) "policy 0x%x session
>> kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len
>> 0x%" PRIu64
>> 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"
>> --
>> 2.20.1 (Apple Git-117)
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret
2020-09-21 20:33 ` Tobin Feldman-Fitzthum
@ 2020-09-21 22:14 ` Tom Lendacky
2020-10-12 15:57 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 11+ messages in thread
From: Tom Lendacky @ 2020-09-21 22:14 UTC (permalink / raw)
To: Tobin Feldman-Fitzthum, Dr. David Alan Gilbert
Cc: brijesh.singh, jejb, tobin, qemu-devel, pbonzini
On 9/21/20 3:33 PM, Tobin Feldman-Fitzthum wrote:
> On 2020-09-21 15:16, Dr. David Alan Gilbert wrote:
>> * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
>>> AMD SEV allows a guest owner to inject a secret blob
>>> into the memory of a virtual machine. The secret is
>>> encrypted with the SEV Transport Encryption Key and
>>> integrity is guaranteed with the Transport Integrity
>>> Key. Although QEMU faciliates the injection of the
>>> launch secret, it cannot access the secret.
>>>
>>> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
>>
>> Hi Tobin,
>> Did the ovmf stuff for agreeing the GUID for automating this ever
>> happen?
>>
> OVMF patches have not been upstreamed yet. I think we are planning
> to do that relatively soon.
The SEV-ES code was recently accepted and uses a GUID for locating SEV
related information. You can probably build upon that GUID or stack
another GUID above that. The GUID block has a size indicator that can help
determine what information is available or for tracking stacked GUIDs.
https://github.com/tianocore/edk2/commit/30937f2f98c42496f2f143fe8374ae7f7e684847
Thanks,
Tom
>
> -Tobin
>> Dave
>>
>>> ---
>>> include/monitor/monitor.h | 3 ++
>>> include/sysemu/sev.h | 2 ++
>>> monitor/misc.c | 8 ++---
>>> qapi/misc-target.json | 18 +++++++++++
>>> target/i386/monitor.c | 9 ++++++
>>> target/i386/sev-stub.c | 5 +++
>>> target/i386/sev.c | 66 +++++++++++++++++++++++++++++++++++++++
>>> target/i386/trace-events | 1 +
>>> 8 files changed, 108 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
>>> index 1018d754a6..bf049c5b00 100644
>>> --- a/include/monitor/monitor.h
>>> +++ b/include/monitor/monitor.h
>>> @@ -4,6 +4,7 @@
>>> #include "block/block.h"
>>> #include "qapi/qapi-types-misc.h"
>>> #include "qemu/readline.h"
>>> +#include "include/exec/hwaddr.h"
>>>
>>> extern __thread Monitor *cur_mon;
>>> typedef struct MonitorHMP MonitorHMP;
>>> @@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
>>> int monitor_set_cpu(int cpu_index);
>>> int monitor_get_cpu_index(void);
>>>
>>> +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error
>>> **errp);
>>> +
>>> void monitor_read_command(MonitorHMP *mon, int show_prompt);
>>> int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
>>> void *opaque);
>>> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
>>> index 98c1ec8d38..b279b293e8 100644
>>> --- a/include/sysemu/sev.h
>>> +++ b/include/sysemu/sev.h
>>> @@ -18,4 +18,6 @@
>>>
>>> void *sev_guest_init(const char *id);
>>> int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
>>> +int sev_inject_launch_secret(const char *hdr, const char *secret,
>>> + uint64_t gpa);
>>> #endif
>>> diff --git a/monitor/misc.c b/monitor/misc.c
>>> index 89bb970b00..b9ec8ba410 100644
>>> --- a/monitor/misc.c
>>> +++ b/monitor/misc.c
>>> @@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor
>>> *mon, const QDict *qdict)
>>> memory_dump(mon, count, format, size, addr, 1);
>>> }
>>>
>>> -static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
>>> +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error
>>> **errp)
>>> {
>>> MemoryRegionSection mrs = memory_region_find(get_system_memory(),
>>> - addr, 1);
>>> + addr, size);
>>>
>>> if (!mrs.mr) {
>>> error_setg(errp, "No memory is mapped at address 0x%"
>>> HWADDR_PRIx, addr);
>>> @@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict
>>> *qdict)
>>> MemoryRegion *mr = NULL;
>>> void *ptr;
>>>
>>> - ptr = gpa2hva(&mr, addr, &local_err);
>>> + ptr = gpa2hva(&mr, addr, 1, &local_err);
>>> if (local_err) {
>>> error_report_err(local_err);
>>> return;
>>> @@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict
>>> *qdict)
>>> void *ptr;
>>> uint64_t physaddr;
>>>
>>> - ptr = gpa2hva(&mr, addr, &local_err);
>>> + ptr = gpa2hva(&mr, addr, 1, &local_err);
>>> if (local_err) {
>>> error_report_err(local_err);
>>> return;
>>> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>>> index dee3b45930..d145f916b3 100644
>>> --- a/qapi/misc-target.json
>>> +++ b/qapi/misc-target.json
>>> @@ -200,6 +200,24 @@
>>> { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
>>> 'if': 'defined(TARGET_I386)' }
>>>
>>> +##
>>> +# @sev-inject-launch-secret:
>>> +#
>>> +# This command injects a secret blob into memory of SEV guest.
>>> +#
>>> +# @packet-header: the launch secret packet header encoded in base64
>>> +#
>>> +# @secret: the launch secret data to be injected encoded in base64
>>> +#
>>> +# @gpa: the guest physical address where secret will be injected.
>>> +#
>>> +# Since: 5.1
>>> +#
>>> +##
>>> +{ 'command': 'sev-inject-launch-secret',
>>> + 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' },
>>> + 'if': 'defined(TARGET_I386)' }
>>> +
>>> ##
>>> # @dump-skeys:
>>> #
>>> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
>>> index 27ebfa3ad2..42bcfe6dc0 100644
>>> --- a/target/i386/monitor.c
>>> +++ b/target/i386/monitor.c
>>> @@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error
>>> **errp)
>>>
>>> return data;
>>> }
>>> +
>>> +void qmp_sev_inject_launch_secret(const char *packet_hdr,
>>> + const char *secret, uint64_t gpa,
>>> + Error **errp)
>>> +{
>>> + if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
>>> + error_setg(errp, "SEV inject secret failed");
>>> + }
>>> +}
>>> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
>>> index e5ee13309c..fed4588185 100644
>>> --- a/target/i386/sev-stub.c
>>> +++ b/target/i386/sev-stub.c
>>> @@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
>>> {
>>> return NULL;
>>> }
>>> +int sev_inject_launch_secret(const char *hdr, const char *secret,
>>> + uint64_t gpa)
>>> +{
>>> + return 1;
>>> +}
>>> diff --git a/target/i386/sev.c b/target/i386/sev.c
>>> index d273174ad3..cbeb8f2e02 100644
>>> --- a/target/i386/sev.c
>>> +++ b/target/i386/sev.c
>>> @@ -28,6 +28,8 @@
>>> #include "sysemu/runstate.h"
>>> #include "trace.h"
>>> #include "migration/blocker.h"
>>> +#include "exec/address-spaces.h"
>>> +#include "monitor/monitor.h"
>>>
>>> #define TYPE_SEV_GUEST "sev-guest"
>>> #define SEV_GUEST(obj) \
>>> @@ -769,6 +771,70 @@ sev_encrypt_data(void *handle, uint8_t *ptr,
>>> uint64_t len)
>>> return 0;
>>> }
>>>
>>> +int sev_inject_launch_secret(const char *packet_hdr,
>>> + const char *secret, uint64_t gpa)
>>> +{
>>> + struct kvm_sev_launch_secret input;
>>> + guchar *data = NULL, *hdr = NULL;
>>> + int error, ret = 1;
>>> + void *hva;
>>> + gsize hdr_sz = 0, data_sz = 0;
>>> + Error *local_err = NULL;
>>> + MemoryRegion *mr = NULL;
>>> +
>>> + /* secret can be inject only in this state */
>>> + if (!sev_check_state(sev_guest, SEV_STATE_LAUNCH_SECRET)) {
>>> + error_report("SEV: Not in correct state. (LSECRET) %x",
>>> + sev_guest->state);
>>> + return 1;
>>> + }
>>> +
>>> + hdr = g_base64_decode(packet_hdr, &hdr_sz);
>>> + if (!hdr || !hdr_sz) {
>>> + error_report("SEV: Failed to decode sequence header");
>>> + return 1;
>>> + }
>>> +
>>> + data = g_base64_decode(secret, &data_sz);
>>> + if (!data || !data_sz) {
>>> + error_report("SEV: Failed to decode data");
>>> + goto err;
>>> + }
>>> +
>>> + hva = gpa2hva(&mr, gpa, data_sz, &local_err);
>>> + if (!hva) {
>>> + error_report("SEV: Failed to calculate guest address.");
>>> + goto err;
>>> + }
>>> +
>>> + input.hdr_uaddr = (uint64_t)(unsigned long)hdr;
>>> + input.hdr_len = hdr_sz;
>>> +
>>> + input.trans_uaddr = (uint64_t)(unsigned long)data;
>>> + input.trans_len = data_sz;
>>> +
>>> + input.guest_uaddr = (uint64_t)(unsigned long)hva;
>>> + input.guest_len = data_sz;
>>> +
>>> + trace_kvm_sev_launch_secret(gpa, input.guest_uaddr,
>>> + input.trans_uaddr, input.trans_len);
>>> +
>>> + ret = sev_ioctl(sev_guest->sev_fd, KVM_SEV_LAUNCH_SECRET,
>>> + &input, &error);
>>> + if (ret) {
>>> + error_report("SEV: failed to inject secret ret=%d fw_error=%d
>>> '%s'",
>>> + ret, error, fw_error_to_str(error));
>>> + goto err;
>>> + }
>>> +
>>> + ret = 0;
>>> +
>>> +err:
>>> + g_free(data);
>>> + g_free(hdr);
>>> + return ret;
>>> +}
>>> +
>>> static void
>>> sev_register_types(void)
>>> {
>>> diff --git a/target/i386/trace-events b/target/i386/trace-events
>>> index 789c700d4a..9f299e94a2 100644
>>> --- a/target/i386/trace-events
>>> +++ b/target/i386/trace-events
>>> @@ -15,3 +15,4 @@ kvm_sev_launch_start(int policy, void *session, void
>>> *pdh) "policy 0x%x session
>>> kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%"
>>> PRIu64
>>> 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"
>>> --
>>> 2.20.1 (Apple Git-117)
>>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret
2020-09-21 20:33 ` Tobin Feldman-Fitzthum
2020-09-21 22:14 ` Tom Lendacky
@ 2020-10-12 15:57 ` Dr. David Alan Gilbert
2020-10-12 16:00 ` James Bottomley
1 sibling, 1 reply; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2020-10-12 15:57 UTC (permalink / raw)
To: Tobin Feldman-Fitzthum
Cc: thomas.lendacky, brijesh.singh, jejb, tobin, qemu-devel, pbonzini
* Tobin Feldman-Fitzthum (tobin@linux.ibm.com) wrote:
> On 2020-09-21 15:16, Dr. David Alan Gilbert wrote:
> > * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
> > > AMD SEV allows a guest owner to inject a secret blob
> > > into the memory of a virtual machine. The secret is
> > > encrypted with the SEV Transport Encryption Key and
> > > integrity is guaranteed with the Transport Integrity
> > > Key. Although QEMU faciliates the injection of the
> > > launch secret, it cannot access the secret.
> > >
> > > Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
> >
> > Hi Tobin,
> > Did the ovmf stuff for agreeing the GUID for automating this ever
> > happen?
> >
> OVMF patches have not been upstreamed yet. I think we are planning
> to do that relatively soon.
So as we're getting to the end of another qemu dev cycle; do we aim
to get this one in by itself, or to wait for the GUID?
Dave
> -Tobin
> > Dave
> >
> > > ---
> > > include/monitor/monitor.h | 3 ++
> > > include/sysemu/sev.h | 2 ++
> > > monitor/misc.c | 8 ++---
> > > qapi/misc-target.json | 18 +++++++++++
> > > target/i386/monitor.c | 9 ++++++
> > > target/i386/sev-stub.c | 5 +++
> > > target/i386/sev.c | 66
> > > +++++++++++++++++++++++++++++++++++++++
> > > target/i386/trace-events | 1 +
> > > 8 files changed, 108 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> > > index 1018d754a6..bf049c5b00 100644
> > > --- a/include/monitor/monitor.h
> > > +++ b/include/monitor/monitor.h
> > > @@ -4,6 +4,7 @@
> > > #include "block/block.h"
> > > #include "qapi/qapi-types-misc.h"
> > > #include "qemu/readline.h"
> > > +#include "include/exec/hwaddr.h"
> > >
> > > extern __thread Monitor *cur_mon;
> > > typedef struct MonitorHMP MonitorHMP;
> > > @@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
> > > int monitor_set_cpu(int cpu_index);
> > > int monitor_get_cpu_index(void);
> > >
> > > +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size,
> > > Error **errp);
> > > +
> > > void monitor_read_command(MonitorHMP *mon, int show_prompt);
> > > int monitor_read_password(MonitorHMP *mon, ReadLineFunc
> > > *readline_func,
> > > void *opaque);
> > > diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
> > > index 98c1ec8d38..b279b293e8 100644
> > > --- a/include/sysemu/sev.h
> > > +++ b/include/sysemu/sev.h
> > > @@ -18,4 +18,6 @@
> > >
> > > void *sev_guest_init(const char *id);
> > > int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
> > > +int sev_inject_launch_secret(const char *hdr, const char *secret,
> > > + uint64_t gpa);
> > > #endif
> > > diff --git a/monitor/misc.c b/monitor/misc.c
> > > index 89bb970b00..b9ec8ba410 100644
> > > --- a/monitor/misc.c
> > > +++ b/monitor/misc.c
> > > @@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor
> > > *mon, const QDict *qdict)
> > > memory_dump(mon, count, format, size, addr, 1);
> > > }
> > >
> > > -static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
> > > +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size,
> > > Error **errp)
> > > {
> > > MemoryRegionSection mrs = memory_region_find(get_system_memory(),
> > > - addr, 1);
> > > + addr, size);
> > >
> > > if (!mrs.mr) {
> > > error_setg(errp, "No memory is mapped at address 0x%"
> > > HWADDR_PRIx, addr);
> > > @@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const
> > > QDict *qdict)
> > > MemoryRegion *mr = NULL;
> > > void *ptr;
> > >
> > > - ptr = gpa2hva(&mr, addr, &local_err);
> > > + ptr = gpa2hva(&mr, addr, 1, &local_err);
> > > if (local_err) {
> > > error_report_err(local_err);
> > > return;
> > > @@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const
> > > QDict *qdict)
> > > void *ptr;
> > > uint64_t physaddr;
> > >
> > > - ptr = gpa2hva(&mr, addr, &local_err);
> > > + ptr = gpa2hva(&mr, addr, 1, &local_err);
> > > if (local_err) {
> > > error_report_err(local_err);
> > > return;
> > > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > > index dee3b45930..d145f916b3 100644
> > > --- a/qapi/misc-target.json
> > > +++ b/qapi/misc-target.json
> > > @@ -200,6 +200,24 @@
> > > { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
> > > 'if': 'defined(TARGET_I386)' }
> > >
> > > +##
> > > +# @sev-inject-launch-secret:
> > > +#
> > > +# This command injects a secret blob into memory of SEV guest.
> > > +#
> > > +# @packet-header: the launch secret packet header encoded in base64
> > > +#
> > > +# @secret: the launch secret data to be injected encoded in base64
> > > +#
> > > +# @gpa: the guest physical address where secret will be injected.
> > > +#
> > > +# Since: 5.1
> > > +#
> > > +##
> > > +{ 'command': 'sev-inject-launch-secret',
> > > + 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa':
> > > 'uint64' },
> > > + 'if': 'defined(TARGET_I386)' }
> > > +
> > > ##
> > > # @dump-skeys:
> > > #
> > > diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> > > index 27ebfa3ad2..42bcfe6dc0 100644
> > > --- a/target/i386/monitor.c
> > > +++ b/target/i386/monitor.c
> > > @@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error
> > > **errp)
> > >
> > > return data;
> > > }
> > > +
> > > +void qmp_sev_inject_launch_secret(const char *packet_hdr,
> > > + const char *secret, uint64_t gpa,
> > > + Error **errp)
> > > +{
> > > + if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
> > > + error_setg(errp, "SEV inject secret failed");
> > > + }
> > > +}
> > > diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> > > index e5ee13309c..fed4588185 100644
> > > --- a/target/i386/sev-stub.c
> > > +++ b/target/i386/sev-stub.c
> > > @@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
> > > {
> > > return NULL;
> > > }
> > > +int sev_inject_launch_secret(const char *hdr, const char *secret,
> > > + uint64_t gpa)
> > > +{
> > > + return 1;
> > > +}
> > > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > > index d273174ad3..cbeb8f2e02 100644
> > > --- a/target/i386/sev.c
> > > +++ b/target/i386/sev.c
> > > @@ -28,6 +28,8 @@
> > > #include "sysemu/runstate.h"
> > > #include "trace.h"
> > > #include "migration/blocker.h"
> > > +#include "exec/address-spaces.h"
> > > +#include "monitor/monitor.h"
> > >
> > > #define TYPE_SEV_GUEST "sev-guest"
> > > #define SEV_GUEST(obj) \
> > > @@ -769,6 +771,70 @@ sev_encrypt_data(void *handle, uint8_t *ptr,
> > > uint64_t len)
> > > return 0;
> > > }
> > >
> > > +int sev_inject_launch_secret(const char *packet_hdr,
> > > + const char *secret, uint64_t gpa)
> > > +{
> > > + struct kvm_sev_launch_secret input;
> > > + guchar *data = NULL, *hdr = NULL;
> > > + int error, ret = 1;
> > > + void *hva;
> > > + gsize hdr_sz = 0, data_sz = 0;
> > > + Error *local_err = NULL;
> > > + MemoryRegion *mr = NULL;
> > > +
> > > + /* secret can be inject only in this state */
> > > + if (!sev_check_state(sev_guest, SEV_STATE_LAUNCH_SECRET)) {
> > > + error_report("SEV: Not in correct state. (LSECRET) %x",
> > > + sev_guest->state);
> > > + return 1;
> > > + }
> > > +
> > > + hdr = g_base64_decode(packet_hdr, &hdr_sz);
> > > + if (!hdr || !hdr_sz) {
> > > + error_report("SEV: Failed to decode sequence header");
> > > + return 1;
> > > + }
> > > +
> > > + data = g_base64_decode(secret, &data_sz);
> > > + if (!data || !data_sz) {
> > > + error_report("SEV: Failed to decode data");
> > > + goto err;
> > > + }
> > > +
> > > + hva = gpa2hva(&mr, gpa, data_sz, &local_err);
> > > + if (!hva) {
> > > + error_report("SEV: Failed to calculate guest address.");
> > > + goto err;
> > > + }
> > > +
> > > + input.hdr_uaddr = (uint64_t)(unsigned long)hdr;
> > > + input.hdr_len = hdr_sz;
> > > +
> > > + input.trans_uaddr = (uint64_t)(unsigned long)data;
> > > + input.trans_len = data_sz;
> > > +
> > > + input.guest_uaddr = (uint64_t)(unsigned long)hva;
> > > + input.guest_len = data_sz;
> > > +
> > > + trace_kvm_sev_launch_secret(gpa, input.guest_uaddr,
> > > + input.trans_uaddr, input.trans_len);
> > > +
> > > + ret = sev_ioctl(sev_guest->sev_fd, KVM_SEV_LAUNCH_SECRET,
> > > + &input, &error);
> > > + if (ret) {
> > > + error_report("SEV: failed to inject secret ret=%d
> > > fw_error=%d '%s'",
> > > + ret, error, fw_error_to_str(error));
> > > + goto err;
> > > + }
> > > +
> > > + ret = 0;
> > > +
> > > +err:
> > > + g_free(data);
> > > + g_free(hdr);
> > > + return ret;
> > > +}
> > > +
> > > static void
> > > sev_register_types(void)
> > > {
> > > diff --git a/target/i386/trace-events b/target/i386/trace-events
> > > index 789c700d4a..9f299e94a2 100644
> > > --- a/target/i386/trace-events
> > > +++ b/target/i386/trace-events
> > > @@ -15,3 +15,4 @@ kvm_sev_launch_start(int policy, void *session,
> > > void *pdh) "policy 0x%x session
> > > kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len
> > > 0x%" PRIu64
> > > 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"
> > > --
> > > 2.20.1 (Apple Git-117)
> > >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret
2020-10-12 15:57 ` Dr. David Alan Gilbert
@ 2020-10-12 16:00 ` James Bottomley
2020-10-12 16:38 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 11+ messages in thread
From: James Bottomley @ 2020-10-12 16:00 UTC (permalink / raw)
To: Dr. David Alan Gilbert, Tobin Feldman-Fitzthum
Cc: thomas.lendacky, brijesh.singh, tobin, qemu-devel, pbonzini
On Mon, 2020-10-12 at 16:57 +0100, Dr. David Alan Gilbert wrote:
> * Tobin Feldman-Fitzthum (tobin@linux.ibm.com) wrote:
> > On 2020-09-21 15:16, Dr. David Alan Gilbert wrote:
> > > * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
> > > > AMD SEV allows a guest owner to inject a secret blob
> > > > into the memory of a virtual machine. The secret is
> > > > encrypted with the SEV Transport Encryption Key and
> > > > integrity is guaranteed with the Transport Integrity
> > > > Key. Although QEMU faciliates the injection of the
> > > > launch secret, it cannot access the secret.
> > > >
> > > > Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com
> > > > >
> > >
> > > Hi Tobin,
> > > Did the ovmf stuff for agreeing the GUID for automating this
> > > ever happen?
> > >
> > OVMF patches have not been upstreamed yet. I think we are planning
> > to do that relatively soon.
>
> So as we're getting to the end of another qemu dev cycle; do we aim
> to get this one in by itself, or to wait for the GUID?
Since they're independent of each other, I'd say get this one in now if
it's acceptable. The GUID will come as a discoverable way of setting
the GPA, but this patch at least gives people a way to play with SEV
secret injection. I'm also reworking the OVMF GUID patch to tack on to
the reset vector GUID that just went upstream, so it will be a few more
weeks yet before we have it all integrated with the -ES patch set.
James
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret
2020-07-06 21:54 [PATCH v3] SEV: QMP support for Inject-Launch-Secret Tobin Feldman-Fitzthum
2020-07-06 22:00 ` tobin
2020-09-21 19:16 ` Dr. David Alan Gilbert
@ 2020-10-12 16:21 ` Dr. David Alan Gilbert
2020-10-12 16:49 ` Daniel P. Berrangé
2 siblings, 1 reply; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2020-10-12 16:21 UTC (permalink / raw)
To: Tobin Feldman-Fitzthum
Cc: thomas.lendacky, brijesh.singh, jejb, tobin, qemu-devel, pbonzini
* Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
> AMD SEV allows a guest owner to inject a secret blob
> into the memory of a virtual machine. The secret is
> encrypted with the SEV Transport Encryption Key and
> integrity is guaranteed with the Transport Integrity
> Key. Although QEMU faciliates the injection of the
> launch secret, it cannot access the secret.
>
> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
> ---
> include/monitor/monitor.h | 3 ++
> include/sysemu/sev.h | 2 ++
> monitor/misc.c | 8 ++---
> qapi/misc-target.json | 18 +++++++++++
> target/i386/monitor.c | 9 ++++++
> target/i386/sev-stub.c | 5 +++
> target/i386/sev.c | 66 +++++++++++++++++++++++++++++++++++++++
> target/i386/trace-events | 1 +
> 8 files changed, 108 insertions(+), 4 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 1018d754a6..bf049c5b00 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -4,6 +4,7 @@
> #include "block/block.h"
> #include "qapi/qapi-types-misc.h"
> #include "qemu/readline.h"
> +#include "include/exec/hwaddr.h"
>
> extern __thread Monitor *cur_mon;
> typedef struct MonitorHMP MonitorHMP;
> @@ -36,6 +37,8 @@ void monitor_flush(Monitor *mon);
> int monitor_set_cpu(int cpu_index);
> int monitor_get_cpu_index(void);
>
> +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp);
> +
> void monitor_read_command(MonitorHMP *mon, int show_prompt);
> int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
> void *opaque);
> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
> index 98c1ec8d38..b279b293e8 100644
> --- a/include/sysemu/sev.h
> +++ b/include/sysemu/sev.h
> @@ -18,4 +18,6 @@
>
> void *sev_guest_init(const char *id);
> int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
> +int sev_inject_launch_secret(const char *hdr, const char *secret,
> + uint64_t gpa);
> #endif
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 89bb970b00..b9ec8ba410 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -674,10 +674,10 @@ static void hmp_physical_memory_dump(Monitor *mon, const QDict *qdict)
> memory_dump(mon, count, format, size, addr, 1);
> }
>
> -static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, Error **errp)
> +void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, uint64_t size, Error **errp)
> {
> MemoryRegionSection mrs = memory_region_find(get_system_memory(),
> - addr, 1);
> + addr, size);
>
> if (!mrs.mr) {
> error_setg(errp, "No memory is mapped at address 0x%" HWADDR_PRIx, addr);
> @@ -701,7 +701,7 @@ static void hmp_gpa2hva(Monitor *mon, const QDict *qdict)
> MemoryRegion *mr = NULL;
> void *ptr;
>
> - ptr = gpa2hva(&mr, addr, &local_err);
> + ptr = gpa2hva(&mr, addr, 1, &local_err);
> if (local_err) {
> error_report_err(local_err);
> return;
> @@ -777,7 +777,7 @@ static void hmp_gpa2hpa(Monitor *mon, const QDict *qdict)
> void *ptr;
> uint64_t physaddr;
>
> - ptr = gpa2hva(&mr, addr, &local_err);
> + ptr = gpa2hva(&mr, addr, 1, &local_err);
> if (local_err) {
> error_report_err(local_err);
> return;
> diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> index dee3b45930..d145f916b3 100644
> --- a/qapi/misc-target.json
> +++ b/qapi/misc-target.json
> @@ -200,6 +200,24 @@
> { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
> 'if': 'defined(TARGET_I386)' }
>
> +##
> +# @sev-inject-launch-secret:
> +#
> +# This command injects a secret blob into memory of SEV guest.
> +#
> +# @packet-header: the launch secret packet header encoded in base64
> +#
> +# @secret: the launch secret data to be injected encoded in base64
> +#
> +# @gpa: the guest physical address where secret will be injected.
> +#
> +# Since: 5.1
> +#
> +##
> +{ 'command': 'sev-inject-launch-secret',
> + 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' },
> + 'if': 'defined(TARGET_I386)' }
> +
> ##
> # @dump-skeys:
> #
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index 27ebfa3ad2..42bcfe6dc0 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
>
> return data;
> }
> +
> +void qmp_sev_inject_launch_secret(const char *packet_hdr,
> + const char *secret, uint64_t gpa,
> + Error **errp)
> +{
> + if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
> + error_setg(errp, "SEV inject secret failed");
> + }
> +}
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> index e5ee13309c..fed4588185 100644
> --- a/target/i386/sev-stub.c
> +++ b/target/i386/sev-stub.c
> @@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
> {
> return NULL;
> }
> +int sev_inject_launch_secret(const char *hdr, const char *secret,
> + uint64_t gpa)
> +{
> + return 1;
> +}
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index d273174ad3..cbeb8f2e02 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -28,6 +28,8 @@
> #include "sysemu/runstate.h"
> #include "trace.h"
> #include "migration/blocker.h"
> +#include "exec/address-spaces.h"
> +#include "monitor/monitor.h"
>
> #define TYPE_SEV_GUEST "sev-guest"
> #define SEV_GUEST(obj) \
> @@ -769,6 +771,70 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
> return 0;
> }
>
> +int sev_inject_launch_secret(const char *packet_hdr,
> + const char *secret, uint64_t gpa)
> +{
> + struct kvm_sev_launch_secret input;
> + guchar *data = NULL, *hdr = NULL;
> + int error, ret = 1;
> + void *hva;
> + gsize hdr_sz = 0, data_sz = 0;
> + Error *local_err = NULL;
> + MemoryRegion *mr = NULL;
> +
> + /* secret can be inject only in this state */
> + if (!sev_check_state(sev_guest, SEV_STATE_LAUNCH_SECRET)) {
> + error_report("SEV: Not in correct state. (LSECRET) %x",
> + sev_guest->state);
> + return 1;
> + }
> +
> + hdr = g_base64_decode(packet_hdr, &hdr_sz);
> + if (!hdr || !hdr_sz) {
> + error_report("SEV: Failed to decode sequence header");
> + return 1;
> + }
> +
> + data = g_base64_decode(secret, &data_sz);
> + if (!data || !data_sz) {
> + error_report("SEV: Failed to decode data");
> + goto err;
> + }
> +
> + hva = gpa2hva(&mr, gpa, data_sz, &local_err);
> + if (!hva) {
> + error_report("SEV: Failed to calculate guest address.");
Note this is leaking local_err; you need to turn that into probably an
error_reportf_err(local_err, "SEV: Failed to calculate guest address:");
Also the '5.1' above needs to change to 5.2.
I think with that it looks OK to me.
Dave
> + goto err;
> + }
> +
> + input.hdr_uaddr = (uint64_t)(unsigned long)hdr;
> + input.hdr_len = hdr_sz;
> +
> + input.trans_uaddr = (uint64_t)(unsigned long)data;
> + input.trans_len = data_sz;
> +
> + input.guest_uaddr = (uint64_t)(unsigned long)hva;
> + input.guest_len = data_sz;
> +
> + trace_kvm_sev_launch_secret(gpa, input.guest_uaddr,
> + input.trans_uaddr, input.trans_len);
> +
> + ret = sev_ioctl(sev_guest->sev_fd, KVM_SEV_LAUNCH_SECRET,
> + &input, &error);
> + if (ret) {
> + error_report("SEV: failed to inject secret ret=%d fw_error=%d '%s'",
> + ret, error, fw_error_to_str(error));
> + goto err;
> + }
> +
> + ret = 0;
> +
> +err:
> + g_free(data);
> + g_free(hdr);
> + return ret;
> +}
> +
> static void
> sev_register_types(void)
> {
> diff --git a/target/i386/trace-events b/target/i386/trace-events
> index 789c700d4a..9f299e94a2 100644
> --- a/target/i386/trace-events
> +++ b/target/i386/trace-events
> @@ -15,3 +15,4 @@ kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session
> kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIu64
> 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"
> --
> 2.20.1 (Apple Git-117)
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret
2020-10-12 16:00 ` James Bottomley
@ 2020-10-12 16:38 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2020-10-12 16:38 UTC (permalink / raw)
To: James Bottomley, pbonzini
Cc: thomas.lendacky, brijesh.singh, tobin, qemu-devel,
Tobin Feldman-Fitzthum
* James Bottomley (jejb@linux.ibm.com) wrote:
> On Mon, 2020-10-12 at 16:57 +0100, Dr. David Alan Gilbert wrote:
> > * Tobin Feldman-Fitzthum (tobin@linux.ibm.com) wrote:
> > > On 2020-09-21 15:16, Dr. David Alan Gilbert wrote:
> > > > * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
> > > > > AMD SEV allows a guest owner to inject a secret blob
> > > > > into the memory of a virtual machine. The secret is
> > > > > encrypted with the SEV Transport Encryption Key and
> > > > > integrity is guaranteed with the Transport Integrity
> > > > > Key. Although QEMU faciliates the injection of the
> > > > > launch secret, it cannot access the secret.
> > > > >
> > > > > Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com
> > > > > >
> > > >
> > > > Hi Tobin,
> > > > Did the ovmf stuff for agreeing the GUID for automating this
> > > > ever happen?
> > > >
> > > OVMF patches have not been upstreamed yet. I think we are planning
> > > to do that relatively soon.
> >
> > So as we're getting to the end of another qemu dev cycle; do we aim
> > to get this one in by itself, or to wait for the GUID?
>
> Since they're independent of each other, I'd say get this one in now if
> it's acceptable. The GUID will come as a discoverable way of setting
> the GPA, but this patch at least gives people a way to play with SEV
> secret injection. I'm also reworking the OVMF GUID patch to tack on to
> the reset vector GUID that just went upstream, so it will be a few more
> weeks yet before we have it all integrated with the -ES patch set.
OK, so I've just replied with a minor error leak that needs fixing, but
other than that it looks OK to me.
I've not quite figured out who would pull it, Paolo?
Dave
> James
>
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret
2020-10-12 16:21 ` Dr. David Alan Gilbert
@ 2020-10-12 16:49 ` Daniel P. Berrangé
2020-10-13 21:56 ` tobin
0 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2020-10-12 16:49 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: thomas.lendacky, brijesh.singh, jejb, tobin, qemu-devel,
Tobin Feldman-Fitzthum, pbonzini
On Mon, Oct 12, 2020 at 05:21:15PM +0100, Dr. David Alan Gilbert wrote:
> * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
> > AMD SEV allows a guest owner to inject a secret blob
> > into the memory of a virtual machine. The secret is
> > encrypted with the SEV Transport Encryption Key and
> > integrity is guaranteed with the Transport Integrity
> > Key. Although QEMU faciliates the injection of the
Trivial typo s/faciliates/facilitates/
> > launch secret, it cannot access the secret.
> >
> > Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
> > ---
> > include/monitor/monitor.h | 3 ++
> > include/sysemu/sev.h | 2 ++
> > monitor/misc.c | 8 ++---
> > qapi/misc-target.json | 18 +++++++++++
> > target/i386/monitor.c | 9 ++++++
> > target/i386/sev-stub.c | 5 +++
> > target/i386/sev.c | 66 +++++++++++++++++++++++++++++++++++++++
> > target/i386/trace-events | 1 +
> > 8 files changed, 108 insertions(+), 4 deletions(-)
> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
> > index dee3b45930..d145f916b3 100644
> > --- a/qapi/misc-target.json
> > +++ b/qapi/misc-target.json
> > @@ -200,6 +200,24 @@
> > { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
> > 'if': 'defined(TARGET_I386)' }
> >
> > +##
> > +# @sev-inject-launch-secret:
> > +#
> > +# This command injects a secret blob into memory of SEV guest.
> > +#
> > +# @packet-header: the launch secret packet header encoded in base64
> > +#
> > +# @secret: the launch secret data to be injected encoded in base64
Just to double check, this "secret" is /not/ in clear text, so there's
no way either QEMU nor the QMP client can access sensitive info, right ?
If 'secret' was clear text, then we would need to pass the data across
QMP in a different way.
> > +#
> > +# @gpa: the guest physical address where secret will be injected.
> > +#
> > +# Since: 5.1
s/5.1/5.2/
> > +#
> > +##
> > +{ 'command': 'sev-inject-launch-secret',
> > + 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' },
> > + 'if': 'defined(TARGET_I386)' }
> > +
> > ##
> > # @dump-skeys:
> > #
> > diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> > index 27ebfa3ad2..42bcfe6dc0 100644
> > --- a/target/i386/monitor.c
> > +++ b/target/i386/monitor.c
> > @@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
> >
> > return data;
> > }
> > +
> > +void qmp_sev_inject_launch_secret(const char *packet_hdr,
> > + const char *secret, uint64_t gpa,
> > + Error **errp)
> > +{
> > + if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
> > + error_setg(errp, "SEV inject secret failed");
This generic error message is useless - sev_inject_launch_secret() needs
to take the 'errp' parameter and report what actually failed.
> > + }
> > +}
> > diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> > index e5ee13309c..fed4588185 100644
> > --- a/target/i386/sev-stub.c
> > +++ b/target/i386/sev-stub.c
> > @@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
> > {
> > return NULL;
> > }
> > +int sev_inject_launch_secret(const char *hdr, const char *secret,
> > + uint64_t gpa)
> > +{
> > + return 1;
> > +}
> > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > index d273174ad3..cbeb8f2e02 100644
> > --- a/target/i386/sev.c
> > +++ b/target/i386/sev.c
> > @@ -28,6 +28,8 @@
> > #include "sysemu/runstate.h"
> > #include "trace.h"
> > #include "migration/blocker.h"
> > +#include "exec/address-spaces.h"
> > +#include "monitor/monitor.h"
> >
> > #define TYPE_SEV_GUEST "sev-guest"
> > #define SEV_GUEST(obj) \
> > @@ -769,6 +771,70 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
> > return 0;
> > }
> >
> > +int sev_inject_launch_secret(const char *packet_hdr,
> > + const char *secret, uint64_t gpa)
> > +{
> > + struct kvm_sev_launch_secret input;
> > + guchar *data = NULL, *hdr = NULL;
If you declare with 'g_autofree' you don't need the manual 'g_free'
calls later. This in turn means you can get rid of the "goto err"
jumps and instead directly return.
> > + int error, ret = 1;
> > + void *hva;
> > + gsize hdr_sz = 0, data_sz = 0;
> > + Error *local_err = NULL;
Declare with
g_autoptr(Error) local_err = NULL
to fix the leak David mentions
> > + MemoryRegion *mr = NULL;
> > +
> > + /* secret can be inject only in this state */
> > + if (!sev_check_state(sev_guest, SEV_STATE_LAUNCH_SECRET)) {
> > + error_report("SEV: Not in correct state. (LSECRET) %x",
> > + sev_guest->state);
> > + return 1;
> > + }
> > +
> > + hdr = g_base64_decode(packet_hdr, &hdr_sz);
> > + if (!hdr || !hdr_sz) {
> > + error_report("SEV: Failed to decode sequence header");
> > + return 1;
> > + }
> > +
> > + data = g_base64_decode(secret, &data_sz);
> > + if (!data || !data_sz) {
> > + error_report("SEV: Failed to decode data");
> > + goto err;
> > + }
> > +
> > + hva = gpa2hva(&mr, gpa, data_sz, &local_err);
> > + if (!hva) {
> > + error_report("SEV: Failed to calculate guest address.");
>
> Note this is leaking local_err; you need to turn that into probably an
> error_reportf_err(local_err, "SEV: Failed to calculate guest address:");
Actually this method needs to take an "Error **errp" parameter, so that the
error is propagated back to the QMP command handler, and from there
back to the client app.
> Also the '5.1' above needs to change to 5.2.
>
> I think with that it looks OK to me.
> > + goto err;
> > + }
> > +
> > + input.hdr_uaddr = (uint64_t)(unsigned long)hdr;
> > + input.hdr_len = hdr_sz;
> > +
> > + input.trans_uaddr = (uint64_t)(unsigned long)data;
> > + input.trans_len = data_sz;
> > +
> > + input.guest_uaddr = (uint64_t)(unsigned long)hva;
> > + input.guest_len = data_sz;
> > +
> > + trace_kvm_sev_launch_secret(gpa, input.guest_uaddr,
> > + input.trans_uaddr, input.trans_len);
> > +
> > + ret = sev_ioctl(sev_guest->sev_fd, KVM_SEV_LAUNCH_SECRET,
> > + &input, &error);
> > + if (ret) {
> > + error_report("SEV: failed to inject secret ret=%d fw_error=%d '%s'",
> > + ret, error, fw_error_to_str(error));
> > + goto err;
> > + }
> > +
> > + ret = 0;
> > +
> > +err:
> > + g_free(data);
> > + g_free(hdr);
> > + return ret;
> > +}
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret
2020-10-12 16:49 ` Daniel P. Berrangé
@ 2020-10-13 21:56 ` tobin
0 siblings, 0 replies; 11+ messages in thread
From: tobin @ 2020-10-13 21:56 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: thomas.lendacky, brijesh.singh, jejb, tobin,
Dr. David Alan Gilbert, qemu-devel, pbonzini
On 2020-10-12 12:49, Daniel P. Berrangé wrote:
> On Mon, Oct 12, 2020 at 05:21:15PM +0100, Dr. David Alan Gilbert wrote:
>> * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
>> > AMD SEV allows a guest owner to inject a secret blob
>> > into the memory of a virtual machine. The secret is
>> > encrypted with the SEV Transport Encryption Key and
>> > integrity is guaranteed with the Transport Integrity
>> > Key. Although QEMU faciliates the injection of the
>
> Trivial typo s/faciliates/facilitates/
>
>> > launch secret, it cannot access the secret.
>> >
>> > Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
>> > ---
>> > include/monitor/monitor.h | 3 ++
>> > include/sysemu/sev.h | 2 ++
>> > monitor/misc.c | 8 ++---
>> > qapi/misc-target.json | 18 +++++++++++
>> > target/i386/monitor.c | 9 ++++++
>> > target/i386/sev-stub.c | 5 +++
>> > target/i386/sev.c | 66 +++++++++++++++++++++++++++++++++++++++
>> > target/i386/trace-events | 1 +
>> > 8 files changed, 108 insertions(+), 4 deletions(-)
>
>
>> > diff --git a/qapi/misc-target.json b/qapi/misc-target.json
>> > index dee3b45930..d145f916b3 100644
>> > --- a/qapi/misc-target.json
>> > +++ b/qapi/misc-target.json
>> > @@ -200,6 +200,24 @@
>> > { 'command': 'query-sev-capabilities', 'returns': 'SevCapability',
>> > 'if': 'defined(TARGET_I386)' }
>> >
>> > +##
>> > +# @sev-inject-launch-secret:
>> > +#
>> > +# This command injects a secret blob into memory of SEV guest.
>> > +#
>> > +# @packet-header: the launch secret packet header encoded in base64
>> > +#
>> > +# @secret: the launch secret data to be injected encoded in base64
>
> Just to double check, this "secret" is /not/ in clear text, so there's
> no way either QEMU nor the QMP client can access sensitive info, right
> ?
>
You are correct. QEMU would only be able to read the secret in the case
of serious user error.
Thank you for your feedback. I have rebased made the changes. Will send
new version in the morning.
-Tobin
> If 'secret' was clear text, then we would need to pass the data across
> QMP in a different way.
>
>> > +#
>> > +# @gpa: the guest physical address where secret will be injected.
>> > +#
>> > +# Since: 5.1
>
> s/5.1/5.2/
>
>> > +#
>> > +##
>> > +{ 'command': 'sev-inject-launch-secret',
>> > + 'data': { 'packet-header': 'str', 'secret': 'str', 'gpa': 'uint64' },
>> > + 'if': 'defined(TARGET_I386)' }
>> > +
>> > ##
>> > # @dump-skeys:
>> > #
>> > diff --git a/target/i386/monitor.c b/target/i386/monitor.c
>> > index 27ebfa3ad2..42bcfe6dc0 100644
>> > --- a/target/i386/monitor.c
>> > +++ b/target/i386/monitor.c
>> > @@ -736,3 +736,12 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
>> >
>> > return data;
>> > }
>> > +
>> > +void qmp_sev_inject_launch_secret(const char *packet_hdr,
>> > + const char *secret, uint64_t gpa,
>> > + Error **errp)
>> > +{
>> > + if (sev_inject_launch_secret(packet_hdr, secret, gpa) != 0) {
>> > + error_setg(errp, "SEV inject secret failed");
>
> This generic error message is useless - sev_inject_launch_secret()
> needs
> to take the 'errp' parameter and report what actually failed.
>
>> > + }
>> > +}
>> > diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
>> > index e5ee13309c..fed4588185 100644
>> > --- a/target/i386/sev-stub.c
>> > +++ b/target/i386/sev-stub.c
>> > @@ -48,3 +48,8 @@ SevCapability *sev_get_capabilities(void)
>> > {
>> > return NULL;
>> > }
>> > +int sev_inject_launch_secret(const char *hdr, const char *secret,
>> > + uint64_t gpa)
>> > +{
>> > + return 1;
>> > +}
>> > diff --git a/target/i386/sev.c b/target/i386/sev.c
>> > index d273174ad3..cbeb8f2e02 100644
>> > --- a/target/i386/sev.c
>> > +++ b/target/i386/sev.c
>> > @@ -28,6 +28,8 @@
>> > #include "sysemu/runstate.h"
>> > #include "trace.h"
>> > #include "migration/blocker.h"
>> > +#include "exec/address-spaces.h"
>> > +#include "monitor/monitor.h"
>> >
>> > #define TYPE_SEV_GUEST "sev-guest"
>> > #define SEV_GUEST(obj) \
>> > @@ -769,6 +771,70 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
>> > return 0;
>> > }
>> >
>> > +int sev_inject_launch_secret(const char *packet_hdr,
>> > + const char *secret, uint64_t gpa)
>> > +{
>> > + struct kvm_sev_launch_secret input;
>> > + guchar *data = NULL, *hdr = NULL;
>
> If you declare with 'g_autofree' you don't need the manual 'g_free'
> calls later. This in turn means you can get rid of the "goto err"
> jumps and instead directly return.
>
>> > + int error, ret = 1;
>> > + void *hva;
>> > + gsize hdr_sz = 0, data_sz = 0;
>> > + Error *local_err = NULL;
>
> Declare with
>
> g_autoptr(Error) local_err = NULL
>
> to fix the leak David mentions
>
>
>> > + MemoryRegion *mr = NULL;
>> > +
>> > + /* secret can be inject only in this state */
>> > + if (!sev_check_state(sev_guest, SEV_STATE_LAUNCH_SECRET)) {
>> > + error_report("SEV: Not in correct state. (LSECRET) %x",
>> > + sev_guest->state);
>> > + return 1;
>> > + }
>> > +
>> > + hdr = g_base64_decode(packet_hdr, &hdr_sz);
>> > + if (!hdr || !hdr_sz) {
>> > + error_report("SEV: Failed to decode sequence header");
>> > + return 1;
>> > + }
>> > +
>> > + data = g_base64_decode(secret, &data_sz);
>> > + if (!data || !data_sz) {
>> > + error_report("SEV: Failed to decode data");
>> > + goto err;
>> > + }
>> > +
>> > + hva = gpa2hva(&mr, gpa, data_sz, &local_err);
>> > + if (!hva) {
>> > + error_report("SEV: Failed to calculate guest address.");
>>
>> Note this is leaking local_err; you need to turn that into probably an
>> error_reportf_err(local_err, "SEV: Failed to calculate guest
>> address:");
>
> Actually this method needs to take an "Error **errp" parameter, so that
> the
> error is propagated back to the QMP command handler, and from there
> back to the client app.
>
>> Also the '5.1' above needs to change to 5.2.
>>
>> I think with that it looks OK to me.
>
>> > + goto err;
>> > + }
>> > +
>> > + input.hdr_uaddr = (uint64_t)(unsigned long)hdr;
>> > + input.hdr_len = hdr_sz;
>> > +
>> > + input.trans_uaddr = (uint64_t)(unsigned long)data;
>> > + input.trans_len = data_sz;
>> > +
>> > + input.guest_uaddr = (uint64_t)(unsigned long)hva;
>> > + input.guest_len = data_sz;
>> > +
>> > + trace_kvm_sev_launch_secret(gpa, input.guest_uaddr,
>> > + input.trans_uaddr, input.trans_len);
>> > +
>> > + ret = sev_ioctl(sev_guest->sev_fd, KVM_SEV_LAUNCH_SECRET,
>> > + &input, &error);
>> > + if (ret) {
>> > + error_report("SEV: failed to inject secret ret=%d fw_error=%d '%s'",
>> > + ret, error, fw_error_to_str(error));
>> > + goto err;
>> > + }
>> > +
>> > + ret = 0;
>> > +
>> > +err:
>> > + g_free(data);
>> > + g_free(hdr);
>> > + return ret;
>> > +}
>
>
> Regards,
> Daniel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-10-13 21:57 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-06 21:54 [PATCH v3] SEV: QMP support for Inject-Launch-Secret Tobin Feldman-Fitzthum
2020-07-06 22:00 ` tobin
2020-09-21 19:16 ` Dr. David Alan Gilbert
2020-09-21 20:33 ` Tobin Feldman-Fitzthum
2020-09-21 22:14 ` Tom Lendacky
2020-10-12 15:57 ` Dr. David Alan Gilbert
2020-10-12 16:00 ` James Bottomley
2020-10-12 16:38 ` Dr. David Alan Gilbert
2020-10-12 16:21 ` Dr. David Alan Gilbert
2020-10-12 16:49 ` Daniel P. Berrangé
2020-10-13 21:56 ` tobin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.