Re: [Qemu-arm] [PATCH] target-arm: fix MMU index in arm_cpu_do_unaligned_access

[Peter Maydell]

On 15 January 2016 at 09:59, Alvise Rigo <address@hidden> wrote:
> arm_regime_using_lpae_format checks whether the LPAE extension is used
> for stage 1 translation regimes. MMU indexes not exclusively of a stage 1
> regime won't work with this method.
>
> In case of ARMMMUIdx_S12NSE0 or ARMMMUIdx_S12NSE1, offset these values
> by ARMMMUIdx_S1NSE0 to get the right index indicating a stage 1
> translation regime.
>
> Rename also the function to arm_s1_regime_using_lpae_format and update
> the comments to reflect the change.
>
> Signed-off-by: Alvise Rigo <address@hidden>
> ---
>  target-arm/helper.c    | 8 ++++----
>  target-arm/internals.h | 5 +++--
>  target-arm/op_helper.c | 8 ++++++--
>  3 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 59d5a41..8317ff5 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -5996,10 +5996,10 @@ static inline bool 
> regime_using_lpae_format(CPUARMState *env,
>      return false;
>  }
>
> -/* Returns true if the translation regime is using LPAE format page tables.
> - * Used when raising alignment exceptions, whose FSR changes depending on
> - * whether the long or short descriptor format is in use. */
> -bool arm_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
> +/* Returns true if the stage 1 translation regime is using LPAE format page
> + * tables. Used when raising alignment exceptions, whose FSR changes 
> depending
> + * on whether the long or short descriptor format is in use. */
> +bool arm_s1_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx)
>  {
>      return regime_using_lpae_format(env, mmu_idx);
>  }
> diff --git a/target-arm/internals.h b/target-arm/internals.h
> index b925aaa..d226bbe 100644
> --- a/target-arm/internals.h
> +++ b/target-arm/internals.h
> @@ -441,8 +441,9 @@ struct ARMMMUFaultInfo {
>  bool arm_tlb_fill(CPUState *cpu, vaddr address, int rw, int mmu_idx,
>                    uint32_t *fsr, ARMMMUFaultInfo *fi);
>
> -/* Return true if the translation regime is using LPAE format page tables */
> -bool arm_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx);
> +/* Return true if the stage 1 translation regime is using LPAE format page
> + * tables */
> +bool arm_s1_regime_using_lpae_format(CPUARMState *env, ARMMMUIdx mmu_idx);
>
>  /* Raise a data fault alignment exception for the specified virtual address 
> */
>  void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, int is_write,
> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
> index e42d287..ccc505d 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -133,7 +133,7 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr 
> vaddr, int is_write,
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
> -    int target_el;
> +    int target_el, mmu_idx;
>      bool same_el;
>
>      if (retaddr) {
> @@ -146,10 +146,14 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr 
> vaddr, int is_write,
>
>      env->exception.vaddress = vaddr;
>
> +    mmu_idx = cpu_mmu_index(env, false);
> +    if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) {
> +        mmu_idx += ARMMMUIdx_S1NSE0;
> +    }

I would let the arm_s1_regime_using_lpae_format() function do this conversion
from the S12 index to the S1 index.

Otherwise this looks good to me.

>      /* the DFSR for an alignment fault depends on whether we're using
>       * the LPAE long descriptor format, or the short descriptor format
>       */
> -    if (arm_regime_using_lpae_format(env, cpu_mmu_index(env, false))) {
> +    if (arm_s1_regime_using_lpae_format(env, mmu_idx)) {
>          env->exception.fsr = 0x21;
>      } else {
>          env->exception.fsr = 0x1;
> --
> 2.7.0

thanks
-- PMM