On 10 March 2015 at 16:54, Andrew Jones <drjo...@redhat.com> wrote: > On Tue, Mar 10, 2015 at 03:57:27PM +0000, Peter Maydell wrote: >> On 12 February 2015 at 17:08, Andrew Jones <drjo...@redhat.com> 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. -- PMM