* [PATCH v2] SEV: QMP support for Inject-Launch-Secret
@ 2020-07-02 19:42 Tobin Feldman-Fitzthum
2020-07-03 11:11 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 7+ messages in thread
From: Tobin Feldman-Fitzthum @ 2020-07-02 19:42 UTC (permalink / raw)
To: qemu-devel
Cc: thomas.lendacky, jejb, tobin, dgilbert, Tobin Feldman-Fitzthum,
pbonzini, brijesh.singh
From: Tobin Feldman-Fitzthum <tobin@ibm.com>
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/sev_i386.h | 3 ++
target/i386/trace-events | 1 +
9 files changed, 111 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..3b2b3c8d8b 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 != SEV_SECRET_HEADER_LEN) {
+ error_report("SEV: Failed to decode sequence header");
+ return 1;
+ }
+
+ data = g_base64_decode(secret, &data_sz);
+ if (!data || data_sz <= 0 || data_sz > SEV_SECRET_MAX_LEN) {
+ 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)hdr;
+ input.hdr_len = hdr_sz;
+
+ input.trans_uaddr = (uint64_t)data;
+ input.trans_len = data_sz;
+
+ input.guest_uaddr = (uint64_t)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/sev_i386.h b/target/i386/sev_i386.h
index 8eb7de1bef..b9ed89d48c 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -28,6 +28,9 @@
#define SEV_POLICY_DOMAIN 0x10
#define SEV_POLICY_SEV 0x20
+#define SEV_SECRET_HEADER_LEN 0x34
+#define SEV_SECRET_MAX_LEN 0x3E80
+
extern bool sev_enabled(void);
extern uint64_t sev_get_me_mask(void);
extern SevInfo *sev_get_info(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] 7+ messages in thread
* Re: [PATCH v2] SEV: QMP support for Inject-Launch-Secret
2020-07-02 19:42 [PATCH v2] SEV: QMP support for Inject-Launch-Secret Tobin Feldman-Fitzthum
@ 2020-07-03 11:11 ` Dr. David Alan Gilbert
2020-07-03 13:25 ` Brijesh Singh
2020-07-03 15:54 ` James Bottomley
0 siblings, 2 replies; 7+ messages in thread
From: Dr. David Alan Gilbert @ 2020-07-03 11:11 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:
> From: Tobin Feldman-Fitzthum <tobin@ibm.com>
>
> 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/sev_i386.h | 3 ++
> target/i386/trace-events | 1 +
> 9 files changed, 111 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..3b2b3c8d8b 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 != SEV_SECRET_HEADER_LEN) {
> + error_report("SEV: Failed to decode sequence header");
> + return 1;
> + }
> +
> + data = g_base64_decode(secret, &data_sz);
> + if (!data || data_sz <= 0 || data_sz > SEV_SECRET_MAX_LEN) {
> + 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)hdr;
> + input.hdr_len = hdr_sz;
> +
> + input.trans_uaddr = (uint64_t)data;
> + input.trans_len = data_sz;
> +
> + input.guest_uaddr = (uint64_t)hva;
Thanks for changing these; although it fails a 32bit build (which is
probably mostly pointless for SEV, but it fails the build rather than
building it out). The easy fix here seems to be:
(uint64_t)(uintptr_t)
> + 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/sev_i386.h b/target/i386/sev_i386.h
> index 8eb7de1bef..b9ed89d48c 100644
> --- a/target/i386/sev_i386.h
> +++ b/target/i386/sev_i386.h
> @@ -28,6 +28,9 @@
> #define SEV_POLICY_DOMAIN 0x10
> #define SEV_POLICY_SEV 0x20
>
> +#define SEV_SECRET_HEADER_LEN 0x34
> +#define SEV_SECRET_MAX_LEN 0x3E80
> +
Where does that maximum come from - I didn't find it in the spec.
Other than the 32bit ism above, and that minor question; I think this is
fine.
Dave
> extern bool sev_enabled(void);
> extern uint64_t sev_get_me_mask(void);
> extern SevInfo *sev_get_info(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] 7+ messages in thread
* Re: [PATCH v2] SEV: QMP support for Inject-Launch-Secret
2020-07-03 11:11 ` Dr. David Alan Gilbert
@ 2020-07-03 13:25 ` Brijesh Singh
2020-07-03 22:25 ` tobin
2020-07-03 15:54 ` James Bottomley
1 sibling, 1 reply; 7+ messages in thread
From: Brijesh Singh @ 2020-07-03 13:25 UTC (permalink / raw)
To: Dr. David Alan Gilbert, Tobin Feldman-Fitzthum
Cc: thomas.lendacky, brijesh.singh, jejb, tobin, qemu-devel, pbonzini
On 7/3/20 6:11 AM, Dr. David Alan Gilbert wrote:
> * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
>> From: Tobin Feldman-Fitzthum <tobin@ibm.com>
>>
>> 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/sev_i386.h | 3 ++
>> target/i386/trace-events | 1 +
>> 9 files changed, 111 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..3b2b3c8d8b 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 != SEV_SECRET_HEADER_LEN) {
>> + error_report("SEV: Failed to decode sequence header");
>> + return 1;
>> + }
>> +
>> + data = g_base64_decode(secret, &data_sz);
>> + if (!data || data_sz <= 0 || data_sz > SEV_SECRET_MAX_LEN) {
>> + 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)hdr;
>> + input.hdr_len = hdr_sz;
>> +
>> + input.trans_uaddr = (uint64_t)data;
>> + input.trans_len = data_sz;
>> +
>> + input.guest_uaddr = (uint64_t)hva;
> Thanks for changing these; although it fails a 32bit build (which is
> probably mostly pointless for SEV, but it fails the build rather than
> building it out). The easy fix here seems to be:
> (uint64_t)(uintptr_t)
>
>> + 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/sev_i386.h b/target/i386/sev_i386.h
>> index 8eb7de1bef..b9ed89d48c 100644
>> --- a/target/i386/sev_i386.h
>> +++ b/target/i386/sev_i386.h
>> @@ -28,6 +28,9 @@
>> #define SEV_POLICY_DOMAIN 0x10
>> #define SEV_POLICY_SEV 0x20
>>
>> +#define SEV_SECRET_HEADER_LEN 0x34
>> +#define SEV_SECRET_MAX_LEN 0x3E80
>> +
> Where does that maximum come from - I didn't find it in the spec.
The size of packet header is documented in Table 55 [1]. But, I am not
sure how Tobin calculated the size of the secret max length. FW spec
says that if it does not support the passed secret length then it will
error out with INVALID_LENGTH.
I would avoid doing the header and secret length check in the Qemu.
From HV point of view we consider them as a opaque blob and pass them
as-is to the FW. We have seen that the size of the blob changed from one
version to another.
[1]
https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf
-Brijesh
> Other than the 32bit ism above, and that minor question; I think this is
> fine.
>
> Dave
>
>> extern bool sev_enabled(void);
>> extern uint64_t sev_get_me_mask(void);
>> extern SevInfo *sev_get_info(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] 7+ messages in thread
* Re: [PATCH v2] SEV: QMP support for Inject-Launch-Secret
2020-07-03 11:11 ` Dr. David Alan Gilbert
2020-07-03 13:25 ` Brijesh Singh
@ 2020-07-03 15:54 ` James Bottomley
2020-07-03 16:02 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 7+ messages in thread
From: James Bottomley @ 2020-07-03 15:54 UTC (permalink / raw)
To: Dr. David Alan Gilbert, Tobin Feldman-Fitzthum
Cc: thomas.lendacky, brijesh.singh, tobin, qemu-devel, pbonzini
On Fri, 2020-07-03 at 12:11 +0100, Dr. David Alan Gilbert wrote:
> * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
[...]
> > + input.trans_uaddr = (uint64_t)data;
> > + input.trans_len = data_sz;
> > +
> > + input.guest_uaddr = (uint64_t)hva;
>
> Thanks for changing these; although it fails a 32bit build (which is
> probably mostly pointless for SEV, but it fails the build rather than
> building it out). The easy fix here seems to be:
> (uint64_t)(uintptr_t)
That's a pointer width issue. The recommended way to communicate to
the compiler that we really want to cast a 32 bit pointer to a 64 bit
value is actually to cast to unsigned long before casting to pointer,
so
(uint64_t)(unsigned long)hva
Many other things work, of course, but if you follow the recommendation
you (hopefully) don't trip future compiler warnings.
James
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] SEV: QMP support for Inject-Launch-Secret
2020-07-03 15:54 ` James Bottomley
@ 2020-07-03 16:02 ` Dr. David Alan Gilbert
2020-07-03 16:33 ` James Bottomley
0 siblings, 1 reply; 7+ messages in thread
From: Dr. David Alan Gilbert @ 2020-07-03 16:02 UTC (permalink / raw)
To: James Bottomley
Cc: thomas.lendacky, brijesh.singh, tobin, qemu-devel,
Tobin Feldman-Fitzthum, pbonzini
* James Bottomley (jejb@linux.ibm.com) wrote:
> On Fri, 2020-07-03 at 12:11 +0100, Dr. David Alan Gilbert wrote:
> > * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
> [...]
> > > + input.trans_uaddr = (uint64_t)data;
> > > + input.trans_len = data_sz;
> > > +
> > > + input.guest_uaddr = (uint64_t)hva;
> >
> > Thanks for changing these; although it fails a 32bit build (which is
> > probably mostly pointless for SEV, but it fails the build rather than
> > building it out). The easy fix here seems to be:
> > (uint64_t)(uintptr_t)
>
> That's a pointer width issue. The recommended way to communicate to
> the compiler that we really want to cast a 32 bit pointer to a 64 bit
> value is actually to cast to unsigned long before casting to pointer,
> so
>
> (uint64_t)(unsigned long)hva
>
> Many other things work, of course, but if you follow the recommendation
> you (hopefully) don't trip future compiler warnings.
OK, fair enough
(Out of curiosity can you explain why unsigned long not uintptr_t?)
Dave
> James
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] SEV: QMP support for Inject-Launch-Secret
2020-07-03 16:02 ` Dr. David Alan Gilbert
@ 2020-07-03 16:33 ` James Bottomley
0 siblings, 0 replies; 7+ messages in thread
From: James Bottomley @ 2020-07-03 16:33 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: thomas.lendacky, brijesh.singh, tobin, qemu-devel,
Tobin Feldman-Fitzthum, pbonzini
On Fri, 2020-07-03 at 17:02 +0100, Dr. David Alan Gilbert wrote:
> * James Bottomley (jejb@linux.ibm.com) wrote:
> > On Fri, 2020-07-03 at 12:11 +0100, Dr. David Alan Gilbert wrote:
> > > * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
> >
> > [...]
> > > > + input.trans_uaddr = (uint64_t)data;
> > > > + input.trans_len = data_sz;
> > > > +
> > > > + input.guest_uaddr = (uint64_t)hva;
> > >
> > > Thanks for changing these; although it fails a 32bit build (which
> > > is probably mostly pointless for SEV, but it fails the build
> > > rather than building it out). The easy fix here seems to be:
> > > (uint64_t)(uintptr_t)
> >
> > That's a pointer width issue. The recommended way to communicate
> > to the compiler that we really want to cast a 32 bit pointer to a
> > 64 bit value is actually to cast to unsigned long before casting to
> > pointer, so
> >
> > (uint64_t)(unsigned long)hva
> >
> > Many other things work, of course, but if you follow the
> > recommendation you (hopefully) don't trip future compiler warnings.
>
> OK, fair enough
> (Out of curiosity can you explain why unsigned long not uintptr_t?)
You have to cast it to a matching variable size integer first, so both
unsigned long and uintptr_t work because they're 64 bits on LP64 and 32
bits on ILP32. The only minor problem with uintptr_t is that the
checker may or may not be programmed to know what it does and there's
nothing more annoying than trying to get a warning message you know to
be wrong fixed.
James
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] SEV: QMP support for Inject-Launch-Secret
2020-07-03 13:25 ` Brijesh Singh
@ 2020-07-03 22:25 ` tobin
0 siblings, 0 replies; 7+ messages in thread
From: tobin @ 2020-07-03 22:25 UTC (permalink / raw)
To: Brijesh Singh
Cc: thomas.lendacky, jejb, tobin, Dr. David Alan Gilbert, qemu-devel,
pbonzini
On 2020-07-03 09:25, Brijesh Singh wrote:
> On 7/3/20 6:11 AM, Dr. David Alan Gilbert wrote:
>> * Tobin Feldman-Fitzthum (tobin@linux.vnet.ibm.com) wrote:
>>> From: Tobin Feldman-Fitzthum <tobin@ibm.com>
>>>
>>> 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/sev_i386.h | 3 ++
>>> target/i386/trace-events | 1 +
>>> 9 files changed, 111 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..3b2b3c8d8b 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 != SEV_SECRET_HEADER_LEN) {
>>> + error_report("SEV: Failed to decode sequence header");
>>> + return 1;
>>> + }
>>> +
>>> + data = g_base64_decode(secret, &data_sz);
>>> + if (!data || data_sz <= 0 || data_sz > SEV_SECRET_MAX_LEN) {
>>> + 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)hdr;
>>> + input.hdr_len = hdr_sz;
>>> +
>>> + input.trans_uaddr = (uint64_t)data;
>>> + input.trans_len = data_sz;
>>> +
>>> + input.guest_uaddr = (uint64_t)hva;
>> Thanks for changing these; although it fails a 32bit build (which is
>> probably mostly pointless for SEV, but it fails the build rather than
>> building it out). The easy fix here seems to be:
>> (uint64_t)(uintptr_t)
>>
>>> + 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/sev_i386.h b/target/i386/sev_i386.h
>>> index 8eb7de1bef..b9ed89d48c 100644
>>> --- a/target/i386/sev_i386.h
>>> +++ b/target/i386/sev_i386.h
>>> @@ -28,6 +28,9 @@
>>> #define SEV_POLICY_DOMAIN 0x10
>>> #define SEV_POLICY_SEV 0x20
>>>
>>> +#define SEV_SECRET_HEADER_LEN 0x34
>>> +#define SEV_SECRET_MAX_LEN 0x3E80
>>> +
>> Where does that maximum come from - I didn't find it in the spec.
>
>
> The size of packet header is documented in Table 55 [1]. But, I am not
> sure how Tobin calculated the size of the secret max length. FW spec
> says that if it does not support the passed secret length then it will
> error out with INVALID_LENGTH.
>
> I would avoid doing the header and secret length check in the Qemu.
> From HV point of view we consider them as a opaque blob and pass them
> as-is to the FW. We have seen that the size of the blob changed from
> one
> version to another.
>
> [1]
> https://www.amd.com/system/files/TechDocs/55766_SEV-KM_API_Specification.pdf
>
>
> -Brijesh
>
Thanks, Brijesh. I was hoping you could confirm some of those values.
I got the max secret length from table 54 which has a max value for the
"GUEST_LENGTH". I am not entirely sure if I am interpreting the table
correctly and I agree with you that checking the length is probably
best left to the PSP.
-Tobin
>> Other than the 32bit ism above, and that minor question; I think this
>> is
>> fine.
>>
>> Dave
>>
>>> extern bool sev_enabled(void);
>>> extern uint64_t sev_get_me_mask(void);
>>> extern SevInfo *sev_get_info(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] 7+ messages in thread
end of thread, other threads:[~2020-07-03 22:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-02 19:42 [PATCH v2] SEV: QMP support for Inject-Launch-Secret Tobin Feldman-Fitzthum
2020-07-03 11:11 ` Dr. David Alan Gilbert
2020-07-03 13:25 ` Brijesh Singh
2020-07-03 22:25 ` tobin
2020-07-03 15:54 ` James Bottomley
2020-07-03 16:02 ` Dr. David Alan Gilbert
2020-07-03 16:33 ` James Bottomley
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.