On 12 February 2015 at 15:05, Andrew Jones <drjo...@redhat.com> 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 <drjo...@redhat.com>
> ---
> 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 <peter.mayd...@linaro.org>
-- PMM
