[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 06/23] target/arm: Make HSTR_EL2 traps take priority over UND
From: |
Peter Maydell |
Subject: |
Re: [PATCH 06/23] target/arm: Make HSTR_EL2 traps take priority over UNDEF-at-EL1 |
Date: |
Sat, 28 Jan 2023 14:34:34 +0000 |
On Sat, 28 Jan 2023 at 01:47, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 1/27/23 07:54, Peter Maydell wrote:
> > +void HELPER(hstr_trap_check)(CPUARMState *env, uint32_t mask, uint32_t
> > syndrome)
> > +{
> > + if (env->cp15.hstr_el2 & mask) {
> > + raise_exception(env, EXCP_UDEF, syndrome, 2);
> > + }
>
> This is so simple...
>
>
> > @@ -4760,6 +4761,28 @@ static void do_coproc_insn(DisasContext *s, int
> > cpnum, int is64,
> > break;
> > }
> >
> > + if (s->hstr_active && cpnum == 15 && s->current_el == 1) {
> > + /*
> > + * At EL1, check for a HSTR_EL2 trap, which must take precedence
> > + * over the UNDEF for "no such register" or the UNDEF for "access
> > + * permissions forbid this EL1 access". HSTR_EL2 traps from EL0
> > + * only happen if the cpreg doesn't UNDEF at EL0, so we do those in
> > + * access_check_cp_reg(), after the checks for whether the access
> > + * configurably trapped to EL1.
> > + */
> > + uint32_t maskbit = is64 ? crm : crn;
> > +
> > + if (maskbit != 4 && maskbit != 14) {
> > + /* T4 and T14 are RES0 so never cause traps */
> > + gen_set_condexec(s);
> > + gen_update_pc(s, 0);
> > + emitted_update_pc = true;
> > + gen_helper_hstr_trap_check(cpu_env,
> > + tcg_constant_i32(1 << maskbit),
> > + tcg_constant_i32(syndrome));
> > + }
>
> How about
>
> if (maskbit...) {
> TCGv_i32 t = load_cpu_offset(offsetoflow32(CPUARMState, hstr_el2));
> DisasLabel *over = gen_disas_label(s);
>
> tcg_gen_andi_i32(t, t, 1u << maskbit);
> tcg_gen_brcondi_i32(TCG_COND_EQ, t, 0, over.label);
> tcg_temp_free_i32(t);
>
> gen_exception_insn(s, 0, EXCP_UDEF, syndrome);
> set_disas_label(s, over);
> }
>
> which also eliminates the need for emitted_update_pc.
I really dislike use of brcond in generated TCG, because of the
massive beartrap it sets up where all your temporaries get nuked
but there's no compile-time checking that you didn't try to keep
using one after the brcond. So I generally prefer an approach that
avoids brcond over one that uses it, if it's available.
thanks
-- PMM
- [PATCH 02/23] target/arm: Correct syndrome for ATS12NSO* at Secure EL1, (continued)
- [PATCH 02/23] target/arm: Correct syndrome for ATS12NSO* at Secure EL1, Peter Maydell, 2023/01/27
- [PATCH 05/23] target/arm: All UNDEF-at-EL0 traps take priority over HSTR_EL2 traps, Peter Maydell, 2023/01/27
- [PATCH 03/23] target/arm: Remove CP_ACCESS_TRAP_UNCATEGORIZED_{EL2, EL3}, Peter Maydell, 2023/01/27
- [PATCH 11/23] target/arm: Mark up sysregs for HFGRTR bits 12..23, Peter Maydell, 2023/01/27
- [PATCH 06/23] target/arm: Make HSTR_EL2 traps take priority over UNDEF-at-EL1, Peter Maydell, 2023/01/27
- [PATCH 17/23] target/arm: Mark up sysregs for HFGITR bits 12..17, Peter Maydell, 2023/01/27
- [PATCH 18/23] target/arm: Mark up sysregs for HFGITR bits 18..47, Peter Maydell, 2023/01/27
- [PATCH 16/23] target/arm: Mark up sysregs for HFGITR bits 0..11, Peter Maydell, 2023/01/27
- [PATCH 09/23] target/arm: Implement FGT trapping infrastructure, Peter Maydell, 2023/01/27
- [PATCH 14/23] target/arm: Mark up sysregs for HDFGRTR bits 0..11, Peter Maydell, 2023/01/27