On Mon, Mar 16, 2015 at 22:23:24 +0000, Peter Maydell wrote:
> On 16 March 2015 at 20:08, Emilio G. Cota <c...@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
