Commit a688e73b authored by Emilio G. Cota's avatar Emilio G. Cota Committed by Richard Henderson
Browse files

translate-all: fix locking of TBs whose two pages share the same physical page 



Commit 0b5c91f7 ("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: 0b5c91f7
Reported-by: default avatarMax Filippov <jcmvbkbc@gmail.com>
Tested-by: default avatarMax Filippov <jcmvbkbc@gmail.com>
Tested-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: default avatarRichard Henderson <richard.henderson@linaro.org>
Signed-off-by: default avatarEmilio G. Cota <cota@braap.org>
Message-Id: <1529944302-14186-1-git-send-email-cota@braap.org>
Signed-off-by: default avatarRichard Henderson <richard.henderson@linaro.org>
parent 2d58e33e

accel/tcg/translate-all.c

+25 −7
Original line number Diff line number Diff line
@@ -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;

        }

    p2 = page_find_alloc(phys2 >> TARGET_PAGE_BITS, alloc);

        return;

    }

    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);