From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35295) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVLld-0006Pq-Ee for qemu-devel@nongnu.org; Tue, 10 Mar 2015 11:08:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVLlX-0007Sn-PP for qemu-devel@nongnu.org; Tue, 10 Mar 2015 11:08:21 -0400 Received: from mail-la0-f52.google.com ([209.85.215.52]:40209) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVLlX-0007SL-Is for qemu-devel@nongnu.org; Tue, 10 Mar 2015 11:08:15 -0400 Received: by labge10 with SMTP id ge10so2350889lab.7 for ; Tue, 10 Mar 2015 08:08:14 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1423753507-30542-2-git-send-email-drjones@redhat.com> References: <1423753507-30542-1-git-send-email-drjones@redhat.com> <1423753507-30542-2-git-send-email-drjones@redhat.com> From: Peter Maydell Date: Tue, 10 Mar 2015 15:07:54 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 1/5] target-arm: convert check_ap to get_rw_prot List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrew Jones Cc: QEMU Developers On 12 February 2015 at 15:05, Andrew Jones 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 > --- > 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 -- PMM