On Thu, 4 Dec 2025 at 15:54, Alex Bennée <[email protected]> wrote:
>
> PC alignment faults have priority over instruction aborts and we have
> code to deal with this in the translation front-ends. However during
> tb_lookup we can see a potentially faulting probe which doesn't get a
> MemOp set. If the page isn't available this results in
> EC_INSNABORT (0x20) instead of EC_PCALIGNMENT (0x22).
>
> As there is no easy way to set the appropriate MemOp in the
> instruction fetch probe path lets just detect it in
> arm_cpu_tlb_fill_align() and set memop appropriately.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3233
> Signed-off-by: Alex Bennée <[email protected]>
> ---
> target/arm/tcg/tlb_helper.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/target/arm/tcg/tlb_helper.c b/target/arm/tcg/tlb_helper.c
> index f1983a5732e..78c85cb3306 100644
> --- a/target/arm/tcg/tlb_helper.c
> +++ b/target/arm/tcg/tlb_helper.c
> @@ -345,6 +345,17 @@ bool arm_cpu_tlb_fill_align(CPUState *cs,
> CPUTLBEntryFull *out, vaddr address,
> fi = memset(&local_fi, 0, sizeof(local_fi));
> }
>
> + /*
> + * PC alignment faults should be dealt with at translation time
> + * but we also need to make sure other faults don't preempt them
> + * while being probed.
> + */
> + if (access_type == MMU_INST_FETCH && !cpu->env.thumb) {
> + /* probe sets memop to 0 */
> + g_assert(!memop);
> + memop |= MO_ALIGN_4;
> + }
I see that this does in practice get the right syndrome,
but I don't entirely understand why. Looking at the code,
what it seems like will happen is we'll come in here, set
the memop align bits, then that will trip the
existing check on address alignment if the PC isn't
4-aligned. So we'll set fi->type = ARMFault_Alignment,
and then (assuming not a mere probe) call
arm_deliver_fault(), which always uses syn_insn_abort()
for MMU_INST_FETCH.
I think this must be working because in practice we
call it once as a probe, and then the translate.c
code catches the misaligned-PC case before we get to
the point of doing a non-probe load. But the result
is that this function will do the wrong thing if it
ever is called for a non-probe load.
Plus we pass a different memop into get_phys_addr(), which
might do something unexpected as a result.
Maybe we should instead do:
if (access_type == MMU_INST_FETCH && !cpu->env.thumb &&
(address & 3)) {
fi->type = ARMFault_Alignment;
} else if (address & ....) {
fi->type = ARMFault_Alignment;
} else if (!get_phys_addr(...)) {
...
}
plus have arm_deliver_fault() do
if (access_type == MMU_INST_FETCH) {
if (fi->type = ARMFault_Alignment) {
syn = syn_pcalignment();
} else {
syn = syn_insn_abort(same_el, fi->ea, fi->s1ptw, fsc);
}
exc = EXCP_PREFETCH_ABORT;
} ...
?
thanks
-- PMM
