target/mips: fix spurious EXCP_AdEL on ll/lld under UC_TLB_VIRTUAL#2346
target/mips: fix spurious EXCP_AdEL on ll/lld under UC_TLB_VIRTUAL#2346retrocpugeek wants to merge 5 commits into
Conversation
The LL atomic helper (HELPER_LD_ATOMIC) performs the load through the soft-TLB but additionally calls do_translate_address() purely to populate CP0_LLAddr. That walks the MIPS segments and rejects any address outside useg (e.g. an address >= 0x80000000 with UX unset), raising EXCP_AdEL even though the soft-TLB / virtual-TLB hook maps the address and the load itself succeeds. In virtual-TLB mode, record the virtual address into CP0_LLAddr directly instead of segment-translating it. The alignment check is left intact, so a genuinely misaligned ll/lld still raises AdEL. sc/scd are unaffected (they go through tcg_gen_atomic_cmpxchg_tl and never call do_translate_address). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Under UC_TLB_VIRTUAL, a Load-Linked from an address above useg (>= 2GB) used to raise a spurious EXCP_AdEL because the ll helper segment-checked the address when populating CP0_LLAddr. Adds a test that ll from a high address now reads correctly, plus a guard that a misaligned ll still raises an Address Error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
I had only a quick look, but you missed that the virtual tlb option also allows hooks to change the mapping. |
Extends the MIPS virtual-TLB ll regression with the case PhilippTakacs raised on the PR: UC_TLB_VIRTUAL lets the fill hook map a page to an arbitrary physical address, and CP0_LLAddr must hold that mapped physical address (readable by the guest via mfc0 LLAddr), not the virtual address. The added hook maps the lock page to a different physical page; the test performs an ll and checks that mfc0 LLAddr reads back the physical address. It fails against the previous fix (which recorded the virtual address) and passes with the follow-up commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The earlier fix recorded the virtual address into CP0_LLAddr in virtual-TLB mode. But UC_TLB_VIRTUAL lets the fill hook remap a page to any physical address, and CP0_LLAddr is architecturally the physical address of the linked load (a guest can read it via mfc0/dmfc0 LLAddr). Recording the raw virtual address is wrong whenever the hook does not map identity. Recover the hook-mapped physical address from the soft-TLB via tlb_vaddr_to_paddr() (which probes the fill hook non-faulting) instead. This still avoids the spurious EXCP_AdEL from the MIPS segment walk and now also reports the correct physical address. The non-virtual-TLB path is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @PhilippTakacs |
|
Looks good to me. @PhilippTakacs do you also approve this? |
|
there is still an issue, because with the mips mmu the code raises an exeption (see |
Addresses review feedback on unicorn-engine#2346: the previous vtlb path probed the soft-TLB non-faulting (tlb_vaddr_to_paddr) and, when a UC_TLB_VIRTUAL fill hook declined the page, fell back to recording the raw virtual address in CP0_LLAddr - effectively putting a 1:1 mapping back in and masking the fact that the hook is allowed to fail a translation. Instead, under virtual-TLB perform the soft-TLB load first: it goes through the normal softmmu slow path and raises the proper MMU fault if the hook declines the page, then recover the hook-mapped physical address from the now-populated soft-TLB. The non-virtual-TLB path is unchanged (cpu_mips_translate_address before the load). (An earlier draft called CPUClass::tlb_fill() directly with probe=false, as suggested in review, but unicorn_fill_tlb() calls cpu_restore_state() unconditionally, which is only safe when exiting the CPU loop on a fault; driving it inline and then continuing corrupts CPU state. Doing the load first gets the same faulting semantics through the supported path.) Add a regression test asserting that ll on a page the vtlb hook declines raises UC_ERR_MMU_READ. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @PhilippTakacs, good catch — you're right that the previous version put a 1:1 mapping back in and swallowed a translation the vtlb hook is allowed to decline. I tried your suggestion of calling So instead I get the same faulting semantics through the supported path: under Added a regression test asserting that Pushed as 5573568. |
3 participants
Fixes #2345.
Under
UC_TLB_VIRTUAL, MIPSll/lldraised a spuriousEXCP_AdELfor addresses above useg (>= 0x80000000) becauseHELPER_LD_ATOMIC(qemu/target/mips/op_helper.c) calleddo_translate_address()to populateCP0_LLAddr, which segment-checks the address and rejects anything outside useg. The load itself already goes through the soft-TLB, so in virtual-TLB mode this records the virtual address intoCP0_LLAddrdirectly instead of segment-translating it.The alignment check is preserved (a misaligned
llstill raises AdEL);sc/scdare unaffected (they usetcg_gen_atomic_cmpxchg_tl, which never callsdo_translate_address).Adds a regression test in
tests/unit/test_mips.c(high-addressllreads correctly; misalignedllstill faults).Downstream tracking: qilingframework/qiling#1645 (Qiling MIPS64 real-ELF/glibc support gates its CI tests on this fix).