On Sat, 2 Jul 2022 at 15:19, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> On 7/1/22 01:11, Peter Maydell wrote:
> > +static inline bool isar_feature_any_doublelock(const ARMISARegisters *id)
> > +{
> > + /*
> > + * We can't just OR together the aa32 and aa64 checks, because
> > + * if there is no AArch64 support the aa64 function will default
> > + * to returning true for a zero id_aa64dfr0.
> > + * We use "is id_aa64pfr0 non-zero" as a proxy for "do we have
> > + * the AArch64 ID register values in id", because it's always the
> > + * case that ID_AA64PFR0_EL1.EL0 at least will be non-zero.
> > + */
> > + if (id->id_aa64pfr0) {
> > + return isar_feature_aa64_doublelock(id);
> > + }
> > + return isar_feature_aa32_doublelock(id);
> > +}
>
> If you're going to rely on this, you need to clear this register for -cpu
> aarch64=off.
> It's probably easier to drop this function...
>
> > +static void osdlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
> > + uint64_t value)
> > +{
> > + /*
> > + * Only defined bit is bit 0 (DLK); if Feat_DoubleLock is not
> > + * implemented this is RAZ/WI.
> > + */
> > + if (cpu_isar_feature(any_doublelock, env_archcpu(env))) {
>
> ... and use
>
> if (arm_feature(env, ARM_FEATURE_AARCH64)
> ? cpu_isar_feature(aa64_doublelock, cpu)
> : cpu_isar_feature(aa32_doublelock, cpu)) {
>
> since it's just used once.
If I squash in this change:
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 069dfe1d308..1f4f3e0485c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -4218,22 +4218,6 @@ static inline bool isar_feature_any_ras(const
ARMISARegisters *id)
return isar_feature_aa64_ras(id) || isar_feature_aa32_ras(id);
}
-static inline bool isar_feature_any_doublelock(const ARMISARegisters *id)
-{
- /*
- * We can't just OR together the aa32 and aa64 checks, because
- * if there is no AArch64 support the aa64 function will default
- * to returning true for a zero id_aa64dfr0.
- * We use "is id_aa64pfr0 non-zero" as a proxy for "do we have
- * the AArch64 ID register values in id", because it's always the
- * case that ID_AA64PFR0_EL1.EL0 at least will be non-zero.
- */
- if (id->id_aa64pfr0) {
- return isar_feature_aa64_doublelock(id);
- }
- return isar_feature_aa32_doublelock(id);
-}
-
/*
* Forward to the above feature tests given an ARMCPU pointer.
*/
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 62bd8f85383..d09fccb0a4f 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -617,11 +617,14 @@ static void oslar_write(CPUARMState *env, const
ARMCPRegInfo *ri,
static void osdlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
+ ARMCPU *cpu = env_archcpu(env);
/*
* Only defined bit is bit 0 (DLK); if Feat_DoubleLock is not
* implemented this is RAZ/WI.
*/
- if (cpu_isar_feature(any_doublelock, env_archcpu(env))) {
+ if(arm_feature(env, ARM_FEATURE_AARCH64)
+ ? cpu_isar_feature(aa64_doublelock, cpu)
+ : cpu_isar_feature(aa32_doublelock, cpu)) {
env->cp15.osdlr_el1 = value & 1;
}
}
can I have a reviewed-by? Would save me doing a respin.
thanks
-- PMM
