target/hexagon/genptr.c | 43 ++++- tests/tcg/hexagon/Makefile.target | 3 + tests/tcg/hexagon/reg_mut.c | 307 ++++++++++++++++++++++++++++++ 3 files changed, 351 insertions(+), 2 deletions(-) create mode 100644 tests/tcg/hexagon/reg_mut.c
Some registers are defined to have immutable bits, this commit
will implement that behavior.
Signed-off-by: Marco Liebel <quic_mliebel@quicinc.com>
---
This incorporates the feedback given on Brian's patch
target/hexagon/genptr.c | 43 ++++-
tests/tcg/hexagon/Makefile.target | 3 +
tests/tcg/hexagon/reg_mut.c | 307 ++++++++++++++++++++++++++++++
3 files changed, 351 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 806d0974ff..a91db8c76d 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -30,6 +30,32 @@
#include "gen_tcg.h"
#include "gen_tcg_hvx.h"
+static const target_ulong reg_immut_masks[TOTAL_PER_THREAD_REGS] = {
+ [HEX_REG_USR] = 0xc13000c0,
+ [HEX_REG_PC] = UINT32_MAX,
+ [HEX_REG_GP] = 0x3f,
+ [HEX_REG_UPCYCLELO] = UINT32_MAX,
+ [HEX_REG_UPCYCLEHI] = UINT32_MAX,
+ [HEX_REG_UTIMERLO] = UINT32_MAX,
+ [HEX_REG_UTIMERHI] = UINT32_MAX,
+};
+
+static inline void gen_masked_reg_write(TCGv result, TCGv new_val, TCGv cur_val,
+ target_ulong reg_mask)
+{
+ if (reg_mask) {
+ TCGv tmp = tcg_temp_new();
+
+ /* out_val = (in_val & reg_mask) | (cur_val & ~reg_mask) */
+ /* result is used to avoid creating a second temporary variable */
+ tcg_gen_andi_tl(result, new_val, ~reg_mask);
+ tcg_gen_andi_tl(tmp, cur_val, reg_mask);
+ tcg_gen_or_tl(result, result, tmp);
+
+ tcg_temp_free(tmp);
+ }
+}
+
static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int slot)
{
TCGv zero = tcg_constant_tl(0);
@@ -55,6 +81,9 @@ static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int slot)
static inline void gen_log_reg_write(int rnum, TCGv val)
{
+ const target_ulong reg_mask = reg_immut_masks[rnum];
+
+ gen_masked_reg_write(val, 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 */
@@ -99,19 +128,29 @@ static void gen_log_predicated_reg_write_pair(int rnum, TCGv_i64 val, int slot)
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, 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, 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);
}
static inline void gen_log_pred_write(DisasContext *ctx, int pnum, TCGv val)
diff --git a/tests/tcg/hexagon/Makefile.target b/tests/tcg/hexagon/Makefile.target
index 96a4d7a614..4e9a20960b 100644
--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -43,9 +43,12 @@ HEX_TESTS += load_align
HEX_TESTS += atomics
HEX_TESTS += fpstuff
HEX_TESTS += overflow
+HEX_TESTS += reg_mut
TESTS += $(HEX_TESTS)
+reg_mut: CFLAGS += -I$(SRC_PATH)/target/hexagon/
+
# This test has to be compiled for the -mv67t target
usr: usr.c
$(CC) $(CFLAGS) -mv67t -O2 -Wno-inline-asm -Wno-expansion-to-defined $< -o $@ $(LDFLAGS)
diff --git a/tests/tcg/hexagon/reg_mut.c b/tests/tcg/hexagon/reg_mut.c
new file mode 100644
index 0000000000..8bbc1559dd
--- /dev/null
+++ b/tests/tcg/hexagon/reg_mut.c
@@ -0,0 +1,307 @@
+
+/*
+ * 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>
+
+#include "hex_regs.h"
+
+static int err;
+
+enum {
+ HEX_REG_PAIR_C9_8,
+ HEX_REG_PAIR_C11_10,
+ HEX_REG_PAIR_C15_14,
+ HEX_REG_PAIR_C31_30,
+};
+
+#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(reg_name, output, input) \
+ asm volatile(reg_name " = %1\n\t" \
+ "%0 = " reg_name "\n\t" \
+ : "=r"(output) \
+ : "r"(input) \
+ : reg_name);
+
+#define WRITE_REG_IN_PACKET(reg_name, output, input) \
+ asm volatile("{ " reg_name " = %1 }\n\t" \
+ "%0 = " reg_name "\n\t" \
+ : "=r"(output) \
+ : "r"(input) \
+ : reg_name);
+
+#define WRITE_REG_ENCODED(reg_name, encoding, output, input) \
+ asm volatile("r0 = %1\n\t" \
+ encoding \
+ "%0 = " reg_name "\n\t" \
+ : "=r"(output) \
+ : "r"(input) \
+ : "r0");
+
+#define WRITE_REG_ENCODED_IN_PACKET(reg_name, encoding, output, input) \
+ asm volatile("{ r0 = %1 }\n\t" \
+ encoding \
+ "%0 = " reg_name "\n\t" \
+ : "=r"(output) \
+ : "r"(input) \
+ : "r0");
+
+#define WRITE_REG_PAIR_ENCODED(reg_name, encoding, output, input) \
+ asm volatile("r1:0 = %1\n\t" \
+ encoding \
+ "%0 = " reg_name "\n\t" \
+ : "=r"(output) \
+ : "r"(input) \
+ : "r1:0");
+
+#define WRITE_REG_PAIR_ENCODED_IN_PACKET(reg_name, encoding, output, input) \
+ asm volatile("{ r1:0 = %1 }\n\t" \
+ encoding \
+ "%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\n\t"
+#define GP_EQ_R0 ".word 0x6220c00b\n\t"
+#define UPCYCLELO_EQ_R0 ".word 0x6220c00e\n\t"
+#define UPCYCLEHI_EQ_R0 ".word 0x6220c00f\n\t"
+#define UTIMERLO_EQ_R0 ".word 0x6220c01e\n\t"
+#define UTIMERHI_EQ_R0 ".word 0x6220c01f\n\t"
+
+#define C9_8_EQ_R1_0 ".word 0x6320c008\n\t"
+#define C11_10_EQ_R1_0 ".word 0x6320c00a\n\t"
+#define C15_14_EQ_R1_0 ".word 0x6320c00e\n\t"
+#define C31_30_EQ_R1_0 ".word 0x6320c01e\n\t"
+
+static inline uint32_t write_reg(int rnum, uint32_t val)
+{
+ uint32_t result;
+ switch (rnum) {
+ case HEX_REG_USR:
+ WRITE_REG("usr", result, val);
+ break;
+ case HEX_REG_PC:
+ WRITE_REG_ENCODED("pc", PC_EQ_R0, result, val);
+ break;
+ case HEX_REG_GP:
+ WRITE_REG_ENCODED("gp", GP_EQ_R0, result, val);
+ break;
+ case HEX_REG_UPCYCLELO:
+ WRITE_REG_ENCODED("upcyclelo", UPCYCLELO_EQ_R0, result, val);
+ break;
+ case HEX_REG_UPCYCLEHI:
+ WRITE_REG_ENCODED("upcyclehi", UPCYCLEHI_EQ_R0, result, val);
+ break;
+ case HEX_REG_UTIMERLO:
+ WRITE_REG_ENCODED("utimerlo", UTIMERLO_EQ_R0, result, val);
+ break;
+ case HEX_REG_UTIMERHI:
+ WRITE_REG_ENCODED("utimerhi", UTIMERHI_EQ_R0, result, val);
+ break;
+ }
+ return result;
+}
+
+static inline uint32_t write_reg_in_packet(int rnum, uint32_t val)
+{
+ uint32_t result;
+ switch (rnum) {
+ case HEX_REG_USR:
+ WRITE_REG_IN_PACKET("usr", result, val);
+ break;
+ case HEX_REG_PC:
+ WRITE_REG_ENCODED_IN_PACKET("pc", PC_EQ_R0, result, val);
+ break;
+ case HEX_REG_GP:
+ WRITE_REG_ENCODED_IN_PACKET("gp", GP_EQ_R0, result, val);
+ break;
+ case HEX_REG_UPCYCLELO:
+ WRITE_REG_ENCODED_IN_PACKET("upcyclelo", UPCYCLELO_EQ_R0, result, val);
+ break;
+ case HEX_REG_UPCYCLEHI:
+ WRITE_REG_ENCODED_IN_PACKET("upcyclehi", UPCYCLEHI_EQ_R0, result, val);
+ break;
+ case HEX_REG_UTIMERLO:
+ WRITE_REG_ENCODED_IN_PACKET("utimerlo", UTIMERLO_EQ_R0, result, val);
+ break;
+ case HEX_REG_UTIMERHI:
+ WRITE_REG_ENCODED_IN_PACKET("utimerhi", UTIMERHI_EQ_R0, result, val);
+ break;
+ }
+ return result;
+}
+
+static inline uint64_t write_reg_pair(int rnum, uint32_t val_hi,
+ uint32_t val_lo)
+{
+ uint64_t val = (uint64_t) val_hi << 32 | val_lo;
+ uint64_t result;
+ switch (rnum) {
+ case HEX_REG_PAIR_C9_8:
+ WRITE_REG_PAIR_ENCODED("c9:8", C9_8_EQ_R1_0, result, val);
+ break;
+ case HEX_REG_PAIR_C11_10:
+ WRITE_REG_PAIR_ENCODED("c11:10", C11_10_EQ_R1_0, result, val);
+ break;
+ case HEX_REG_PAIR_C15_14:
+ WRITE_REG_PAIR_ENCODED("c15:14", C15_14_EQ_R1_0, result, val);
+ break;
+ case HEX_REG_PAIR_C31_30:
+ WRITE_REG_PAIR_ENCODED("c31:30", C31_30_EQ_R1_0, result, val);
+ break;
+ }
+ return result;
+}
+
+static inline uint64_t write_reg_pair_in_packet(int rnum, uint32_t val_hi,
+ uint32_t val_lo)
+{
+ uint64_t val = (uint64_t) val_hi << 32 | val_lo;
+ uint64_t result;
+ switch (rnum) {
+ case HEX_REG_PAIR_C9_8:
+ WRITE_REG_PAIR_ENCODED_IN_PACKET("c9:8", C9_8_EQ_R1_0, result, val);
+ break;
+ case HEX_REG_PAIR_C11_10:
+ WRITE_REG_PAIR_ENCODED_IN_PACKET("c11:10", C11_10_EQ_R1_0, result, val);
+ break;
+ case HEX_REG_PAIR_C15_14:
+ WRITE_REG_PAIR_ENCODED_IN_PACKET("c15:14", C15_14_EQ_R1_0, result, val);
+ break;
+ case HEX_REG_PAIR_C31_30:
+ WRITE_REG_PAIR_ENCODED_IN_PACKET("c31:30", C31_30_EQ_R1_0, result, val);
+ break;
+ }
+ return result;
+}
+
+static inline void write_control_registers(void)
+{
+ check(write_reg(HEX_REG_USR, 0xffffffff), 0x3ecfff3f);
+ check(write_reg(HEX_REG_GP, 0xffffffff), 0xffffffc0);
+ check(write_reg(HEX_REG_UPCYCLELO, 0xffffffff), 0x0);
+ check(write_reg(HEX_REG_UPCYCLEHI, 0xffffffff), 0x0);
+ check(write_reg(HEX_REG_UTIMERLO, 0xffffffff), 0x0);
+ check(write_reg(HEX_REG_UTIMERHI, 0xffffffff), 0x0);
+
+ /*
+ * PC is special. Setting it to these values
+ * should cause a catastrophic failure.
+ */
+ check_ne(write_reg(HEX_REG_PC, 0x00000000), 0x00000000);
+ check_ne(write_reg(HEX_REG_PC, 0x00000000), 0x00000001);
+ check_ne(write_reg(HEX_REG_PC, 0xffffffff), 0xffffffff);
+ check_ne(write_reg(HEX_REG_PC, 0x00000000), 0x00000000);
+}
+
+static inline void write_control_registers_in_packets(void)
+{
+ check(write_reg_in_packet(HEX_REG_USR, 0xffffffff), 0x3ecfff3f);
+ check(write_reg_in_packet(HEX_REG_GP, 0xffffffff), 0xffffffc0);
+ check(write_reg_in_packet(HEX_REG_UPCYCLELO, 0xffffffff), 0x0);
+ check(write_reg_in_packet(HEX_REG_UPCYCLEHI, 0xffffffff), 0x0);
+ check(write_reg_in_packet(HEX_REG_UTIMERLO, 0xffffffff), 0x0);
+ check(write_reg_in_packet(HEX_REG_UTIMERHI, 0xffffffff), 0x0);
+
+ check_ne(write_reg_in_packet(HEX_REG_PC, 0x00000000), 0x00000000);
+ check_ne(write_reg_in_packet(HEX_REG_PC, 0x00000001), 0x00000001);
+ check_ne(write_reg_in_packet(HEX_REG_PC, 0xffffffff), 0xffffffff);
+ check_ne(write_reg_in_packet(HEX_REG_PC, 0x00000000), 0x00000000);
+}
+
+static inline void write_control_register_pairs(void)
+{
+ check(write_reg_pair(HEX_REG_PAIR_C11_10, 0xffffffff, 0xffffffff),
+ 0xffffffc0ffffffff);
+ check(write_reg_pair(HEX_REG_PAIR_C15_14, 0xffffffff, 0xffffffff), 0x0);
+ check(write_reg_pair(HEX_REG_PAIR_C31_30, 0xffffffff, 0xffffffff), 0x0);
+
+ check_ne(write_reg_pair(HEX_REG_PAIR_C9_8, 0x00000000, 0x00000000),
+ 0x0000000000000000);
+ check_ne(write_reg_pair(HEX_REG_PAIR_C9_8, 0x00000001, 0x00000000),
+ 0x0000000100000000);
+ check_ne(write_reg_pair(HEX_REG_PAIR_C9_8, 0xffffffff, 0xffffffff),
+ 0xffffffffffffffff);
+ check_ne(write_reg_pair(HEX_REG_PAIR_C9_8, 0x00000000, 0x00000000),
+ 0x0000000000000000);
+}
+
+static inline void write_control_register_pairs_in_packets(void)
+{
+ check(write_reg_pair_in_packet(HEX_REG_PAIR_C11_10, 0xffffffff, 0xffffffff),
+ 0xffffffc0ffffffff);
+ check(write_reg_pair_in_packet(HEX_REG_PAIR_C15_14, 0xffffffff, 0xffffffff),
+ 0x0);
+ check(write_reg_pair_in_packet(HEX_REG_PAIR_C31_30, 0xffffffff, 0xffffffff),
+ 0x0);
+
+ check_ne(write_reg_pair_in_packet(HEX_REG_PAIR_C9_8, 0x00000000,
+ 0x00000000), 0x0000000000000000);
+ check_ne(write_reg_pair_in_packet(HEX_REG_PAIR_C9_8, 0x00000001,
+ 0x00000000), 0x0000000100000000);
+ check_ne(write_reg_pair_in_packet(HEX_REG_PAIR_C9_8, 0xffffffff,
+ 0xffffffff), 0xffffffffffffffff);
+ check_ne(write_reg_pair_in_packet(HEX_REG_PAIR_C9_8, 0x00000000,
+ 0x00000000), 0x0000000000000000);
+}
+
+int main()
+{
+ err = 0;
+
+ write_control_registers();
+ write_control_registers_in_packets();
+ write_control_register_pairs();
+ write_control_register_pairs_in_packets();
+
+ puts(err ? "FAIL" : "PASS");
+ return err;
+}
--
2.25.1
> -----Original Message----- > From: Marco Liebel (QUIC) <quic_mliebel@quicinc.com> > Sent: Friday, December 16, 2022 2:04 PM > To: qemu-devel@nongnu.org > Cc: Brian Cain <bcain@quicinc.com>; Taylor Simpson > <tsimpson@quicinc.com>; Marco Liebel (QUIC) <quic_mliebel@quicinc.com> > Subject: [PATCH v2] Hexagon (target/hexagon) implement mutability mask > for GPRs > > Some registers are defined to have immutable bits, this commit will > implement that behavior. > > Signed-off-by: Marco Liebel <quic_mliebel@quicinc.com> > --- > This incorporates the feedback given on Brian's patch > > target/hexagon/genptr.c | 43 ++++- > tests/tcg/hexagon/Makefile.target | 3 + > tests/tcg/hexagon/reg_mut.c | 307 > ++++++++++++++++++++++++++++++ > 3 files changed, 351 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 > 806d0974ff..a91db8c76d 100644 > --- a/target/hexagon/genptr.c > +++ b/target/hexagon/genptr.c > @@ -30,6 +30,32 @@ > #include "gen_tcg.h" > #include "gen_tcg_hvx.h" > > +static const target_ulong reg_immut_masks[TOTAL_PER_THREAD_REGS] = > { > + [HEX_REG_USR] = 0xc13000c0, > + [HEX_REG_PC] = UINT32_MAX, > + [HEX_REG_GP] = 0x3f, > + [HEX_REG_UPCYCLELO] = UINT32_MAX, > + [HEX_REG_UPCYCLEHI] = UINT32_MAX, > + [HEX_REG_UTIMERLO] = UINT32_MAX, > + [HEX_REG_UTIMERHI] = UINT32_MAX, > +}; UINT_MAX is confusing. Use ~0 instead. #define IMMUTABLE (~0) ... [HEX_REG_PC] = IMMUTABLE ... > + > +static inline void gen_masked_reg_write(TCGv result, TCGv new_val, TCGv > cur_val, > + target_ulong reg_mask) { > + if (reg_mask) { > + TCGv tmp = tcg_temp_new(); > + > + /* out_val = (in_val & reg_mask) | (cur_val & ~reg_mask) */ Comment doesn't match the TCG code below. Should be /* result = (new_val & ~reg_mask) | (cur_val & reg_mask) */ > + /* result is used to avoid creating a second temporary variable */ > + tcg_gen_andi_tl(result, new_val, ~reg_mask); > + tcg_gen_andi_tl(tmp, cur_val, reg_mask); > + tcg_gen_or_tl(result, result, tmp); > + > + tcg_temp_free(tmp); > + } > +} All of the callers of this function pass the same TCGv for result and new_val. So, we are getting lucky that this function doesn't set result when reg_mask is zero. In order to future-proof this function, either eliminate the result parameter or assign to it from new_val when reg_mask is zero. > + > static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int slot) > { > TCGv zero = tcg_constant_tl(0); > diff --git a/tests/tcg/hexagon/Makefile.target > b/tests/tcg/hexagon/Makefile.target > index 96a4d7a614..4e9a20960b 100644 > --- a/tests/tcg/hexagon/Makefile.target > +++ b/tests/tcg/hexagon/Makefile.target > @@ -43,9 +43,12 @@ HEX_TESTS += load_align HEX_TESTS += atomics > HEX_TESTS += fpstuff HEX_TESTS += overflow > +HEX_TESTS += reg_mut > > TESTS += $(HEX_TESTS) > > +reg_mut: CFLAGS += -I$(SRC_PATH)/target/hexagon/ > + You won't need this when you remove hex_regs.h from the test (see below). > # This test has to be compiled for the -mv67t target > usr: usr.c > $(CC) $(CFLAGS) -mv67t -O2 -Wno-inline-asm -Wno-expansion-to- > defined $< -o $@ $(LDFLAGS) diff --git a/tests/tcg/hexagon/reg_mut.c > b/tests/tcg/hexagon/reg_mut.c new file mode 100644 index > 0000000000..8bbc1559dd > --- /dev/null > +++ b/tests/tcg/hexagon/reg_mut.c > @@ -0,0 +1,307 @@ > + > +/* > + * 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> > + > +#include "hex_regs.h" Don't include headers from QEMU itself in the test cases. You don't need this because you are just converting from the enum to a string. Just use the string itself and skip the switch statement. > + > +static int err; > + > +enum { > + HEX_REG_PAIR_C9_8, > + HEX_REG_PAIR_C11_10, > + HEX_REG_PAIR_C15_14, > + HEX_REG_PAIR_C31_30, > +}; Shouldn't need this if you switch to using strings for the register names. > +#define WRITE_REG(reg_name, output, input) \ > + asm volatile(reg_name " = %1\n\t" \ > + "%0 = " reg_name "\n\t" \ > + : "=r"(output) \ > + : "r"(input) \ > + : reg_name); > + > +#define WRITE_REG_IN_PACKET(reg_name, output, input) \ > + asm volatile("{ " reg_name " = %1 }\n\t" \ This is no different from the WRITE_REG above. Instructions on a line with no curly braces are a single packet. > + "%0 = " reg_name "\n\t" \ > + : "=r"(output) \ > + : "r"(input) \ > + : reg_name); > + > +#define WRITE_REG_ENCODED(reg_name, encoding, output, input) \ > + asm volatile("r0 = %1\n\t" \ > + encoding \ > + "%0 = " reg_name "\n\t" \ > + : "=r"(output) \ > + : "r"(input) \ > + : "r0"); > + > +#define WRITE_REG_ENCODED_IN_PACKET(reg_name, encoding, output, > input) \ > + asm volatile("{ r0 = %1 }\n\t" \ This is also the same as WRITE_REG_ENCODED. > + encoding \ > + "%0 = " reg_name "\n\t" \ > + : "=r"(output) \ > + : "r"(input) \ > + : "r0"); > + > +#define WRITE_REG_PAIR_ENCODED(reg_name, encoding, output, input) \ > + asm volatile("r1:0 = %1\n\t" \ > + encoding \ > + "%0 = " reg_name "\n\t" \ > + : "=r"(output) \ > + : "r"(input) \ > + : "r1:0"); > + > +#define WRITE_REG_PAIR_ENCODED_IN_PACKET(reg_name, encoding, > output, input) \ > + asm volatile("{ r1:0 = %1 }\n\t" \ > + encoding \ > + "%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\n\t" > +#define GP_EQ_R0 ".word 0x6220c00b\n\t" > +#define UPCYCLELO_EQ_R0 ".word 0x6220c00e\n\t" > +#define UPCYCLEHI_EQ_R0 ".word 0x6220c00f\n\t" > +#define UTIMERLO_EQ_R0 ".word 0x6220c01e\n\t" > +#define UTIMERHI_EQ_R0 ".word 0x6220c01f\n\t" > + > +#define C9_8_EQ_R1_0 ".word 0x6320c008\n\t" > +#define C11_10_EQ_R1_0 ".word 0x6320c00a\n\t" > +#define C15_14_EQ_R1_0 ".word 0x6320c00e\n\t" > +#define C31_30_EQ_R1_0 ".word 0x6320c01e\n\t" Only the assignment to PC and C9 (which is an alias for PC) are not allowed by the assembler. For the others, use the normal assembly syntax. > + > +static inline uint32_t write_reg(int rnum, uint32_t val) { > + uint32_t result; > + switch (rnum) { > + case HEX_REG_USR: > + WRITE_REG("usr", result, val); > + break; > + case HEX_REG_PC: > + WRITE_REG_ENCODED("pc", PC_EQ_R0, result, val); > + break; > + case HEX_REG_GP: > + WRITE_REG_ENCODED("gp", GP_EQ_R0, result, val); > + break; > + case HEX_REG_UPCYCLELO: > + WRITE_REG_ENCODED("upcyclelo", UPCYCLELO_EQ_R0, result, val); > + break; > + case HEX_REG_UPCYCLEHI: > + WRITE_REG_ENCODED("upcyclehi", UPCYCLEHI_EQ_R0, result, val); > + break; > + case HEX_REG_UTIMERLO: > + WRITE_REG_ENCODED("utimerlo", UTIMERLO_EQ_R0, result, val); > + break; > + case HEX_REG_UTIMERHI: > + WRITE_REG_ENCODED("utimerhi", UTIMERHI_EQ_R0, result, val); > + break; > + } > + return result; > +} There's a 1-1 mapping from the invocations of this to a line in the switch statement. Replace the invocation with the body of each case, so you won't have to include hex_regs.h > + > +static inline uint32_t write_reg_in_packet(int rnum, uint32_t val) { > + uint32_t result; > + switch (rnum) { > + case HEX_REG_USR: > + WRITE_REG_IN_PACKET("usr", result, val); > + break; > + case HEX_REG_PC: > + WRITE_REG_ENCODED_IN_PACKET("pc", PC_EQ_R0, result, val); > + break; > + case HEX_REG_GP: > + WRITE_REG_ENCODED_IN_PACKET("gp", GP_EQ_R0, result, val); > + break; > + case HEX_REG_UPCYCLELO: > + WRITE_REG_ENCODED_IN_PACKET("upcyclelo", UPCYCLELO_EQ_R0, > result, val); > + break; > + case HEX_REG_UPCYCLEHI: > + WRITE_REG_ENCODED_IN_PACKET("upcyclehi", UPCYCLEHI_EQ_R0, > result, val); > + break; > + case HEX_REG_UTIMERLO: > + WRITE_REG_ENCODED_IN_PACKET("utimerlo", UTIMERLO_EQ_R0, > result, val); > + break; > + case HEX_REG_UTIMERHI: > + WRITE_REG_ENCODED_IN_PACKET("utimerhi", UTIMERHI_EQ_R0, > result, val); > + break; > + } > + return result; > +} This is redundant because the IN_PACKET macros are redundant. > + > +static inline uint64_t write_reg_pair(int rnum, uint32_t val_hi, > + uint32_t val_lo) { > + uint64_t val = (uint64_t) val_hi << 32 | val_lo; > + uint64_t result; > + switch (rnum) { > + case HEX_REG_PAIR_C9_8: > + WRITE_REG_PAIR_ENCODED("c9:8", C9_8_EQ_R1_0, result, val); > + break; > + case HEX_REG_PAIR_C11_10: > + WRITE_REG_PAIR_ENCODED("c11:10", C11_10_EQ_R1_0, result, val); > + break; > + case HEX_REG_PAIR_C15_14: > + WRITE_REG_PAIR_ENCODED("c15:14", C15_14_EQ_R1_0, result, val); > + break; > + case HEX_REG_PAIR_C31_30: > + WRITE_REG_PAIR_ENCODED("c31:30", C31_30_EQ_R1_0, result, val); > + break; > + } > + return result; > +} > + > +static inline uint64_t write_reg_pair_in_packet(int rnum, uint32_t val_hi, > + uint32_t val_lo) { > + uint64_t val = (uint64_t) val_hi << 32 | val_lo; > + uint64_t result; > + switch (rnum) { > + case HEX_REG_PAIR_C9_8: > + WRITE_REG_PAIR_ENCODED_IN_PACKET("c9:8", C9_8_EQ_R1_0, > result, val); > + break; > + case HEX_REG_PAIR_C11_10: > + WRITE_REG_PAIR_ENCODED_IN_PACKET("c11:10", C11_10_EQ_R1_0, > result, val); > + break; > + case HEX_REG_PAIR_C15_14: > + WRITE_REG_PAIR_ENCODED_IN_PACKET("c15:14", C15_14_EQ_R1_0, > result, val); > + break; > + case HEX_REG_PAIR_C31_30: > + WRITE_REG_PAIR_ENCODED_IN_PACKET("c31:30", C31_30_EQ_R1_0, > result, val); > + break; > + } > + return result; > +} This is redundant also. > + > +static inline void write_control_registers(void) { > + check(write_reg(HEX_REG_USR, 0xffffffff), 0x3ecfff3f); > + check(write_reg(HEX_REG_GP, 0xffffffff), 0xffffffc0); > + check(write_reg(HEX_REG_UPCYCLELO, 0xffffffff), 0x0); > + check(write_reg(HEX_REG_UPCYCLEHI, 0xffffffff), 0x0); > + check(write_reg(HEX_REG_UTIMERLO, 0xffffffff), 0x0); > + check(write_reg(HEX_REG_UTIMERHI, 0xffffffff), 0x0); > + > + /* > + * PC is special. Setting it to these values > + * should cause a catastrophic failure. > + */ > + check_ne(write_reg(HEX_REG_PC, 0x00000000), 0x00000000); > + check_ne(write_reg(HEX_REG_PC, 0x00000000), 0x00000001); > + check_ne(write_reg(HEX_REG_PC, 0xffffffff), 0xffffffff); > + check_ne(write_reg(HEX_REG_PC, 0x00000000), 0x00000000); } This is the same as the first test for PC. > + > +static inline void write_control_registers_in_packets(void) > +{ > + check(write_reg_in_packet(HEX_REG_USR, 0xffffffff), 0x3ecfff3f); > + check(write_reg_in_packet(HEX_REG_GP, 0xffffffff), 0xffffffc0); > + check(write_reg_in_packet(HEX_REG_UPCYCLELO, 0xffffffff), 0x0); > + check(write_reg_in_packet(HEX_REG_UPCYCLEHI, 0xffffffff), 0x0); > + check(write_reg_in_packet(HEX_REG_UTIMERLO, 0xffffffff), 0x0); > + check(write_reg_in_packet(HEX_REG_UTIMERHI, 0xffffffff), 0x0); > + > + check_ne(write_reg_in_packet(HEX_REG_PC, 0x00000000), 0x00000000); > + check_ne(write_reg_in_packet(HEX_REG_PC, 0x00000001), 0x00000001); > + check_ne(write_reg_in_packet(HEX_REG_PC, 0xffffffff), 0xffffffff); > + check_ne(write_reg_in_packet(HEX_REG_PC, 0x00000000), 0x00000000); > +} This is redundant. > + > +static inline void write_control_register_pairs(void) > +{ > + check(write_reg_pair(HEX_REG_PAIR_C11_10, 0xffffffff, 0xffffffff), > + 0xffffffc0ffffffff); > + check(write_reg_pair(HEX_REG_PAIR_C15_14, 0xffffffff, 0xffffffff), 0x0); > + check(write_reg_pair(HEX_REG_PAIR_C31_30, 0xffffffff, 0xffffffff), > +0x0); > + > + check_ne(write_reg_pair(HEX_REG_PAIR_C9_8, 0x00000000, > 0x00000000), > + 0x0000000000000000); > + check_ne(write_reg_pair(HEX_REG_PAIR_C9_8, 0x00000001, > 0x00000000), > + 0x0000000100000000); > + check_ne(write_reg_pair(HEX_REG_PAIR_C9_8, 0xffffffff, 0xffffffff), > + 0xffffffffffffffff); > + check_ne(write_reg_pair(HEX_REG_PAIR_C9_8, 0x00000000, > 0x00000000), > + 0x0000000000000000); > +} > + > +static inline void write_control_register_pairs_in_packets(void) > +{ > + check(write_reg_pair_in_packet(HEX_REG_PAIR_C11_10, 0xffffffff, > 0xffffffff), > + 0xffffffc0ffffffff); > + check(write_reg_pair_in_packet(HEX_REG_PAIR_C15_14, 0xffffffff, > 0xffffffff), > + 0x0); > + check(write_reg_pair_in_packet(HEX_REG_PAIR_C31_30, 0xffffffff, > 0xffffffff), > + 0x0); > + > + check_ne(write_reg_pair_in_packet(HEX_REG_PAIR_C9_8, 0x00000000, > + 0x00000000), 0x0000000000000000); > + check_ne(write_reg_pair_in_packet(HEX_REG_PAIR_C9_8, 0x00000001, > + 0x00000000), 0x0000000100000000); > + check_ne(write_reg_pair_in_packet(HEX_REG_PAIR_C9_8, 0xffffffff, > + 0xffffffff), 0xffffffffffffffff); > + check_ne(write_reg_pair_in_packet(HEX_REG_PAIR_C9_8, 0x00000000, > + 0x00000000), 0x0000000000000000); } Also redundant. > + > +int main() > +{ > + err = 0; > + > + write_control_registers(); > + write_control_registers_in_packets(); > + write_control_register_pairs(); > + write_control_register_pairs_in_packets(); > + > + puts(err ? "FAIL" : "PASS"); > + return err; > +} > -- > 2.25.1
> > +#define WRITE_REG_IN_PACKET(reg_name, output, input) \ > > + asm volatile("{ " reg_name " = %1 }\n\t" \ > > This is no different from the WRITE_REG above. Instructions on a line with > no curly braces are a single packet. > Understood. The feedback on Brian's patch said to write tests that do transfers in a packet. Should I write some? (Just not in the way I did it above) > > + > > +/* > > + * 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\n\t" > > +#define GP_EQ_R0 ".word 0x6220c00b\n\t" > > +#define UPCYCLELO_EQ_R0 ".word 0x6220c00e\n\t" > > +#define UPCYCLEHI_EQ_R0 ".word 0x6220c00f\n\t" > > +#define UTIMERLO_EQ_R0 ".word 0x6220c01e\n\t" > > +#define UTIMERHI_EQ_R0 ".word 0x6220c01f\n\t" > > + > > +#define C9_8_EQ_R1_0 ".word 0x6320c008\n\t" > > +#define C11_10_EQ_R1_0 ".word 0x6320c00a\n\t" > > +#define C15_14_EQ_R1_0 ".word 0x6320c00e\n\t" > > +#define C31_30_EQ_R1_0 ".word 0x6320c01e\n\t" > > Only the assignment to PC and C9 (which is an alias for PC) are not allowed by > the assembler. For the others, use the normal assembly syntax. > I used the regular names at first, but when running `make check-tcg` it generates errors. For example: error: unknown register name 'gp' in asm WRITE_REG(result, "gp", 0xffffffff); Should I use them anyway?
> -----Original Message----- > From: Marco Liebel <mliebel@qti.qualcomm.com> > Sent: Wednesday, December 21, 2022 1:34 PM > To: Taylor Simpson <tsimpson@quicinc.com>; Marco Liebel (QUIC) > <quic_mliebel@quicinc.com>; qemu-devel@nongnu.org > Cc: Brian Cain <bcain@quicinc.com> > Subject: RE: [PATCH v2] Hexagon (target/hexagon) implement mutability > mask for GPRs > > > > +#define WRITE_REG_IN_PACKET(reg_name, output, input) \ > > > + asm volatile("{ " reg_name " = %1 }\n\t" \ > > > > This is no different from the WRITE_REG above. Instructions on a line > > with no curly braces are a single packet. > > > > Understood. The feedback on Brian's patch said to write tests that do > transfers in a packet. Should I write some? (Just not in the way I did it above) Put some more instructions in the packet with the assignment. I recommend a read from the same register and verify you get the old value. > > > > + > > > +/* > > > + * 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\n\t" > > > +#define GP_EQ_R0 ".word 0x6220c00b\n\t" > > > +#define UPCYCLELO_EQ_R0 ".word 0x6220c00e\n\t" > > > +#define UPCYCLEHI_EQ_R0 ".word 0x6220c00f\n\t" > > > +#define UTIMERLO_EQ_R0 ".word 0x6220c01e\n\t" > > > +#define UTIMERHI_EQ_R0 ".word 0x6220c01f\n\t" > > > + > > > +#define C9_8_EQ_R1_0 ".word 0x6320c008\n\t" > > > +#define C11_10_EQ_R1_0 ".word 0x6320c00a\n\t" > > > +#define C15_14_EQ_R1_0 ".word 0x6320c00e\n\t" > > > +#define C31_30_EQ_R1_0 ".word 0x6320c01e\n\t" > > > > Only the assignment to PC and C9 (which is an alias for PC) are not > > allowed by the assembler. For the others, use the normal assembly syntax. > > > > I used the regular names at first, but when running `make check-tcg` it > generates errors. For example: > error: unknown register name 'gp' in asm WRITE_REG(result, "gp", > 0xffffffff); > > Should I use them anyway? Don't use the WRITE_REG macro because that puts reg_name in the clobbers list and the compiler doesn't know about these registers. If you use the WRITE_REG_ENCODED macro, it should work - though you might want to change the name of the macro to WRITE_REG_NOCLOBBER. Thanks, Taylor
> -----Original Message----- > From: Taylor Simpson <tsimpson@quicinc.com> > Sent: Mittwoch, 21. Dezember 2022 21:06 > To: Marco Liebel <mliebel@qti.qualcomm.com>; Marco Liebel (QUIC) > <quic_mliebel@quicinc.com>; qemu-devel@nongnu.org > Cc: Brian Cain <bcain@quicinc.com> > Subject: RE: [PATCH v2] Hexagon (target/hexagon) implement mutability > mask for GPRs > > > > > -----Original Message----- > > From: Marco Liebel <mliebel@qti.qualcomm.com> > > Sent: Wednesday, December 21, 2022 1:34 PM > > To: Taylor Simpson <tsimpson@quicinc.com>; Marco Liebel (QUIC) > > <quic_mliebel@quicinc.com>; qemu-devel@nongnu.org > > Cc: Brian Cain <bcain@quicinc.com> > > Subject: RE: [PATCH v2] Hexagon (target/hexagon) implement mutability > > mask for GPRs > > > > > > +#define WRITE_REG_IN_PACKET(reg_name, output, input) \ > > > > + asm volatile("{ " reg_name " = %1 }\n\t" \ > > > > > > This is no different from the WRITE_REG above. Instructions on a > > > line with no curly braces are a single packet. > > > > > > > Understood. The feedback on Brian's patch said to write tests that do > > transfers in a packet. Should I write some? (Just not in the way I did > > it above) > > Put some more instructions in the packet with the assignment. I recommend > a read from the same register and verify you get the old value. > Reading and writing a control register in a single packet isn't possible, because CR instructions can only be executed in slot 3. I was thinking to put a nop inside the packet, just so the packet gets generated. Or is there something else that's useful, other than reading the previous value?
> -----Original Message----- > From: Marco Liebel <mliebel@qti.qualcomm.com> > Sent: Wednesday, January 4, 2023 2:28 AM > To: Taylor Simpson <tsimpson@quicinc.com>; Marco Liebel (QUIC) > <quic_mliebel@quicinc.com>; qemu-devel@nongnu.org > Cc: Brian Cain <bcain@quicinc.com> > Subject: RE: [PATCH v2] Hexagon (target/hexagon) implement mutability > mask for GPRs > > > -----Original Message----- > > From: Taylor Simpson <tsimpson@quicinc.com> > > Sent: Mittwoch, 21. Dezember 2022 21:06 > > To: Marco Liebel <mliebel@qti.qualcomm.com>; Marco Liebel (QUIC) > > <quic_mliebel@quicinc.com>; qemu-devel@nongnu.org > > Cc: Brian Cain <bcain@quicinc.com> > > Subject: RE: [PATCH v2] Hexagon (target/hexagon) implement mutability > > mask for GPRs > > > > > > > > > -----Original Message----- > > > From: Marco Liebel <mliebel@qti.qualcomm.com> > > > Sent: Wednesday, December 21, 2022 1:34 PM > > > To: Taylor Simpson <tsimpson@quicinc.com>; Marco Liebel (QUIC) > > > <quic_mliebel@quicinc.com>; qemu-devel@nongnu.org > > > Cc: Brian Cain <bcain@quicinc.com> > > > Subject: RE: [PATCH v2] Hexagon (target/hexagon) implement > > > mutability mask for GPRs > > > > > > > > +#define WRITE_REG_IN_PACKET(reg_name, output, input) \ > > > > > + asm volatile("{ " reg_name " = %1 }\n\t" \ > > > > > > > > This is no different from the WRITE_REG above. Instructions on a > > > > line with no curly braces are a single packet. > > > > > > > > > > Understood. The feedback on Brian's patch said to write tests that > > > do transfers in a packet. Should I write some? (Just not in the way > > > I did it above) > > > > Put some more instructions in the packet with the assignment. I > > recommend a read from the same register and verify you get the old value. > > > > Reading and writing a control register in a single packet isn't possible, > because CR instructions can only be executed in slot 3. I was thinking to put a > nop inside the packet, just so the packet gets generated. Or is there > something else that's useful, other than reading the previous value? You are right. Let's move forward without any packet tests. Sorry for the confusion. Taylor
© 2016 - 2024 Red Hat, Inc.