From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: "Singh, Brijesh" <brijesh.singh@amd.com>
Cc: "pbonzini@redhat.com" <pbonzini@redhat.com>,
"Lendacky, Thomas" <Thomas.Lendacky@amd.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
"ehabkost@redhat.com" <ehabkost@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page encryption bitmap
Date: Tue, 13 Aug 2019 19:57:52 +0100 [thread overview]
Message-ID: <20190813185752.GJ2763@work-vm> (raw)
In-Reply-To: <20190806165429.19327-12-brijesh.singh@amd.com>
* 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
next prev parent reply other threads:[~2019-08-13 18:58 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190813185752.GJ2763@work-vm \
--to=dgilbert@redhat.com \
--cc=Thomas.Lendacky@amd.com \
--cc=brijesh.singh@amd.com \
--cc=ehabkost@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).