[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page
From: |
Dr. David Alan Gilbert |
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 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
* Singh, Brijesh (address@hidden) 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 <address@hidden>
> ---
> 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 <address@hidden>
> + /*
> + * 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 / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH v3 07/14] target/i386: sev: provide callback to setup outgoing context, (continued)
- [Qemu-devel] [PATCH v3 06/14] hw/machine: introduce MachineMemoryEncryptionOps for encrypted VMs, Singh, Brijesh, 2019/08/06
- [Qemu-devel] [PATCH v3 10/14] target/i386: sev: add support to load incoming encrypted page, Singh, Brijesh, 2019/08/06
- [Qemu-devel] [PATCH v3 09/14] target/i386: sev: add support to encrypt the outgoing page, Singh, Brijesh, 2019/08/06
- [Qemu-devel] [PATCH v3 13/14] migration/ram: add support to send encrypted pages, Singh, Brijesh, 2019/08/06
- [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page encryption bitmap, Singh, Brijesh, 2019/08/06
- Re: [Qemu-devel] [PATCH v3 11/14] migration: add support to migrate page encryption bitmap,
Dr. David Alan Gilbert <=
- [Qemu-devel] [PATCH v3 14/14] target/i386: sev: remove migration blocker, Singh, Brijesh, 2019/08/06
- [Qemu-devel] [PATCH v3 12/14] kvm: add support to sync the page encryption state bitmap, Singh, Brijesh, 2019/08/06