The frame that allocframe pushes is {LR, FP}, and what is scrambled is
the LR word: FRAMEKEY is XOR'd into the upper 32 bits.  The generated
code for fFRAME_SCRAMBLE built the key from FRAMEKEY in both halves of
a 64-bit value, so it scrambled FP as well.  deallocframe and
dealloc_return only undo the upper word, so a FP restored from a frame
pushed with a non-zero FRAMEKEY was corrupt.

Uncovered by testing some boot images using QuRT.

Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
---
 target/hexagon/idef-parser/parser-helpers.c |  2 +-
 tests/tcg/hexagon/framekey.c                | 67 +++++++++++++++++++++
 tests/tcg/hexagon/meson.build               |  1 +
 3 files changed, 69 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/hexagon/framekey.c

diff --git a/target/hexagon/idef-parser/parser-helpers.c b/target/hexagon/idef-parser/parser-helpers.c
index 49cc11fddd8..14840d98737 100644
--- a/target/hexagon/idef-parser/parser-helpers.c
+++ b/target/hexagon/idef-parser/parser-helpers.c
@@ -1989,7 +1989,7 @@ HexValue gen_rvalue_fscr(Context *c, YYLTYPE *locp, HexValue *value)
     *value = gen_rvalue_extend(c, locp, value);
     OUT(c, locp, "gen_read_reg(", &frame_key, ", HEX_REG_FRAMEKEY);\n");
     OUT(c, locp, "tcg_gen_concat_i32_i64(",
-        &key, ", ", &frame_key, ", ", &frame_key, ");\n");
+        &key, ", tcg_constant_tl(0), ", &frame_key, ");\n");
     OUT(c, locp, "tcg_gen_xor_i64(", &res, ", ", value, ", ", &key, ");\n");
     return res;
 }
diff --git a/tests/tcg/hexagon/framekey.c b/tests/tcg/hexagon/framekey.c
new file mode 100644
index 00000000000..5d579293477
--- /dev/null
+++ b/tests/tcg/hexagon/framekey.c
@@ -0,0 +1,67 @@
+/*
+ *  Copyright(c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ *  SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <stdio.h>
+#include <stdint.h>
+
+int err;
+
+#include "hex_test.h"
+
+/*
+ * allocframe pushes {LR, FP}, scrambling LR with FRAMEKEY, and deallocframe
+ * restores them.  Push a frame and return the words that were stored, FP
+ * through the return value and LR through @lr_stored, and the FP and LR
+ * that deallocframe restores.
+ */
+static inline uint32_t frame_roundtrip(uint32_t key, uint32_t fp,
+                                       uint32_t lr, uint32_t *lr_stored,
+                                       uint32_t *fp_back, uint32_t *lr_back)
+{
+    uint32_t fp_stored, saved_fp, saved_lr;
+
+    asm volatile("%[saved_fp] = r30\n\t"
+                 "%[saved_lr] = r31\n\t"
+                 "framekey = %[key]\n\t"
+                 "r30 = %[fp]\n\t"
+                 "r31 = %[lr]\n\t"
+                 "allocframe(#0)\n\t"
+                 "%[fp_stored] = memw(r30)\n\t"
+                 "%[lr_stored] = memw(r30 + #4)\n\t"
+                 "r31 = #0\n\t"
+                 "deallocframe\n\t"
+                 "%[fp_back] = r30\n\t"
+                 "%[lr_back] = r31\n\t"
+                 "framekey = %[zero]\n\t"
+                 "r30 = %[saved_fp]\n\t"
+                 "r31 = %[saved_lr]\n"
+                 : [fp_stored] "=&r"(fp_stored), [lr_stored] "=&r"(*lr_stored),
+                   [fp_back] "=&r"(*fp_back), [lr_back] "=&r"(*lr_back),
+                   [saved_fp] "=&r"(saved_fp), [saved_lr] "=&r"(saved_lr)
+                 : [key] "r"(key), [fp] "r"(fp), [lr] "r"(lr), [zero] "r"(0)
+                 : "memory", "framekey");
+    return fp_stored;
+}
+
+static void check_frame(uint32_t key, uint32_t fp, uint32_t lr)
+{
+    uint32_t lr_stored, fp_back, lr_back;
+
+    /* Only LR is scrambled, FP is stored as is */
+    check32(frame_roundtrip(key, fp, lr, &lr_stored, &fp_back, &lr_back), fp);
+    check32(lr_stored, lr ^ key);
+    check32(fp_back, fp);
+    check32(lr_back, lr);
+}
+
+int main()
+{
+    check_frame(0, 0x1234fff0, 0x5678abcc);
+    check_frame(0xf21f20a0, 0x1234fff0, 0x5678abcc);
+
+    puts(err ? "FAIL" : "PASS");
+    return err;
+}
diff --git a/tests/tcg/hexagon/meson.build b/tests/tcg/hexagon/meson.build
index a6ace4c77c2..0fd62ce7e4c 100644
--- a/tests/tcg/hexagon/meson.build
+++ b/tests/tcg/hexagon/meson.build
@@ -68,6 +68,7 @@ tests += {
   'overflow.c': {'cflags': cflags, 'exe_name': 'hex-overflow'},
   'preg_alias.c': {'cflags': cflags},
   'dual_stores.c': {'cflags': cflags},
+  'framekey.c': {'cflags': cflags},
   'multi_result.c': {'cflags': cflags},
   'mem_noshuf.c': {'cflags': cflags},
   'mem_noshuf_exception.c': {'cflags': cflags},
-- 
2.34.1

On Fri, Oct 16, 2026 at 12:08 PM Brian Cain <brian.cain@oss.qualcomm.com> wrote:
>
> The frame that allocframe pushes is {LR, FP}, and what is scrambled is
> the LR word: FRAMEKEY is XOR'd into the upper 32 bits.  The generated
> code for fFRAME_SCRAMBLE built the key from FRAMEKEY in both halves of
> a 64-bit value, so it scrambled FP as well.  deallocframe and
> dealloc_return only undo the upper word, so a FP restored from a frame
> pushed with a non-zero FRAMEKEY was corrupt.

I think the commit message is not quite right. AFAICT the error only
shows up when idef-parser is used, FRAMEKEY is not 0, and allocframe
is followed by dealloc_return.

That also means the test doesn't cover the case that actually breaks,
because it pops the frame with deallocframe. allocframe and
deallocframe have the same bug, so FP comes back correct either way.
Only the first check catches the bug, through the wrong FP word that
allocframe stores in memory. The other checks pass with or without the
fix.

On 10/19/2026 5:11 PM, Marco Liebel wrote:
> On Fri, Oct 16, 2026 at 12:08 PM Brian Cain <brian.cain@oss.qualcomm.com> wrote:
>> The frame that allocframe pushes is {LR, FP}, and what is scrambled is
>> the LR word: FRAMEKEY is XOR'd into the upper 32 bits.  The generated
>> code for fFRAME_SCRAMBLE built the key from FRAMEKEY in both halves of
>> a 64-bit value, so it scrambled FP as well.  deallocframe and
>> dealloc_return only undo the upper word, so a FP restored from a frame
>> pushed with a non-zero FRAMEKEY was corrupt.
> I think the commit message is not quite right. AFAICT the error only
> shows up when idef-parser is used, FRAMEKEY is not 0, and allocframe
> is followed by dealloc_return.
>
> That also means the test doesn't cover the case that actually breaks,
> because it pops the frame with deallocframe. allocframe and
> deallocframe have the same bug, so FP comes back correct either way.
> Only the first check catches the bug, through the wrong FP word that
> allocframe stores in memory. The other checks pass with or without the
> fix.


fixed in v2: 
https://lore.kernel.org/qemu-devel/20261020015154.3600098-1-brian.cain@oss.qualcomm.com/

On 10/16/2026 10:08 AM, Brian Cain wrote:
> The frame that allocframe pushes is {LR, FP}, and what is scrambled is
> the LR word: FRAMEKEY is XOR'd into the upper 32 bits.  The generated
> code for fFRAME_SCRAMBLE built the key from FRAMEKEY in both halves of
> a 64-bit value, so it scrambled FP as well.  deallocframe and
> dealloc_return only undo the upper word, so a FP restored from a frame
> pushed with a non-zero FRAMEKEY was corrupt.
> 
> Uncovered by testing some boot images using QuRT.
> 
> Signed-off-by: Brian Cain <brian.cain@oss.qualcomm.com>
> ---
>  target/hexagon/idef-parser/parser-helpers.c |  2 +-
>  tests/tcg/hexagon/framekey.c                | 67 +++++++++++++++++++++
>  tests/tcg/hexagon/meson.build               |  1 +
>  3 files changed, 69 insertions(+), 1 deletion(-)
>  create mode 100644 tests/tcg/hexagon/framekey.c
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>