From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46170) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVNdg-0000X3-DZ for qemu-devel@nongnu.org; Tue, 10 Mar 2015 13:08:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVNdc-0003Vr-So for qemu-devel@nongnu.org; Tue, 10 Mar 2015 13:08:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39888) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVNdc-0003Vm-L6 for qemu-devel@nongnu.org; Tue, 10 Mar 2015 13:08:12 -0400 Date: Tue, 10 Mar 2015 18:08:05 +0100 From: Andrew Jones Message-ID: <20150310170805.GG6320@hawk.usersys.redhat.com> References: <1423753507-30542-1-git-send-email-drjones@redhat.com> <1423753507-30542-6-git-send-email-drjones@redhat.com> <20150212170827.GA10193@hawk.usersys.redhat.com> <20150310165410.GD6320@hawk.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 5/5] target-arm: apply get_S1prot to get_phys_addr_v6 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers On Tue, Mar 10, 2015 at 05:03:48PM +0000, Peter Maydell wrote: > On 10 March 2015 at 16:54, Andrew Jones wrote: > > On Tue, Mar 10, 2015 at 03:57:27PM +0000, Peter Maydell wrote: > >> On 12 February 2015 at 17:08, Andrew Jones wrote: > >> > Actually, I should point out that this isn't just a cleanup, but > >> > also a fix. See below. > >> > >> > The original code didn't take into account that it may be calling check_ap > >> > with a simple AP, AP[2:1]. > >> > >> No, because check_ap() always takes AP[2:0]... > > > > No, it's really wrong. It's not the 2 vs. 3 bit issue that's the > > problem, it's the cases. You snipped most of my reply to myself. > > This part is pertinent > > > >> As a simple AP wouldn't be properly translated to protection flags with > >> check_ap (except for case 6), then I think this should have caused some > >> problems. Maybe this path just hasn't been tested? I don't see CR_AFE > >> getting used by Linux, so possibly not. > > > > So, yes, a simple (3-bit) ap would be handled by the 8-case switch with > > cases 0, 2, 4, and 6, but only case 6 would give the correct result. > > Well, we didn't correctly support the simple permission model at all > before your patches. But the point is that check_ap() always takes > AP[2:0] regardless. > > Hmm, or you could have check_simple_ap() be totally separate > from check_ap(), and call it from inside the check in the v6 > code path for AFE that we already have. Agreed. Creating a check_simple_ap function for the new switch added by 2/5 would look better. drew