[PATCH v3] target/arm: handle unaligned PC during tlb probe

Alex Bennée posted 1 patch 6 days, 22 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20251204203540.1381896-1-alex.bennee@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/tcg/tlb_helper.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
[PATCH v3] target/arm: handle unaligned PC during tlb probe
Posted by Alex Bennée 6 days, 22 hours ago
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() ahead of the main alignment check. We also
teach arm_deliver_fault to deliver the right syndrome for
MMU_INST_FETCH alignment issues.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3233
Tested-by: Jessica Clarke <jrtc27@jrtc27.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v2
  - don't mess with MemOp for alignment check
  - expand arm_deliver_fault to pick up alignment issues
v3
  - update commit message
---
 target/arm/tcg/tlb_helper.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/target/arm/tcg/tlb_helper.c b/target/arm/tcg/tlb_helper.c
index f1983a5732e..5c689d3b69f 100644
--- a/target/arm/tcg/tlb_helper.c
+++ b/target/arm/tcg/tlb_helper.c
@@ -250,7 +250,11 @@ void arm_deliver_fault(ARMCPU *cpu, vaddr addr,
     fsr = compute_fsr_fsc(env, fi, target_el, mmu_idx, &fsc);
 
     if (access_type == MMU_INST_FETCH) {
-        syn = syn_insn_abort(same_el, fi->ea, fi->s1ptw, fsc);
+        if (fi->type == ARMFault_Alignment) {
+            syn = syn_pcalignment();
+        } else {
+            syn = syn_insn_abort(same_el, fi->ea, fi->s1ptw, fsc);
+        }
         exc = EXCP_PREFETCH_ABORT;
     } else {
         bool gcs = regime_is_gcs(core_to_arm_mmu_idx(env, mmu_idx));
@@ -346,11 +350,18 @@ bool arm_cpu_tlb_fill_align(CPUState *cs, CPUTLBEntryFull *out, vaddr address,
     }
 
     /*
-     * Per R_XCHFJ, alignment fault not due to memory type has
-     * highest precedence.  Otherwise, walk the page table and
-     * and collect the page description.
+     * PC alignment faults should be dealt with at translation time
+     * but we also need to catch them while being probed.
+     *
+     * Then per R_XCHFJ, alignment fault not due to memory type take
+     * precedence. Otherwise, walk the page table and and collect the
+     * page description.
+     *
      */
-    if (address & ((1 << memop_alignment_bits(memop)) - 1)) {
+    if (access_type == MMU_INST_FETCH && !cpu->env.thumb &&
+        (address & 3)) {
+        fi->type = ARMFault_Alignment;
+    } else if (address & ((1 << memop_alignment_bits(memop)) - 1)) {
         fi->type = ARMFault_Alignment;
     } else if (!get_phys_addr(&cpu->env, address, access_type, memop,
                               core_to_arm_mmu_idx(&cpu->env, mmu_idx),
-- 
2.47.3


Re: [PATCH v3] target/arm: handle unaligned PC during tlb probe
Posted by Michael Tokarev 1 day, 8 hours ago
On 12/4/25 23:35, 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() ahead of the main alignment check. We also
> teach arm_deliver_fault to deliver the right syndrome for
> MMU_INST_FETCH alignment issues.
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3233
> Tested-by: Jessica Clarke <jrtc27@jrtc27.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

This feels like a qemu-stable material (for all active stable
branches).

I'm picking it up for 10.0.x and 10.1.x.  Please let me know
if I shouldn't.

Thanks,

/mjt

Re: [PATCH v3] target/arm: handle unaligned PC during tlb probe
Posted by Alex Bennée 1 day, 8 hours ago
Michael Tokarev <mjt@tls.msk.ru> writes:

> On 12/4/25 23:35, 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() ahead of the main alignment check. We also
>> teach arm_deliver_fault to deliver the right syndrome for
>> MMU_INST_FETCH alignment issues.
>> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3233
>> Tested-by: Jessica Clarke <jrtc27@jrtc27.com>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> This feels like a qemu-stable material (for all active stable
> branches).

By all means - its pretty self-contained.

>
> I'm picking it up for 10.0.x and 10.1.x.  Please let me know
> if I shouldn't.
>
> Thanks,
>
> /mjt

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v3] target/arm: handle unaligned PC during tlb probe
Posted by Richard Henderson 6 days, 3 hours ago
On 12/4/25 14:35, 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() ahead of the main alignment check. We also
> teach arm_deliver_fault to deliver the right syndrome for
> MMU_INST_FETCH alignment issues.
> 
> Fixes:https://gitlab.com/qemu-project/qemu/-/issues/3233
> Tested-by: Jessica Clarke<jrtc27@jrtc27.com>
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> 
> ---
> v2
>    - don't mess with MemOp for alignment check
>    - expand arm_deliver_fault to pick up alignment issues
> v3
>    - update commit message
> ---
>   target/arm/tcg/tlb_helper.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3] target/arm: handle unaligned PC during tlb probe
Posted by Jessica Clarke 6 days, 17 hours ago
On Thu, Dec 04, 2025 at 08:35:40PM +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() ahead of the main alignment check. We also
> teach arm_deliver_fault to deliver the right syndrome for
> MMU_INST_FETCH alignment issues.
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3233
> Tested-by: Jessica Clarke <jrtc27@jrtc27.com>

v3 is different enough from the tested RFC that maybe this shouldn't
have been carried forwards, but I've now tested this v3 and it does
indeed still fix the issue in my testing.

Thanks,
Jessica
Re: [PATCH v3] target/arm: handle unaligned PC during tlb probe
Posted by Alex Bennée 6 days, 9 hours ago
Jessica Clarke <jrtc27@jrtc27.com> writes:

> On Thu, Dec 04, 2025 at 08:35:40PM +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() ahead of the main alignment check. We also
>> teach arm_deliver_fault to deliver the right syndrome for
>> MMU_INST_FETCH alignment issues.
>> 
>> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/3233
>> Tested-by: Jessica Clarke <jrtc27@jrtc27.com>
>
> v3 is different enough from the tested RFC that maybe this shouldn't
> have been carried forwards, but I've now tested this v3 and it does
> indeed still fix the issue in my testing.

I did re-test myself and figured it was only adding to the robustness
but thanks for re-confirming its working for you.

>
> Thanks,
> Jessica

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro