[PATCH v3 00/11] Mitigation of "failed to load cpu:cpreg_vmstate_array_len" migration failures

[PATCH v3 00/11] Mitigation of "failed to load cpu:cpreg_vmstate_array_len" migration failures

[Eric Auger]

When migrating ARM guests accross same machines with different host
kernels we are likely to encounter failures such as:

"failed to load cpu:cpreg_vmstate_array_len"

This is due to the fact KVM exposes a different number of registers
to qemu on source and destination. When trying to migrate a bigger
register set to a smaller one, qemu cannot save the CPU state.

For example, recently we faced such kind of situations with:
- unconditionnal exposure of KVM_REG_ARM_VENDOR_HYP_BMAP_2 FW pseudo
  register from v6.16 onwards. Causes backward migration failure.
- removal of unconditionnal exposure of TCR2_EL1, PIRE0_EL1, PIR_EL1
  from v6.13 onwards. Causes forward migration failure.

This situation is really problematic for distributions which want to
guarantee forward and backward migration of a given machine type
between different releases.

While the series mainly targets KVM acceleration, this problem
also exists with TCG. For instance some registers may be exposed
while they shouldn't. Then it is tricky to fix that situation
without breaking forward migration. An example was provided by
Peter: 4f2b82f60 ("target/arm: Reinstate bogus AArch32 DBGDTRTX
register for migration compat).

This series introduces 2 CPU array properties that list
- the CPU registers to hide from the exposes sysregs (aims
  at removing registers from the destination)
- The CPU registers that may not exist but which can be found
  in the incoming migration stream (aims at ignoring extra
  registers in the incoming state)

An example is given to illustrate how those props
could be used to apply compats for machine types supposed to "see" the
same register set accross various host kernels.

Mitigation of DBGDTRTX issue would be achived by setting
x-mig-safe-missing-regs=0x40200000200e0298 which matches
AArch32 DBGDTRTX register index.

The first patch improves the tracing so that we can quickly detect
which registers do not match between the incoming stream and the
exposed sysregs

---

History:

v2 -> v3:
- revert target/arm: Reinstate bogus AArch32 DBGDTRTX register for migration compat
- fix some typos and rework target/arm/cpu.h hidden_regs comment (Connie)
- Even for TCG we use KVM index

v1 -> v2:
- fixed typos (Connie)
- Make it less KVM specific (tentative hidding of TCG regs, not
  tested)
- Tested DBGDTRTX TCG case reported by Peter
- No change to the property format yet. Ran out of idea. However
  I changed the name of the property with x-mig prefix
- Changed the terminology, kept hidding but remove fake which was
  confusing
- Simplified the logic for regs missing in the incoming stream and
  do not check anymore they are exposed on dest

Available at:
https://github.com/eauger/qemu/tree/mitig-v3


Eric Auger (11):
  hw/arm/virt: Rename arm_virtio_compat into arm_virt_compat_defaults
  target/arm/machine: Improve traces on register mismatch during
    migration
  target/arm/cpu: Allow registers to be hidden
  target/arm/machine: Allow extra regs in the incoming stream
  target/arm/helper: Skip hidden registers
  kvm-all: Add the capability to blacklist some KVM regs
  target/arm/cpu: Implement hide_reg callback()
  target/arm/cpu: Expose x-mig-hidden-regs and x-mig-safe-missing-regs
    properties
  hw/arm/virt: Declare AArch32 DBGDTRTX as safe to ignore in incoming
    stream
  Revert "target/arm: Reinstate bogus AArch32 DBGDTRTX register for
    migration compat"
  hw/arm/virt: [DO NOT UPSTREAM] Enforce compatibility with older
    kernels

 include/hw/core/cpu.h     |  2 ++
 target/arm/cpu.h          | 48 +++++++++++++++++++++++++++
 accel/kvm/kvm-all.c       | 12 +++++++
 hw/arm/virt.c             | 41 +++++++++++++++++++----
 target/arm/cpu.c          | 11 ++++++
 target/arm/debug_helper.c | 29 ----------------
 target/arm/helper.c       | 12 ++++++-
 target/arm/kvm.c          | 35 +++++++++++++++++++-
 target/arm/machine.c      | 70 +++++++++++++++++++++++++++++++++++----
 target/arm/trace-events   | 10 ++++++
 10 files changed, 227 insertions(+), 43 deletions(-)

-- 
2.52.0

[PATCH v3 01/11] hw/arm/virt: Rename arm_virtio_compat into arm_virt_compat_defaults

[Eric Auger]

Renaming arm_virtio_compat into arm_virt_compat_defaults
makes more obvious that those compats apply to all machine
types by default, if not overriden for specific ones. This also
matches the terminology used for pc-q35.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/virt.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 25fb2bab56..ec0af8e6e7 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -94,20 +94,21 @@
 #include "hw/cxl/cxl_host.h"
 #include "qemu/guest-random.h"
 
-static GlobalProperty arm_virt_compat[] = {
+static GlobalProperty arm_virt_compat_defaults[] = {
     { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "48" },
 };
-static const size_t arm_virt_compat_len = G_N_ELEMENTS(arm_virt_compat);
+static const size_t arm_virt_compat_defaults_len =
+    G_N_ELEMENTS(arm_virt_compat_defaults);
 
 /*
  * This cannot be called from the virt_machine_class_init() because
  * TYPE_VIRT_MACHINE is abstract and mc->compat_props g_ptr_array_new()
  * only is called on virt non abstract class init.
  */
-static void arm_virt_compat_set(MachineClass *mc)
+static void arm_virt_compat_default_set(MachineClass *mc)
 {
-    compat_props_add(mc->compat_props, arm_virt_compat,
-                     arm_virt_compat_len);
+    compat_props_add(mc->compat_props, arm_virt_compat_defaults,
+                     arm_virt_compat_defaults_len);
 }
 
 #define DEFINE_VIRT_MACHINE_IMPL(latest, ...) \
@@ -116,7 +117,7 @@ static void arm_virt_compat_set(MachineClass *mc)
         const void *data) \
     { \
         MachineClass *mc = MACHINE_CLASS(oc); \
-        arm_virt_compat_set(mc); \
+        arm_virt_compat_default_set(mc); \
         MACHINE_VER_SYM(options, virt, __VA_ARGS__)(mc); \
         mc->desc = "QEMU " MACHINE_VER_STR(__VA_ARGS__) " ARM Virtual Machine"; \
         MACHINE_VER_DEPRECATION(__VA_ARGS__); \
-- 
2.52.0

[PATCH v3 02/11] target/arm/machine: Improve traces on register mismatch during migration

[Eric Auger]

Currently when the number of KVM registers exposed by the source is
larger than the one exposed on the destination, the migration fails
with: "failed to load cpu:cpreg_vmstate_array_len"

This gives no information about which registers are causing the trouble.

This patch reworks the target/arm/machine code so that it becomes
able to handle an input stream with a larger set of registers than
the destination and print useful information about which registers
are causing the trouble. The migration outcome is unchanged:
- unexpected registers still will fail the migration
- missing ones are printed but will not fail the migration, as done today.

The input stream can contain MAX_CPREG_VMSTATE_ANOMALIES(10) extra
registers compared to what exists on the target.

If there are more registers we will still hit the previous
"load cpu:cpreg_vmstate_array_len" error.

At most, MAX_CPREG_VMSTATE_ANOMALIES missing registers
and MAX_CPREG_VMSTATE_ANOMALIES unexpected registers are printed.

Example:

qemu-system-aarch64: kvm_arm_cpu_post_load Missing register in input stream: 0 0x6030000000160003 fw feat reg 3
qemu-system-aarch64: kvm_arm_cpu_post_load Unexpected register in input stream: 0 0x603000000013c103 op0:3 op1:0 crn:2 crm:0 op2:3
qemu-system-aarch64: kvm_arm_cpu_post_load Unexpected register in input stream: 1 0x603000000013c512 op0:3 op1:0 crn:10 crm:2 op2:2
qemu-system-aarch64: kvm_arm_cpu_post_load Unexpected register in input stream: 2 0x603000000013c513 op0:3 op1:0 crn:10 crm:2 op2:3
qemu-system-aarch64: error while loading state for instance 0x0 of device 'cpu'
qemu-system-aarch64: load of migration failed: Operation not permitted

With TCG there is no user friendly formatting of the faulting
register indexes as with KVM. However the 2 added trace points
help to identify the culprit indexes.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>

---

v2 -> v3:
- some extra typos (Connie)
- collected Connie's R-b

v1 -> v2:
- fixed some type in the commit msg
---
 target/arm/cpu.h        |  6 +++++
 target/arm/kvm.c        | 23 ++++++++++++++++
 target/arm/machine.c    | 58 ++++++++++++++++++++++++++++++++++++-----
 target/arm/trace-events |  7 +++++
 4 files changed, 88 insertions(+), 6 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 39f2b2e54d..077b0cce5b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -938,6 +938,12 @@ struct ArchCPU {
     uint64_t *cpreg_vmstate_values;
     int32_t cpreg_vmstate_array_len;
 
+    #define MAX_CPREG_VMSTATE_ANOMALIES 10
+    uint64_t cpreg_vmstate_missing_indexes[MAX_CPREG_VMSTATE_ANOMALIES];
+    int32_t cpreg_vmstate_missing_indexes_array_len;
+    uint64_t cpreg_vmstate_unexpected_indexes[MAX_CPREG_VMSTATE_ANOMALIES];
+    int32_t cpreg_vmstate_unexpected_indexes_array_len;
+
     DynamicGDBFeatureInfo dyn_sysreg_feature;
     DynamicGDBFeatureInfo dyn_svereg_feature;
     DynamicGDBFeatureInfo dyn_smereg_feature;
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 0d57081e69..58c6075a9e 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -1023,6 +1023,29 @@ void kvm_arm_cpu_pre_save(ARMCPU *cpu)
 
 bool kvm_arm_cpu_post_load(ARMCPU *cpu)
 {
+    int i;
+
+    for (i = 0; i < cpu->cpreg_vmstate_missing_indexes_array_len; i++) {
+        gchar *name;
+
+        name = kvm_print_register_name(cpu->cpreg_vmstate_missing_indexes[i]);
+        trace_kvm_arm_cpu_post_load_missing_reg(name);
+        g_free(name);
+    }
+
+    for (i = 0; i < cpu->cpreg_vmstate_unexpected_indexes_array_len; i++) {
+        gchar *name;
+
+        name = kvm_print_register_name(cpu->cpreg_vmstate_unexpected_indexes[i]);
+        error_report("%s Unexpected register in input stream: %i 0x%"PRIx64" %s",
+                     __func__, i, cpu->cpreg_vmstate_unexpected_indexes[i], name);
+        g_free(name);
+    }
+    /* Fail the migration if we detect unexpected registers */
+    if (cpu->cpreg_vmstate_unexpected_indexes_array_len) {
+        return false;
+    }
+
     if (!write_list_to_kvmstate(cpu, KVM_PUT_FULL_STATE)) {
         return false;
     }
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 0befdb0b28..f06a920aba 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -10,6 +10,7 @@
 #include "migration/vmstate.h"
 #include "target/arm/gtimer.h"
 #include "hw/arm/machines-qom.h"
+#include "trace.h"
 
 static bool vfp_needed(void *opaque)
 {
@@ -990,7 +991,13 @@ static int cpu_pre_load(void *opaque)
 {
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
+    int arraylen = cpu->cpreg_vmstate_array_len + MAX_CPREG_VMSTATE_ANOMALIES;
 
+    cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
+                                         arraylen);
+    cpu->cpreg_vmstate_values = g_renew(uint64_t, cpu->cpreg_vmstate_values,
+                                        arraylen);
+    cpu->cpreg_vmstate_array_len = arraylen;
     /*
      * In an inbound migration where on the source FPSCR/FPSR/FPCR are 0,
      * there will be no fpcr_fpsr subsection so we won't call vfp_set_fpcr()
@@ -1023,7 +1030,7 @@ static int cpu_post_load(void *opaque, int version_id)
 {
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
-    int i, v;
+    int i = 0, j = 0, k = 0, v = 0;
 
     /*
      * Handle migration compatibility from old QEMU which didn't
@@ -1051,27 +1058,66 @@ static int cpu_post_load(void *opaque, int version_id)
      * entries with the right slots in our own values array.
      */
 
-    for (i = 0, v = 0; i < cpu->cpreg_array_len
-             && v < cpu->cpreg_vmstate_array_len; i++) {
+    trace_cpu_post_load_len(cpu->cpreg_array_len, cpu->cpreg_vmstate_array_len);
+    for (; i < cpu->cpreg_array_len && v < cpu->cpreg_vmstate_array_len;) {
+        trace_cpu_post_load(i, v , cpu->cpreg_indexes[i]);
         if (cpu->cpreg_vmstate_indexes[v] > cpu->cpreg_indexes[i]) {
             /* register in our list but not incoming : skip it */
+            trace_cpu_post_load_missing(i, cpu->cpreg_indexes[i], v);
+            if (j < MAX_CPREG_VMSTATE_ANOMALIES) {
+                cpu->cpreg_vmstate_missing_indexes[j++] = cpu->cpreg_indexes[i];
+            }
+            i++;
             continue;
         }
         if (cpu->cpreg_vmstate_indexes[v] < cpu->cpreg_indexes[i]) {
-            /* register in their list but not ours: fail migration */
-            return -1;
+            /* register in their list but not ours: those will fail migration */
+            trace_cpu_post_load_unexpected(v, cpu->cpreg_vmstate_indexes[v], i);
+            if (k < MAX_CPREG_VMSTATE_ANOMALIES) {
+                cpu->cpreg_vmstate_unexpected_indexes[k++] =
+                    cpu->cpreg_vmstate_indexes[v];
+            }
+            v++;
+            continue;
         }
         /* matching register, copy the value over */
         cpu->cpreg_values[i] = cpu->cpreg_vmstate_values[v];
         v++;
+        i++;
     }
+    /*
+     * if we have reached the end of the incoming array but there are
+     * still regs in cpreg, continue parsing the regs which are missing
+     * in the input stream
+     */
+    for ( ; i < cpu->cpreg_array_len; i++) {
+        if (j < MAX_CPREG_VMSTATE_ANOMALIES) {
+            trace_cpu_post_load_missing(i, cpu->cpreg_indexes[i], v);
+            cpu->cpreg_vmstate_missing_indexes[j++] = cpu->cpreg_indexes[i];
+        }
+    }
+    /*
+     * if we have reached the end of the cpreg array but there are
+     * still regs in the input stream, continue parsing the vmstate array
+     */
+    for ( ; v < cpu->cpreg_vmstate_array_len; v++) {
+        if (k < MAX_CPREG_VMSTATE_ANOMALIES) {
+            trace_cpu_post_load_unexpected(v, cpu->cpreg_vmstate_indexes[v], i);
+            cpu->cpreg_vmstate_unexpected_indexes[k++] =
+                cpu->cpreg_vmstate_indexes[v];
+        }
+    }
+
+    cpu->cpreg_vmstate_missing_indexes_array_len = j;
+    cpu->cpreg_vmstate_unexpected_indexes_array_len = k;
 
     if (kvm_enabled()) {
         if (!kvm_arm_cpu_post_load(cpu)) {
             return -1;
         }
     } else {
-        if (!write_list_to_cpustate(cpu)) {
+        if (cpu->cpreg_vmstate_unexpected_indexes_array_len ||
+            !write_list_to_cpustate(cpu)) {
             return -1;
         }
     }
diff --git a/target/arm/trace-events b/target/arm/trace-events
index 676d29fe51..0a5ed3e69d 100644
--- a/target/arm/trace-events
+++ b/target/arm/trace-events
@@ -13,6 +13,7 @@ arm_gt_update_irq(int timer, int irqstate) "gt_update_irq: timer %d irqstate %d"
 
 # kvm.c
 kvm_arm_fixup_msi_route(uint64_t iova, uint64_t gpa) "MSI iova = 0x%"PRIx64" is translated into 0x%"PRIx64
+kvm_arm_cpu_post_load_missing_reg(char *name) "Missing register in input stream: %s"
 
 # cpu.c
 arm_cpu_reset(uint64_t mp_aff) "cpu %" PRIu64
@@ -26,3 +27,9 @@ arm_powerctl_reset_cpu(uint64_t mp_aff) "cpu %" PRIu64
 
 # tcg/psci.c and hvf/hvf.c
 arm_psci_call(uint64_t x0, uint64_t x1, uint64_t x2, uint64_t x3, uint32_t cpuid) "PSCI Call x0=0x%016"PRIx64" x1=0x%016"PRIx64" x2=0x%016"PRIx64" x3=0x%016"PRIx64" cpuid=0x%x"
+
+# machine.c
+cpu_post_load_len(int cpreg_array_len, int cpreg_vmstate_array_len) "cpreg_array_len=%d cpreg_vmstate_array_len=%d"
+cpu_post_load(int i, int v, uint64_t regidx) "i=%d v=%d regidx=0x%"PRIx64
+cpu_post_load_missing(int i, uint64_t regidx, int v) "missing register in input stream: i=%d index=0x%"PRIx64" (v=%d)"
+cpu_post_load_unexpected(int v, uint64_t regidx, int i) "unexpected register in input stream: v=%d index=0x%"PRIx64" (i=%d)"
-- 
2.52.0

[PATCH v3 03/11] target/arm/cpu: Allow registers to be hidden

[Eric Auger]

More recent kernels sometimes expose new registers in an
unconditionnal manner. This situation breaks backward migration
as qemu notices there are more registers in the input stream
than supported on the destination host. This leads to a
"failed to load cpu:cpreg_vmstate_array_len" error.

A good example is the introduction of KVM_REG_ARM_VENDOR_HYP_BMAP_2
pseudo FW register in v6.16 by commit C0000e58c74e (“KVM: arm64:
Introduce KVM_REG_ARM_VENDOR_HYP_BMAP_2”). Trying to do backward
migration from a host kernel that features the commit to a destination
host that doesn't, fail with above error.

Currently QEMU is not using that feature so ignoring this latter
is not a problem. An easy way to fix the migration issue is to teach
qemu we don't care about that register and we can simply ignore it
when syncing its state during migration.

This patch introduces an array of such hidden registers. Soon it will
be settable through an array property.

If hidden, the register is moved out of the array of cpreg which is
built in kvm_arm_init_cpreg_list(). That way their state won't be
synced.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v1 -> v2:
- Move the property in a separate patch
- improve the commit msg
- change the trace point to just print info in
  kvm_arm_init_cpreg_list()
- improve comment in cpu.h (Connie)
---
 target/arm/cpu.h        | 20 ++++++++++++++++++++
 target/arm/kvm.c        | 12 +++++++++++-
 target/arm/trace-events |  2 ++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 077b0cce5b..dcbdeaa742 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1044,6 +1044,15 @@ struct ArchCPU {
     /* KVM steal time */
     OnOffAuto kvm_steal_time;
 
+    /*
+     * Array of register indexes that need to be hidden to allow migration
+     * in certain cases, i.e. when a register is exposed in KVM or defined
+     * in TCG but not actually used in QEMU. Indexes are described in Linux
+     * Documentation/virt/kvm/api.rst for both KVM and TCG.
+     */
+    uint64_t *hidden_regs;
+    uint32_t nr_hidden_regs;
+
     /* Uniprocessor system with MP extensions */
     bool mp_is_up;
 
@@ -1184,6 +1193,17 @@ struct ARMCPUClass {
     ResettablePhases parent_phases;
 };
 
+static inline bool
+arm_cpu_hidden_reg(ARMCPU *cpu, uint64_t regidx)
+{
+    for (int i = 0; i < cpu->nr_hidden_regs; i++) {
+        if (cpu->hidden_regs[i] == regidx) {
+            return true;
+        }
+    }
+    return false;
+}
+
 /* Callback functions for the generic timer's timers. */
 void arm_gt_ptimer_cb(void *opaque);
 void arm_gt_vtimer_cb(void *opaque);
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 58c6075a9e..575a668f49 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -788,7 +788,10 @@ static int kvm_arm_init_cpreg_list(ARMCPU *cpu)
     qsort(&rlp->reg, rlp->n, sizeof(rlp->reg[0]), compare_u64);
 
     for (i = 0, arraylen = 0; i < rlp->n; i++) {
-        if (!kvm_arm_reg_syncs_via_cpreg_list(rlp->reg[i])) {
+        uint64_t regidx = rlp->reg[i];
+
+        if (!kvm_arm_reg_syncs_via_cpreg_list(regidx) ||
+            arm_cpu_hidden_reg(cpu, regidx)) {
             continue;
         }
         switch (rlp->reg[i] & KVM_REG_SIZE_MASK) {
@@ -804,6 +807,8 @@ static int kvm_arm_init_cpreg_list(ARMCPU *cpu)
         arraylen++;
     }
 
+    trace_kvm_arm_init_cpreg_list_arraylen(arraylen);
+
     cpu->cpreg_indexes = g_renew(uint64_t, cpu->cpreg_indexes, arraylen);
     cpu->cpreg_values = g_renew(uint64_t, cpu->cpreg_values, arraylen);
     cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
@@ -815,9 +820,14 @@ static int kvm_arm_init_cpreg_list(ARMCPU *cpu)
 
     for (i = 0, arraylen = 0; i < rlp->n; i++) {
         uint64_t regidx = rlp->reg[i];
+
         if (!kvm_arm_reg_syncs_via_cpreg_list(regidx)) {
             continue;
         }
+        if (arm_cpu_hidden_reg(cpu, regidx)) {
+            trace_kvm_arm_init_cpreg_list_skip_hidden_reg(rlp->reg[i]);
+            continue;
+        }
         cpu->cpreg_indexes[arraylen] = regidx;
         arraylen++;
     }
diff --git a/target/arm/trace-events b/target/arm/trace-events
index 0a5ed3e69d..20f4b4f2cd 100644
--- a/target/arm/trace-events
+++ b/target/arm/trace-events
@@ -14,6 +14,8 @@ arm_gt_update_irq(int timer, int irqstate) "gt_update_irq: timer %d irqstate %d"
 # kvm.c
 kvm_arm_fixup_msi_route(uint64_t iova, uint64_t gpa) "MSI iova = 0x%"PRIx64" is translated into 0x%"PRIx64
 kvm_arm_cpu_post_load_missing_reg(char *name) "Missing register in input stream: %s"
+kvm_arm_init_cpreg_list_arraylen(uint32_t arraylen) "arraylen=%d"
+kvm_arm_init_cpreg_list_skip_hidden_reg(uint64_t regidx) "hidden 0x%"PRIx64" is skipped"
 
 # cpu.c
 arm_cpu_reset(uint64_t mp_aff) "cpu %" PRIu64
-- 
2.52.0

[PATCH v3 04/11] target/arm/machine: Allow extra regs in the incoming stream

[Eric Auger]

Newer kernels may revoke exposure of KVM regs to userspace. This can
happen when one notices that some registers were unconditionnally
exposed whether they shall be conditionnally exposed for example.

An example of such situation is: TCR2_EL1, PIRE0_EL1,  PIR_EL1.
Associated kernel commits were:
0fcb4eea5345  KVM: arm64: Hide TCR2_EL1 from userspace when disabled for guests
a68cddbe47ef  KVM: arm64: Hide S1PIE registers from userspace when disabled for guests

Those commits were actual fixes but the cons is that is breaks forward
migration on some HW. Indeed when migrating from an old kernel that
does not feature those commits to a more recent one, destination
qemu detects there are more KVM regs in the input migration stream than
exposed by the destination host and the migration fails with:
"failed to load cpu:cpreg_vmstate_array_len"

This patchs adds the capability to define an array of register indexes
that may exist in the migration incoming stream but may be not
exposed by KVM on the destination.

We provision for extra space in cpreg_vmstate_* arrays during the preload
phase to allow the state to be saved without overflow, in case the
registers only are in the inbound data.

On postload we make sure to ignore them when analyzing potential
mismatch between registers. The actual cpreg array is never altered
meaning those registers are never accessed nor saved.

The array will be populated with a dedicated array property.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v2 -> v3:
- add a missing_as_expected trace point

v1 -> v2:
- get rid of the enforced/fake terminology
- remove the useless array of fake regs. Only the number of missing
  regs is needed

RFC -> v1:
- improve comment in target/arm/cpu.h (Connie)
---
 target/arm/cpu.h        | 22 ++++++++++++++++++++++
 target/arm/machine.c    | 30 +++++++++++++++++++++---------
 target/arm/trace-events |  1 +
 3 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index dcbdeaa742..9f0aa02fe7 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1053,6 +1053,15 @@ struct ArchCPU {
     uint64_t *hidden_regs;
     uint32_t nr_hidden_regs;
 
+    /*
+     * Registers that are likely to be part of the migration
+     * incoming stream but not exposed on destination. If
+     * their indexes are stored in this array, it is OK to
+     * ignore those registers in the inbound data.
+     */
+    uint64_t *mig_safe_missing_regs;
+    uint32_t nr_mig_safe_missing_regs;
+
     /* Uniprocessor system with MP extensions */
     bool mp_is_up;
 
@@ -1204,6 +1213,19 @@ arm_cpu_hidden_reg(ARMCPU *cpu, uint64_t regidx)
     return false;
 }
 
+
+static inline bool
+arm_cpu_safe_missing_reg(ARMCPU *cpu, uint64_t regidx)
+{
+    for (int i = 0; i < cpu->nr_mig_safe_missing_regs; i++) {
+        if (regidx == cpu->mig_safe_missing_regs[i]) {
+            return true;
+        }
+    }
+    return false;
+}
+
+
 /* Callback functions for the generic timer's timers. */
 void arm_gt_ptimer_cb(void *opaque);
 void arm_gt_vtimer_cb(void *opaque);
diff --git a/target/arm/machine.c b/target/arm/machine.c
index f06a920aba..98851afc61 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -991,7 +991,8 @@ static int cpu_pre_load(void *opaque)
 {
     ARMCPU *cpu = opaque;
     CPUARMState *env = &cpu->env;
-    int arraylen = cpu->cpreg_vmstate_array_len + MAX_CPREG_VMSTATE_ANOMALIES;
+    int arraylen = cpu->cpreg_vmstate_array_len +
+                   cpu->nr_mig_safe_missing_regs + MAX_CPREG_VMSTATE_ANOMALIES;
 
     cpu->cpreg_vmstate_indexes = g_renew(uint64_t, cpu->cpreg_vmstate_indexes,
                                          arraylen);
@@ -1058,6 +1059,10 @@ static int cpu_post_load(void *opaque, int version_id)
      * entries with the right slots in our own values array.
      */
 
+    /*
+     * at this point cpu->cpreg_vmstate_array_len was migrated with the
+     * actual length saved on source
+     */
     trace_cpu_post_load_len(cpu->cpreg_array_len, cpu->cpreg_vmstate_array_len);
     for (; i < cpu->cpreg_array_len && v < cpu->cpreg_vmstate_array_len;) {
         trace_cpu_post_load(i, v , cpu->cpreg_indexes[i]);
@@ -1072,10 +1077,15 @@ static int cpu_post_load(void *opaque, int version_id)
         }
         if (cpu->cpreg_vmstate_indexes[v] < cpu->cpreg_indexes[i]) {
             /* register in their list but not ours: those will fail migration */
-            trace_cpu_post_load_unexpected(v, cpu->cpreg_vmstate_indexes[v], i);
-            if (k < MAX_CPREG_VMSTATE_ANOMALIES) {
-                cpu->cpreg_vmstate_unexpected_indexes[k++] =
-                    cpu->cpreg_vmstate_indexes[v];
+            if (!arm_cpu_safe_missing_reg(cpu, cpu->cpreg_vmstate_indexes[v])) {
+                trace_cpu_post_load_unexpected(v, cpu->cpreg_vmstate_indexes[v], i);
+                if (k < MAX_CPREG_VMSTATE_ANOMALIES) {
+                    cpu->cpreg_vmstate_unexpected_indexes[k++] =
+                        cpu->cpreg_vmstate_indexes[v];
+                }
+            } else {
+                trace_cpu_post_load_missing_as_expected(v, cpu->cpreg_vmstate_indexes[v],
+                                                        i);
             }
             v++;
             continue;
@@ -1101,10 +1111,12 @@ static int cpu_post_load(void *opaque, int version_id)
      * still regs in the input stream, continue parsing the vmstate array
      */
     for ( ; v < cpu->cpreg_vmstate_array_len; v++) {
-        if (k < MAX_CPREG_VMSTATE_ANOMALIES) {
-            trace_cpu_post_load_unexpected(v, cpu->cpreg_vmstate_indexes[v], i);
-            cpu->cpreg_vmstate_unexpected_indexes[k++] =
-                cpu->cpreg_vmstate_indexes[v];
+        if (!arm_cpu_safe_missing_reg(cpu, cpu->cpreg_vmstate_indexes[v])) {
+            if (k < MAX_CPREG_VMSTATE_ANOMALIES) {
+                trace_cpu_post_load_unexpected(v, cpu->cpreg_vmstate_indexes[v], i);
+                cpu->cpreg_vmstate_unexpected_indexes[k++] =
+                    cpu->cpreg_vmstate_indexes[v];
+            }
         }
     }
 
diff --git a/target/arm/trace-events b/target/arm/trace-events
index 20f4b4f2cd..2e22012c69 100644
--- a/target/arm/trace-events
+++ b/target/arm/trace-events
@@ -35,3 +35,4 @@ cpu_post_load_len(int cpreg_array_len, int cpreg_vmstate_array_len) "cpreg_array
 cpu_post_load(int i, int v, uint64_t regidx) "i=%d v=%d regidx=0x%"PRIx64
 cpu_post_load_missing(int i, uint64_t regidx, int v) "missing register in input stream: i=%d index=0x%"PRIx64" (v=%d)"
 cpu_post_load_unexpected(int v, uint64_t regidx, int i) "unexpected register in input stream: v=%d index=0x%"PRIx64" (i=%d)"
+cpu_post_load_missing_as_expected(int v, uint64_t regidx, int i) "register missing as expected in input stream: v=%d index=0x%"PRIx64" (i=%d)"
-- 
2.52.0

[PATCH v3 05/11] target/arm/helper: Skip hidden registers

[Eric Auger]

In case a cpreg is hidden, skip it when initialing the cpreg
list.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v2 -> v3:
- use kvm_regidx
---
 target/arm/helper.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 27ebc6f29b..91ebefc50c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -235,9 +235,13 @@ static void add_cpreg_to_list(gpointer key, gpointer value, gpointer opaque)
     ARMCPU *cpu = opaque;
     uint32_t regidx = (uintptr_t)key;
     const ARMCPRegInfo *ri = value;
+    uint64_t kvm_regidx = cpreg_to_kvm_id(regidx);
 
+    if (arm_cpu_hidden_reg(cpu, kvm_regidx)) {
+        return;
+    }
     if (!(ri->type & (ARM_CP_NO_RAW | ARM_CP_ALIAS))) {
-        cpu->cpreg_indexes[cpu->cpreg_array_len] = cpreg_to_kvm_id(regidx);
+        cpu->cpreg_indexes[cpu->cpreg_array_len] = kvm_regidx;
         /* The value array need not be initialized at this point */
         cpu->cpreg_array_len++;
     }
@@ -247,6 +251,12 @@ static void count_cpreg(gpointer key, gpointer value, gpointer opaque)
 {
     ARMCPU *cpu = opaque;
     const ARMCPRegInfo *ri = value;
+    uint32_t regidx = (uintptr_t)key;
+    uint64_t kvm_regidx = cpreg_to_kvm_id(regidx);
+
+    if (arm_cpu_hidden_reg(cpu, kvm_regidx)) {
+        return;
+    }
 
     if (!(ri->type & (ARM_CP_NO_RAW | ARM_CP_ALIAS))) {
         cpu->cpreg_array_len++;
-- 
2.52.0

[PATCH v3 06/11] kvm-all: Add the capability to blacklist some KVM regs

[Eric Auger]

On ARM we want to be able to blacklist registers that are exposed
by KVM. To mitigate some mitigation failures that occur when a new
register is exposed and does not exist on the destination, some
registers are tagged "hidden" and their state won't be saved. As the
state is not saved and they are expected not to be used, we want to
enforce they aren't. So let's check this. The new CPUClass hide_reg()
callback is optional and will be implemented on ARM in a subsequent
patch.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 include/hw/core/cpu.h |  2 ++
 accel/kvm/kvm-all.c   | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 9615051774..5390e3e3d1 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -142,6 +142,7 @@ struct SysemuCPUOps;
  * the caller will not g_free() it.
  * @disas_set_info: Setup architecture specific components of disassembly info
  * @adjust_watchpoint_address: Perform a target-specific adjustment to an
+ * @hide_reg: Check if a register must be hidden (optional)
  * address before attempting to match it against watchpoints.
  * @deprecation_note: If this CPUClass is deprecated, this field provides
  *                    related information.
@@ -167,6 +168,7 @@ struct CPUClass {
     int (*gdb_read_register)(CPUState *cpu, GByteArray *buf, int reg);
     int (*gdb_write_register)(CPUState *cpu, uint8_t *buf, int reg);
     vaddr (*gdb_adjust_breakpoint)(CPUState *cpu, vaddr addr);
+    bool (*hide_reg)(CPUState *cpu, uint64_t regidex);
 
     const char *gdb_core_xml_file;
     const char * (*gdb_arch_name)(CPUState *cpu);
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f9254ae654..d047d49c0f 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3784,9 +3784,15 @@ bool kvm_device_supported(int vmfd, uint64_t type)
 
 int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source)
 {
+    CPUClass *cc = CPU_GET_CLASS(cs);
     struct kvm_one_reg reg;
     int r;
 
+    if (cc->hide_reg && cc->hide_reg(cs, id)) {
+        error_report("%s reg 0x%"PRIx64" is hidden and shall never been accessed",
+                     __func__, id);
+        g_assert_not_reached();
+    }
     reg.id = id;
     reg.addr = (uintptr_t) source;
     r = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
@@ -3798,9 +3804,15 @@ int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source)
 
 int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target)
 {
+    CPUClass *cc = CPU_GET_CLASS(cs);
     struct kvm_one_reg reg;
     int r;
 
+    if (cc->hide_reg && cc->hide_reg(cs, id)) {
+        error_report("%s reg 0x%"PRIx64" is hidden and shall never been accessed",
+                     __func__, id);
+        g_assert_not_reached();
+    }
     reg.id = id;
     reg.addr = (uintptr_t) target;
     r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
-- 
2.52.0

[PATCH v3 07/11] target/arm/cpu: Implement hide_reg callback()

[Eric Auger]

Check if the register is hidden.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 target/arm/cpu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 39292fb9bc..066746d76f 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2369,6 +2369,11 @@ static const TCGCPUOps arm_tcg_ops = {
 };
 #endif /* CONFIG_TCG */
 
+static inline bool arm_cpu_hide_reg(CPUState *s, uint64_t regidx)
+{
+    return arm_cpu_hidden_reg(ARM_CPU(s), regidx);
+}
+
 static void arm_cpu_class_init(ObjectClass *oc, const void *data)
 {
     ARMCPUClass *acc = ARM_CPU_CLASS(oc);
@@ -2397,6 +2402,7 @@ static void arm_cpu_class_init(ObjectClass *oc, const void *data)
     cc->gdb_get_core_xml_file = arm_gdb_get_core_xml_file;
     cc->gdb_stop_before_watchpoint = true;
     cc->disas_set_info = arm_disas_set_info;
+    cc->hide_reg = arm_cpu_hide_reg;
 
 #ifdef CONFIG_TCG
     cc->tcg_ops = &arm_tcg_ops;
-- 
2.52.0

[PATCH v3 08/11] target/arm/cpu: Expose x-mig-hidden-regs and x-mig-safe-missing-regs properties

[Eric Auger]

Allows to set both array properties along with arm cpus. Their "x-" prefix
reminds that those shall be used carefully for distro specific use cases
to garantee cross kernel migration.

This will allow to define such compat machine props like:

    static GlobalProperty arm_virt_kernel_compat_10_1[] = {
        /* KVM_REG_ARM_VENDOR_HYP_BMAP_2 */
        { TYPE_ARM_CPU, "x-mig-hidden-regs", "0x6030000000160003" },
        { TYPE_ARM_CPU, "x-mig-safe-missing-regs",
          /* TCR_EL1, PIRE0_EL1, PIR_EL1 */
          "0x603000000013c103, 0x603000000013c512, 0x603000000013c513" },
    }

The first one means KVM_REG_ARM_VENDOR_HYP_BMAP_2 shall always
been hidden for machine types older than 10.1. The second one means
that along with 10.1 machine type we may receive in the incoming
migration stream, 3 registers that are unknown on destination.

Obvioulsy, using the reg index as defined in
linux/Documentation/virt/kvm/api.rst is not user friendly. However
those options, prefixed with "x-" are supposed to be used
rarely by people who know the details.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 target/arm/cpu.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 066746d76f..c41774cb4c 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2248,6 +2248,11 @@ static const Property arm_cpu_properties[] = {
     DEFINE_PROP_BOOL("backcompat-cntfrq", ARMCPU, backcompat_cntfrq, false),
     DEFINE_PROP_BOOL("backcompat-pauth-default-use-qarma5", ARMCPU,
                       backcompat_pauth_default_use_qarma5, false),
+    DEFINE_PROP_ARRAY("x-mig-hidden-regs", ARMCPU,
+                      nr_hidden_regs, hidden_regs, qdev_prop_uint64, uint64_t),
+    DEFINE_PROP_ARRAY("x-mig-safe-missing-regs", ARMCPU,
+                      nr_mig_safe_missing_regs, mig_safe_missing_regs,
+                      qdev_prop_uint64, uint64_t),
 };
 
 static const gchar *arm_gdb_arch_name(CPUState *cs)
-- 
2.52.0

[PATCH v3 09/11] hw/arm/virt: Declare AArch32 DBGDTRTX as safe to ignore in incoming stream

[Eric Auger]

With the new infrastructure in place it is now feasible to teach
qemu that it is safe to ignore a sysreg in the incoming migration
stream. So with the plan to revert commit 4f2b82f60431 ("target/arm:
Reinstate bogus AArch32 DBGDTRTX register for migration compat) from
qemu 11 onwards, let's add a compat in 10.2 machine options stating
that this reg is safe to ignore. from 11.0 onwards we will not need
that register anymore.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

Theoretically removing that register could also impact other ARM
machines but I don't think we have any compat outside of arm virt,
do we?
---
 hw/arm/virt.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ec0af8e6e7..c6a5146c92 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -100,6 +100,14 @@ static GlobalProperty arm_virt_compat_defaults[] = {
 static const size_t arm_virt_compat_defaults_len =
     G_N_ELEMENTS(arm_virt_compat_defaults);
 
+#define DBGDTRTX 0x40200000200e0298
+
+static GlobalProperty arm_virt_compat_10_2[] = {
+    { TYPE_ARM_CPU, "x-mig-safe-missing-regs", stringify(DBGDTRTX)},
+};
+static const size_t arm_virt_compat_10_2_len =
+    G_N_ELEMENTS(arm_virt_compat_10_2);
+
 /*
  * This cannot be called from the virt_machine_class_init() because
  * TYPE_VIRT_MACHINE is abstract and mc->compat_props g_ptr_array_new()
@@ -3536,6 +3544,7 @@ type_init(machvirt_machine_init);
 
 static void virt_machine_10_2_options(MachineClass *mc)
 {
+    compat_props_add(mc->compat_props, arm_virt_compat_10_2, arm_virt_compat_10_2_len);
 }
 DEFINE_VIRT_MACHINE_AS_LATEST(10, 2)
 
-- 
2.52.0

[PATCH v3 10/11] Revert "target/arm: Reinstate bogus AArch32 DBGDTRTX register for migration compat"

[Eric Auger]

This reverts commit 4f2b82f60431e4792ecfd86a4d6b824248ee4c21. We don't
need that commit anymore as the AArch32 DBGDTRTX register is declared to
be safe to ignore in the incoming migration stream using a compat
in arm virt machine.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 target/arm/debug_helper.c | 29 -----------------------------
 1 file changed, 29 deletions(-)

diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 579516e154..aee06d4d42 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -940,13 +940,6 @@ static void dbgclaimclr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     env->cp15.dbgclaim &= ~(value & 0xFF);
 }
 
-static CPAccessResult access_bogus(CPUARMState *env, const ARMCPRegInfo *ri,
-                                   bool isread)
-{
-    /* Always UNDEF, as if this cpreg didn't exist */
-    return CP_ACCESS_UNDEFINED;
-}
-
 static const ARMCPRegInfo debug_cp_reginfo[] = {
     /*
      * DBGDRAR, DBGDSAR: always RAZ since we don't implement memory mapped
@@ -1009,28 +1002,6 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
       .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 4, .opc2 = 0,
       .access = PL0_RW, .accessfn = access_tdcc,
       .type = ARM_CP_CONST, .resetvalue = 0 },
-    /*
-     * This is not a real AArch32 register. We used to incorrectly expose
-     * this due to a QEMU bug; to avoid breaking migration compatibility we
-     * need to continue to provide it so that we don't fail the inbound
-     * migration when it tells us about a sysreg that we don't have.
-     * We set an always-fails .accessfn, which means that the guest doesn't
-     * actually see this register (it will always UNDEF, identically to if
-     * there were no cpreg definition for it other than that we won't print
-     * a LOG_UNIMP message about it), and we set the ARM_CP_NO_GDB flag so the
-     * gdbstub won't see it either.
-     * (We can't just set .access = 0, because add_cpreg_to_hashtable()
-     * helpfully ignores cpregs which aren't accessible to the highest
-     * implemented EL.)
-     *
-     * TODO: implement a system for being able to describe "this register
-     * can be ignored if it appears in the inbound stream"; then we can
-     * remove this temporary hack.
-     */
-    { .name = "BOGUS_DBGDTR_EL0", .state = ARM_CP_STATE_AA32,
-      .cp = 14, .opc1 = 3, .crn = 0, .crm = 5, .opc2 = 0,
-      .access = PL0_RW, .accessfn = access_bogus,
-      .type = ARM_CP_CONST | ARM_CP_NO_GDB, .resetvalue = 0 },
     /*
      * OSECCR_EL1 provides a mechanism for an operating system
      * to access the contents of EDECCR. EDECCR is not implemented though,
-- 
2.52.0

[PATCH v3 11/11] hw/arm/virt: [DO NOT UPSTREAM] Enforce compatibility with older kernels

[Eric Auger]

This is an example on how to use the new CPU options. This catters to
distributions who want machines to be migratable (forward and backward)
accross different host kernel versions in case KVM registers exposed
to qemu vary accross kernels. This patch is not meant to be upstreamed
as it is really kernel dependent. The goal is to illustrate how this
would be used.

In this example, For 10_1 machines types and older we apply the following
host kernel related compats:

1) Make sure the KVM_REG_ARM_VENDOR_HYP_BMAP_2 exposed from v6.15 onwards
   is ignored/hidden.
2) Make sure TCR_EL1, PIRE0_EL1, PIR_EL1 are always seen by qemu
   although not exposed by KVM. They were unconditionnally exposed before
   v6.13 while from v6.13 they are only exposed if supported by the guest.

This will allow 10_1 machines types and older machines to migrate
forward and backward from old downstream kernels that do not feature
those changes to newer kernels (>= v6.15).

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/arm/virt.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index c6a5146c92..2ffdebff98 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -108,6 +108,23 @@ static GlobalProperty arm_virt_compat_10_2[] = {
 static const size_t arm_virt_compat_10_2_len =
     G_N_ELEMENTS(arm_virt_compat_10_2);
 
+/*
+ * if a 10_1 machine type or older is used:
+ * 1) make sure TCR_EL1, PIRE0_EL1, PIR_EL1 are enforced, even if they are not
+ *    exposed by the kernel
+ * 2) hide KVM_REG_ARM_VENDOR_HYP_BMAP_2
+ */
+static GlobalProperty arm_virt_kernel_compat_10_1[] = {
+    /* KVM_REG_ARM_VENDOR_HYP_BMAP_2 */
+    { TYPE_ARM_CPU, "x-mig-hidden-regs", "0x6030000000160003" },
+    /* TCR_EL1, PIRE0_EL1, PIR_EL1 */
+    { TYPE_ARM_CPU, "x-mig-safe-missing-regs",
+      "0x603000000013c103, 0x603000000013c512, 0x603000000013c513" },
+};
+static const size_t arm_virt_kernel_compat_10_1_len =
+    G_N_ELEMENTS(arm_virt_kernel_compat_10_1);
+
+
 /*
  * This cannot be called from the virt_machine_class_init() because
  * TYPE_VIRT_MACHINE is abstract and mc->compat_props g_ptr_array_new()
@@ -3553,6 +3570,8 @@ static void virt_machine_10_1_options(MachineClass *mc)
     virt_machine_10_2_options(mc);
     mc->smbios_memory_device_size = 2047 * TiB;
     compat_props_add(mc->compat_props, hw_compat_10_1, hw_compat_10_1_len);
+    compat_props_add(mc->compat_props,
+                     arm_virt_kernel_compat_10_1, arm_virt_kernel_compat_10_1_len);
 }
 DEFINE_VIRT_MACHINE(10, 1)
 
-- 
2.52.0

Re: [PATCH v3 01/11] hw/arm/virt: Rename arm_virtio_compat into arm_virt_compat_defaults

[Cornelia Huck]

On Tue, Nov 25 2025, Eric Auger <eric.auger@redhat.com> wrote:

> Renaming arm_virtio_compat into arm_virt_compat_defaults
> makes more obvious that those compats apply to all machine
> types by default, if not overriden for specific ones. This also
> matches the terminology used for pc-q35.
>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  hw/arm/virt.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v3 03/11] target/arm/cpu: Allow registers to be hidden

[Cornelia Huck]

On Tue, Nov 25 2025, Eric Auger <eric.auger@redhat.com> wrote:

> More recent kernels sometimes expose new registers in an
> unconditionnal manner. This situation breaks backward migration
> as qemu notices there are more registers in the input stream
> than supported on the destination host. This leads to a
> "failed to load cpu:cpreg_vmstate_array_len" error.
>
> A good example is the introduction of KVM_REG_ARM_VENDOR_HYP_BMAP_2
> pseudo FW register in v6.16 by commit C0000e58c74e (“KVM: arm64:
> Introduce KVM_REG_ARM_VENDOR_HYP_BMAP_2”). Trying to do backward
> migration from a host kernel that features the commit to a destination
> host that doesn't, fail with above error.
>
> Currently QEMU is not using that feature so ignoring this latter
> is not a problem. An easy way to fix the migration issue is to teach
> qemu we don't care about that register and we can simply ignore it
> when syncing its state during migration.
>
> This patch introduces an array of such hidden registers. Soon it will
> be settable through an array property.
>
> If hidden, the register is moved out of the array of cpreg which is
> built in kvm_arm_init_cpreg_list(). That way their state won't be
> synced.

I'm wondering whether the patch description should also mention non-KVM
cases (e.g. the bogus reg that was exposed). It might also make sense to
merge patch 5 here?

>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Otherwise, LGTM.

Re: [PATCH v3 04/11] target/arm/machine: Allow extra regs in the incoming stream

[Cornelia Huck]

On Tue, Nov 25 2025, Eric Auger <eric.auger@redhat.com> wrote:

> Newer kernels may revoke exposure of KVM regs to userspace. This can
> happen when one notices that some registers were unconditionnally
> exposed whether they shall be conditionnally exposed for example.
>
> An example of such situation is: TCR2_EL1, PIRE0_EL1,  PIR_EL1.
> Associated kernel commits were:
> 0fcb4eea5345  KVM: arm64: Hide TCR2_EL1 from userspace when disabled for guests
> a68cddbe47ef  KVM: arm64: Hide S1PIE registers from userspace when disabled for guests
>
> Those commits were actual fixes but the cons is that is breaks forward
> migration on some HW. Indeed when migrating from an old kernel that
> does not feature those commits to a more recent one, destination
> qemu detects there are more KVM regs in the input migration stream than
> exposed by the destination host and the migration fails with:
> "failed to load cpu:cpreg_vmstate_array_len"
>
> This patchs adds the capability to define an array of register indexes
> that may exist in the migration incoming stream but may be not
> exposed by KVM on the destination.
>
> We provision for extra space in cpreg_vmstate_* arrays during the preload
> phase to allow the state to be saved without overflow, in case the
> registers only are in the inbound data.
>
> On postload we make sure to ignore them when analyzing potential
> mismatch between registers. The actual cpreg array is never altered
> meaning those registers are never accessed nor saved.
>
> The array will be populated with a dedicated array property.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v2 -> v3:
> - add a missing_as_expected trace point
>
> v1 -> v2:
> - get rid of the enforced/fake terminology
> - remove the useless array of fake regs. Only the number of missing
>   regs is needed
>
> RFC -> v1:
> - improve comment in target/arm/cpu.h (Connie)
> ---
>  target/arm/cpu.h        | 22 ++++++++++++++++++++++
>  target/arm/machine.c    | 30 +++++++++++++++++++++---------
>  target/arm/trace-events |  1 +
>  3 files changed, 44 insertions(+), 9 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v3 06/11] kvm-all: Add the capability to blacklist some KVM regs

[Cornelia Huck]

On Tue, Nov 25 2025, Eric Auger <eric.auger@redhat.com> wrote:

> On ARM we want to be able to blacklist registers that are exposed
> by KVM. To mitigate some mitigation failures that occur when a new
> register is exposed and does not exist on the destination, some
> registers are tagged "hidden" and their state won't be saved. As the
> state is not saved and they are expected not to be used, we want to
> enforce they aren't. So let's check this. The new CPUClass hide_reg()
> callback is optional and will be implemented on ARM in a subsequent
> patch.

Maybe "hide" or "ignore" instead of "blacklist"?

>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  include/hw/core/cpu.h |  2 ++
>  accel/kvm/kvm-all.c   | 12 ++++++++++++
>  2 files changed, 14 insertions(+)
>

Otherwise,

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v3 07/11] target/arm/cpu: Implement hide_reg callback()

[Cornelia Huck]

On Tue, Nov 25 2025, Eric Auger <eric.auger@redhat.com> wrote:

> Check if the register is hidden.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  target/arm/cpu.c | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [PATCH v3 08/11] target/arm/cpu: Expose x-mig-hidden-regs and x-mig-safe-missing-regs properties

[Cornelia Huck]

On Tue, Nov 25 2025, Eric Auger <eric.auger@redhat.com> wrote:

> Allows to set both array properties along with arm cpus. Their "x-" prefix
> reminds that those shall be used carefully for distro specific use cases
> to garantee cross kernel migration.

s/garantee/enable/ ?

>
> This will allow to define such compat machine props like:
>
>     static GlobalProperty arm_virt_kernel_compat_10_1[] = {
>         /* KVM_REG_ARM_VENDOR_HYP_BMAP_2 */
>         { TYPE_ARM_CPU, "x-mig-hidden-regs", "0x6030000000160003" },
>         { TYPE_ARM_CPU, "x-mig-safe-missing-regs",
>           /* TCR_EL1, PIRE0_EL1, PIR_EL1 */
>           "0x603000000013c103, 0x603000000013c512, 0x603000000013c513" },
>     }
>
> The first one means KVM_REG_ARM_VENDOR_HYP_BMAP_2 shall always
> been hidden for machine types older than 10.1. The second one means

s/been/be/

> that along with 10.1 machine type we may receive in the incoming
> migration stream, 3 registers that are unknown on destination.
>
> Obvioulsy, using the reg index as defined in

s/Obvioulsy/Obviously/

> linux/Documentation/virt/kvm/api.rst is not user friendly. However
> those options, prefixed with "x-" are supposed to be used
> rarely by people who know the details.

"These options are supposed to be used to enable specific, rare cases,
and in general, by people trying to configure distribution defaults
familiar with those specific cases." ?

I'm not sure whether the "x-" prefix is the right choice for "do not use
unless you really know what you're doing", but OTOH, it's the prefix we
have...

>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>  target/arm/cpu.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 066746d76f..c41774cb4c 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2248,6 +2248,11 @@ static const Property arm_cpu_properties[] = {
>      DEFINE_PROP_BOOL("backcompat-cntfrq", ARMCPU, backcompat_cntfrq, false),
>      DEFINE_PROP_BOOL("backcompat-pauth-default-use-qarma5", ARMCPU,
>                        backcompat_pauth_default_use_qarma5, false),
> +    DEFINE_PROP_ARRAY("x-mig-hidden-regs", ARMCPU,
> +                      nr_hidden_regs, hidden_regs, qdev_prop_uint64, uint64_t),
> +    DEFINE_PROP_ARRAY("x-mig-safe-missing-regs", ARMCPU,
> +                      nr_mig_safe_missing_regs, mig_safe_missing_regs,
> +                      qdev_prop_uint64, uint64_t),
>  };
>  
>  static const gchar *arm_gdb_arch_name(CPUState *cs)

Re: [PATCH v3 09/11] hw/arm/virt: Declare AArch32 DBGDTRTX as safe to ignore in incoming stream

[Cornelia Huck]

On Tue, Nov 25 2025, Eric Auger <eric.auger@redhat.com> wrote:

> With the new infrastructure in place it is now feasible to teach
> qemu that it is safe to ignore a sysreg in the incoming migration
> stream. So with the plan to revert commit 4f2b82f60431 ("target/arm:
> Reinstate bogus AArch32 DBGDTRTX register for migration compat) from
> qemu 11 onwards, let's add a compat in 10.2 machine options stating
> that this reg is safe to ignore. from 11.0 onwards we will not need
> that register anymore.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> Theoretically removing that register could also impact other ARM
> machines but I don't think we have any compat outside of arm virt,
> do we?

I don't think we give any guarantees outside of versioned machines (of
which virt is the only one for ARM.)

> ---
>  hw/arm/virt.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ec0af8e6e7..c6a5146c92 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -100,6 +100,14 @@ static GlobalProperty arm_virt_compat_defaults[] = {
>  static const size_t arm_virt_compat_defaults_len =
>      G_N_ELEMENTS(arm_virt_compat_defaults);
>  
> +#define DBGDTRTX 0x40200000200e0298

This probably wants a comment such as "bogus register erronously exposed
on 10.2 and earlier" or so.

> +
> +static GlobalProperty arm_virt_compat_10_2[] = {
> +    { TYPE_ARM_CPU, "x-mig-safe-missing-regs", stringify(DBGDTRTX)},
> +};
> +static const size_t arm_virt_compat_10_2_len =
> +    G_N_ELEMENTS(arm_virt_compat_10_2);
> +
>  /*
>   * This cannot be called from the virt_machine_class_init() because
>   * TYPE_VIRT_MACHINE is abstract and mc->compat_props g_ptr_array_new()
> @@ -3536,6 +3544,7 @@ type_init(machvirt_machine_init);
>  
>  static void virt_machine_10_2_options(MachineClass *mc)
>  {
> +    compat_props_add(mc->compat_props, arm_virt_compat_10_2, arm_virt_compat_10_2_len);
>  }
>  DEFINE_VIRT_MACHINE_AS_LATEST(10, 2)
>  
> -- 
> 2.52.0

Re: [PATCH v3 01/11] hw/arm/virt: Rename arm_virtio_compat into arm_virt_compat_defaults

[Sebastian Ott]

On Tue, 25 Nov 2025, Eric Auger wrote:
> Renaming arm_virtio_compat into arm_virt_compat_defaults
> makes more obvious that those compats apply to all machine
> types by default, if not overriden for specific ones. This also
> matches the terminology used for pc-q35.
>

s/virtio/virt/

Re: [PATCH v3 03/11] target/arm/cpu: Allow registers to be hidden

[Sebastian Ott]

[-- Attachment #1: Type: text/plain, Size: 1284 bytes --]

On Tue, 25 Nov 2025, Eric Auger wrote:
> More recent kernels sometimes expose new registers in an
> unconditionnal manner. This situation breaks backward migration
> as qemu notices there are more registers in the input stream
> than supported on the destination host. This leads to a
> "failed to load cpu:cpreg_vmstate_array_len" error.
>
> A good example is the introduction of KVM_REG_ARM_VENDOR_HYP_BMAP_2
> pseudo FW register in v6.16 by commit C0000e58c74e (“KVM: arm64:
> Introduce KVM_REG_ARM_VENDOR_HYP_BMAP_2”). Trying to do backward
> migration from a host kernel that features the commit to a destination
> host that doesn't, fail with above error.
>
> Currently QEMU is not using that feature so ignoring this latter
> is not a problem. An easy way to fix the migration issue is to teach
> qemu we don't care about that register and we can simply ignore it
> when syncing its state during migration.
>
> This patch introduces an array of such hidden registers. Soon it will
> be settable through an array property.
>
> If hidden, the register is moved out of the array of cpreg which is
> built in kvm_arm_init_cpreg_list(). That way their state won't be
> synced.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Reviewed-by: Sebastian Ott <sebott@redhat.com>

Re: [PATCH v3 01/11] hw/arm/virt: Rename arm_virtio_compat into arm_virt_compat_defaults

[Sebastian Ott]

On Tue, 25 Nov 2025, Eric Auger wrote:
> Renaming arm_virtio_compat into arm_virt_compat_defaults
> makes more obvious that those compats apply to all machine
> types by default, if not overriden for specific ones. This also
> matches the terminology used for pc-q35.
>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Reviewed-by: Sebastian Ott <sebott@redhat.com>

Re: [PATCH v3 06/11] kvm-all: Add the capability to blacklist some KVM regs

[Sebastian Ott]

On Tue, 25 Nov 2025, Eric Auger wrote:
> On ARM we want to be able to blacklist registers that are exposed
> by KVM. To mitigate some mitigation failures that occur when a new
                            ^
                            migration
> register is exposed and does not exist on the destination, some
> registers are tagged "hidden" and their state won't be saved. As the
> state is not saved and they are expected not to be used, we want to
> enforce they aren't. So let's check this. The new CPUClass hide_reg()
> callback is optional and will be implemented on ARM in a subsequent
> patch.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

How about "kvm-all: ensure hidden regs are not accessed"?

Reviewed-by: Sebastian Ott <sebott@redhat.com>

Re: [PATCH v3 07/11] target/arm/cpu: Implement hide_reg callback()

[Sebastian Ott]

On Tue, 25 Nov 2025, Eric Auger wrote:
> Check if the register is hidden.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

Reviewed-by: Sebastian Ott <sebott@redhat.com>