[PATCH] target/arm: add support for 64-bit PMCCNTR in AArch32 mode
See
https://developer.arm.com/documentation/ddi0601/2024-06/AArch32-Registers/PMCCNTR--Performance-Monitors-Cycle-Count-Register?lang=en
Signed-off-by: Alex Richardson
---
target/arm/helper.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 8fb4b474e8..94900667c3 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5952,6 +5952,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
.access = PL1_RW, .accessfn = access_trap_aa32s_el1,
.writefn = sdcr_write,
.fieldoffset = offsetoflow32(CPUARMState, cp15.mdcr_el3) },
+{ .name = "PMCCNTR", .state = ARM_CP_STATE_AA32,
+ .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_64BIT,
+ .cp = 15, .crm = 9, .opc1 = 0,
+ .access = PL0_RW, .resetvalue = 0, .fgt = FGT_PMCCNTR_EL0,
+ .readfn = pmccntr_read, .writefn = pmccntr_write,
+ .accessfn = pmreg_access_ccntr },
};
/* These are present only when EL1 supports AArch32 */
--
2.46.0.rc2.264.g509ed76dc8-goog
[PATCH v2 2/2] target/riscv: Call check_access() before tcg_temp_new()
---
target/riscv/insn_trans/trans_rvh.c.inc | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvh.c.inc
b/target/riscv/insn_trans/trans_rvh.c.inc
index c66268a9b0..c6bbc54d68 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -32,11 +32,11 @@ static bool gen_hlv(DisasContext *ctx, int rs1, int rd,
MemOp memop)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
+check_access(ctx);
+
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
-check_access(ctx);
-
gen_get_gpr(t0, rs1);
tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
memop);
@@ -55,11 +55,11 @@ static bool gen_hsv(DisasContext *ctx, int rs1, int rs2,
MemOp memop)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
+check_access(ctx);
+
TCGv t0 = tcg_temp_new();
TCGv dat = tcg_temp_new();
-check_access(ctx);
-
gen_get_gpr(t0, rs1);
gen_get_gpr(dat, rs2);
@@ -134,11 +134,11 @@ static bool trans_hlvx_hu(DisasContext *ctx, arg_hlvx_hu
*a)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
+check_access(ctx);
+
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
-check_access(ctx);
-
gen_get_gpr(t0, a->rs1);
gen_helper_hyp_hlvx_hu(t1, cpu_env, t0);
@@ -156,11 +156,11 @@ static bool trans_hlvx_wu(DisasContext *ctx, arg_hlvx_wu
*a)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
+check_access(ctx);
+
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
-check_access(ctx);
-
gen_get_gpr(t0, a->rs1);
gen_helper_hyp_hlvx_wu(t1, cpu_env, t0);
--
2.30.0
[PATCH v2 1/2] target/riscv: Reduce duplicated code in trans_rvh.c.inc
I rencently merged CHERI QEMU up to 5.2, and we have to modify all
functions that perform memory accesses. Factoring these almost-indentical
functions into two shared helpers makes our changes a lot smaller and
should also make this code easier to maintain.
---
target/riscv/insn_trans/trans_rvh.c.inc | 199
1 file changed, 29 insertions(+), 170 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvh.c.inc
b/target/riscv/insn_trans/trans_rvh.c.inc
index ce7ed5affb..c66268a9b0 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -28,7 +28,7 @@ static void check_access(DisasContext *ctx) {
}
#endif
-static bool trans_hlv_b(DisasContext *ctx, arg_hlv_b *a)
+static bool gen_hlv(DisasContext *ctx, int rs1, int rd, MemOp memop)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
@@ -37,10 +37,10 @@ static bool trans_hlv_b(DisasContext *ctx, arg_hlv_b *a)
check_access(ctx);
-gen_get_gpr(t0, a->rs1);
+gen_get_gpr(t0, rs1);
-tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_SB);
-gen_set_gpr(a->rd, t1);
+tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
memop);
+gen_set_gpr(rd, t1);
tcg_temp_free(t0);
tcg_temp_free(t1);
@@ -50,224 +50,83 @@ static bool trans_hlv_b(DisasContext *ctx, arg_hlv_b *a)
#endif
}
-static bool trans_hlv_h(DisasContext *ctx, arg_hlv_h *a)
+
+static bool gen_hsv(DisasContext *ctx, int rs1, int rs2, MemOp memop)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
+TCGv dat = tcg_temp_new();
check_access(ctx);
-gen_get_gpr(t0, a->rs1);
+gen_get_gpr(t0, rs1);
+gen_get_gpr(dat, rs2);
-tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_TESW);
-gen_set_gpr(a->rd, t1);
+tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
memop);
tcg_temp_free(t0);
-tcg_temp_free(t1);
+tcg_temp_free(dat);
return true;
#else
return false;
#endif
}
-static bool trans_hlv_w(DisasContext *ctx, arg_hlv_w *a)
+static bool trans_hlv_b(DisasContext *ctx, arg_hlv_b *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a->rs1);
+return gen_hlv(ctx, a->rs1, a->rd, MO_SB);
+}
-tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_TESL);
-gen_set_gpr(a->rd, t1);
+static bool trans_hlv_h(DisasContext *ctx, arg_hlv_h *a)
+{
+return gen_hlv(ctx, a->rs1, a->rd, MO_TESW);
+}
-tcg_temp_free(t0);
-tcg_temp_free(t1);
-return true;
-#else
-return false;
-#endif
+static bool trans_hlv_w(DisasContext *ctx, arg_hlv_w *a)
+{
+return gen_hlv(ctx, a->rs1, a->rd, MO_TESL);
}
static bool trans_hlv_bu(DisasContext *ctx, arg_hlv_bu *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a->rs1);
-
-tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_UB);
-gen_set_gpr(a->rd, t1);
-
-tcg_temp_free(t0);
-tcg_temp_free(t1);
-return true;
-#else
-return false;
-#endif
+return gen_hlv(ctx, a->rs1, a->rd, MO_UB);
}
static bool trans_hlv_hu(DisasContext *ctx, arg_hlv_hu *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a->rs1);
-tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_TEUW);
-gen_set_gpr(a->rd, t1);
-
-tcg_temp_free(t0);
-tcg_temp_free(t1);
-return true;
-#else
-return false;
-#endif
+return gen_hlv(ctx, a->rs1, a->rd, MO_TEUW);
}
static bool trans_hsv_b(DisasContext *ctx, arg_hsv_b *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv dat = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a->rs1);
-gen_get_gpr(dat, a->rs2);
-
-tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_SB);
-
-tcg_temp_free(t0);
-tcg_temp_free(dat);
-return true;
-#else
-return false;
-#endif
+return gen_hlv(ctx, a->rs1, a->rs2, MO_SB);
}
static bool trans_hsv_h(DisasContext *ctx, arg_hsv_h *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv dat = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a->rs1);
-gen_get_gpr(dat, a->rs2);
-
-tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_TESW);
-
-tcg_temp_free(t0);
-tcg_temp_free(dat);
-return true;
-#else
-return false;
-#endif
+return gen_hlv(ctx, a->rs1, a->rs2, MO_TESW);
}
static
[PATCH 2/2] target/riscv: Call check_access() before tcg_temp_new()
---
target/riscv/insn_trans/trans_rvh.c.inc | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvh.c.inc
b/target/riscv/insn_trans/trans_rvh.c.inc
index 203a620723..b0a9ea1dca 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -32,11 +32,11 @@ static bool gen_hlv(DisasContext *ctx, arg_hlv_b *a, MemOp
memop)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
+check_access(ctx);
+
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
-check_access(ctx);
-
gen_get_gpr(t0, a->rs1);
tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
memop);
@@ -55,10 +55,11 @@ static bool gen_hsv(DisasContext *ctx, arg_hsv_b *a, MemOp
memop)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
+check_access(ctx);
+
TCGv t0 = tcg_temp_new();
TCGv dat = tcg_temp_new();
-check_access(ctx);
gen_get_gpr(t0, a->rs1);
gen_get_gpr(dat, a->rs2);
@@ -134,10 +135,11 @@ static bool trans_hlvx_hu(DisasContext *ctx, arg_hlvx_hu
*a)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
+check_access(ctx);
+
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
-check_access(ctx);
gen_get_gpr(t0, a->rs1);
@@ -156,10 +158,11 @@ static bool trans_hlvx_wu(DisasContext *ctx, arg_hlvx_wu
*a)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
+check_access(ctx);
+
TCGv t0 = tcg_temp_new();
TCGv t1 = tcg_temp_new();
-check_access(ctx);
gen_get_gpr(t0, a->rs1);
--
2.30.0
[PATCH 1/2] target/riscv: Reduce duplicated code in trans_rvh.c.inc
I rencently merged CHERI QEMU up to 5.2, and we have to modify all
functions that perform memory accesses. Factoring these almost-indentical
functions into two shared helpers makes our changes a lot smaller and
should also make this code easier to maintain.
---
target/riscv/insn_trans/trans_rvh.c.inc | 193
1 file changed, 26 insertions(+), 167 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvh.c.inc
b/target/riscv/insn_trans/trans_rvh.c.inc
index ce7ed5affb..203a620723 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -28,7 +28,7 @@ static void check_access(DisasContext *ctx) {
}
#endif
-static bool trans_hlv_b(DisasContext *ctx, arg_hlv_b *a)
+static bool gen_hlv(DisasContext *ctx, arg_hlv_b *a, MemOp memop)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
@@ -39,7 +39,7 @@ static bool trans_hlv_b(DisasContext *ctx, arg_hlv_b *a)
gen_get_gpr(t0, a->rs1);
-tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_SB);
+tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
memop);
gen_set_gpr(a->rd, t1);
tcg_temp_free(t0);
@@ -50,224 +50,83 @@ static bool trans_hlv_b(DisasContext *ctx, arg_hlv_b *a)
#endif
}
-static bool trans_hlv_h(DisasContext *ctx, arg_hlv_h *a)
+
+static bool gen_hsv(DisasContext *ctx, arg_hsv_b *a, MemOp memop)
{
REQUIRE_EXT(ctx, RVH);
#ifndef CONFIG_USER_ONLY
TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
+TCGv dat = tcg_temp_new();
check_access(ctx);
gen_get_gpr(t0, a->rs1);
+gen_get_gpr(dat, a->rs2);
-tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_TESW);
-gen_set_gpr(a->rd, t1);
+tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
memop);
tcg_temp_free(t0);
-tcg_temp_free(t1);
+tcg_temp_free(dat);
return true;
#else
return false;
#endif
}
-static bool trans_hlv_w(DisasContext *ctx, arg_hlv_w *a)
+static bool trans_hlv_b(DisasContext *ctx, arg_hlv_b *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a->rs1);
+return gen_hlv(ctx, a, MO_SB);
+}
-tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_TESL);
-gen_set_gpr(a->rd, t1);
+static bool trans_hlv_h(DisasContext *ctx, arg_hlv_h *a)
+{
+return gen_hlv(ctx, a, MO_TESW);
+}
-tcg_temp_free(t0);
-tcg_temp_free(t1);
-return true;
-#else
-return false;
-#endif
+static bool trans_hlv_w(DisasContext *ctx, arg_hlv_w *a)
+{
+return gen_hlv(ctx, a, MO_TESL);
}
static bool trans_hlv_bu(DisasContext *ctx, arg_hlv_bu *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a->rs1);
-
-tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_UB);
-gen_set_gpr(a->rd, t1);
-
-tcg_temp_free(t0);
-tcg_temp_free(t1);
-return true;
-#else
-return false;
-#endif
+return gen_hlv(ctx, a, MO_UB);
}
static bool trans_hlv_hu(DisasContext *ctx, arg_hlv_hu *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a->rs1);
-tcg_gen_qemu_ld_tl(t1, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_TEUW);
-gen_set_gpr(a->rd, t1);
-
-tcg_temp_free(t0);
-tcg_temp_free(t1);
-return true;
-#else
-return false;
-#endif
+return gen_hlv(ctx, a, MO_TEUW);
}
static bool trans_hsv_b(DisasContext *ctx, arg_hsv_b *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv dat = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a->rs1);
-gen_get_gpr(dat, a->rs2);
-
-tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_SB);
-
-tcg_temp_free(t0);
-tcg_temp_free(dat);
-return true;
-#else
-return false;
-#endif
+return gen_hsv(ctx, a, MO_SB);
}
static bool trans_hsv_h(DisasContext *ctx, arg_hsv_h *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv dat = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a->rs1);
-gen_get_gpr(dat, a->rs2);
-
-tcg_gen_qemu_st_tl(dat, t0, ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK,
MO_TESW);
-
-tcg_temp_free(t0);
-tcg_temp_free(dat);
-return true;
-#else
-return false;
-#endif
+return gen_hsv(ctx, a, MO_TESW);
}
static bool trans_hsv_w(DisasContext *ctx, arg_hsv_w *a)
{
-REQUIRE_EXT(ctx, RVH);
-#ifndef CONFIG_USER_ONLY
-TCGv t0 = tcg_temp_new();
-TCGv dat = tcg_temp_new();
-
-check_access(ctx);
-
-gen_get_gpr(t0, a-
[PATCH] target/riscv: Fix definition of MSTATUS_TW and MSTATUS_TSR
The TW and TSR fields should be bits 21 and 22 and not 30/29. This was found while comparing QEMU behaviour against the sail formal model (https://github.com/rems-project/sail-riscv/). Signed-off-by: Alex Richardson --- target/riscv/cpu_bits.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h index 24b24c69c5..92147332c6 100644 --- a/target/riscv/cpu_bits.h +++ b/target/riscv/cpu_bits.h @@ -379,8 +379,8 @@ #define MSTATUS_MXR 0x0008 #define MSTATUS_VM 0x1F00 /* until: priv-1.9.1 */ #define MSTATUS_TVM 0x0010 /* since: priv-1.10 */ -#define MSTATUS_TW 0x2000 /* since: priv-1.10 */ -#define MSTATUS_TSR 0x4000 /* since: priv-1.10 */ +#define MSTATUS_TW 0x0020 /* since: priv-1.10 */ +#define MSTATUS_TSR 0x0040 /* since: priv-1.10 */ #define MSTATUS_GVA 0x40ULL #define MSTATUS_MPV 0x80ULL -- 2.29.2
[PATCH] Fix MIPS add.s after 1ace099f2acb952eaaef0ba7725879949a7e4406
After merging latest QEMU upstream into our CHERI fork, I noticed that
some of the FPU tests in our MIPS baremetal testsuite
(https://github.com/CTSRD-CHERI/cheritest) started failing. It turns out
this commit accidentally changed add.s into a subtract.
Signed-off-by: Alex Richardson
---
target/mips/fpu_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/mips/fpu_helper.c b/target/mips/fpu_helper.c
index 7a3a61cab3..56beda49d8 100644
--- a/target/mips/fpu_helper.c
+++ b/target/mips/fpu_helper.c
@@ -1221,7 +1221,7 @@ uint32_t helper_float_add_s(CPUMIPSState *env,
{
uint32_t wt2;
-wt2 = float32_sub(fst0, fst1, &env->active_fpu.fp_status);
+wt2 = float32_add(fst0, fst1, &env->active_fpu.fp_status);
update_fcr31(env, GETPC());
return wt2;
}
--
2.27.0
[PATCH v2] target/arm: add support for 64-bit PMCCNTR in AArch32 mode
In the PMUv3, a new AArch32 64-bit (MCRR/MRRC) accessor for the
PMCCNTR was added. In QEMU we forgot to implement this, so only
provide the 32-bit accessor. Since we have a 64-bit PMCCNTR
sysreg for AArch64, adding the 64-bit AArch32 version is easy.
We add the PMCCNTR to the v8_cp_reginfo because PMUv3 was added
in the ARMv8 architecture. This is consistent with how we
handle the existing PMCCNTR support, where we always implement
it for all v7 CPUs. This is arguably something we should
clean up so it is gated on ARM_FEATURE_PMU and/or an ID
register check for the relevant PMU version, but we should
do that as its own tidyup rather than being inconsistent between
this PMCCNTR accessor and the others.
Since the register name is the same as the 32-bit PMCCNTR, we set
ARM_CP_NO_GDB to avoid generating an invalid GDB XML.
See
https://developer.arm.com/documentation/ddi0601/2024-06/AArch32-Registers/PMCCNTR--Performance-Monitors-Cycle-Count-Register?lang=en
Signed-off-by: Alex Richardson
---
target/arm/cpregs-pmu.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
index 0f295b1376..ef176e4045 100644
--- a/target/arm/cpregs-pmu.c
+++ b/target/arm/cpregs-pmu.c
@@ -1276,6 +1276,12 @@ void define_pm_cpregs(ARMCPU *cpu)
.access = PL0_R, .accessfn = pmreg_access, .type = ARM_CP_CONST,
.fgt = FGT_PMCEIDN_EL0,
.resetvalue = cpu->pmceid1 },
+{ .name = "PMCCNTR", .state = ARM_CP_STATE_AA32,
+ .cp = 15, .crm = 9, .opc1 = 0,
+ .access = PL0_RW, .accessfn = pmreg_access_ccntr, .resetvalue =
0,
+ .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_64BIT | ARM_CP_NO_GDB,
+ .fgt = FGT_PMCCNTR_EL0, .readfn = pmccntr_read,
+ .writefn = pmccntr_write, },
};
define_arm_cp_regs(cpu, v8_pm_reginfo);
}
--
2.50.1.470.g6ba607880d-goog
[PATCH v3] target/arm: add support for 64-bit PMCCNTR in AArch32 mode
In the PMUv3, a new AArch32 64-bit (MCRR/MRRC) accessor for the
PMCCNTR was added. In QEMU we forgot to implement this, so only
provide the 32-bit accessor. Since we have a 64-bit PMCCNTR
sysreg for AArch64, adding the 64-bit AArch32 version is easy.
We add the PMCCNTR to the v8_cp_reginfo because PMUv3 was added
in the ARMv8 architecture. This is consistent with how we
handle the existing PMCCNTR support, where we always implement
it for all v7 CPUs. This is arguably something we should
clean up so it is gated on ARM_FEATURE_PMU and/or an ID
register check for the relevant PMU version, but we should
do that as its own tidyup rather than being inconsistent between
this PMCCNTR accessor and the others.
Since the register name is the same as the 32-bit PMCCNTR, we set
ARM_CP_NO_GDB on the 32-bit one to avoid generating an invalid GDB XML.
See
https://developer.arm.com/documentation/ddi0601/2024-06/AArch32-Registers/PMCCNTR--Performance-Monitors-Cycle-Count-Register?lang=en
Change v2->v3:
- Moved ARM_CP_NO_GDB to the 32-bit register if Armv8 is supported
Changes v1->v2:
- Moved to new file
- Updated commit message
- Added ARM_CP_NO_GDB
Signed-off-by: Alex Richardson
---
target/arm/cpregs-pmu.c | 24 +++-
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
index 0f295b1376..144e339c76 100644
--- a/target/arm/cpregs-pmu.c
+++ b/target/arm/cpregs-pmu.c
@@ -1067,11 +1067,6 @@ static const ARMCPRegInfo v7_pm_reginfo[] = {
.fgt = FGT_PMSELR_EL0,
.fieldoffset = offsetof(CPUARMState, cp15.c9_pmselr),
.writefn = pmselr_write, .raw_writefn = raw_write, },
-{ .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0,
- .access = PL0_RW, .resetvalue = 0, .type = ARM_CP_ALIAS | ARM_CP_IO,
- .fgt = FGT_PMCCNTR_EL0,
- .readfn = pmccntr_read, .writefn = pmccntr_write32,
- .accessfn = pmreg_access_ccntr },
{ .name = "PMCCNTR_EL0", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 3, .crn = 9, .crm = 13, .opc2 = 0,
.access = PL0_RW, .accessfn = pmreg_access_ccntr,
@@ -1211,6 +1206,19 @@ void define_pm_cpregs(ARMCPU *cpu)
define_one_arm_cp_reg(cpu, &pmcr);
define_one_arm_cp_reg(cpu, &pmcr64);
define_arm_cp_regs(cpu, v7_pm_reginfo);
+/* When Armv8 is supported, PMCCNTR aliases the new 64-bit version */
+ARMCPRegInfo pmccntr = {
+.name = "PMCCNTR",
+.cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0,
+.access = PL0_RW, .accessfn = pmreg_access_ccntr,
+.resetvalue = 0, .type = ARM_CP_ALIAS | ARM_CP_IO,
+.fgt = FGT_PMCCNTR_EL0,
+.readfn = pmccntr_read, .writefn = pmccntr_write32,
+};
+if (arm_feature(env, ARM_FEATURE_V8)) {
+pmccntr.type |= ARM_CP_NO_GDB;
+}
+define_one_arm_cp_reg(cpu, &pmccntr);
for (unsigned i = 0, pmcrn = pmu_num_counters(env); i < pmcrn; i++) {
g_autofree char *pmevcntr_name = g_strdup_printf("PMEVCNTR%d", i);
@@ -1276,6 +1284,12 @@ void define_pm_cpregs(ARMCPU *cpu)
.access = PL0_R, .accessfn = pmreg_access, .type = ARM_CP_CONST,
.fgt = FGT_PMCEIDN_EL0,
.resetvalue = cpu->pmceid1 },
+{ .name = "PMCCNTR", .state = ARM_CP_STATE_AA32,
+ .cp = 15, .crm = 9, .opc1 = 0,
+ .access = PL0_RW, .accessfn = pmreg_access_ccntr, .resetvalue =
0,
+ .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_64BIT,
+ .fgt = FGT_PMCCNTR_EL0, .readfn = pmccntr_read,
+ .writefn = pmccntr_write, },
};
define_arm_cp_regs(cpu, v8_pm_reginfo);
}
--
2.50.1.470.g6ba607880d-goog
Re: [PATCH v2] target/arm: add support for 64-bit PMCCNTR in AArch32 mode
On Fri, 25 Jul 2025 at 02:19, Peter Maydell wrote:
>
> On Fri, 25 Jul 2025 at 01:10, Alex Richardson
> wrote:
> >
> > In the PMUv3, a new AArch32 64-bit (MCRR/MRRC) accessor for the
> > PMCCNTR was added. In QEMU we forgot to implement this, so only
> > provide the 32-bit accessor. Since we have a 64-bit PMCCNTR
> > sysreg for AArch64, adding the 64-bit AArch32 version is easy.
> >
> > We add the PMCCNTR to the v8_cp_reginfo because PMUv3 was added
> > in the ARMv8 architecture. This is consistent with how we
> > handle the existing PMCCNTR support, where we always implement
> > it for all v7 CPUs. This is arguably something we should
> > clean up so it is gated on ARM_FEATURE_PMU and/or an ID
> > register check for the relevant PMU version, but we should
> > do that as its own tidyup rather than being inconsistent between
> > this PMCCNTR accessor and the others.
> >
> > Since the register name is the same as the 32-bit PMCCNTR, we set
> > ARM_CP_NO_GDB to avoid generating an invalid GDB XML.
>
> This is the wrong way around, I think. We should prefer to expose
> to GDB the 64-bit view of it, not the 32-bit view, because
> it's more comprehensive. Compare handling of the "PAR" definition
> in target/arm/helper.c.
>
Thanks for the reference to the existing code, will send out v3 shortly.
> >
> > See
> > https://developer.arm.com/documentation/ddi0601/2024-06/AArch32-Registers/PMCCNTR--Performance-Monitors-Cycle-Count-Register?lang=en
> >
> > Signed-off-by: Alex Richardson
> > ---
>
> It would have been helpful to mention here what the changes
> from v1 were -- I had to go and look up the list archives to
> remind myself of why we had to drop v1.
Apologies, I usually contribute via code review systems that track
changes and am new to contributing to email-based workflows.
>
> > target/arm/cpregs-pmu.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/target/arm/cpregs-pmu.c b/target/arm/cpregs-pmu.c
> > index 0f295b1376..ef176e4045 100644
> > --- a/target/arm/cpregs-pmu.c
> > +++ b/target/arm/cpregs-pmu.c
> > @@ -1276,6 +1276,12 @@ void define_pm_cpregs(ARMCPU *cpu)
> >.access = PL0_R, .accessfn = pmreg_access, .type =
> > ARM_CP_CONST,
> >.fgt = FGT_PMCEIDN_EL0,
> >.resetvalue = cpu->pmceid1 },
> > +{ .name = "PMCCNTR", .state = ARM_CP_STATE_AA32,
> > + .cp = 15, .crm = 9, .opc1 = 0,
> > + .access = PL0_RW, .accessfn = pmreg_access_ccntr,
> > .resetvalue = 0,
> > + .type = ARM_CP_ALIAS | ARM_CP_IO | ARM_CP_64BIT |
> > ARM_CP_NO_GDB,
> > + .fgt = FGT_PMCCNTR_EL0, .readfn = pmccntr_read,
> > + .writefn = pmccntr_write, },
> > };
> > define_arm_cp_regs(cpu, v8_pm_reginfo);
> > }
> > --
> > 2.50.1.470.g6ba607880d-goog
>
> thanks
> -- PMM
