* [PATCH v5] Hexagon: add PC alignment check and exception
@ 2024-05-02 19:20 Matheus Tavares Bernardino
2024-05-02 20:00 ` Richard Henderson
0 siblings, 1 reply; 4+ messages in thread
From: Matheus Tavares Bernardino @ 2024-05-02 19:20 UTC (permalink / raw)
To: qemu-devel
Cc: bcain, sidneym, ale, anjo, ltaylorsimpson, richard.henderson,
Laurent Vivier
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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Taylor Simpson <ltaylorsimpson@gmail.com>
---
Changes in v5:
- Merged asm and C test files into a single file.
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 | 98 +++++++++++++++++++++++++++++++
tests/tcg/hexagon/Makefile.target | 2 +
7 files changed, 119 insertions(+), 8 deletions(-)
create mode 100644 tests/tcg/hexagon/unaligned_pc.c
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..de50e5be9d
--- /dev/null
+++ b/tests/tcg/hexagon/unaligned_pc.c
@@ -0,0 +1,98 @@
+#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", "r29", "r30", "r31", "framekey");
+}
+
+void test_endloop(void)
+{
+ asm volatile(
+ "loop0(1f, #2)\n"
+ "1: r0 = #0x3\n"
+ "sa0 = r0\n"
+ "{ nop }:endloop0\n"
+ :
+ :
+ : "r0", "sa0", "lc0", "usr");
+}
+
+asm(".org 0x3\n"
+ ".global test_multi_cof_unaligned\n"
+ "test_multi_cof_unaligned:\n"
+ " jumpr r31\n");
+
+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 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", "framekey");
+ 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..e5182c01d8 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)
@@ -107,6 +108,7 @@ overflow: overflow.c hex_test.h
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
# This test has to be compiled for the -mv67t target
usr: usr.c hex_test.h
--
2.37.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v5] Hexagon: add PC alignment check and exception
2024-05-02 19:20 [PATCH v5] Hexagon: add PC alignment check and exception Matheus Tavares Bernardino
@ 2024-05-02 20:00 ` Richard Henderson
2024-05-03 13:38 ` Matheus Tavares Bernardino
0 siblings, 1 reply; 4+ messages in thread
From: Richard Henderson @ 2024-05-02 20:00 UTC (permalink / raw)
To: Matheus Tavares Bernardino, qemu-devel
Cc: bcain, sidneym, ale, anjo, ltaylorsimpson, Laurent Vivier
On 5/2/24 12:20, 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>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> ---
> Changes in v5:
> - Merged asm and C test files into a single file.
>
> 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 | 98 +++++++++++++++++++++++++++++++
> tests/tcg/hexagon/Makefile.target | 2 +
> 7 files changed, 119 insertions(+), 8 deletions(-)
> create mode 100644 tests/tcg/hexagon/unaligned_pc.c
>
> 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..de50e5be9d
> --- /dev/null
> +++ b/tests/tcg/hexagon/unaligned_pc.c
> @@ -0,0 +1,98 @@
> +#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", "r29", "r30", "r31", "framekey");
> +}
> +
> +void test_endloop(void)
> +{
> + asm volatile(
> + "loop0(1f, #2)\n"
> + "1: r0 = #0x3\n"
> + "sa0 = r0\n"
> + "{ nop }:endloop0\n"
> + :
> + :
> + : "r0", "sa0", "lc0", "usr");
> +}
> +
> +asm(".org 0x3\n"
> + ".global test_multi_cof_unaligned\n"
> + "test_multi_cof_unaligned:\n"
> + " jumpr r31\n");
This seems fragile, because you don't really know that the compiler has not emitted
something else before setting origin to 3. Nor do you really know that you're emitting
this hunk into the text section.
> +
> +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 1f\n"
> + "}\n"
> + "1: nop\n"
Does it work to write "jump 1f+1" or something?
While it shouldn't matter, perhaps trap[01] would be better than nop here?
Also, the bike shed should be green.
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v5] Hexagon: add PC alignment check and exception
2024-05-02 20:00 ` Richard Henderson
@ 2024-05-03 13:38 ` Matheus Tavares Bernardino
2024-05-03 14:56 ` Richard Henderson
0 siblings, 1 reply; 4+ messages in thread
From: Matheus Tavares Bernardino @ 2024-05-03 13:38 UTC (permalink / raw)
To: richard.henderson
Cc: quic_mathbern, qemu-devel, bcain, sidneym, ale, anjo,
ltaylorsimpson, laurent
On Thu, 2 May 2024 13:00:34 -0700 Richard Henderson <richard.henderson@linaro.org> wrote:
>
> On 5/2/24 12:20, 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"
> > + " if (!p0) jump 1f\n"
> > + "}\n"
> > + "1: nop\n"
>
> Does it work to write "jump 1f+1" or something?
Unfortunately no :( The assembler will align the address when encoding the
instruction. The only working examples I could think of is using a separated
file, like before, or manually encoding the instruction with a misaligned
address and place it with a `.word` directive... Any preferences, or other
suggestions?
> While it shouldn't matter, perhaps trap[01] would be better than nop here?
Ok!
> Also, the bike shed should be green.
hehe :)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v5] Hexagon: add PC alignment check and exception
2024-05-03 13:38 ` Matheus Tavares Bernardino
@ 2024-05-03 14:56 ` Richard Henderson
0 siblings, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2024-05-03 14:56 UTC (permalink / raw)
To: Matheus Tavares Bernardino
Cc: qemu-devel, bcain, sidneym, ale, anjo, ltaylorsimpson, laurent
On 5/3/24 06:38, Matheus Tavares Bernardino wrote:
> On Thu, 2 May 2024 13:00:34 -0700 Richard Henderson <richard.henderson@linaro.org> wrote:
>>
>> On 5/2/24 12:20, 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"
>>> + " if (!p0) jump 1f\n"
>>> + "}\n"
>>> + "1: nop\n"
>>
>> Does it work to write "jump 1f+1" or something?
>
> Unfortunately no :( The assembler will align the address when encoding the
> instruction. The only working examples I could think of is using a separated
> file, like before, or manually encoding the instruction with a misaligned
> address and place it with a `.word` directive... Any preferences, or other
> suggestions?
Oof. The assembler is being too helpful. :-P
Perhaps using a different section could solve the fragility issue:
asm("
.pushsection .text.evil
.org 3
...
.popsection
");
(adjusting syntax as necessary for correctness), then it doesn't matter where in the
output assembly the fragment lands.
r~
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-05-03 14:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-02 19:20 [PATCH v5] Hexagon: add PC alignment check and exception Matheus Tavares Bernardino
2024-05-02 20:00 ` Richard Henderson
2024-05-03 13:38 ` Matheus Tavares Bernardino
2024-05-03 14:56 ` Richard Henderson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).