[qemu] How to reliably obtain physaddr from vaddr

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

On 16 March 2015 at 20:08, Emilio G. Cota <cota@braap.org> wrote:
> 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.

This isn't how tlb_fill handles page faults. What happens is:

1. tlb_fill calls arm_cpu_handle_mmu_fault to do the page table walk
2. if the page table indicates that the vaddr is invalid (ie
   we need to deliver a guest exception) then we return non-zero
   to tlb_fill
3. tlb_fill calls cpu_restore_state, passing it the address
   in generated TCG code where we were when the exception
   happened; magic happens here to fix up the CPU state
   (notably the guest PC) to the exact correct values at the
   guest load/store insn that caused the fault [using the
   (host) retaddr to determine exactly where in the TB we were
   and so which guest insn faulted]
3. tlb_fill calls raise_exception, which calls cpu_loop_exit
4. cpu_loop_exit *longjmps out of anything you were in the
   middle of*, back to the top level loop in cpu-exec.c
5. based on the changes to the CPU state made before calling
   cpu_loop_exit, the main loop determines that there's a
   pending exception, and resumes execution at the exception
   entry point
6. the guest OS may or may not end up fixing up the page tables
   and reattempting execution of whatever failed, but that's
   entirely up to it and might never happen

I suspect your problem is that the host retaddr in step 3
is wrong, which will result in our generating the guest
exception with a wrong value for "guest PC at point of fault".
Linux makes extensive use of "if guest PC for this fault
is in this magic bit of code then fix up the result so it
looks like this kernel load/store accessor function returned
-EFAULT". If you're reporting the wrong guest PC this won't
work and the kernel will end up in the default case path
of it being an unexpected kernel mode data abort and Oopsing.

I suggest you check whether the exception PC reported to
the guest is correct (it's probably reported by the kernel
somewhere in the oops output) compared to the addresses in
the kernel of the load/store/whatever that's faulted.

thanks
-- PMM

On Mon, Mar 16, 2015 at 22:23:24 +0000, Peter Maydell wrote:
> On 16 March 2015 at 20:08, Emilio G. Cota <cota@braap.org> wrote:
> > 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.
> 
> This isn't how tlb_fill handles page faults. What happens is:
> 
> 1. tlb_fill calls arm_cpu_handle_mmu_fault to do the page table walk
(snip)
> 6. the guest OS may or may not end up fixing up the page tables
>    and reattempting execution of whatever failed, but that's
>    entirely up to it and might never happen

Great description--the last point wasn't all that clear to me.

> I suspect your problem is that the host retaddr in step 3
> is wrong, which will result in our generating the guest
> exception with a wrong value for "guest PC at point of fault".
> Linux makes extensive use of "if guest PC for this fault
> is in this magic bit of code then fix up the result so it
> looks like this kernel load/store accessor function returned
> -EFAULT". If you're reporting the wrong guest PC this won't
> work and the kernel will end up in the default case path
> of it being an unexpected kernel mode data abort and Oopsing.

That was indeed the problem, the TB from that retaddr couldn't
be found.

[ BTW why don't we check the return value of cpu_restore_state?
  I see this has been discussed before:
     http://lists.gnu.org/archive/html/qemu-devel/2012-09/msg02589.html
  Certainly an assert there would have helped me debug this. ]

> I suggest you check whether the exception PC reported to
> the guest is correct (it's probably reported by the kernel
> somewhere in the oops output) compared to the addresses in
> the kernel of the load/store/whatever that's faulted.

Yep. The fix is trivial:

+++ b/target-arm/helper.c
@@ -5797,7 +5797,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val)
 
 void HELPER(st_pre)(CPUARMState *env, uint32_t vaddr)
 {
-    cpu_st_paddr_data(env, vaddr);
+    helper_ret_st_paddr(env, vaddr, cpu_mmu_index(env), GETRA());
 }

.. and with this I learned that cpu_ld/st are only to be called
from op helpers.

Thanks a lot for your help.

		Emilio