In v7A HCR.TID1 is defined to trap for TCMTR, TLBTR, REVIDR and AIDR.
We incorrectly use an accessfn for REVIDR and AIDR that only traps on
v8A cores. Fix this by collapsing access_aa64_tid1() and
access_aa32_tid1() together and never doing a check for v8 vs v7.
The accessfn is also used for SMIDR_EL1, which is fine as this
register is AArch64 only.
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
target/arm/helper.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index c4f73eb3f3..0896e90965 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -924,8 +924,8 @@ static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri)
return ret;
}
-static CPAccessResult access_aa64_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
- bool isread)
+static CPAccessResult access_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
+ bool isread)
{
if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID1)) {
return CP_ACCESS_TRAP_EL2;
@@ -934,16 +934,6 @@ static CPAccessResult access_aa64_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
return CP_ACCESS_OK;
}
-static CPAccessResult access_aa32_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
- bool isread)
-{
- if (arm_feature(env, ARM_FEATURE_V8)) {
- return access_aa64_tid1(env, ri, isread);
- }
-
- return CP_ACCESS_OK;
-}
-
static const ARMCPRegInfo v7_cp_reginfo[] = {
/* the old v6 WFI, UNPREDICTABLE in v7 but we choose to NOP */
{ .name = "NOP", .cp = 15, .crn = 7, .crm = 0, .opc1 = 0, .opc2 = 4,
@@ -969,7 +959,7 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
{ .name = "AIDR", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .opc1 = 1, .crn = 0, .crm = 0, .opc2 = 7,
.access = PL1_R, .type = ARM_CP_CONST,
- .accessfn = access_aa64_tid1,
+ .accessfn = access_tid1,
.fgt = FGT_AIDR_EL1,
.resetvalue = 0 },
/*
@@ -4997,7 +4987,7 @@ static const ARMCPRegInfo sme_reginfo[] = {
.writefn = smcr_write, .raw_writefn = raw_write },
{ .name = "SMIDR_EL1", .state = ARM_CP_STATE_AA64,
.opc0 = 3, .opc1 = 1, .crn = 0, .crm = 0, .opc2 = 6,
- .access = PL1_R, .accessfn = access_aa64_tid1,
+ .access = PL1_R, .accessfn = access_tid1,
/*
* IMPLEMENTOR = 0 (software)
* REVISION = 0 (implementation defined)
@@ -7094,7 +7084,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
{ .name = "REVIDR_EL1", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 6,
.access = PL1_R,
- .accessfn = access_aa64_tid1,
+ .accessfn = access_tid1,
.fgt = FGT_REVIDR_EL1,
.type = ARM_CP_CONST, .resetvalue = cpu->revidr },
};
@@ -7118,7 +7108,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
{ .name = "TCMTR",
.cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 2,
.access = PL1_R,
- .accessfn = access_aa32_tid1,
+ .accessfn = access_tid1,
.type = ARM_CP_CONST, .resetvalue = 0 },
};
/* TLBTR is specific to VMSA */
@@ -7126,7 +7116,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
.name = "TLBTR",
.cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3,
.access = PL1_R,
- .accessfn = access_aa32_tid1,
+ .accessfn = access_tid1,
.type = ARM_CP_CONST, .resetvalue = 0,
};
/* MPUIR is specific to PMSA V6+ */
--
2.47.3On 1/1/26 04:08, Peter Maydell wrote: > In v7A HCR.TID1 is defined to trap for TCMTR, TLBTR, REVIDR and AIDR. > We incorrectly use an accessfn for REVIDR and AIDR that only traps on > v8A cores. Fix this by collapsing access_aa64_tid1() and > access_aa32_tid1() together and never doing a check for v8 vs v7. > > The accessfn is also used for SMIDR_EL1, which is fine as this > register is AArch64 only. > > Cc:qemu-stable@nongnu.org > Signed-off-by: Peter Maydell<peter.maydell@linaro.org> > --- > target/arm/helper.c | 24 +++++++----------------- > 1 file changed, 7 insertions(+), 17 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Peter Maydell <peter.maydell@linaro.org> writes:
> In v7A HCR.TID1 is defined to trap for TCMTR, TLBTR, REVIDR and AIDR.
> We incorrectly use an accessfn for REVIDR and AIDR that only traps on
> v8A cores. Fix this by collapsing access_aa64_tid1() and
> access_aa32_tid1() together and never doing a check for v8 vs v7.
>
> The accessfn is also used for SMIDR_EL1, which is fine as this
> register is AArch64 only.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> target/arm/helper.c | 24 +++++++-----------------
> 1 file changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index c4f73eb3f3..0896e90965 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -924,8 +924,8 @@ static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri)
> return ret;
> }
>
> -static CPAccessResult access_aa64_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
> - bool isread)
> +static CPAccessResult access_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
> + bool isread)
> {
> if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID1)) {
> return CP_ACCESS_TRAP_EL2;
> @@ -934,16 +934,6 @@ static CPAccessResult access_aa64_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
> return CP_ACCESS_OK;
> }
>
> -static CPAccessResult access_aa32_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
> - bool isread)
> -{
> - if (arm_feature(env, ARM_FEATURE_V8)) {
> - return access_aa64_tid1(env, ri, isread);
> - }
> -
> - return CP_ACCESS_OK;
> -}
> -
This logic makes more sense from the descriptions compared to 2/4.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
© 2016 - 2026 Red Hat, Inc.