Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret

[tobin]

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 (to...@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 
> ---
>  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, re

[PATCH v4] sev: add sev-inject-launch-secret

[tobin]

From: Tobin Feldman-Fitzthum 

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 facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c|  8 +++---
 qapi/misc-target.json | 18 
 target/i386/monitor.c |  7 +
 target/i386/sev-stub.c|  5 
 target/i386/sev.c | 60 +++
 target/i386/trace-events  |  1 +
 8 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..af3887bb71 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"
 
 typedef struct MonitorHMP MonitorHMP;
 typedef struct MonitorOptions MonitorOptions;
@@ -37,6 +38,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(Monitor *mon, int cpu_index);
 int monitor_get_cpu_index(Monitor *mon);
 
+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..7ab6e3e31d 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, Error **errp);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 4a859fb24a..f1ade245d5 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -667,10 +667,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);
@@ -694,7 +694,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;
@@ -770,7 +770,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 1e561fa97b..4486a543ae 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -201,6 +201,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.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 7abae3c8df..f9d4951465 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -728,3 +728,10 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
 {
 return sev_get_capabilities(errp);
 }
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+  const char *secret, uint64_t gpa,
+  Error **errp)
+{
+sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
+}
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 88e3f39a1e..2d2ee54cc6 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@

[PATCH v5] sev: add sev-inject-launch-secret

[tobin]

From: Tobin Feldman-Fitzthum 

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 facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
Reviewed-by: Daniel P. Berrangé 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c|  8 ++---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  7 +
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 65 +++
 target/i386/trace-events  |  1 +
 8 files changed, 105 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..af3887bb71 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"
 
 typedef struct MonitorHMP MonitorHMP;
 typedef struct MonitorOptions MonitorOptions;
@@ -37,6 +38,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(Monitor *mon, int cpu_index);
 int monitor_get_cpu_index(Monitor *mon);
 
+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..7ab6e3e31d 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, Error **errp);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 4a859fb24a..f1ade245d5 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -667,10 +667,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);
@@ -694,7 +694,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;
@@ -770,7 +770,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 1e561fa97b..4486a543ae 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -201,6 +201,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.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 7abae3c8df..f9d4951465 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -728,3 +728,10 @@ SevCapability *qmp_query_sev_capabilities(Error **errp)
 {
 return sev_get_capabilities(errp);
 }
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+  const char *secret, uint64_t gpa,
+  Error **errp)
+{
+sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
+}
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 88e3f39a1e..2d2ee54cc6 100644
--- a/target/i386/se

[PATCH v6] sev: add sev-inject-launch-secret

[tobin]

From: Tobin Feldman-Fitzthum 

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 facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Brijesh Singh 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c| 13 +---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  7 +
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 65 +++
 target/i386/trace-events  |  1 +
 8 files changed, 110 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..af3887bb71 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"
 
 typedef struct MonitorHMP MonitorHMP;
 typedef struct MonitorOptions MonitorOptions;
@@ -37,6 +38,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(Monitor *mon, int cpu_index);
 int monitor_get_cpu_index(Monitor *mon);
 
+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..7ab6e3e31d 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, Error **errp);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 4a859fb24a..dd148be5da 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -667,10 +667,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);
@@ -683,6 +683,11 @@ static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, 
Error **errp)
 return NULL;
 }
 
+if (mrs.size < size) {
+error_setg(errp, "Size of memory region at 0x%" HWADDR_PRIx
+   " exceeded.", addr);
+}
+
 *p_mr = mrs.mr;
 return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
 }
@@ -694,7 +699,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;
@@ -770,7 +775,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 1e561fa97b..4486a543ae 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -201,6 +201,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.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 7abae3c8df..f9d4951465 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -728,3 +728,10 @@ SevCapability *qmp_query_sev_capabilities(Error **errp

[PATCH v7] sev: add sev-inject-launch-secret

[tobin]

From: Tobin Feldman-Fitzthum 

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 facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Brijesh Singh 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c| 15 ++---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  7 +
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 65 +++
 target/i386/trace-events  |  1 +
 8 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..af3887bb71 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"
 
 typedef struct MonitorHMP MonitorHMP;
 typedef struct MonitorOptions MonitorOptions;
@@ -37,6 +38,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(Monitor *mon, int cpu_index);
 int monitor_get_cpu_index(Monitor *mon);
 
+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..7ab6e3e31d 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, Error **errp);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 4a859fb24a..b6b2c1c60f 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -667,10 +667,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);
@@ -683,6 +683,13 @@ static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, 
Error **errp)
 return NULL;
 }
 
+if (mrs.size < size) {
+error_setg(errp, "Size of memory region at 0x%" HWADDR_PRIx
+   " exceeded.", addr);
+memory_region_unref(mrs.mr);
+return NULL;
+}
+
 *p_mr = mrs.mr;
 return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
 }
@@ -694,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;
@@ -770,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 1e561fa97b..4486a543ae 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -201,6 +201,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.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 7abae3c8df..f9d4951465 100644
--- a/target/i386/monitor.c
+

[PATCH v8] sev: add sev-inject-launch-secret

[tobin]

From: Tobin Feldman-Fitzthum 

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 facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Brijesh Singh 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c| 17 +++---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  7 +
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 65 +++
 target/i386/trace-events  |  1 +
 8 files changed, 114 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..af3887bb71 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"
 
 typedef struct MonitorHMP MonitorHMP;
 typedef struct MonitorOptions MonitorOptions;
@@ -37,6 +38,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(Monitor *mon, int cpu_index);
 int monitor_get_cpu_index(Monitor *mon);
 
+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..7ab6e3e31d 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, Error **errp);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 4a859fb24a..53ff4e4c63 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -667,10 +667,11 @@ 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)
 {
+Int128 gpa_region_size;
 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);
@@ -683,6 +684,14 @@ static void *gpa2hva(MemoryRegion **p_mr, hwaddr addr, 
Error **errp)
 return NULL;
 }
 
+gpa_region_size = int128_make64(size);
+if (int128_lt(mrs.size, gpa_region_size)) {
+error_setg(errp, "Size of memory region at 0x%" HWADDR_PRIx
+   " exceeded.", addr);
+memory_region_unref(mrs.mr);
+return NULL;
+}
+
 *p_mr = mrs.mr;
 return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
 }
@@ -694,7 +703,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;
@@ -770,7 +779,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 1e561fa97b..4486a543ae 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -201,6 +201,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.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

Re: [PATCH 1/2] sev: add sev-inject-launch-secret

[tobin]

On 2020-05-28 17:42, Eric Blake wrote:

On 5/28/20 3:51 PM, Tobin Feldman-Fitzthum wrote:

From: Tobin Feldman-Fitzthum 

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



+++ b/qapi/misc-target.json
@@ -200,6 +200,26 @@
  { '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.
+GPA provided here will be ignored if guest ROM specifies
+the a launch secret GPA.


Missing # on the wrapped lines.


+#
+# Since: 5.0.0


You've missed 5.0, and more sites tend to use x.y instead of x.y.z
(although we aren't consistent); this should be 'Since: 5.1'


+#
+##
+{ 'command': 'sev-inject-launch-secret',
+  'data': { 'packet_hdr': 'str', 'secret': 'str', 'gpa': 'uint64' },


This does not match your documentation above, which named it
'packet-header'.  Should 'gpa' be optional, to account for the case
where ROM specifies it?


My bad on the syntax issues. I think making GPA optional makes sense.
In the first patch we can have it be required and in the second
we add the option to scan the ROM.

Re: [PATCH 1/2] sev: add sev-inject-launch-secret

[tobin]

On 2020-05-28 17:00, James Bottomley wrote:

On Thu, 2020-05-28 at 16:51 -0400, Tobin Feldman-Fitzthum wrote:

--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -200,6 +200,26 @@
 { '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.
+GPA provided here will be ignored if guest ROM specifies
+the a launch secret GPA.


Shouldn't we eliminate the gpa argument to this now the gpa is
extracted from OVMF?  You add it here but don't take it out in the next
patch.


I think having GPA as an optional argument might make the most sense.
Users may or may not know how to use the argument, but it is probably
a good idea to give another option besides sticking the GPA into the 
ROM.



+# Since: 5.0.0
+#
+##
+{ 'command': 'sev-inject-launch-secret',
+  'data': { 'packet_hdr': 'str', 'secret': 'str', 'gpa': 'uint64' },


Java (i.e. Json) people hate underscores and abbreviations.  I bet
they'll want this to be 'packet-header'


Happy to change this.


+  'if': 'defined(TARGET_I386)' }
+
 ##
 # @dump-skeys:
 #
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 27ebfa3ad2..5c2b7d2c17 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -736,3 +736,11 @@ 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..2b8c5f1f53 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 846018a12d..774e47d9d1 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -28,6 +28,7 @@
 #include "sysemu/runstate.h"
 #include "trace.h"
 #include "migration/blocker.h"
+#include "exec/address-spaces.h"

 #define DEFAULT_GUEST_POLICY0x1 /* disable debug */
 #define DEFAULT_SEV_DEVICE  "/dev/sev"
@@ -743,6 +744,88 @@ sev_encrypt_data(void *handle, uint8_t *ptr,
uint64_t len)
 return 0;
 }

+
+static void *
+gpa2hva(hwaddr addr, uint64_t size)
+{
+MemoryRegionSection mrs =
memory_region_find(get_system_memory(),
+ addr, size);
+
+if (!mrs.mr) {
+error_report("No memory is mapped at address 0x%"
HWADDR_PRIx, addr);
+return NULL;
+}
+
+if (!memory_region_is_ram(mrs.mr) &&
!memory_region_is_romd(mrs.mr)) {
+error_report("Memory at address 0x%" HWADDR_PRIx "is not
RAM", addr);
+memory_region_unref(mrs.mr);
+return NULL;
+}


We can still check this, but it should be like an assertion failure.
Since the GPA is selected by the OVMF build there should be no way it
can't be mapped into the host.

[...]

--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -93,10 +93,10 @@ static bool query_is_blacklisted(const char *cmd)
 /* Success depends on target-specific build configuration:
*/
 "query-pci",  /* CONFIG_PCI */
 /* Success depends on launching SEV guest */
-"query-sev-launch-measure",
+// "query-sev-launch-measure",
 /* Success depends on Host or Hypervisor SEV support */
-"query-sev",
-"query-sev-capabilities",
+// "query-sev",
+// "query-sev-capabilities",


We're eliminating existing tests ... is that just a stray hunk that you
forgot to remove?


Yes.

James

Re: [PATCH 1/1] SEV: QMP support for Inject-Launch-Secret

[tobin]

On 2020-07-02 11:53, Dr. David Alan Gilbert wrote:

* Tobin Feldman-Fitzthum (to...@linux.vnet.ibm.com) wrote:

From: Tobin Feldman-Fitzthum 

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

Re: [PATCH v2] SEV: QMP support for Inject-Launch-Secret

[tobin]

On 2020-07-03 09:25, Brijesh Singh wrote:

On 7/3/20 6:11 AM, Dr. David Alan Gilbert wrote:

* Tobin Feldman-Fitzthum (to...@linux.vnet.ibm.com) wrote:

From: Tobin Feldman-Fitzthum 

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

Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret

[tobin]

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

[Qemu-devel] [Bug 685096] Re: USB Passthrough not working for Windows 7 guest

[Tobin Davis]

*** This bug is a duplicate of bug 1033727 ***
https://bugs.launchpad.net/bugs/1033727

** This bug has been marked a duplicate of bug 1033727
   USB passthrough doesn't work anymore with qemu-kvm 1.1.1

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/685096

Title:
  USB Passthrough not working for Windows 7 guest

Status in QEMU:
  Confirmed
Status in “qemu-kvm” package in Ubuntu:
  Confirmed

Bug description:
  USB Passthrough from host to guest is not working for a 32-bit Windows
  7 guest, while it works perfectly for a 32-bit Windows XP guest.

  The device appears in the device manager of Windows 7, but with "Error
  code 10: device cannot start". I have tried this with numerous USB
  thumbdrives and a USB wireless NIC, all with the same result. The
  device name and functionality is recognized, so at least some USB
  negotiation is taking place.

  I am trying this with the latest git-pull of QEMU-KVM.

  The command line to launch qemu-kvm for win7 is:
  sudo /home/user/local_install/bin/qemu-system-x86_64 -cpu core2duo -m 1024 
-smp 2 -vga std -hda ./disk_images/win7.qcow -vnc :1 -boot c -usb -usbdevice 
tablet -usbdevice host:0781:5150

  The command line to launch qemu-kvm for winxp is:
  sudo /home/user/local_install/bin/qemu-system-x86_64 -cpu core2duo -m 1024 
-smp 2 -usb -vga std -hda ./winxpsp3.qcow -vnc :0 -boot c -usbdevice tablet 
-usbdevice host:0781:5150

  Any help is appreciated.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/685096/+subscriptions

Re: [PATCH] qapi, i386/sev: Add debug-launch-digest to launch-measure response

[Tobin Feldman-Fitzthum]

on time and the second is more work
for the KBS at runtime. Like I said, the guest owner will need to know
an expected VMSA either way so maybe we should think of that as a
separate issue.
> 
> 
> I wonder if we're thinking of this at the wrong level though. Does
> it actually need to be QEMU providing this info to the guest owner ?
> 
The CSP could generate the hash that it expects to boot without the help
of QEMU, although it might be more complicated for SEV-ES. Even so, it
would be convenient if the CSP could ask QEMU/libvirt for the expected
hashes via the same interface that it gets the measurement. The CSP will
have to report the real launch measurement to the KBS. It would be handy
if the debug measurement were available at the same time with no extra
bookkeeping.

-Tobin

SEV: QMP support for Inject-Launch-Secret

[Tobin Feldman-Fitzthum]

This is an update to part of a patch submitted previously to
provide support for injecting a secret blob into guest memory
using AMD SEV.

The user provides a header and a wrapped secret blob via QMP,
which are provided to the AMD Secure Processor and injected
into the guest.

Note that this patch requires the user to provide the guest
physical address where the secret will be injected via QMP.

Tobin Feldman-Fitzthum (1):
  sev: add sev-inject-launch-secret

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

-- 
2.20.1 (Apple Git-117)

[PATCH 1/1] SEV: QMP support for Inject-Launch-Secret

[Tobin Feldman-Fitzthum]

From: Tobin Feldman-Fitzthum 

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

Re: [PATCH v4] sev: add sev-inject-launch-secret

[Tobin Feldman-Fitzthum]

On 2020-10-14 11:42, Brijesh Singh wrote:

On 10/14/20 10:17 AM, to...@linux.ibm.com wrote:

From: Tobin Feldman-Fitzthum 

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 facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c|  8 +++---
 qapi/misc-target.json | 18 
 target/i386/monitor.c |  7 +
 target/i386/sev-stub.c|  5 
 target/i386/sev.c | 60 
+++

 target/i386/trace-events  |  1 +
 8 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..af3887bb71 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"

 typedef struct MonitorHMP MonitorHMP;
 typedef struct MonitorOptions MonitorOptions;
@@ -37,6 +38,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(Monitor *mon, int cpu_index);
 int monitor_get_cpu_index(Monitor *mon);

+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..7ab6e3e31d 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, Error **errp);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 4a859fb24a..f1ade245d5 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -667,10 +667,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);
@@ -694,7 +694,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;
@@ -770,7 +770,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 1e561fa97b..4486a543ae 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -201,6 +201,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.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 7abae3c8df..f9d4951465 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -728,3 +728,10 @@ SevCapability *qmp_query_sev_capabilities(Error 
**errp)

 {
 return sev_get_capabilities(errp);
 }
+
+void qmp_sev_inject_launch_secret(const char *packet_hdr,
+  const char *secret, uint64_t gpa,
+  Error **errp)
+{
+sev_inject_launch_secret(packet_hdr, secret, gpa, errp);
+}
diff --git a/target/i386/sev-stub.c b/target/i386/sev-st

Re: [PATCH v5] sev: add sev-inject-launch-secret

[Tobin Feldman-Fitzthum]

On 2020-10-19 12:47, Eduardo Habkost wrote:

On Mon, Oct 19, 2020 at 12:46:08PM -0400, Eduardo Habkost wrote:

On Thu, Oct 15, 2020 at 10:37:13AM -0400, to...@linux.ibm.com wrote:
[...]
> diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
> index 88e3f39a1e..2d2ee54cc6 100644
> --- a/target/i386/sev-stub.c
> +++ b/target/i386/sev-stub.c
> @@ -49,3 +49,8 @@ SevCapability *sev_get_capabilities(Error **errp)
>  error_setg(errp, "SEV is not available in this QEMU");
>  return NULL;
>  }
> +int sev_inject_launch_secret(const char *hdr, const char *secret,
> + uint64_t gpa)
> +{
> +return 1;
> +}

This doesn't match the actual function prototype.  I had to apply the 
following

fixup:

---
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 2d2ee54cc6..62a2587e7b 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -49,8 +49,10 @@ SevCapability *sev_get_capabilities(Error **errp)
 error_setg(errp, "SEV is not available in this QEMU");
 return NULL;
 }
+
 int sev_inject_launch_secret(const char *hdr, const char *secret,
- uint64_t gpa)
+ uint64_t gpa, Error *errp)


Oops. Fixing up the fixup:


Thanks Eduardo.

-Tobin


---
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 62a2587e7b..e4e60d9a7d 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -51,7 +51,7 @@ SevCapability *sev_get_capabilities(Error **errp)
 }

 int sev_inject_launch_secret(const char *hdr, const char *secret,
- uint64_t gpa, Error *errp)
+ uint64_t gpa, Error **errp)
 {
 error_setg(errp, "SEV is not available in this QEMU");
 return 1;

Re: [PATCH v5] sev: add sev-inject-launch-secret

[Tobin Feldman-Fitzthum]

On 2020-10-20 11:56, Paolo Bonzini wrote:

On 20/10/20 15:54, Eduardo Habkost wrote:

On Tue, Oct 20, 2020 at 11:03:51AM +0200, Paolo Bonzini wrote:

On 15/10/20 16:37, to...@linux.ibm.com wrote:
-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);


You need to check size against mrs.size and fail if mrs.size is 
smaller.

 Otherwise, the ioctl can access memory out of range.


Good catch!  I'm dequeuing it.

Is there a reason memory_region_find() doesn't ensure that by
default?


IIRC memory_region_find() was used to do DMA in the very first versions
of "virtio-blk dataplane" so you would call it multiple times in a 
loop.

 So it's like that because it maps the way address_space_map() works.


The call at virtio_balloon_handle_output() looks suspicious,
though, because it looks for a BALLOON_PAGE_SIZE range, but
there's no check for the returned section size.


I think it's not a bug because ultimately it's checked in
ram_block_discard_range, but it's not pretty.

Paolo


Ok, it seems like the best solution is, as Paolo suggested, to add a
simple check at the end of gpa2hva that makes sure mr.size is equal
to size. gpa2hva is used one other place where the size is hard-coded
as 1, so adding the check isn't going to break anything.

Would you like me to resubmit with this tweak?

-Tobin

Re: [PATCH v6] sev: add sev-inject-launch-secret

[Tobin Feldman-Fitzthum]

On 2020-10-22 00:16, to...@linux.ibm.com wrote:

From: Tobin Feldman-Fitzthum 

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 facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Brijesh Singh 
---
 include/monitor/monitor.h |  3 ++
 include/sysemu/sev.h  |  2 ++
 monitor/misc.c| 13 +---
 qapi/misc-target.json | 18 +++
 target/i386/monitor.c |  7 +
 target/i386/sev-stub.c|  5 +++
 target/i386/sev.c | 65 +++
 target/i386/trace-events  |  1 +
 8 files changed, 110 insertions(+), 4 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..af3887bb71 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"

 typedef struct MonitorHMP MonitorHMP;
 typedef struct MonitorOptions MonitorOptions;
@@ -37,6 +38,8 @@ void monitor_flush(Monitor *mon);
 int monitor_set_cpu(Monitor *mon, int cpu_index);
 int monitor_get_cpu_index(Monitor *mon);

+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..7ab6e3e31d 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, Error **errp);
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index 4a859fb24a..dd148be5da 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -667,10 +667,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);
@@ -683,6 +683,11 @@ static void *gpa2hva(MemoryRegion **p_mr, hwaddr
addr, Error **errp)
 return NULL;
 }

+if (mrs.size < size) {
+error_setg(errp, "Size of memory region at 0x%" HWADDR_PRIx
+   " exceeded.", addr);
+}
+


Forgot to return :( Will update.


 *p_mr = mrs.mr;
 return qemu_map_ram_ptr(mrs.mr->ram_block, 
mrs.offset_within_region);

 }
@@ -694,7 +699,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;
@@ -770,7 +775,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 1e561fa97b..4486a543ae 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -201,6 +201,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.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 7abae3c8df..f9d4951465 100644
--- a/target/i386/monitor.c
+++

Re: [PATCH v7] sev: add sev-inject-launch-secret

[Tobin Feldman-Fitzthum]

On 2020-10-27 09:35, Eduardo Habkost wrote:

On Thu, Oct 22, 2020 at 01:39:09AM -0400, to...@linux.ibm.com wrote:

From: Tobin Feldman-Fitzthum 

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 facilitates the injection of the
launch secret, it cannot access the secret.

Signed-off-by: Tobin Feldman-Fitzthum 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Brijesh Singh 


I was going to queue it, but unfortunately it failed to build on some 
hosts:


https://gitlab.com/ehabkost/qemu/-/jobs/814250096

[1892/5203] Compiling C object 
libqemu-alpha-softmmu.fa.p/monitor_misc.c.o

FAILED: libqemu-alpha-softmmu.fa.p/monitor_misc.c.o
arm-linux-gnueabi-gcc -Ilibqemu-alpha-softmmu.fa.p -I. -I..
-Itarget/alpha -I../target/alpha -I../capstone/include/capstone -Iqapi
-Itrace -Iui -Iui/shader -I/usr/include/libdrm -I/usr/include/pixman-1
-I/usr/include/glib-2.0 -I/usr/lib/arm-linux-gnueabi/glib-2.0/include
-fdiagnostics-color=auto -pipe -Wall -Winvalid-pch -Werror -std=gnu99
-O2 -g -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
-Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes
-fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration
-Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
-Wendif-labels -Wexpansion-to-defined -Wno-missing-include-dirs
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong
-DLEGACY_RDMA_REG_MR -isystem /builds/ehabkost/qemu/linux-headers
-isystem linux-headers -iquote /builds/ehabkost/qemu/tcg/arm -iquote .
-iquote /builds/ehabkost/qemu -iquote /builds/ehabkost/qemu/accel/tcg
-iquote /builds/ehabkost/qemu/include -iquote
/builds/ehabkost/qemu/disas/libvixl -pthread -fPIC
-isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H
'-DCONFIG_TARGET="alpha-softmmu-config-target.h"'
'-DCONFIG_DEVICES="alpha-softmmu-config-devices.h"' -MD -MQ
libqemu-alpha-softmmu.fa.p/monitor_misc.c.o -MF
libqemu-alpha-softmmu.fa.p/monitor_misc.c.o.d -o
libqemu-alpha-softmmu.fa.p/monitor_misc.c.o -c ../monitor/misc.c
../monitor/misc.c: In function 'gpa2hva':
../monitor/misc.c:686:18: error: invalid operands to binary < (have
'Int128' {aka 'struct Int128'} and 'uint64_t' {aka 'long long unsigned
int'})
 if (mrs.size < size) {
  ^
[1893/5203] Compiling C object 
libqemu-alpha-softmmu.fa.p/softmmu_physmem.c.o

ninja: build stopped: subcommand failed.


I am not easily able to replicate this (perhaps an issue for ARM only?).

Either way, I think it would be better to make size into an Int128
and use the appropriate comparison function. I will submit a new 
version.

I can test this better with a bit more time. For now, up to you if you
want to try building it.

-Tobin

[PATCH 2/2] sev: scan guest ROM for launch secret address

[Tobin Feldman-Fitzthum]

From: Tobin Feldman-Fitzthum 

In addition to using QMP to provide the guest memory address
that the launch secret blob will be injected into, the
secret address can also be specified in the guest ROM. This
patch adds sev_find_secret_gpa, which scans the ROM page by
page to find a launch secret table identified by a GUID. If
the table is found, the address it contains will be used
in place of any address specified via QMP.

Signed-off-by: Tobin Feldman-Fitzthum 
---
 target/i386/sev.c  | 34 --
 target/i386/sev_i386.h | 16 
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 774e47d9d1..4adc56d7e3 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -706,6 +706,8 @@ sev_guest_init(const char *id)
 s->api_major = status.api_major;
 s->api_minor = status.api_minor;
 
+s->secret_gpa = 0;
+
 trace_kvm_sev_init();
 ret = sev_ioctl(s->sev_fd, KVM_SEV_INIT, NULL, &fw_error);
 if (ret) {
@@ -731,6 +733,28 @@ err:
 return NULL;
 }
 
+static void
+sev_find_secret_gpa(uint8_t *ptr, uint64_t len)
+{
+uint64_t offset;
+
+SevROMSecretTable *secret_table;
+QemuUUID secret_table_guid;
+
+qemu_uuid_parse(SEV_ROM_SECRET_GUID,&secret_table_guid);
+secret_table_guid = qemu_uuid_bswap(secret_table_guid);
+
+offset = len - 0x1000;
+while(offset > 0) {
+secret_table = (SevROMSecretTable *)(ptr + offset);
+if(qemu_uuid_is_equal(&secret_table_guid, (QemuUUID *) secret_table)){
+sev_state->secret_gpa = (long unsigned int) secret_table->base;
+break;
+}
+offset -= 0x1000;
+}
+}
+
 int
 sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
 {
@@ -738,6 +762,9 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
 
 /* if SEV is in update state then encrypt the data else do nothing */
 if (sev_check_state(SEV_STATE_LAUNCH_UPDATE)) {
+if(!sev_state->secret_gpa) {
+sev_find_secret_gpa(ptr, len);
+   }
 return sev_launch_update_data(ptr, len);
 }
 
@@ -776,8 +803,8 @@ int sev_inject_launch_secret(const char *packet_hdr,
 
 /* secret can be inject only in this state */
 if (!sev_check_state(SEV_STATE_LAUNCH_SECRET)) {
-   error_report("Not in correct state. %x",sev_state->state);
-   return 1;
+error_report("Not in correct state. %x",sev_state->state);
+return 1;
 }
 
 hdr = g_base64_decode(packet_hdr, &hdr_sz);
@@ -792,6 +819,9 @@ int sev_inject_launch_secret(const char *packet_hdr,
 goto err;
 }
 
+if(sev_state->secret_gpa)
+gpa = sev_state->secret_gpa;
+
 hva = gpa2hva(gpa, data_sz);
 if (!hva) {
 goto err;
diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 8ada9d385d..b1f9ab93bb 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -19,6 +19,7 @@
 #include "sysemu/kvm.h"
 #include "sysemu/sev.h"
 #include "qemu/error-report.h"
+#include "qemu/uuid.h"
 #include "qapi/qapi-types-misc-target.h"
 
 #define SEV_POLICY_NODBG0x1
@@ -28,6 +29,8 @@
 #define SEV_POLICY_DOMAIN   0x10
 #define SEV_POLICY_SEV  0x20
 
+#define SEV_ROM_SECRET_GUID "adf956ad-e98c-484c-ae11-b51c7d336447"
+
 #define TYPE_QSEV_GUEST_INFO "sev-guest"
 #define QSEV_GUEST_INFO(obj)  \
 OBJECT_CHECK(QSevGuestInfo, (obj), TYPE_QSEV_GUEST_INFO)
@@ -42,6 +45,18 @@ extern SevCapability *sev_get_capabilities(void);
 
 typedef struct QSevGuestInfo QSevGuestInfo;
 typedef struct QSevGuestInfoClass QSevGuestInfoClass;
+typedef struct SevROMSecretTable SevROMSecretTable;
+
+/**
+ * If guest physical address for the launch secret is
+ * provided in the ROM, it should be in the following
+ * page-aligned structure.
+ */
+struct SevROMSecretTable {
+QemuUUID guid;
+unsigned int base;
+unsigned int size;
+};
 
 /**
  * QSevGuestInfo:
@@ -78,6 +93,7 @@ struct SEVState {
 uint32_t cbitpos;
 uint32_t reduced_phys_bits;
 uint32_t handle;
+uint64_t secret_gpa;
 int sev_fd;
 SevState state;
 gchar *measurement;
-- 
2.20.1 (Apple Git-117)

[PATCH 1/2] sev: add sev-inject-launch-secret

[Tobin Feldman-Fitzthum]

From: Tobin Feldman-Fitzthum 

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 
---
 include/sysemu/sev.h   |  2 +
 qapi/misc-target.json  | 20 +
 target/i386/monitor.c  |  8 
 target/i386/sev-stub.c |  5 +++
 target/i386/sev.c  | 83 ++
 target/i386/trace-events   |  1 +
 tests/qtest/qmp-cmd-test.c |  6 +--
 7 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 98c1ec8d38..313ee30fc8 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/qapi/misc-target.json b/qapi/misc-target.json
index dee3b45930..27458b765b 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -200,6 +200,26 @@
 { '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.
+GPA provided here will be ignored if guest ROM specifies 
+the a launch secret GPA.
+#
+# Since: 5.0.0
+#
+##
+{ 'command': 'sev-inject-launch-secret',
+  'data': { 'packet_hdr': '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..5c2b7d2c17 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -736,3 +736,11 @@ 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..2b8c5f1f53 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 846018a12d..774e47d9d1 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -28,6 +28,7 @@
 #include "sysemu/runstate.h"
 #include "trace.h"
 #include "migration/blocker.h"
+#include "exec/address-spaces.h"
 
 #define DEFAULT_GUEST_POLICY0x1 /* disable debug */
 #define DEFAULT_SEV_DEVICE  "/dev/sev"
@@ -743,6 +744,88 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
 return 0;
 }
 
+
+static void *
+gpa2hva(hwaddr addr, uint64_t size)
+{
+MemoryRegionSection mrs = memory_region_find(get_system_memory(),
+ addr, size);
+
+if (!mrs.mr) {
+error_report("No memory is mapped at address 0x%" HWADDR_PRIx, addr);
+return NULL;
+}
+
+if (!memory_region_is_ram(mrs.mr) && !memory_region_is_romd(mrs.mr)) {
+error_report("Memory at address 0x%" HWADDR_PRIx "is not RAM", addr);
+memory_region_unref(mrs.mr);
+return NULL;
+}
+
+return qemu_map_ram_ptr(mrs.mr->ram_block, mrs.offset_within_region);
+}
+
+int sev_inject_launch_secret(const char *packet_hdr,
+ const char *secret, uint64_t gpa)
+{
+struct kvm_sev_launch_secret *input = NULL;
+guchar *data = NULL, *hdr = NULL;
+int error, ret = 1;
+void *hva;
+gsize hdr_sz = 0, data_sz = 0;
+
+/* secret can be inject only in this state */
+if (!sev_check_state(SEV_STATE_LAUNCH_SECRET)) {
+   error_report("Not in correct state. %x",sev_state->state);
+   return 1;
+}
+
+hdr = g_base64_decode(packet_hdr, &hdr_sz);
+if (!hdr || !hdr_sz) {
+error_report("SEV: Failed to decode sequence header");
+return 

[PATCH 0/2] Add support for SEV Launch Secret Injection

[Tobin Feldman-Fitzthum]

This patchset contains two patches. The first enables QEMU
to facilitate the injection of a secret blob into the guest
memory.

The second enables QEMU to parse the guest ROM to determine
the address at which the secret should be injected.

Tobin Feldman-Fitzthum (2):
  sev: add sev-inject-launch-secret
  sev: scan guest ROM for launch secret address

 include/sysemu/sev.h   |   2 +
 qapi/misc-target.json  |  20 +++
 target/i386/monitor.c  |   8 +++
 target/i386/sev-stub.c |   5 ++
 target/i386/sev.c  | 113 +
 target/i386/sev_i386.h |  16 ++
 target/i386/trace-events   |   1 +
 tests/qtest/qmp-cmd-test.c |   6 +-
 8 files changed, 168 insertions(+), 3 deletions(-)

-- 
2.20.1 (Apple Git-117)

Re: [RFC PATCH 00/13] Add support for Mirror VM.

[Tobin Feldman-Fitzthum]

On 8/17/21 12:32 PM, Paolo Bonzini wrote:

On 17/08/21 01:53, Steve Rutherford wrote:

Separately, I'm a little weary of leaving the migration helper mapped
into the shared address space as writable.


A related question here is what the API should be for how the 
migration helper sees the memory in both physical and virtual address.


First of all, I would like the addresses passed to and from the 
migration helper to *not* be guest physical addresses (this is what I 
referred to as QEMU's ram_addr_t in other messages).  The reason is 
that some unmapped memory regions, such as virtio-mem hotplugged 
memory, would still have to be transferred and could be encrypted.  
While the guest->host hypercall interface uses guest physical 
addresses to communicate which pages are encrypted, the host can do 
the GPA->ram_addr_t conversion and remember the encryption status of 
currently-unmapped regions.


This poses a problem, in that the guest needs to prepare the page 
tables for the migration helper and those need to use the migration 
helper's physical address space.


There's three possibilities for this:

1) the easy one: the bottom 4G of guest memory are mapped in the 
mirror VM 1:1.  The ram_addr_t-based addresses are shifted by either 
4G or a huge value such as 2^42 (MAXPHYADDR - physical address 
reduction - 1). This even lets the migration helper reuse the OVMF 
runtime services memory map (but be careful about thread safety...).


This is essentially what we do in our prototype, although we have an 
even simpler approach. We have a 1:1 mapping that maps an address to 
itself with the cbit set. During Migration QEMU asks the migration 
handler to import/export encrypted pages and provides the GPA for said 
page. Since the migration handler only exports/imports encrypted pages, 
we can have the cbit set for every page in our mapping. We can still use 
OVMF functions with these mappings because they are on encrypted pages. 
The MH does need to use a few shared pages (to communicate with QEMU, 
for instance), so we have another mapping without the cbit that is at a 
large offset.


I think this is basically equivalent to what you suggest. As you point 
out above, this approach does require that any page that will be 
exported/imported by the MH is mapped in the guest. Is this a bad 
assumption? The VMSA for SEV-ES is one example of a region that is 
encrypted but not mapped in the guest (the PSP handles it directly). We 
have been planning to map the VMSA into the guest to support migration 
with SEV-ES (along with other changes).


2) the more future-proof one.  Here, the migration helper tells QEMU 
which area to copy from the guest to the mirror VM, as a (main GPA, 
length, mirror GPA) tuple.  This could happen for example the first 
time the guest writes 1 to MSR_KVM_MIGRATION_CONTROL.  When migration 
starts, QEMU uses this information to issue KVM_SET_USER_MEMORY_REGION 
accordingly.  The page tables are built for this (usually very high) 
mirror GPA and the migration helper operates in a completely separate 
address space.  However, the backing memory would still be shared 
between the main and mirror VMs.  I am saying this is more future 
proof because we have more flexibility in setting up the physical 
address space of the mirror VM.


The Migration Handler in OVMF is not a contiguous region of memory. The 
MH uses OVMF helper functions that are allocated in various regions of 
runtime memory. I guess I can see how separating the memory of the MH 
and the guest OS could be positive. On the other hand, since the MH is 
in OVMF, it is fundamentally designed to coexist with the guest OS.


What do you envision in terms of future changes to the mirror address space?

3) the paranoid one, which I think is what you hint at above: this is 
an extension of (2), where userspace invokes the PSP send/receive API 
to copy the small requested area of the main VM into the mirror VM.  
The mirror VM code and data are completely separate from the main VM.  
All that the mirror VM shares is the ram_addr_t data. Though I am not 
even sure it is possible to use the send/receive API this way...


Yeah not sure if you could use the PSP for this.

-Tobin



What do you think?

Paolo

Re: [RFC PATCH 00/13] Add support for Mirror VM.

[Tobin Feldman-Fitzthum]

On 8/17/21 6:04 PM, Steve Rutherford wrote:

On Tue, Aug 17, 2021 at 1:50 PM Tobin Feldman-Fitzthum
 wrote:

This is essentially what we do in our prototype, although we have an
even simpler approach. We have a 1:1 mapping that maps an address to
itself with the cbit set. During Migration QEMU asks the migration
handler to import/export encrypted pages and provides the GPA for said
page. Since the migration handler only exports/imports encrypted pages,
we can have the cbit set for every page in our mapping. We can still use
OVMF functions with these mappings because they are on encrypted pages.
The MH does need to use a few shared pages (to communicate with QEMU,
for instance), so we have another mapping without the cbit that is at a
large offset.

I think this is basically equivalent to what you suggest. As you point
out above, this approach does require that any page that will be
exported/imported by the MH is mapped in the guest. Is this a bad
assumption? The VMSA for SEV-ES is one example of a region that is
encrypted but not mapped in the guest (the PSP handles it directly). We
have been planning to map the VMSA into the guest to support migration
with SEV-ES (along with other changes).

Ahh, It sounds like you are looking into sidestepping the existing
AMD-SP flows for migration. I assume the idea is to spin up a VM on
the target side, and have the two VMs attest to each other. How do the
two sides know if the other is legitimate? I take it that the source
is directing the LAUNCH flows?


Yeah we don't use PSP migration flows at all. We don't need to send the 
MH code from the source to the target because the MH lives in firmware, 
which is common between the two. We start the target like a normal VM 
rather than waiting for an incoming migration. The plan is to treat the 
target like a normal VM for attestation as well. The guest owner will 
attest the target VM just like they would any other VM that is started 
on their behalf. Secret injection can be used to establish a shared key 
for the source and target.


-Tobin



--Steve

Re: [RFC PATCH 00/13] Add support for Mirror VM.

[Tobin Feldman-Fitzthum]

On 8/18/21 3:04 PM, Dr. David Alan Gilbert wrote:

* Tobin Feldman-Fitzthum (to...@linux.ibm.com) wrote:

On 8/17/21 6:04 PM, Steve Rutherford wrote:

Ahh, It sounds like you are looking into sidestepping the existing
AMD-SP flows for migration. I assume the idea is to spin up a VM on
the target side, and have the two VMs attest to each other. How do the
two sides know if the other is legitimate? I take it that the source
is directing the LAUNCH flows?

Yeah we don't use PSP migration flows at all. We don't need to send the MH
code from the source to the target because the MH lives in firmware, which
is common between the two.

Are you relying on the target firmware to be *identical* or purely for
it to be *compatible* ?  It's normal for a migration to be the result of
wanting to do an upgrade; and that means the destination build of OVMF
might be newer (or older, or ...).

Dave


This is a good point. The migration handler on the source and target 
must have the same memory footprint or bad things will happen. Using the 
same firmware on the source and target is an easy way to guarantee this. 
Since the MH in OVMF is not a contiguous region of memory, but a group 
of functions scattered around OVMF, it is a bit difficult to guarantee 
that the memory footprint is the same if the build is different.


-Tobin





We start the target like a normal VM rather than
waiting for an incoming migration. The plan is to treat the target like a
normal VM for attestation as well. The guest owner will attest the target VM
just like they would any other VM that is started on their behalf. Secret
injection can be used to establish a shared key for the source and target.

-Tobin


--Steve

Re: [RFC PATCH 00/13] Add support for Mirror VM.

[Tobin Feldman-Fitzthum]

On 8/19/21 4:22 AM, Dr. David Alan Gilbert wrote:

* Tobin Feldman-Fitzthum (to...@linux.ibm.com) wrote:

On 8/18/21 3:04 PM, Dr. David Alan Gilbert wrote:


Are you relying on the target firmware to be *identical* or purely for
it to be *compatible* ?  It's normal for a migration to be the result of
wanting to do an upgrade; and that means the destination build of OVMF
might be newer (or older, or ...).

Dave

This is a good point. The migration handler on the source and target must
have the same memory footprint or bad things will happen. Using the same
firmware on the source and target is an easy way to guarantee this. Since
the MH in OVMF is not a contiguous region of memory, but a group of
functions scattered around OVMF, it is a bit difficult to guarantee that the
memory footprint is the same if the build is different.

Can you explain what the 'memory footprint' consists of? Can't it just
be the whole of the OVMF rom space if you have no way of nudging the MH
into it's own chunk?


The footprint is not massive. It is mainly ConfidentialMigrationDxe and 
the OVMF crypto support. It might be feasible to copy these components 
to a fixed location that would be the same across fw builds. It might 
also be feasible to pin these components to certain addresses. OVMF sort 
of supports doing this. We can raise the question in that community.


It also might work to protect the entirety of OVMF as you suggest. 
Currently we don't copy any of the OVMF ROM (as in flash0) over. That 
said, the MH doesn't run from the ROM so we would need to protect the 
memory used by OVMF as well. In some ways it might seem easier to 
protect all of the OVMF memory rather than just a couple of packages, 
but there are some complexities. For one thing, we would only want to 
protect efi runtime memory, as boot memory may be in use by the OS and 
would need to be migrated. The MH could check whether each page is efi 
runtime memory and skip any pages that are. Runtime memory won't be a 
contiguous blob, however, so for this approach the layout of the runtime 
memory would need to be the same on the source and target.


We can sidestep these issues entirely by using identical firmware 
images. That said, there are a number of strategies for developing 
compatible OVMF images and I definitely see the value of doing so.


-Tobin



I think it really does have to cope with migration to a new version of
host.

Dave


-Tobin




We start the target like a normal VM rather than
waiting for an incoming migration. The plan is to treat the target like a
normal VM for attestation as well. The guest owner will attest the target VM
just like they would any other VM that is started on their behalf. Secret
injection can be used to establish a shared key for the source and target.

-Tobin


--Steve

Re: [RFC PATCH 00/13] Add support for Mirror VM.

[Tobin Feldman-Fitzthum]

On 8/23/21 8:26 AM, Dr. David Alan Gilbert wrote:


* James Bottomley (j...@linux.ibm.com) wrote:


(is there an attest of the destination happening here?)
There will be in the final version.  The attestations of the source and
target, being the hash of the OVMF (with the registers in the -ES
case), should be the same (modulo any firmware updates to the PSP,
whose firmware version is also hashed) to guarantee the OVMF is the
same on both sides.  We'll definitely take an action to get QEMU to
verify this ... made a lot easier now we have signed attestations ...

Hmm; I'm not sure you're allowed to have QEMU verify that - we don't
trust it; you need to have either the firmware say it's OK to migrate
to the destination (using the existing PSP mechanism) or get the source
MH to verify a quote from the destination.


I think the check in QEMU would only be a convenience. The launch 
measurement of the target (verified by the guest owner) is what 
guarantees that the firmware, as well as the policy, of the target is 
what is expected. In PSP-assisted migration the source verifies the 
target, but our plan is to have the guest owner verify both the source 
and the target. The target will only be provisioned with the transport 
key if the measurement checks out. We will have some more details about 
this key agreement scheme soon.



[Somewhere along the line, if you're not using the PSP, I think you also
need to check the guest policy to check it is allowed to migrate].


Sources that aren't allowed to migrate won't be provisioned with 
transport key to encrypt pages. A non-migrateable guest could also be 
booted with OvmfPkg firmware, which does not contain the migration handler.


-Tobin


Dave


James

Re: Fw: [EXTERNAL] Re: [RFC PATCH 00/13] Add support for Mirror VM.

[Tobin Feldman-Fitzthum]

On Mon, Aug 16, 2021 at 04:15:46PM +0200, Paolo Bonzini wrote:

> Hi,
>
> first of all, thanks for posting this work and starting the discussion.
>
> However, I am not sure if the in-guest migration helper vCPUs should use
> the existing KVM support code.  For example, they probably can just
> always work with host CPUID (copied directly from
> KVM_GET_SUPPORTED_CPUID), and they do not need to interface with QEMU's
> MMIO logic.  They would just sit on a "HLT" instruction and communicate
> with the main migration loop using some kind of standardized ring buffer
> protocol; the migration loop then executes KVM_RUN in order to start the
> processing of pages, and expects a KVM_EXIT_HLT when the VM has nothing
> to do or requires processing on the host.
> The migration helper can then also use its own address space, for
> example operating directly on ram_addr_t values with the helper running
> at very high virtual addresses.  Migration code can use a
> RAMBlockNotifier to invoke KVM_SET_USER_MEMORY_REGION on the mirror VM
> (and never enable dirty memory logging on the mirror VM, too, which has
> better performance).
>
> With this implementation, the number of mirror vCPUs does not even have
> to be indicated on the command line.  The VM and its vCPUs can simply be
> created when migration starts.  In the SEV-ES case, the guest can even
> provide the VMSA that starts the migration helper.

It might make sense to tweak the mirror support code so that it is more
closely tied to migration and the migration handler. On the other hand,
the usage of a mirror VM might be more general than just migration. In
some ways the mirror offers similar functionality to the VMPL in SNP,
providing a way to run non-workload code inside the enclave. This
potentially has uses beyond migration. If this is the case, do maybe we
want to keep the mirror more general.

It's also worth noting that the SMP interface that Ashish is using to
specify the mirror might come in handy if we ever want to have more than
one vCPU in the mirror. For instance we might want to use multiple MH
vCPUs to increase throughput.

-Tobin

> The disadvantage is that, as you point out, in the future some of the
> infrastructure you introduce might be useful for VMPL0 operation on
> SEV-SNP.  My proposal above might require some code duplication.
> However, it might even be that VMPL0 operation works best with a model
> more similar to my sketch of the migration helper; it's really too early
> to say.
>
> Paolo

Re: [RFC PATCH 00/13] Add support for Mirror VM.

[Tobin Feldman-Fitzthum]

On Mon, Aug 16 at 10:44 AM Ashish Kalra wrote:

> I am not sure if we really don't need QEMU's MMIO logic, I think that 
once the>
> mirror VM starts booting and running the UEFI code, it might be only 
during

> the PEI or DXE phase where it will start actually running the MH code,
> so mirror VM probably still need to handles KVM_EXIT_IO when SEC 
phase does I/O,
> I can see PIC accesses and Debug Agent initialization stuff in SEC 
startup code.


The migration handler prototype that we are releasing in the near future 
does not use the SEC or PEI phases in the mirror. We have some support 
code that runs in the main VM and sets up the migration handler entry 
point. QEMU starts the mirror pointing to this entry point, which does 
some more setup (like switching to long mode) and jumps to the migration 
handler.


-Tobin

> Addtionally this still requires CPUState{..} structure and the backing
> "X86CPU" structure, for example, as part of kvm_arch_post_run() to get
> the MemTxAttrs needed by kvm_handle_io().

> Thanks,
> Ashish

Re: [PATCH v3] SEV: QMP support for Inject-Launch-Secret

[Tobin Feldman-Fitzthum]

On 2020-09-21 15:16, Dr. David Alan Gilbert wrote:

* Tobin Feldman-Fitzthum (to...@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 


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)
+{
+

RFC: Fast Migration for SEV and SEV-ES - blueprint and proof of concept

[Tobin Feldman-Fitzthum]

Hello,

Dov Murik, James Bottomley, Hubertus Franke, and I have been working on 
a plan for fast live migration with SEV and SEV-ES. We just posted an 
RFC about it to the edk2 list. It includes a proof-of-concept for what 
we feel to be the most difficult part of fast live migration with SEV-ES.


https://edk2.groups.io/g/devel/topic/77875297

This was posted to the edk2 list because OVMF is one of the main 
components of our approach to live migration. With SEV/SEV-ES the 
hypervisor generally does not have access to guest memory/CPU state. We 
propose a Migration Handler in OVMF that runs inside the guest and 
assists the hypervisor with migration. One major challenge to this 
approach is that for SEV-ES this Migration Handler must be able to set 
the CPU state of the target to the CPU state of the source while the 
target is running. Our demo shows that this is feasible.


While OVMF is a major component of our approach, QEMU obviously has a 
huge part to play as well. We want to start thinking about the best way 
to support fast live migration for SEV and for encrypted VMs in general. 
A handful of issues are starting to come into focus. For instance, the 
target VM needs to be started before we begin receiving pages from the 
source VM. We will also need to start an extra vCPU for the Migration 
Handler to run on. We are currently working on a demo of end-to-end live 
migration for SEV (non-ES) VMs that should help crystallize these 
issues. It should be on the list around the end of the year. For now, 
check out our other post, which has a lot more information and let me 
know if you have any thoughts.


-Tobin

Re: RFC: Fast Migration for SEV and SEV-ES - blueprint and proof of concept

[Tobin Feldman-Fitzthum]

On 2020-10-30 16:02, Dr. David Alan Gilbert wrote:

* Tobin Feldman-Fitzthum (to...@linux.ibm.com) wrote:

Hello,

Dov Murik, James Bottomley, Hubertus Franke, and I have been working 
on a
plan for fast live migration with SEV and SEV-ES. We just posted an 
RFC
about it to the edk2 list. It includes a proof-of-concept for what we 
feel

to be the most difficult part of fast live migration with SEV-ES.

https://edk2.groups.io/g/devel/topic/77875297

This was posted to the edk2 list because OVMF is one of the main 
components
of our approach to live migration. With SEV/SEV-ES the hypervisor 
generally

does not have access to guest memory/CPU state. We propose a Migration
Handler in OVMF that runs inside the guest and assists the hypervisor 
with
migration. One major challenge to this approach is that for SEV-ES 
this
Migration Handler must be able to set the CPU state of the target to 
the CPU
state of the source while the target is running. Our demo shows that 
this is

feasible.

While OVMF is a major component of our approach, QEMU obviously has a 
huge

part to play as well. We want to start thinking about the best way to
support fast live migration for SEV and for encrypted VMs in general. 
A
handful of issues are starting to come into focus. For instance, the 
target
VM needs to be started before we begin receiving pages from the source 
VM.


That might not be that hard to fudge; we already start the VM in
postcopy mode before we have all of RAM; now in that we have to do 
stuff

to protect the RAM because we expect the guest to access it; in this
case you possibly don't need to.


I'll need to think a bit about this. The basic setup is that we want the
VM to boot into OVMF and initialize the Migration Handler. Then QEMU 
will start
receiving encrypted pages and passing them into OVMF via some shared 
address.

The Migration Handler will decrypt the pages and put them into place,
overwriting everything around it. The Migration Handler will be a 
runtime

driver so it should avoid overwriting itself.

We will also need to start an extra vCPU for the Migration Handler to 
run
on. We are currently working on a demo of end-to-end live migration 
for SEV
(non-ES) VMs that should help crystallize these issues. It should be 
on the
list around the end of the year. For now, check out our other post, 
which

has a lot more information and let me know if you have any thoughts.


I don't think I understood why you'd want to map the VMSA, or why it
would be encrypted in such a way it was useful to you.


We map the VMSA into the guest because it gives us an easy way to
securely send the CPU state to the target.

Each time there is a VMExit, the CPU state of the guest
is stored in the VMSA. Although the VMSA is encrypted with the guest's 
key,

it usually isn't mapped into the guest. During migration we securely
transfer guest memory from source to destination (the Migration Handler
on source and target share a key which they use to encrypt/decrypt the
pages). If we tweak the NPT to add the VMSA to the guest, the VMSA will 
be

sent along with the all the other pages.

There are some details with the timing. We'll need to force a VMExit 
once
we get convergence and re-send the VMSA page to make sure it's up to 
date.
Once the target has all the pages, the Migration Handler can just read 
the

CPU state from some known address.

-Tobin


Dave



-Tobin

[PATCH v2] SEV: QMP support for Inject-Launch-Secret

[Tobin Feldman-Fitzthum]

From: Tobin Feldman-Fitzthum 

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

[PATCH v3] SEV: QMP support for Inject-Launch-Secret

[Tobin Feldman-Fitzthum]

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