[0/4] target/arm: fix arm_cpu_get_phys_page_attrs_debug

Commit Message

Pierrick Bouvier April 10, 2025, 9 p.m. UTC

It was reported that QEMU monitor command gva2gpa was reporting unmapped
memory for a valid access (qemu-system-aarch64), during a copy from
kernel to user space (__arch_copy_to_user symbol in Linux) [1].
This was affecting cpu_memory_rw_debug also, which
is used in numerous places in our codebase. After investigating, the
problem was specific to arm_cpu_get_phys_page_attrs_debug.

[1] https://lists.nongnu.org/archive/html/qemu-discuss/2025-04/msg00013.html

When performing user access from a privileged space, we need to do a
second lookup for user mmu idx, following what get_a64_user_mem_index is
doing at translation time.

This series first extract some functions, and then perform the second lookup
expected using extracted functions.

Besides running all QEMU tests, it was explicitely checked that during a linux
boot sequence, accesses now report a valid physical address inconditionnally
using this (non sent) patch:


Pierrick Bouvier (4):
  target/arm/ptw: extract arm_mmu_idx_to_security_space
  target/arm/ptw: get current security_space for current mmu_idx
  target/arm/ptw: extract arm_cpu_get_phys_page
  target/arm/ptw: fix arm_cpu_get_phys_page_attrs_debug

 target/arm/ptw.c | 65 +++++++++++++++++++++++++++++++++++-------------
 1 file changed, 48 insertions(+), 17 deletions(-)

Comments

Richard Henderson April 12, 2025, 5:11 p.m. UTC | #1

On 4/10/25 14:00, Pierrick Bouvier wrote:
> Pierrick Bouvier (4):
>    target/arm/ptw: extract arm_mmu_idx_to_security_space
>    target/arm/ptw: get current security_space for current mmu_idx
>    target/arm/ptw: extract arm_cpu_get_phys_page
>    target/arm/ptw: fix arm_cpu_get_phys_page_attrs_debug

Reviewed-by: Richard Henderson <[e-mail]>

r~

Patch

--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -997,9 +997,7 @@  static inline void tlb_set_compare(CPUTLBEntryFull *full, CPUTLBEntry *ent,
     if (enable) {
         address |= flags & TLB_FLAGS_MASK;
         flags &= TLB_SLOW_FLAGS_MASK;
-        if (flags) {
             address |= TLB_FORCE_SLOW;
-        }
     } else {
         address = -1;
         flags = 0;
@@ -1658,6 +1656,10 @@  static bool mmu_lookup1(CPUState *cpu, MMULookupPageData *data, MemOp memop,
         tlb_addr = tlb_read_idx(entry, access_type) & ~TLB_INVALID_MASK;
     }

+    vaddr page = addr & TARGET_PAGE_MASK;
+    hwaddr physaddr = cpu_get_phys_page_debug(cpu, page);
+    g_assert(physaddr != -1);
+
     full = &cpu->neg.tlb.d[mmu_idx].fulltlb[index];
     flags = tlb_addr & (TLB_FLAGS_MASK & ~TLB_FORCE_SLOW);
     flags |= full->slow_flags[access_type];