Re: [PATCH v3 25/60] target/arm: Reorg CPAccessResult and access_check_cp_reg

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> Rearrange the values of the enumerators of CPAccessResult
> so that we may directly extract the target el. For the two
> special cases in access_check_cp_reg, use CPAccessResult.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/cpregs.h    | 26 ++++++++++++--------
>  target/arm/op_helper.c | 56 +++++++++++++++++++++---------------------
>  2 files changed, 44 insertions(+), 38 deletions(-)
>
> diff --git a/target/arm/cpregs.h b/target/arm/cpregs.h
> index 005aa2d3a5..700fcc1478 100644
> --- a/target/arm/cpregs.h
> +++ b/target/arm/cpregs.h
> @@ -167,26 +167,32 @@ static inline bool cptype_valid(int cptype)
>  typedef enum CPAccessResult {
>      /* Access is permitted */
>      CP_ACCESS_OK = 0,
> +
> +    /*
> +     * Combined with one of the following, the low 2 bits indicate the
> +     * target exception level.  If 0, the exception is taken to the usual
> +     * target EL (EL1 or PL1 if in EL0, otherwise to the current EL).
> +     */
> +    CP_ACCESS_EL_MASK = 3,
> +
>      /*
>       * Access fails due to a configurable trap or enable which would
>       * result in a categorized exception syndrome giving information about
>       * the failing instruction (ie syndrome category 0x3, 0x4, 0x5, 0x6,
> -     * 0xc or 0x18). The exception is taken to the usual target EL (EL1 or
> -     * PL1 if in EL0, otherwise to the current EL).
> +     * 0xc or 0x18).
>       */
> -    CP_ACCESS_TRAP = 1,
> +    CP_ACCESS_TRAP = (1 << 2),
> +    CP_ACCESS_TRAP_EL2 = CP_ACCESS_TRAP | 2,
> +    CP_ACCESS_TRAP_EL3 = CP_ACCESS_TRAP | 3,
> +
>      /*
>       * Access fails and results in an exception syndrome 0x0 ("uncategorized").
>       * Note that this is not a catch-all case -- the set of cases which may
>       * result in this failure is specifically defined by the architecture.
>       */
> -    CP_ACCESS_TRAP_UNCATEGORIZED = 2,
> -    /* As CP_ACCESS_TRAP, but for traps directly to EL2 or EL3 */
> -    CP_ACCESS_TRAP_EL2 = 3,
> -    CP_ACCESS_TRAP_EL3 = 4,
> -    /* As CP_ACCESS_UNCATEGORIZED, but for traps directly to EL2 or EL3 */
> -    CP_ACCESS_TRAP_UNCATEGORIZED_EL2 = 5,
> -    CP_ACCESS_TRAP_UNCATEGORIZED_EL3 = 6,
> +    CP_ACCESS_TRAP_UNCATEGORIZED = (2 << 2),
> +    CP_ACCESS_TRAP_UNCATEGORIZED_EL2 = CP_ACCESS_TRAP_UNCATEGORIZED | 2,
> +    CP_ACCESS_TRAP_UNCATEGORIZED_EL3 = CP_ACCESS_TRAP_UNCATEGORIZED | 3,
>  } CPAccessResult;

This does feel like we are moving from an enum to a bunch of #defines
for bitfields. I guess we keep type checking though....

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


-- 
Alex Bennée