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 <alex.bennee@linaro.org>
---
 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;
+    }
+
     /*
      * Per R_XCHFJ, alignment fault not due to memory type has
      * highest precedence.  Otherwise, walk the page table and
-- 
2.47.3

On Thu, 4 Dec 2025 at 15:54, Alex Bennée <alex.bennee@linaro.org> 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 <alex.bennee@linaro.org>
> ---
>  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

On Thu, Dec 04, 2025 at 03:54:27PM +0000, Alex Bennée 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 <alex.bennee@linaro.org>

Seems to give a PC alignment fault when adapting both pcalign-a64.c
(branching to 0x1) and pcalign-a32.c (branching to 0x2) where previously
it gave an instruction abort. Whether there are other unintended
consequences of checking for the fault here I will leave to others
(which is in part why I filed an issue for this rather than naively
making a similar change myself).

Tested-by: Jessica Clarke <jrtc27@jrtc27.com>