On Tue, Mar 10, 2015 at 05:14:21PM +0000, Peter Maydell wrote: > On 10 March 2015 at 17:02, Andrew Jones <drjo...@redhat.com> wrote: > > On Tue, Mar 10, 2015 at 04:55:53PM +0000, Peter Maydell wrote: > >> On 10 March 2015 at 16:48, Andrew Jones <drjo...@redhat.com> wrote: > >> > On Tue, Mar 10, 2015 at 03:56:11PM +0000, Peter Maydell wrote: > >> > >> >> For instance, you're missing a shift here on the ap bits, because > >> >> get_rw_prot needs AP[2:0] and 'ap' here is AP[2:1]. > >> > > >> > Don't need the shift because get_rw_prot supports the 2-bit format. > >> > >> No it doesn't... > > > > Yes it does :-) That's the support patch 2/5 adds. > > No, because patch 2 doesn't do anything in the callers to > make them pass only bits [2:1]. So after patch 2 get_rw_prot > still requires bits [2:0]. Except it's broken, because the > function itself assumes it gets bits [2:1].
You've lost me. Patch 2 adds support for 2-bit ap, but doesn't remove support for 3-bit. There are not callers expecting it to support the simple model as 2 or 3 bits at that time, except v6, but that was broken already, and we fix it in patch 5 (and we add the ap shift there too). IOW, how can preparing a function for new callers, while still supporting the old callers, be 'broken'? > > Having thought a bit more about it, I think we should > just have two totally separate functions for the old > style and simplified permission checks, because we can > always call the correct one (always old for v5, either > one for v6 since we already have the SCTLR.AFE check, > and the simplified one for the lpae code path). I like that. Thanks, drew
