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

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

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