qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support
@ 2019-08-06 16:54 Singh, Brijesh
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 01/14] doc: update AMD SEV API spec web link Singh, Brijesh
                   ` (13 more replies)
  0 siblings, 14 replies; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

AMD SEV encrypts the memory of VMs and because this encryption is done using
an address tweak, the hypervisor will not be able to simply copy ciphertext
between machines to migrate a VM. Instead the AMD SEV Key Management API
provides a set of functions which the hypervisor can use to package a
guest encrypted pages for migration, while maintaining the confidentiality
provided by AMD SEV.

The patch series add the support required in Qemu to perform the SEV
guest live migration. Before initiating the live migration a user
should use newly added 'migrate-set-sev-info' command to pass the
target machines certificate chain. See the docs/amd-memory-encryption.txt
for further details.

The patch series depends on kernel patches available here:
https://marc.info/?l=kvm&m=156278967226011&w=2

The complete tree with patch is available at:
https://github.com/codomania/qemu/tree/sev-migration-v3

Known Issues:
 - failed to reboot the guest after migration.
 - The top 10 lines of the vga buffer is sent as encrypted and because of that
   we get a garage on destination. I am still debugging it.

Changes since v2:
 - Remove direct kvm_memcrpt calls from migration.
 - Add MemoryEcryptionOps in machine which will be used by migration
   instead of kvm_memcrypt calls.
 - drop the RAM_SAVE_FLAG_PAGE_ENCRYPTED_BITMAP. Now the RAM_SAVE_FLAG_ENCRYPTED_PAGE
   can be used for sending bitmap as well as guest RAM encrypted pages
 - add some bound checks on incoming data
 - drop migrate-sev-set-info object
 - extend the migrate-parameters to include the SEV specific certificate fields.
 - multiple fixes based on the review comments from Dave
 
Changes since v1:
 - use the dirty log sync APIs to also sync the page encryption bitmap
   when SEV is active.

Brijesh Singh (14):
  doc: update AMD SEV API spec web link
  doc: update AMD SEV to include Live migration flow
  migration.json: add AMD SEV specific migration parameters
  linux-headers: update kernel header to include SEV migration commands
  hw/machine: add helper to query the memory encryption state
  hw/machine: introduce MachineMemoryEncryptionOps for encrypted VMs
  target/i386: sev: provide callback to setup outgoing context
  target/i386: sev: do not create launch context for an incoming guest
  target/i386: sev: add support to encrypt the outgoing page
  target/i386: sev: add support to load incoming encrypted page
  migration: add support to migrate page encryption bitmap
  kvm: add support to sync the page encryption state bitmap
  migration/ram: add support to send encrypted pages
  target/i386: sev: remove migration blocker

 accel/kvm/kvm-all.c            |  91 ++++++
 accel/kvm/sev-stub.c           |  28 ++
 docs/amd-memory-encryption.txt |  48 ++-
 hw/core/machine.c              |   5 +
 include/exec/ram_addr.h        | 199 +++++++++++++
 include/exec/ramlist.h         |   3 +-
 include/hw/boards.h            |  25 ++
 include/sysemu/sev.h           |  11 +
 linux-headers/linux/kvm.h      |  53 ++++
 migration/migration.c          |  61 ++++
 migration/ram.c                | 148 +++++++++-
 monitor/hmp-cmds.c             |  18 ++
 qapi/migration.json            |  41 ++-
 target/i386/sev.c              | 513 ++++++++++++++++++++++++++++++++-
 target/i386/sev_i386.h         |   8 +
 target/i386/trace-events       |   8 +
 16 files changed, 1234 insertions(+), 26 deletions(-)

-- 
2.17.1



^ permalink raw reply	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 01/14] doc: update AMD SEV API spec web link
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-06 19:00   ` Dr. David Alan Gilbert
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 02/14] doc: update AMD SEV to include Live migration flow Singh, Brijesh
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 docs/amd-memory-encryption.txt | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
index 43bf3ee6a5..8822cadda1 100644
--- a/docs/amd-memory-encryption.txt
+++ b/docs/amd-memory-encryption.txt
@@ -67,8 +67,8 @@ expects.
 LAUNCH_FINISH command finalizes the guest launch and destroy's the cryptographic
 context.
 
-See SEV KM API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
-complete flow chart.
+See Secure Encrypted Virtualization Key Management API spec section
+'Launching a guest' usage flow  (Appendix A) for the complete flow chart.
 
 To launch a SEV guest
 
@@ -97,8 +97,8 @@ References
 AMD Memory Encryption whitepaper:
 http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf
 
-Secure Encrypted Virtualization Key Management:
-[1] http://support.amd.com/TechDocs/55766_SEV-KM API_Specification.pdf
+Secure Encrypted Virtualization Key Management API Spec:
+[1] https://developer.amd.com/sev/ (Secure Encrypted Virtualization API)
 
 KVM Forum slides:
 http://www.linux-kvm.org/images/7/74/02x08A-Thomas_Lendacky-AMDs_Virtualizatoin_Memory_Encryption_Technology.pdf
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 02/14] doc: update AMD SEV to include Live migration flow
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 01/14] doc: update AMD SEV API spec web link Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-07 11:01   ` Dr. David Alan Gilbert
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters Singh, Brijesh
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 docs/amd-memory-encryption.txt | 40 +++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
index 8822cadda1..01d95089a8 100644
--- a/docs/amd-memory-encryption.txt
+++ b/docs/amd-memory-encryption.txt
@@ -89,7 +89,45 @@ TODO
 
 Live Migration
 ----------------
-TODO
+AMD SEV encrypts the memory of VMs and because a different key is used
+in each VM, the hypervisor will be unable to simply copy the
+ciphertext from one VM to another to migrate the VM. Instead the AMD SEV Key
+Management API provides sets of function which the hypervisor can use
+to package a guest page for migration, while maintaining the confidentiality
+provided by AMD SEV.
+
+SEV guest VMs have the concept of private and shared memory. The private
+memory is encrypted with the guest-specific key, while shared memory may
+be encrypted with the hypervisor key. The migration APIs provided by the
+SEV API spec should be used for migrating the private pages. The
+KVM_GET_PAGE_ENC_BITMAP ioctl can be used to get the guest page encryption
+bitmap. The bitmap can be used to check if the given guest page is
+private or shared.
+
+Before initiating the migration, we need to know the targets machine's public
+Diffie-Hellman key (PDH) and certificate chain. It can be retrieved
+with the 'query-sev-capabilities' QMP command or using the sev-tool. The
+migrate-set-parameter can be used to pass the target machine's PDH and
+certificate chain.
+
+During the migration flow, the SEND_START is called on the source hypervisor
+to create an outgoing encryption context. The SEV guest policy dictates whether
+the certificate passed through the migrate-sev-set-info command will be
+validated. SEND_UPDATE_DATA is called to encrypt the guest private pages.
+After migration is completed, SEND_FINISH is called to destroy the encryption
+context and make the VM non-runnable to protect it against cloning.
+
+On the target machine, RECEIVE_START is called first to create an
+incoming encryption context. The RECEIVE_UPDATE_DATA is called to copy
+the received encrypted page into guest memory. After migration has
+completed, RECEIVE_FINISH is called to make the VM runnable.
+
+For more information about the migration see SEV API Appendix A
+Usage flow (Live migration section).
+
+NOTE:
+To protect against the memory clone SEV APIs are designed to make the VM
+unrunnable in case of the migration failure.
 
 References
 -----------------
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 01/14] doc: update AMD SEV API spec web link Singh, Brijesh
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 02/14] doc: update AMD SEV to include Live migration flow Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-07 11:06   ` Dr. David Alan Gilbert
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 04/14] linux-headers: update kernel header to include SEV migration commands Singh, Brijesh
                   ` (10 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

AMD SEV migration flow requires that target machine's public Diffie-Hellman
key (PDH) and certificate chain must be passed before initiating the guest
migration. User can use QMP 'migrate-set-parameters' to pass the certificate
chain. The certificate chain will be used while creating the outgoing
encryption context.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---

I was able to pass the certificate chain through the HMP but somehow
QMP socket interface is not working for me. If anyone has any tips on
what I am missing in the patch then please let me know. In meantime,
I will also continue my investigation on why its not working for me.

 migration/migration.c | 61 +++++++++++++++++++++++++++++++++++++++++++
 monitor/hmp-cmds.c    | 18 +++++++++++++
 qapi/migration.json   | 41 ++++++++++++++++++++++++++---
 3 files changed, 116 insertions(+), 4 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 8a607fe1e2..de66a0eb7e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -783,6 +783,12 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->announce_rounds = s->parameters.announce_rounds;
     params->has_announce_step = true;
     params->announce_step = s->parameters.announce_step;
+    params->has_sev_pdh = true;
+    params->sev_pdh = g_strdup(s->parameters.sev_pdh);
+    params->has_sev_plat_cert = true;
+    params->sev_plat_cert = g_strdup(s->parameters.sev_plat_cert);
+    params->has_sev_amd_cert = true;
+    params->sev_amd_cert = g_strdup(s->parameters.sev_amd_cert);
 
     return params;
 }
@@ -1289,6 +1295,18 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
     if (params->has_announce_step) {
         dest->announce_step = params->announce_step;
     }
+    if (params->has_sev_pdh) {
+        assert(params->sev_pdh->type == QTYPE_QSTRING);
+        dest->sev_pdh = g_strdup(params->sev_pdh->u.s);
+    }
+    if (params->has_sev_plat_cert) {
+        assert(params->sev_plat_cert->type == QTYPE_QSTRING);
+        dest->sev_plat_cert = g_strdup(params->sev_plat_cert->u.s);
+    }
+    if (params->has_sev_amd_cert) {
+        assert(params->sev_amd_cert->type == QTYPE_QSTRING);
+        dest->sev_amd_cert = g_strdup(params->sev_amd_cert->u.s);
+    }
 }
 
 static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
@@ -1390,6 +1408,21 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
     if (params->has_announce_step) {
         s->parameters.announce_step = params->announce_step;
     }
+    if (params->has_sev_pdh) {
+        g_free(s->parameters.sev_pdh);
+        assert(params->sev_pdh->type == QTYPE_QSTRING);
+        s->parameters.sev_pdh = g_strdup(params->sev_pdh->u.s);
+    }
+    if (params->has_sev_plat_cert) {
+        g_free(s->parameters.sev_plat_cert);
+        assert(params->sev_plat_cert->type == QTYPE_QSTRING);
+        s->parameters.sev_plat_cert = g_strdup(params->sev_plat_cert->u.s);
+    }
+    if (params->has_sev_amd_cert) {
+        g_free(s->parameters.sev_amd_cert);
+        assert(params->sev_amd_cert->type == QTYPE_QSTRING);
+        s->parameters.sev_amd_cert = g_strdup(params->sev_amd_cert->u.s);
+    }
 }
 
 void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
@@ -1410,6 +1443,27 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
         params->tls_hostname->type = QTYPE_QSTRING;
         params->tls_hostname->u.s = strdup("");
     }
+    /* TODO Rewrite "" to null instead */
+    if (params->has_sev_pdh
+        && params->sev_pdh->type == QTYPE_QNULL) {
+        qobject_unref(params->sev_pdh->u.n);
+        params->sev_pdh->type = QTYPE_QSTRING;
+        params->sev_pdh->u.s = strdup("");
+    }
+    /* TODO Rewrite "" to null instead */
+    if (params->has_sev_plat_cert
+        && params->sev_plat_cert->type == QTYPE_QNULL) {
+        qobject_unref(params->sev_plat_cert->u.n);
+        params->sev_plat_cert->type = QTYPE_QSTRING;
+        params->sev_plat_cert->u.s = strdup("");
+    }
+    /* TODO Rewrite "" to null instead */
+    if (params->has_sev_amd_cert
+        && params->sev_amd_cert->type == QTYPE_QNULL) {
+        qobject_unref(params->sev_amd_cert->u.n);
+        params->sev_amd_cert->type = QTYPE_QSTRING;
+        params->sev_amd_cert->u.s = strdup("");
+    }
 
     migrate_params_test_apply(params, &tmp);
 
@@ -3466,6 +3520,9 @@ static void migration_instance_finalize(Object *obj)
     qemu_mutex_destroy(&ms->qemu_file_lock);
     g_free(params->tls_hostname);
     g_free(params->tls_creds);
+    g_free(params->sev_pdh);
+    g_free(params->sev_plat_cert);
+    g_free(params->sev_amd_cert);
     qemu_sem_destroy(&ms->rate_limit_sem);
     qemu_sem_destroy(&ms->pause_sem);
     qemu_sem_destroy(&ms->postcopy_pause_sem);
@@ -3507,6 +3564,10 @@ static void migration_instance_init(Object *obj)
     params->has_announce_rounds = true;
     params->has_announce_step = true;
 
+    params->sev_pdh = g_strdup("");
+    params->sev_plat_cert = g_strdup("");
+    params->sev_amd_cert = g_strdup("");
+
     qemu_sem_init(&ms->postcopy_pause_sem, 0);
     qemu_sem_init(&ms->postcopy_pause_rp_sem, 0);
     qemu_sem_init(&ms->rp_state.rp_sem, 0);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 5ca3ebe942..354219f27a 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1872,6 +1872,24 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
         p->has_announce_step = true;
         visit_type_size(v, param, &p->announce_step, &err);
         break;
+    case MIGRATION_PARAMETER_SEV_PDH:
+        p->has_sev_pdh = true;
+        p->sev_pdh = g_new0(StrOrNull, 1);
+        p->sev_pdh->type = QTYPE_QSTRING;
+        visit_type_str(v, param, &p->sev_pdh->u.s, &err);
+        break;
+    case MIGRATION_PARAMETER_SEV_PLAT_CERT:
+        p->has_sev_plat_cert = true;
+        p->sev_plat_cert = g_new0(StrOrNull, 1);
+        p->sev_plat_cert->type = QTYPE_QSTRING;
+        visit_type_str(v, param, &p->sev_plat_cert->u.s, &err);
+        break;
+    case MIGRATION_PARAMETER_SEV_AMD_CERT:
+        p->has_sev_amd_cert = true;
+        p->sev_amd_cert = g_new0(StrOrNull, 1);
+        p->sev_amd_cert->type = QTYPE_QSTRING;
+        visit_type_str(v, param, &p->sev_amd_cert->u.s, &err);
+        break;
     default:
         assert(0);
     }
diff --git a/qapi/migration.json b/qapi/migration.json
index 9cfbaf8c6c..bb07995d2c 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -580,6 +580,15 @@
 # @max-cpu-throttle: maximum cpu throttle percentage.
 #                    Defaults to 99. (Since 3.1)
 #
+# @sev-pdh: The target host platform diffie-hellman key encoded in base64
+#           (Since 4.2)
+#
+# @sev-plat-cert: The target host platform certificate chain encoded in base64
+#                 (Since 4.2)
+#
+# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
+#                base64 (Since 4.2)
+#
 # Since: 2.4
 ##
 { 'enum': 'MigrationParameter',
@@ -592,7 +601,7 @@
            'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
            'multifd-channels',
            'xbzrle-cache-size', 'max-postcopy-bandwidth',
-           'max-cpu-throttle' ] }
+           'max-cpu-throttle', 'sev-pdh', 'sev-plat-cert', 'sev-amd-cert' ] }
 
 ##
 # @MigrateSetParameters:
@@ -682,6 +691,15 @@
 # @max-cpu-throttle: maximum cpu throttle percentage.
 #                    The default value is 99. (Since 3.1)
 #
+# @sev-pdh: The target host platform diffie-hellman key encoded in base64
+#           (Since 4.2)
+#
+# @sev-plat-cert: The target host platform certificate chain encoded in base64
+#                 (Since 4.2)
+#
+# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
+#                base64 (Since 4.2)
+#
 # Since: 2.4
 ##
 # TODO either fuse back into MigrationParameters, or make
@@ -707,7 +725,10 @@
             '*multifd-channels': 'int',
             '*xbzrle-cache-size': 'size',
             '*max-postcopy-bandwidth': 'size',
-	    '*max-cpu-throttle': 'int' } }
+            '*max-cpu-throttle': 'int',
+            '*sev-pdh':'StrOrNull',
+            '*sev-plat-cert': 'StrOrNull',
+            '*sev-amd-cert' : 'StrOrNull' } }
 
 ##
 # @migrate-set-parameters:
@@ -817,6 +838,15 @@
 #                    Defaults to 99.
 #                     (Since 3.1)
 #
+# @sev-pdh: The target host platform diffie-hellman key encoded in base64
+#           (Since 4.2)
+#
+# @sev-plat-cert: The target host platform certificate chain encoded in base64
+#                 (Since 4.2)
+#
+# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
+#                base64 (Since 4.2)
+#
 # Since: 2.4
 ##
 { 'struct': 'MigrationParameters',
@@ -839,8 +869,11 @@
             '*block-incremental': 'bool' ,
             '*multifd-channels': 'uint8',
             '*xbzrle-cache-size': 'size',
-	    '*max-postcopy-bandwidth': 'size',
-            '*max-cpu-throttle':'uint8'} }
+            '*max-postcopy-bandwidth': 'size',
+            '*max-cpu-throttle':'uint8',
+            '*sev-pdh':'str',
+            '*sev-plat-cert': 'str',
+            '*sev-amd-cert' : 'str'} }
 
 ##
 # @query-migrate-parameters:
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 04/14] linux-headers: update kernel header to include SEV migration commands
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (2 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 05/14] hw/machine: add helper to query the memory encryption state Singh, Brijesh
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 linux-headers/linux/kvm.h | 53 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index c8423e760c..2b0a2a97b8 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -492,6 +492,16 @@ struct kvm_dirty_log {
 	};
 };
 
+/* for KVM_GET_PAGE_ENC_BITMAP */
+struct kvm_page_enc_bitmap {
+        __u64 start_gfn;
+        __u64 num_pages;
+	union {
+		void *enc_bitmap; /* one bit per page */
+		__u64 padding2;
+	};
+};
+
 /* for KVM_CLEAR_DIRTY_LOG */
 struct kvm_clear_dirty_log {
 	__u32 slot;
@@ -1451,6 +1461,9 @@ struct kvm_enc_region {
 /* Available with KVM_CAP_ARM_SVE */
 #define KVM_ARM_VCPU_FINALIZE	  _IOW(KVMIO,  0xc2, int)
 
+#define KVM_GET_PAGE_ENC_BITMAP  	 _IOW(KVMIO, 0xc2, struct kvm_page_enc_bitmap)
+#define KVM_SET_PAGE_ENC_BITMAP  	 _IOW(KVMIO, 0xc3, struct kvm_page_enc_bitmap)
+
 /* Secure Encrypted Virtualization command */
 enum sev_cmd_id {
 	/* Guest initialization commands */
@@ -1531,6 +1544,46 @@ struct kvm_sev_dbg {
 	__u32 len;
 };
 
+struct kvm_sev_send_start {
+	__u32 policy;
+	__u64 pdh_cert_uaddr;
+	__u32 pdh_cert_len;
+	__u64 plat_cert_uaddr;
+	__u32 plat_cert_len;
+	__u64 amd_cert_uaddr;
+	__u32 amd_cert_len;
+	__u64 session_uaddr;
+	__u32 session_len;
+};
+
+struct kvm_sev_send_update_data {
+	__u64 hdr_uaddr;
+	__u32 hdr_len;
+	__u64 guest_uaddr;
+	__u32 guest_len;
+	__u64 trans_uaddr;
+	__u32 trans_len;
+};
+
+struct kvm_sev_receive_start {
+	__u32 handle;
+	__u32 policy;
+	__u64 pdh_uaddr;
+	__u32 pdh_len;
+	__u64 session_uaddr;
+	__u32 session_len;
+};
+
+struct kvm_sev_receive_update_data {
+	__u64 hdr_uaddr;
+	__u32 hdr_len;
+	__u64 guest_uaddr;
+	__u32 guest_len;
+	__u64 trans_uaddr;
+	__u32 trans_len;
+};
+
+
 #define KVM_DEV_ASSIGN_ENABLE_IOMMU	(1 << 0)
 #define KVM_DEV_ASSIGN_PCI_2_3		(1 << 1)
 #define KVM_DEV_ASSIGN_MASK_INTX	(1 << 2)
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 05/14] hw/machine: add helper to query the memory encryption state
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (3 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 04/14] linux-headers: update kernel header to include SEV migration commands Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-07 16:14   ` Dr. David Alan Gilbert
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 06/14] hw/machine: introduce MachineMemoryEncryptionOps for encrypted VMs Singh, Brijesh
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

To enable a memory encryption inside a VM, user must pass the object
name used for the encryption in command line parameter as shown below.

# $(QEMU) \
  -machine memory-encryption=<object_name>

Add a helper machine_memory_encryption_enabled() which will return a bool
indicating whether the encryption object has been specified in the command
line parameter.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 hw/core/machine.c   | 5 +++++
 include/hw/boards.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index c58a8e594e..f1e1b3661f 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -1031,6 +1031,11 @@ bool machine_mem_merge(MachineState *machine)
     return machine->mem_merge;
 }
 
+bool machine_memory_encryption_enabled(MachineState *machine)
+{
+    return machine->memory_encryption ? true : false;
+}
+
 static char *cpu_slot_to_string(const CPUArchId *cpu)
 {
     GString *s = g_string_new(NULL);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index a71d1a53a5..c5446a39cf 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -76,6 +76,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
                                Error **errp);
 
 void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char *type);
+bool machine_memory_encryption_enabled(MachineState *machine);
 
 
 /**
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 06/14] hw/machine: introduce MachineMemoryEncryptionOps for encrypted VMs
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (4 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 05/14] hw/machine: add helper to query the memory encryption state Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-07 16:36   ` Dr. David Alan Gilbert
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 08/14] target/i386: sev: do not create launch context for an incoming guest Singh, Brijesh
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

When memory encryption is enabled in VM, the guest RAM will be encrypted
with the guest-specific key, to protect the confidentiality of data while
in transit we need to platform specific hooks to save or migrate the
guest RAM. The MemoryEncryptionOps introduced in this patch will be later
used by the migration.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 include/hw/boards.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index c5446a39cf..ba80c236fe 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -105,6 +105,29 @@ typedef struct {
     CPUArchId cpus[0];
 } CPUArchIdList;
 
+/**
+ * The functions registers with MachineMemoryEncryptionOps will be used during
+ * the encrypted guest migration.
+ */
+struct MachineMemoryEncryptionOps {
+    /* Initialize the platform specific state before starting the migration */
+    int (*save_setup)(const char *pdh, const char *plat_cert,
+                      const char *amd_cert);
+
+    /* Write the encrypted page and metadata associated with it */
+    int (*save_outgoing_page)(QEMUFile *f, uint8_t *ptr, uint32_t size,
+                              uint64_t *bytes_sent);
+
+    /* Load the incoming encrypted page into guest memory */
+    int (*load_incoming_page)(QEMUFile *f, uint8_t *ptr);
+
+    /* Write the page encryption state bitmap */
+    int (*save_outgoing_bitmap)(QEMUFile *f);
+
+    /* Load the incoming page encryption bitmap */
+    int (*load_incoming_bitmap)(QEMUFile *f);
+};
+
 /**
  * MachineClass:
  * @deprecation_reason: If set, the machine is marked as deprecated. The
@@ -228,6 +251,7 @@ struct MachineClass {
                                                          unsigned cpu_index);
     const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
     int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
+    struct MachineMemoryEncryptionOps *memory_encryption_ops;
 };
 
 /**
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 08/14] target/i386: sev: do not create launch context for an incoming guest
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (5 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 06/14] hw/machine: introduce MachineMemoryEncryptionOps for encrypted VMs Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 07/14] target/i386: sev: provide callback to setup outgoing context Singh, Brijesh
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

The LAUNCH_START is used for creating an encryption context to encrypt
newly created guest, for an incoming guest the RECEIVE_START should be
used.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 target/i386/sev.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 70e9d86815..483d9bb0fa 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -845,10 +845,16 @@ sev_guest_init(const char *id)
         goto err;
     }
 
-    ret = sev_launch_start(s);
-    if (ret) {
-        error_report("%s: failed to create encryption context", __func__);
-        goto err;
+    /*
+     * The LAUNCH context is used for new guest, if its an incoming guest
+     * then RECEIVE context will be created after the connection is established.
+     */
+    if (!runstate_check(RUN_STATE_INMIGRATE)) {
+        ret = sev_launch_start(s);
+        if (ret) {
+            error_report("%s: failed to create encryption context", __func__);
+            goto err;
+        }
     }
 
     ram_block_notifier_add(&sev_ram_notifier);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 07/14] target/i386: sev: provide callback to setup outgoing context
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (6 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 08/14] target/i386: sev: do not create launch context for an incoming guest Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-08 11:19   ` Dr. David Alan Gilbert
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 09/14] target/i386: sev: add support to encrypt the outgoing page Singh, Brijesh
                   ` (5 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

The user provides the target machine's Platform Diffie-Hellman key (PDH)
and certificate chain before starting the SEV guest migration. Cache the
certificate chain as we need them while creating the outgoing context.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 accel/kvm/kvm-all.c    | 12 +++++++++++
 accel/kvm/sev-stub.c   |  6 ++++++
 include/sysemu/sev.h   |  2 ++
 target/i386/sev.c      | 45 ++++++++++++++++++++++++++++++++++++++++++
 target/i386/sev_i386.h |  6 ++++++
 5 files changed, 71 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f450f25295..d0304c6947 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -165,6 +165,17 @@ bool kvm_memcrypt_enabled(void)
     return false;
 }
 
+static int kvm_memcrypt_save_setup(const char *pdh, const char *plat_cert,
+                                   const char *amd_cert)
+{
+    return sev_save_setup(kvm_state->memcrypt_handle, pdh,
+                          plat_cert, amd_cert);
+}
+
+static struct MachineMemoryEncryptionOps sev_memory_encryption_ops = {
+    .save_setup = kvm_memcrypt_save_setup,
+};
+
 int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len)
 {
     if (kvm_state->memcrypt_handle &&
@@ -1968,6 +1979,7 @@ static int kvm_init(MachineState *ms)
         }
 
         kvm_state->memcrypt_encrypt_data = sev_encrypt_data;
+        mc->memory_encryption_ops = &sev_memory_encryption_ops;
     }
 
     ret = kvm_arch_init(ms, s);
diff --git a/accel/kvm/sev-stub.c b/accel/kvm/sev-stub.c
index 4f97452585..528f8cf7f1 100644
--- a/accel/kvm/sev-stub.c
+++ b/accel/kvm/sev-stub.c
@@ -24,3 +24,9 @@ void *sev_guest_init(const char *id)
 {
     return NULL;
 }
+
+int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
+                   const char *amd_cert)
+{
+    return 1;
+}
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index 98c1ec8d38..d5123d4fa3 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_save_setup(void *handle, const char *pdh, const char *plat_cert,
+                   const char *amd_cert);
 #endif
diff --git a/target/i386/sev.c b/target/i386/sev.c
index f1423cb0c0..70e9d86815 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -27,6 +27,7 @@
 #include "sysemu/sysemu.h"
 #include "trace.h"
 #include "migration/blocker.h"
+#include "migration/qemu-file.h"
 
 #define DEFAULT_GUEST_POLICY    0x1 /* disable debug */
 #define DEFAULT_SEV_DEVICE      "/dev/sev"
@@ -62,6 +63,8 @@ static const char *const sev_fw_errlist[] = {
 
 #define SEV_FW_MAX_ERROR      ARRAY_SIZE(sev_fw_errlist)
 
+#define SEV_FW_BLOB_MAX_SIZE            0x4000          /* 16KB */
+
 static int
 sev_ioctl(int fd, int cmd, void *data, int *error)
 {
@@ -729,6 +732,48 @@ sev_vm_state_change(void *opaque, int running, RunState state)
     }
 }
 
+static inline bool check_blob_length(size_t value)
+{
+    if (value > SEV_FW_BLOB_MAX_SIZE) {
+        error_report("invalid length max=%ld got=%d",
+                     value, SEV_FW_BLOB_MAX_SIZE);
+        return false;
+    }
+
+    return true;
+}
+
+int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
+                   const char *amd_cert)
+{
+    SEVState *s = (SEVState *)handle;
+
+    s->remote_pdh = g_base64_decode(pdh, &s->remote_pdh_len);
+    if (!check_blob_length(s->remote_pdh_len)) {
+        goto error;
+    }
+
+    s->remote_plat_cert = g_base64_decode(plat_cert,
+                                          &s->remote_plat_cert_len);
+    if (!check_blob_length(s->remote_plat_cert_len)) {
+        goto error;
+    }
+
+    s->amd_cert = g_base64_decode(amd_cert, &s->amd_cert_len);
+    if (!check_blob_length(s->amd_cert_len)) {
+        goto error;
+    }
+
+    return 0;
+
+error:
+    g_free(s->remote_pdh);
+    g_free(s->remote_plat_cert);
+    g_free(s->amd_cert);
+
+    return 1;
+}
+
 void *
 sev_guest_init(const char *id)
 {
diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 55313441ae..32906de998 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -81,6 +81,12 @@ struct SEVState {
     int sev_fd;
     SevState state;
     gchar *measurement;
+    guchar *remote_pdh;
+    size_t remote_pdh_len;
+    guchar *remote_plat_cert;
+    size_t remote_plat_cert_len;
+    guchar *amd_cert;
+    size_t amd_cert_len;
 };
 
 typedef struct SEVState SEVState;
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 09/14] target/i386: sev: add support to encrypt the outgoing page
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (7 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 07/14] target/i386: sev: provide callback to setup outgoing context Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-09 18:54   ` Dr. David Alan Gilbert
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 10/14] target/i386: sev: add support to load incoming encrypted page Singh, Brijesh
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

The sev_save_outgoing_page() provide the implementation to encrypt the
guest private pages during the transit. The routines uses the SEND_START
command to create the outgoing encryption context on the first call then
uses the SEND_UPDATE_DATA command to encrypt the data before writing it
to the socket. While encrypting the data SEND_UPDATE_DATA produces some
metadata (e.g MAC, IV). The metadata is also sent to the target machine.
After migration is completed, we issue the SEND_FINISH command to transition
the SEV guest state from sending to unrunnable state.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 accel/kvm/kvm-all.c      |   9 ++
 accel/kvm/sev-stub.c     |   6 ++
 include/sysemu/sev.h     |   2 +
 target/i386/sev.c        | 216 +++++++++++++++++++++++++++++++++++++++
 target/i386/sev_i386.h   |   2 +
 target/i386/trace-events |   3 +
 6 files changed, 238 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index d0304c6947..a5b0ae9363 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -172,8 +172,17 @@ static int kvm_memcrypt_save_setup(const char *pdh, const char *plat_cert,
                           plat_cert, amd_cert);
 }
 
+static int kvm_memcrypt_save_outgoing_page(QEMUFile *f, uint8_t *ptr,
+                                           uint32_t size,
+                                           uint64_t *bytes_sent)
+{
+    return sev_save_outgoing_page(kvm_state->memcrypt_handle, f, ptr, size,
+                                  bytes_sent);
+}
+
 static struct MachineMemoryEncryptionOps sev_memory_encryption_ops = {
     .save_setup = kvm_memcrypt_save_setup,
+    .save_outgoing_page = kvm_memcrypt_save_outgoing_page,
 };
 
 int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len)
diff --git a/accel/kvm/sev-stub.c b/accel/kvm/sev-stub.c
index 528f8cf7f1..51b17b8141 100644
--- a/accel/kvm/sev-stub.c
+++ b/accel/kvm/sev-stub.c
@@ -30,3 +30,9 @@ int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
 {
     return 1;
 }
+
+int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
+                           uint32_t size, uint64_t *bytes_sent)
+{
+    return 1;
+}
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index d5123d4fa3..f06fd203cd 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -20,4 +20,6 @@ void *sev_guest_init(const char *id);
 int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
 int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
                    const char *amd_cert);
+int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
+                           uint32_t size, uint64_t *bytes_sent);
 #endif
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 483d9bb0fa..1820c62a71 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -28,6 +28,7 @@
 #include "trace.h"
 #include "migration/blocker.h"
 #include "migration/qemu-file.h"
+#include "migration/misc.h"
 
 #define DEFAULT_GUEST_POLICY    0x1 /* disable debug */
 #define DEFAULT_SEV_DEVICE      "/dev/sev"
@@ -774,6 +775,40 @@ error:
     return 1;
 }
 
+static void
+sev_send_finish(void)
+{
+    int ret, error;
+
+    trace_kvm_sev_send_finish();
+    ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_SEND_FINISH, 0, &error);
+    if (ret) {
+        error_report("%s: SEND_FINISH ret=%d fw_error=%d '%s'",
+                     __func__, ret, error, fw_error_to_str(error));
+    }
+
+    g_free(sev_state->send_packet_hdr);
+    sev_set_guest_state(SEV_STATE_RUNNING);
+}
+
+static void
+sev_migration_state_notifier(Notifier *notifier, void *data)
+{
+    MigrationState *s = data;
+
+    if (migration_has_finished(s) ||
+        migration_in_postcopy_after_devices(s) ||
+        migration_has_failed(s)) {
+        if (sev_check_state(SEV_STATE_SEND_UPDATE)) {
+            sev_send_finish();
+        }
+    }
+}
+
+static Notifier sev_migration_state_notify = {
+    .notify = sev_migration_state_notifier,
+};
+
 void *
 sev_guest_init(const char *id)
 {
@@ -860,6 +895,7 @@ sev_guest_init(const char *id)
     ram_block_notifier_add(&sev_ram_notifier);
     qemu_add_machine_init_done_notifier(&sev_machine_done_notify);
     qemu_add_vm_change_state_handler(sev_vm_state_change, s);
+    add_migration_state_change_notifier(&sev_migration_state_notify);
 
     return s;
 err:
@@ -881,6 +917,186 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
     return 0;
 }
 
+static int
+sev_get_send_session_length(void)
+{
+    int ret, fw_err = 0;
+    struct kvm_sev_send_start start = {};
+
+    ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_SEND_START, &start, &fw_err);
+    if (fw_err != SEV_RET_INVALID_LEN) {
+        ret = -1;
+        error_report("%s: failed to get session length ret=%d fw_error=%d '%s'",
+                     __func__, ret, fw_err, fw_error_to_str(fw_err));
+        goto err;
+    }
+
+    ret = start.session_len;
+err:
+    return ret;
+}
+
+static int
+sev_send_start(SEVState *s, QEMUFile *f, uint64_t *bytes_sent)
+{
+    gsize pdh_len = 0, plat_cert_len;
+    int session_len, ret, fw_error;
+    struct kvm_sev_send_start start = { };
+    guchar *pdh = NULL, *plat_cert = NULL, *session = NULL;
+
+    if (!s->remote_pdh || !s->remote_plat_cert || !s->amd_cert_len) {
+        error_report("%s: missing remote PDH or PLAT_CERT", __func__);
+        return 1;
+    }
+
+    start.pdh_cert_uaddr = (uintptr_t) s->remote_pdh;
+    start.pdh_cert_len = s->remote_pdh_len;
+
+    start.plat_cert_uaddr = (uintptr_t)s->remote_plat_cert;
+    start.plat_cert_len = s->remote_plat_cert_len;
+
+    start.amd_cert_uaddr = (uintptr_t)s->amd_cert;
+    start.amd_cert_len = s->amd_cert_len;
+
+    /* get the session length */
+    session_len = sev_get_send_session_length();
+    if (session_len < 0) {
+        ret = 1;
+        goto err;
+    }
+
+    session = g_new0(guchar, session_len);
+    start.session_uaddr = (unsigned long)session;
+    start.session_len = session_len;
+
+    /* Get our PDH certificate */
+    ret = sev_get_pdh_info(s->sev_fd, &pdh, &pdh_len,
+                           &plat_cert, &plat_cert_len);
+    if (ret) {
+        error_report("Failed to get our PDH cert");
+        goto err;
+    }
+
+    trace_kvm_sev_send_start(start.pdh_cert_uaddr, start.pdh_cert_len,
+                             start.plat_cert_uaddr, start.plat_cert_len,
+                             start.amd_cert_uaddr, start.amd_cert_len);
+
+    ret = sev_ioctl(s->sev_fd, KVM_SEV_SEND_START, &start, &fw_error);
+    if (ret < 0) {
+        error_report("%s: SEND_START ret=%d fw_error=%d '%s'",
+                __func__, ret, fw_error, fw_error_to_str(fw_error));
+        goto err;
+    }
+
+    qemu_put_be32(f, start.policy);
+    qemu_put_be32(f, pdh_len);
+    qemu_put_buffer(f, (uint8_t *)pdh, pdh_len);
+    qemu_put_be32(f, start.session_len);
+    qemu_put_buffer(f, (uint8_t *)start.session_uaddr, start.session_len);
+    *bytes_sent = 12 + pdh_len + start.session_len;
+
+    sev_set_guest_state(SEV_STATE_SEND_UPDATE);
+
+err:
+    g_free(pdh);
+    g_free(plat_cert);
+    return ret;
+}
+
+static int
+sev_send_get_packet_len(int *fw_err)
+{
+    int ret;
+    struct kvm_sev_send_update_data update = {};
+
+    ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_SEND_UPDATE_DATA,
+                    &update, fw_err);
+    if (*fw_err != SEV_RET_INVALID_LEN) {
+        ret = -1;
+        error_report("%s: failed to get session length ret=%d fw_error=%d '%s'",
+                    __func__, ret, *fw_err, fw_error_to_str(*fw_err));
+        goto err;
+    }
+
+    ret = update.hdr_len;
+
+err:
+    return ret;
+}
+
+static int
+sev_send_update_data(SEVState *s, QEMUFile *f, uint8_t *ptr, uint32_t size,
+                     uint64_t *bytes_sent)
+{
+    int ret, fw_error;
+    guchar *trans;
+    struct kvm_sev_send_update_data update = { };
+
+    /*
+     * If this is first call then query the packet header bytes and allocate
+     * the packet buffer.
+     */
+    if (!s->send_packet_hdr) {
+        s->send_packet_hdr_len = sev_send_get_packet_len(&fw_error);
+        if (s->send_packet_hdr_len < 1) {
+            error_report("%s: SEND_UPDATE fw_error=%d '%s'",
+                    __func__, fw_error, fw_error_to_str(fw_error));
+            return 1;
+        }
+
+        s->send_packet_hdr = g_new(gchar, s->send_packet_hdr_len);
+    }
+
+    /* allocate transport buffer */
+    trans = g_new(guchar, size);
+
+    update.hdr_uaddr = (uintptr_t)s->send_packet_hdr;
+    update.hdr_len = s->send_packet_hdr_len;
+    update.guest_uaddr = (uintptr_t)ptr;
+    update.guest_len = size;
+    update.trans_uaddr = (uintptr_t)trans;
+    update.trans_len = size;
+
+    trace_kvm_sev_send_update_data(ptr, trans, size);
+
+    ret = sev_ioctl(s->sev_fd, KVM_SEV_SEND_UPDATE_DATA, &update, &fw_error);
+    if (ret) {
+        error_report("%s: SEND_UPDATE_DATA ret=%d fw_error=%d '%s'",
+                __func__, ret, fw_error, fw_error_to_str(fw_error));
+        goto err;
+    }
+
+    qemu_put_be32(f, update.hdr_len);
+    qemu_put_buffer(f, (uint8_t *)update.hdr_uaddr, update.hdr_len);
+    *bytes_sent = 4 + update.hdr_len;
+
+    qemu_put_be32(f, update.trans_len);
+    qemu_put_buffer(f, (uint8_t *)update.trans_uaddr, update.trans_len);
+    *bytes_sent += (4 + update.trans_len);
+
+err:
+    g_free(trans);
+    return ret;
+}
+
+int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
+                           uint32_t sz, uint64_t *bytes_sent)
+{
+    SEVState *s = sev_state;
+
+    /*
+     * If this is a first buffer then create outgoing encryption context
+     * and write our PDH, policy and session data.
+     */
+    if (!sev_check_state(SEV_STATE_SEND_UPDATE) &&
+        sev_send_start(s, f, bytes_sent)) {
+        error_report("Failed to create outgoing context");
+        return 1;
+    }
+
+    return sev_send_update_data(s, f, ptr, sz, bytes_sent);
+}
+
 static void
 sev_register_types(void)
 {
diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 32906de998..e475304f5f 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -87,6 +87,8 @@ struct SEVState {
     size_t remote_plat_cert_len;
     guchar *amd_cert;
     size_t amd_cert_len;
+    gchar *send_packet_hdr;
+    size_t send_packet_hdr_len;
 };
 
 typedef struct SEVState SEVState;
diff --git a/target/i386/trace-events b/target/i386/trace-events
index 789c700d4a..b41516cf9f 100644
--- a/target/i386/trace-events
+++ b/target/i386/trace-events
@@ -15,3 +15,6 @@ kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session
 kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIu64
 kvm_sev_launch_measurement(const char *value) "data %s"
 kvm_sev_launch_finish(void) ""
+kvm_sev_send_start(uint64_t pdh, int l1, uint64_t plat, int l2, uint64_t amd, int l3) "pdh 0x%" PRIx64 " len %d plat 0x%" PRIx64 " len %d amd 0x%" PRIx64 " len %d"
+kvm_sev_send_update_data(void *src, void *dst, int len) "guest %p trans %p len %d"
+kvm_sev_send_finish(void) ""
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 10/14] target/i386: sev: add support to load incoming encrypted page
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (8 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 09/14] target/i386: sev: add support to encrypt the outgoing page Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-13 17:38   ` Dr. David Alan Gilbert
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page encryption bitmap Singh, Brijesh
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

The sev_load_incoming_page() provide the implementation to read the
incoming guest private pages from the socket and load it into the guest
memory. The routines uses the RECEIVE_START command to create the
incoming encryption context on the first call then uses the
RECEIEVE_UPDATE_DATA command to load the encrypted pages into the guest
memory. After migration is completed, we issue the RECEIVE_FINISH command
to transition the SEV guest to the runnable state so that it can be
executed.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 accel/kvm/kvm-all.c      |   6 ++
 accel/kvm/sev-stub.c     |   5 ++
 include/sysemu/sev.h     |   1 +
 target/i386/sev.c        | 137 ++++++++++++++++++++++++++++++++++++++-
 target/i386/trace-events |   3 +
 5 files changed, 151 insertions(+), 1 deletion(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index a5b0ae9363..ba0e7fa2be 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -180,9 +180,15 @@ static int kvm_memcrypt_save_outgoing_page(QEMUFile *f, uint8_t *ptr,
                                   bytes_sent);
 }
 
+static int kvm_memcrypt_load_incoming_page(QEMUFile *f, uint8_t *ptr)
+{
+    return sev_load_incoming_page(kvm_state->memcrypt_handle, f, ptr);
+}
+
 static struct MachineMemoryEncryptionOps sev_memory_encryption_ops = {
     .save_setup = kvm_memcrypt_save_setup,
     .save_outgoing_page = kvm_memcrypt_save_outgoing_page,
+    .load_incoming_page = kvm_memcrypt_load_incoming_page,
 };
 
 int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len)
diff --git a/accel/kvm/sev-stub.c b/accel/kvm/sev-stub.c
index 51b17b8141..1b6773ef72 100644
--- a/accel/kvm/sev-stub.c
+++ b/accel/kvm/sev-stub.c
@@ -36,3 +36,8 @@ int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
 {
     return 1;
 }
+
+int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr)
+{
+    return 1;
+}
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index f06fd203cd..e9371bd2dd 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -22,4 +22,5 @@ int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
                    const char *amd_cert);
 int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
                            uint32_t size, uint64_t *bytes_sent);
+int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr);
 #endif
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 1820c62a71..a689011991 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -721,13 +721,34 @@ sev_launch_finish(SEVState *s)
     }
 }
 
+static int
+sev_receive_finish(SEVState *s)
+{
+    int error, ret = 1;
+
+    trace_kvm_sev_receive_finish();
+    ret = sev_ioctl(s->sev_fd, KVM_SEV_RECEIVE_FINISH, 0, &error);
+    if (ret) {
+        error_report("%s: RECEIVE_FINISH ret=%d fw_error=%d '%s'",
+                __func__, ret, error, fw_error_to_str(error));
+        goto err;
+    }
+
+    sev_set_guest_state(SEV_STATE_RUNNING);
+err:
+    return ret;
+}
+
+
 static void
 sev_vm_state_change(void *opaque, int running, RunState state)
 {
     SEVState *s = opaque;
 
     if (running) {
-        if (!sev_check_state(SEV_STATE_RUNNING)) {
+        if (sev_check_state(SEV_STATE_RECEIVE_UPDATE)) {
+            sev_receive_finish(s);
+        } else if (!sev_check_state(SEV_STATE_RUNNING)) {
             sev_launch_finish(s);
         }
     }
@@ -1097,6 +1118,120 @@ int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
     return sev_send_update_data(s, f, ptr, sz, bytes_sent);
 }
 
+static int
+sev_receive_start(QSevGuestInfo *sev, QEMUFile *f)
+{
+    int ret = 1;
+    int fw_error;
+    struct kvm_sev_receive_start start = { };
+    gchar *session = NULL, *pdh_cert = NULL;
+
+    /* get SEV guest handle */
+    start.handle = object_property_get_int(OBJECT(sev), "handle",
+                                           &error_abort);
+
+    /* get the source policy */
+    start.policy = qemu_get_be32(f);
+
+    /* get source PDH key */
+    start.pdh_len = qemu_get_be32(f);
+    if (!check_blob_length(start.pdh_len)) {
+        return 1;
+    }
+
+    pdh_cert = g_new(gchar, start.pdh_len);
+    qemu_get_buffer(f, (uint8_t *)pdh_cert, start.pdh_len);
+    start.pdh_uaddr = (uintptr_t)pdh_cert;
+
+    /* get source session data */
+    start.session_len = qemu_get_be32(f);
+    if (!check_blob_length(start.session_len)) {
+        return 1;
+    }
+    session = g_new(gchar, start.session_len);
+    qemu_get_buffer(f, (uint8_t *)session, start.session_len);
+    start.session_uaddr = (uintptr_t)session;
+
+    trace_kvm_sev_receive_start(start.policy, session, pdh_cert);
+
+    ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_RECEIVE_START,
+                    &start, &fw_error);
+    if (ret < 0) {
+        error_report("Error RECEIVE_START ret=%d fw_error=%d '%s'",
+                ret, fw_error, fw_error_to_str(fw_error));
+        goto err;
+    }
+
+    object_property_set_int(OBJECT(sev), start.handle, "handle", &error_abort);
+    sev_set_guest_state(SEV_STATE_RECEIVE_UPDATE);
+err:
+    g_free(session);
+    g_free(pdh_cert);
+
+    return ret;
+}
+
+static int sev_receive_update_data(QEMUFile *f, uint8_t *ptr)
+{
+    int ret = 1, fw_error = 0;
+    gchar *hdr = NULL, *trans = NULL;
+    struct kvm_sev_receive_update_data update = {};
+
+    /* get packet header */
+    update.hdr_len = qemu_get_be32(f);
+    if (!check_blob_length(update.hdr_len)) {
+        return 1;
+    }
+
+    hdr = g_new(gchar, update.hdr_len);
+    qemu_get_buffer(f, (uint8_t *)hdr, update.hdr_len);
+    update.hdr_uaddr = (uintptr_t)hdr;
+
+    /* get transport buffer */
+    update.trans_len = qemu_get_be32(f);
+    if (!check_blob_length(update.trans_len)) {
+        goto err;
+    }
+
+    trans = g_new(gchar, update.trans_len);
+    update.trans_uaddr = (uintptr_t)trans;
+    qemu_get_buffer(f, (uint8_t *)update.trans_uaddr, update.trans_len);
+
+    update.guest_uaddr = (uintptr_t) ptr;
+    update.guest_len = update.trans_len;
+
+    trace_kvm_sev_receive_update_data(trans, ptr, update.guest_len,
+            hdr, update.hdr_len);
+
+    ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_RECEIVE_UPDATE_DATA,
+                    &update, &fw_error);
+    if (ret) {
+        error_report("Error RECEIVE_UPDATE_DATA ret=%d fw_error=%d '%s'",
+                ret, fw_error, fw_error_to_str(fw_error));
+        goto err;
+    }
+err:
+    g_free(trans);
+    g_free(hdr);
+    return ret;
+}
+
+int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr)
+{
+    SEVState *s = (SEVState *)handle;
+
+    /*
+     * If this is first buffer and SEV is not in recieiving state then
+     * use RECEIVE_START command to create a encryption context.
+     */
+    if (!sev_check_state(SEV_STATE_RECEIVE_UPDATE) &&
+        sev_receive_start(s->sev_info, f)) {
+        return 1;
+    }
+
+    return sev_receive_update_data(f, ptr);
+}
+
 static void
 sev_register_types(void)
 {
diff --git a/target/i386/trace-events b/target/i386/trace-events
index b41516cf9f..609752cca7 100644
--- a/target/i386/trace-events
+++ b/target/i386/trace-events
@@ -18,3 +18,6 @@ kvm_sev_launch_finish(void) ""
 kvm_sev_send_start(uint64_t pdh, int l1, uint64_t plat, int l2, uint64_t amd, int l3) "pdh 0x%" PRIx64 " len %d plat 0x%" PRIx64 " len %d amd 0x%" PRIx64 " len %d"
 kvm_sev_send_update_data(void *src, void *dst, int len) "guest %p trans %p len %d"
 kvm_sev_send_finish(void) ""
+kvm_sev_receive_start(int policy, void *session, void *pdh) "policy 0x%x session %p pdh %p"
+kvm_sev_receive_update_data(void *src, void *dst, int len, void *hdr, int hdr_len) "guest %p trans %p len %d hdr %p hdr_len %d"
+kvm_sev_receive_finish(void) ""
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page encryption bitmap
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (9 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 10/14] target/i386: sev: add support to load incoming encrypted page Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-13 18:57   ` Dr. David Alan Gilbert
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 13/14] migration/ram: add support to send encrypted pages Singh, Brijesh
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

When memory encryption is enabled, the hypervisor maintains a page
encryption bitmap which is referred by hypervisor during migratoin to check
if page is private or shared. The bitmap is built during the VM bootup and
must be migrated to the target host so that hypervisor on target host can
use it for future migration. The KVM_{SET,GET}_PAGE_ENC_BITMAP can be used
to get and set the bitmap for a given gfn range.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 accel/kvm/kvm-all.c      | 27 ++++++++++++
 accel/kvm/sev-stub.c     | 11 +++++
 include/sysemu/sev.h     |  6 +++
 target/i386/sev.c        | 93 ++++++++++++++++++++++++++++++++++++++++
 target/i386/trace-events |  2 +
 5 files changed, 139 insertions(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index ba0e7fa2be..f4d136b022 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -185,10 +185,37 @@ static int kvm_memcrypt_load_incoming_page(QEMUFile *f, uint8_t *ptr)
     return sev_load_incoming_page(kvm_state->memcrypt_handle, f, ptr);
 }
 
+static int kvm_memcrypt_save_outgoing_bitmap(QEMUFile *f)
+{
+    KVMMemoryListener *kml = &kvm_state->memory_listener;
+    KVMState *s = kvm_state;
+    int ret = 1, i;
+
+    /* iterate through all the registered slots and send the bitmap */
+    for (i = 0; i < s->nr_slots; i++) {
+        KVMSlot *mem = &kml->slots[i];
+        ret = sev_save_outgoing_bitmap(s->memcrypt_handle, f, mem->start_addr,
+                                       mem->memory_size,
+                                       (i + 1) == s->nr_slots);
+        if (ret) {
+            return 1;
+        }
+    }
+
+    return ret;
+}
+
+static int kvm_memcrypt_load_incoming_bitmap(QEMUFile *f)
+{
+    return sev_load_incoming_bitmap(kvm_state->memcrypt_handle, f);
+}
+
 static struct MachineMemoryEncryptionOps sev_memory_encryption_ops = {
     .save_setup = kvm_memcrypt_save_setup,
     .save_outgoing_page = kvm_memcrypt_save_outgoing_page,
     .load_incoming_page = kvm_memcrypt_load_incoming_page,
+    .save_outgoing_bitmap = kvm_memcrypt_save_outgoing_bitmap,
+    .load_incoming_bitmap = kvm_memcrypt_load_incoming_bitmap,
 };
 
 int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len)
diff --git a/accel/kvm/sev-stub.c b/accel/kvm/sev-stub.c
index 1b6773ef72..fa96225abc 100644
--- a/accel/kvm/sev-stub.c
+++ b/accel/kvm/sev-stub.c
@@ -41,3 +41,14 @@ int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr)
 {
     return 1;
 }
+
+int sev_save_outgoing_bitmap(void *handle, QEMUFile *f,
+                             unsigned long start, uint64_t length, bool last)
+{
+    return 1;
+}
+
+int sev_load_incoming_bitmap(void *handle, QEMUFile *f)
+{
+    return 1;
+}
diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
index e9371bd2dd..f777083c94 100644
--- a/include/sysemu/sev.h
+++ b/include/sysemu/sev.h
@@ -16,6 +16,9 @@
 
 #include "sysemu/kvm.h"
 
+#define RAM_SAVE_ENCRYPTED_PAGE        0x1
+#define RAM_SAVE_ENCRYPTED_BITMAP      0x2
+
 void *sev_guest_init(const char *id);
 int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
 int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
@@ -23,4 +26,7 @@ int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
 int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
                            uint32_t size, uint64_t *bytes_sent);
 int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr);
+int sev_load_incoming_bitmap(void *handle, QEMUFile *f);
+int sev_save_outgoing_bitmap(void *handle, QEMUFile *f, unsigned long start,
+                             uint64_t length, bool last);
 #endif
diff --git a/target/i386/sev.c b/target/i386/sev.c
index a689011991..9d643e720c 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -65,6 +65,8 @@ static const char *const sev_fw_errlist[] = {
 #define SEV_FW_MAX_ERROR      ARRAY_SIZE(sev_fw_errlist)
 
 #define SEV_FW_BLOB_MAX_SIZE            0x4000          /* 16KB */
+#define ENCRYPTED_BITMAP_CONTINUE       0x1
+#define ENCRYPTED_BITMAP_END            0x2
 
 static int
 sev_ioctl(int fd, int cmd, void *data, int *error)
@@ -1232,6 +1234,97 @@ int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr)
     return sev_receive_update_data(f, ptr);
 }
 
+#define ALIGN(x, y)  (((x) + (y) - 1) & ~((y) - 1))
+
+int sev_load_incoming_bitmap(void *handle, QEMUFile *f)
+{
+    void *bmap;
+    unsigned long bmap_size, base_gpa;
+    unsigned long npages, expected_size, length;
+    struct kvm_page_enc_bitmap e = {};
+    int status;
+
+    status = qemu_get_be32(f);
+
+    while (status != ENCRYPTED_BITMAP_END) {
+        base_gpa = qemu_get_be64(f);
+        npages = qemu_get_be64(f);
+        bmap_size = qemu_get_be64(f);
+
+        /*
+         * Before allocating the bitmap buffer, lets do some bound check to
+         * ensure that we are not dealing with corrupted stream.
+         */
+        length = npages << TARGET_PAGE_BITS;
+        expected_size = ALIGN((length >> TARGET_PAGE_BITS), 64) / 8;
+        if (expected_size != bmap_size) {
+            error_report("corrupted bitmap expected size %ld got %ld",
+                    expected_size, bmap_size);
+            return 1;
+        }
+
+        bmap = g_malloc0(bmap_size);
+        qemu_get_buffer(f, (uint8_t *)bmap, bmap_size);
+
+        trace_kvm_sev_load_bitmap(base_gpa, npages << TARGET_PAGE_BITS);
+
+        e.start_gfn = base_gpa >> TARGET_PAGE_BITS;
+        e.num_pages = npages;
+        e.enc_bitmap = bmap;
+        if (kvm_vm_ioctl(kvm_state, KVM_SET_PAGE_ENC_BITMAP, &e) == -1) {
+            error_report("KVM_SET_PAGE_ENC_BITMAP ioctl failed %d", errno);
+            g_free(bmap);
+            return 1;
+        }
+
+        g_free(bmap);
+
+        status = qemu_get_be32(f);
+    }
+
+    return 0;
+}
+
+int sev_save_outgoing_bitmap(void *handle, QEMUFile *f,
+                             unsigned long start, uint64_t length, bool last)
+{
+    uint64_t size;
+    struct kvm_page_enc_bitmap e = {};
+
+    if (!length) {
+        /* nothing to send */
+        goto done;
+    }
+
+    size = ALIGN((length >> TARGET_PAGE_BITS), 64) / 8;
+    e.enc_bitmap = g_malloc0(size);
+    e.start_gfn = start >> TARGET_PAGE_BITS;
+    e.num_pages = length >> TARGET_PAGE_BITS;
+
+    trace_kvm_sev_save_bitmap(start, length);
+
+    if (kvm_vm_ioctl(kvm_state, KVM_GET_PAGE_ENC_BITMAP, &e) == -1) {
+        error_report("%s: KVM_GET_PAGE_ENC_BITMAP ioctl failed %d",
+                    __func__, errno);
+        g_free(e.enc_bitmap);
+        return 1;
+    }
+
+    qemu_put_be32(f, ENCRYPTED_BITMAP_CONTINUE);
+    qemu_put_be64(f, start);
+    qemu_put_be64(f, e.num_pages);
+    qemu_put_be64(f, size);
+    qemu_put_buffer(f, (uint8_t *)e.enc_bitmap, size);
+
+    g_free(e.enc_bitmap);
+
+done:
+    if (last) {
+        qemu_put_be32(f, ENCRYPTED_BITMAP_END);
+    }
+    return 0;
+}
+
 static void
 sev_register_types(void)
 {
diff --git a/target/i386/trace-events b/target/i386/trace-events
index 609752cca7..853a3870ab 100644
--- a/target/i386/trace-events
+++ b/target/i386/trace-events
@@ -21,3 +21,5 @@ kvm_sev_send_finish(void) ""
 kvm_sev_receive_start(int policy, void *session, void *pdh) "policy 0x%x session %p pdh %p"
 kvm_sev_receive_update_data(void *src, void *dst, int len, void *hdr, int hdr_len) "guest %p trans %p len %d hdr %p hdr_len %d"
 kvm_sev_receive_finish(void) ""
+kvm_sev_save_bitmap(uint64_t start, uint64_t len) "start 0x%" PRIx64 " len 0x%" PRIx64
+kvm_sev_load_bitmap(uint64_t start, uint64_t len) "start 0x%" PRIx64 " len 0x%" PRIx64
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 13/14] migration/ram: add support to send encrypted pages
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (10 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page encryption bitmap Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-14 16:37   ` Dr. David Alan Gilbert
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 12/14] kvm: add support to sync the page encryption state bitmap Singh, Brijesh
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 14/14] target/i386: sev: remove migration blocker Singh, Brijesh
  13 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

When memory encryption is enabled, the guest memory will be encrypted with
the guest specific key. The patch introduces RAM_SAVE_FLAG_ENCRYPTED_PAGE
flag to distinguish the encrypted data from plaintext. Encrypted pages
may need special handling. The kvm_memcrypt_save_outgoing_page() is used
by the sender to write the encrypted pages onto the socket, similarly the
kvm_memcrypt_load_incoming_page() is used by the target to read the
encrypted pages from the socket and load into the guest memory.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 migration/ram.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 130 insertions(+), 1 deletion(-)

diff --git a/migration/ram.c b/migration/ram.c
index 57c707525b..100a5a10cd 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -59,6 +59,9 @@
 #include "qemu/iov.h"
 #include "hw/boards.h"
 
+/* Defines RAM_SAVE_ENCRYPTED_PAGE and  RAM_SAVE_ENCRYPTED_BITMAP */
+#include "sysemu/sev.h"
+
 /***********************************************************/
 /* ram save/restore */
 
@@ -77,6 +80,7 @@
 #define RAM_SAVE_FLAG_XBZRLE   0x40
 /* 0x80 is reserved in migration.h start with 0x100 next */
 #define RAM_SAVE_FLAG_COMPRESS_PAGE    0x100
+#define RAM_SAVE_FLAG_ENCRYPTED_DATA   0x200
 
 static inline bool is_zero_range(uint8_t *p, uint64_t size)
 {
@@ -460,6 +464,9 @@ static QemuCond decomp_done_cond;
 
 static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
                                  ram_addr_t offset, uint8_t *source_buf);
+static int ram_save_encrypted_page(RAMState *rs, PageSearchStatus *pss,
+                                   bool last_stage);
+
 
 static void *do_data_compress(void *opaque)
 {
@@ -2039,6 +2046,73 @@ static int save_normal_page(RAMState *rs, RAMBlock *block, ram_addr_t offset,
     return 1;
 }
 
+/**
+ * ram_save_encrypted_page - send the given encrypted page to the stream
+ */
+static int ram_save_encrypted_page(RAMState *rs, PageSearchStatus *pss,
+                                   bool last_stage)
+{
+    int ret;
+    uint8_t *p;
+    RAMBlock *block = pss->block;
+    ram_addr_t offset = pss->page << TARGET_PAGE_BITS;
+    uint64_t bytes_xmit;
+    MachineState *ms = MACHINE(qdev_get_machine());
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+    struct MachineMemoryEncryptionOps *ops = mc->memory_encryption_ops;
+
+    p = block->host + offset;
+
+    ram_counters.transferred +=
+        save_page_header(rs, rs->f, block,
+                    offset | RAM_SAVE_FLAG_ENCRYPTED_DATA);
+
+    qemu_put_be32(rs->f, RAM_SAVE_ENCRYPTED_PAGE);
+    ret = ops->save_outgoing_page(rs->f, p, TARGET_PAGE_SIZE, &bytes_xmit);
+    if (ret) {
+        return -1;
+    }
+
+    ram_counters.transferred += bytes_xmit;
+    ram_counters.normal++;
+
+    return 1;
+}
+
+/**
+ * ram_save_encrypted_bitmap: send the encrypted page state bitmap
+ */
+static int ram_save_encrypted_bitmap(RAMState *rs, QEMUFile *f)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+    struct MachineMemoryEncryptionOps *ops = mc->memory_encryption_ops;
+
+    save_page_header(rs, rs->f, rs->last_seen_block,
+                     RAM_SAVE_FLAG_ENCRYPTED_DATA);
+    qemu_put_be32(rs->f, RAM_SAVE_ENCRYPTED_BITMAP);
+    return ops->save_outgoing_bitmap(rs->f);
+}
+
+static int load_encrypted_data(QEMUFile *f, uint8_t *ptr)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+    struct MachineMemoryEncryptionOps *ops = mc->memory_encryption_ops;
+    int flag;
+
+    flag = qemu_get_be32(f);
+
+    if (flag == RAM_SAVE_ENCRYPTED_PAGE) {
+        return ops->load_incoming_page(f, ptr);
+    } else if (flag == RAM_SAVE_ENCRYPTED_BITMAP) {
+        return ops->load_incoming_bitmap(f);
+    } else {
+        error_report("unknown encrypted flag %x", flag);
+        return 1;
+    }
+}
+
 /**
  * ram_save_page: send the given page to the stream
  *
@@ -2528,6 +2602,22 @@ static bool save_compress_page(RAMState *rs, RAMBlock *block, ram_addr_t offset)
     return false;
 }
 
+/**
+ * encrypted_test_bitmap: check if the page is encrypted
+ *
+ * Returns a bool indicating whether the page is encrypted.
+ */
+static bool encrypted_test_bitmap(RAMState *rs, RAMBlock *block,
+                                  unsigned long page)
+{
+    /* ROM devices contains the unencrypted data */
+    if (memory_region_is_rom(block->mr)) {
+        return false;
+    }
+
+    return test_bit(page, block->encbmap);
+}
+
 /**
  * ram_save_target_page: save one target page
  *
@@ -2548,6 +2638,17 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss,
         return res;
     }
 
+    /*
+     * If memory encryption is enabled then use memory encryption APIs
+     * to write the outgoing buffer to the wire. The encryption APIs
+     * will take care of accessing the guest memory and re-encrypt it
+     * for the transport purposes.
+     */
+    if (memcrypt_enabled() &&
+        encrypted_test_bitmap(rs, pss->block, pss->page)) {
+        return ram_save_encrypted_page(rs, pss, last_stage);
+    }
+
     if (save_compress_page(rs, block, offset)) {
         return 1;
     }
@@ -3445,6 +3546,16 @@ void qemu_guest_free_page_hint(void *addr, size_t len)
     }
 }
 
+static int ram_encrypted_save_setup(void)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+    MachineClass *mc = MACHINE_GET_CLASS(ms);
+    MigrationParameters *p = &migrate_get_current()->parameters;
+    struct MachineMemoryEncryptionOps *ops = mc->memory_encryption_ops;
+
+    return ops->save_setup(p->sev_pdh, p->sev_plat_cert, p->sev_amd_cert);
+}
+
 /*
  * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
  * long-running RCU critical section.  When rcu-reclaims in the code
@@ -3480,6 +3591,12 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
 
     rcu_read_lock();
 
+    if (memcrypt_enabled()) {
+        if (ram_encrypted_save_setup()) {
+            return -1;
+        }
+    }
+
     qemu_put_be64(f, ram_bytes_total_common(true) | RAM_SAVE_FLAG_MEM_SIZE);
 
     RAMBLOCK_FOREACH_MIGRATABLE(block) {
@@ -3644,6 +3761,11 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
     flush_compressed_data(rs);
     ram_control_after_iterate(f, RAM_CONTROL_FINISH);
 
+    /* send the page encryption state bitmap */
+    if (memcrypt_enabled()) {
+        ret = ram_save_encrypted_bitmap(rs, f);
+    }
+
     rcu_read_unlock();
 
     multifd_send_sync_main();
@@ -4391,7 +4513,8 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
         }
 
         if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
-                     RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
+                     RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE |
+                     RAM_SAVE_FLAG_ENCRYPTED_DATA)) {
             RAMBlock *block = ram_block_from_stream(f, flags);
 
             /*
@@ -4505,6 +4628,12 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
                 break;
             }
             break;
+        case RAM_SAVE_FLAG_ENCRYPTED_DATA:
+            if (load_encrypted_data(f, host)) {
+                    error_report("Failed to load encrypted data");
+                    ret = -EINVAL;
+            }
+            break;
         case RAM_SAVE_FLAG_EOS:
             /* normal exit */
             multifd_recv_sync_main();
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 12/14] kvm: add support to sync the page encryption state bitmap
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (11 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 13/14] migration/ram: add support to send encrypted pages Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 14/14] target/i386: sev: remove migration blocker Singh, Brijesh
  13 siblings, 0 replies; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

The SEV VMs have concept of private and shared memory. The private memory
is encrypted with guest-specific key, while shared memory may be encrypted
with hyperivosr key. The KVM_GET_PAGE_ENC_BITMAP can be used to get a
bitmap indicating whether the guest page is private or shared. A private
page must be transmitted using the SEV migration commands.

Add a cpu_physical_memory_sync_encrypted_bitmap() which can be used to get
the page encryption bitmap for a given memory region.

The page encryption bitmap is not exactly same as dirty bitmap. The page
encryption bitmap is a purely a matter of state about the page is encrypted
or not. To avoid some confusion we clone few functions for clarity.

Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 accel/kvm/kvm-all.c     |  37 ++++++++
 include/exec/ram_addr.h | 199 ++++++++++++++++++++++++++++++++++++++++
 include/exec/ramlist.h  |   3 +-
 migration/ram.c         |  17 ++++
 4 files changed, 255 insertions(+), 1 deletion(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f4d136b022..d942e10896 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -560,6 +560,36 @@ static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section,
 
 #define ALIGN(x, y)  (((x)+(y)-1) & ~((y)-1))
 
+/* sync page_enc bitmap */
+static int kvm_sync_page_enc_bitmap(KVMMemoryListener *kml,
+                                    MemoryRegionSection *section,
+                                    KVMSlot *mem)
+{
+    unsigned long size;
+    KVMState *s = kvm_state;
+    struct kvm_page_enc_bitmap e = {};
+    ram_addr_t pages = int128_get64(section->size) / getpagesize();
+    ram_addr_t start = section->offset_within_region +
+                       memory_region_get_ram_addr(section->mr);
+
+    size = ALIGN(((mem->memory_size) >> TARGET_PAGE_BITS), 64) / 8;
+    e.enc_bitmap = g_malloc0(size);
+    e.start_gfn = mem->start_addr >> TARGET_PAGE_BITS;
+    e.num_pages = pages;
+    if (kvm_vm_ioctl(s, KVM_GET_PAGE_ENC_BITMAP, &e) == -1) {
+        DPRINTF("KVM_GET_PAGE_ENC_BITMAP ioctl failed %d\n", errno);
+        g_free(e.enc_bitmap);
+        return 1;
+    }
+
+    cpu_physical_memory_set_encrypted_lebitmap(e.enc_bitmap,
+                                               start, pages);
+
+    g_free(e.enc_bitmap);
+
+    return 0;
+}
+
 /**
  * kvm_physical_sync_dirty_bitmap - Sync dirty bitmap from kernel space
  *
@@ -616,6 +646,13 @@ static int kvm_physical_sync_dirty_bitmap(KVMMemoryListener *kml,
         }
 
         kvm_get_dirty_pages_log_range(section, d.dirty_bitmap);
+
+        if (kvm_memcrypt_enabled() &&
+            kvm_sync_page_enc_bitmap(kml, section, mem)) {
+            g_free(d.dirty_bitmap);
+            return -1;
+        }
+
     }
 out:
     return ret;
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index b7b2e60ff6..6dbeac6567 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -67,6 +67,8 @@ struct RAMBlock {
      */
     unsigned long *clear_bmap;
     uint8_t clear_bmap_shift;
+    /* bitmap of page encryption state for an encrypted guest */
+    unsigned long *encbmap;
 };
 
 /**
@@ -323,6 +325,60 @@ static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
     rcu_read_unlock();
 }
 
+static inline void cpu_physical_memory_set_encrypted_range(ram_addr_t start,
+                                                           ram_addr_t length,
+                                                           unsigned long val)
+{
+    unsigned long page;
+    unsigned long * const *src;
+
+    page = start >> TARGET_PAGE_BITS;
+
+    rcu_read_lock();
+
+    src = atomic_rcu_read(
+            &ram_list.dirty_memory[DIRTY_MEMORY_ENCRYPTED])->blocks;
+
+    if (length) {
+        unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE;
+        unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE;
+        int m = (start) & (BITS_PER_LONG - 1);
+        int n = MIN(length, BITS_PER_LONG - m);
+        unsigned long old_val = atomic_read(&src[idx][BIT_WORD(offset)]);
+        unsigned long mask;
+
+        mask = (~0UL >> n);
+        mask = mask << m;
+
+        old_val &= ~mask;
+        val &= mask;
+
+        atomic_xchg(&src[idx][BIT_WORD(offset)], old_val | val);
+        page += n;
+        length -= n;
+    }
+
+    /* remaining bits */
+    if (length) {
+        unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE;
+        unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE;
+        int m = (start) & (BITS_PER_LONG - 1);
+        int n = MIN(length, BITS_PER_LONG - m);
+        unsigned long old_val = atomic_read(&src[idx][BIT_WORD(offset)]);
+        unsigned long mask;
+
+        mask = (~0UL >> n);
+        mask = mask << m;
+
+        old_val &= ~mask;
+        val &= mask;
+
+        atomic_xchg(&src[idx][BIT_WORD(offset)], old_val | val);
+    }
+
+    rcu_read_unlock();
+}
+
 static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
                                                        ram_addr_t length,
                                                        uint8_t mask)
@@ -376,6 +432,62 @@ static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
 }
 
 #if !defined(_WIN32)
+static inline void cpu_physical_memory_set_encrypted_lebitmap(
+                                                        unsigned long *bitmap,
+                                                        ram_addr_t start,
+                                                        ram_addr_t pages)
+{
+    unsigned long i;
+    unsigned long hpratio = getpagesize() / TARGET_PAGE_SIZE;
+    unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS);
+
+    /* start address is aligned at the start of a word? */
+    if ((((page * BITS_PER_LONG) << TARGET_PAGE_BITS) == start) &&
+        (hpratio == 1)) {
+        unsigned long **blocks[DIRTY_MEMORY_NUM];
+        unsigned long idx;
+        unsigned long offset;
+        long k;
+        long nr = BITS_TO_LONGS(pages);
+
+        idx = (start >> TARGET_PAGE_BITS) / DIRTY_MEMORY_BLOCK_SIZE;
+        offset = BIT_WORD((start >> TARGET_PAGE_BITS) %
+                          DIRTY_MEMORY_BLOCK_SIZE);
+
+        rcu_read_lock();
+
+        for (i = 0; i < DIRTY_MEMORY_NUM; i++) {
+            blocks[i] = atomic_rcu_read(&ram_list.dirty_memory[i])->blocks;
+        }
+
+        for (k = 0; k < nr; k++) {
+            if (bitmap[k]) {
+                unsigned long temp = leul_to_cpu(bitmap[k]);
+
+                atomic_xchg(&blocks[DIRTY_MEMORY_ENCRYPTED][idx][offset], temp);
+            }
+
+            if (++offset >= BITS_TO_LONGS(DIRTY_MEMORY_BLOCK_SIZE)) {
+                offset = 0;
+                idx++;
+            }
+        }
+
+        rcu_read_unlock();
+    } else {
+        i = 0;
+        while (pages > 0) {
+            unsigned long len = MIN(pages, BITS_PER_LONG);
+
+            cpu_physical_memory_set_encrypted_range(start, len,
+                            leul_to_cpu(bitmap[i]));
+            start += len;
+            i++;
+            pages -= len;
+        }
+    }
+}
+
 static inline void cpu_physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
                                                           ram_addr_t start,
                                                           ram_addr_t pages)
@@ -478,6 +590,8 @@ static inline void cpu_physical_memory_clear_dirty_range(ram_addr_t start,
     cpu_physical_memory_test_and_clear_dirty(start, length, DIRTY_MEMORY_MIGRATION);
     cpu_physical_memory_test_and_clear_dirty(start, length, DIRTY_MEMORY_VGA);
     cpu_physical_memory_test_and_clear_dirty(start, length, DIRTY_MEMORY_CODE);
+    cpu_physical_memory_test_and_clear_dirty(start, length,
+                                             DIRTY_MEMORY_ENCRYPTED);
 }
 
 
@@ -556,5 +670,90 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb,
 
     return num_dirty;
 }
+
+static inline bool cpu_physical_memory_test_encrypted(ram_addr_t start,
+                                                      ram_addr_t length)
+{
+    unsigned long end, page;
+    bool enc = false;
+    unsigned long * const *src;
+
+    if (length == 0) {
+        return enc;
+    }
+
+    end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
+    page = start >> TARGET_PAGE_BITS;
+
+    rcu_read_lock();
+
+    src = atomic_rcu_read(
+            &ram_list.dirty_memory[DIRTY_MEMORY_ENCRYPTED])->blocks;
+
+    while (page < end) {
+        unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE;
+        unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE;
+        unsigned long num = MIN(end - page, DIRTY_MEMORY_BLOCK_SIZE - offset);
+
+        enc |= atomic_read(&src[idx][BIT_WORD(offset)]);
+        page += num;
+    }
+
+    rcu_read_unlock();
+
+    return enc;
+}
+
+static inline
+void cpu_physical_memory_sync_encrypted_bitmap(RAMBlock *rb,
+                                               ram_addr_t start,
+                                               ram_addr_t length)
+{
+    ram_addr_t addr;
+    unsigned long word = BIT_WORD((start + rb->offset) >> TARGET_PAGE_BITS);
+    unsigned long *dest = rb->encbmap;
+
+    /* start address and length is aligned at the start of a word? */
+    if (((word * BITS_PER_LONG) << TARGET_PAGE_BITS) ==
+         (start + rb->offset) &&
+        !(length & ((BITS_PER_LONG << TARGET_PAGE_BITS) - 1))) {
+        int k;
+        int nr = BITS_TO_LONGS(length >> TARGET_PAGE_BITS);
+        unsigned long * const *src;
+        unsigned long idx = (word * BITS_PER_LONG) / DIRTY_MEMORY_BLOCK_SIZE;
+        unsigned long offset = BIT_WORD((word * BITS_PER_LONG) %
+                                        DIRTY_MEMORY_BLOCK_SIZE);
+        unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS);
+
+        rcu_read_lock();
+
+        src = atomic_rcu_read(
+                &ram_list.dirty_memory[DIRTY_MEMORY_ENCRYPTED])->blocks;
+
+        for (k = page; k < page + nr; k++) {
+            unsigned long bits = atomic_read(&src[idx][offset]);
+            dest[k] = bits;
+
+            if (++offset >= BITS_TO_LONGS(DIRTY_MEMORY_BLOCK_SIZE)) {
+                offset = 0;
+                idx++;
+            }
+        }
+
+        rcu_read_unlock();
+    } else {
+        ram_addr_t offset = rb->offset;
+
+        for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) {
+            long k = (start + addr) >> TARGET_PAGE_BITS;
+            if (cpu_physical_memory_test_encrypted(start + addr + offset,
+                                                   TARGET_PAGE_SIZE)) {
+                set_bit(k, dest);
+            } else {
+                clear_bit(k, dest);
+            }
+        }
+    }
+}
 #endif
 #endif
diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
index bc4faa1b00..2a5eab8b11 100644
--- a/include/exec/ramlist.h
+++ b/include/exec/ramlist.h
@@ -11,7 +11,8 @@ typedef struct RAMBlockNotifier RAMBlockNotifier;
 #define DIRTY_MEMORY_VGA       0
 #define DIRTY_MEMORY_CODE      1
 #define DIRTY_MEMORY_MIGRATION 2
-#define DIRTY_MEMORY_NUM       3        /* num of dirty bits */
+#define DIRTY_MEMORY_ENCRYPTED 3
+#define DIRTY_MEMORY_NUM       4        /* num of dirty bits */
 
 /* The dirty memory bitmap is split into fixed-size blocks to allow growth
  * under RCU.  The bitmap for a block can be accessed as follows:
diff --git a/migration/ram.c b/migration/ram.c
index 889148dd84..57c707525b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -57,6 +57,7 @@
 #include "qemu/uuid.h"
 #include "savevm.h"
 #include "qemu/iov.h"
+#include "hw/boards.h"
 
 /***********************************************************/
 /* ram save/restore */
@@ -700,6 +701,13 @@ typedef struct {
     QemuSemaphore sem_sync;
 } MultiFDRecvParams;
 
+static inline bool memcrypt_enabled(void)
+{
+    MachineState *ms = MACHINE(qdev_get_machine());
+
+    return machine_memory_encryption_enabled(ms);
+}
+
 static int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp)
 {
     MultiFDInit_t msg;
@@ -1754,6 +1762,9 @@ static void migration_bitmap_sync_range(RAMState *rs, RAMBlock *rb,
     rs->migration_dirty_pages +=
         cpu_physical_memory_sync_dirty_bitmap(rb, 0, length,
                                               &rs->num_dirty_pages_period);
+    if (memcrypt_enabled()) {
+        cpu_physical_memory_sync_encrypted_bitmap(rb, 0, length);
+    }
 }
 
 /**
@@ -2768,6 +2779,8 @@ static void ram_save_cleanup(void *opaque)
         block->bmap = NULL;
         g_free(block->unsentmap);
         block->unsentmap = NULL;
+        g_free(block->encbmap);
+        block->encbmap = NULL;
     }
 
     xbzrle_cleanup();
@@ -3310,6 +3323,10 @@ static void ram_list_init_bitmaps(void)
                 block->unsentmap = bitmap_new(pages);
                 bitmap_set(block->unsentmap, 0, pages);
             }
+            if (memcrypt_enabled()) {
+                block->encbmap = bitmap_new(pages);
+                bitmap_set(block->encbmap, 0, pages);
+            }
         }
     }
 }
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [Qemu-devel] [PATCH v3 14/14] target/i386: sev: remove migration blocker
  2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
                   ` (12 preceding siblings ...)
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 12/14] kvm: add support to sync the page encryption state bitmap Singh, Brijesh
@ 2019-08-06 16:54 ` Singh, Brijesh
  13 siblings, 0 replies; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-06 16:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, dgilbert, ehabkost

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 target/i386/sev.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 9d643e720c..72b841a458 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -34,7 +34,6 @@
 #define DEFAULT_SEV_DEVICE      "/dev/sev"
 
 static SEVState *sev_state;
-static Error *sev_mig_blocker;
 
 static const char *const sev_fw_errlist[] = {
     "",
@@ -700,7 +699,6 @@ static void
 sev_launch_finish(SEVState *s)
 {
     int ret, error;
-    Error *local_err = NULL;
 
     trace_kvm_sev_launch_finish();
     ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_LAUNCH_FINISH, 0, &error);
@@ -711,16 +709,6 @@ sev_launch_finish(SEVState *s)
     }
 
     sev_set_guest_state(SEV_STATE_RUNNING);
-
-    /* add migration blocker */
-    error_setg(&sev_mig_blocker,
-               "SEV: Migration is not implemented");
-    ret = migrate_add_blocker(sev_mig_blocker, &local_err);
-    if (local_err) {
-        error_report_err(local_err);
-        error_free(sev_mig_blocker);
-        exit(1);
-    }
 }
 
 static int
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 01/14] doc: update AMD SEV API spec web link
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 01/14] doc: update AMD SEV API spec web link Singh, Brijesh
@ 2019-08-06 19:00   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-06 19:00 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  docs/amd-memory-encryption.txt | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
> index 43bf3ee6a5..8822cadda1 100644
> --- a/docs/amd-memory-encryption.txt
> +++ b/docs/amd-memory-encryption.txt
> @@ -67,8 +67,8 @@ expects.
>  LAUNCH_FINISH command finalizes the guest launch and destroy's the cryptographic
>  context.
>  
> -See SEV KM API Spec [1] 'Launching a guest' usage flow (Appendix A) for the
> -complete flow chart.
> +See Secure Encrypted Virtualization Key Management API spec section
> +'Launching a guest' usage flow  (Appendix A) for the complete flow chart.
>  
>  To launch a SEV guest
>  
> @@ -97,8 +97,8 @@ References
>  AMD Memory Encryption whitepaper:
>  http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf
>  
> -Secure Encrypted Virtualization Key Management:
> -[1] http://support.amd.com/TechDocs/55766_SEV-KM API_Specification.pdf
> +Secure Encrypted Virtualization Key Management API Spec:
> +[1] https://developer.amd.com/sev/ (Secure Encrypted Virtualization API)

Yep, that's better; note the document name seems to have lost the words
'key management' at some time; but:


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

>  KVM Forum slides:
>  http://www.linux-kvm.org/images/7/74/02x08A-Thomas_Lendacky-AMDs_Virtualizatoin_Memory_Encryption_Technology.pdf
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 02/14] doc: update AMD SEV to include Live migration flow
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 02/14] doc: update AMD SEV to include Live migration flow Singh, Brijesh
@ 2019-08-07 11:01   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-07 11:01 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  docs/amd-memory-encryption.txt | 40 +++++++++++++++++++++++++++++++++-
>  1 file changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/amd-memory-encryption.txt b/docs/amd-memory-encryption.txt
> index 8822cadda1..01d95089a8 100644
> --- a/docs/amd-memory-encryption.txt
> +++ b/docs/amd-memory-encryption.txt
> @@ -89,7 +89,45 @@ TODO
>  
>  Live Migration
>  ----------------
> -TODO
> +AMD SEV encrypts the memory of VMs and because a different key is used
> +in each VM, the hypervisor will be unable to simply copy the
> +ciphertext from one VM to another to migrate the VM. Instead the AMD SEV Key
> +Management API provides sets of function which the hypervisor can use
> +to package a guest page for migration, while maintaining the confidentiality
> +provided by AMD SEV.
> +
> +SEV guest VMs have the concept of private and shared memory. The private
> +memory is encrypted with the guest-specific key, while shared memory may
> +be encrypted with the hypervisor key. The migration APIs provided by the
> +SEV API spec should be used for migrating the private pages. The
> +KVM_GET_PAGE_ENC_BITMAP ioctl can be used to get the guest page encryption
> +bitmap. The bitmap can be used to check if the given guest page is
> +private or shared.
> +
> +Before initiating the migration, we need to know the targets machine's public
> +Diffie-Hellman key (PDH) and certificate chain. It can be retrieved
> +with the 'query-sev-capabilities' QMP command or using the sev-tool. The
> +migrate-set-parameter can be used to pass the target machine's PDH and
> +certificate chain.
> +
> +During the migration flow, the SEND_START is called on the source hypervisor
> +to create an outgoing encryption context. The SEV guest policy dictates whether
> +the certificate passed through the migrate-sev-set-info command will be
> +validated. SEND_UPDATE_DATA is called to encrypt the guest private pages.
> +After migration is completed, SEND_FINISH is called to destroy the encryption
> +context and make the VM non-runnable to protect it against cloning.
> +
> +On the target machine, RECEIVE_START is called first to create an
> +incoming encryption context. The RECEIVE_UPDATE_DATA is called to copy
> +the received encrypted page into guest memory. After migration has
> +completed, RECEIVE_FINISH is called to make the VM runnable.
> +
> +For more information about the migration see SEV API Appendix A
> +Usage flow (Live migration section).
> +
> +NOTE:
> +To protect against the memory clone SEV APIs are designed to make the VM
> +unrunnable in case of the migration failure.
>  
>  References
>  -----------------
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters Singh, Brijesh
@ 2019-08-07 11:06   ` Dr. David Alan Gilbert
  2019-08-08  2:25     ` Singh, Brijesh
  0 siblings, 1 reply; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-07 11:06 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> AMD SEV migration flow requires that target machine's public Diffie-Hellman
> key (PDH) and certificate chain must be passed before initiating the guest
> migration. User can use QMP 'migrate-set-parameters' to pass the certificate
> chain. The certificate chain will be used while creating the outgoing
> encryption context.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> 
> I was able to pass the certificate chain through the HMP but somehow
> QMP socket interface is not working for me. If anyone has any tips on
> what I am missing in the patch then please let me know. In meantime,
> I will also continue my investigation on why its not working for me.

It looks OK to me; what's the qmp you're trying and what's the failure
error?

Dave

>  migration/migration.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>  monitor/hmp-cmds.c    | 18 +++++++++++++
>  qapi/migration.json   | 41 ++++++++++++++++++++++++++---
>  3 files changed, 116 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 8a607fe1e2..de66a0eb7e 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -783,6 +783,12 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>      params->announce_rounds = s->parameters.announce_rounds;
>      params->has_announce_step = true;
>      params->announce_step = s->parameters.announce_step;
> +    params->has_sev_pdh = true;
> +    params->sev_pdh = g_strdup(s->parameters.sev_pdh);
> +    params->has_sev_plat_cert = true;
> +    params->sev_plat_cert = g_strdup(s->parameters.sev_plat_cert);
> +    params->has_sev_amd_cert = true;
> +    params->sev_amd_cert = g_strdup(s->parameters.sev_amd_cert);
>  
>      return params;
>  }
> @@ -1289,6 +1295,18 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>      if (params->has_announce_step) {
>          dest->announce_step = params->announce_step;
>      }
> +    if (params->has_sev_pdh) {
> +        assert(params->sev_pdh->type == QTYPE_QSTRING);
> +        dest->sev_pdh = g_strdup(params->sev_pdh->u.s);
> +    }
> +    if (params->has_sev_plat_cert) {
> +        assert(params->sev_plat_cert->type == QTYPE_QSTRING);
> +        dest->sev_plat_cert = g_strdup(params->sev_plat_cert->u.s);
> +    }
> +    if (params->has_sev_amd_cert) {
> +        assert(params->sev_amd_cert->type == QTYPE_QSTRING);
> +        dest->sev_amd_cert = g_strdup(params->sev_amd_cert->u.s);
> +    }
>  }
>  
>  static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> @@ -1390,6 +1408,21 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>      if (params->has_announce_step) {
>          s->parameters.announce_step = params->announce_step;
>      }
> +    if (params->has_sev_pdh) {
> +        g_free(s->parameters.sev_pdh);
> +        assert(params->sev_pdh->type == QTYPE_QSTRING);
> +        s->parameters.sev_pdh = g_strdup(params->sev_pdh->u.s);
> +    }
> +    if (params->has_sev_plat_cert) {
> +        g_free(s->parameters.sev_plat_cert);
> +        assert(params->sev_plat_cert->type == QTYPE_QSTRING);
> +        s->parameters.sev_plat_cert = g_strdup(params->sev_plat_cert->u.s);
> +    }
> +    if (params->has_sev_amd_cert) {
> +        g_free(s->parameters.sev_amd_cert);
> +        assert(params->sev_amd_cert->type == QTYPE_QSTRING);
> +        s->parameters.sev_amd_cert = g_strdup(params->sev_amd_cert->u.s);
> +    }
>  }
>  
>  void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
> @@ -1410,6 +1443,27 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>          params->tls_hostname->type = QTYPE_QSTRING;
>          params->tls_hostname->u.s = strdup("");
>      }
> +    /* TODO Rewrite "" to null instead */
> +    if (params->has_sev_pdh
> +        && params->sev_pdh->type == QTYPE_QNULL) {
> +        qobject_unref(params->sev_pdh->u.n);
> +        params->sev_pdh->type = QTYPE_QSTRING;
> +        params->sev_pdh->u.s = strdup("");
> +    }
> +    /* TODO Rewrite "" to null instead */
> +    if (params->has_sev_plat_cert
> +        && params->sev_plat_cert->type == QTYPE_QNULL) {
> +        qobject_unref(params->sev_plat_cert->u.n);
> +        params->sev_plat_cert->type = QTYPE_QSTRING;
> +        params->sev_plat_cert->u.s = strdup("");
> +    }
> +    /* TODO Rewrite "" to null instead */
> +    if (params->has_sev_amd_cert
> +        && params->sev_amd_cert->type == QTYPE_QNULL) {
> +        qobject_unref(params->sev_amd_cert->u.n);
> +        params->sev_amd_cert->type = QTYPE_QSTRING;
> +        params->sev_amd_cert->u.s = strdup("");
> +    }
>  
>      migrate_params_test_apply(params, &tmp);
>  
> @@ -3466,6 +3520,9 @@ static void migration_instance_finalize(Object *obj)
>      qemu_mutex_destroy(&ms->qemu_file_lock);
>      g_free(params->tls_hostname);
>      g_free(params->tls_creds);
> +    g_free(params->sev_pdh);
> +    g_free(params->sev_plat_cert);
> +    g_free(params->sev_amd_cert);
>      qemu_sem_destroy(&ms->rate_limit_sem);
>      qemu_sem_destroy(&ms->pause_sem);
>      qemu_sem_destroy(&ms->postcopy_pause_sem);
> @@ -3507,6 +3564,10 @@ static void migration_instance_init(Object *obj)
>      params->has_announce_rounds = true;
>      params->has_announce_step = true;
>  
> +    params->sev_pdh = g_strdup("");
> +    params->sev_plat_cert = g_strdup("");
> +    params->sev_amd_cert = g_strdup("");
> +
>      qemu_sem_init(&ms->postcopy_pause_sem, 0);
>      qemu_sem_init(&ms->postcopy_pause_rp_sem, 0);
>      qemu_sem_init(&ms->rp_state.rp_sem, 0);
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 5ca3ebe942..354219f27a 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1872,6 +1872,24 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>          p->has_announce_step = true;
>          visit_type_size(v, param, &p->announce_step, &err);
>          break;
> +    case MIGRATION_PARAMETER_SEV_PDH:
> +        p->has_sev_pdh = true;
> +        p->sev_pdh = g_new0(StrOrNull, 1);
> +        p->sev_pdh->type = QTYPE_QSTRING;
> +        visit_type_str(v, param, &p->sev_pdh->u.s, &err);
> +        break;
> +    case MIGRATION_PARAMETER_SEV_PLAT_CERT:
> +        p->has_sev_plat_cert = true;
> +        p->sev_plat_cert = g_new0(StrOrNull, 1);
> +        p->sev_plat_cert->type = QTYPE_QSTRING;
> +        visit_type_str(v, param, &p->sev_plat_cert->u.s, &err);
> +        break;
> +    case MIGRATION_PARAMETER_SEV_AMD_CERT:
> +        p->has_sev_amd_cert = true;
> +        p->sev_amd_cert = g_new0(StrOrNull, 1);
> +        p->sev_amd_cert->type = QTYPE_QSTRING;
> +        visit_type_str(v, param, &p->sev_amd_cert->u.s, &err);
> +        break;
>      default:
>          assert(0);
>      }
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 9cfbaf8c6c..bb07995d2c 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -580,6 +580,15 @@
>  # @max-cpu-throttle: maximum cpu throttle percentage.
>  #                    Defaults to 99. (Since 3.1)
>  #
> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
> +#           (Since 4.2)
> +#
> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
> +#                 (Since 4.2)
> +#
> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
> +#                base64 (Since 4.2)
> +#
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> @@ -592,7 +601,7 @@
>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
>             'multifd-channels',
>             'xbzrle-cache-size', 'max-postcopy-bandwidth',
> -           'max-cpu-throttle' ] }
> +           'max-cpu-throttle', 'sev-pdh', 'sev-plat-cert', 'sev-amd-cert' ] }
>  
>  ##
>  # @MigrateSetParameters:
> @@ -682,6 +691,15 @@
>  # @max-cpu-throttle: maximum cpu throttle percentage.
>  #                    The default value is 99. (Since 3.1)
>  #
> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
> +#           (Since 4.2)
> +#
> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
> +#                 (Since 4.2)
> +#
> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
> +#                base64 (Since 4.2)
> +#
>  # Since: 2.4
>  ##
>  # TODO either fuse back into MigrationParameters, or make
> @@ -707,7 +725,10 @@
>              '*multifd-channels': 'int',
>              '*xbzrle-cache-size': 'size',
>              '*max-postcopy-bandwidth': 'size',
> -	    '*max-cpu-throttle': 'int' } }
> +            '*max-cpu-throttle': 'int',
> +            '*sev-pdh':'StrOrNull',
> +            '*sev-plat-cert': 'StrOrNull',
> +            '*sev-amd-cert' : 'StrOrNull' } }
>  
>  ##
>  # @migrate-set-parameters:
> @@ -817,6 +838,15 @@
>  #                    Defaults to 99.
>  #                     (Since 3.1)
>  #
> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
> +#           (Since 4.2)
> +#
> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
> +#                 (Since 4.2)
> +#
> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
> +#                base64 (Since 4.2)
> +#
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> @@ -839,8 +869,11 @@
>              '*block-incremental': 'bool' ,
>              '*multifd-channels': 'uint8',
>              '*xbzrle-cache-size': 'size',
> -	    '*max-postcopy-bandwidth': 'size',
> -            '*max-cpu-throttle':'uint8'} }
> +            '*max-postcopy-bandwidth': 'size',
> +            '*max-cpu-throttle':'uint8',
> +            '*sev-pdh':'str',
> +            '*sev-plat-cert': 'str',
> +            '*sev-amd-cert' : 'str'} }
>  
>  ##
>  # @query-migrate-parameters:
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 05/14] hw/machine: add helper to query the memory encryption state
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 05/14] hw/machine: add helper to query the memory encryption state Singh, Brijesh
@ 2019-08-07 16:14   ` Dr. David Alan Gilbert
  2019-08-08  2:25     ` Singh, Brijesh
  0 siblings, 1 reply; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-07 16:14 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> To enable a memory encryption inside a VM, user must pass the object
> name used for the encryption in command line parameter as shown below.
> 
> # $(QEMU) \
>   -machine memory-encryption=<object_name>
> 
> Add a helper machine_memory_encryption_enabled() which will return a bool
> indicating whether the encryption object has been specified in the command
> line parameter.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

There's a check in accel/kvm/kvm-all.c:kvm_init which has:
       if (ms->memory_encryption) {

which you might want to replace by this.

Dave

> ---
>  hw/core/machine.c   | 5 +++++
>  include/hw/boards.h | 1 +
>  2 files changed, 6 insertions(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index c58a8e594e..f1e1b3661f 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1031,6 +1031,11 @@ bool machine_mem_merge(MachineState *machine)
>      return machine->mem_merge;
>  }
>  
> +bool machine_memory_encryption_enabled(MachineState *machine)
> +{
> +    return machine->memory_encryption ? true : false;
> +}
> +
>  static char *cpu_slot_to_string(const CPUArchId *cpu)
>  {
>      GString *s = g_string_new(NULL);
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index a71d1a53a5..c5446a39cf 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -76,6 +76,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
>                                 Error **errp);
>  
>  void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char *type);
> +bool machine_memory_encryption_enabled(MachineState *machine);
>  
>  
>  /**
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 06/14] hw/machine: introduce MachineMemoryEncryptionOps for encrypted VMs
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 06/14] hw/machine: introduce MachineMemoryEncryptionOps for encrypted VMs Singh, Brijesh
@ 2019-08-07 16:36   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-07 16:36 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> When memory encryption is enabled in VM, the guest RAM will be encrypted
> with the guest-specific key, to protect the confidentiality of data while
> in transit we need to platform specific hooks to save or migrate the
> guest RAM. The MemoryEncryptionOps introduced in this patch will be later
> used by the migration.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>

OK, I can imagine adding some Error ** parameters to those perhaps or
maybe some different length types; but for now that's a good start;

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  include/hw/boards.h | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index c5446a39cf..ba80c236fe 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -105,6 +105,29 @@ typedef struct {
>      CPUArchId cpus[0];
>  } CPUArchIdList;
>  
> +/**
> + * The functions registers with MachineMemoryEncryptionOps will be used during
> + * the encrypted guest migration.
> + */
> +struct MachineMemoryEncryptionOps {
> +    /* Initialize the platform specific state before starting the migration */
> +    int (*save_setup)(const char *pdh, const char *plat_cert,
> +                      const char *amd_cert);
> +
> +    /* Write the encrypted page and metadata associated with it */
> +    int (*save_outgoing_page)(QEMUFile *f, uint8_t *ptr, uint32_t size,
> +                              uint64_t *bytes_sent);
> +
> +    /* Load the incoming encrypted page into guest memory */
> +    int (*load_incoming_page)(QEMUFile *f, uint8_t *ptr);
> +
> +    /* Write the page encryption state bitmap */
> +    int (*save_outgoing_bitmap)(QEMUFile *f);
> +
> +    /* Load the incoming page encryption bitmap */
> +    int (*load_incoming_bitmap)(QEMUFile *f);
> +};
> +
>  /**
>   * MachineClass:
>   * @deprecation_reason: If set, the machine is marked as deprecated. The
> @@ -228,6 +251,7 @@ struct MachineClass {
>                                                           unsigned cpu_index);
>      const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
>      int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
> +    struct MachineMemoryEncryptionOps *memory_encryption_ops;
>  };
>  
>  /**
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters
  2019-08-07 11:06   ` Dr. David Alan Gilbert
@ 2019-08-08  2:25     ` Singh, Brijesh
  2019-08-08 10:48       ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-08  2:25 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, qemu-devel, ehabkost


On 8/7/19 6:06 AM, Dr. David Alan Gilbert wrote:
> * Singh, Brijesh (brijesh.singh@amd.com) wrote:
>> AMD SEV migration flow requires that target machine's public Diffie-Hellman
>> key (PDH) and certificate chain must be passed before initiating the guest
>> migration. User can use QMP 'migrate-set-parameters' to pass the certificate
>> chain. The certificate chain will be used while creating the outgoing
>> encryption context.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>> ---
>>
>> I was able to pass the certificate chain through the HMP but somehow
>> QMP socket interface is not working for me. If anyone has any tips on
>> what I am missing in the patch then please let me know. In meantime,
>> I will also continue my investigation on why its not working for me.
> It looks OK to me; what's the qmp you're trying and what's the failure
> error?


I am not seeing any error. I am using the below command through qmp-shell.

(qmp) migrate-set-paramaters sev-pdh="...." sev-plat-cert="...."
sev-amd-cert="..."


The command does not return any error. I added some debugs in
migrate_params_test_apply() and qmp_migrate_set_parameters() to see the
valye of params->has_sev_pdh and its always zero. The functions are
getting called when I issue the migrate-set-parameters qmp but the
values are not passed hence the memory_encryption->setup() never gets
called.


> Dave
>
>>  migration/migration.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>>  monitor/hmp-cmds.c    | 18 +++++++++++++
>>  qapi/migration.json   | 41 ++++++++++++++++++++++++++---
>>  3 files changed, 116 insertions(+), 4 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 8a607fe1e2..de66a0eb7e 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -783,6 +783,12 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
>>      params->announce_rounds = s->parameters.announce_rounds;
>>      params->has_announce_step = true;
>>      params->announce_step = s->parameters.announce_step;
>> +    params->has_sev_pdh = true;
>> +    params->sev_pdh = g_strdup(s->parameters.sev_pdh);
>> +    params->has_sev_plat_cert = true;
>> +    params->sev_plat_cert = g_strdup(s->parameters.sev_plat_cert);
>> +    params->has_sev_amd_cert = true;
>> +    params->sev_amd_cert = g_strdup(s->parameters.sev_amd_cert);
>>  
>>      return params;
>>  }
>> @@ -1289,6 +1295,18 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
>>      if (params->has_announce_step) {
>>          dest->announce_step = params->announce_step;
>>      }
>> +    if (params->has_sev_pdh) {
>> +        assert(params->sev_pdh->type == QTYPE_QSTRING);
>> +        dest->sev_pdh = g_strdup(params->sev_pdh->u.s);
>> +    }
>> +    if (params->has_sev_plat_cert) {
>> +        assert(params->sev_plat_cert->type == QTYPE_QSTRING);
>> +        dest->sev_plat_cert = g_strdup(params->sev_plat_cert->u.s);
>> +    }
>> +    if (params->has_sev_amd_cert) {
>> +        assert(params->sev_amd_cert->type == QTYPE_QSTRING);
>> +        dest->sev_amd_cert = g_strdup(params->sev_amd_cert->u.s);
>> +    }
>>  }
>>  
>>  static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>> @@ -1390,6 +1408,21 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
>>      if (params->has_announce_step) {
>>          s->parameters.announce_step = params->announce_step;
>>      }
>> +    if (params->has_sev_pdh) {
>> +        g_free(s->parameters.sev_pdh);
>> +        assert(params->sev_pdh->type == QTYPE_QSTRING);
>> +        s->parameters.sev_pdh = g_strdup(params->sev_pdh->u.s);
>> +    }
>> +    if (params->has_sev_plat_cert) {
>> +        g_free(s->parameters.sev_plat_cert);
>> +        assert(params->sev_plat_cert->type == QTYPE_QSTRING);
>> +        s->parameters.sev_plat_cert = g_strdup(params->sev_plat_cert->u.s);
>> +    }
>> +    if (params->has_sev_amd_cert) {
>> +        g_free(s->parameters.sev_amd_cert);
>> +        assert(params->sev_amd_cert->type == QTYPE_QSTRING);
>> +        s->parameters.sev_amd_cert = g_strdup(params->sev_amd_cert->u.s);
>> +    }
>>  }
>>  
>>  void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>> @@ -1410,6 +1443,27 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>>          params->tls_hostname->type = QTYPE_QSTRING;
>>          params->tls_hostname->u.s = strdup("");
>>      }
>> +    /* TODO Rewrite "" to null instead */
>> +    if (params->has_sev_pdh
>> +        && params->sev_pdh->type == QTYPE_QNULL) {
>> +        qobject_unref(params->sev_pdh->u.n);
>> +        params->sev_pdh->type = QTYPE_QSTRING;
>> +        params->sev_pdh->u.s = strdup("");
>> +    }
>> +    /* TODO Rewrite "" to null instead */
>> +    if (params->has_sev_plat_cert
>> +        && params->sev_plat_cert->type == QTYPE_QNULL) {
>> +        qobject_unref(params->sev_plat_cert->u.n);
>> +        params->sev_plat_cert->type = QTYPE_QSTRING;
>> +        params->sev_plat_cert->u.s = strdup("");
>> +    }
>> +    /* TODO Rewrite "" to null instead */
>> +    if (params->has_sev_amd_cert
>> +        && params->sev_amd_cert->type == QTYPE_QNULL) {
>> +        qobject_unref(params->sev_amd_cert->u.n);
>> +        params->sev_amd_cert->type = QTYPE_QSTRING;
>> +        params->sev_amd_cert->u.s = strdup("");
>> +    }
>>  
>>      migrate_params_test_apply(params, &tmp);
>>  
>> @@ -3466,6 +3520,9 @@ static void migration_instance_finalize(Object *obj)
>>      qemu_mutex_destroy(&ms->qemu_file_lock);
>>      g_free(params->tls_hostname);
>>      g_free(params->tls_creds);
>> +    g_free(params->sev_pdh);
>> +    g_free(params->sev_plat_cert);
>> +    g_free(params->sev_amd_cert);
>>      qemu_sem_destroy(&ms->rate_limit_sem);
>>      qemu_sem_destroy(&ms->pause_sem);
>>      qemu_sem_destroy(&ms->postcopy_pause_sem);
>> @@ -3507,6 +3564,10 @@ static void migration_instance_init(Object *obj)
>>      params->has_announce_rounds = true;
>>      params->has_announce_step = true;
>>  
>> +    params->sev_pdh = g_strdup("");
>> +    params->sev_plat_cert = g_strdup("");
>> +    params->sev_amd_cert = g_strdup("");
>> +
>>      qemu_sem_init(&ms->postcopy_pause_sem, 0);
>>      qemu_sem_init(&ms->postcopy_pause_rp_sem, 0);
>>      qemu_sem_init(&ms->rp_state.rp_sem, 0);
>> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>> index 5ca3ebe942..354219f27a 100644
>> --- a/monitor/hmp-cmds.c
>> +++ b/monitor/hmp-cmds.c
>> @@ -1872,6 +1872,24 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>>          p->has_announce_step = true;
>>          visit_type_size(v, param, &p->announce_step, &err);
>>          break;
>> +    case MIGRATION_PARAMETER_SEV_PDH:
>> +        p->has_sev_pdh = true;
>> +        p->sev_pdh = g_new0(StrOrNull, 1);
>> +        p->sev_pdh->type = QTYPE_QSTRING;
>> +        visit_type_str(v, param, &p->sev_pdh->u.s, &err);
>> +        break;
>> +    case MIGRATION_PARAMETER_SEV_PLAT_CERT:
>> +        p->has_sev_plat_cert = true;
>> +        p->sev_plat_cert = g_new0(StrOrNull, 1);
>> +        p->sev_plat_cert->type = QTYPE_QSTRING;
>> +        visit_type_str(v, param, &p->sev_plat_cert->u.s, &err);
>> +        break;
>> +    case MIGRATION_PARAMETER_SEV_AMD_CERT:
>> +        p->has_sev_amd_cert = true;
>> +        p->sev_amd_cert = g_new0(StrOrNull, 1);
>> +        p->sev_amd_cert->type = QTYPE_QSTRING;
>> +        visit_type_str(v, param, &p->sev_amd_cert->u.s, &err);
>> +        break;
>>      default:
>>          assert(0);
>>      }
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 9cfbaf8c6c..bb07995d2c 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -580,6 +580,15 @@
>>  # @max-cpu-throttle: maximum cpu throttle percentage.
>>  #                    Defaults to 99. (Since 3.1)
>>  #
>> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
>> +#           (Since 4.2)
>> +#
>> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
>> +#                 (Since 4.2)
>> +#
>> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
>> +#                base64 (Since 4.2)
>> +#
>>  # Since: 2.4
>>  ##
>>  { 'enum': 'MigrationParameter',
>> @@ -592,7 +601,7 @@
>>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
>>             'multifd-channels',
>>             'xbzrle-cache-size', 'max-postcopy-bandwidth',
>> -           'max-cpu-throttle' ] }
>> +           'max-cpu-throttle', 'sev-pdh', 'sev-plat-cert', 'sev-amd-cert' ] }
>>  
>>  ##
>>  # @MigrateSetParameters:
>> @@ -682,6 +691,15 @@
>>  # @max-cpu-throttle: maximum cpu throttle percentage.
>>  #                    The default value is 99. (Since 3.1)
>>  #
>> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
>> +#           (Since 4.2)
>> +#
>> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
>> +#                 (Since 4.2)
>> +#
>> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
>> +#                base64 (Since 4.2)
>> +#
>>  # Since: 2.4
>>  ##
>>  # TODO either fuse back into MigrationParameters, or make
>> @@ -707,7 +725,10 @@
>>              '*multifd-channels': 'int',
>>              '*xbzrle-cache-size': 'size',
>>              '*max-postcopy-bandwidth': 'size',
>> -	    '*max-cpu-throttle': 'int' } }
>> +            '*max-cpu-throttle': 'int',
>> +            '*sev-pdh':'StrOrNull',
>> +            '*sev-plat-cert': 'StrOrNull',
>> +            '*sev-amd-cert' : 'StrOrNull' } }
>>  
>>  ##
>>  # @migrate-set-parameters:
>> @@ -817,6 +838,15 @@
>>  #                    Defaults to 99.
>>  #                     (Since 3.1)
>>  #
>> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
>> +#           (Since 4.2)
>> +#
>> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
>> +#                 (Since 4.2)
>> +#
>> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
>> +#                base64 (Since 4.2)
>> +#
>>  # Since: 2.4
>>  ##
>>  { 'struct': 'MigrationParameters',
>> @@ -839,8 +869,11 @@
>>              '*block-incremental': 'bool' ,
>>              '*multifd-channels': 'uint8',
>>              '*xbzrle-cache-size': 'size',
>> -	    '*max-postcopy-bandwidth': 'size',
>> -            '*max-cpu-throttle':'uint8'} }
>> +            '*max-postcopy-bandwidth': 'size',
>> +            '*max-cpu-throttle':'uint8',
>> +            '*sev-pdh':'str',
>> +            '*sev-plat-cert': 'str',
>> +            '*sev-amd-cert' : 'str'} }
>>  
>>  ##
>>  # @query-migrate-parameters:
>> -- 
>> 2.17.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 05/14] hw/machine: add helper to query the memory encryption state
  2019-08-07 16:14   ` Dr. David Alan Gilbert
@ 2019-08-08  2:25     ` Singh, Brijesh
  0 siblings, 0 replies; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-08  2:25 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, qemu-devel, ehabkost


On 8/7/19 11:14 AM, Dr. David Alan Gilbert wrote:
> * Singh, Brijesh (brijesh.singh@amd.com) wrote:
>> To enable a memory encryption inside a VM, user must pass the object
>> name used for the encryption in command line parameter as shown below.
>>
>> # $(QEMU) \
>>   -machine memory-encryption=<object_name>
>>
>> Add a helper machine_memory_encryption_enabled() which will return a bool
>> indicating whether the encryption object has been specified in the command
>> line parameter.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> There's a check in accel/kvm/kvm-all.c:kvm_init which has:
>        if (ms->memory_encryption) {
>
> which you might want to replace by this.


Ah, sure will make the changes in next rev. thanks


> Dave
>
>> ---
>>  hw/core/machine.c   | 5 +++++
>>  include/hw/boards.h | 1 +
>>  2 files changed, 6 insertions(+)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index c58a8e594e..f1e1b3661f 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -1031,6 +1031,11 @@ bool machine_mem_merge(MachineState *machine)
>>      return machine->mem_merge;
>>  }
>>  
>> +bool machine_memory_encryption_enabled(MachineState *machine)
>> +{
>> +    return machine->memory_encryption ? true : false;
>> +}
>> +
>>  static char *cpu_slot_to_string(const CPUArchId *cpu)
>>  {
>>      GString *s = g_string_new(NULL);
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index a71d1a53a5..c5446a39cf 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -76,6 +76,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
>>                                 Error **errp);
>>  
>>  void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char *type);
>> +bool machine_memory_encryption_enabled(MachineState *machine);
>>  
>>  
>>  /**
>> -- 
>> 2.17.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters
  2019-08-08  2:25     ` Singh, Brijesh
@ 2019-08-08 10:48       ` Dr. David Alan Gilbert
  2019-08-09 20:00         ` Singh, Brijesh
  0 siblings, 1 reply; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-08 10:48 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> 
> On 8/7/19 6:06 AM, Dr. David Alan Gilbert wrote:
> > * Singh, Brijesh (brijesh.singh@amd.com) wrote:
> >> AMD SEV migration flow requires that target machine's public Diffie-Hellman
> >> key (PDH) and certificate chain must be passed before initiating the guest
> >> migration. User can use QMP 'migrate-set-parameters' to pass the certificate
> >> chain. The certificate chain will be used while creating the outgoing
> >> encryption context.
> >>
> >> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> >> ---
> >>
> >> I was able to pass the certificate chain through the HMP but somehow
> >> QMP socket interface is not working for me. If anyone has any tips on
> >> what I am missing in the patch then please let me know. In meantime,
> >> I will also continue my investigation on why its not working for me.
> > It looks OK to me; what's the qmp you're trying and what's the failure
> > error?
> 

Before I forget, you've not updated hmp_info_migrate_parameters in
hmp-cmds.c, e.g.:

             MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
             params->has_tls_authz ? params->tls_authz : "");
+        monitor_printf(mon, "%s:'%s'\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_SEV_PDH),
+            params->has_sev_pdh ? params->sev_pdh : "");
     }

> I am not seeing any error. I am using the below command through qmp-shell.
> 
> (qmp) migrate-set-paramaters sev-pdh="...." sev-plat-cert="...."
> sev-amd-cert="..."
> 
> 
> The command does not return any error. I added some debugs in
> migrate_params_test_apply() and qmp_migrate_set_parameters() to see the
> valye of params->has_sev_pdh and its always zero. The functions are
> getting called when I issue the migrate-set-parameters qmp but the
> values are not passed hence the memory_encryption->setup() never gets
> called.

Driving QMP by hand I'm seeing it apparently be stored in the
parameters:

Escape character is '^]'.
{"QMP": {"version": {"qemu": {"micro": 94, "minor": 0, "major": 4}, "package": "v4.1.0-rc4-16-ge6e1f28afd-dirty"}, "capabilities": ["oob"]}}
{ "execute": "qmp_capabilities" }
{"return": {}}
{ "execute": "migrate-set-parameters" , "arguments": { "sev-pdh": "foo" } }
{"return": {}}

then from HMP with the patch above:
(qemu) info migrate_parameters 
....
sev-pdh:'foo'

Dave

> 
> > Dave
> >
> >>  migration/migration.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> >>  monitor/hmp-cmds.c    | 18 +++++++++++++
> >>  qapi/migration.json   | 41 ++++++++++++++++++++++++++---
> >>  3 files changed, 116 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/migration/migration.c b/migration/migration.c
> >> index 8a607fe1e2..de66a0eb7e 100644
> >> --- a/migration/migration.c
> >> +++ b/migration/migration.c
> >> @@ -783,6 +783,12 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
> >>      params->announce_rounds = s->parameters.announce_rounds;
> >>      params->has_announce_step = true;
> >>      params->announce_step = s->parameters.announce_step;
> >> +    params->has_sev_pdh = true;
> >> +    params->sev_pdh = g_strdup(s->parameters.sev_pdh);
> >> +    params->has_sev_plat_cert = true;
> >> +    params->sev_plat_cert = g_strdup(s->parameters.sev_plat_cert);
> >> +    params->has_sev_amd_cert = true;
> >> +    params->sev_amd_cert = g_strdup(s->parameters.sev_amd_cert);
> >>  
> >>      return params;
> >>  }
> >> @@ -1289,6 +1295,18 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
> >>      if (params->has_announce_step) {
> >>          dest->announce_step = params->announce_step;
> >>      }
> >> +    if (params->has_sev_pdh) {
> >> +        assert(params->sev_pdh->type == QTYPE_QSTRING);
> >> +        dest->sev_pdh = g_strdup(params->sev_pdh->u.s);
> >> +    }
> >> +    if (params->has_sev_plat_cert) {
> >> +        assert(params->sev_plat_cert->type == QTYPE_QSTRING);
> >> +        dest->sev_plat_cert = g_strdup(params->sev_plat_cert->u.s);
> >> +    }
> >> +    if (params->has_sev_amd_cert) {
> >> +        assert(params->sev_amd_cert->type == QTYPE_QSTRING);
> >> +        dest->sev_amd_cert = g_strdup(params->sev_amd_cert->u.s);
> >> +    }
> >>  }
> >>  
> >>  static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> >> @@ -1390,6 +1408,21 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
> >>      if (params->has_announce_step) {
> >>          s->parameters.announce_step = params->announce_step;
> >>      }
> >> +    if (params->has_sev_pdh) {
> >> +        g_free(s->parameters.sev_pdh);
> >> +        assert(params->sev_pdh->type == QTYPE_QSTRING);
> >> +        s->parameters.sev_pdh = g_strdup(params->sev_pdh->u.s);
> >> +    }
> >> +    if (params->has_sev_plat_cert) {
> >> +        g_free(s->parameters.sev_plat_cert);
> >> +        assert(params->sev_plat_cert->type == QTYPE_QSTRING);
> >> +        s->parameters.sev_plat_cert = g_strdup(params->sev_plat_cert->u.s);
> >> +    }
> >> +    if (params->has_sev_amd_cert) {
> >> +        g_free(s->parameters.sev_amd_cert);
> >> +        assert(params->sev_amd_cert->type == QTYPE_QSTRING);
> >> +        s->parameters.sev_amd_cert = g_strdup(params->sev_amd_cert->u.s);
> >> +    }
> >>  }
> >>  
> >>  void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
> >> @@ -1410,6 +1443,27 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
> >>          params->tls_hostname->type = QTYPE_QSTRING;
> >>          params->tls_hostname->u.s = strdup("");
> >>      }
> >> +    /* TODO Rewrite "" to null instead */
> >> +    if (params->has_sev_pdh
> >> +        && params->sev_pdh->type == QTYPE_QNULL) {
> >> +        qobject_unref(params->sev_pdh->u.n);
> >> +        params->sev_pdh->type = QTYPE_QSTRING;
> >> +        params->sev_pdh->u.s = strdup("");
> >> +    }
> >> +    /* TODO Rewrite "" to null instead */
> >> +    if (params->has_sev_plat_cert
> >> +        && params->sev_plat_cert->type == QTYPE_QNULL) {
> >> +        qobject_unref(params->sev_plat_cert->u.n);
> >> +        params->sev_plat_cert->type = QTYPE_QSTRING;
> >> +        params->sev_plat_cert->u.s = strdup("");
> >> +    }
> >> +    /* TODO Rewrite "" to null instead */
> >> +    if (params->has_sev_amd_cert
> >> +        && params->sev_amd_cert->type == QTYPE_QNULL) {
> >> +        qobject_unref(params->sev_amd_cert->u.n);
> >> +        params->sev_amd_cert->type = QTYPE_QSTRING;
> >> +        params->sev_amd_cert->u.s = strdup("");
> >> +    }
> >>  
> >>      migrate_params_test_apply(params, &tmp);
> >>  
> >> @@ -3466,6 +3520,9 @@ static void migration_instance_finalize(Object *obj)
> >>      qemu_mutex_destroy(&ms->qemu_file_lock);
> >>      g_free(params->tls_hostname);
> >>      g_free(params->tls_creds);
> >> +    g_free(params->sev_pdh);
> >> +    g_free(params->sev_plat_cert);
> >> +    g_free(params->sev_amd_cert);
> >>      qemu_sem_destroy(&ms->rate_limit_sem);
> >>      qemu_sem_destroy(&ms->pause_sem);
> >>      qemu_sem_destroy(&ms->postcopy_pause_sem);
> >> @@ -3507,6 +3564,10 @@ static void migration_instance_init(Object *obj)
> >>      params->has_announce_rounds = true;
> >>      params->has_announce_step = true;
> >>  
> >> +    params->sev_pdh = g_strdup("");
> >> +    params->sev_plat_cert = g_strdup("");
> >> +    params->sev_amd_cert = g_strdup("");
> >> +
> >>      qemu_sem_init(&ms->postcopy_pause_sem, 0);
> >>      qemu_sem_init(&ms->postcopy_pause_rp_sem, 0);
> >>      qemu_sem_init(&ms->rp_state.rp_sem, 0);
> >> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> >> index 5ca3ebe942..354219f27a 100644
> >> --- a/monitor/hmp-cmds.c
> >> +++ b/monitor/hmp-cmds.c
> >> @@ -1872,6 +1872,24 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
> >>          p->has_announce_step = true;
> >>          visit_type_size(v, param, &p->announce_step, &err);
> >>          break;
> >> +    case MIGRATION_PARAMETER_SEV_PDH:
> >> +        p->has_sev_pdh = true;
> >> +        p->sev_pdh = g_new0(StrOrNull, 1);
> >> +        p->sev_pdh->type = QTYPE_QSTRING;
> >> +        visit_type_str(v, param, &p->sev_pdh->u.s, &err);
> >> +        break;
> >> +    case MIGRATION_PARAMETER_SEV_PLAT_CERT:
> >> +        p->has_sev_plat_cert = true;
> >> +        p->sev_plat_cert = g_new0(StrOrNull, 1);
> >> +        p->sev_plat_cert->type = QTYPE_QSTRING;
> >> +        visit_type_str(v, param, &p->sev_plat_cert->u.s, &err);
> >> +        break;
> >> +    case MIGRATION_PARAMETER_SEV_AMD_CERT:
> >> +        p->has_sev_amd_cert = true;
> >> +        p->sev_amd_cert = g_new0(StrOrNull, 1);
> >> +        p->sev_amd_cert->type = QTYPE_QSTRING;
> >> +        visit_type_str(v, param, &p->sev_amd_cert->u.s, &err);
> >> +        break;
> >>      default:
> >>          assert(0);
> >>      }
> >> diff --git a/qapi/migration.json b/qapi/migration.json
> >> index 9cfbaf8c6c..bb07995d2c 100644
> >> --- a/qapi/migration.json
> >> +++ b/qapi/migration.json
> >> @@ -580,6 +580,15 @@
> >>  # @max-cpu-throttle: maximum cpu throttle percentage.
> >>  #                    Defaults to 99. (Since 3.1)
> >>  #
> >> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
> >> +#           (Since 4.2)
> >> +#
> >> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
> >> +#                 (Since 4.2)
> >> +#
> >> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
> >> +#                base64 (Since 4.2)
> >> +#
> >>  # Since: 2.4
> >>  ##
> >>  { 'enum': 'MigrationParameter',
> >> @@ -592,7 +601,7 @@
> >>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
> >>             'multifd-channels',
> >>             'xbzrle-cache-size', 'max-postcopy-bandwidth',
> >> -           'max-cpu-throttle' ] }
> >> +           'max-cpu-throttle', 'sev-pdh', 'sev-plat-cert', 'sev-amd-cert' ] }
> >>  
> >>  ##
> >>  # @MigrateSetParameters:
> >> @@ -682,6 +691,15 @@
> >>  # @max-cpu-throttle: maximum cpu throttle percentage.
> >>  #                    The default value is 99. (Since 3.1)
> >>  #
> >> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
> >> +#           (Since 4.2)
> >> +#
> >> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
> >> +#                 (Since 4.2)
> >> +#
> >> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
> >> +#                base64 (Since 4.2)
> >> +#
> >>  # Since: 2.4
> >>  ##
> >>  # TODO either fuse back into MigrationParameters, or make
> >> @@ -707,7 +725,10 @@
> >>              '*multifd-channels': 'int',
> >>              '*xbzrle-cache-size': 'size',
> >>              '*max-postcopy-bandwidth': 'size',
> >> -	    '*max-cpu-throttle': 'int' } }
> >> +            '*max-cpu-throttle': 'int',
> >> +            '*sev-pdh':'StrOrNull',
> >> +            '*sev-plat-cert': 'StrOrNull',
> >> +            '*sev-amd-cert' : 'StrOrNull' } }
> >>  
> >>  ##
> >>  # @migrate-set-parameters:
> >> @@ -817,6 +838,15 @@
> >>  #                    Defaults to 99.
> >>  #                     (Since 3.1)
> >>  #
> >> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
> >> +#           (Since 4.2)
> >> +#
> >> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
> >> +#                 (Since 4.2)
> >> +#
> >> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
> >> +#                base64 (Since 4.2)
> >> +#
> >>  # Since: 2.4
> >>  ##
> >>  { 'struct': 'MigrationParameters',
> >> @@ -839,8 +869,11 @@
> >>              '*block-incremental': 'bool' ,
> >>              '*multifd-channels': 'uint8',
> >>              '*xbzrle-cache-size': 'size',
> >> -	    '*max-postcopy-bandwidth': 'size',
> >> -            '*max-cpu-throttle':'uint8'} }
> >> +            '*max-postcopy-bandwidth': 'size',
> >> +            '*max-cpu-throttle':'uint8',
> >> +            '*sev-pdh':'str',
> >> +            '*sev-plat-cert': 'str',
> >> +            '*sev-amd-cert' : 'str'} }
> >>  
> >>  ##
> >>  # @query-migrate-parameters:
> >> -- 
> >> 2.17.1
> >>
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 07/14] target/i386: sev: provide callback to setup outgoing context
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 07/14] target/i386: sev: provide callback to setup outgoing context Singh, Brijesh
@ 2019-08-08 11:19   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-08 11:19 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> The user provides the target machine's Platform Diffie-Hellman key (PDH)
> and certificate chain before starting the SEV guest migration. Cache the
> certificate chain as we need them while creating the outgoing context.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  accel/kvm/kvm-all.c    | 12 +++++++++++
>  accel/kvm/sev-stub.c   |  6 ++++++
>  include/sysemu/sev.h   |  2 ++
>  target/i386/sev.c      | 45 ++++++++++++++++++++++++++++++++++++++++++
>  target/i386/sev_i386.h |  6 ++++++
>  5 files changed, 71 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index f450f25295..d0304c6947 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -165,6 +165,17 @@ bool kvm_memcrypt_enabled(void)
>      return false;
>  }
>  
> +static int kvm_memcrypt_save_setup(const char *pdh, const char *plat_cert,
> +                                   const char *amd_cert)
> +{
> +    return sev_save_setup(kvm_state->memcrypt_handle, pdh,
> +                          plat_cert, amd_cert);
> +}
> +
> +static struct MachineMemoryEncryptionOps sev_memory_encryption_ops = {
> +    .save_setup = kvm_memcrypt_save_setup,
> +};
> +
>  int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len)
>  {
>      if (kvm_state->memcrypt_handle &&
> @@ -1968,6 +1979,7 @@ static int kvm_init(MachineState *ms)
>          }
>  
>          kvm_state->memcrypt_encrypt_data = sev_encrypt_data;
> +        mc->memory_encryption_ops = &sev_memory_encryption_ops;

It surprises me that this isn't in target/i386/kvm.c somehow

>      }
>  
>      ret = kvm_arch_init(ms, s);
> diff --git a/accel/kvm/sev-stub.c b/accel/kvm/sev-stub.c
> index 4f97452585..528f8cf7f1 100644
> --- a/accel/kvm/sev-stub.c
> +++ b/accel/kvm/sev-stub.c
> @@ -24,3 +24,9 @@ void *sev_guest_init(const char *id)
>  {
>      return NULL;
>  }
> +
> +int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
> +                   const char *amd_cert)
> +{
> +    return 1;
> +}
> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
> index 98c1ec8d38..d5123d4fa3 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_save_setup(void *handle, const char *pdh, const char *plat_cert,
> +                   const char *amd_cert);
>  #endif
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index f1423cb0c0..70e9d86815 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -27,6 +27,7 @@
>  #include "sysemu/sysemu.h"
>  #include "trace.h"
>  #include "migration/blocker.h"
> +#include "migration/qemu-file.h"

Do you need that yet?

>  #define DEFAULT_GUEST_POLICY    0x1 /* disable debug */
>  #define DEFAULT_SEV_DEVICE      "/dev/sev"
> @@ -62,6 +63,8 @@ static const char *const sev_fw_errlist[] = {
>  
>  #define SEV_FW_MAX_ERROR      ARRAY_SIZE(sev_fw_errlist)
>  
> +#define SEV_FW_BLOB_MAX_SIZE            0x4000          /* 16KB */
> +
>  static int
>  sev_ioctl(int fd, int cmd, void *data, int *error)
>  {
> @@ -729,6 +732,48 @@ sev_vm_state_change(void *opaque, int running, RunState state)
>      }
>  }
>  
> +static inline bool check_blob_length(size_t value)
> +{
> +    if (value > SEV_FW_BLOB_MAX_SIZE) {
> +        error_report("invalid length max=%ld got=%d",
> +                     value, SEV_FW_BLOB_MAX_SIZE);

Those two parameters are the wrong way around aren't they?

> +        return false;
> +    }
> +
> +    return true;
> +}
> +
> +int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
> +                   const char *amd_cert)
> +{
> +    SEVState *s = (SEVState *)handle;
> +
> +    s->remote_pdh = g_base64_decode(pdh, &s->remote_pdh_len);
> +    if (!check_blob_length(s->remote_pdh_len)) {

Print something to say what went wrong.

> +        goto error;
> +    }
> +
> +    s->remote_plat_cert = g_base64_decode(plat_cert,
> +                                          &s->remote_plat_cert_len);
> +    if (!check_blob_length(s->remote_plat_cert_len)) {
> +        goto error;
> +    }
> +
> +    s->amd_cert = g_base64_decode(amd_cert, &s->amd_cert_len);
> +    if (!check_blob_length(s->amd_cert_len)) {
> +        goto error;
> +    }
> +
> +    return 0;
> +
> +error:
> +    g_free(s->remote_pdh);
> +    g_free(s->remote_plat_cert);
> +    g_free(s->amd_cert);
> +
> +    return 1;
> +}
> +
>  void *
>  sev_guest_init(const char *id)
>  {
> diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
> index 55313441ae..32906de998 100644
> --- a/target/i386/sev_i386.h
> +++ b/target/i386/sev_i386.h
> @@ -81,6 +81,12 @@ struct SEVState {
>      int sev_fd;
>      SevState state;
>      gchar *measurement;
> +    guchar *remote_pdh;
> +    size_t remote_pdh_len;
> +    guchar *remote_plat_cert;
> +    size_t remote_plat_cert_len;
> +    guchar *amd_cert;
> +    size_t amd_cert_len;
>  };
>  
>  typedef struct SEVState SEVState;
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 09/14] target/i386: sev: add support to encrypt the outgoing page
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 09/14] target/i386: sev: add support to encrypt the outgoing page Singh, Brijesh
@ 2019-08-09 18:54   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-09 18:54 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> The sev_save_outgoing_page() provide the implementation to encrypt the
> guest private pages during the transit. The routines uses the SEND_START
> command to create the outgoing encryption context on the first call then
> uses the SEND_UPDATE_DATA command to encrypt the data before writing it
> to the socket. While encrypting the data SEND_UPDATE_DATA produces some
> metadata (e.g MAC, IV). The metadata is also sent to the target machine.
> After migration is completed, we issue the SEND_FINISH command to transition
> the SEV guest state from sending to unrunnable state.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  accel/kvm/kvm-all.c      |   9 ++
>  accel/kvm/sev-stub.c     |   6 ++
>  include/sysemu/sev.h     |   2 +
>  target/i386/sev.c        | 216 +++++++++++++++++++++++++++++++++++++++
>  target/i386/sev_i386.h   |   2 +
>  target/i386/trace-events |   3 +
>  6 files changed, 238 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index d0304c6947..a5b0ae9363 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -172,8 +172,17 @@ static int kvm_memcrypt_save_setup(const char *pdh, const char *plat_cert,
>                            plat_cert, amd_cert);
>  }
>  
> +static int kvm_memcrypt_save_outgoing_page(QEMUFile *f, uint8_t *ptr,
> +                                           uint32_t size,
> +                                           uint64_t *bytes_sent)
> +{
> +    return sev_save_outgoing_page(kvm_state->memcrypt_handle, f, ptr, size,
> +                                  bytes_sent);
> +}
> +
>  static struct MachineMemoryEncryptionOps sev_memory_encryption_ops = {
>      .save_setup = kvm_memcrypt_save_setup,
> +    .save_outgoing_page = kvm_memcrypt_save_outgoing_page,
>  };
>  
>  int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len)
> diff --git a/accel/kvm/sev-stub.c b/accel/kvm/sev-stub.c
> index 528f8cf7f1..51b17b8141 100644
> --- a/accel/kvm/sev-stub.c
> +++ b/accel/kvm/sev-stub.c
> @@ -30,3 +30,9 @@ int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
>  {
>      return 1;
>  }
> +
> +int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
> +                           uint32_t size, uint64_t *bytes_sent)
> +{
> +    return 1;
> +}
> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
> index d5123d4fa3..f06fd203cd 100644
> --- a/include/sysemu/sev.h
> +++ b/include/sysemu/sev.h
> @@ -20,4 +20,6 @@ void *sev_guest_init(const char *id);
>  int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
>  int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
>                     const char *amd_cert);
> +int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
> +                           uint32_t size, uint64_t *bytes_sent);
>  #endif
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 483d9bb0fa..1820c62a71 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -28,6 +28,7 @@
>  #include "trace.h"
>  #include "migration/blocker.h"
>  #include "migration/qemu-file.h"
> +#include "migration/misc.h"
>  
>  #define DEFAULT_GUEST_POLICY    0x1 /* disable debug */
>  #define DEFAULT_SEV_DEVICE      "/dev/sev"
> @@ -774,6 +775,40 @@ error:
>      return 1;
>  }
>  
> +static void
> +sev_send_finish(void)
> +{
> +    int ret, error;
> +
> +    trace_kvm_sev_send_finish();
> +    ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_SEND_FINISH, 0, &error);
> +    if (ret) {
> +        error_report("%s: SEND_FINISH ret=%d fw_error=%d '%s'",
> +                     __func__, ret, error, fw_error_to_str(error));
> +    }
> +
> +    g_free(sev_state->send_packet_hdr);
> +    sev_set_guest_state(SEV_STATE_RUNNING);
> +}
> +
> +static void
> +sev_migration_state_notifier(Notifier *notifier, void *data)
> +{
> +    MigrationState *s = data;
> +
> +    if (migration_has_finished(s) ||
> +        migration_in_postcopy_after_devices(s) ||
> +        migration_has_failed(s)) {
> +        if (sev_check_state(SEV_STATE_SEND_UPDATE)) {
> +            sev_send_finish();
> +        }
> +    }
> +}
> +
> +static Notifier sev_migration_state_notify = {
> +    .notify = sev_migration_state_notifier,
> +};
> +
>  void *
>  sev_guest_init(const char *id)
>  {
> @@ -860,6 +895,7 @@ sev_guest_init(const char *id)
>      ram_block_notifier_add(&sev_ram_notifier);
>      qemu_add_machine_init_done_notifier(&sev_machine_done_notify);
>      qemu_add_vm_change_state_handler(sev_vm_state_change, s);
> +    add_migration_state_change_notifier(&sev_migration_state_notify);
>  
>      return s;
>  err:
> @@ -881,6 +917,186 @@ sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len)
>      return 0;
>  }
>  
> +static int
> +sev_get_send_session_length(void)
> +{
> +    int ret, fw_err = 0;
> +    struct kvm_sev_send_start start = {};
> +
> +    ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_SEND_START, &start, &fw_err);
> +    if (fw_err != SEV_RET_INVALID_LEN) {
> +        ret = -1;
> +        error_report("%s: failed to get session length ret=%d fw_error=%d '%s'",
> +                     __func__, ret, fw_err, fw_error_to_str(fw_err));
> +        goto err;
> +    }
> +
> +    ret = start.session_len;
> +err:
> +    return ret;
> +}
> +
> +static int
> +sev_send_start(SEVState *s, QEMUFile *f, uint64_t *bytes_sent)
> +{
> +    gsize pdh_len = 0, plat_cert_len;
> +    int session_len, ret, fw_error;
> +    struct kvm_sev_send_start start = { };
> +    guchar *pdh = NULL, *plat_cert = NULL, *session = NULL;
> +
> +    if (!s->remote_pdh || !s->remote_plat_cert || !s->amd_cert_len) {
> +        error_report("%s: missing remote PDH or PLAT_CERT", __func__);
> +        return 1;
> +    }
> +
> +    start.pdh_cert_uaddr = (uintptr_t) s->remote_pdh;
> +    start.pdh_cert_len = s->remote_pdh_len;
> +
> +    start.plat_cert_uaddr = (uintptr_t)s->remote_plat_cert;
> +    start.plat_cert_len = s->remote_plat_cert_len;
> +
> +    start.amd_cert_uaddr = (uintptr_t)s->amd_cert;
> +    start.amd_cert_len = s->amd_cert_len;
> +
> +    /* get the session length */
> +    session_len = sev_get_send_session_length();
> +    if (session_len < 0) {
> +        ret = 1;
> +        goto err;
> +    }
> +
> +    session = g_new0(guchar, session_len);
> +    start.session_uaddr = (unsigned long)session;
> +    start.session_len = session_len;
> +
> +    /* Get our PDH certificate */
> +    ret = sev_get_pdh_info(s->sev_fd, &pdh, &pdh_len,
> +                           &plat_cert, &plat_cert_len);
> +    if (ret) {
> +        error_report("Failed to get our PDH cert");
> +        goto err;
> +    }
> +
> +    trace_kvm_sev_send_start(start.pdh_cert_uaddr, start.pdh_cert_len,
> +                             start.plat_cert_uaddr, start.plat_cert_len,
> +                             start.amd_cert_uaddr, start.amd_cert_len);
> +
> +    ret = sev_ioctl(s->sev_fd, KVM_SEV_SEND_START, &start, &fw_error);
> +    if (ret < 0) {
> +        error_report("%s: SEND_START ret=%d fw_error=%d '%s'",
> +                __func__, ret, fw_error, fw_error_to_str(fw_error));
> +        goto err;
> +    }
> +
> +    qemu_put_be32(f, start.policy);
> +    qemu_put_be32(f, pdh_len);
> +    qemu_put_buffer(f, (uint8_t *)pdh, pdh_len);
> +    qemu_put_be32(f, start.session_len);
> +    qemu_put_buffer(f, (uint8_t *)start.session_uaddr, start.session_len);
> +    *bytes_sent = 12 + pdh_len + start.session_len;
> +
> +    sev_set_guest_state(SEV_STATE_SEND_UPDATE);
> +
> +err:
> +    g_free(pdh);
> +    g_free(plat_cert);
> +    return ret;
> +}
> +
> +static int
> +sev_send_get_packet_len(int *fw_err)
> +{
> +    int ret;
> +    struct kvm_sev_send_update_data update = {};
> +
> +    ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_SEND_UPDATE_DATA,
> +                    &update, fw_err);
> +    if (*fw_err != SEV_RET_INVALID_LEN) {
> +        ret = -1;
> +        error_report("%s: failed to get session length ret=%d fw_error=%d '%s'",
> +                    __func__, ret, *fw_err, fw_error_to_str(*fw_err));
> +        goto err;
> +    }
> +
> +    ret = update.hdr_len;
> +
> +err:
> +    return ret;
> +}
> +
> +static int
> +sev_send_update_data(SEVState *s, QEMUFile *f, uint8_t *ptr, uint32_t size,
> +                     uint64_t *bytes_sent)
> +{
> +    int ret, fw_error;
> +    guchar *trans;
> +    struct kvm_sev_send_update_data update = { };
> +
> +    /*
> +     * If this is first call then query the packet header bytes and allocate
> +     * the packet buffer.
> +     */
> +    if (!s->send_packet_hdr) {
> +        s->send_packet_hdr_len = sev_send_get_packet_len(&fw_error);
> +        if (s->send_packet_hdr_len < 1) {
> +            error_report("%s: SEND_UPDATE fw_error=%d '%s'",
> +                    __func__, fw_error, fw_error_to_str(fw_error));
> +            return 1;
> +        }
> +
> +        s->send_packet_hdr = g_new(gchar, s->send_packet_hdr_len);
> +    }
> +
> +    /* allocate transport buffer */
> +    trans = g_new(guchar, size);
> +
> +    update.hdr_uaddr = (uintptr_t)s->send_packet_hdr;
> +    update.hdr_len = s->send_packet_hdr_len;
> +    update.guest_uaddr = (uintptr_t)ptr;
> +    update.guest_len = size;
> +    update.trans_uaddr = (uintptr_t)trans;
> +    update.trans_len = size;
> +
> +    trace_kvm_sev_send_update_data(ptr, trans, size);
> +
> +    ret = sev_ioctl(s->sev_fd, KVM_SEV_SEND_UPDATE_DATA, &update, &fw_error);
> +    if (ret) {
> +        error_report("%s: SEND_UPDATE_DATA ret=%d fw_error=%d '%s'",
> +                __func__, ret, fw_error, fw_error_to_str(fw_error));
> +        goto err;
> +    }
> +
> +    qemu_put_be32(f, update.hdr_len);
> +    qemu_put_buffer(f, (uint8_t *)update.hdr_uaddr, update.hdr_len);
> +    *bytes_sent = 4 + update.hdr_len;
> +
> +    qemu_put_be32(f, update.trans_len);
> +    qemu_put_buffer(f, (uint8_t *)update.trans_uaddr, update.trans_len);
> +    *bytes_sent += (4 + update.trans_len);
> +
> +err:
> +    g_free(trans);
> +    return ret;
> +}
> +
> +int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
> +                           uint32_t sz, uint64_t *bytes_sent)
> +{
> +    SEVState *s = sev_state;
> +
> +    /*
> +     * If this is a first buffer then create outgoing encryption context
> +     * and write our PDH, policy and session data.
> +     */
> +    if (!sev_check_state(SEV_STATE_SEND_UPDATE) &&
> +        sev_send_start(s, f, bytes_sent)) {
> +        error_report("Failed to create outgoing context");
> +        return 1;
> +    }
> +
> +    return sev_send_update_data(s, f, ptr, sz, bytes_sent);
> +}
> +
>  static void
>  sev_register_types(void)
>  {
> diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
> index 32906de998..e475304f5f 100644
> --- a/target/i386/sev_i386.h
> +++ b/target/i386/sev_i386.h
> @@ -87,6 +87,8 @@ struct SEVState {
>      size_t remote_plat_cert_len;
>      guchar *amd_cert;
>      size_t amd_cert_len;
> +    gchar *send_packet_hdr;
> +    size_t send_packet_hdr_len;
>  };
>  
>  typedef struct SEVState SEVState;
> diff --git a/target/i386/trace-events b/target/i386/trace-events
> index 789c700d4a..b41516cf9f 100644
> --- a/target/i386/trace-events
> +++ b/target/i386/trace-events
> @@ -15,3 +15,6 @@ kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session
>  kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIu64
>  kvm_sev_launch_measurement(const char *value) "data %s"
>  kvm_sev_launch_finish(void) ""
> +kvm_sev_send_start(uint64_t pdh, int l1, uint64_t plat, int l2, uint64_t amd, int l3) "pdh 0x%" PRIx64 " len %d plat 0x%" PRIx64 " len %d amd 0x%" PRIx64 " len %d"
> +kvm_sev_send_update_data(void *src, void *dst, int len) "guest %p trans %p len %d"
> +kvm_sev_send_finish(void) ""
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters
  2019-08-08 10:48       ` Dr. David Alan Gilbert
@ 2019-08-09 20:00         ` Singh, Brijesh
  0 siblings, 0 replies; 29+ messages in thread
From: Singh, Brijesh @ 2019-08-09 20:00 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: pbonzini, Lendacky, Thomas, Singh, Brijesh, qemu-devel, ehabkost


On 8/8/19 5:48 AM, Dr. David Alan Gilbert wrote:
> * Singh, Brijesh (brijesh.singh@amd.com) wrote:
>> On 8/7/19 6:06 AM, Dr. David Alan Gilbert wrote:
>>> * Singh, Brijesh (brijesh.singh@amd.com) wrote:
>>>> AMD SEV migration flow requires that target machine's public Diffie-Hellman
>>>> key (PDH) and certificate chain must be passed before initiating the guest
>>>> migration. User can use QMP 'migrate-set-parameters' to pass the certificate
>>>> chain. The certificate chain will be used while creating the outgoing
>>>> encryption context.
>>>>
>>>> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
>>>> ---
>>>>
>>>> I was able to pass the certificate chain through the HMP but somehow
>>>> QMP socket interface is not working for me. If anyone has any tips on
>>>> what I am missing in the patch then please let me know. In meantime,
>>>> I will also continue my investigation on why its not working for me.
>>> It looks OK to me; what's the qmp you're trying and what's the failure
>>> error?
> Before I forget, you've not updated hmp_info_migrate_parameters in
> hmp-cmds.c, e.g.:
>
>              MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
>              params->has_tls_authz ? params->tls_authz : "");
> +        monitor_printf(mon, "%s:'%s'\n",
> +            MigrationParameter_str(MIGRATION_PARAMETER_SEV_PDH),
> +            params->has_sev_pdh ? params->sev_pdh : "");
>      }


thanks, i will address it in next rev.


>> I am not seeing any error. I am using the below command through qmp-shell.
>>
>> (qmp) migrate-set-paramaters sev-pdh="...." sev-plat-cert="...."
>> sev-amd-cert="..."
>>
>>
>> The command does not return any error. I added some debugs in
>> migrate_params_test_apply() and qmp_migrate_set_parameters() to see the
>> valye of params->has_sev_pdh and its always zero. The functions are
>> getting called when I issue the migrate-set-parameters qmp but the
>> values are not passed hence the memory_encryption->setup() never gets
>> called.
> Driving QMP by hand I'm seeing it apparently be stored in the
> parameters:
>
> Escape character is '^]'.
> {"QMP": {"version": {"qemu": {"micro": 94, "minor": 0, "major": 4}, "package": "v4.1.0-rc4-16-ge6e1f28afd-dirty"}, "capabilities": ["oob"]}}
> { "execute": "qmp_capabilities" }
> {"return": {}}
> { "execute": "migrate-set-parameters" , "arguments": { "sev-pdh": "foo" } }
> {"return": {}}
>
> then from HMP with the patch above:
> (qemu) info migrate_parameters 
> ....
> sev-pdh:'foo'


Hmm, I have been using scripts to automate all these certificating
passing. let me closely look into it and see if its script bug.


>>>>  void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>>>> @@ -1410,6 +1443,27 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
>>>>          params->tls_hostname->type = QTYPE_QSTRING;
>>>>          params->tls_hostname->u.s = strdup("");
>>>>      }
>>>> +    /* TODO Rewrite "" to null instead */
>>>> +    if (params->has_sev_pdh
>>>> +        && params->sev_pdh->type == QTYPE_QNULL) {
>>>> +        qobject_unref(params->sev_pdh->u.n);
>>>> +        params->sev_pdh->type = QTYPE_QSTRING;
>>>> +        params->sev_pdh->u.s = strdup("");
>>>> +    }
>>>> +    /* TODO Rewrite "" to null instead */
>>>> +    if (params->has_sev_plat_cert
>>>> +        && params->sev_plat_cert->type == QTYPE_QNULL) {
>>>> +        qobject_unref(params->sev_plat_cert->u.n);
>>>> +        params->sev_plat_cert->type = QTYPE_QSTRING;
>>>> +        params->sev_plat_cert->u.s = strdup("");
>>>> +    }
>>>> +    /* TODO Rewrite "" to null instead */
>>>> +    if (params->has_sev_amd_cert
>>>> +        && params->sev_amd_cert->type == QTYPE_QNULL) {
>>>> +        qobject_unref(params->sev_amd_cert->u.n);
>>>> +        params->sev_amd_cert->type = QTYPE_QSTRING;
>>>> +        params->sev_amd_cert->u.s = strdup("");
>>>> +    }
>>>>  
>>>>      migrate_params_test_apply(params, &tmp);
>>>>  
>>>> @@ -3466,6 +3520,9 @@ static void migration_instance_finalize(Object *obj)
>>>>      qemu_mutex_destroy(&ms->qemu_file_lock);
>>>>      g_free(params->tls_hostname);
>>>>      g_free(params->tls_creds);
>>>> +    g_free(params->sev_pdh);
>>>> +    g_free(params->sev_plat_cert);
>>>> +    g_free(params->sev_amd_cert);
>>>>      qemu_sem_destroy(&ms->rate_limit_sem);
>>>>      qemu_sem_destroy(&ms->pause_sem);
>>>>      qemu_sem_destroy(&ms->postcopy_pause_sem);
>>>> @@ -3507,6 +3564,10 @@ static void migration_instance_init(Object *obj)
>>>>      params->has_announce_rounds = true;
>>>>      params->has_announce_step = true;
>>>>  
>>>> +    params->sev_pdh = g_strdup("");
>>>> +    params->sev_plat_cert = g_strdup("");
>>>> +    params->sev_amd_cert = g_strdup("");
>>>> +
>>>>      qemu_sem_init(&ms->postcopy_pause_sem, 0);
>>>>      qemu_sem_init(&ms->postcopy_pause_rp_sem, 0);
>>>>      qemu_sem_init(&ms->rp_state.rp_sem, 0);
>>>> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>>>> index 5ca3ebe942..354219f27a 100644
>>>> --- a/monitor/hmp-cmds.c
>>>> +++ b/monitor/hmp-cmds.c
>>>> @@ -1872,6 +1872,24 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>>>>          p->has_announce_step = true;
>>>>          visit_type_size(v, param, &p->announce_step, &err);
>>>>          break;
>>>> +    case MIGRATION_PARAMETER_SEV_PDH:
>>>> +        p->has_sev_pdh = true;
>>>> +        p->sev_pdh = g_new0(StrOrNull, 1);
>>>> +        p->sev_pdh->type = QTYPE_QSTRING;
>>>> +        visit_type_str(v, param, &p->sev_pdh->u.s, &err);
>>>> +        break;
>>>> +    case MIGRATION_PARAMETER_SEV_PLAT_CERT:
>>>> +        p->has_sev_plat_cert = true;
>>>> +        p->sev_plat_cert = g_new0(StrOrNull, 1);
>>>> +        p->sev_plat_cert->type = QTYPE_QSTRING;
>>>> +        visit_type_str(v, param, &p->sev_plat_cert->u.s, &err);
>>>> +        break;
>>>> +    case MIGRATION_PARAMETER_SEV_AMD_CERT:
>>>> +        p->has_sev_amd_cert = true;
>>>> +        p->sev_amd_cert = g_new0(StrOrNull, 1);
>>>> +        p->sev_amd_cert->type = QTYPE_QSTRING;
>>>> +        visit_type_str(v, param, &p->sev_amd_cert->u.s, &err);
>>>> +        break;
>>>>      default:
>>>>          assert(0);
>>>>      }
>>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>>> index 9cfbaf8c6c..bb07995d2c 100644
>>>> --- a/qapi/migration.json
>>>> +++ b/qapi/migration.json
>>>> @@ -580,6 +580,15 @@
>>>>  # @max-cpu-throttle: maximum cpu throttle percentage.
>>>>  #                    Defaults to 99. (Since 3.1)
>>>>  #
>>>> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
>>>> +#           (Since 4.2)
>>>> +#
>>>> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
>>>> +#                 (Since 4.2)
>>>> +#
>>>> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
>>>> +#                base64 (Since 4.2)
>>>> +#
>>>>  # Since: 2.4
>>>>  ##
>>>>  { 'enum': 'MigrationParameter',
>>>> @@ -592,7 +601,7 @@
>>>>             'downtime-limit', 'x-checkpoint-delay', 'block-incremental',
>>>>             'multifd-channels',
>>>>             'xbzrle-cache-size', 'max-postcopy-bandwidth',
>>>> -           'max-cpu-throttle' ] }
>>>> +           'max-cpu-throttle', 'sev-pdh', 'sev-plat-cert', 'sev-amd-cert' ] }
>>>>  
>>>>  ##
>>>>  # @MigrateSetParameters:
>>>> @@ -682,6 +691,15 @@
>>>>  # @max-cpu-throttle: maximum cpu throttle percentage.
>>>>  #                    The default value is 99. (Since 3.1)
>>>>  #
>>>> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
>>>> +#           (Since 4.2)
>>>> +#
>>>> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
>>>> +#                 (Since 4.2)
>>>> +#
>>>> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
>>>> +#                base64 (Since 4.2)
>>>> +#
>>>>  # Since: 2.4
>>>>  ##
>>>>  # TODO either fuse back into MigrationParameters, or make
>>>> @@ -707,7 +725,10 @@
>>>>              '*multifd-channels': 'int',
>>>>              '*xbzrle-cache-size': 'size',
>>>>              '*max-postcopy-bandwidth': 'size',
>>>> -	    '*max-cpu-throttle': 'int' } }
>>>> +            '*max-cpu-throttle': 'int',
>>>> +            '*sev-pdh':'StrOrNull',
>>>> +            '*sev-plat-cert': 'StrOrNull',
>>>> +            '*sev-amd-cert' : 'StrOrNull' } }
>>>>  
>>>>  ##
>>>>  # @migrate-set-parameters:
>>>> @@ -817,6 +838,15 @@
>>>>  #                    Defaults to 99.
>>>>  #                     (Since 3.1)
>>>>  #
>>>> +# @sev-pdh: The target host platform diffie-hellman key encoded in base64
>>>> +#           (Since 4.2)
>>>> +#
>>>> +# @sev-plat-cert: The target host platform certificate chain encoded in base64
>>>> +#                 (Since 4.2)
>>>> +#
>>>> +# @sev-amd-cert: AMD certificate chain which include ASK and OCA encoded in
>>>> +#                base64 (Since 4.2)
>>>> +#
>>>>  # Since: 2.4
>>>>  ##
>>>>  { 'struct': 'MigrationParameters',
>>>> @@ -839,8 +869,11 @@
>>>>              '*block-incremental': 'bool' ,
>>>>              '*multifd-channels': 'uint8',
>>>>              '*xbzrle-cache-size': 'size',
>>>> -	    '*max-postcopy-bandwidth': 'size',
>>>> -            '*max-cpu-throttle':'uint8'} }
>>>> +            '*max-postcopy-bandwidth': 'size',
>>>> +            '*max-cpu-throttle':'uint8',
>>>> +            '*sev-pdh':'str',
>>>> +            '*sev-plat-cert': 'str',
>>>> +            '*sev-amd-cert' : 'str'} }
>>>>  
>>>>  ##
>>>>  # @query-migrate-parameters:
>>>> -- 
>>>> 2.17.1
>>>>
>>> --
>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 10/14] target/i386: sev: add support to load incoming encrypted page
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 10/14] target/i386: sev: add support to load incoming encrypted page Singh, Brijesh
@ 2019-08-13 17:38   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-13 17:38 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> The sev_load_incoming_page() provide the implementation to read the
> incoming guest private pages from the socket and load it into the guest
> memory. The routines uses the RECEIVE_START command to create the
> incoming encryption context on the first call then uses the
> RECEIEVE_UPDATE_DATA command to load the encrypted pages into the guest
> memory. After migration is completed, we issue the RECEIVE_FINISH command
> to transition the SEV guest to the runnable state so that it can be
> executed.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>

OK, some comments about the return values of the functions would help,
but other than that.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  accel/kvm/kvm-all.c      |   6 ++
>  accel/kvm/sev-stub.c     |   5 ++
>  include/sysemu/sev.h     |   1 +
>  target/i386/sev.c        | 137 ++++++++++++++++++++++++++++++++++++++-
>  target/i386/trace-events |   3 +
>  5 files changed, 151 insertions(+), 1 deletion(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index a5b0ae9363..ba0e7fa2be 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -180,9 +180,15 @@ static int kvm_memcrypt_save_outgoing_page(QEMUFile *f, uint8_t *ptr,
>                                    bytes_sent);
>  }
>  
> +static int kvm_memcrypt_load_incoming_page(QEMUFile *f, uint8_t *ptr)
> +{
> +    return sev_load_incoming_page(kvm_state->memcrypt_handle, f, ptr);
> +}
> +
>  static struct MachineMemoryEncryptionOps sev_memory_encryption_ops = {
>      .save_setup = kvm_memcrypt_save_setup,
>      .save_outgoing_page = kvm_memcrypt_save_outgoing_page,
> +    .load_incoming_page = kvm_memcrypt_load_incoming_page,
>  };
>  
>  int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len)
> diff --git a/accel/kvm/sev-stub.c b/accel/kvm/sev-stub.c
> index 51b17b8141..1b6773ef72 100644
> --- a/accel/kvm/sev-stub.c
> +++ b/accel/kvm/sev-stub.c
> @@ -36,3 +36,8 @@ int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
>  {
>      return 1;
>  }
> +
> +int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr)
> +{
> +    return 1;
> +}
> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
> index f06fd203cd..e9371bd2dd 100644
> --- a/include/sysemu/sev.h
> +++ b/include/sysemu/sev.h
> @@ -22,4 +22,5 @@ int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
>                     const char *amd_cert);
>  int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
>                             uint32_t size, uint64_t *bytes_sent);
> +int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr);
>  #endif
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 1820c62a71..a689011991 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -721,13 +721,34 @@ sev_launch_finish(SEVState *s)
>      }
>  }
>  
> +static int
> +sev_receive_finish(SEVState *s)
> +{
> +    int error, ret = 1;
> +
> +    trace_kvm_sev_receive_finish();
> +    ret = sev_ioctl(s->sev_fd, KVM_SEV_RECEIVE_FINISH, 0, &error);
> +    if (ret) {
> +        error_report("%s: RECEIVE_FINISH ret=%d fw_error=%d '%s'",
> +                __func__, ret, error, fw_error_to_str(error));
> +        goto err;
> +    }
> +
> +    sev_set_guest_state(SEV_STATE_RUNNING);
> +err:
> +    return ret;
> +}
> +
> +
>  static void
>  sev_vm_state_change(void *opaque, int running, RunState state)
>  {
>      SEVState *s = opaque;
>  
>      if (running) {
> -        if (!sev_check_state(SEV_STATE_RUNNING)) {
> +        if (sev_check_state(SEV_STATE_RECEIVE_UPDATE)) {
> +            sev_receive_finish(s);
> +        } else if (!sev_check_state(SEV_STATE_RUNNING)) {
>              sev_launch_finish(s);
>          }
>      }
> @@ -1097,6 +1118,120 @@ int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
>      return sev_send_update_data(s, f, ptr, sz, bytes_sent);
>  }
>  
> +static int
> +sev_receive_start(QSevGuestInfo *sev, QEMUFile *f)
> +{
> +    int ret = 1;
> +    int fw_error;
> +    struct kvm_sev_receive_start start = { };
> +    gchar *session = NULL, *pdh_cert = NULL;
> +
> +    /* get SEV guest handle */
> +    start.handle = object_property_get_int(OBJECT(sev), "handle",
> +                                           &error_abort);
> +
> +    /* get the source policy */
> +    start.policy = qemu_get_be32(f);
> +
> +    /* get source PDH key */
> +    start.pdh_len = qemu_get_be32(f);
> +    if (!check_blob_length(start.pdh_len)) {
> +        return 1;
> +    }
> +
> +    pdh_cert = g_new(gchar, start.pdh_len);
> +    qemu_get_buffer(f, (uint8_t *)pdh_cert, start.pdh_len);
> +    start.pdh_uaddr = (uintptr_t)pdh_cert;
> +
> +    /* get source session data */
> +    start.session_len = qemu_get_be32(f);
> +    if (!check_blob_length(start.session_len)) {
> +        return 1;
> +    }
> +    session = g_new(gchar, start.session_len);
> +    qemu_get_buffer(f, (uint8_t *)session, start.session_len);
> +    start.session_uaddr = (uintptr_t)session;
> +
> +    trace_kvm_sev_receive_start(start.policy, session, pdh_cert);
> +
> +    ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_RECEIVE_START,
> +                    &start, &fw_error);
> +    if (ret < 0) {
> +        error_report("Error RECEIVE_START ret=%d fw_error=%d '%s'",
> +                ret, fw_error, fw_error_to_str(fw_error));
> +        goto err;
> +    }
> +
> +    object_property_set_int(OBJECT(sev), start.handle, "handle", &error_abort);
> +    sev_set_guest_state(SEV_STATE_RECEIVE_UPDATE);
> +err:
> +    g_free(session);
> +    g_free(pdh_cert);
> +
> +    return ret;
> +}
> +
> +static int sev_receive_update_data(QEMUFile *f, uint8_t *ptr)
> +{
> +    int ret = 1, fw_error = 0;
> +    gchar *hdr = NULL, *trans = NULL;
> +    struct kvm_sev_receive_update_data update = {};
> +
> +    /* get packet header */
> +    update.hdr_len = qemu_get_be32(f);
> +    if (!check_blob_length(update.hdr_len)) {
> +        return 1;
> +    }
> +
> +    hdr = g_new(gchar, update.hdr_len);
> +    qemu_get_buffer(f, (uint8_t *)hdr, update.hdr_len);
> +    update.hdr_uaddr = (uintptr_t)hdr;
> +
> +    /* get transport buffer */
> +    update.trans_len = qemu_get_be32(f);
> +    if (!check_blob_length(update.trans_len)) {
> +        goto err;
> +    }
> +
> +    trans = g_new(gchar, update.trans_len);
> +    update.trans_uaddr = (uintptr_t)trans;
> +    qemu_get_buffer(f, (uint8_t *)update.trans_uaddr, update.trans_len);
> +
> +    update.guest_uaddr = (uintptr_t) ptr;
> +    update.guest_len = update.trans_len;
> +
> +    trace_kvm_sev_receive_update_data(trans, ptr, update.guest_len,
> +            hdr, update.hdr_len);
> +
> +    ret = sev_ioctl(sev_state->sev_fd, KVM_SEV_RECEIVE_UPDATE_DATA,
> +                    &update, &fw_error);
> +    if (ret) {
> +        error_report("Error RECEIVE_UPDATE_DATA ret=%d fw_error=%d '%s'",
> +                ret, fw_error, fw_error_to_str(fw_error));
> +        goto err;
> +    }
> +err:
> +    g_free(trans);
> +    g_free(hdr);
> +    return ret;
> +}
> +
> +int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr)
> +{
> +    SEVState *s = (SEVState *)handle;
> +
> +    /*
> +     * If this is first buffer and SEV is not in recieiving state then
> +     * use RECEIVE_START command to create a encryption context.
> +     */
> +    if (!sev_check_state(SEV_STATE_RECEIVE_UPDATE) &&
> +        sev_receive_start(s->sev_info, f)) {
> +        return 1;
> +    }
> +
> +    return sev_receive_update_data(f, ptr);
> +}
> +
>  static void
>  sev_register_types(void)
>  {
> diff --git a/target/i386/trace-events b/target/i386/trace-events
> index b41516cf9f..609752cca7 100644
> --- a/target/i386/trace-events
> +++ b/target/i386/trace-events
> @@ -18,3 +18,6 @@ kvm_sev_launch_finish(void) ""
>  kvm_sev_send_start(uint64_t pdh, int l1, uint64_t plat, int l2, uint64_t amd, int l3) "pdh 0x%" PRIx64 " len %d plat 0x%" PRIx64 " len %d amd 0x%" PRIx64 " len %d"
>  kvm_sev_send_update_data(void *src, void *dst, int len) "guest %p trans %p len %d"
>  kvm_sev_send_finish(void) ""
> +kvm_sev_receive_start(int policy, void *session, void *pdh) "policy 0x%x session %p pdh %p"
> +kvm_sev_receive_update_data(void *src, void *dst, int len, void *hdr, int hdr_len) "guest %p trans %p len %d hdr %p hdr_len %d"
> +kvm_sev_receive_finish(void) ""
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page encryption bitmap
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page encryption bitmap Singh, Brijesh
@ 2019-08-13 18:57   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-13 18:57 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> When memory encryption is enabled, the hypervisor maintains a page
> encryption bitmap which is referred by hypervisor during migratoin to check
> if page is private or shared. The bitmap is built during the VM bootup and
> must be migrated to the target host so that hypervisor on target host can
> use it for future migration. The KVM_{SET,GET}_PAGE_ENC_BITMAP can be used
> to get and set the bitmap for a given gfn range.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  accel/kvm/kvm-all.c      | 27 ++++++++++++
>  accel/kvm/sev-stub.c     | 11 +++++
>  include/sysemu/sev.h     |  6 +++
>  target/i386/sev.c        | 93 ++++++++++++++++++++++++++++++++++++++++
>  target/i386/trace-events |  2 +
>  5 files changed, 139 insertions(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index ba0e7fa2be..f4d136b022 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -185,10 +185,37 @@ static int kvm_memcrypt_load_incoming_page(QEMUFile *f, uint8_t *ptr)
>      return sev_load_incoming_page(kvm_state->memcrypt_handle, f, ptr);
>  }
>  
> +static int kvm_memcrypt_save_outgoing_bitmap(QEMUFile *f)
> +{
> +    KVMMemoryListener *kml = &kvm_state->memory_listener;
> +    KVMState *s = kvm_state;
> +    int ret = 1, i;
> +
> +    /* iterate through all the registered slots and send the bitmap */
> +    for (i = 0; i < s->nr_slots; i++) {
> +        KVMSlot *mem = &kml->slots[i];
> +        ret = sev_save_outgoing_bitmap(s->memcrypt_handle, f, mem->start_addr,
> +                                       mem->memory_size,
> +                                       (i + 1) == s->nr_slots);
> +        if (ret) {
> +            return 1;
> +        }
> +    }
> +
> +    return ret;
> +}
> +
> +static int kvm_memcrypt_load_incoming_bitmap(QEMUFile *f)
> +{
> +    return sev_load_incoming_bitmap(kvm_state->memcrypt_handle, f);
> +}
> +
>  static struct MachineMemoryEncryptionOps sev_memory_encryption_ops = {
>      .save_setup = kvm_memcrypt_save_setup,
>      .save_outgoing_page = kvm_memcrypt_save_outgoing_page,
>      .load_incoming_page = kvm_memcrypt_load_incoming_page,
> +    .save_outgoing_bitmap = kvm_memcrypt_save_outgoing_bitmap,
> +    .load_incoming_bitmap = kvm_memcrypt_load_incoming_bitmap,
>  };
>  
>  int kvm_memcrypt_encrypt_data(uint8_t *ptr, uint64_t len)
> diff --git a/accel/kvm/sev-stub.c b/accel/kvm/sev-stub.c
> index 1b6773ef72..fa96225abc 100644
> --- a/accel/kvm/sev-stub.c
> +++ b/accel/kvm/sev-stub.c
> @@ -41,3 +41,14 @@ int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr)
>  {
>      return 1;
>  }
> +
> +int sev_save_outgoing_bitmap(void *handle, QEMUFile *f,
> +                             unsigned long start, uint64_t length, bool last)
> +{
> +    return 1;
> +}
> +
> +int sev_load_incoming_bitmap(void *handle, QEMUFile *f)
> +{
> +    return 1;
> +}
> diff --git a/include/sysemu/sev.h b/include/sysemu/sev.h
> index e9371bd2dd..f777083c94 100644
> --- a/include/sysemu/sev.h
> +++ b/include/sysemu/sev.h
> @@ -16,6 +16,9 @@
>  
>  #include "sysemu/kvm.h"
>  
> +#define RAM_SAVE_ENCRYPTED_PAGE        0x1
> +#define RAM_SAVE_ENCRYPTED_BITMAP      0x2
> +
>  void *sev_guest_init(const char *id);
>  int sev_encrypt_data(void *handle, uint8_t *ptr, uint64_t len);
>  int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
> @@ -23,4 +26,7 @@ int sev_save_setup(void *handle, const char *pdh, const char *plat_cert,
>  int sev_save_outgoing_page(void *handle, QEMUFile *f, uint8_t *ptr,
>                             uint32_t size, uint64_t *bytes_sent);
>  int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr);
> +int sev_load_incoming_bitmap(void *handle, QEMUFile *f);
> +int sev_save_outgoing_bitmap(void *handle, QEMUFile *f, unsigned long start,
> +                             uint64_t length, bool last);
>  #endif
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index a689011991..9d643e720c 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -65,6 +65,8 @@ static const char *const sev_fw_errlist[] = {
>  #define SEV_FW_MAX_ERROR      ARRAY_SIZE(sev_fw_errlist)
>  
>  #define SEV_FW_BLOB_MAX_SIZE            0x4000          /* 16KB */
> +#define ENCRYPTED_BITMAP_CONTINUE       0x1
> +#define ENCRYPTED_BITMAP_END            0x2
>  
>  static int
>  sev_ioctl(int fd, int cmd, void *data, int *error)
> @@ -1232,6 +1234,97 @@ int sev_load_incoming_page(void *handle, QEMUFile *f, uint8_t *ptr)
>      return sev_receive_update_data(f, ptr);
>  }
>  
> +#define ALIGN(x, y)  (((x) + (y) - 1) & ~((y) - 1))
> +
> +int sev_load_incoming_bitmap(void *handle, QEMUFile *f)
> +{
> +    void *bmap;
> +    unsigned long bmap_size, base_gpa;
> +    unsigned long npages, expected_size, length;
> +    struct kvm_page_enc_bitmap e = {};
> +    int status;
> +
> +    status = qemu_get_be32(f);
> +
> +    while (status != ENCRYPTED_BITMAP_END) {

It would be good to be more defensive - I think you're always expecting
ENCRYPTED_BITMAP_CONTINUE?   I'd error out if the status
isn't one of the few things you expect.

(I know I might be a bit paranoid, but I'm used to very broken migration
streams!)

> +        base_gpa = qemu_get_be64(f);
> +        npages = qemu_get_be64(f);
> +        bmap_size = qemu_get_be64(f);

It's also a useful place to try a qemu_file_get_error(f) - it helps spot
whether an error is really a broken header or the stream ran out of
bytes unexpectedly.

But what you have is OK, but never any harm to make it more defensive.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> +        /*
> +         * Before allocating the bitmap buffer, lets do some bound check to
> +         * ensure that we are not dealing with corrupted stream.
> +         */
> +        length = npages << TARGET_PAGE_BITS;
> +        expected_size = ALIGN((length >> TARGET_PAGE_BITS), 64) / 8;
> +        if (expected_size != bmap_size) {
> +            error_report("corrupted bitmap expected size %ld got %ld",
> +                    expected_size, bmap_size);
> +            return 1;
> +        }
> +
> +        bmap = g_malloc0(bmap_size);
> +        qemu_get_buffer(f, (uint8_t *)bmap, bmap_size);
> +
> +        trace_kvm_sev_load_bitmap(base_gpa, npages << TARGET_PAGE_BITS);
> +
> +        e.start_gfn = base_gpa >> TARGET_PAGE_BITS;
> +        e.num_pages = npages;
> +        e.enc_bitmap = bmap;
> +        if (kvm_vm_ioctl(kvm_state, KVM_SET_PAGE_ENC_BITMAP, &e) == -1) {
> +            error_report("KVM_SET_PAGE_ENC_BITMAP ioctl failed %d", errno);
> +            g_free(bmap);
> +            return 1;
> +        }
> +
> +        g_free(bmap);
> +
> +        status = qemu_get_be32(f);
> +    }
> +
> +    return 0;
> +}
> +
> +int sev_save_outgoing_bitmap(void *handle, QEMUFile *f,
> +                             unsigned long start, uint64_t length, bool last)
> +{
> +    uint64_t size;
> +    struct kvm_page_enc_bitmap e = {};
> +
> +    if (!length) {
> +        /* nothing to send */
> +        goto done;
> +    }
> +
> +    size = ALIGN((length >> TARGET_PAGE_BITS), 64) / 8;
> +    e.enc_bitmap = g_malloc0(size);
> +    e.start_gfn = start >> TARGET_PAGE_BITS;
> +    e.num_pages = length >> TARGET_PAGE_BITS;
> +
> +    trace_kvm_sev_save_bitmap(start, length);
> +
> +    if (kvm_vm_ioctl(kvm_state, KVM_GET_PAGE_ENC_BITMAP, &e) == -1) {
> +        error_report("%s: KVM_GET_PAGE_ENC_BITMAP ioctl failed %d",
> +                    __func__, errno);
> +        g_free(e.enc_bitmap);
> +        return 1;
> +    }
> +
> +    qemu_put_be32(f, ENCRYPTED_BITMAP_CONTINUE);
> +    qemu_put_be64(f, start);
> +    qemu_put_be64(f, e.num_pages);
> +    qemu_put_be64(f, size);
> +    qemu_put_buffer(f, (uint8_t *)e.enc_bitmap, size);
> +
> +    g_free(e.enc_bitmap);
> +
> +done:
> +    if (last) {
> +        qemu_put_be32(f, ENCRYPTED_BITMAP_END);
> +    }
> +    return 0;
> +}
> +
>  static void
>  sev_register_types(void)
>  {
> diff --git a/target/i386/trace-events b/target/i386/trace-events
> index 609752cca7..853a3870ab 100644
> --- a/target/i386/trace-events
> +++ b/target/i386/trace-events
> @@ -21,3 +21,5 @@ kvm_sev_send_finish(void) ""
>  kvm_sev_receive_start(int policy, void *session, void *pdh) "policy 0x%x session %p pdh %p"
>  kvm_sev_receive_update_data(void *src, void *dst, int len, void *hdr, int hdr_len) "guest %p trans %p len %d hdr %p hdr_len %d"
>  kvm_sev_receive_finish(void) ""
> +kvm_sev_save_bitmap(uint64_t start, uint64_t len) "start 0x%" PRIx64 " len 0x%" PRIx64
> +kvm_sev_load_bitmap(uint64_t start, uint64_t len) "start 0x%" PRIx64 " len 0x%" PRIx64
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [Qemu-devel] [PATCH v3 13/14] migration/ram: add support to send encrypted pages
  2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 13/14] migration/ram: add support to send encrypted pages Singh, Brijesh
@ 2019-08-14 16:37   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 29+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-14 16:37 UTC (permalink / raw)
  To: Singh, Brijesh; +Cc: pbonzini, Lendacky, Thomas, qemu-devel, ehabkost

* Singh, Brijesh (brijesh.singh@amd.com) wrote:
> When memory encryption is enabled, the guest memory will be encrypted with
> the guest specific key. The patch introduces RAM_SAVE_FLAG_ENCRYPTED_PAGE
> flag to distinguish the encrypted data from plaintext. Encrypted pages
> may need special handling. The kvm_memcrypt_save_outgoing_page() is used
> by the sender to write the encrypted pages onto the socket, similarly the
> kvm_memcrypt_load_incoming_page() is used by the target to read the
> encrypted pages from the socket and load into the guest memory.
> 
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  migration/ram.c | 131 +++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 130 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 57c707525b..100a5a10cd 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -59,6 +59,9 @@
>  #include "qemu/iov.h"
>  #include "hw/boards.h"
>  
> +/* Defines RAM_SAVE_ENCRYPTED_PAGE and  RAM_SAVE_ENCRYPTED_BITMAP */
> +#include "sysemu/sev.h"
> +
>  /***********************************************************/
>  /* ram save/restore */
>  
> @@ -77,6 +80,7 @@
>  #define RAM_SAVE_FLAG_XBZRLE   0x40
>  /* 0x80 is reserved in migration.h start with 0x100 next */
>  #define RAM_SAVE_FLAG_COMPRESS_PAGE    0x100
> +#define RAM_SAVE_FLAG_ENCRYPTED_DATA   0x200
>  
>  static inline bool is_zero_range(uint8_t *p, uint64_t size)
>  {
> @@ -460,6 +464,9 @@ static QemuCond decomp_done_cond;
>  
>  static bool do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block,
>                                   ram_addr_t offset, uint8_t *source_buf);
> +static int ram_save_encrypted_page(RAMState *rs, PageSearchStatus *pss,
> +                                   bool last_stage);
> +
>  
>  static void *do_data_compress(void *opaque)
>  {
> @@ -2039,6 +2046,73 @@ static int save_normal_page(RAMState *rs, RAMBlock *block, ram_addr_t offset,
>      return 1;
>  }
>  
> +/**
> + * ram_save_encrypted_page - send the given encrypted page to the stream
> + */
> +static int ram_save_encrypted_page(RAMState *rs, PageSearchStatus *pss,
> +                                   bool last_stage)
> +{
> +    int ret;
> +    uint8_t *p;
> +    RAMBlock *block = pss->block;
> +    ram_addr_t offset = pss->page << TARGET_PAGE_BITS;
> +    uint64_t bytes_xmit;
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +    struct MachineMemoryEncryptionOps *ops = mc->memory_encryption_ops;
> +
> +    p = block->host + offset;
> +
> +    ram_counters.transferred +=
> +        save_page_header(rs, rs->f, block,
> +                    offset | RAM_SAVE_FLAG_ENCRYPTED_DATA);
> +
> +    qemu_put_be32(rs->f, RAM_SAVE_ENCRYPTED_PAGE);
> +    ret = ops->save_outgoing_page(rs->f, p, TARGET_PAGE_SIZE, &bytes_xmit);
> +    if (ret) {
> +        return -1;
> +    }
> +
> +    ram_counters.transferred += bytes_xmit;
> +    ram_counters.normal++;
> +
> +    return 1;
> +}
> +
> +/**
> + * ram_save_encrypted_bitmap: send the encrypted page state bitmap
> + */
> +static int ram_save_encrypted_bitmap(RAMState *rs, QEMUFile *f)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +    struct MachineMemoryEncryptionOps *ops = mc->memory_encryption_ops;
> +
> +    save_page_header(rs, rs->f, rs->last_seen_block,
> +                     RAM_SAVE_FLAG_ENCRYPTED_DATA);
> +    qemu_put_be32(rs->f, RAM_SAVE_ENCRYPTED_BITMAP);
> +    return ops->save_outgoing_bitmap(rs->f);
> +}
> +
> +static int load_encrypted_data(QEMUFile *f, uint8_t *ptr)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +    struct MachineMemoryEncryptionOps *ops = mc->memory_encryption_ops;
> +    int flag;
> +
> +    flag = qemu_get_be32(f);
> +
> +    if (flag == RAM_SAVE_ENCRYPTED_PAGE) {
> +        return ops->load_incoming_page(f, ptr);
> +    } else if (flag == RAM_SAVE_ENCRYPTED_BITMAP) {
> +        return ops->load_incoming_bitmap(f);
> +    } else {
> +        error_report("unknown encrypted flag %x", flag);
> +        return 1;
> +    }
> +}
> +
>  /**
>   * ram_save_page: send the given page to the stream
>   *
> @@ -2528,6 +2602,22 @@ static bool save_compress_page(RAMState *rs, RAMBlock *block, ram_addr_t offset)
>      return false;
>  }
>  
> +/**
> + * encrypted_test_bitmap: check if the page is encrypted
> + *
> + * Returns a bool indicating whether the page is encrypted.
> + */
> +static bool encrypted_test_bitmap(RAMState *rs, RAMBlock *block,
> +                                  unsigned long page)
> +{
> +    /* ROM devices contains the unencrypted data */
> +    if (memory_region_is_rom(block->mr)) {
> +        return false;
> +    }
> +
> +    return test_bit(page, block->encbmap);
> +}
> +
>  /**
>   * ram_save_target_page: save one target page
>   *
> @@ -2548,6 +2638,17 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss,
>          return res;
>      }
>  
> +    /*
> +     * If memory encryption is enabled then use memory encryption APIs
> +     * to write the outgoing buffer to the wire. The encryption APIs
> +     * will take care of accessing the guest memory and re-encrypt it
> +     * for the transport purposes.
> +     */
> +    if (memcrypt_enabled() &&
> +        encrypted_test_bitmap(rs, pss->block, pss->page)) {
> +        return ram_save_encrypted_page(rs, pss, last_stage);
> +    }
> +
>      if (save_compress_page(rs, block, offset)) {
>          return 1;
>      }
> @@ -3445,6 +3546,16 @@ void qemu_guest_free_page_hint(void *addr, size_t len)
>      }
>  }
>  
> +static int ram_encrypted_save_setup(void)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    MachineClass *mc = MACHINE_GET_CLASS(ms);
> +    MigrationParameters *p = &migrate_get_current()->parameters;
> +    struct MachineMemoryEncryptionOps *ops = mc->memory_encryption_ops;
> +
> +    return ops->save_setup(p->sev_pdh, p->sev_plat_cert, p->sev_amd_cert);
> +}
> +
>  /*
>   * Each of ram_save_setup, ram_save_iterate and ram_save_complete has
>   * long-running RCU critical section.  When rcu-reclaims in the code
> @@ -3480,6 +3591,12 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>  
>      rcu_read_lock();
>  
> +    if (memcrypt_enabled()) {
> +        if (ram_encrypted_save_setup()) {
> +            return -1;
> +        }
> +    }
> +
>      qemu_put_be64(f, ram_bytes_total_common(true) | RAM_SAVE_FLAG_MEM_SIZE);
>  
>      RAMBLOCK_FOREACH_MIGRATABLE(block) {
> @@ -3644,6 +3761,11 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
>      flush_compressed_data(rs);
>      ram_control_after_iterate(f, RAM_CONTROL_FINISH);
>  
> +    /* send the page encryption state bitmap */
> +    if (memcrypt_enabled()) {
> +        ret = ram_save_encrypted_bitmap(rs, f);
> +    }
> +
>      rcu_read_unlock();
>  
>      multifd_send_sync_main();
> @@ -4391,7 +4513,8 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>          }
>  
>          if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
> -                     RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
> +                     RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE |
> +                     RAM_SAVE_FLAG_ENCRYPTED_DATA)) {
>              RAMBlock *block = ram_block_from_stream(f, flags);
>  
>              /*
> @@ -4505,6 +4628,12 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
>                  break;
>              }
>              break;
> +        case RAM_SAVE_FLAG_ENCRYPTED_DATA:
> +            if (load_encrypted_data(f, host)) {
> +                    error_report("Failed to load encrypted data");
> +                    ret = -EINVAL;
> +            }
> +            break;
>          case RAM_SAVE_FLAG_EOS:
>              /* normal exit */
>              multifd_recv_sync_main();
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2019-08-14 16:38 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-06 16:54 [Qemu-devel] [PATCH v3 00/14] Add SEV guest live migration support Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 01/14] doc: update AMD SEV API spec web link Singh, Brijesh
2019-08-06 19:00   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 02/14] doc: update AMD SEV to include Live migration flow Singh, Brijesh
2019-08-07 11:01   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 03/14] migration.json: add AMD SEV specific migration parameters Singh, Brijesh
2019-08-07 11:06   ` Dr. David Alan Gilbert
2019-08-08  2:25     ` Singh, Brijesh
2019-08-08 10:48       ` Dr. David Alan Gilbert
2019-08-09 20:00         ` Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 04/14] linux-headers: update kernel header to include SEV migration commands Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 05/14] hw/machine: add helper to query the memory encryption state Singh, Brijesh
2019-08-07 16:14   ` Dr. David Alan Gilbert
2019-08-08  2:25     ` Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 06/14] hw/machine: introduce MachineMemoryEncryptionOps for encrypted VMs Singh, Brijesh
2019-08-07 16:36   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 08/14] target/i386: sev: do not create launch context for an incoming guest Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 07/14] target/i386: sev: provide callback to setup outgoing context Singh, Brijesh
2019-08-08 11:19   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 09/14] target/i386: sev: add support to encrypt the outgoing page Singh, Brijesh
2019-08-09 18:54   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 10/14] target/i386: sev: add support to load incoming encrypted page Singh, Brijesh
2019-08-13 17:38   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page encryption bitmap Singh, Brijesh
2019-08-13 18:57   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 13/14] migration/ram: add support to send encrypted pages Singh, Brijesh
2019-08-14 16:37   ` Dr. David Alan Gilbert
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 12/14] kvm: add support to sync the page encryption state bitmap Singh, Brijesh
2019-08-06 16:54 ` [Qemu-devel] [PATCH v3 14/14] target/i386: sev: remove migration blocker Singh, Brijesh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).