From a688e73ba8c2e3b6f3e5e683c4a9c37d2ddc8984 Mon Sep 17 00:00:00 2001 From: "Emilio G. Cota" Date: Mon, 25 Jun 2018 12:31:42 -0400 Subject: [PATCH] translate-all: fix locking of TBs whose two pages share the same physical page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 0b5c91f ("translate-all: use per-page locking in !user-mode", 2018-06-15) introduced per-page locking. It assumed that the physical pages corresponding to a TB (at most two pages) are always distinct, which is wrong. For instance, an xtensa test provided by Max Filippov is broken by the commit, since the test maps two virtual pages to the same physical page: virt1: 7fff, virt2: 8000 phys1 6000fff, phys2 6000000 Fix it by removing the assumption from page_lock_pair. If the two physical page addresses are equal, we only lock the PageDesc once. Note that the two callers of page_lock_pair, namely page_unlock_tb and tb_link_page, are also updated so that we do not try to unlock the same PageDesc twice. Fixes: 0b5c91f74f3c83a36f37740969df8c775c997e69 Reported-by: Max Filippov Tested-by: Max Filippov Tested-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Signed-off-by: Emilio G. Cota Message-Id: <1529944302-14186-1-git-send-email-cota@braap.org> Signed-off-by: Richard Henderson --- accel/tcg/translate-all.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index e8228bf3e6..170b95793f 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -669,9 +669,15 @@ static inline void page_lock_tb(const TranslationBlock *tb) static inline void page_unlock_tb(const TranslationBlock *tb) { - page_unlock(page_find(tb->page_addr[0] >> TARGET_PAGE_BITS)); + PageDesc *p1 = page_find(tb->page_addr[0] >> TARGET_PAGE_BITS); + + page_unlock(p1); if (unlikely(tb->page_addr[1] != -1)) { - page_unlock(page_find(tb->page_addr[1] >> TARGET_PAGE_BITS)); + PageDesc *p2 = page_find(tb->page_addr[1] >> TARGET_PAGE_BITS); + + if (p2 != p1) { + page_unlock(p2); + } } } @@ -850,22 +856,34 @@ static void page_lock_pair(PageDesc **ret_p1, tb_page_addr_t phys1, PageDesc **ret_p2, tb_page_addr_t phys2, int alloc) { PageDesc *p1, *p2; + tb_page_addr_t page1; + tb_page_addr_t page2; assert_memory_lock(); - g_assert(phys1 != -1 && phys1 != phys2); - p1 = page_find_alloc(phys1 >> TARGET_PAGE_BITS, alloc); + g_assert(phys1 != -1); + + page1 = phys1 >> TARGET_PAGE_BITS; + page2 = phys2 >> TARGET_PAGE_BITS; + + p1 = page_find_alloc(page1, alloc); if (ret_p1) { *ret_p1 = p1; } if (likely(phys2 == -1)) { page_lock(p1); return; + } else if (page1 == page2) { + page_lock(p1); + if (ret_p2) { + *ret_p2 = p1; + } + return; } - p2 = page_find_alloc(phys2 >> TARGET_PAGE_BITS, alloc); + p2 = page_find_alloc(page2, alloc); if (ret_p2) { *ret_p2 = p2; } - if (phys1 < phys2) { + if (page1 < page2) { page_lock(p1); page_lock(p2); } else { @@ -1623,7 +1641,7 @@ tb_link_page(TranslationBlock *tb, tb_page_addr_t phys_pc, tb = existing_tb; } - if (p2) { + if (p2 && p2 != p) { page_unlock(p2); } page_unlock(p);