Not sure of the exact reasoning as I inherited the change. However, when I
went through this code before I took it that the change was needed to
filter out the case where SCTLR may be either of the ARMv8 variants
(SCTLR_EL1 or SCTLR_EL3) as neither of them have a SCTLR_V bits. In fact,
looking quickly through the ARMv8 ARM, I don't see any mention of hivec
support for AArch64.
I think the more appropriate check in this case is to check whether the
current EL is 32-bit instead of ARMv8. Made this change in v9 along with
a comment.
I'll leave your review-by off until you okay this.
Greg
On 31 October 2014 09:07, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 30 October 2014 21:28, Greg Bellows <greg.bell...@linaro.org> wrote:
> > From: Fabian Aggeler <aggel...@ethz.ch>
> >
> > Implements SCTLR_EL3 and uses secure/non-secure instance when
> > needed.
> >
> > Signed-off-by: Fabian Aggeler <aggel...@ethz.ch>
> > Signed-off-by: Greg Bellows <greg.bell...@linaro.org>
>
> > diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> > index e0b82a6..18f4726 100644
> > --- a/target-arm/cpu.c
> > +++ b/target-arm/cpu.c
> > @@ -109,7 +109,7 @@ static void arm_cpu_reset(CPUState *s)
> > #if defined(CONFIG_USER_ONLY)
> > env->pstate = PSTATE_MODE_EL0t;
> > /* Userspace expects access to DC ZVA, CTL_EL0 and the cache
> ops */
> > - env->cp15.c1_sys |= SCTLR_UCT | SCTLR_UCI | SCTLR_DZE;
> > + env->cp15.sctlr_el[1] |= SCTLR_UCT | SCTLR_UCI | SCTLR_DZE;
> > /* and to the FP/Neon instructions */
> > env->cp15.c1_coproc = deposit64(env->cp15.c1_coproc, 20, 2, 3);
> > #else
> > @@ -167,7 +167,8 @@ static void arm_cpu_reset(CPUState *s)
> > env->thumb = initial_pc & 1;
> > }
> >
> > - if (env->cp15.c1_sys & SCTLR_V) {
> > + if (!arm_feature(env, ARM_FEATURE_V8)
> > + && (A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_V)) {
> > env->regs[15] = 0xFFFF0000;
>
> Why has this condition had an "if not v8" added to it? The v8
> spec doesn't drop support for hivecs as far as I can tell...
>
> Patch looks good otherwise.
>
> -- PMM
>
