Re: [Qemu-devel] [PATCH v4 06/33] target-arm: make arm_current_pl() return PL3

[Greg Bellows]

Hi Peter,

Perhaps it is best to eliminate the made up "PL3" to avoid confusion with EL3.  Then this function can simply always return the correct level whether it is PL or EL.  Anywhere we require knowing whether we are secure or not can be checked separately, which may be clearer anyhow.  As well, we could add a is_secure_pl1() function that would combine the checks.

Thoughts?

Regards,

Greg

On 26 August 2014 09:29, Peter Maydell <address@hidden> wrote:

On 1 July 2014 00:09,  <address@hidden> wrote:
> From: Fabian Aggeler <address@hidden>
>
> Make arm_current_pl() return PL3 for secure PL1 and monitor mode.
> Increase MMU modes since mmu_index is directly infered from arm_
> current_pl(). Changes assertion in arm_el_is_aa64() to allow EL3.

> @@ -963,9 +963,12 @@ static inline int arm_current_pl(CPUARMState *env)
>
>      if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR) {
>          return 0;
> +    } else if (arm_is_secure(env)) {
> +        /* Secure PL1 and monitor mode are mapped to PL3 */
> +        return 3;
>      }
> -    /* We don't currently implement the Virtualization or TrustZone
> -     * extensions, so PL2 and PL3 don't exist for us.
> +    /* We currently do not implement the Virtualization extensions, so PL2 does
> +     * not exist for us.
>       */
>      return 1;
>  }

This worries me a bit, because I suspect we have code that's
treating arm_current_pl() as if it were arm_current_el(), ie that
Secure EL1 will return 1, not 3. Perhaps we need to have
both functions and check that all the callers are using the
right one?

thanks
-- PMM