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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/11/23 06:13, Cornelia Huck wrote:
@@ -2136,7 +2136,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",
                       kvm_enabled() ? "KVM" : "HVF");

Not your bug, but noticing this should use current_accel_name().

+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;
+    }
+#endif
+    return false;
+}

True for CONFIG_USER_ONLY, via page_get_target_data().
You should have seen check-tcg test failures...

+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:
+        if (!kvm_enabled()) {

tcg_enabled(), here and everywhere else you test for !kvm.

+#ifdef CONFIG_KVM
+        if (kvm_enabled() && !kvm_arm_mte_supported()) {

kvm_arm.h should get a stub inline returning false, so that the ifdef is removed.
See e.g. kvm_arm_sve_supported().

+    default: /* AUTO */
+        if (!kvm_enabled()) {

tcg_enabled.

+    /* accelerator-specific enablement */
+    if (kvm_enabled()) {
+#ifdef CONFIG_KVM
+        if (kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_MTE, 0)) {
+            error_setg(errp, "Failed to enable KVM_CAP_ARM_MTE");

Ideally this ifdef could go away as well.

+        } else {
+            /* TODO: add proper migration support with MTE enabled */
+            if (!mte_migration_blocker) {

Move the global variable here, as a local static?

I guess this check is to avoid adding one blocker per cpu?
I would guess the cap doesn't need enabling more than once either?


+                error_setg(&mte_migration_blocker,
+                           "Live migration disabled due to MTE enabled");
+                if (migrate_add_blocker(mte_migration_blocker, NULL)) {

You pass NULL to the migrate_add_blocker errp argument...

+                    error_setg(errp, "Failed to add MTE migration blocker");

... then make up your own generic reason for why it failed.
In this case it seems only related to another command-line option: --only-migratable.


Anyway, I wonder about hiding all of this in target/arm/kvm.c:

bool kvm_arm_enable_mte(Error *errp)
{
    static bool once = false;
    Error *blocker;

    if (once) {
        return;
    }
    once = true;

    if (kvm_vm_enable_cap(kvm_state, KVM_CAP_ARM_MTE, 0)) {
        error_setg_errno(errp, "Failed to enable KVM_CAP_ARM_MTE");
        return false;
    }

    blocker = g_new0(Error);
    error_setg(blocker, "Live migration disabled....");
    return !migrate_add_blocker(blocker, errp);
}

with

static inline bool kvm_arm_enable_mte(Error *errp)
{
    g_assert_not_reached();
}

in the !CONFIG_KVM block in kvm_arm.h.





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux