[PATCH for-10.1] target/arm: Reinstate bogus AArch32 DBGDTRTX register for migration compat

[PATCH for-10.1] target/arm: Reinstate bogus AArch32 DBGDTRTX register for migration compat

[Peter Maydell]

In commit 655659a74a we fixed some bugs in the encoding of the
Debug Communications Channel registers, including that we were
incorrectly exposing an AArch32 register at p14, 3, c0, c5, 0.

Unfortunately removing a register is a break of forwards migration
compatibility for TCG, because we will fail the migration if the
source QEMU passes us a cpreg which the destination QEMU does not
have.  We don't have a mechanism for saying "it's OK to ignore this
sysreg in the inbound data", so for the 10.1 release reinstate the
incorrect AArch32 register.

(We probably have had other cases in the past of breaking migration
compatibility like this, but we didn't notice because we didn't test
and in any case not that many people care about TCG migration
compatibility.  KVM migration compat is not affected because for KVM
we treat the kernel as the source of truth for what system registers
are present.)

Fixes: 655659a74a36b ("target/arm: Correct encoding of Debug Communications Channel registers")
Reported-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Fabiano's suggestion for a migration compat mechanism is
https://patchew.org/QEMU/20250730205245.2118-1-farosas@suse.de/20250730205245.2118-2-farosas@suse.de/
---
 target/arm/debug_helper.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index aee06d4d426..579516e1541 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -940,6 +940,13 @@ 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
@@ -1002,6 +1009,28 @@ 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.43.0

Re: [PATCH for-10.1] target/arm: Reinstate bogus AArch32 DBGDTRTX register for migration compat

[Fabiano Rosas]

Peter Maydell <peter.maydell@linaro.org> writes:

> In commit 655659a74a we fixed some bugs in the encoding of the
> Debug Communications Channel registers, including that we were
> incorrectly exposing an AArch32 register at p14, 3, c0, c5, 0.
>
> Unfortunately removing a register is a break of forwards migration
> compatibility for TCG, because we will fail the migration if the
> source QEMU passes us a cpreg which the destination QEMU does not
> have.  We don't have a mechanism for saying "it's OK to ignore this
> sysreg in the inbound data", so for the 10.1 release reinstate the
> incorrect AArch32 register.
>
> (We probably have had other cases in the past of breaking migration
> compatibility like this, but we didn't notice because we didn't test
> and in any case not that many people care about TCG migration
> compatibility.  KVM migration compat is not affected because for KVM
> we treat the kernel as the source of truth for what system registers
> are present.)
>
> Fixes: 655659a74a36b ("target/arm: Correct encoding of Debug Communications Channel registers")
> Reported-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH for-10.1] target/arm: Reinstate bogus AArch32 DBGDTRTX register for migration compat

[Pierrick Bouvier]

On 7/31/25 6:43 AM, Peter Maydell wrote:
> In commit 655659a74a we fixed some bugs in the encoding of the
> Debug Communications Channel registers, including that we were
> incorrectly exposing an AArch32 register at p14, 3, c0, c5, 0.
> 
> Unfortunately removing a register is a break of forwards migration
> compatibility for TCG, because we will fail the migration if the
> source QEMU passes us a cpreg which the destination QEMU does not
> have.  We don't have a mechanism for saying "it's OK to ignore this
> sysreg in the inbound data", so for the 10.1 release reinstate the
> incorrect AArch32 register.
> 
> (We probably have had other cases in the past of breaking migration
> compatibility like this, but we didn't notice because we didn't test
> and in any case not that many people care about TCG migration
> compatibility.  KVM migration compat is not affected because for KVM
> we treat the kernel as the source of truth for what system registers
> are present.)
> 
> Fixes: 655659a74a36b ("target/arm: Correct encoding of Debug Communications Channel registers")
> Reported-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Fabiano's suggestion for a migration compat mechanism is
> https://patchew.org/QEMU/20250730205245.2118-1-farosas@suse.de/20250730205245.2118-2-farosas@suse.de/
> ---
>   target/arm/debug_helper.c | 29 +++++++++++++++++++++++++++++
>   1 file changed, 29 insertions(+)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH for-10.1] target/arm: Reinstate bogus AArch32 DBGDTRTX register for migration compat

[Richard Henderson]

On 7/31/25 23:43, Peter Maydell wrote:
> In commit 655659a74a we fixed some bugs in the encoding of the
> Debug Communications Channel registers, including that we were
> incorrectly exposing an AArch32 register at p14, 3, c0, c5, 0.
> 
> Unfortunately removing a register is a break of forwards migration
> compatibility for TCG, because we will fail the migration if the
> source QEMU passes us a cpreg which the destination QEMU does not
> have.  We don't have a mechanism for saying "it's OK to ignore this
> sysreg in the inbound data", so for the 10.1 release reinstate the
> incorrect AArch32 register.
> 
> (We probably have had other cases in the past of breaking migration
> compatibility like this, but we didn't notice because we didn't test
> and in any case not that many people care about TCG migration
> compatibility.  KVM migration compat is not affected because for KVM
> we treat the kernel as the source of truth for what system registers
> are present.)
> 
> Fixes: 655659a74a36b ("target/arm: Correct encoding of Debug Communications Channel registers")
> Reported-by: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~