* [PULL 1/7] linux-user/hexagon: fix signal context save & restore
2023-01-05 17:38 [PULL 0/7] Hexagon update Taylor Simpson
@ 2023-01-05 17:38 ` Taylor Simpson
2023-01-05 17:38 ` [PULL 2/7] target/hexagon: rename aliased register HEX_REG_P3_0 Taylor Simpson
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Taylor Simpson @ 2023-01-05 17:38 UTC (permalink / raw)
To: qemu-devel
Cc: tsimpson, richard.henderson, philmd, peter.maydell, bcain,
quic_mathbern, stefanha, Mukilan Thiyagarajan
From: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com>
This patch fixes the issue originally reported in
this thread:
https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg01102.html
The root cause of the issue is a bug in the hexagon specific
logic for saving & restoring context during signal delivery.
The CPU state has two different representations for the
predicate registers. The current logic saves & restores only
the aliased HEX_REG_P3_O register, which is part of env->gpr[]
field in the CPU state, but not the individual byte-level
predicate registers (pO, p1, p2, p3) backed by env->pred[].
Since all predicated instructions refer only to the
indiviual registers, switching to and back from a signal handler
can clobber these registers if the signal handler writes to them
causing the normal application code to behave unpredictably when
context is restored.
In the reported issue with the 'signals' test, since the updated
hexagon toolchain had built musl with -O2, the functions called
from non_trivial_free were inlined. This meant that the code
emitted reused predicate P0 computed in the entry translation
block of the function non_trivial_free in one of the child TB
as part of an assertion. Since P0 is clobbered by the signal
handler in the signals test, the assertion in non_trivial_free
fails incorectly. Since musl for hexagon implements the 'abort'
function by deliberately writing to memory via null pointer,
this causes the test to fail with segmentation fault.
This patch modifies the signal context save & restore logic
to include the individual p0, p1, p2, p3 and excludes the
32b p3_0 register since its value is derived from the former
registers. It also adds a new test case that reliabily
reproduces the issue for all four predicate registers.
Buglink: https://github.com/quic/toolchain_for_hexagon/issues/6
Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
Message-Id: <20221229092006.10709-2-quic_mthiyaga@quicinc.com>
---
linux-user/hexagon/signal.c | 17 +++---
tests/tcg/hexagon/signal_context.c | 84 ++++++++++++++++++++++++++++++
tests/tcg/hexagon/Makefile.target | 1 +
3 files changed, 96 insertions(+), 6 deletions(-)
create mode 100644 tests/tcg/hexagon/signal_context.c
diff --git a/linux-user/hexagon/signal.c b/linux-user/hexagon/signal.c
index ad4e3822d5..60fa7e1bce 100644
--- a/linux-user/hexagon/signal.c
+++ b/linux-user/hexagon/signal.c
@@ -39,15 +39,12 @@ struct target_sigcontext {
target_ulong m0;
target_ulong m1;
target_ulong usr;
- target_ulong p3_0;
target_ulong gp;
target_ulong ugp;
target_ulong pc;
target_ulong cause;
target_ulong badva;
- target_ulong pad1;
- target_ulong pad2;
- target_ulong pad3;
+ target_ulong pred[NUM_PREGS];
};
struct target_ucontext {
@@ -118,10 +115,14 @@ static void setup_sigcontext(struct target_sigcontext *sc, CPUHexagonState *env)
__put_user(env->gpr[HEX_REG_M0], &sc->m0);
__put_user(env->gpr[HEX_REG_M1], &sc->m1);
__put_user(env->gpr[HEX_REG_USR], &sc->usr);
- __put_user(env->gpr[HEX_REG_P3_0], &sc->p3_0);
__put_user(env->gpr[HEX_REG_GP], &sc->gp);
__put_user(env->gpr[HEX_REG_UGP], &sc->ugp);
__put_user(env->gpr[HEX_REG_PC], &sc->pc);
+
+ int i;
+ for (i = 0; i < NUM_PREGS; i++) {
+ __put_user(env->pred[i], &(sc->pred[i]));
+ }
}
static void setup_ucontext(struct target_ucontext *uc,
@@ -230,10 +231,14 @@ static void restore_sigcontext(CPUHexagonState *env,
__get_user(env->gpr[HEX_REG_M0], &sc->m0);
__get_user(env->gpr[HEX_REG_M1], &sc->m1);
__get_user(env->gpr[HEX_REG_USR], &sc->usr);
- __get_user(env->gpr[HEX_REG_P3_0], &sc->p3_0);
__get_user(env->gpr[HEX_REG_GP], &sc->gp);
__get_user(env->gpr[HEX_REG_UGP], &sc->ugp);
__get_user(env->gpr[HEX_REG_PC], &sc->pc);
+
+ int i;
+ for (i = 0; i < NUM_PREGS; i++) {
+ __get_user(env->pred[i], &(sc->pred[i]));
+ }
}
static void restore_ucontext(CPUHexagonState *env, struct target_ucontext *uc)
diff --git a/tests/tcg/hexagon/signal_context.c b/tests/tcg/hexagon/signal_context.c
new file mode 100644
index 0000000000..7202fa64b6
--- /dev/null
+++ b/tests/tcg/hexagon/signal_context.c
@@ -0,0 +1,84 @@
+/*
+ * Copyright(c) 2022 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdio.h>
+#include <signal.h>
+#include <time.h>
+
+void sig_user(int sig, siginfo_t *info, void *puc)
+{
+ asm("r7 = #0\n\t"
+ "p0 = r7\n\t"
+ "p1 = r7\n\t"
+ "p2 = r7\n\t"
+ "p3 = r7\n\t"
+ : : : "r7", "p0", "p1", "p2", "p3");
+}
+
+int main()
+{
+ int err = 0;
+ unsigned int i = 100000;
+ struct sigaction act;
+ struct itimerspec it;
+ timer_t tid;
+ struct sigevent sev;
+
+ act.sa_sigaction = sig_user;
+ sigemptyset(&act.sa_mask);
+ act.sa_flags = SA_SIGINFO;
+ sigaction(SIGUSR1, &act, NULL);
+ sev.sigev_notify = SIGEV_SIGNAL;
+ sev.sigev_signo = SIGUSR1;
+ sev.sigev_value.sival_ptr = &tid;
+ timer_create(CLOCK_REALTIME, &sev, &tid);
+ it.it_interval.tv_sec = 0;
+ it.it_interval.tv_nsec = 100000;
+ it.it_value.tv_sec = 0;
+ it.it_value.tv_nsec = 100000;
+ timer_settime(tid, 0, &it, NULL);
+
+ asm("loop0(1f, %1)\n\t"
+ "1: r8 = #0xff\n\t"
+ " p0 = r8\n\t"
+ " p1 = r8\n\t"
+ " p2 = r8\n\t"
+ " p3 = r8\n\t"
+ " jump 3f\n\t"
+ "2: memb(%0) = #1\n\t"
+ " jump 4f\n\t"
+ "3:\n\t"
+ " r8 = p0\n\t"
+ " p0 = cmp.eq(r8, #0xff)\n\t"
+ " if (!p0) jump 2b\n\t"
+ " r8 = p1\n\t"
+ " p0 = cmp.eq(r8, #0xff)\n\t"
+ " if (!p0) jump 2b\n\t"
+ " r8 = p2\n\t"
+ " p0 = cmp.eq(r8, #0xff)\n\t"
+ " if (!p0) jump 2b\n\t"
+ " r8 = p3\n\t"
+ " p0 = cmp.eq(r8, #0xff)\n\t"
+ " if (!p0) jump 2b\n\t"
+ "4: {}: endloop0\n\t"
+ :
+ : "r"(&err), "r"(i)
+ : "memory", "r8", "p0", "p1", "p2", "p3");
+
+ puts(err ? "FAIL" : "PASS");
+ return err;
+}
diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target
index 9ee1faa1e1..f1378d86a9 100644
--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -43,6 +43,7 @@ HEX_TESTS += load_align
HEX_TESTS += atomics
HEX_TESTS += fpstuff
HEX_TESTS += overflow
+HEX_TESTS += signal_context
HEX_TESTS += test_abs
HEX_TESTS += test_bitcnt
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL 2/7] target/hexagon: rename aliased register HEX_REG_P3_0
2023-01-05 17:38 [PULL 0/7] Hexagon update Taylor Simpson
2023-01-05 17:38 ` [PULL 1/7] linux-user/hexagon: fix signal context save & restore Taylor Simpson
@ 2023-01-05 17:38 ` Taylor Simpson
2023-01-05 17:38 ` [PULL 3/7] tests/tcg/hexagon: fix underspecifed asm constraints Taylor Simpson
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Taylor Simpson @ 2023-01-05 17:38 UTC (permalink / raw)
To: qemu-devel
Cc: tsimpson, richard.henderson, philmd, peter.maydell, bcain,
quic_mathbern, stefanha, Mukilan Thiyagarajan
From: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com>
The patch renames the identifier of the 32bit register
HEX_REG_P3_0 to HEX_REG_P3_0_ALIASED.
This change is to intended to provide some warning that
HEX_REG_P3_0 is an aliased register which has multiple
representations in CPU state and therefore might require
special handling in some contexts. The hope is to prevent
accidental misuse of this register e.g the issue reported
for the signals tests failure [here][1].
[1]: https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg01102.html
Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
Message-Id: <20221229092006.10709-3-quic_mthiyaga@quicinc.com>
---
target/hexagon/hex_regs.h | 2 +-
target/hexagon/cpu.c | 6 +++---
target/hexagon/genptr.c | 12 ++++++------
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/target/hexagon/hex_regs.h b/target/hexagon/hex_regs.h
index a63c2c0fd5..bddfc28021 100644
--- a/target/hexagon/hex_regs.h
+++ b/target/hexagon/hex_regs.h
@@ -58,7 +58,7 @@ enum {
HEX_REG_LC0 = 33,
HEX_REG_SA1 = 34,
HEX_REG_LC1 = 35,
- HEX_REG_P3_0 = 36,
+ HEX_REG_P3_0_ALIASED = 36,
HEX_REG_M0 = 38,
HEX_REG_M1 = 39,
HEX_REG_USR = 40,
diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 658ca4ff78..807037c586 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -86,7 +86,7 @@ static target_ulong adjust_stack_ptrs(CPUHexagonState *env, target_ulong addr)
return addr;
}
-/* HEX_REG_P3_0 (aka C4) is an alias for the predicate registers */
+/* HEX_REG_P3_0_ALIASED (aka C4) is an alias for the predicate registers */
static target_ulong read_p3_0(CPUHexagonState *env)
{
int32_t control_reg = 0;
@@ -102,7 +102,7 @@ static void print_reg(FILE *f, CPUHexagonState *env, int regnum)
{
target_ulong value;
- if (regnum == HEX_REG_P3_0) {
+ if (regnum == HEX_REG_P3_0_ALIASED) {
value = read_p3_0(env);
} else {
value = regnum < 32 ? adjust_stack_ptrs(env, env->gpr[regnum])
@@ -198,7 +198,7 @@ static void hexagon_dump(CPUHexagonState *env, FILE *f, int flags)
print_reg(f, env, HEX_REG_M0);
print_reg(f, env, HEX_REG_M1);
print_reg(f, env, HEX_REG_USR);
- print_reg(f, env, HEX_REG_P3_0);
+ print_reg(f, env, HEX_REG_P3_0_ALIASED);
print_reg(f, env, HEX_REG_GP);
print_reg(f, env, HEX_REG_UGP);
print_reg(f, env, HEX_REG_PC);
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 6cf2e0ed43..66a968c884 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -163,7 +163,7 @@ static inline void gen_read_p3_0(TCGv control_reg)
/*
* Certain control registers require special handling on read
- * HEX_REG_P3_0 aliased to the predicate registers
+ * HEX_REG_P3_0_ALIASED aliased to the predicate registers
* -> concat the 4 predicate registers together
* HEX_REG_PC actual value stored in DisasContext
* -> assign from ctx->base.pc_next
@@ -173,7 +173,7 @@ static inline void gen_read_p3_0(TCGv control_reg)
static inline void gen_read_ctrl_reg(DisasContext *ctx, const int reg_num,
TCGv dest)
{
- if (reg_num == HEX_REG_P3_0) {
+ if (reg_num == HEX_REG_P3_0_ALIASED) {
gen_read_p3_0(dest);
} else if (reg_num == HEX_REG_PC) {
tcg_gen_movi_tl(dest, ctx->base.pc_next);
@@ -194,7 +194,7 @@ static inline void gen_read_ctrl_reg(DisasContext *ctx, const int reg_num,
static inline void gen_read_ctrl_reg_pair(DisasContext *ctx, const int reg_num,
TCGv_i64 dest)
{
- if (reg_num == HEX_REG_P3_0) {
+ if (reg_num == HEX_REG_P3_0_ALIASED) {
TCGv p3_0 = tcg_temp_new();
gen_read_p3_0(p3_0);
tcg_gen_concat_i32_i64(dest, p3_0, hex_gpr[reg_num + 1]);
@@ -238,7 +238,7 @@ static void gen_write_p3_0(DisasContext *ctx, TCGv control_reg)
/*
* Certain control registers require special handling on write
- * HEX_REG_P3_0 aliased to the predicate registers
+ * HEX_REG_P3_0_ALIASED aliased to the predicate registers
* -> break the value across 4 predicate registers
* HEX_REG_QEMU_*_CNT changes in current TB in DisasContext
* -> clear the changes
@@ -246,7 +246,7 @@ static void gen_write_p3_0(DisasContext *ctx, TCGv control_reg)
static inline void gen_write_ctrl_reg(DisasContext *ctx, int reg_num,
TCGv val)
{
- if (reg_num == HEX_REG_P3_0) {
+ if (reg_num == HEX_REG_P3_0_ALIASED) {
gen_write_p3_0(ctx, val);
} else {
gen_log_reg_write(reg_num, val);
@@ -266,7 +266,7 @@ static inline void gen_write_ctrl_reg(DisasContext *ctx, int reg_num,
static inline void gen_write_ctrl_reg_pair(DisasContext *ctx, int reg_num,
TCGv_i64 val)
{
- if (reg_num == HEX_REG_P3_0) {
+ if (reg_num == HEX_REG_P3_0_ALIASED) {
TCGv val32 = tcg_temp_new();
tcg_gen_extrl_i64_i32(val32, val);
gen_write_p3_0(ctx, val32);
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL 3/7] tests/tcg/hexagon: fix underspecifed asm constraints
2023-01-05 17:38 [PULL 0/7] Hexagon update Taylor Simpson
2023-01-05 17:38 ` [PULL 1/7] linux-user/hexagon: fix signal context save & restore Taylor Simpson
2023-01-05 17:38 ` [PULL 2/7] target/hexagon: rename aliased register HEX_REG_P3_0 Taylor Simpson
@ 2023-01-05 17:38 ` Taylor Simpson
2023-01-05 17:38 ` [PULL 4/7] target/hexagon/idef-parser: fix two typos in README Taylor Simpson
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Taylor Simpson @ 2023-01-05 17:38 UTC (permalink / raw)
To: qemu-devel
Cc: tsimpson, richard.henderson, philmd, peter.maydell, bcain,
quic_mathbern, stefanha, Mukilan Thiyagarajan
From: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com>
There are two test cases where the inline asm doesn't
have the correct constraints causing them to fail.
In misc.c, the 'result' output needs the early clobber
modifier since the rest of the inputs are read after
assignment to the output register.
In mem_noshuf.c, the register r7 is written to but
not specified in the clobber list.
Signed-off-by: Mukilan Thiyagarajan <quic_mthiyaga@quicinc.com>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
Message-Id: <20221229081836.12130-1-quic_mthiyaga@quicinc.com>
---
tests/tcg/hexagon/mem_noshuf.c | 2 +-
tests/tcg/hexagon/misc.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/tcg/hexagon/mem_noshuf.c b/tests/tcg/hexagon/mem_noshuf.c
index 0f4064e700..210b2f1208 100644
--- a/tests/tcg/hexagon/mem_noshuf.c
+++ b/tests/tcg/hexagon/mem_noshuf.c
@@ -144,7 +144,7 @@ static inline long long pred_ld_sd_pi(int pred, long long *p, long long *q,
"}:mem_noshuf\n"
: "=&r"(ret)
: "r"(p), "r"(q), "r"(x), "r"(y), "r"(pred)
- : "p0", "memory");
+ : "r7", "p0", "memory");
return ret;
}
diff --git a/tests/tcg/hexagon/misc.c b/tests/tcg/hexagon/misc.c
index f0b1947fb3..e73ab57334 100644
--- a/tests/tcg/hexagon/misc.c
+++ b/tests/tcg/hexagon/misc.c
@@ -186,10 +186,10 @@ static int L2_ploadrifnew_pi(void *p, int pred)
int result;
asm volatile("%0 = #31\n\t"
"{\n\t"
- " p0 = cmp.eq(%1, #1)\n\t"
- " if (!p0.new) %0 = memw(%2++#4)\n\t"
+ " p0 = cmp.eq(%2, #1)\n\t"
+ " if (!p0.new) %0 = memw(%1++#4)\n\t"
"}\n\t"
- : "=r"(result) : "r"(pred), "r"(p)
+ : "=&r"(result), "+r"(p) : "r"(pred)
: "p0");
return result;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL 4/7] target/hexagon/idef-parser: fix two typos in README
2023-01-05 17:38 [PULL 0/7] Hexagon update Taylor Simpson
` (2 preceding siblings ...)
2023-01-05 17:38 ` [PULL 3/7] tests/tcg/hexagon: fix underspecifed asm constraints Taylor Simpson
@ 2023-01-05 17:38 ` Taylor Simpson
2023-01-05 17:38 ` [PULL 5/7] target/hexagon: suppress unused variable warning Taylor Simpson
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Taylor Simpson @ 2023-01-05 17:38 UTC (permalink / raw)
To: qemu-devel
Cc: tsimpson, richard.henderson, philmd, peter.maydell, bcain,
quic_mathbern, stefanha
From: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Alessandro Di Federico <ale@rev.ng>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <fe67371e03f9dde38eb7554937da0a96a230730e.1672174122.git.quic_mathbern@quicinc.com>
---
target/hexagon/idef-parser/README.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/hexagon/idef-parser/README.rst b/target/hexagon/idef-parser/README.rst
index 65e6bf4ee5..ff6d14150a 100644
--- a/target/hexagon/idef-parser/README.rst
+++ b/target/hexagon/idef-parser/README.rst
@@ -552,11 +552,11 @@ to compare our buggy CPU state against, running
::
- meson configure -Dhexagon_idef_parser_enabled=false
+ meson configure -Dhexagon_idef_parser=false
will disable the idef-parser for all instructions and fallback on manual
tinycode generator overrides, or on helper function implementations. Recompiling
-gives us ``qemu-hexagon`` which passes all tests. If ``qemu-heaxgon-buggy`` is
+gives us ``qemu-hexagon`` which passes all tests. If ``qemu-hexagon-buggy`` is
our binary with the incorrect tinycode generators, we can compare the CPU state
between the two versions
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL 5/7] target/hexagon: suppress unused variable warning
2023-01-05 17:38 [PULL 0/7] Hexagon update Taylor Simpson
` (3 preceding siblings ...)
2023-01-05 17:38 ` [PULL 4/7] target/hexagon/idef-parser: fix two typos in README Taylor Simpson
@ 2023-01-05 17:38 ` Taylor Simpson
2023-01-05 17:38 ` [PULL 6/7] Hexagon (target/hexagon) implement mutability mask for GPRs Taylor Simpson
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Taylor Simpson @ 2023-01-05 17:38 UTC (permalink / raw)
To: qemu-devel
Cc: tsimpson, richard.henderson, philmd, peter.maydell, bcain,
quic_mathbern, stefanha, Alessandro Di Federico
From: Alessandro Di Federico <ale@rev.ng>
This patch manually suppresses a warning for an unused variable
(yynerrs) emitted by bison.
This warning has been triggered for the first time by clang 15.
This patch also disables `-Wextra`, which is not usually adopted in
QEMU. However, clang 15 triggers the warning fixed in this patch even in
absence of `-Wextra`.
Signed-off-by: Alessandro Di Federico <ale@rev.ng>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
Tested-by: Taylor Simpson <tsimpson@quicinc.com>
Message-Id: <20221221155327.1504117-1-ale@rev.ng>
---
target/hexagon/idef-parser/idef-parser.y | 2 ++
target/hexagon/meson.build | 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/hexagon/idef-parser/idef-parser.y b/target/hexagon/idef-parser/idef-parser.y
index 8be44a0ad1..c14cb39500 100644
--- a/target/hexagon/idef-parser/idef-parser.y
+++ b/target/hexagon/idef-parser/idef-parser.y
@@ -99,6 +99,8 @@
/* Input file containing the description of each hexagon instruction */
input : instructions
{
+ /* Suppress warning about unused yynerrs */
+ (void) yynerrs;
YYACCEPT;
}
;
diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
index e8f250fcac..c9d31d095c 100644
--- a/target/hexagon/meson.build
+++ b/target/hexagon/meson.build
@@ -197,7 +197,6 @@ if idef_parser_enabled and 'hexagon-linux-user' in target_dirs
idef_parser_dir / 'parser-helpers.c'],
include_directories: ['idef-parser', '../../include/'],
dependencies: [glib_dep],
- c_args: ['-Wextra'],
native: true
)
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL 6/7] Hexagon (target/hexagon) implement mutability mask for GPRs
2023-01-05 17:38 [PULL 0/7] Hexagon update Taylor Simpson
` (4 preceding siblings ...)
2023-01-05 17:38 ` [PULL 5/7] target/hexagon: suppress unused variable warning Taylor Simpson
@ 2023-01-05 17:38 ` Taylor Simpson
2023-01-05 17:38 ` [PULL 7/7] Update scripts/meson-buildoptions.sh Taylor Simpson
2023-01-06 13:34 ` [PULL 0/7] Hexagon update Peter Maydell
7 siblings, 0 replies; 9+ messages in thread
From: Taylor Simpson @ 2023-01-05 17:38 UTC (permalink / raw)
To: qemu-devel
Cc: tsimpson, richard.henderson, philmd, peter.maydell, bcain,
quic_mathbern, stefanha, Marco Liebel
From: Marco Liebel <quic_mliebel@quicinc.com>
Some registers are defined to have immutable bits, this commit
will implement that behavior.
Signed-off-by: Marco Liebel <quic_mliebel@quicinc.com>
Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Message-Id: <20230105102349.2181856-1-quic_mliebel@quicinc.com>
---
target/hexagon/genptr.c | 44 ++++++++-
tests/tcg/hexagon/reg_mut.c | 152 ++++++++++++++++++++++++++++++
tests/tcg/hexagon/Makefile.target | 1 +
3 files changed, 195 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/hexagon/reg_mut.c
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 66a968c884..90db99024f 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -43,6 +43,33 @@ TCGv gen_read_preg(TCGv pred, uint8_t num)
return pred;
}
+#define IMMUTABLE (~0)
+
+static const target_ulong reg_immut_masks[TOTAL_PER_THREAD_REGS] = {
+ [HEX_REG_USR] = 0xc13000c0,
+ [HEX_REG_PC] = IMMUTABLE,
+ [HEX_REG_GP] = 0x3f,
+ [HEX_REG_UPCYCLELO] = IMMUTABLE,
+ [HEX_REG_UPCYCLEHI] = IMMUTABLE,
+ [HEX_REG_UTIMERLO] = IMMUTABLE,
+ [HEX_REG_UTIMERHI] = IMMUTABLE,
+};
+
+static inline void gen_masked_reg_write(TCGv new_val, TCGv cur_val,
+ target_ulong reg_mask)
+{
+ if (reg_mask) {
+ TCGv tmp = tcg_temp_new();
+
+ /* new_val = (new_val & ~reg_mask) | (cur_val & reg_mask) */
+ tcg_gen_andi_tl(new_val, new_val, ~reg_mask);
+ tcg_gen_andi_tl(tmp, cur_val, reg_mask);
+ tcg_gen_or_tl(new_val, new_val, tmp);
+
+ tcg_temp_free(tmp);
+ }
+}
+
static inline void gen_log_predicated_reg_write(int rnum, TCGv val,
uint32_t slot)
{
@@ -69,6 +96,9 @@ static inline void gen_log_predicated_reg_write(int rnum, TCGv val,
void gen_log_reg_write(int rnum, TCGv val)
{
+ const target_ulong reg_mask = reg_immut_masks[rnum];
+
+ gen_masked_reg_write(val, hex_gpr[rnum], reg_mask);
tcg_gen_mov_tl(hex_new_value[rnum], val);
if (HEX_DEBUG) {
/* Do this so HELPER(debug_commit_end) will know */
@@ -114,19 +144,29 @@ static void gen_log_predicated_reg_write_pair(int rnum, TCGv_i64 val,
static void gen_log_reg_write_pair(int rnum, TCGv_i64 val)
{
+ const target_ulong reg_mask_low = reg_immut_masks[rnum];
+ const target_ulong reg_mask_high = reg_immut_masks[rnum + 1];
+ TCGv val32 = tcg_temp_new();
+
/* Low word */
- tcg_gen_extrl_i64_i32(hex_new_value[rnum], val);
+ tcg_gen_extrl_i64_i32(val32, val);
+ gen_masked_reg_write(val32, hex_gpr[rnum], reg_mask_low);
+ tcg_gen_mov_tl(hex_new_value[rnum], val32);
if (HEX_DEBUG) {
/* Do this so HELPER(debug_commit_end) will know */
tcg_gen_movi_tl(hex_reg_written[rnum], 1);
}
/* High word */
- tcg_gen_extrh_i64_i32(hex_new_value[rnum + 1], val);
+ tcg_gen_extrh_i64_i32(val32, val);
+ gen_masked_reg_write(val32, hex_gpr[rnum + 1], reg_mask_high);
+ tcg_gen_mov_tl(hex_new_value[rnum + 1], val32);
if (HEX_DEBUG) {
/* Do this so HELPER(debug_commit_end) will know */
tcg_gen_movi_tl(hex_reg_written[rnum + 1], 1);
}
+
+ tcg_temp_free(val32);
}
void gen_log_pred_write(DisasContext *ctx, int pnum, TCGv val)
diff --git a/tests/tcg/hexagon/reg_mut.c b/tests/tcg/hexagon/reg_mut.c
new file mode 100644
index 0000000000..910e663ace
--- /dev/null
+++ b/tests/tcg/hexagon/reg_mut.c
@@ -0,0 +1,152 @@
+
+/*
+ * Copyright(c) 2022 Qualcomm Innovation Center, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdio.h>
+#include <stdint.h>
+
+static int err;
+
+#define check(N, EXPECT) \
+ do { \
+ uint64_t value = N; \
+ uint64_t expect = EXPECT; \
+ if (value != EXPECT) { \
+ printf("ERROR: \"%s\" 0x%04llx != 0x%04llx at %s:%d\n", #N, value, \
+ expect, __FILE__, __LINE__); \
+ err++; \
+ } \
+ } while (0)
+
+#define check_ne(N, EXPECT) \
+ do { \
+ uint64_t value = N; \
+ uint64_t expect = EXPECT; \
+ if (value == EXPECT) { \
+ printf("ERROR: \"%s\" 0x%04llx == 0x%04llx at %s:%d\n", #N, value, \
+ expect, __FILE__, __LINE__); \
+ err++; \
+ } \
+ } while (0)
+
+#define WRITE_REG_NOCLOBBER(output, reg_name, input) \
+ asm volatile(reg_name " = %1\n\t" \
+ "%0 = " reg_name "\n\t" \
+ : "=r"(output) \
+ : "r"(input) \
+ : );
+
+#define WRITE_REG_ENCODED(output, reg_name, input, encoding) \
+ asm volatile("r0 = %1\n\t" \
+ encoding "\n\t" \
+ "%0 = " reg_name "\n\t" \
+ : "=r"(output) \
+ : "r"(input) \
+ : "r0");
+
+#define WRITE_REG_PAIR_ENCODED(output, reg_name, input, encoding) \
+ asm volatile("r1:0 = %1\n\t" \
+ encoding "\n\t" \
+ "%0 = " reg_name "\n\t" \
+ : "=r"(output) \
+ : "r"(input) \
+ : "r1:0");
+
+/*
+ * Instruction word: { pc = r0 }
+ *
+ * This instruction is barred by the assembler.
+ *
+ * 3 2 1
+ * 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0 9 8 7 6 5 4 3 2 1 0
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | Opc[A2_tfrrcr] | Src[R0] |P P| | C9/PC |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ */
+#define PC_EQ_R0 ".word 0x6220c009"
+#define C9_8_EQ_R1_0 ".word 0x6320c008"
+
+static inline void write_control_registers(void)
+{
+ uint32_t result = 0;
+
+ WRITE_REG_NOCLOBBER(result, "usr", 0xffffffff);
+ check(result, 0x3ecfff3f);
+
+ WRITE_REG_NOCLOBBER(result, "gp", 0xffffffff);
+ check(result, 0xffffffc0);
+
+ WRITE_REG_NOCLOBBER(result, "upcyclelo", 0xffffffff);
+ check(result, 0x00000000);
+
+ WRITE_REG_NOCLOBBER(result, "upcyclehi", 0xffffffff);
+ check(result, 0x00000000);
+
+ WRITE_REG_NOCLOBBER(result, "utimerlo", 0xffffffff);
+ check(result, 0x00000000);
+
+ WRITE_REG_NOCLOBBER(result, "utimerhi", 0xffffffff);
+ check(result, 0x00000000);
+
+ /*
+ * PC is special. Setting it to these values
+ * should cause a catastrophic failure.
+ */
+ WRITE_REG_ENCODED(result, "pc", 0x00000000, PC_EQ_R0);
+ check_ne(result, 0x00000000);
+
+ WRITE_REG_ENCODED(result, "pc", 0x00000001, PC_EQ_R0);
+ check_ne(result, 0x00000001);
+
+ WRITE_REG_ENCODED(result, "pc", 0xffffffff, PC_EQ_R0);
+ check_ne(result, 0xffffffff);
+}
+
+static inline void write_control_register_pairs(void)
+{
+ uint64_t result = 0;
+
+ WRITE_REG_NOCLOBBER(result, "c11:10", 0xffffffffffffffff);
+ check(result, 0xffffffc0ffffffff);
+
+ WRITE_REG_NOCLOBBER(result, "c15:14", 0xffffffffffffffff);
+ check(result, 0x0000000000000000);
+
+ WRITE_REG_NOCLOBBER(result, "c31:30", 0xffffffffffffffff);
+ check(result, 0x0000000000000000);
+
+ WRITE_REG_PAIR_ENCODED(result, "c9:8", (uint64_t) 0x0000000000000000,
+ C9_8_EQ_R1_0);
+ check_ne(result, 0x000000000000000);
+
+ WRITE_REG_PAIR_ENCODED(result, "c9:8", 0x0000000100000000, C9_8_EQ_R1_0);
+ check_ne(result, 0x0000000100000000);
+
+ WRITE_REG_PAIR_ENCODED(result, "c9:8", 0xffffffffffffffff, C9_8_EQ_R1_0);
+ check_ne(result, 0xffffffffffffffff);
+}
+
+int main()
+{
+ err = 0;
+
+ write_control_registers();
+ write_control_register_pairs();
+
+ puts(err ? "FAIL" : "PASS");
+ return err;
+}
diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target
index f1378d86a9..18e6a5969e 100644
--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -44,6 +44,7 @@ HEX_TESTS += atomics
HEX_TESTS += fpstuff
HEX_TESTS += overflow
HEX_TESTS += signal_context
+HEX_TESTS += reg_mut
HEX_TESTS += test_abs
HEX_TESTS += test_bitcnt
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PULL 7/7] Update scripts/meson-buildoptions.sh
2023-01-05 17:38 [PULL 0/7] Hexagon update Taylor Simpson
` (5 preceding siblings ...)
2023-01-05 17:38 ` [PULL 6/7] Hexagon (target/hexagon) implement mutability mask for GPRs Taylor Simpson
@ 2023-01-05 17:38 ` Taylor Simpson
2023-01-06 13:34 ` [PULL 0/7] Hexagon update Peter Maydell
7 siblings, 0 replies; 9+ messages in thread
From: Taylor Simpson @ 2023-01-05 17:38 UTC (permalink / raw)
To: qemu-devel
Cc: tsimpson, richard.henderson, philmd, peter.maydell, bcain,
quic_mathbern, stefanha, Alessandro Di Federico
From: Alessandro Di Federico <ale@rev.ng>
Note: `Makefile` relies on modification dates in the source tree to
detect changes to `meson_options.txt`. However, git does not track
those. Therefore, the following was necessary to regenerate
`meson-buildoptions.sh`:
touch meson_options.txt
cd "$BUILD_DIR"
make update-buildoptions
Signed-off-by: Alessandro Di Federico <ale@rev.ng>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
Message-Id: <20230102104113.3438895-1-ale@rev.ng>
---
scripts/meson-buildoptions.sh | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index aa6e30ea91..0f71e92dcb 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -10,6 +10,9 @@ meson_options_help() {
printf "%s\n" ' affects only QEMU, not tools like qemu-img)'
printf "%s\n" ' --datadir=VALUE Data file directory [share]'
printf "%s\n" ' --disable-coroutine-pool coroutine freelist (better performance)'
+ printf "%s\n" ' --disable-hexagon-idef-parser'
+ printf "%s\n" ' use idef-parser to automatically generate TCG'
+ printf "%s\n" ' code for the Hexagon frontend'
printf "%s\n" ' --disable-install-blobs install provided firmware blobs'
printf "%s\n" ' --docdir=VALUE Base directory for documentation installation'
printf "%s\n" ' (can be empty) [share/doc]'
@@ -40,7 +43,8 @@ meson_options_help() {
printf "%s\n" ' --enable-trace-backends=CHOICES'
printf "%s\n" ' Set available tracing backends [log] (choices:'
printf "%s\n" ' dtrace/ftrace/log/nop/simple/syslog/ust)'
- printf "%s\n" ' --firmwarepath=VALUES search PATH for firmware files [share/qemu-firmware]'
+ printf "%s\n" ' --firmwarepath=VALUES search PATH for firmware files [share/qemu-'
+ printf "%s\n" ' firmware]'
printf "%s\n" ' --iasl=VALUE Path to ACPI disassembler'
printf "%s\n" ' --includedir=VALUE Header file directory [include]'
printf "%s\n" ' --interp-prefix=VALUE where to find shared libraries etc., use %M for'
@@ -93,7 +97,7 @@ meson_options_help() {
printf "%s\n" ' glusterfs Glusterfs block device driver'
printf "%s\n" ' gnutls GNUTLS cryptography support'
printf "%s\n" ' gtk GTK+ user interface'
- printf "%s\n" ' gtk-clipboard clipboard support for GTK (EXPERIMENTAL, MAY HANG)'
+ printf "%s\n" ' gtk-clipboard clipboard support for the gtk UI (EXPERIMENTAL, MAY HANG)'
printf "%s\n" ' guest-agent Build QEMU Guest Agent'
printf "%s\n" ' guest-agent-msi Build MSI package for the QEMU Guest Agent'
printf "%s\n" ' hax HAX acceleration support'
@@ -156,6 +160,8 @@ meson_options_help() {
printf "%s\n" ' usb-redir libusbredir support'
printf "%s\n" ' vde vde network backend support'
printf "%s\n" ' vdi vdi image format support'
+ printf "%s\n" ' vduse-blk-export'
+ printf "%s\n" ' VDUSE block export support'
printf "%s\n" ' vfio-user-server'
printf "%s\n" ' vfio-user server support'
printf "%s\n" ' vhost-crypto vhost-user crypto backend support'
@@ -164,8 +170,6 @@ meson_options_help() {
printf "%s\n" ' vhost-user vhost-user backend support'
printf "%s\n" ' vhost-user-blk-server'
printf "%s\n" ' build vhost-user-blk server'
- printf "%s\n" ' vduse-blk-export'
- printf "%s\n" ' VDUSE block export support'
printf "%s\n" ' vhost-vdpa vhost-vdpa kernel backend support'
printf "%s\n" ' virglrenderer virgl rendering support'
printf "%s\n" ' virtfs virtio-9p support'
@@ -283,6 +287,8 @@ _meson_option_parse() {
--disable-guest-agent-msi) printf "%s" -Dguest_agent_msi=disabled ;;
--enable-hax) printf "%s" -Dhax=enabled ;;
--disable-hax) printf "%s" -Dhax=disabled ;;
+ --enable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=true ;;
+ --disable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=false ;;
--enable-hvf) printf "%s" -Dhvf=enabled ;;
--disable-hvf) printf "%s" -Dhvf=disabled ;;
--iasl=*) quote_sh "-Diasl=$2" ;;
@@ -429,6 +435,8 @@ _meson_option_parse() {
--disable-vde) printf "%s" -Dvde=disabled ;;
--enable-vdi) printf "%s" -Dvdi=enabled ;;
--disable-vdi) printf "%s" -Dvdi=disabled ;;
+ --enable-vduse-blk-export) printf "%s" -Dvduse_blk_export=enabled ;;
+ --disable-vduse-blk-export) printf "%s" -Dvduse_blk_export=disabled ;;
--enable-vfio-user-server) printf "%s" -Dvfio_user_server=enabled ;;
--disable-vfio-user-server) printf "%s" -Dvfio_user_server=disabled ;;
--enable-vhost-crypto) printf "%s" -Dvhost_crypto=enabled ;;
@@ -441,8 +449,6 @@ _meson_option_parse() {
--disable-vhost-user) printf "%s" -Dvhost_user=disabled ;;
--enable-vhost-user-blk-server) printf "%s" -Dvhost_user_blk_server=enabled ;;
--disable-vhost-user-blk-server) printf "%s" -Dvhost_user_blk_server=disabled ;;
- --enable-vduse-blk-export) printf "%s" -Dvduse_blk_export=enabled ;;
- --disable-vduse-blk-export) printf "%s" -Dvduse_blk_export=disabled ;;
--enable-vhost-vdpa) printf "%s" -Dvhost_vdpa=enabled ;;
--disable-vhost-vdpa) printf "%s" -Dvhost_vdpa=disabled ;;
--enable-virglrenderer) printf "%s" -Dvirglrenderer=enabled ;;
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PULL 0/7] Hexagon update
2023-01-05 17:38 [PULL 0/7] Hexagon update Taylor Simpson
` (6 preceding siblings ...)
2023-01-05 17:38 ` [PULL 7/7] Update scripts/meson-buildoptions.sh Taylor Simpson
@ 2023-01-06 13:34 ` Peter Maydell
7 siblings, 0 replies; 9+ messages in thread
From: Peter Maydell @ 2023-01-06 13:34 UTC (permalink / raw)
To: Taylor Simpson
Cc: qemu-devel, richard.henderson, philmd, bcain, quic_mathbern,
stefanha
On Thu, 5 Jan 2023 at 17:38, Taylor Simpson <tsimpson@quicinc.com> wrote:
>
> The following changes since commit cb9c6a8e5ad6a1f0ce164d352e3102df46986e22:
>
> .gitlab-ci.d/windows: Work-around timeout and OpenGL problems of the MSYS2 jobs (2023-01-04 18:58:33 +0000)
>
> are available in the Git repository at:
>
> https://github.com/quic/qemu tags/pull-hex-20230105
>
> for you to fetch changes up to dc63b1492c2d8140d3b47093700bb9bb52c0d97b:
>
> Update scripts/meson-buildoptions.sh (2023-01-05 09:19:02 -0800)
>
> ----------------------------------------------------------------
> Hexagon update: patches from several folks
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 9+ messages in thread