Re: [Qemu-devel] [PATCH 05/11] target/arm: Default handling of BTYPE during translation

[Peter Maydell]

On Thu, 10 Jan 2019 at 12:17, Richard Henderson
<address@hidden> wrote:
>
> The branch target exception for guarded pages has high priority,
> and only 8 instructions are valid for that case.  Perform this
> check before doing any other decode.
>
> Clear BTYPE after all insns that neither set BTYPE nor exit via
> exception (DISAS_NORETURN).
>
> Not yet handled are insns that exit via DISAS_NORETURN for some
> other reason, like direct branches.
> diff --git a/target/arm/translate.h b/target/arm/translate.h
> index 3d5e8bacac..f73939d7b4 100644
> --- a/target/arm/translate.h
> +++ b/target/arm/translate.h
> @@ -71,8 +71,13 @@ typedef struct DisasContext {
>      bool pauth_active;
>      /* True with v8.5-BTI and SCTLR_ELx.BT* set.  */
>      bool bt;
> -    /* A copy of PSTATE.BTYPE, which will be 0 without v8.5-BTI.  */
> -    uint8_t btype;
> +    /*
> +     * >= 0, a copy of PSTATE.BTYPE, which will be 0 without v8.5-BTI.
> +     *  < 0, set by the current instruction.
> +     */
> +    int8_t btype;

You could have made this int8_t to start with...

> +    /* True if this page is guarded.  */
> +    bool guarded_page;
>      /* Bottom two bits of XScale c15_cpar coprocessor access control reg */
>      int c15_cpar;
>      /* TCG op of the current insn_start.  */
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index ca2ae40701..68eb27089a 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -128,6 +128,16 @@ static inline int get_a64_user_mem_index(DisasContext *s)
>      return arm_to_core_mmu_idx(useridx);
>  }
>
> +static void reset_btype(DisasContext *s)
> +{
> +    if (s->btype != 0) {
> +        TCGv_i32 zero = tcg_const_i32(0);
> +        tcg_gen_st_i32(zero, cpu_env, offsetof(CPUARMState, btype));
> +        tcg_temp_free_i32(zero);
> +        s->btype = 0;
> +    }
> +}
> +
>  void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
>                              fprintf_function cpu_fprintf, int flags)
>  {
> @@ -13716,6 +13726,90 @@ static void disas_data_proc_simd_fp(DisasContext *s, 
> uint32_t insn)
>      }
>  }
>
> +/**
> + * is_guarded_page:
> + * @env: The cpu environment
> + * @s: The DisasContext
> + *
> + * Return true if the page is guarded.
> + */
> +static bool is_guarded_page(CPUARMState *env, DisasContext *s)
> +{
> +#ifdef CONFIG_USER_ONLY
> +    return false;  /* FIXME */
> +#else
> +    uint64_t addr = s->base.pc_first;
> +    int mmu_idx = arm_to_core_mmu_idx(s->mmu_idx);
> +    unsigned int index = tlb_index(env, mmu_idx, addr);
> +    CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
> +
> +    /*
> +     * We test this immediately after reading an insn, which means
> +     * that any normal page must be in the TLB.  The only exception
> +     * would be for executing from flash or device memory, which
> +     * does not retain the TLB entry.
> +     *
> +     * FIXME: Assume false for those, for now.  We could use
> +     * arm_cpu_get_phys_page_attrs_debug to re-read the page
> +     * table entry even for that case.
> +     */
> +    return (tlb_hit(entry->addr_code, addr) &&
> +            env->iotlb[mmu_idx][index].attrs.guarded);
> +#endif
> +}
> +
> +/**
> + * btype_destination_ok:
> + * @insn: The instruction at the branch destination
> + * @bt: SCTLR_ELx.BT
> + * @btype: PSTATE.BTYPE, and is non-zero
> + *
> + * On a guarded page, there are a limited number of insns
> + * that may be present at the branch target:
> + *   - branch target identifiers,
> + *   - paciasp, pacibsp,
> + *   - BRK insn
> + *   - HLT insn
> + * Anything else causes a Branch Target Exception.
> + *
> + * Return true if the branch is compatible, false to raise BTITRAP.
> + */
> +static bool btype_destination_ok(uint32_t insn, bool bt, int btype)
> +{
> +    if ((insn & 0xfffff01fu) == 0xd503201fu) {
> +        /* HINT space */
> +        switch (extract32(insn, 5, 7)) {
> +        case 031: /* PACIASP */
> +        case 033: /* PACIBSP */

Octal again...

> +            /*
> +             * If SCTLR_ELx.BT, then PACI*SP are not compatible
> +             * with btype == 3.  Otherwise all btype are ok.
> +             */
> +            return !bt || btype != 3;
> +        case 040: /* BTI */
> +            /* Not compatible with any btype.  */
> +            return false;
> +        case 042: /* BTI c */
> +            /* Not compatible with btype == 3 */
> +            return btype != 3;
> +        case 044: /* BTI j */
> +            /* Not compatible with btype == 2 */
> +            return btype != 2;
> +        case 046: /* BTI jc */
> +            /* Compatible with any btype.  */
> +            return true;
> +        }

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM