Re: [Qemu-devel] [PATCH 4/5] target-arm: Enable the AArch32 ATS12NSO ops

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Fri, Jul 24, 2015 at 04:21:02PM +0100, Peter Maydell wrote:
> Apply the correct conditions in the ats_access() function for
> the ATS12NSO* address translation operations:
>  * succeed at EL2 or EL3
>  * normal UNDEF trap from NS EL1
>  * trap to EL3 from S EL1 (only possible if EL3 is AArch64)
> 
> (This change means they're now available in our EL3-supporting
> CPUs when they would previously always UNDEF.)
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> ---
>  target-arm/helper.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 1974fa6..67d108e 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1477,12 +1477,17 @@ static void par_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>  static CPAccessResult ats_access(CPUARMState *env, const ARMCPRegInfo *ri)
>  {
>      if (ri->opc2 & 4) {
> -        /* Other states are only available with TrustZone; in
> -         * a non-TZ implementation these registers don't exist
> -         * at all, which is an Uncategorized trap. This underdecoding
> -         * is safe because the reginfo is NO_RAW.
> +        /* The ATS12NSO* operations must trap to EL3 if executed in
> +         * Secure EL1 (which can only happen if EL3 is AArch64).
> +         * They are simply UNDEF if executed from NS EL1.
> +         * They function normally from EL2 or EL3.
>           */
> -        return CP_ACCESS_TRAP_UNCATEGORIZED;
> +        if (arm_current_el(env) == 1) {
> +            if (arm_is_secure_below_el3(env)) {
> +                return CP_ACCESS_TRAP_UNCATEGORIZED_EL3;
> +            }
> +            return CP_ACCESS_TRAP_UNCATEGORIZED;
> +        }
>      }
>      return CP_ACCESS_OK;
>  }
> @@ -1657,6 +1662,7 @@ static const ARMCPRegInfo vapa_cp_reginfo[] = {
>                               offsetoflow32(CPUARMState, cp15.par_ns) },
>        .writefn = par_write },
>  #ifndef CONFIG_USER_ONLY
> +    /* This underdecoding is safe because the reginfo is NO_RAW. */
>      { .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY,
>        .access = PL1_W, .accessfn = ats_access,
>        .writefn = ats_write, .type = ARM_CP_NO_RAW },
> -- 
> 1.9.1
>