On Mon, 20 Sept 2021 at 03:48, Richard Henderson <richard.hender...@linaro.org> wrote: > > For A64, any input to an indirect branch can cause this. > > For A32, many indirect branch paths force the branch to be aligned, > but BXWritePC does not. This includes the BX instruction but also > other interworking changes to PC. Prior to v8, this case is UNDEFINED. > With v8, this is CONSTRAINED UNPREDICTABLE and may either raise an > exception or force align the PC. > > We choose to raise an exception because we have the infrastructure, > it makes the generated code for gen_bx simpler, and it has the > possibility of catching more guest bugs.
> +void helper_exception_pc_alignment(CPUARMState *env, target_ulong pc) > +{ > + int target_el = exception_target_el(env); > + > + if (target_el == 2 || arm_el_is_aa64(env, target_el)) { > + /* > + * To aarch64 and aarch32 el2, pc alignment has a > + * special exception class. > + */ > + env->exception.vaddress = pc; > + env->exception.fsr = 0; > + raise_exception(env, EXCP_PREFETCH_ABORT, syn_pcalignment(), > target_el); > + } else { > + /* > + * To aarch32 el1, pc alignment is like data alignment > + * except with a prefetch abort. > + */ > + ARMMMUFaultInfo fi = { .type = ARMFault_Alignment }; > + arm_deliver_fault(env_archcpu(env), pc, MMU_INST_FETCH, > + cpu_mmu_index(env, true), &fi); > + } I still don't believe that you need to special case AArch32-non-Hyp like this. Can you expand on what you think the difference is? > +} > + > #if !defined(CONFIG_USER_ONLY) > > /* > diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c > index ab6b346e35..8c72e37de3 100644 > --- a/target/arm/translate-a64.c > +++ b/target/arm/translate-a64.c > @@ -14752,8 +14752,10 @@ static void > aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) > { > DisasContext *s = container_of(dcbase, DisasContext, base); > CPUARMState *env = cpu->env_ptr; > + uint64_t pc = s->base.pc_next; > uint32_t insn; > > + /* Singlestep exceptions have the highest priority. */ > if (s->ss_active && !s->pstate_ss) { > /* Singlestep state is Active-pending. > * If we're in this state at the start of a TB then either > @@ -14768,13 +14770,28 @@ static void > aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) > assert(s->base.num_insns == 1); > gen_swstep_exception(s, 0, 0); > s->base.is_jmp = DISAS_NORETURN; > + s->base.pc_next = pc + 4; Why are we adding this in this patch? Isn't this a separate bugfix ? > return; > } thanks -- PMM