Re: [Qemu-devel] [RFC PATCH 03/21] target-arm: adjust TTBCR for TrustZone feature

[Fedorov Sergey]

On 12/03/2013 04:15 PM, Peter Crosthwaite wrote:
> On Tue, Dec 3, 2013 at 6:48 PM, Sergey Fedorov <address@hidden> wrote:
> > TTBCR has additional fields PD0 and PD1 when using Short-descriptor
> > translation table format on a CPU with TrustZone feature support.
> >
> > Signed-off-by: Sergey Fedorov <address@hidden>
> > ---
> >   target-arm/helper.c |    4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index a247ca0..6642e53 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -1159,8 +1159,10 @@ static int vmsa_ttbcr_raw_write(CPUARMState *env, const 
> > ARMCPRegInfo *ri,
> >   {
> >       int maskshift = extract32(value, 0, 3);
> >
> > -    if (arm_feature(env, ARM_FEATURE_LPAE)) {
> > +    if (arm_feature(env, ARM_FEATURE_LPAE) && (value & (1 << 31))) {
> This appears to be changing more than just trustzone dependent
> behavior. That is, if we take just this hunk and ignore the one below
> you see a change in the non-tz behaviour. Is the hunk legitimate
> irrespective of trustzone support?

Yes, current implementation is not accurate according to ARMv7-AR 
reference manual. See "B4.1.153 TTBCR, Translation Table Base Control 
Register, VMSA | TTBCR format when using the Long-descriptor translation 
table format". When LPAE feature is supported, EAE, bit[31] selects 
translation descriptor format and, therefore, TTBCR format.

> >           value &= ~((7 << 19) | (3 << 14) | (0xf << 3));
> > +    } else if (arm_feature(env, ARM_FEATURE_TRUSTZONE)) {
> > +        value &= 0x37;
> >       } else {
> >           value &= 7;
> >       }
> There are a few magic numbers in the patch probably worth macrofiying.

As I can see, magic numbers are widely used through all of this file to 
represent CP register fields and other things. Maybe the macrofying 
should be done separately from this patch series?

> Regards,
> Peter
>
> > --
> > 1.7.9.5

Best regards,
Sergey Fedorov