diff mbox series

[v2] Hexagon: add PC alignment check and exception

Message ID e559b521d1920f804df10244c8c07564431aeba5.1714419461.git.quic_mathbern@quicinc.com (mailing list archive)
State New, archived
Headers show
Series [v2] Hexagon: add PC alignment check and exception | expand

Commit Message

Matheus Tavares Bernardino April 29, 2024, 7:40 p.m. UTC
The Hexagon Programmer's Reference Manual says that the exception 0x1e
should be raised upon an unaligned program counter. Let's implement that
and also add tests for both the most common case as well as packets with
multiple change-of-flow instructions.

Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
---
v1: https://lore.kernel.org/qemu-devel/c7af62451b02ffdc1d68bc00093b40a8080bc3ff.1714155331.git.quic_mathbern@quicinc.com/

Changes in v2:
- Moved PC alignment check from gen_write_new_pc_addr to
  cpu_get_tb_cpu_state, in order to get the right PC address at
  exception handling. (And also include relative PC instructions)
- Added test for endloop.
- Updated pkt_raises_exception for A_COF instructions.

 target/hexagon/cpu.h                       |  7 +++++++
 target/hexagon/cpu_bits.h                  |  4 ++++
 target/hexagon/macros.h                    |  3 ---
 target/hexagon/op_helper.c                 |  9 ++++-----
 target/hexagon/translate.c                 |  5 +++--
 tests/tcg/hexagon/Makefile.target          | 16 ++++++++++++++++
 tests/tcg/hexagon/unaligned_pc.S           | 10 ++++++++++
 tests/tcg/hexagon/unaligned_pc_endloop.S   |  8 ++++++++
 tests/tcg/hexagon/unaligned_pc_multi_cof.S | 13 +++++++++++++
 9 files changed, 65 insertions(+), 10 deletions(-)
 create mode 100644 tests/tcg/hexagon/unaligned_pc.S
 create mode 100644 tests/tcg/hexagon/unaligned_pc_endloop.S
 create mode 100644 tests/tcg/hexagon/unaligned_pc_multi_cof.S

Comments

Richard Henderson April 29, 2024, 8:59 p.m. UTC | #1
On 4/29/24 12:40, Matheus Tavares Bernardino wrote:
> @@ -144,6 +148,9 @@ static inline void cpu_get_tb_cpu_state(CPUHexagonState *env, vaddr *pc,
>           hex_flags = FIELD_DP32(hex_flags, TB_FLAGS, IS_TIGHT_LOOP, 1);
>       }
>       *flags = hex_flags;
> +    if (*pc & PCALIGN_MASK) {
> +        hexagon_raise_exception_err(env, HEX_EXCP_PC_NOT_ALIGNED, 0);
> +    }
>   }

With the test here ...

> diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
> index 47a870f42d..26e6809976 100644
> --- a/target/hexagon/translate.c
> +++ b/target/hexagon/translate.c
> @@ -346,8 +346,9 @@ static void mark_implicit_pred_writes(DisasContext *ctx)
>   static bool pkt_raises_exception(Packet *pkt)
>   {
>       if (check_for_attrib(pkt, A_LOAD) ||
> -        check_for_attrib(pkt, A_STORE)) {
> -        return true;
> +        check_for_attrib(pkt, A_STORE) ||
> +        check_for_attrib(pkt, A_COF)) {
> +            return true;
>       }

... you don't need a change here, because we don't raise the exception from within the packet.


> +run-unaligned_pc run-unaligned_pc_endloop run-unaligned_pc_multi_cof:
> +	$(call run-test, $<, $(QEMU) $< 2> $<.stderr,"$< on $(TARGET_NAME)"); \
> +	if [ $$? -ne 1 ] ; then \
> +		return 1; \
> +	fi
> +	$(call quiet-command, \
> +		grep -q "exception 0x1e" $<.stderr, \
> +		"GREP", "exception 0x1e");

What's missing in the patch set is handling this exception in 
linux-user/hexagon/cpu_loop.c.  This would correspond to misaligned_instruction() in the 
kernel.

At which point you could improve the test case to set up a signal handler in C, generate 
the exception, and test for correct values in the signal frame.


r~


PS: I think the kernel could be improved to use

    force_sig_fault(SIGBUS, BUS_ADRALN, regs->r31);
instead of
    force_sig(SIGBUS);

and similarly for misaligned_data_{load,store}.
diff mbox series

Patch

diff --git a/target/hexagon/cpu.h b/target/hexagon/cpu.h
index 3eef58fe8f..764f3c38cc 100644
--- a/target/hexagon/cpu.h
+++ b/target/hexagon/cpu.h
@@ -134,6 +134,10 @@  struct ArchCPU {
 
 FIELD(TB_FLAGS, IS_TIGHT_LOOP, 0, 1)
 
+G_NORETURN void hexagon_raise_exception_err(CPUHexagonState *env,
+                                            uint32_t exception,
+                                            uintptr_t pc);
+
 static inline void cpu_get_tb_cpu_state(CPUHexagonState *env, vaddr *pc,
                                         uint64_t *cs_base, uint32_t *flags)
 {
@@ -144,6 +148,9 @@  static inline void cpu_get_tb_cpu_state(CPUHexagonState *env, vaddr *pc,
         hex_flags = FIELD_DP32(hex_flags, TB_FLAGS, IS_TIGHT_LOOP, 1);
     }
     *flags = hex_flags;
+    if (*pc & PCALIGN_MASK) {
+        hexagon_raise_exception_err(env, HEX_EXCP_PC_NOT_ALIGNED, 0);
+    }
 }
 
 typedef HexagonCPU ArchCPU;
diff --git a/target/hexagon/cpu_bits.h b/target/hexagon/cpu_bits.h
index 96fef71729..4279281a71 100644
--- a/target/hexagon/cpu_bits.h
+++ b/target/hexagon/cpu_bits.h
@@ -20,9 +20,13 @@ 
 
 #include "qemu/bitops.h"
 
+#define PCALIGN 4
+#define PCALIGN_MASK (PCALIGN - 1)
+
 #define HEX_EXCP_FETCH_NO_UPAGE  0x012
 #define HEX_EXCP_INVALID_PACKET  0x015
 #define HEX_EXCP_INVALID_OPCODE  0x015
+#define HEX_EXCP_PC_NOT_ALIGNED  0x01e
 #define HEX_EXCP_PRIV_NO_UREAD   0x024
 #define HEX_EXCP_PRIV_NO_UWRITE  0x025
 
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index 1376d6ccc1..f375471a98 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -22,9 +22,6 @@ 
 #include "hex_regs.h"
 #include "reg_fields.h"
 
-#define PCALIGN 4
-#define PCALIGN_MASK (PCALIGN - 1)
-
 #define GET_FIELD(FIELD, REGIN) \
     fEXTRACTU_BITS(REGIN, reg_field_info[FIELD].width, \
                    reg_field_info[FIELD].offset)
diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index da10ac5847..ae5a605513 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -36,10 +36,9 @@ 
 #define SF_MANTBITS    23
 
 /* Exceptions processing helpers */
-static G_NORETURN
-void do_raise_exception_err(CPUHexagonState *env,
-                            uint32_t exception,
-                            uintptr_t pc)
+G_NORETURN void hexagon_raise_exception_err(CPUHexagonState *env,
+                                            uint32_t exception,
+                                            uintptr_t pc)
 {
     CPUState *cs = env_cpu(env);
     qemu_log_mask(CPU_LOG_INT, "%s: %d\n", __func__, exception);
@@ -49,7 +48,7 @@  void do_raise_exception_err(CPUHexagonState *env,
 
 G_NORETURN void HELPER(raise_exception)(CPUHexagonState *env, uint32_t excp)
 {
-    do_raise_exception_err(env, excp, 0);
+    hexagon_raise_exception_err(env, excp, 0);
 }
 
 void log_store32(CPUHexagonState *env, target_ulong addr,
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 47a870f42d..26e6809976 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -346,8 +346,9 @@  static void mark_implicit_pred_writes(DisasContext *ctx)
 static bool pkt_raises_exception(Packet *pkt)
 {
     if (check_for_attrib(pkt, A_LOAD) ||
-        check_for_attrib(pkt, A_STORE)) {
-        return true;
+        check_for_attrib(pkt, A_STORE) ||
+        check_for_attrib(pkt, A_COF)) {
+            return true;
     }
     return false;
 }
diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target
index f839b2c0d5..36d6e1f1d4 100644
--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -51,6 +51,22 @@  HEX_TESTS += scatter_gather
 HEX_TESTS += hvx_misc
 HEX_TESTS += hvx_histogram
 HEX_TESTS += invalid-slots
+HEX_TESTS += unaligned_pc
+HEX_TESTS += unaligned_pc_endloop
+HEX_TESTS += unaligned_pc_multi_cof
+
+run-unaligned_pc: unaligned_pc
+run-unaligned_pc_endloop: unaligned_pc_endloop
+run-unaligned_pc_multi_cof: unaligned_pc_multi_cof
+
+run-unaligned_pc run-unaligned_pc_endloop run-unaligned_pc_multi_cof:
+	$(call run-test, $<, $(QEMU) $< 2> $<.stderr,"$< on $(TARGET_NAME)"); \
+	if [ $$? -ne 1 ] ; then \
+		return 1; \
+	fi
+	$(call quiet-command, \
+		grep -q "exception 0x1e" $<.stderr, \
+		"GREP", "exception 0x1e");
 
 run-and-check-exception = $(call run-test,$2,$3 2>$2.stderr; \
 	test $$? -eq 1 && grep -q "exception $(strip $1)" $2.stderr)
diff --git a/tests/tcg/hexagon/unaligned_pc.S b/tests/tcg/hexagon/unaligned_pc.S
new file mode 100644
index 0000000000..39d6b2060b
--- /dev/null
+++ b/tests/tcg/hexagon/unaligned_pc.S
@@ -0,0 +1,10 @@ 
+test:
+	allocframe(#0x8)
+	r0 = #0xffffffff
+	framekey = r0
+	dealloc_return
+
+.global _start
+_start:
+	call test
+	jump pass
diff --git a/tests/tcg/hexagon/unaligned_pc_endloop.S b/tests/tcg/hexagon/unaligned_pc_endloop.S
new file mode 100644
index 0000000000..d6a4edc262
--- /dev/null
+++ b/tests/tcg/hexagon/unaligned_pc_endloop.S
@@ -0,0 +1,8 @@ 
+.global _start
+_start:
+	loop0(loop, #2)
+loop:
+	r0 = #0x3
+	sa0 = r0
+	{ nop }:endloop0
+	jump pass
diff --git a/tests/tcg/hexagon/unaligned_pc_multi_cof.S b/tests/tcg/hexagon/unaligned_pc_multi_cof.S
new file mode 100644
index 0000000000..a83e248ece
--- /dev/null
+++ b/tests/tcg/hexagon/unaligned_pc_multi_cof.S
@@ -0,0 +1,13 @@ 
+.org 0x3
+test:
+	nop
+	jumpr r31
+
+.global _start
+_start:
+	p0 = cmp.eq(r0, r0)
+	{
+		if (p0) jump test
+		jump pass
+	}
+	jump pass