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

["Emilio G. Cota" <cota@braap.org>]

On Sun, Mar 15, 2015 at 20:42:31 -0400, Emilio G. Cota wrote:
> On Sun, Mar 15, 2015 at 16:10:21 -0700, Richard Henderson wrote:
> > 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.
> 
> 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.

Please disregard this point--the oops doesn't happen with the code
I appended (it was triggered by previous iterations of it).

> 2. The kernel (vexpress-a9 from buildroot) doesn't boot.

Removing the call to tlb_fill() on a TLB miss solves the problem.
But of course this also means the helper doesn't work as intended.

I fail to see why calling tlb_fill() from the helper causes
trouble.  What I thought would happen is that the exception
(if any) is started from the helper, gets serviced, and then
both the helper and the subsequent store hit in the TLB.  I was
seeing this as a "TLB prefetch", but I cannot make it work.

What am I missing?

FWIW I'm appending the delta wrt my previous email.

Thanks,

		Emilio

diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h
index 39cde9d..48c54f9 100644
--- a/include/exec/cpu_ldst_template.h
+++ b/include/exec/cpu_ldst_template.h
@@ -140,14 +140,6 @@ glue(cpu_st_paddr, MEMSUFFIX)(CPUArchState *env, target_ulong ptr)
     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 !=
diff --git a/softmmu_template.h b/softmmu_template.h
index 172b718..1b6655e 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -476,7 +476,7 @@ hwaddr helper_ret_st_paddr(CPUArchState *env, target_ulong addr,
     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 0;
         }
     }
     return env->tlb_table[mmu_idx][index].addr_phys;