Re: [Qemu-devel] [qemu] How to reliably obtain physaddr from vaddr

[Emilio G. Cota]

On Sun, Mar 15, 2015 at 16:10:21 -0700, Richard Henderson wrote:
> On 03/15/2015 03:00 AM, Emilio G. Cota wrote:
> > On a TLB hit this is trivial (just do nothing), but on
> > a TLB miss I'm lost on what to do--I cannot even follow
> > where helper_ld/st go (grep doesn't help), although I
> > suspect it's TCG backend ops and I don't see an obvious
> > way of adding a new operation there.
> 
> It goes into softmmu_template.h.  Which then tests a victim tlb, and finally
> calls tlb_fill.  You'll probably need to do the same.

Thanks, figured this out this morning after getting some sleep.

I've defined this vaddr->paddr as a helper and I'm calling it
before every aa32 store. However, this isn't a smooth sailing:

1. futex_init in the kernel causes an oops--it passes vaddr=0
   but the call happens with pagefaults disabled:
     http://lxr.free-electrons.com/source/kernel/futex.c?v=3.18#L590
   in the code below I'm just returning to avoid the oops.

2. The kernel (vexpress-a9 from buildroot) doesn't boot. It dies with:

> [...]
> devtmpfs: mounted
> Freeing unused kernel memory: 256K (805ea000 - 8062a000)
> Cannot continue, found non relative relocs during the bootstrap.
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000e00
> 
> CPU: 0 PID: 1 Comm: init Not tainted 3.18.8 #2
> [<800147cc>] (unwind_backtrace) from [<800115d4>] (show_stack+0x10/0x14)
> [<800115d4>] (show_stack) from [<80473e8c>] (dump_stack+0x84/0x94)
> [<80473e8c>] (dump_stack) from [<80471180>] (panic+0xa0/0x1f8)
> [<80471180>] (panic) from [<800240dc>] (complete_and_exit+0x0/0x1c)
> [<800240dc>] (complete_and_exit) from [<87827f70>] (0x87827f70)
> ---[ end Kernel panic - not syncing: Attempted to kill init! 
> exitcode=0x00000e00

  Note that if I only call the helper before the "str[hb]*" stores,
  the kernel boots fine.

I'm appending the code (applies on top of current HEAD, 7ccfb495),
any help on what's going on greatly appreciated.

Thanks,

                Emilio

diff --git a/cputlb.c b/cputlb.c
index 38f2151..5596bae 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -329,6 +329,7 @@ void tlb_set_page(CPUState *cpu, target_ulong vaddr,
     } else {
         te->addr_write = -1;
     }
+    te->addr_phys = paddr;
 }
 
 /* NOTE: this function can trigger an exception */
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 0ca6f0b..65f340c 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -74,10 +74,10 @@ typedef uint64_t target_ulong;
 /* use a fully associative victim tlb of 8 entries */
 #define CPU_VTLB_SIZE 8
 
-#if HOST_LONG_BITS == 32 && TARGET_LONG_BITS == 32
-#define CPU_TLB_ENTRY_BITS 4
-#else
+#if TARGET_LONG_BITS == 32
 #define CPU_TLB_ENTRY_BITS 5
+#else
+#define CPU_TLB_ENTRY_BITS 6
 #endif
 
 typedef struct CPUTLBEntry {
@@ -90,13 +90,14 @@ typedef struct CPUTLBEntry {
     target_ulong addr_read;
     target_ulong addr_write;
     target_ulong addr_code;
+    target_ulong addr_phys;
     /* Addend to virtual address to get host address.  IO accesses
        use the corresponding iotlb value.  */
     uintptr_t addend;
     /* padding to get a power of two size */
     uint8_t dummy[(1 << CPU_TLB_ENTRY_BITS) -
-                  (sizeof(target_ulong) * 3 +
-                   ((-sizeof(target_ulong) * 3) & (sizeof(uintptr_t) - 1)) +
+                  (sizeof(target_ulong) * 4 +
+                   ((-sizeof(target_ulong) * 4) & (sizeof(uintptr_t) - 1)) +
                    sizeof(uintptr_t))];
 } CPUTLBEntry;
 
diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 1673287..168cde9 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -127,6 +127,9 @@ void helper_stl_mmu(CPUArchState *env, target_ulong addr,
 void helper_stq_mmu(CPUArchState *env, target_ulong addr,
                     uint64_t val, int mmu_idx);
 
+hwaddr helper_st_paddr_mmu(CPUArchState *env, target_ulong addr,
+                           int mmu_idx);
+
 uint8_t helper_ldb_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
 uint16_t helper_ldw_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
 uint32_t helper_ldl_cmmu(CPUArchState *env, target_ulong addr, int mmu_idx);
diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h
index 95ab750..39cde9d 100644
--- a/include/exec/cpu_ldst_template.h
+++ b/include/exec/cpu_ldst_template.h
@@ -129,6 +129,39 @@ glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, 
target_ulong ptr,
     }
 }
 
+#if DATA_SIZE == 1
+
+static inline hwaddr
+glue(cpu_st_paddr, MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
+{
+    int page_index;
+    target_ulong addr;
+    int mmu_idx;
+    hwaddr ret;
+
+    addr = ptr;
+    /*
+     * XXX Understand why this is necessary.
+     * futex_init on linux bootup calls cmpxchg on a NULL pointer. It expects
+     * -EFAULT to be read back, but when we do the below we get a kernel oops.
+     * However, when doing the load from TCG -EFAULT is read just fine--no 
oops.
+     */
+    if (unlikely(addr == 0))
+           return 0;
+    page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
+    mmu_idx = CPU_MMU_INDEX;
+    if (unlikely(env->tlb_table[mmu_idx][page_index].addr_write !=
+                 (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) {
+        ret = glue(helper_st_paddr, MMUSUFFIX)(env, addr, mmu_idx);
+    } else {
+        ret = env->tlb_table[mmu_idx][page_index].addr_phys;
+    }
+    ret |= (addr & ~TARGET_PAGE_MASK);
+    return ret;
+}
+
+#endif /* DATA_SIZE == 1 */
+
 #endif /* !SOFTMMU_CODE_ACCESS */
 
 #undef RES_TYPE
diff --git a/softmmu_template.h b/softmmu_template.h
index 0e3dd35..172b718 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -461,6 +461,36 @@ void helper_le_st_name(CPUArchState *env, target_ulong 
addr, DATA_TYPE val,
 #endif
 }
 
+#if DATA_SIZE == 1
+
+hwaddr helper_ret_st_paddr(CPUArchState *env, target_ulong addr,
+                           int mmu_idx, uintptr_t retaddr)
+{
+    int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
+    target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write;
+
+    /* Adjust the given return address.  */
+    retaddr -= GETPC_ADJ;
+
+    /* If the TLB entry is for a different page, reload and try again.  */
+    if ((addr & TARGET_PAGE_MASK)
+        != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) {
+        if (!VICTIM_TLB_HIT(addr_write)) {
+            tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr);
+        }
+    }
+    return env->tlb_table[mmu_idx][index].addr_phys;
+}
+
+hwaddr
+glue(helper_st_paddr, MMUSUFFIX)(CPUArchState *env, target_ulong addr,
+                                 int mmu_idx)
+{
+    return helper_ret_st_paddr(env, addr, mmu_idx, GETRA());
+}
+
+#endif /* DATA_SIZE == 1 */
+
 #if DATA_SIZE > 1
 void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val,
                        int mmu_idx, uintptr_t retaddr)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3bc20af..d329b42 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5795,6 +5795,11 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, 
uint32_t val)
 
 #endif
 
+void HELPER(st_pre)(CPUARMState *env, uint32_t vaddr)
+{
+    cpu_st_paddr_data(env, vaddr);
+}
+
 void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in)
 {
     /* Implement DC ZVA, which zeroes a fixed-length block of memory.
diff --git a/target-arm/helper.h b/target-arm/helper.h
index dec3728..335b970 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -526,6 +526,8 @@ DEF_HELPER_FLAGS_3(crc32, TCG_CALL_NO_RWG_SE, i32, i32, 
i32, i32)
 DEF_HELPER_FLAGS_3(crc32c, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32)
 DEF_HELPER_2(dc_zva, void, env, i64)
 
+DEF_HELPER_2(st_pre, void, env, i32)
+
 DEF_HELPER_FLAGS_2(neon_pmull_64_lo, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 DEF_HELPER_FLAGS_2(neon_pmull_64_hi, TCG_CALL_NO_RWG_SE, i64, i64, i64)
 
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 36868ed..b37c6a7 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -896,6 +896,7 @@ static inline void gen_aa32_ld##SUFF(TCGv_i32 val, TCGv_i32 
addr, int index) \
 #define DO_GEN_ST(SUFF, OPC)                                             \
 static inline void gen_aa32_st##SUFF(TCGv_i32 val, TCGv_i32 addr, int index) \
 {                                                                        \
+    gen_helper_st_pre(cpu_env, addr);                                    \
     tcg_gen_qemu_st_i32(val, addr, index, OPC);                          \
 }
 
diff --git a/tcg/tcg.h b/tcg/tcg.h
index f941965..0771ecf 100644
--- a/tcg/tcg.h
+++ b/tcg/tcg.h
@@ -902,6 +902,9 @@ void helper_be_stl_mmu(CPUArchState *env, target_ulong 
addr, uint32_t val,
 void helper_be_stq_mmu(CPUArchState *env, target_ulong addr, uint64_t val,
                        int mmu_idx, uintptr_t retaddr);
 
+hwaddr helper_ret_st_paddr(CPUArchState *env, target_ulong addr,
+                           int mmu_idx, uintptr_t retaddr);
+
 /* Temporary aliases until backends are converted.  */
 #ifdef TARGET_WORDS_BIGENDIAN
 # define helper_ret_ldsw_mmu  helper_be_ldsw_mmu