Ah... I was confused and thinking of ttbcr_raw_write.  Yes, raw_write does
not actually touch the masks.

The reason we used raw_write is that it updates the correct ri offset.  We
could be calling with the secure or non-secure ri so it was necessary to
make sure the correct offset gets updated.

Staying with this pattern, I introduced a new utility function raw_ptr()
that gives back the pointer calculation based on the ri fieldoffset.  For
TCR, this will be the base of the TCR struct which we can then update.

On 4 November 2014 17:27, Peter Maydell <peter.mayd...@linaro.org> wrote:

> On 4 November 2014 22:46, Greg Bellows <greg.bell...@linaro.org> wrote:
> >
> >
> > On 31 October 2014 10:26, Peter Maydell <peter.mayd...@linaro.org>
> wrote:
> >>
> >> On 30 October 2014 21:28, Greg Bellows <greg.bell...@linaro.org> wrote:
> >> >  static void vmsa_ttbcr_reset(CPUARMState *env, const ARMCPRegInfo
> *ri)
> >> >  {
> >> > -    env->cp15.c2_base_mask = 0xffffc000u;
> >> > +    /* Rest both the TTBCR as well as the masks corresponding to the
> >> > bank of
> >> > +     * the TTBCR being reset.
> >> > +     */
> >> > +    A32_BANKED_REG_SET(env, c2_base_mask,
> >> > +                       ARM_CP_SECSTATE_TEST(ri, ARM_CP_SECSTATE_S),
> >> > +                       0xffffc000u);
> >> > +    A32_BANKED_REG_SET(env, c2_mask,
> >> > +                       ARM_CP_SECSTATE_TEST(ri, ARM_CP_SECSTATE_S),
> 0);
> >> >      raw_write(env, ri, 0);
> >> > -    env->cp15.c2_mask = 0;
> >>
> >> Similarly this will be much cleaner.
> >
> >
> > Changed in v9.  Question on reset.  We call raw_write() which also sets
> the
> > masks, but we set the masks separately here too, but different values.
> It
> > seems we should only need to set them in raw_write() is this true?
>
> No, raw_write() won't set the masks -- it just writes 32 or 64 bits
> to the field pointed to by fieldoffset. Which makes it a pretty
> obfuscated way of saying env->cp15.c2_control = 0; and I don't
> know why we do it this way currently. If we go to having a TTBCR
> struct we should just set all the fields directly here I think.
>
> -- PMM
>

Reply via email to