Re: [PATCH v1 14/19] target/arm: Check alignment in helper_mte_check

[Peter Maydell]

On Thu, 16 Feb 2023 at 03:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Fixes a bug in that with SCTLR.A set, we should raise any
> alignment fault before raising any MTE check fault.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/internals.h     |  3 ++-
>  target/arm/mte_helper.c    | 18 ++++++++++++++++++
>  target/arm/translate-a64.c |  2 ++
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index e1e018da46..fa264e368c 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1222,7 +1222,8 @@ FIELD(MTEDESC, MIDX,  0, 4)
>  FIELD(MTEDESC, TBI,   4, 2)
>  FIELD(MTEDESC, TCMA,  6, 2)
>  FIELD(MTEDESC, WRITE, 8, 1)
> -FIELD(MTEDESC, SIZEM1, 9, SIMD_DATA_BITS - 9)  /* size - 1 */
> +FIELD(MTEDESC, ALIGN, 9, 3)
> +FIELD(MTEDESC, SIZEM1, 12, SIMD_DATA_BITS - 12)  /* size - 1 */
>
>  bool mte_probe(CPUARMState *env, uint32_t desc, uint64_t ptr);
>  uint64_t mte_check(CPUARMState *env, uint32_t desc, uint64_t ptr, uintptr_t 
> ra);
> diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
> index 98bcf59c22..e50bb4ea13 100644
> --- a/target/arm/mte_helper.c
> +++ b/target/arm/mte_helper.c
> @@ -784,6 +784,24 @@ uint64_t mte_check(CPUARMState *env, uint32_t desc, 
> uint64_t ptr, uintptr_t ra)
>
>  uint64_t HELPER(mte_check)(CPUARMState *env, uint32_t desc, uint64_t ptr)
>  {
> +    /*
> +     * In the Arm ARM pseudocode, the alignment check happens at the top
> +     * of Mem[], while the MTE check happens later in AArch64.MemSingle[].
> +     * Thus the alignment check has priority.
> +     * When the mte check is disabled, tcg performs the alignment check
> +     * during the code generated for the memory access.
> +     */

Also described in the text: the I_ZFGJP priority table lists
MTE faults at priority 33, basically lower than anything else
except an external abort.

Looking at the code, is this really the only case here where
we were mis-prioritizing tag check faults? Have we already
checked things like "no page table entry" and all the other
cases that can cause data aborts at this point?

thanks
-- PMM