Re: [Qemu-devel] [PATCH 1/5] target-arm: convert check_ap to get_rw_prot

[Peter Maydell]

On 12 February 2015 at 15:05, Andrew Jones <address@hidden> wrote:
> Instead of mixing access permission checking with access permissions
> to page protection flags translation, just do the translation, and
> leave it to the caller to check the protection flags against the access
> type. As this function only considers READ/WRITE, not EXEC, then name
> it accordingly.
>
> Signed-off-by: Andrew Jones <address@hidden>
> ---
>  target-arm/helper.c | 47 +++++++++++++++++------------------------------
>  1 file changed, 17 insertions(+), 30 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 1a1a00577e780..610f305c4d661 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -4692,34 +4692,23 @@ static inline bool regime_is_user(CPUARMState *env, 
> ARMMMUIdx mmu_idx)
>      }
>  }
>
> -/* Check section/page access permissions.
> -   Returns the page protection flags, or zero if the access is not
> -   permitted.  */
> -static inline int check_ap(CPUARMState *env, ARMMMUIdx mmu_idx,
> -                           int ap, int domain_prot,
> -                           int access_type)
> -{
> -    int prot_ro;
> +/* Translate section/page access permissions to page
> + * R/W protection flags
> + */

Given that the 'ap' parameter isn't just the AP bits this
could use a mention in the comment:

/* Translate section/page access permissions to page
 * R/W protection flags. The 'ap' parameter is the concatenation
 * of the APX:AP bits (with APX zero for the descriptor formats
 * which don't have it).
 */

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

-- PMM