Re: [Qemu-devel] [PATCH v2 07/11] target-arm: Split AArch64 cases out of ats_write()

[Edgar E. Iglesias]

On Thu, Jan 29, 2015 at 06:55:13PM +0000, Peter Maydell wrote:
> Instead of simply reusing ats_write() as the handler for both AArch32
> and AArch64 address translation operations, use a different function
> for each with the common code in a third function. This is necessary
> because the semantics for selecting the right translation regime are
> different; we are only getting away with sharing currently because
> we don't support EL2 and only support EL3 in AArch32.
> 
> Signed-off-by: Peter Maydell <address@hidden>
> Reviewed-by: Greg Bellows <address@hidden>

Sounds good:

Reviewed-by: Edgar E. Iglesias <address@hidden>


> ---
>  target-arm/helper.c | 33 ++++++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 06478d8..04bc0a1 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1435,13 +1435,13 @@ static CPAccessResult ats_access(CPUARMState *env, 
> const ARMCPRegInfo *ri)
>      return CP_ACCESS_OK;
>  }
>  
> -static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
> +static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
> +                             int access_type, int is_user)
>  {
>      hwaddr phys_addr;
>      target_ulong page_size;
>      int prot;
> -    int ret, is_user = ri->opc2 & 2;
> -    int access_type = ri->opc2 & 1;
> +    int ret;
>      uint64_t par64;
>  
>      ret = get_phys_addr(env, value, access_type, is_user,
> @@ -1481,9 +1481,28 @@ static void ats_write(CPUARMState *env, const 
> ARMCPRegInfo *ri, uint64_t value)
>                      ((ret & 0xf) << 1) | 1;
>          }
>      }
> +    return par64;
> +}
> +
> +static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
> +{
> +    int is_user = ri->opc2 & 2;
> +    int access_type = ri->opc2 & 1;
> +    uint64_t par64;
> +
> +    par64 = do_ats_write(env, value, access_type, is_user);
>  
>      A32_BANKED_CURRENT_REG_SET(env, par, par64);
>  }
> +
> +static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
> +                        uint64_t value)
> +{
> +    int is_user = ri->opc2 & 2;
> +    int access_type = ri->opc2 & 1;
> +
> +    env->cp15.par_el[1] = do_ats_write(env, value, access_type, is_user);
> +}
>  #endif
>  
>  static const ARMCPRegInfo vapa_cp_reginfo[] = {
> @@ -2257,16 +2276,16 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>      /* 64 bit address translation operations */
>      { .name = "AT_S1E1R", .state = ARM_CP_STATE_AA64,
>        .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 0,
> -      .access = PL1_W, .type = ARM_CP_NO_MIGRATE, .writefn = ats_write },
> +      .access = PL1_W, .type = ARM_CP_NO_MIGRATE, .writefn = ats_write64 },
>      { .name = "AT_S1E1W", .state = ARM_CP_STATE_AA64,
>        .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 1,
> -      .access = PL1_W, .type = ARM_CP_NO_MIGRATE, .writefn = ats_write },
> +      .access = PL1_W, .type = ARM_CP_NO_MIGRATE, .writefn = ats_write64 },
>      { .name = "AT_S1E0R", .state = ARM_CP_STATE_AA64,
>        .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 2,
> -      .access = PL1_W, .type = ARM_CP_NO_MIGRATE, .writefn = ats_write },
> +      .access = PL1_W, .type = ARM_CP_NO_MIGRATE, .writefn = ats_write64 },
>      { .name = "AT_S1E0W", .state = ARM_CP_STATE_AA64,
>        .opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 3,
> -      .access = PL1_W, .type = ARM_CP_NO_MIGRATE, .writefn = ats_write },
> +      .access = PL1_W, .type = ARM_CP_NO_MIGRATE, .writefn = ats_write64 },
>  #endif
>      /* TLB invalidate last level of translation table walk */
>      { .name = "TLBIMVALIS", .cp = 15, .opc1 = 0, .crn = 8, .crm = 3, .opc2 = 
> 5,
> -- 
> 1.9.1
>