[RFC PATCH 0/5] target/arm: Add MTE support to KVM guest

[RFC PATCH 0/5] target/arm: Add MTE support to KVM guest

[Haibo Xu]

This series add support for MTE(Memory Tagging Extension)[1]
in KVM guest. It's based on Steven Price's kernel KVM patches
V7[2], and has been tested to ensure that test case[3] can be
passed in a KVM guest. Basic pre-copy migration test also passed
between two MTE enabled kvm guest. 

This is a RFC patch series becuase:
(1) Need to add some arm MTE specific codes to the ram migration
    loop. There may be better way to do that in a more abstract way.
(2) Only pre-copy migration is supported and tested currently,
    post-copy as well as compress/zero page migration are still WIP.
    
All kinds of feedbacks are very welcomed, especially for the migration
support. 

Note:
(1) To support MTE migration, tags for one page are appended to  
    the page data during ram save iteration which make it easier
    to sync the page data and tags.

[1] https://community.arm.com/developer/ip-products/processors/b/
    processors-ip-blog/posts/enhancing-memory-safety
[2] https://lwn.net/Articles/842827/
[3] https://elixir.bootlin.com/linux/latest/source/Documentation/
    arm64/memory-tagging-extension.rst

Haibo Xu (5):
  Update Linux headers with new MTE support
  Add basic MTE support to KVM guest
  Add APIs to get/set MTE tags
  Add migration support for KVM guest with MTE
  Enable the MTE support for KVM guest

 hw/arm/virt.c             | 44 +++++++++++++-------
 include/hw/arm/virt.h     |  2 +
 include/migration/misc.h  |  1 +
 linux-headers/linux/kvm.h | 15 +++++++
 migration/ram.c           | 86 ++++++++++++++++++++++++++++++++++++++-
 target/arm/cpu.c          |  2 +-
 target/arm/kvm.c          |  9 ++++
 target/arm/kvm64.c        | 31 ++++++++++++++
 target/arm/kvm_arm.h      |  2 +
 9 files changed, 176 insertions(+), 16 deletions(-)

-- 
2.17.1

[RFC PATCH 1/5] Update Linux headers with new MTE support

[Haibo Xu]

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 linux-headers/linux/kvm.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 020b62a619..6a291a9a35 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1056,6 +1056,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_ENFORCE_PV_FEATURE_CPUID 190
 #define KVM_CAP_SYS_HYPERV_CPUID 191
 #define KVM_CAP_DIRTY_LOG_RING 192
+#define KVM_CAP_ARM_MTE 193
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -1241,6 +1242,19 @@ struct kvm_arm_device_addr {
 	__u64 addr;
 };
 
+#define KVM_ARM_TAGS_TO_GUEST           0
+#define KVM_ARM_TAGS_FROM_GUEST         1
+
+struct kvm_arm_copy_mte_tags {
+	__u64 guest_ipa;
+	__u64 length;
+	union {
+		void *addr;
+		__u64 padding;
+	};
+	__u64 flags;
+};
+
 /*
  * Device control API, available with KVM_CAP_DEVICE_CTRL
  */
@@ -1396,6 +1410,7 @@ struct kvm_s390_ucas_mapping {
 /* Available with KVM_CAP_PMU_EVENT_FILTER */
 #define KVM_SET_PMU_EVENT_FILTER  _IOW(KVMIO,  0xb2, struct kvm_pmu_event_filter)
 #define KVM_PPC_SVM_OFF		  _IO(KVMIO,  0xb3)
+#define KVM_ARM_MTE_COPY_TAGS	  _IOR(KVMIO,  0xb4, struct kvm_arm_copy_mte_tags)
 
 /* ioctl for vm fd */
 #define KVM_CREATE_DEVICE	  _IOWR(KVMIO,  0xe0, struct kvm_create_device)
-- 
2.17.1

[RFC PATCH 2/5] Add basic MTE support to KVM guest

[Haibo Xu]

Enable the virt machine feature "mte" to work with
KVM guest. This feature is still hiden from the user
in this patch, and will be available in a later patch.

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 hw/arm/virt.c      | 22 +++++++++++-----------
 target/arm/cpu.c   |  2 +-
 target/arm/kvm.c   |  9 +++++++++
 target/arm/kvm64.c |  7 +++++++
 4 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 399da73454..623d5e9397 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1974,18 +1974,18 @@ static void machvirt_init(MachineState *machine)
         }
 
         if (vms->mte) {
+            /*
+             * The property exists only if MemTag is supported.
+             * If it is, we must allocate the ram to back that up.
+             */
+            if (!object_property_find(cpuobj, "tag-memory")) {
+                error_report("MTE requested, but not supported "
+                             "by the guest CPU");
+                exit(1);
+            }
+
             /* Create the memory region only once, but link to all cpus. */
-            if (!tag_sysmem) {
-                /*
-                 * The property exists only if MemTag is supported.
-                 * If it is, we must allocate the ram to back that up.
-                 */
-                if (!object_property_find(cpuobj, "tag-memory")) {
-                    error_report("MTE requested, but not supported "
-                                 "by the guest CPU");
-                    exit(1);
-                }
-
+            if (!tag_sysmem && !kvm_enabled()) {
                 tag_sysmem = g_new(MemoryRegion, 1);
                 memory_region_init(tag_sysmem, OBJECT(machine),
                                    "tag-memory", UINT64_MAX / 32);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 40142ac141..50f3223944 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1831,7 +1831,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
                                cpu->secure_memory);
     }
 
-    if (cpu->tag_memory != NULL) {
+    if (cpu->tag_memory != NULL && !kvm_enabled()) {
         cpu_address_space_init(cs, ARMASIdx_TagNS, "cpu-tag-memory",
                                cpu->tag_memory);
         if (has_secure) {
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index ffe186de8d..33630b2b70 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -32,6 +32,7 @@
 #include "hw/boards.h"
 #include "hw/irq.h"
 #include "qemu/log.h"
+#include "hw/arm/virt.h"
 
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
     KVM_CAP_LAST_INFO
@@ -272,6 +273,14 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
         }
     }
 
+    if (kvm_check_extension(s, KVM_CAP_ARM_MTE) &&
+        object_dynamic_cast(OBJECT(ms), TYPE_VIRT_MACHINE) &&
+        VIRT_MACHINE(ms)->mte) {
+            if (kvm_vm_enable_cap(s, KVM_CAP_ARM_MTE, 0)) {
+                error_report("Failed to enable KVM_CAP_ARM_MTE cap");
+            }
+    }
+
     return ret;
 }
 
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 3c37fc4fb6..23f34034db 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -500,6 +500,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
      */
     int fdarray[3];
     bool sve_supported;
+    bool mte_supported;
     uint64_t features = 0;
     uint64_t t;
     int err;
@@ -646,6 +647,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
     }
 
     sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 0;
+    mte_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_MTE) > 0;
 
     kvm_arm_destroy_scratch_host_vcpu(fdarray);
 
@@ -659,6 +661,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
         t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
         ahcf->isar.id_aa64pfr0 = t;
     }
+    if (mte_supported) {
+        t = ahcf->isar.id_aa64pfr1;
+        t = FIELD_DP64(t, ID_AA64PFR1, MTE, 2);
+        ahcf->isar.id_aa64pfr1 = t;
+    }
 
     /*
      * We can assume any KVM supporting CPU is at least a v8
-- 
2.17.1

[RFC PATCH 3/5] Add APIs to get/set MTE tags

[Haibo Xu]

MTE spec provide instructions to retrieve the memory tags:
(1) LDG, at 16 bytes granularity, and available in both user
    and kernel space;
(2) LDGM, at 256 bytes granularity in maximum, and only
    available in kernel space

To improve the performance, KVM has exposed the LDGM capability
to user space by providing a new APIs. This patch is just a
wrapper for the KVM APIs.

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 target/arm/kvm64.c   | 24 ++++++++++++++++++++++++
 target/arm/kvm_arm.h |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 23f34034db..4a6790d53b 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -1608,3 +1608,27 @@ bool kvm_arm_verify_ext_dabt_pending(CPUState *cs)
     }
     return false;
 }
+
+int kvm_arm_mte_get_tags(uint64_t ipa, uint64_t len, uint8_t *buf)
+{
+    struct kvm_arm_copy_mte_tags args = {
+        .guest_ipa = ipa,
+        .length = len,
+        .addr = buf,
+        .flags = KVM_ARM_TAGS_FROM_GUEST,
+    };
+
+    return kvm_vm_ioctl(kvm_state, KVM_ARM_MTE_COPY_TAGS, &args);
+}
+
+int kvm_arm_mte_set_tags(uint64_t ipa, uint64_t len, uint8_t *buf)
+{
+    struct kvm_arm_copy_mte_tags args = {
+        .guest_ipa = ipa,
+        .length = len,
+        .addr = buf,
+        .flags = KVM_ARM_TAGS_TO_GUEST,
+    };
+
+    return kvm_vm_ioctl(kvm_state, KVM_ARM_MTE_COPY_TAGS, &args);
+}
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index eb81b7059e..1b94dbe7c8 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -358,6 +358,8 @@ int kvm_arm_vgic_probe(void);
 
 void kvm_arm_pmu_set_irq(CPUState *cs, int irq);
 void kvm_arm_pmu_init(CPUState *cs);
+int kvm_arm_mte_get_tags(uint64_t ipa, uint64_t len, uint8_t *buf);
+int kvm_arm_mte_set_tags(uint64_t ipa, uint64_t len, uint8_t *buf);
 
 /**
  * kvm_arm_pvtime_init:
-- 
2.17.1

[RFC PATCH 4/5] Add migration support for KVM guest with MTE

[Haibo Xu]

To make it easier to keep the page tags sync with
the page data, tags for one page are appended to
the data during ram save iteration.

This patch only add the pre-copy migration support.
Post-copy and compress as well as zero page saving
are not supported yet.

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 include/hw/arm/virt.h    |  2 +
 include/migration/misc.h |  1 +
 migration/ram.c          | 86 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 36fcb29641..6182b3e1d1 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -165,6 +165,8 @@ struct VirtMachineState {
     DeviceState *acpi_dev;
     Notifier powerdown_notifier;
     PCIBus *bus;
+    /* migrate memory tags */
+    NotifierWithReturn precopy_notifier;
 };
 
 #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index bccc1b6b44..005133f471 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -38,6 +38,7 @@ void precopy_add_notifier(NotifierWithReturn *n);
 void precopy_remove_notifier(NotifierWithReturn *n);
 int precopy_notify(PrecopyNotifyReason reason, Error **errp);
 void precopy_enable_free_page_optimization(void);
+void precopy_enable_metadata_migration(void);
 
 void ram_mig_init(void);
 void qemu_guest_free_page_hint(void *addr, size_t len);
diff --git a/migration/ram.c b/migration/ram.c
index 7811cde643..32f764680d 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -53,9 +53,11 @@
 #include "block.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/cpu-throttle.h"
+#include "sysemu/kvm.h"
 #include "savevm.h"
 #include "qemu/iov.h"
 #include "multifd.h"
+#include "kvm_arm.h"
 
 /***********************************************************/
 /* ram save/restore */
@@ -75,6 +77,9 @@
 #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_MTE              0x200
+
+#define MTE_GRANULE_SIZE   (16)
 
 static inline bool is_zero_range(uint8_t *p, uint64_t size)
 {
@@ -310,6 +315,8 @@ struct RAMState {
     bool ram_bulk_stage;
     /* The free page optimization is enabled */
     bool fpo_enabled;
+    /* The RAM meta data(e.t memory tag) is enabled */
+    bool metadata_enabled;
     /* How many times we have dirty too many pages */
     int dirty_rate_high_cnt;
     /* these variables are used for bitmap sync */
@@ -387,6 +394,15 @@ void precopy_enable_free_page_optimization(void)
     ram_state->fpo_enabled = true;
 }
 
+void precopy_enable_metadata_migration(void)
+{
+    if (!ram_state) {
+        return;
+    }
+
+    ram_state->metadata_enabled = true;
+}
+
 uint64_t ram_bytes_remaining(void)
 {
     return ram_state ? (ram_state->migration_dirty_pages * TARGET_PAGE_SIZE) :
@@ -1127,6 +1143,61 @@ static bool control_save_page(RAMState *rs, RAMBlock *block, ram_addr_t offset,
     return true;
 }
 
+static int save_normal_page_mte_tags(QEMUFile *f, uint8_t *addr)
+{
+    uint8_t *tag_buf = NULL;
+    uint64_t ipa;
+    int size = TARGET_PAGE_SIZE / MTE_GRANULE_SIZE;
+
+    if (kvm_physical_memory_addr_from_host(kvm_state, addr, &ipa)) {
+        /* Buffer for the page tags(one byte per tag) */
+        tag_buf = g_try_malloc0(size);
+        if (!tag_buf) {
+            error_report("%s: Error allocating MTE tag_buf", __func__);
+            return 0;
+        }
+
+        if (kvm_arm_mte_get_tags(ipa, TARGET_PAGE_SIZE, tag_buf) < 0) {
+            error_report("%s: Can't get MTE tags from guest", __func__);
+            g_free(tag_buf);
+            return 0;
+        }
+
+        qemu_put_buffer(f, tag_buf, size);
+
+        g_free(tag_buf);
+
+        return size;
+    }
+
+    return 0;
+}
+
+static void load_normal_page_mte_tags(QEMUFile *f, uint8_t *addr)
+{
+    uint8_t *tag_buf = NULL;
+    uint64_t ipa;
+    int size = TARGET_PAGE_SIZE / MTE_GRANULE_SIZE;
+
+    if (kvm_physical_memory_addr_from_host(kvm_state, addr, &ipa)) {
+        /* Buffer for the page tags(one byte per tag) */
+        tag_buf = g_try_malloc0(size);
+        if (!tag_buf) {
+            error_report("%s: Error allocating MTE tag_buf", __func__);
+            return;
+        }
+
+        qemu_get_buffer(f, tag_buf, size);
+        if (kvm_arm_mte_set_tags(ipa, TARGET_PAGE_SIZE, tag_buf) < 0) {
+            error_report("%s: Can't set MTE tags to guest", __func__);
+        }
+
+        g_free(tag_buf);
+    }
+
+    return;
+}
+
 /*
  * directly send the page to the stream
  *
@@ -1141,6 +1212,10 @@ static bool control_save_page(RAMState *rs, RAMBlock *block, ram_addr_t offset,
 static int save_normal_page(RAMState *rs, RAMBlock *block, ram_addr_t offset,
                             uint8_t *buf, bool async)
 {
+    if (rs->metadata_enabled) {
+        offset |= RAM_SAVE_FLAG_MTE;
+    }
+
     ram_counters.transferred += save_page_header(rs, rs->f, block,
                                                  offset | RAM_SAVE_FLAG_PAGE);
     if (async) {
@@ -1152,6 +1227,11 @@ static int save_normal_page(RAMState *rs, RAMBlock *block, ram_addr_t offset,
     }
     ram_counters.transferred += TARGET_PAGE_SIZE;
     ram_counters.normal++;
+
+    if (rs->metadata_enabled) {
+        ram_counters.transferred += save_normal_page_mte_tags(rs->f, buf);
+    }
+
     return 1;
 }
 
@@ -1905,6 +1985,7 @@ static void ram_state_reset(RAMState *rs)
     rs->last_version = ram_list.version;
     rs->ram_bulk_stage = true;
     rs->fpo_enabled = false;
+    rs->metadata_enabled = false;
 }
 
 #define MAX_WAIT 50 /* ms, half buffered_file limit */
@@ -3492,7 +3573,7 @@ static int ram_load_precopy(QEMUFile *f)
             trace_ram_load_loop(block->idstr, (uint64_t)addr, flags, host);
         }
 
-        switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
+        switch (flags & ~(RAM_SAVE_FLAG_CONTINUE | RAM_SAVE_FLAG_MTE)) {
         case RAM_SAVE_FLAG_MEM_SIZE:
             /* Synchronize RAM block list */
             total_ram_bytes = addr;
@@ -3562,6 +3643,9 @@ static int ram_load_precopy(QEMUFile *f)
 
         case RAM_SAVE_FLAG_PAGE:
             qemu_get_buffer(f, host, TARGET_PAGE_SIZE);
+            if (flags & RAM_SAVE_FLAG_MTE) {
+                load_normal_page_mte_tags(f, host);
+            }
             break;
 
         case RAM_SAVE_FLAG_COMPRESS_PAGE:
-- 
2.17.1

[RFC PATCH 5/5] Enable the MTE support for KVM guest

[Haibo Xu]

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 hw/arm/virt.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 623d5e9397..c2358cf4c5 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -79,6 +79,7 @@
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/char/pl011.h"
 #include "qemu/guest-random.h"
+#include "migration/misc.h"
 
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
     static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -821,6 +822,21 @@ static void virt_powerdown_req(Notifier *n, void *opaque)
     }
 }
 
+static int virt_precopy_notify(NotifierWithReturn *n, void *data)
+{
+    PrecopyNotifyData *pnd = data;
+
+    switch (pnd->reason) {
+    case PRECOPY_NOTIFY_SETUP:
+        precopy_enable_metadata_migration();
+        break;
+    default:
+        break;
+    }
+
+    return 0;
+}
+
 static void create_gpio_keys(const VirtMachineState *vms,
                              DeviceState *pl061_dev,
                              uint32_t phandle)
@@ -1898,9 +1914,9 @@ static void machvirt_init(MachineState *machine)
     }
 
     if (vms->mte && kvm_enabled()) {
-        error_report("mach-virt: KVM does not support providing "
-                     "MTE to the guest CPU");
-        exit(1);
+        /* connect migration precopy request */
+        vms->precopy_notifier.notify = virt_precopy_notify;
+        precopy_add_notifier(&vms->precopy_notifier);
     }
 
     create_fdt(vms);
-- 
2.17.1

Re: [RFC PATCH 0/5] target/arm: Add MTE support to KVM guest

[Peter Maydell]

Adding the migration maintainers to the cc list, because
the real meat of this patchset is "how should the migration
of MTE tags be integrated into the migration/ram.c code",
which is pretty well out of my area of expertise...

thanks
-- PMM

On Mon, 8 Feb 2021 at 03:20, Haibo Xu <haibo.xu@linaro.org> wrote:
>
> This series add support for MTE(Memory Tagging Extension)[1]
> in KVM guest. It's based on Steven Price's kernel KVM patches
> V7[2], and has been tested to ensure that test case[3] can be
> passed in a KVM guest. Basic pre-copy migration test also passed
> between two MTE enabled kvm guest.
>
> This is a RFC patch series becuase:
> (1) Need to add some arm MTE specific codes to the ram migration
>     loop. There may be better way to do that in a more abstract way.
> (2) Only pre-copy migration is supported and tested currently,
>     post-copy as well as compress/zero page migration are still WIP.
>
> All kinds of feedbacks are very welcomed, especially for the migration
> support.
>
> Note:
> (1) To support MTE migration, tags for one page are appended to
>     the page data during ram save iteration which make it easier
>     to sync the page data and tags.
>
> [1] https://community.arm.com/developer/ip-products/processors/b/
>     processors-ip-blog/posts/enhancing-memory-safety
> [2] https://lwn.net/Articles/842827/
> [3] https://elixir.bootlin.com/linux/latest/source/Documentation/
>     arm64/memory-tagging-extension.rst
>
> Haibo Xu (5):
>   Update Linux headers with new MTE support
>   Add basic MTE support to KVM guest
>   Add APIs to get/set MTE tags
>   Add migration support for KVM guest with MTE
>   Enable the MTE support for KVM guest
>
>  hw/arm/virt.c             | 44 +++++++++++++-------
>  include/hw/arm/virt.h     |  2 +
>  include/migration/misc.h  |  1 +
>  linux-headers/linux/kvm.h | 15 +++++++
>  migration/ram.c           | 86 ++++++++++++++++++++++++++++++++++++++-
>  target/arm/cpu.c          |  2 +-
>  target/arm/kvm.c          |  9 ++++
>  target/arm/kvm64.c        | 31 ++++++++++++++
>  target/arm/kvm_arm.h      |  2 +
>  9 files changed, 176 insertions(+), 16 deletions(-)

Re: [RFC PATCH 4/5] Add migration support for KVM guest with MTE

[Richard Henderson]

On 2/7/21 7:20 PM, Haibo Xu wrote:
> +    if (kvm_physical_memory_addr_from_host(kvm_state, addr, &ipa)) {
> +        /* Buffer for the page tags(one byte per tag) */
> +        tag_buf = g_try_malloc0(size);
> +        if (!tag_buf) {
> +            error_report("%s: Error allocating MTE tag_buf", __func__);
> +            return 0;
> +        }
> +
> +        if (kvm_arm_mte_get_tags(ipa, TARGET_PAGE_SIZE, tag_buf) < 0) {
> +            error_report("%s: Can't get MTE tags from guest", __func__);
> +            g_free(tag_buf);
> +            return 0;
> +        }
> +
> +        qemu_put_buffer(f, tag_buf, size);
> +
> +        g_free(tag_buf);
> +
> +        return size;
> +    }

So, in patch 2 you disabled the allocation of tag-memory.  Now you're
allocating new memory (admittedly quite a small amount -- 1/16th of a page,
small enough to just be a local variable).

Why don't you allocate tag-memory, copy the data into it, and then let
migration proceed as normal.  Then you don't have to have a random data block
that happens to follow each ram page.

I'm concerned that what you're doing here makes it impossible to migrate
between kvm and tcg.


r~

Re: [RFC PATCH 0/5] target/arm: Add MTE support to KVM guest

[Haibo Xu]

On Tue, 16 Feb 2021 at 19:20, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Adding the migration maintainers to the cc list, because
> the real meat of this patchset is "how should the migration
> of MTE tags be integrated into the migration/ram.c code",
> which is pretty well out of my area of expertise...
>
> thanks
> -- PMM
>

Hi Peter,

Thanks for cc-ing to the related maintainers!

Regards,
Haibo

> On Mon, 8 Feb 2021 at 03:20, Haibo Xu <haibo.xu@linaro.org> wrote:
> >
> > This series add support for MTE(Memory Tagging Extension)[1]
> > in KVM guest. It's based on Steven Price's kernel KVM patches
> > V7[2], and has been tested to ensure that test case[3] can be
> > passed in a KVM guest. Basic pre-copy migration test also passed
> > between two MTE enabled kvm guest.
> >
> > This is a RFC patch series becuase:
> > (1) Need to add some arm MTE specific codes to the ram migration
> >     loop. There may be better way to do that in a more abstract way.
> > (2) Only pre-copy migration is supported and tested currently,
> >     post-copy as well as compress/zero page migration are still WIP.
> >
> > All kinds of feedbacks are very welcomed, especially for the migration
> > support.
> >
> > Note:
> > (1) To support MTE migration, tags for one page are appended to
> >     the page data during ram save iteration which make it easier
> >     to sync the page data and tags.
> >
> > [1] https://community.arm.com/developer/ip-products/processors/b/
> >     processors-ip-blog/posts/enhancing-memory-safety
> > [2] https://lwn.net/Articles/842827/
> > [3] https://elixir.bootlin.com/linux/latest/source/Documentation/
> >     arm64/memory-tagging-extension.rst
> >
> > Haibo Xu (5):
> >   Update Linux headers with new MTE support
> >   Add basic MTE support to KVM guest
> >   Add APIs to get/set MTE tags
> >   Add migration support for KVM guest with MTE
> >   Enable the MTE support for KVM guest
> >
> >  hw/arm/virt.c             | 44 +++++++++++++-------
> >  include/hw/arm/virt.h     |  2 +
> >  include/migration/misc.h  |  1 +
> >  linux-headers/linux/kvm.h | 15 +++++++
> >  migration/ram.c           | 86 ++++++++++++++++++++++++++++++++++++++-
> >  target/arm/cpu.c          |  2 +-
> >  target/arm/kvm.c          |  9 ++++
> >  target/arm/kvm64.c        | 31 ++++++++++++++
> >  target/arm/kvm_arm.h      |  2 +
> >  9 files changed, 176 insertions(+), 16 deletions(-)

Re: [RFC PATCH 4/5] Add migration support for KVM guest with MTE

[Haibo Xu]

On Tue, 16 Feb 2021 at 23:31, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/7/21 7:20 PM, Haibo Xu wrote:
> > +    if (kvm_physical_memory_addr_from_host(kvm_state, addr, &ipa)) {
> > +        /* Buffer for the page tags(one byte per tag) */
> > +        tag_buf = g_try_malloc0(size);
> > +        if (!tag_buf) {
> > +            error_report("%s: Error allocating MTE tag_buf", __func__);
> > +            return 0;
> > +        }
> > +
> > +        if (kvm_arm_mte_get_tags(ipa, TARGET_PAGE_SIZE, tag_buf) < 0) {
> > +            error_report("%s: Can't get MTE tags from guest", __func__);
> > +            g_free(tag_buf);
> > +            return 0;
> > +        }
> > +
> > +        qemu_put_buffer(f, tag_buf, size);
> > +
> > +        g_free(tag_buf);
> > +
> > +        return size;
> > +    }
>
> So, in patch 2 you disabled the allocation of tag-memory.  Now you're
> allocating new memory (admittedly quite a small amount -- 1/16th of a page,
> small enough to just be a local variable).
>

Hi Richard!

Thanks so much for the comments!

Yes, the allocated memory here is for temporary tag storage. As you
mentioned, it can be
defined as a local variable which is reserved for temporary tag buffers.

> Why don't you allocate tag-memory, copy the data into it, and then let
> migration proceed as normal.  Then you don't have to have a random data block
> that happens to follow each ram page.
>

As I mentioned in the cover later, the reason to let the tag go with
the memory data together
is to make it easier to sync with each other. I think if we migratie
them separately, it would be
hard to keep the tags to sync with the data.

Saying if we migration all the data first, then the tags. If the data
got dirty during the migration
of the tag memory, we may need to send the data again, or freeze the
source VM after data
migration? What's more, the KVM_GET_DIRTY_LOG API may not be able to
differentiate
between a tag and data changes.

I'm not sure whether the aforementioned situation does exist. Please
correct me if something goes wrong!

> I'm concerned that what you're doing here makes it impossible to migrate
> between kvm and tcg.
>
>

You mean to migrate from a KVM mode VM to a TCG mode VM?

> r~

Regards,
Haibo

Re: [RFC PATCH 4/5] Add migration support for KVM guest with MTE

[Richard Henderson]

On 2/22/21 1:46 AM, Haibo Xu wrote:
> As I mentioned in the cover later, the reason to let the tag go with the
> memory data together is to make it easier to sync with each other. I think
> if we migratie them separately, it would be hard to keep the tags to sync
> with the data.
Well, maybe, maybe not.  See below.


> Saying if we migration all the data first, then the tags. If the data got
> dirty during the migration of the tag memory, we may need to send the data
> again, or freeze the source VM after data migration? What's more, the
> KVM_GET_DIRTY_LOG API may not be able to differentiate between a tag and
> data changes.
I would certainly expect KVM_GET_DIRTY_LOG to only care about the normal
memory.  That is, pages as viewed by the guest.

I would expect the separate tag_memory block to be private to the host.  If a
normal page is dirty, then we would read the tags into the tag_memory and
manually mark that dirty.  Migration would continue as normal, and eventually
both normal and tag memory would all be clean and migrated.

But I'll admit that it does require that we retain a buffer 1/16 the size of
main memory, which is otherwise unused, and thus this is less than ideal.  So
if we do it your way, we should arrange for tcg to migrate the tag data in the
same way.

I'll still wait for migration experts, of which I am not one.


r~

Re: [RFC PATCH 1/5] Update Linux headers with new MTE support

[Haibo Xu]

++ more migration experts!

On Mon, 8 Feb 2021 at 11:20, Haibo Xu <haibo.xu@linaro.org> wrote:
>
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  linux-headers/linux/kvm.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index 020b62a619..6a291a9a35 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -1056,6 +1056,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_ENFORCE_PV_FEATURE_CPUID 190
>  #define KVM_CAP_SYS_HYPERV_CPUID 191
>  #define KVM_CAP_DIRTY_LOG_RING 192
> +#define KVM_CAP_ARM_MTE 193
>
>  #ifdef KVM_CAP_IRQ_ROUTING
>
> @@ -1241,6 +1242,19 @@ struct kvm_arm_device_addr {
>         __u64 addr;
>  };
>
> +#define KVM_ARM_TAGS_TO_GUEST           0
> +#define KVM_ARM_TAGS_FROM_GUEST         1
> +
> +struct kvm_arm_copy_mte_tags {
> +       __u64 guest_ipa;
> +       __u64 length;
> +       union {
> +               void *addr;
> +               __u64 padding;
> +       };
> +       __u64 flags;
> +};
> +
>  /*
>   * Device control API, available with KVM_CAP_DEVICE_CTRL
>   */
> @@ -1396,6 +1410,7 @@ struct kvm_s390_ucas_mapping {
>  /* Available with KVM_CAP_PMU_EVENT_FILTER */
>  #define KVM_SET_PMU_EVENT_FILTER  _IOW(KVMIO,  0xb2, struct kvm_pmu_event_filter)
>  #define KVM_PPC_SVM_OFF                  _IO(KVMIO,  0xb3)
> +#define KVM_ARM_MTE_COPY_TAGS    _IOR(KVMIO,  0xb4, struct kvm_arm_copy_mte_tags)
>
>  /* ioctl for vm fd */
>  #define KVM_CREATE_DEVICE        _IOWR(KVMIO,  0xe0, struct kvm_create_device)
> --
> 2.17.1
>

Re: [RFC PATCH 2/5] Add basic MTE support to KVM guest

[Haibo Xu]

++ more migration experts!

On Mon, 8 Feb 2021 at 11:20, Haibo Xu <haibo.xu@linaro.org> wrote:
>
> Enable the virt machine feature "mte" to work with
> KVM guest. This feature is still hiden from the user
> in this patch, and will be available in a later patch.
>
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  hw/arm/virt.c      | 22 +++++++++++-----------
>  target/arm/cpu.c   |  2 +-
>  target/arm/kvm.c   |  9 +++++++++
>  target/arm/kvm64.c |  7 +++++++
>  4 files changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 399da73454..623d5e9397 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1974,18 +1974,18 @@ static void machvirt_init(MachineState *machine)
>          }
>
>          if (vms->mte) {
> +            /*
> +             * The property exists only if MemTag is supported.
> +             * If it is, we must allocate the ram to back that up.
> +             */
> +            if (!object_property_find(cpuobj, "tag-memory")) {
> +                error_report("MTE requested, but not supported "
> +                             "by the guest CPU");
> +                exit(1);
> +            }
> +
>              /* Create the memory region only once, but link to all cpus. */
> -            if (!tag_sysmem) {
> -                /*
> -                 * The property exists only if MemTag is supported.
> -                 * If it is, we must allocate the ram to back that up.
> -                 */
> -                if (!object_property_find(cpuobj, "tag-memory")) {
> -                    error_report("MTE requested, but not supported "
> -                                 "by the guest CPU");
> -                    exit(1);
> -                }
> -
> +            if (!tag_sysmem && !kvm_enabled()) {
>                  tag_sysmem = g_new(MemoryRegion, 1);
>                  memory_region_init(tag_sysmem, OBJECT(machine),
>                                     "tag-memory", UINT64_MAX / 32);
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 40142ac141..50f3223944 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -1831,7 +1831,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>                                 cpu->secure_memory);
>      }
>
> -    if (cpu->tag_memory != NULL) {
> +    if (cpu->tag_memory != NULL && !kvm_enabled()) {
>          cpu_address_space_init(cs, ARMASIdx_TagNS, "cpu-tag-memory",
>                                 cpu->tag_memory);
>          if (has_secure) {
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index ffe186de8d..33630b2b70 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -32,6 +32,7 @@
>  #include "hw/boards.h"
>  #include "hw/irq.h"
>  #include "qemu/log.h"
> +#include "hw/arm/virt.h"
>
>  const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>      KVM_CAP_LAST_INFO
> @@ -272,6 +273,14 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>          }
>      }
>
> +    if (kvm_check_extension(s, KVM_CAP_ARM_MTE) &&
> +        object_dynamic_cast(OBJECT(ms), TYPE_VIRT_MACHINE) &&
> +        VIRT_MACHINE(ms)->mte) {
> +            if (kvm_vm_enable_cap(s, KVM_CAP_ARM_MTE, 0)) {
> +                error_report("Failed to enable KVM_CAP_ARM_MTE cap");
> +            }
> +    }
> +
>      return ret;
>  }
>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 3c37fc4fb6..23f34034db 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -500,6 +500,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>       */
>      int fdarray[3];
>      bool sve_supported;
> +    bool mte_supported;
>      uint64_t features = 0;
>      uint64_t t;
>      int err;
> @@ -646,6 +647,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>      }
>
>      sve_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_SVE) > 0;
> +    mte_supported = ioctl(fdarray[0], KVM_CHECK_EXTENSION, KVM_CAP_ARM_MTE) > 0;
>
>      kvm_arm_destroy_scratch_host_vcpu(fdarray);
>
> @@ -659,6 +661,11 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
>          t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
>          ahcf->isar.id_aa64pfr0 = t;
>      }
> +    if (mte_supported) {
> +        t = ahcf->isar.id_aa64pfr1;
> +        t = FIELD_DP64(t, ID_AA64PFR1, MTE, 2);
> +        ahcf->isar.id_aa64pfr1 = t;
> +    }
>
>      /*
>       * We can assume any KVM supporting CPU is at least a v8
> --
> 2.17.1
>

Re: [RFC PATCH 3/5] Add APIs to get/set MTE tags

[Haibo Xu]

++ more migration experts!

On Mon, 8 Feb 2021 at 11:20, Haibo Xu <haibo.xu@linaro.org> wrote:
>
> MTE spec provide instructions to retrieve the memory tags:
> (1) LDG, at 16 bytes granularity, and available in both user
>     and kernel space;
> (2) LDGM, at 256 bytes granularity in maximum, and only
>     available in kernel space
>
> To improve the performance, KVM has exposed the LDGM capability
> to user space by providing a new APIs. This patch is just a
> wrapper for the KVM APIs.
>
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  target/arm/kvm64.c   | 24 ++++++++++++++++++++++++
>  target/arm/kvm_arm.h |  2 ++
>  2 files changed, 26 insertions(+)
>
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 23f34034db..4a6790d53b 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -1608,3 +1608,27 @@ bool kvm_arm_verify_ext_dabt_pending(CPUState *cs)
>      }
>      return false;
>  }
> +
> +int kvm_arm_mte_get_tags(uint64_t ipa, uint64_t len, uint8_t *buf)
> +{
> +    struct kvm_arm_copy_mte_tags args = {
> +        .guest_ipa = ipa,
> +        .length = len,
> +        .addr = buf,
> +        .flags = KVM_ARM_TAGS_FROM_GUEST,
> +    };
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_ARM_MTE_COPY_TAGS, &args);
> +}
> +
> +int kvm_arm_mte_set_tags(uint64_t ipa, uint64_t len, uint8_t *buf)
> +{
> +    struct kvm_arm_copy_mte_tags args = {
> +        .guest_ipa = ipa,
> +        .length = len,
> +        .addr = buf,
> +        .flags = KVM_ARM_TAGS_TO_GUEST,
> +    };
> +
> +    return kvm_vm_ioctl(kvm_state, KVM_ARM_MTE_COPY_TAGS, &args);
> +}
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index eb81b7059e..1b94dbe7c8 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -358,6 +358,8 @@ int kvm_arm_vgic_probe(void);
>
>  void kvm_arm_pmu_set_irq(CPUState *cs, int irq);
>  void kvm_arm_pmu_init(CPUState *cs);
> +int kvm_arm_mte_get_tags(uint64_t ipa, uint64_t len, uint8_t *buf);
> +int kvm_arm_mte_set_tags(uint64_t ipa, uint64_t len, uint8_t *buf);
>
>  /**
>   * kvm_arm_pvtime_init:
> --
> 2.17.1
>

Re: [RFC PATCH 4/5] Add migration support for KVM guest with MTE

[Haibo Xu]

++ more migration experts!

On Tue, 23 Feb 2021 at 06:47, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/22/21 1:46 AM, Haibo Xu wrote:
> > As I mentioned in the cover later, the reason to let the tag go with the
> > memory data together is to make it easier to sync with each other. I think
> > if we migratie them separately, it would be hard to keep the tags to sync
> > with the data.
> Well, maybe, maybe not.  See below.
>
>
> > Saying if we migration all the data first, then the tags. If the data got
> > dirty during the migration of the tag memory, we may need to send the data
> > again, or freeze the source VM after data migration? What's more, the
> > KVM_GET_DIRTY_LOG API may not be able to differentiate between a tag and
> > data changes.
> I would certainly expect KVM_GET_DIRTY_LOG to only care about the normal
> memory.  That is, pages as viewed by the guest.
>
> I would expect the separate tag_memory block to be private to the host.  If a
> normal page is dirty, then we would read the tags into the tag_memory and
> manually mark that dirty.  Migration would continue as normal, and eventually
> both normal and tag memory would all be clean and migrated.
>
> But I'll admit that it does require that we retain a buffer 1/16 the size of
> main memory, which is otherwise unused, and thus this is less than ideal.  So
> if we do it your way, we should arrange for tcg to migrate the tag data in the
> same way.
>
> I'll still wait for migration experts, of which I am not one.
>
>
> r~

Re: [RFC PATCH 5/5] Enable the MTE support for KVM guest

[Haibo Xu]

++ more migration experts!

On Mon, 8 Feb 2021 at 11:20, Haibo Xu <haibo.xu@linaro.org> wrote:
>
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> ---
>  hw/arm/virt.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 623d5e9397..c2358cf4c5 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -79,6 +79,7 @@
>  #include "hw/virtio/virtio-iommu.h"
>  #include "hw/char/pl011.h"
>  #include "qemu/guest-random.h"
> +#include "migration/misc.h"
>
>  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>      static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
> @@ -821,6 +822,21 @@ static void virt_powerdown_req(Notifier *n, void *opaque)
>      }
>  }
>
> +static int virt_precopy_notify(NotifierWithReturn *n, void *data)
> +{
> +    PrecopyNotifyData *pnd = data;
> +
> +    switch (pnd->reason) {
> +    case PRECOPY_NOTIFY_SETUP:
> +        precopy_enable_metadata_migration();
> +        break;
> +    default:
> +        break;
> +    }
> +
> +    return 0;
> +}
> +
>  static void create_gpio_keys(const VirtMachineState *vms,
>                               DeviceState *pl061_dev,
>                               uint32_t phandle)
> @@ -1898,9 +1914,9 @@ static void machvirt_init(MachineState *machine)
>      }
>
>      if (vms->mte && kvm_enabled()) {
> -        error_report("mach-virt: KVM does not support providing "
> -                     "MTE to the guest CPU");
> -        exit(1);
> +        /* connect migration precopy request */
> +        vms->precopy_notifier.notify = virt_precopy_notify;
> +        precopy_add_notifier(&vms->precopy_notifier);
>      }
>
>      create_fdt(vms);
> --
> 2.17.1
>