Re: [Qemu-devel] [PATCH 5/5] target-arm: apply get_S1prot to get_phys_addr_v6

[Peter Maydell]

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