[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/5] translate-all.c: Don't pass puc, locked to
From: |
Sergey Fedorov |
Subject: |
Re: [Qemu-devel] [PATCH 1/5] translate-all.c: Don't pass puc, locked to tb_invalidate_phys_page() |
Date: |
Mon, 16 May 2016 20:13:41 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 |
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 <address@hidden>
> ---
> 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
[Qemu-devel] [PATCH 2/5] user-exec: Push resume-from-signal code out to handle_cpu_signal(), Peter Maydell, 2016/05/16