On 12 February 2015 at 15:05, Andrew Jones <drjo...@redhat.com> wrote:
> Teach get_rw_prot about the simple AP format AP[2:1]. An additional
> switch was added, as opposed to converting ap := AP[2:1] to AP[2:0]
> with a simple shift - and then modifying cases 0,2,4,6, because the
> resulting code is easier to read with the switch.
>
> Signed-off-by: Andrew Jones <drjo...@redhat.com>
> ---
> target-arm/helper.c | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 610f305c4d661..b63ec7b7979f9 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -4698,12 +4698,32 @@ static inline bool regime_is_user(CPUARMState *env,
> ARMMMUIdx mmu_idx)
> static inline int get_rw_prot(CPUARMState *env, ARMMMUIdx mmu_idx,
> int ap, int domain_prot)
> {
> + bool simple_ap = regime_using_lpae_format(env, mmu_idx)
> + || (regime_sctlr(env, mmu_idx) & SCTLR_AFE);
We should check arm_feature(env, ARM_FEATURE_V6K) && (SCTLR.AFE is set);
that bit isn't defined til v6K.
> + bool domain_prot_valid = !regime_using_lpae_format(env, mmu_idx);
Given that the lpae code path is totally separate (and not even
calling this function yet), can't you just have it pass in a
zero domain_prot ? Or have the callers do the domain protection
check themselves...
> bool is_user = regime_is_user(env, mmu_idx);
>
> - if (domain_prot == 3) {
> + if (domain_prot_valid && domain_prot == 3) {
> return PAGE_READ | PAGE_WRITE;
> }
>
> + /* ap is AP[2:1] */
> + if (simple_ap) {
> + switch (ap) {
> + case 0:
> + return is_user ? 0 : PAGE_READ | PAGE_WRITE;
> + case 1:
> + return PAGE_READ | PAGE_WRITE;
> + case 2:
> + return is_user ? 0 : PAGE_READ;
> + case 3:
> + return PAGE_READ;
> + default:
> + g_assert_not_reached();
> + }
> + }
I'm confused. Even if we're using the simple-permissions
model, the ap parameter is still AP[2:0]. Shouldn't this
switch be for cases 0, 2, 4, 6 ?
> +
> + /* ap is AP[2:0] */
> switch (ap) {
> case 0:
> if (arm_feature(env, ARM_FEATURE_V7)) {
> --
> 1.9.3
>
-- PMM
