| Message ID | 5c90567ec28723865e144f386b36f5b676b7a5d3.1714486874.git.quic_mathbern@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] Hexagon: add PC alignment check and exception | expand |
On 4/30/24 07:25, Matheus Tavares Bernardino wrote: > 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 some tests. > > Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> > --- > v2: https://lore.kernel.org/qemu-devel/e559b521d1920f804df10244c8c07564431aeba5.1714419461.git.quic_mathbern@quicinc.com/ > > Thanks for the comments, Richard and Taylor! > > Changed in v3: > - Removed now unnecessary pkt_raises_exception addition. > - Added HEX_EXCP_PC_NOT_ALIGNED handling at > linux-user/hexagon/cpu_loop.c. > - Merged all tests into a C file that uses signal handler to check > that the exception was raised. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 4/30/24 07:25, Matheus Tavares Bernardino wrote: > +void test_multi_cof(void) > +{ > + asm volatile( > + "p0 = cmp.eq(r0, r0)\n" > + "{\n" > + " if (p0) jump test_multi_cof_unaligned\n" > + " jump 1f\n" > + "}\n" > + "1: nop\n" > + : : : "p0"); > +} I will say you could just add the label to the end of the asm here, like .byte 0 test_multi_cof_unaligned: rather than use a separate source file. r~
> -----Original Message----- > From: Richard Henderson <richard.henderson@linaro.org> > Sent: Tuesday, April 30, 2024 10:53 AM > To: Matheus Bernardino (QUIC) <quic_mathbern@quicinc.com>; qemu- > devel@nongnu.org > Cc: Brian Cain <bcain@quicinc.com>; Sid Manning <sidneym@quicinc.com>; > ale@rev.ng; anjo@rev.ng; ltaylorsimpson@gmail.com; Laurent Vivier > <laurent@vivier.eu> > Subject: Re: [PATCH v3] Hexagon: add PC alignment check and exception > > WARNING: This email originated from outside of Qualcomm. Please be wary of > any links or attachments, and do not enable macros. > > On 4/30/24 07:25, Matheus Tavares Bernardino wrote: > > +void test_multi_cof(void) > > +{ > > + asm volatile( > > + "p0 = cmp.eq(r0, r0)\n" > > + "{\n" > > + " if (p0) jump test_multi_cof_unaligned\n" > > + " jump 1f\n" > > + "}\n" > > + "1: nop\n" > > + : : : "p0"); > > +} > > I will say you could just add the label to the end of the asm here, like > > .byte 0 > test_multi_cof_unaligned: > > rather than use a separate source file. Agreed: that would simplify this test case definition and the patch a bit. -Brian
> -----Original Message----- > From: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> > Sent: Tuesday, April 30, 2024 9:25 AM > To: qemu-devel@nongnu.org > Cc: bcain@quicinc.com; sidneym@quicinc.com; ale@rev.ng; anjo@rev.ng; > ltaylorsimpson@gmail.com; richard.henderson@linaro.org; Laurent Vivier > <laurent@vivier.eu> > Subject: [PATCH v3] Hexagon: add PC alignment check and exception > > 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 some tests. > > Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> > --- > v2: https://lore.kernel.org/qemu- > devel/e559b521d1920f804df10244c8c07564431aeba5.1714419461.git.quic_ma > thbern@quicinc.com/ > > Thanks for the comments, Richard and Taylor! > > Changed in v3: > - Removed now unnecessary pkt_raises_exception addition. > - Added HEX_EXCP_PC_NOT_ALIGNED handling at > linux-user/hexagon/cpu_loop.c. > - Merged all tests into a C file that uses signal handler to check > that the exception was raised. > > target/hexagon/cpu.h | 7 ++ > target/hexagon/cpu_bits.h | 4 + > target/hexagon/macros.h | 3 - > linux-user/hexagon/cpu_loop.c | 4 + > target/hexagon/op_helper.c | 9 +-- > tests/tcg/hexagon/unaligned_pc.c | 85 ++++++++++++++++++++++ > tests/tcg/hexagon/Makefile.target | 4 + > tests/tcg/hexagon/unaligned_pc_multi_cof.S | 5 ++ > 8 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 > tests/tcg/hexagon/unaligned_pc.c create mode 100644 > tests/tcg/hexagon/unaligned_pc_multi_cof.S > > a/tests/tcg/hexagon/unaligned_pc.c b/tests/tcg/hexagon/unaligned_pc.c > new file mode 100644 > index 0000000000..1add2d0d99 > --- /dev/null > +++ b/tests/tcg/hexagon/unaligned_pc.c > @@ -0,0 +1,85 @@ > +#include <stdio.h> > +#include <signal.h> > +#include <setjmp.h> > +#include <stdlib.h> > + > +/* will be changed in signal handler */ volatile sig_atomic_t > +completed_tests; static jmp_buf after_test; static int nr_tests; > + > +void __attribute__((naked)) test_return(void) { > + asm volatile( > + "allocframe(#0x8)\n" > + "r0 = #0xffffffff\n" > + "framekey = r0\n" > + "dealloc_return\n" > + : : : "r0"); Add r29, r30, r31 to clobbers list. Add framekey to clobbers list (assuming the compiler will take it). > +} > + > +void test_endloop(void) > +{ > + asm volatile( > + "loop0(1f, #2)\n" > + "1: r0 = #0x3\n" > + "sa0 = r0\n" > + "{ nop }:endloop0\n" > + : : : "r0"); > +} Add sa0, lc0, usr to the clobbers list. > + > +void test_multi_cof(void) > +{ > + asm volatile( > + "p0 = cmp.eq(r0, r0)\n" > + "{\n" > + " if (p0) jump test_multi_cof_unaligned\n" > + " jump 1f\n" > + "}\n" > + "1: nop\n" > + : : : "p0"); > +} > + > +void sigbus_handler(int signum) > +{ > + /* retore framekey after test_return */ > + asm volatile( > + "r0 = #0\n" > + "framekey = r0\n" > + : : : "r0"); Add framekey to the clobbers list. > + printf("Test %d complete\n", completed_tests); > + completed_tests++; > + siglongjmp(after_test, 1); > +} > + > +void test_done(void) > +{ > + int err = (completed_tests != nr_tests); > + puts(err ? "FAIL" : "PASS"); > + exit(err); > +} > + > +typedef void (*test_fn)(void); > + > +int main() > +{ > + test_fn tests[] = { test_return, test_endloop, test_multi_cof, test_done > }; > + nr_tests = (sizeof(tests) / sizeof(tests[0])) - 1; > + > + struct sigaction sa = { > + .sa_sigaction = sigbus_handler, > + .sa_flags = SA_SIGINFO > + }; > + > + if (sigaction(SIGBUS, &sa, NULL) < 0) { > + perror("sigaction"); > + return EXIT_FAILURE; > + } > + > + sigsetjmp(after_test, 1); > + tests[completed_tests](); > + > + /* should never get here */ > + puts("FAIL"); > + return 1; > +} > diff --git a/tests/tcg/hexagon/Makefile.target > b/tests/tcg/hexagon/Makefile.target > index f839b2c0d5..75139e731c 100644 > --- a/tests/tcg/hexagon/Makefile.target > +++ b/tests/tcg/hexagon/Makefile.target > @@ -51,6 +51,7 @@ HEX_TESTS += scatter_gather HEX_TESTS += hvx_misc > HEX_TESTS += hvx_histogram HEX_TESTS += invalid-slots > +HEX_TESTS += unaligned_pc > > run-and-check-exception = $(call run-test,$2,$3 2>$2.stderr; \ > test $$? -eq 1 && grep -q "exception $(strip $1)" $2.stderr) @@ - > 108,6 +109,9 @@ preg_alias: preg_alias.c hex_test.h > read_write_overlap: read_write_overlap.c hex_test.h > reg_mut: reg_mut.c hex_test.h > > +unaligned_pc: unaligned_pc.c unaligned_pc_multi_cof.S > + $(CC) $(CFLAGS) $(CROSS_CC_GUEST_CFLAGS) -mv73 $^ -o $@ > $(LDFLAGS) > + > # This test has to be compiled for the -mv67t target > usr: usr.c hex_test.h > $(CC) $(CFLAGS) -mv67t -O2 -Wno-inline-asm -Wno-expansion-to- > defined $< -o $@ $(LDFLAGS) 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..10accd0057 > --- /dev/null > +++ b/tests/tcg/hexagon/unaligned_pc_multi_cof.S > @@ -0,0 +1,5 @@ > +.org 0x3 > +.global test_multi_cof_unaligned > +test_multi_cof_unaligned: > + nop > + jumpr r31 > -- > 2.37.2
On Tue, 30 Apr 2024 08:52:36 -0700 Richard Henderson <richard.henderson@linaro.org> wrote: > > On 4/30/24 07:25, Matheus Tavares Bernardino wrote: > > +void test_multi_cof(void) > > +{ > > + asm volatile( > > + "p0 = cmp.eq(r0, r0)\n" > > + "{\n" > > + " if (p0) jump test_multi_cof_unaligned\n" > > + " jump 1f\n" > > + "}\n" > > + "1: nop\n" > > + : : : "p0"); > > +} > > I will say you could just add the label to the end of the asm here, like > > .byte 0 > test_multi_cof_unaligned: > > rather than use a separate source file. > That would be nice, but unfortunately that doesn't work because the label gets aligned by the assembler :( diff --git a/tests/tcg/hexagon/unaligned_pc.c b/tests/tcg/hexagon/unaligned_pc.c index 1add2d0d99..3772947a86 100644 --- a/tests/tcg/hexagon/unaligned_pc.c +++ b/tests/tcg/hexagon/unaligned_pc.c @@ -33,10 +33,12 @@ void test_multi_cof(void) asm volatile( "p0 = cmp.eq(r0, r0)\n" "{\n" - " if (p0) jump test_multi_cof_unaligned\n" + " if (p0) jump 2f\n" " jump 1f\n" "}\n" "1: nop\n" + ".byte 0\n" + "2: nop\n" : : : "p0"); } Ends up producing: 00020dc0 <test_multi_cof>: 20dc0: 00 c0 9d a0 a09dc000 { allocframe(#0x0) } 20dc4: 00 c0 00 f2 f200c000 { p0 = cmp.eq(r0,r0) } 20dc8: 06 40 00 5c 5c004006 { if (p0) jump:nt 0x20dd4 <test_multi_cof+0x14> 20dcc: 04 c0 00 58 5800c004 jump 0x20dd0 <test_multi_cof+0x10> } 20dd0: 00 c0 00 7f 7f00c000 { nop } 20dd4: 00 00 c0 00 <unknown>
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/linux-user/hexagon/cpu_loop.c b/linux-user/hexagon/cpu_loop.c index 7f1499ed28..d41159e52a 100644 --- a/linux-user/hexagon/cpu_loop.c +++ b/linux-user/hexagon/cpu_loop.c @@ -60,6 +60,10 @@ void cpu_loop(CPUHexagonState *env) env->gpr[0] = ret; } break; + case HEX_EXCP_PC_NOT_ALIGNED: + force_sig_fault(TARGET_SIGBUS, TARGET_BUS_ADRALN, + env->gpr[HEX_REG_R31]); + break; case EXCP_ATOMIC: cpu_exec_step_atomic(cs); break; 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/tests/tcg/hexagon/unaligned_pc.c b/tests/tcg/hexagon/unaligned_pc.c new file mode 100644 index 0000000000..1add2d0d99 --- /dev/null +++ b/tests/tcg/hexagon/unaligned_pc.c @@ -0,0 +1,85 @@ +#include <stdio.h> +#include <signal.h> +#include <setjmp.h> +#include <stdlib.h> + +/* will be changed in signal handler */ +volatile sig_atomic_t completed_tests; +static jmp_buf after_test; +static int nr_tests; + +void __attribute__((naked)) test_return(void) +{ + asm volatile( + "allocframe(#0x8)\n" + "r0 = #0xffffffff\n" + "framekey = r0\n" + "dealloc_return\n" + : : : "r0"); +} + +void test_endloop(void) +{ + asm volatile( + "loop0(1f, #2)\n" + "1: r0 = #0x3\n" + "sa0 = r0\n" + "{ nop }:endloop0\n" + : : : "r0"); +} + +void test_multi_cof(void) +{ + asm volatile( + "p0 = cmp.eq(r0, r0)\n" + "{\n" + " if (p0) jump test_multi_cof_unaligned\n" + " jump 1f\n" + "}\n" + "1: nop\n" + : : : "p0"); +} + +void sigbus_handler(int signum) +{ + /* retore framekey after test_return */ + asm volatile( + "r0 = #0\n" + "framekey = r0\n" + : : : "r0"); + printf("Test %d complete\n", completed_tests); + completed_tests++; + siglongjmp(after_test, 1); +} + +void test_done(void) +{ + int err = (completed_tests != nr_tests); + puts(err ? "FAIL" : "PASS"); + exit(err); +} + +typedef void (*test_fn)(void); + +int main() +{ + test_fn tests[] = { test_return, test_endloop, test_multi_cof, test_done }; + nr_tests = (sizeof(tests) / sizeof(tests[0])) - 1; + + struct sigaction sa = { + .sa_sigaction = sigbus_handler, + .sa_flags = SA_SIGINFO + }; + + if (sigaction(SIGBUS, &sa, NULL) < 0) { + perror("sigaction"); + return EXIT_FAILURE; + } + + sigsetjmp(after_test, 1); + tests[completed_tests](); + + /* should never get here */ + puts("FAIL"); + return 1; +} diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target index f839b2c0d5..75139e731c 100644 --- a/tests/tcg/hexagon/Makefile.target +++ b/tests/tcg/hexagon/Makefile.target @@ -51,6 +51,7 @@ HEX_TESTS += scatter_gather HEX_TESTS += hvx_misc HEX_TESTS += hvx_histogram HEX_TESTS += invalid-slots +HEX_TESTS += unaligned_pc run-and-check-exception = $(call run-test,$2,$3 2>$2.stderr; \ test $$? -eq 1 && grep -q "exception $(strip $1)" $2.stderr) @@ -108,6 +109,9 @@ preg_alias: preg_alias.c hex_test.h read_write_overlap: read_write_overlap.c hex_test.h reg_mut: reg_mut.c hex_test.h +unaligned_pc: unaligned_pc.c unaligned_pc_multi_cof.S + $(CC) $(CFLAGS) $(CROSS_CC_GUEST_CFLAGS) -mv73 $^ -o $@ $(LDFLAGS) + # This test has to be compiled for the -mv67t target usr: usr.c hex_test.h $(CC) $(CFLAGS) -mv67t -O2 -Wno-inline-asm -Wno-expansion-to-defined $< -o $@ $(LDFLAGS) 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..10accd0057 --- /dev/null +++ b/tests/tcg/hexagon/unaligned_pc_multi_cof.S @@ -0,0 +1,5 @@ +.org 0x3 +.global test_multi_cof_unaligned +test_multi_cof_unaligned: + nop + jumpr r31
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 some tests. Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com> --- v2: https://lore.kernel.org/qemu-devel/e559b521d1920f804df10244c8c07564431aeba5.1714419461.git.quic_mathbern@quicinc.com/ Thanks for the comments, Richard and Taylor! Changed in v3: - Removed now unnecessary pkt_raises_exception addition. - Added HEX_EXCP_PC_NOT_ALIGNED handling at linux-user/hexagon/cpu_loop.c. - Merged all tests into a C file that uses signal handler to check that the exception was raised. target/hexagon/cpu.h | 7 ++ target/hexagon/cpu_bits.h | 4 + target/hexagon/macros.h | 3 - linux-user/hexagon/cpu_loop.c | 4 + target/hexagon/op_helper.c | 9 +-- tests/tcg/hexagon/unaligned_pc.c | 85 ++++++++++++++++++++++ tests/tcg/hexagon/Makefile.target | 4 + tests/tcg/hexagon/unaligned_pc_multi_cof.S | 5 ++ 8 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 tests/tcg/hexagon/unaligned_pc.c create mode 100644 tests/tcg/hexagon/unaligned_pc_multi_cof.S