[PATCH v6 0/2] arm: enable MTE for QEMU + kvm

[PATCH v6 0/2] arm: enable MTE for QEMU + kvm

[Cornelia Huck]

Another respin of my kvm mte series; again, tested via check + check-tcg
and on FVP.

Changes v5->v6:
- "arm/virt: don't try to spell out the accelerator" has been merged
- some more reordering of the enable_mte logic
- added more explanations
- rebase to current master

Cornelia Huck (2):
  arm/kvm: add support for MTE
  qtests/arm: add some mte tests

 docs/system/arm/cpu-features.rst |  21 ++++++
 hw/arm/virt.c                    |   2 +-
 target/arm/cpu.c                 |  18 ++---
 target/arm/cpu.h                 |   1 +
 target/arm/cpu64.c               | 110 +++++++++++++++++++++++++++++++
 target/arm/internals.h           |   1 +
 target/arm/kvm.c                 |  29 ++++++++
 target/arm/kvm64.c               |   5 ++
 target/arm/kvm_arm.h             |  19 ++++++
 target/arm/monitor.c             |   1 +
 tests/qtest/arm-cpu-features.c   |  80 ++++++++++++++++++++++
 11 files changed, 274 insertions(+), 13 deletions(-)

-- 
2.39.2

[PATCH v6 1/2] arm/kvm: add support for MTE

[Cornelia Huck]

Introduce a new cpu feature flag to control MTE support. To preserve
backwards compatibility for tcg, MTE will continue to be enabled as
long as tag memory has been provided.

If MTE has been enabled, we need to disable migration, as we do not
yet have a way to migrate the tags as well. Therefore, MTE will stay
off with KVM unless requested explicitly.

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 docs/system/arm/cpu-features.rst |  21 ++++++
 hw/arm/virt.c                    |   2 +-
 target/arm/cpu.c                 |  18 ++---
 target/arm/cpu.h                 |   1 +
 target/arm/cpu64.c               | 110 +++++++++++++++++++++++++++++++
 target/arm/internals.h           |   1 +
 target/arm/kvm.c                 |  29 ++++++++
 target/arm/kvm64.c               |   5 ++
 target/arm/kvm_arm.h             |  19 ++++++
 target/arm/monitor.c             |   1 +
 10 files changed, 194 insertions(+), 13 deletions(-)

diff --git a/docs/system/arm/cpu-features.rst b/docs/system/arm/cpu-features.rst
index 00c444042ff5..f8b0f339d32d 100644
--- a/docs/system/arm/cpu-features.rst
+++ b/docs/system/arm/cpu-features.rst
@@ -443,3 +443,24 @@ As with ``sve-default-vector-length``, if the default length is larger
 than the maximum vector length enabled, the actual vector length will
 be reduced.  If this property is set to ``-1`` then the default vector
 length is set to the maximum possible length.
+
+MTE CPU Property
+================
+
+The ``mte`` property controls the Memory Tagging Extension. For TCG, it requires
+presence of tag memory (which can be turned on for the ``virt`` machine via
+``mte=on``). For KVM, it requires the ``KVM_CAP_ARM_MTE`` capability; until
+proper migration support is implemented, enabling MTE will install a migration
+blocker.
+
+If not specified explicitly via ``on`` or ``off``, MTE will be available
+according to the following rules:
+
+* When TCG is used, MTE will be available if and only if tag memory is available;
+  i.e. it preserves the behaviour prior to the introduction of the feature.
+
+* When KVM is used, MTE will default to off, so that migration will not
+  unintentionally be blocked. This might change in a future QEMU version.
+
+* Other accelerators currently don't support MTE.
+
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ac626b3bef74..8201bc0dc42d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2146,7 +2146,7 @@ static void machvirt_init(MachineState *machine)
 
     if (vms->mte && (kvm_enabled() || hvf_enabled())) {
         error_report("mach-virt: %s does not support providing "
-                     "MTE to the guest CPU",
+                     "emulated MTE to the guest CPU (tag memory not supported)",
                      current_accel_name());
         exit(1);
     }
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 876ab8f3bf8a..19fbf7df09ec 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1532,6 +1532,11 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp)
             error_propagate(errp, local_err);
             return;
         }
+        arm_cpu_mte_finalize(cpu, &local_err);
+        if (local_err != NULL) {
+            error_propagate(errp, local_err);
+            return;
+        }
     }
 #endif
 
@@ -1608,7 +1613,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         }
         if (cpu->tag_memory) {
             error_setg(errp,
-                       "Cannot enable %s when guest CPUs has MTE enabled",
+                       "Cannot enable %s when guest CPUs has tag memory enabled",
                        current_accel_name());
             return;
         }
@@ -1987,17 +1992,6 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
                                        ID_PFR1, VIRTUALIZATION, 0);
     }
 
-#ifndef CONFIG_USER_ONLY
-    if (cpu->tag_memory == NULL && cpu_isar_feature(aa64_mte, cpu)) {
-        /*
-         * Disable the MTE feature bits if we do not have tag-memory
-         * provided by the machine.
-         */
-        cpu->isar.id_aa64pfr1 =
-            FIELD_DP64(cpu->isar.id_aa64pfr1, ID_AA64PFR1, MTE, 0);
-    }
-#endif
-
     if (tcg_enabled()) {
         /*
          * Don't report the Statistical Profiling Extension in the ID
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 12b1082537c5..0960ae6d3e4e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1051,6 +1051,7 @@ struct ArchCPU {
     bool prop_pauth;
     bool prop_pauth_impdef;
     bool prop_lpa2;
+    OnOffAuto prop_mte;
 
     /* DCZ blocksize, in log_2(words), ie low 4 bits of DCZID_EL0 */
     uint32_t dcz_blocksize;
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 4066950da15c..eb562ae7122c 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -24,11 +24,16 @@
 #include "qemu/module.h"
 #include "sysemu/kvm.h"
 #include "sysemu/hvf.h"
+#include "sysemu/tcg.h"
 #include "kvm_arm.h"
 #include "hvf_arm.h"
 #include "qapi/visitor.h"
 #include "hw/qdev-properties.h"
 #include "internals.h"
+#include "qapi/qapi-visit-common.h"
+#if !defined(CONFIG_USER_ONLY)
+#include "hw/arm/virt.h"
+#endif
 
 static void aarch64_a35_initfn(Object *obj)
 {
@@ -1096,6 +1101,109 @@ static void aarch64_neoverse_n1_initfn(Object *obj)
     cpu->isar.reset_pmcr_el0 = 0x410c3000;
 }
 
+static void aarch64_cpu_get_mte(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+    OnOffAuto mte = cpu->prop_mte;
+
+    visit_type_OnOffAuto(v, name, &mte, errp);
+}
+
+static void aarch64_cpu_set_mte(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    visit_type_OnOffAuto(v, name, &cpu->prop_mte, errp);
+}
+
+static void aarch64_add_mte_properties(Object *obj)
+{
+    /*
+     * For tcg, "AUTO" means turn on mte if tag memory has been provided, and
+     * turn it off (without error) if not.
+     * For kvm, "AUTO" currently means mte off, as migration is not supported
+     * yet.
+     * For all others, "AUTO" means mte off.
+     */
+    object_property_add(obj, "mte", "OnOffAuto", aarch64_cpu_get_mte,
+                        aarch64_cpu_set_mte, NULL, NULL);
+}
+
+static inline bool arm_machine_has_tag_memory(void)
+{
+#ifndef CONFIG_USER_ONLY
+    Object *obj = object_dynamic_cast(qdev_get_machine(), TYPE_VIRT_MACHINE);
+
+    /* so far, only the virt machine has support for tag memory */
+    if (obj) {
+        VirtMachineState *vms = VIRT_MACHINE(obj);
+
+        return vms->mte;
+    }
+    return false;
+#endif
+    return true;
+}
+
+void arm_cpu_mte_finalize(ARMCPU *cpu, Error **errp)
+{
+    bool enable_mte;
+
+    switch (cpu->prop_mte) {
+    case ON_OFF_AUTO_OFF:
+        enable_mte = false;
+        break;
+    case ON_OFF_AUTO_ON:
+        enable_mte = true;
+        if (tcg_enabled()) {
+            if (arm_machine_has_tag_memory()) {
+                break;
+            }
+            error_setg(errp, "mte=on requires tag memory");
+            return;
+        }
+        if (kvm_enabled() && kvm_arm_mte_supported()) {
+            break;
+        }
+        error_setg(errp, "mte not supported by %s", current_accel_name());
+        return;
+    default: /* AUTO */
+        enable_mte = false;
+        if (tcg_enabled()) {
+            if (cpu_isar_feature(aa64_mte, cpu)) {
+                /*
+                 * Tie mte enablement to presence of tag memory, in order to
+                 * preserve pre-existing behaviour.
+                 */
+                enable_mte = arm_machine_has_tag_memory();
+            }
+            break;
+        }
+        if (kvm_enabled()) {
+            /*
+             * This cannot yet be
+             * enable_mte = kvm_arm_mte_supported();
+             * as we don't support migration yet.
+             */
+            enable_mte = false;
+        }
+    }
+
+    if (!enable_mte) {
+        /* Disable MTE feature bits. */
+        cpu->isar.id_aa64pfr1 =
+            FIELD_DP64(cpu->isar.id_aa64pfr1, ID_AA64PFR1, MTE, 0);
+        return;
+    }
+
+    /* accelerator-specific enablement */
+    if (kvm_enabled()) {
+        kvm_arm_enable_mte(errp);
+    }
+}
+
 static void aarch64_host_initfn(Object *obj)
 {
 #if defined(CONFIG_KVM)
@@ -1104,6 +1212,7 @@ static void aarch64_host_initfn(Object *obj)
     if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
         aarch64_add_sve_properties(obj);
         aarch64_add_pauth_properties(obj);
+        aarch64_add_mte_properties(obj);
     }
 #elif defined(CONFIG_HVF)
     ARMCPU *cpu = ARM_CPU(obj);
@@ -1301,6 +1410,7 @@ static void aarch64_max_initfn(Object *obj)
     object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_max_vq,
                         cpu_max_set_sve_max_vq, NULL, NULL);
     qdev_property_add_static(DEVICE(obj), &arm_cpu_lpa2_property);
+    aarch64_add_mte_properties(obj);
 }
 
 static const ARMCPUInfo aarch64_cpus[] = {
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 759b70c646f8..3b9ef2cbb9ae 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1334,6 +1334,7 @@ void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
 void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
 void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
 void arm_cpu_lpa2_finalize(ARMCPU *cpu, Error **errp);
+void arm_cpu_mte_finalize(ARMCPU *cpu, Error **errp);
 #endif
 
 #ifdef CONFIG_USER_ONLY
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index f022c644d2ff..e6f2cb807bde 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -31,6 +31,7 @@
 #include "hw/boards.h"
 #include "hw/irq.h"
 #include "qemu/log.h"
+#include "migration/blocker.h"
 
 const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
     KVM_CAP_LAST_INFO
@@ -1062,3 +1063,31 @@ bool kvm_arch_cpu_check_are_resettable(void)
 void kvm_arch_accel_class_init(ObjectClass *oc)
 {
 }
+
+void kvm_arm_enable_mte(Error **errp)
+{
+    static bool tried_to_enable = false;
+    Error *mte_migration_blocker = NULL;
+    int ret;
+
+    if (tried_to_enable) {
+        /*
+         * MTE on KVM is enabled on a per-VM basis (and retrying doesn't make
+         * sense), and we only want a single migration blocker as well.
+         */
+        return;
+    }
+    tried_to_enable = true;
+
+    if ((ret = kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_MTE, 0))) {
+        error_setg_errno(errp, -ret, "Failed to enable KVM_CAP_ARM_MTE");
+        return;
+    }
+
+    /* TODO: add proper migration support with MTE enabled */
+    error_setg(&mte_migration_blocker,
+               "Live migration disabled due to MTE enabled");
+    if (migrate_add_blocker(mte_migration_blocker, errp)) {
+        error_free(mte_migration_blocker);
+    }
+}
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 1197253d12f7..b777bd0a11d2 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -764,6 +764,11 @@ bool kvm_arm_steal_time_supported(void)
     return kvm_check_extension(kvm_state, KVM_CAP_STEAL_TIME);
 }
 
+bool kvm_arm_mte_supported(void)
+{
+    return kvm_check_extension(kvm_state, KVM_CAP_ARM_MTE);
+}
+
 QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1);
 
 uint32_t kvm_arm_sve_get_vls(CPUState *cs)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 99017b635ce4..9f88b0722293 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -305,6 +305,13 @@ bool kvm_arm_pmu_supported(void);
  */
 bool kvm_arm_sve_supported(void);
 
+/**
+ * kvm_arm_mte_supported:
+ *
+ * Returns: true if KVM can enable MTE, and false otherwise.
+ */
+bool kvm_arm_mte_supported(void);
+
 /**
  * kvm_arm_get_max_vm_ipa_size:
  * @ms: Machine state handle
@@ -369,6 +376,8 @@ void kvm_arm_pvtime_init(CPUState *cs, uint64_t ipa);
 
 int kvm_arm_set_irq(int cpu, int irqtype, int irq, int level);
 
+void kvm_arm_enable_mte(Error **errp);
+
 #else
 
 /*
@@ -395,6 +404,11 @@ static inline bool kvm_arm_steal_time_supported(void)
     return false;
 }
 
+static inline bool kvm_arm_mte_supported(void)
+{
+    return false;
+}
+
 /*
  * These functions should never actually be called without KVM support.
  */
@@ -443,6 +457,11 @@ static inline uint32_t kvm_arm_sve_get_vls(CPUState *cs)
     g_assert_not_reached();
 }
 
+static inline void kvm_arm_enable_mte(Error **errp)
+{
+    g_assert_not_reached();
+}
+
 #endif
 
 static inline const char *gic_class_name(void)
diff --git a/target/arm/monitor.c b/target/arm/monitor.c
index ecdd5ee81742..c419c81612ed 100644
--- a/target/arm/monitor.c
+++ b/target/arm/monitor.c
@@ -96,6 +96,7 @@ static const char *cpu_model_advertised_features[] = {
     "sve1408", "sve1536", "sve1664", "sve1792", "sve1920", "sve2048",
     "kvm-no-adjvtime", "kvm-steal-time",
     "pauth", "pauth-impdef",
+    "mte",
     NULL
 };
 
-- 
2.39.2

[PATCH v6 2/2] qtests/arm: add some mte tests

[Cornelia Huck]

With TCG, verify the interaction of the 'mte' cpu feature with virt
machine tag memory.

With KVM, only verify the existence of the cpu feature, as we cannot
probe or enable the feature.

Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 tests/qtest/arm-cpu-features.c | 80 ++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 1cb08138ad1c..9533646f0dc5 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -22,6 +22,7 @@
 
 #define MACHINE     "-machine virt,gic-version=max -accel tcg "
 #define MACHINE_KVM "-machine virt,gic-version=max -accel kvm "
+#define MACHINE_MTE "-machine virt,gic-version=max,mte=on -accel tcg "
 #define QUERY_HEAD  "{ 'execute': 'query-cpu-model-expansion', " \
                     "  'arguments': { 'type': 'full', "
 #define QUERY_TAIL  "}}"
@@ -156,6 +157,18 @@ static bool resp_get_feature(QDict *resp, const char *feature)
     g_assert(qdict_get_bool(_props, feature) == (expected_value));     \
 })
 
+#define resp_assert_feature_str(resp, feature, expected_value)         \
+({                                                                     \
+    QDict *_props;                                                     \
+                                                                       \
+    g_assert(_resp);                                                   \
+    g_assert(resp_has_props(_resp));                                   \
+    _props = resp_get_props(_resp);                                    \
+    g_assert(qdict_get(_props, feature));                              \
+    g_assert_cmpstr(qdict_get_try_str(_props, feature), ==,            \
+                    expected_value);                                   \
+})
+
 #define assert_feature(qts, cpu_type, feature, expected_value)         \
 ({                                                                     \
     QDict *_resp;                                                      \
@@ -166,6 +179,16 @@ static bool resp_get_feature(QDict *resp, const char *feature)
     qobject_unref(_resp);                                              \
 })
 
+#define assert_feature_str(qts, cpu_type, feature, expected_value)     \
+({                                                                     \
+    QDict *_resp;                                                      \
+                                                                       \
+    _resp = do_query_no_props(qts, cpu_type);                          \
+    g_assert(_resp);                                                   \
+    resp_assert_feature_str(_resp, feature, expected_value);           \
+    qobject_unref(_resp);                                              \
+})
+
 #define assert_set_feature(qts, cpu_type, feature, value)              \
 ({                                                                     \
     const char *_fmt = (value) ? "{ %s: true }" : "{ %s: false }";     \
@@ -177,6 +200,17 @@ static bool resp_get_feature(QDict *resp, const char *feature)
     qobject_unref(_resp);                                              \
 })
 
+#define assert_set_feature_str(qts, cpu_type, feature, value, _fmt)    \
+({                                                                     \
+    const char *__fmt = _fmt;                                          \
+    QDict *_resp;                                                      \
+                                                                       \
+    _resp = do_query(qts, cpu_type, __fmt, feature);                   \
+    g_assert(_resp);                                                   \
+    resp_assert_feature_str(_resp, feature, value);                    \
+    qobject_unref(_resp);                                              \
+})
+
 #define assert_has_feature_enabled(qts, cpu_type, feature)             \
     assert_feature(qts, cpu_type, feature, true)
 
@@ -413,6 +447,24 @@ static void sve_tests_sve_off_kvm(const void *data)
     qtest_quit(qts);
 }
 
+static void mte_tests_tag_memory_on(const void *data)
+{
+    QTestState *qts;
+
+    qts = qtest_init(MACHINE_MTE "-cpu max");
+
+    /*
+     * With tag memory, "mte" should default to on, and explicitly specifying
+     * either on or off should be fine.
+     */
+    assert_has_feature(qts, "max", "mte");
+
+    assert_set_feature_str(qts, "max", "mte", "off", "{ 'mte': 'off' }");
+    assert_set_feature_str(qts, "max", "mte", "on", "{ 'mte': 'on' }");
+
+    qtest_quit(qts);
+}
+
 static void pauth_tests_default(QTestState *qts, const char *cpu_type)
 {
     assert_has_feature_enabled(qts, cpu_type, "pauth");
@@ -425,6 +477,19 @@ static void pauth_tests_default(QTestState *qts, const char *cpu_type)
                  "{ 'pauth': false, 'pauth-impdef': true }");
 }
 
+static void mte_tests_default(QTestState *qts, const char *cpu_type)
+{
+    assert_has_feature(qts, cpu_type, "mte");
+
+    /*
+     * Without tag memory, mte will be off under tcg.
+     * Explicitly enabling it yields an error.
+     */
+    assert_set_feature_str(qts, "max", "mte", "off", "{ 'mte': 'off' }");
+    assert_error(qts, cpu_type, "mte=on requires tag memory",
+                 "{ 'mte': 'on' }");
+}
+
 static void test_query_cpu_model_expansion(const void *data)
 {
     QTestState *qts;
@@ -474,6 +539,7 @@ static void test_query_cpu_model_expansion(const void *data)
 
         sve_tests_default(qts, "max");
         pauth_tests_default(qts, "max");
+        mte_tests_default(qts, "max");
 
         /* Test that features that depend on KVM generate errors without. */
         assert_error(qts, "max",
@@ -517,6 +583,18 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
         assert_set_feature(qts, "host", "pmu", false);
         assert_set_feature(qts, "host", "pmu", true);
 
+        /*
+         * Unfortunately, there's no easy way to test whether this instance
+         * of KVM supports MTE: the cpu model expansion will return "auto" for
+         * the mte prop, regardless whether the host/KVM supports MTE or not.
+         * Even if we got around that hurdle somehow, we would need to setup
+         * proper memory mappings in order to enable MTE, which is not feasible
+         * with qtest.
+         * So we can only assert that the feature is present, but not whether it
+         * can be toggled.
+         */
+        assert_has_feature(qts, "host", "mte");
+
         /*
          * Some features would be enabled by default, but they're disabled
          * because this instance of KVM doesn't support them. Test that the
@@ -631,6 +709,8 @@ int main(int argc, char **argv)
 
         qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
                             NULL, sve_tests_sve_off_kvm);
+        qtest_add_data_func("/arm/max/query-cpu-model-expansion/tag-memory",
+                            NULL, mte_tests_tag_memory_on);
     }
 
     if (qtest_has_accel("tcg")) {
-- 
2.39.2

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Andrea Bolognani]

On Tue, Feb 28, 2023 at 04:02:15PM +0100, Cornelia Huck wrote:
> Introduce a new cpu feature flag to control MTE support. To preserve
> backwards compatibility for tcg, MTE will continue to be enabled as
> long as tag memory has been provided.
>
> If MTE has been enabled, we need to disable migration, as we do not
> yet have a way to migrate the tags as well. Therefore, MTE will stay
> off with KVM unless requested explicitly.
>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  docs/system/arm/cpu-features.rst |  21 ++++++
>  hw/arm/virt.c                    |   2 +-
>  target/arm/cpu.c                 |  18 ++---
>  target/arm/cpu.h                 |   1 +
>  target/arm/cpu64.c               | 110 +++++++++++++++++++++++++++++++
>  target/arm/internals.h           |   1 +
>  target/arm/kvm.c                 |  29 ++++++++
>  target/arm/kvm64.c               |   5 ++
>  target/arm/kvm_arm.h             |  19 ++++++
>  target/arm/monitor.c             |   1 +
>  10 files changed, 194 insertions(+), 13 deletions(-)

I've given a quick look with libvirt integration in mind, and
everything seem fine.

Specifically, MTE is advertised in the output of qom-list-properties
both for max-arm-cpu and the latest virt-X.Y-machine, which means
that libvirt can easily and reliably figure out whether MTE support
is available.

> +MTE CPU Property
> +================
> +
> +The ``mte`` property controls the Memory Tagging Extension. For TCG, it requires
> +presence of tag memory (which can be turned on for the ``virt`` machine via
> +``mte=on``). For KVM, it requires the ``KVM_CAP_ARM_MTE`` capability; until
> +proper migration support is implemented, enabling MTE will install a migration
> +blocker.

Is it okay to use -machine virt,mte=on unconditionally for both KVM
and TCG guests when MTE support is requested, or will that not work
for the former?

> +If not specified explicitly via ``on`` or ``off``, MTE will be available
> +according to the following rules:
> +
> +* When TCG is used, MTE will be available if and only if tag memory is available;
> +  i.e. it preserves the behaviour prior to the introduction of the feature.
> +
> +* When KVM is used, MTE will default to off, so that migration will not
> +  unintentionally be blocked. This might change in a future QEMU version.

If and when this changes, we should ensure that the new default
behavior doesn't affect existing machine types, otherwise we will
break guest ABI for existing VMs.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Cornelia Huck]

On Tue, Feb 28 2023, Andrea Bolognani <abologna@redhat.com> wrote:

> On Tue, Feb 28, 2023 at 04:02:15PM +0100, Cornelia Huck wrote:
>> Introduce a new cpu feature flag to control MTE support. To preserve
>> backwards compatibility for tcg, MTE will continue to be enabled as
>> long as tag memory has been provided.
>>
>> If MTE has been enabled, we need to disable migration, as we do not
>> yet have a way to migrate the tags as well. Therefore, MTE will stay
>> off with KVM unless requested explicitly.
>>
>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>>  docs/system/arm/cpu-features.rst |  21 ++++++
>>  hw/arm/virt.c                    |   2 +-
>>  target/arm/cpu.c                 |  18 ++---
>>  target/arm/cpu.h                 |   1 +
>>  target/arm/cpu64.c               | 110 +++++++++++++++++++++++++++++++
>>  target/arm/internals.h           |   1 +
>>  target/arm/kvm.c                 |  29 ++++++++
>>  target/arm/kvm64.c               |   5 ++
>>  target/arm/kvm_arm.h             |  19 ++++++
>>  target/arm/monitor.c             |   1 +
>>  10 files changed, 194 insertions(+), 13 deletions(-)
>
> I've given a quick look with libvirt integration in mind, and
> everything seem fine.
>
> Specifically, MTE is advertised in the output of qom-list-properties
> both for max-arm-cpu and the latest virt-X.Y-machine, which means
> that libvirt can easily and reliably figure out whether MTE support
> is available.

Great, thanks for having a look!

>
>> +MTE CPU Property
>> +================
>> +
>> +The ``mte`` property controls the Memory Tagging Extension. For TCG, it requires
>> +presence of tag memory (which can be turned on for the ``virt`` machine via
>> +``mte=on``). For KVM, it requires the ``KVM_CAP_ARM_MTE`` capability; until
>> +proper migration support is implemented, enabling MTE will install a migration
>> +blocker.
>
> Is it okay to use -machine virt,mte=on unconditionally for both KVM
> and TCG guests when MTE support is requested, or will that not work
> for the former?

QEMU will error out if you try this with KVM (basically, same behaviour
as before.) Is that a problem for libvirt, or merely a bit inconvinient?

>
>> +If not specified explicitly via ``on`` or ``off``, MTE will be available
>> +according to the following rules:
>> +
>> +* When TCG is used, MTE will be available if and only if tag memory is available;
>> +  i.e. it preserves the behaviour prior to the introduction of the feature.
>> +
>> +* When KVM is used, MTE will default to off, so that migration will not
>> +  unintentionally be blocked. This might change in a future QEMU version.
>
> If and when this changes, we should ensure that the new default
> behavior doesn't affect existing machine types, otherwise we will
> break guest ABI for existing VMs.

Nod, such a change would need proper compat handling. It's not quite
clear yet if we'll ever flip it, though.

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Andrea Bolognani]

On Wed, Mar 01, 2023 at 11:17:40AM +0100, Cornelia Huck wrote:
> On Tue, Feb 28 2023, Andrea Bolognani <abologna@redhat.com> wrote:
> > On Tue, Feb 28, 2023 at 04:02:15PM +0100, Cornelia Huck wrote:
> >> +MTE CPU Property
> >> +================
> >> +
> >> +The ``mte`` property controls the Memory Tagging Extension. For TCG, it requires
> >> +presence of tag memory (which can be turned on for the ``virt`` machine via
> >> +``mte=on``). For KVM, it requires the ``KVM_CAP_ARM_MTE`` capability; until
> >> +proper migration support is implemented, enabling MTE will install a migration
> >> +blocker.
> >
> > Is it okay to use -machine virt,mte=on unconditionally for both KVM
> > and TCG guests when MTE support is requested, or will that not work
> > for the former?
>
> QEMU will error out if you try this with KVM (basically, same behaviour
> as before.) Is that a problem for libvirt, or merely a bit inconvinient?

I'm actually a bit confused. The documentation for the mte property
of the virt machine type says

  mte
    Set on/off to enable/disable emulating a guest CPU which implements
    the Arm Memory Tagging Extensions. The default is off.

So why is there a need to have a CPU property in addition to the
existing machine type property?

From the libvirt integration point of view, setting the machine type
property only for TCG is not a problem.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Cornelia Huck]

On Wed, Mar 01 2023, Andrea Bolognani <abologna@redhat.com> wrote:

> On Wed, Mar 01, 2023 at 11:17:40AM +0100, Cornelia Huck wrote:
>> On Tue, Feb 28 2023, Andrea Bolognani <abologna@redhat.com> wrote:
>> > On Tue, Feb 28, 2023 at 04:02:15PM +0100, Cornelia Huck wrote:
>> >> +MTE CPU Property
>> >> +================
>> >> +
>> >> +The ``mte`` property controls the Memory Tagging Extension. For TCG, it requires
>> >> +presence of tag memory (which can be turned on for the ``virt`` machine via
>> >> +``mte=on``). For KVM, it requires the ``KVM_CAP_ARM_MTE`` capability; until
>> >> +proper migration support is implemented, enabling MTE will install a migration
>> >> +blocker.
>> >
>> > Is it okay to use -machine virt,mte=on unconditionally for both KVM
>> > and TCG guests when MTE support is requested, or will that not work
>> > for the former?
>>
>> QEMU will error out if you try this with KVM (basically, same behaviour
>> as before.) Is that a problem for libvirt, or merely a bit inconvinient?
>
> I'm actually a bit confused. The documentation for the mte property
> of the virt machine type says
>
>   mte
>     Set on/off to enable/disable emulating a guest CPU which implements
>     the Arm Memory Tagging Extensions. The default is off.
>
> So why is there a need to have a CPU property in addition to the
> existing machine type property?

I think the state prior to my patches is actually a bit confusing: the
user needs to set a machine type property (causing tag memory to be
allocated), which in turn enables a cpu feature. Supporting the machine
type property for KVM does not make much sense IMHO: we don't allocate
tag memory for KVM (in fact, that would not work). We have to keep the
previous behaviour, and explicitly instructing QEMU to create cpus with
a certain feature via a cpu property makes the most sense to me.

We might want to tweak the documentation for the machine property to
indicate that it creates tag memory and only implicitly enables mte but
is a pre-req for it -- thoughts?

>
> From the libvirt integration point of view, setting the machine type
> property only for TCG is not a problem.

Ok.

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Andrea Bolognani]

On Wed, Mar 01, 2023 at 03:15:17PM +0100, Cornelia Huck wrote:
> On Wed, Mar 01 2023, Andrea Bolognani <abologna@redhat.com> wrote:
> > I'm actually a bit confused. The documentation for the mte property
> > of the virt machine type says
> >
> >   mte
> >     Set on/off to enable/disable emulating a guest CPU which implements
> >     the Arm Memory Tagging Extensions. The default is off.
> >
> > So why is there a need to have a CPU property in addition to the
> > existing machine type property?
>
> I think the state prior to my patches is actually a bit confusing: the
> user needs to set a machine type property (causing tag memory to be
> allocated), which in turn enables a cpu feature. Supporting the machine
> type property for KVM does not make much sense IMHO: we don't allocate
> tag memory for KVM (in fact, that would not work). We have to keep the
> previous behaviour, and explicitly instructing QEMU to create cpus with
> a certain feature via a cpu property makes the most sense to me.

I agree that a CPU feature makes more sense.

> We might want to tweak the documentation for the machine property to
> indicate that it creates tag memory and only implicitly enables mte but
> is a pre-req for it -- thoughts?

I wonder if it would be possible to flip things around, so that the
machine property is retained with its existing behavior for backwards
compatibility, but both for KVM and for TCG the CPU property can be
used on its own?

Basically, keeping the default of machine.mte to off when cpu.mte is
not specified, but switching it to on when it is. This way, you'd be
able to simply use

  -machine virt -cpu xxx,mte=on

to enable MTE, regardless of whether you're using KVM or TCG, instead
of requiring the above for KVM and

  -machine virt,mte=on -cpu xxx

for TCG.

Note that, from libvirt's point of view, there's no advantage to
doing things that way instead of what you already have. Handling the
additional machine property is a complete non-issue. But it would
make things nicer for people running QEMU directly, I think.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Cornelia Huck]

On Wed, Mar 01 2023, Andrea Bolognani <abologna@redhat.com> wrote:

> On Wed, Mar 01, 2023 at 03:15:17PM +0100, Cornelia Huck wrote:
>> On Wed, Mar 01 2023, Andrea Bolognani <abologna@redhat.com> wrote:
>> > I'm actually a bit confused. The documentation for the mte property
>> > of the virt machine type says
>> >
>> >   mte
>> >     Set on/off to enable/disable emulating a guest CPU which implements
>> >     the Arm Memory Tagging Extensions. The default is off.
>> >
>> > So why is there a need to have a CPU property in addition to the
>> > existing machine type property?
>>
>> I think the state prior to my patches is actually a bit confusing: the
>> user needs to set a machine type property (causing tag memory to be
>> allocated), which in turn enables a cpu feature. Supporting the machine
>> type property for KVM does not make much sense IMHO: we don't allocate
>> tag memory for KVM (in fact, that would not work). We have to keep the
>> previous behaviour, and explicitly instructing QEMU to create cpus with
>> a certain feature via a cpu property makes the most sense to me.
>
> I agree that a CPU feature makes more sense.
>
>> We might want to tweak the documentation for the machine property to
>> indicate that it creates tag memory and only implicitly enables mte but
>> is a pre-req for it -- thoughts?
>
> I wonder if it would be possible to flip things around, so that the
> machine property is retained with its existing behavior for backwards
> compatibility, but both for KVM and for TCG the CPU property can be
> used on its own?
>
> Basically, keeping the default of machine.mte to off when cpu.mte is
> not specified, but switching it to on when it is. This way, you'd be
> able to simply use
>
>   -machine virt -cpu xxx,mte=on
>
> to enable MTE, regardless of whether you're using KVM or TCG, instead
> of requiring the above for KVM and
>
>   -machine virt,mte=on -cpu xxx
>
> for TCG.

The machine prop is a bool; that means that we cannot distinguish
between "user did not set mte at all" and "user explicitly set
mte=off" -- I think we want

  -machine virt,mte=off -cpu xxx,mte=on

to generate an error, but that would still imply that we'd need to error
out for

  -machine virt -cpu xxx,mte=on

as well.

We could make the machine prop OnOffAuto, but that looks like overkill
to me.

>
> Note that, from libvirt's point of view, there's no advantage to
> doing things that way instead of what you already have. Handling the
> additional machine property is a complete non-issue. But it would
> make things nicer for people running QEMU directly, I think.

I'm tempted to simply consider this to be another wart of the QEMU
command line :)

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Andrea Bolognani]

On Thu, Mar 02, 2023 at 02:26:06PM +0100, Cornelia Huck wrote:
> On Wed, Mar 01 2023, Andrea Bolognani <abologna@redhat.com> wrote:
> > Note that, from libvirt's point of view, there's no advantage to
> > doing things that way instead of what you already have. Handling the
> > additional machine property is a complete non-issue. But it would
> > make things nicer for people running QEMU directly, I think.
>
> I'm tempted to simply consider this to be another wart of the QEMU
> command line :)

Fine by me! Papering over such idiosyncrasies is part of libvirt's
core mission after all :)

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Peter Maydell]

On Wed, 1 Mar 2023 at 14:15, Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Wed, Mar 01 2023, Andrea Bolognani <abologna@redhat.com> wrote:
>
> > On Wed, Mar 01, 2023 at 11:17:40AM +0100, Cornelia Huck wrote:
> >> On Tue, Feb 28 2023, Andrea Bolognani <abologna@redhat.com> wrote:
> >> > On Tue, Feb 28, 2023 at 04:02:15PM +0100, Cornelia Huck wrote:
> >> >> +MTE CPU Property
> >> >> +================
> >> >> +
> >> >> +The ``mte`` property controls the Memory Tagging Extension. For TCG, it requires
> >> >> +presence of tag memory (which can be turned on for the ``virt`` machine via
> >> >> +``mte=on``). For KVM, it requires the ``KVM_CAP_ARM_MTE`` capability; until
> >> >> +proper migration support is implemented, enabling MTE will install a migration
> >> >> +blocker.
> >> >
> >> > Is it okay to use -machine virt,mte=on unconditionally for both KVM
> >> > and TCG guests when MTE support is requested, or will that not work
> >> > for the former?
> >>
> >> QEMU will error out if you try this with KVM (basically, same behaviour
> >> as before.) Is that a problem for libvirt, or merely a bit inconvinient?
> >
> > I'm actually a bit confused. The documentation for the mte property
> > of the virt machine type says
> >
> >   mte
> >     Set on/off to enable/disable emulating a guest CPU which implements
> >     the Arm Memory Tagging Extensions. The default is off.
> >
> > So why is there a need to have a CPU property in addition to the
> > existing machine type property?
>
> I think the state prior to my patches is actually a bit confusing: the
> user needs to set a machine type property (causing tag memory to be
> allocated), which in turn enables a cpu feature. Supporting the machine
> type property for KVM does not make much sense IMHO: we don't allocate
> tag memory for KVM (in fact, that would not work). We have to keep the
> previous behaviour, and explicitly instructing QEMU to create cpus with
> a certain feature via a cpu property makes the most sense to me.

This isn't really how the virt board does any other of these
properties, though: secure=on/off and virtualization=on/off also
both work by having a board property that sets up the board related
parts and also sets the CPU property appropriately.

I think having MTE in the specific case of KVM behave differently
to how we've done all these existing properties and how we've
done MTE for TCG would be confusing. The simplest thing is to just
follow the existing UI for TCG MTE.

The underlying reason for this is that MTE in general is not a feature
only of the CPU, but also of the whole system design. It happens
that KVM gives us tagged RAM "for free" but that's an oddity
of the KVM implementation -- in real hardware there needs to
be system level support for tagging.

thanks
-- PMM

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Cornelia Huck]

On Thu, Mar 02 2023, Peter Maydell <peter.maydell@linaro.org> wrote:

> On Wed, 1 Mar 2023 at 14:15, Cornelia Huck <cohuck@redhat.com> wrote:
>>
>> On Wed, Mar 01 2023, Andrea Bolognani <abologna@redhat.com> wrote:
>>
>> > On Wed, Mar 01, 2023 at 11:17:40AM +0100, Cornelia Huck wrote:
>> >> On Tue, Feb 28 2023, Andrea Bolognani <abologna@redhat.com> wrote:
>> >> > On Tue, Feb 28, 2023 at 04:02:15PM +0100, Cornelia Huck wrote:
>> >> >> +MTE CPU Property
>> >> >> +================
>> >> >> +
>> >> >> +The ``mte`` property controls the Memory Tagging Extension. For TCG, it requires
>> >> >> +presence of tag memory (which can be turned on for the ``virt`` machine via
>> >> >> +``mte=on``). For KVM, it requires the ``KVM_CAP_ARM_MTE`` capability; until
>> >> >> +proper migration support is implemented, enabling MTE will install a migration
>> >> >> +blocker.
>> >> >
>> >> > Is it okay to use -machine virt,mte=on unconditionally for both KVM
>> >> > and TCG guests when MTE support is requested, or will that not work
>> >> > for the former?
>> >>
>> >> QEMU will error out if you try this with KVM (basically, same behaviour
>> >> as before.) Is that a problem for libvirt, or merely a bit inconvinient?
>> >
>> > I'm actually a bit confused. The documentation for the mte property
>> > of the virt machine type says
>> >
>> >   mte
>> >     Set on/off to enable/disable emulating a guest CPU which implements
>> >     the Arm Memory Tagging Extensions. The default is off.
>> >
>> > So why is there a need to have a CPU property in addition to the
>> > existing machine type property?
>>
>> I think the state prior to my patches is actually a bit confusing: the
>> user needs to set a machine type property (causing tag memory to be
>> allocated), which in turn enables a cpu feature. Supporting the machine
>> type property for KVM does not make much sense IMHO: we don't allocate
>> tag memory for KVM (in fact, that would not work). We have to keep the
>> previous behaviour, and explicitly instructing QEMU to create cpus with
>> a certain feature via a cpu property makes the most sense to me.
>
> This isn't really how the virt board does any other of these
> properties, though: secure=on/off and virtualization=on/off also
> both work by having a board property that sets up the board related
> parts and also sets the CPU property appropriately.
>
> I think having MTE in the specific case of KVM behave differently
> to how we've done all these existing properties and how we've
> done MTE for TCG would be confusing. The simplest thing is to just
> follow the existing UI for TCG MTE.
>
> The underlying reason for this is that MTE in general is not a feature
> only of the CPU, but also of the whole system design. It happens
> that KVM gives us tagged RAM "for free" but that's an oddity
> of the KVM implementation -- in real hardware there needs to
> be system level support for tagging.

Hm... the Linux kernel actually seems to consider MTE to be a cpu
feature (at least, it lists it in the cpu features).

So, is your suggestion to use the 'mte' prop of the virt machine to mean
"enable all prereqs for MTE, i.e. allocate tag memory for TCG and enable
MTE in the kernel for KVM"? For TCG, we'll get MTE for the max cpu
model; for KVM, we'd get MTE for host (== max), but I'm wondering what
should happen if we get named cpu models and the user specifies one
where we won't have MTE (i.e. some pre-8.5 one)?

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Peter Maydell]

On Thu, 2 Mar 2023 at 14:29, Cornelia Huck <cohuck@redhat.com> wrote:
>
> On Thu, Mar 02 2023, Peter Maydell <peter.maydell@linaro.org> wrote:
> > I think having MTE in the specific case of KVM behave differently
> > to how we've done all these existing properties and how we've
> > done MTE for TCG would be confusing. The simplest thing is to just
> > follow the existing UI for TCG MTE.
> >
> > The underlying reason for this is that MTE in general is not a feature
> > only of the CPU, but also of the whole system design. It happens
> > that KVM gives us tagged RAM "for free" but that's an oddity
> > of the KVM implementation -- in real hardware there needs to
> > be system level support for tagging.
>
> Hm... the Linux kernel actually seems to consider MTE to be a cpu
> feature (at least, it lists it in the cpu features).
>
> So, is your suggestion to use the 'mte' prop of the virt machine to mean
> "enable all prereqs for MTE, i.e. allocate tag memory for TCG and enable
> MTE in the kernel for KVM"? For TCG, we'll get MTE for the max cpu
> model; for KVM, we'd get MTE for host (== max), but I'm wondering what
> should happen if we get named cpu models and the user specifies one
> where we won't have MTE (i.e. some pre-8.5 one)?

I think we can probably cross that bridge when we get to it,
but I imagine the semantics would be "cortex-foo plus MTE"
(in the same way that -cpu cortex-foo,+x,-y can add and
subtract features from a baseline).

thanks
-- PMM

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Peter Maydell]

On Tue, 28 Feb 2023 at 15:02, Cornelia Huck <cohuck@redhat.com> wrote:
>
> Introduce a new cpu feature flag to control MTE support. To preserve
> backwards compatibility for tcg, MTE will continue to be enabled as
> long as tag memory has been provided.
>
> If MTE has been enabled, we need to disable migration, as we do not
> yet have a way to migrate the tags as well. Therefore, MTE will stay
> off with KVM unless requested explicitly.
>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  docs/system/arm/cpu-features.rst |  21 ++++++
>  hw/arm/virt.c                    |   2 +-
>  target/arm/cpu.c                 |  18 ++---
>  target/arm/cpu.h                 |   1 +
>  target/arm/cpu64.c               | 110 +++++++++++++++++++++++++++++++
>  target/arm/internals.h           |   1 +
>  target/arm/kvm.c                 |  29 ++++++++
>  target/arm/kvm64.c               |   5 ++
>  target/arm/kvm_arm.h             |  19 ++++++
>  target/arm/monitor.c             |   1 +
>  10 files changed, 194 insertions(+), 13 deletions(-)



> +static inline bool arm_machine_has_tag_memory(void)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    Object *obj = object_dynamic_cast(qdev_get_machine(), TYPE_VIRT_MACHINE);
> +
> +    /* so far, only the virt machine has support for tag memory */
> +    if (obj) {
> +        VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +        return vms->mte;
> +    }

Code inside target/arm shouldn't be fishing around inside
the details of the board model like this. For TCG I think that
at this point (i.e. at realize) you should be able to tell if
the board has set up tag memory, because it will have set
cpu->tag_memory to non-NULL.

thanks
-- PMM

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Cornelia Huck]

On Fri, Mar 03 2023, Peter Maydell <peter.maydell@linaro.org> wrote:

> On Tue, 28 Feb 2023 at 15:02, Cornelia Huck <cohuck@redhat.com> wrote:
>>
>> Introduce a new cpu feature flag to control MTE support. To preserve
>> backwards compatibility for tcg, MTE will continue to be enabled as
>> long as tag memory has been provided.
>>
>> If MTE has been enabled, we need to disable migration, as we do not
>> yet have a way to migrate the tags as well. Therefore, MTE will stay
>> off with KVM unless requested explicitly.
>>
>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>> ---
>>  docs/system/arm/cpu-features.rst |  21 ++++++
>>  hw/arm/virt.c                    |   2 +-
>>  target/arm/cpu.c                 |  18 ++---
>>  target/arm/cpu.h                 |   1 +
>>  target/arm/cpu64.c               | 110 +++++++++++++++++++++++++++++++
>>  target/arm/internals.h           |   1 +
>>  target/arm/kvm.c                 |  29 ++++++++
>>  target/arm/kvm64.c               |   5 ++
>>  target/arm/kvm_arm.h             |  19 ++++++
>>  target/arm/monitor.c             |   1 +
>>  10 files changed, 194 insertions(+), 13 deletions(-)
>
>
>
>> +static inline bool arm_machine_has_tag_memory(void)
>> +{
>> +#ifndef CONFIG_USER_ONLY
>> +    Object *obj = object_dynamic_cast(qdev_get_machine(), TYPE_VIRT_MACHINE);
>> +
>> +    /* so far, only the virt machine has support for tag memory */
>> +    if (obj) {
>> +        VirtMachineState *vms = VIRT_MACHINE(obj);
>> +
>> +        return vms->mte;
>> +    }
>
> Code inside target/arm shouldn't be fishing around inside
> the details of the board model like this. For TCG I think that
> at this point (i.e. at realize) you should be able to tell if
> the board has set up tag memory, because it will have set
> cpu->tag_memory to non-NULL.

I agree that we shouldn't need to poke into the machine innards here,
but I found that it was actually too early to check for cpu->tag_memory --
details have unfortunately been flushed out of my cache already, can try
to repopulate.

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Cornelia Huck]

On Thu, Mar 02 2023, Peter Maydell <peter.maydell@linaro.org> wrote:

> On Thu, 2 Mar 2023 at 14:29, Cornelia Huck <cohuck@redhat.com> wrote:
>>
>> On Thu, Mar 02 2023, Peter Maydell <peter.maydell@linaro.org> wrote:
>> > I think having MTE in the specific case of KVM behave differently
>> > to how we've done all these existing properties and how we've
>> > done MTE for TCG would be confusing. The simplest thing is to just
>> > follow the existing UI for TCG MTE.
>> >
>> > The underlying reason for this is that MTE in general is not a feature
>> > only of the CPU, but also of the whole system design. It happens
>> > that KVM gives us tagged RAM "for free" but that's an oddity
>> > of the KVM implementation -- in real hardware there needs to
>> > be system level support for tagging.
>>
>> Hm... the Linux kernel actually seems to consider MTE to be a cpu
>> feature (at least, it lists it in the cpu features).
>>
>> So, is your suggestion to use the 'mte' prop of the virt machine to mean
>> "enable all prereqs for MTE, i.e. allocate tag memory for TCG and enable
>> MTE in the kernel for KVM"? For TCG, we'll get MTE for the max cpu
>> model; for KVM, we'd get MTE for host (== max), but I'm wondering what
>> should happen if we get named cpu models and the user specifies one
>> where we won't have MTE (i.e. some pre-8.5 one)?
>
> I think we can probably cross that bridge when we get to it,
> but I imagine the semantics would be "cortex-foo plus MTE"
> (in the same way that -cpu cortex-foo,+x,-y can add and
> subtract features from a baseline).

I'm wondering how we should try to model this, given that
cpu_model_advertised_features is a bit of a weird mix of architecture
flags and implementation-specific knobs.

Given that there are some KVM patchsets floating around to allow
userspace to limit some features for migration compat handling, I don't
think it will be too long before we'll try to figure out how to do cpu
models for KVM in QEMU as well...

Re: [PATCH v6 1/2] arm/kvm: add support for MTE

[Cornelia Huck]

On Thu, Mar 02 2023, Peter Maydell <peter.maydell@linaro.org> wrote:

> On Thu, 2 Mar 2023 at 14:29, Cornelia Huck <cohuck@redhat.com> wrote:
>>
>> On Thu, Mar 02 2023, Peter Maydell <peter.maydell@linaro.org> wrote:
>> > I think having MTE in the specific case of KVM behave differently
>> > to how we've done all these existing properties and how we've
>> > done MTE for TCG would be confusing. The simplest thing is to just
>> > follow the existing UI for TCG MTE.
>> >
>> > The underlying reason for this is that MTE in general is not a feature
>> > only of the CPU, but also of the whole system design. It happens
>> > that KVM gives us tagged RAM "for free" but that's an oddity
>> > of the KVM implementation -- in real hardware there needs to
>> > be system level support for tagging.
>>
>> Hm... the Linux kernel actually seems to consider MTE to be a cpu
>> feature (at least, it lists it in the cpu features).
>>
>> So, is your suggestion to use the 'mte' prop of the virt machine to mean
>> "enable all prereqs for MTE, i.e. allocate tag memory for TCG and enable
>> MTE in the kernel for KVM"? For TCG, we'll get MTE for the max cpu
>> model; for KVM, we'd get MTE for host (== max), but I'm wondering what
>> should happen if we get named cpu models and the user specifies one
>> where we won't have MTE (i.e. some pre-8.5 one)?
>
> I think we can probably cross that bridge when we get to it,
> but I imagine the semantics would be "cortex-foo plus MTE"
> (in the same way that -cpu cortex-foo,+x,-y can add and
> subtract features from a baseline).

While implementing this, I realized another thing that I had managed to
miss before: With tcg, we'll start out with mte=3 and downgrade to mte=0
if we don't have tag memory. With kvm, enabling mte can at most give us
the mte version that the host exposes, so setting mte=on for the machine
might give as well mte=2 in the end [which I still need to implement by
querying the host support, I guess.] This means we have slightly
different semantics for tcg and kvm; but more importantly, we need to
implement something for compat handling.

The Linux kernel distinguishes between 'mte' and 'mte3', and KVM exposes
the MTE cap if mte >=2. Do we need two props as well? If yes, what about
tcg?