On 16/05/16 19:09, Peter Maydell wrote: > The user-mode-only function tb_invalidate_phys_page() is only > called from two places: > * page_unprotect(), which passes in a non-zero pc, a puc pointer > and the value 'true' for the locked argument > * page_set_flags(), which passes in a zero pc, a NULL puc pointer > and a 'false' locked argument > > If the pc is non-zero then we may call cpu_resume_from_signal(), > which does a longjmp out of the calling code (and out of the > signal handler); this is to cover the case of a target CPU with > "precise self-modifying code" (currently only x86) executing > a store instruction which modifies code in the same TB as the > store itself. Rather than doing the longjump directly here, > return a flag to the caller which indicates whether the current > TB was modified, and move the longjump to page_unprotect. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > translate-all.c | 26 +++++++++++++++----------- > 1 file changed, 15 insertions(+), 11 deletions(-) > > diff --git a/translate-all.c b/translate-all.c > index b54f472..4820d2e 100644 > --- a/translate-all.c > +++ b/translate-all.c > @@ -1438,10 +1438,13 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t > start, int len) > } > } > #else > -/* Called with mmap_lock held. */ > -static void tb_invalidate_phys_page(tb_page_addr_t addr, > - uintptr_t pc, void *puc, > - bool locked) > +/* Called with mmap_lock held. If pc is not 0 then it indicates the > + * host PC of the faulting store instruction that caused this invalidate. > + * Returns true if the caller needs to abort execution of the current > + * TB (because it was modified by this store and the guest CPU has > + * precise-SMC semantics). > + */ > +static bool tb_invalidate_phys_page(tb_page_addr_t addr, uintptr_t pc) > { > TranslationBlock *tb; > PageDesc *p; > @@ -1459,7 +1462,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr, > addr &= TARGET_PAGE_MASK; > p = page_find(addr >> TARGET_PAGE_BITS); > if (!p) { > - return; > + return false; > } > tb = p->first_tb; > #ifdef TARGET_HAS_PRECISE_SMC > @@ -1498,12 +1501,10 @@ static void tb_invalidate_phys_page(tb_page_addr_t > addr, > modifying the memory. It will ensure that it cannot modify > itself */ > tb_gen_code(cpu, current_pc, current_cs_base, current_flags, 1); > - if (locked) { > - mmap_unlock(); > - } > - cpu_resume_from_signal(cpu, puc); > + return true; > } > #endif > + return false; > } > #endif > > @@ -1902,7 +1903,7 @@ void page_set_flags(target_ulong start, target_ulong > end, int flags) > if (!(p->flags & PAGE_WRITE) && > (flags & PAGE_WRITE) && > p->first_tb) { > - tb_invalidate_phys_page(addr, 0, NULL, false); > + tb_invalidate_phys_page(addr, 0); > } > p->flags = flags; > } > @@ -1996,7 +1997,10 @@ int page_unprotect(target_ulong address, uintptr_t pc, > void *puc) > > /* and since the content will be modified, we must invalidate > the corresponding translated code. */ > - tb_invalidate_phys_page(addr, pc, puc, true); > + if (tb_invalidate_phys_page(addr, pc)) { > + mmap_unlock(); > + cpu_resume_from_signal(current_cpu, puc); > + } > #ifdef DEBUG_TB_CHECK > tb_invalidate_check(addr); > #endif
Just my 2 cents: we could allow that cpu_resume_from_signal() call and add mmap_lock_reset() similar to tb_lock_reset() to handle resetting mmap_lock after a long jump. Kind regards, Sergey