From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57998) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YICXc-0000Hy-T9 for qemu-devel@nongnu.org; Mon, 02 Feb 2015 03:39:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YICXY-0003On-6K for qemu-devel@nongnu.org; Mon, 02 Feb 2015 03:39:32 -0500 Received: from greensocs.com ([193.104.36.180]:26876) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YICXX-0003NH-Ry for qemu-devel@nongnu.org; Mon, 02 Feb 2015 03:39:28 -0500 Message-ID: <54CF37BD.3020605@greensocs.com> Date: Mon, 02 Feb 2015 09:39:25 +0100 From: Frederic Konrad MIME-Version: 1.0 References: <1421428797-23697-1-git-send-email-fred.konrad@greensocs.com> <1421428797-23697-3-git-send-email-fred.konrad@greensocs.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC 02/10] use a different translation block list for each cpu. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: mttcg@listserver.greensocs.com, "J. Kiszka" , Mark Burton , QEMU Developers , Alexander Graf , Paolo Bonzini On 29/01/2015 16:24, Peter Maydell wrote: > On 16 January 2015 at 17:19, wrote: >> From: KONRAD Frederic >> >> We need a different TranslationBlock list for each core in case of multithread >> TCG. >> >> Signed-off-by: KONRAD Frederic >> --- >> translate-all.c | 40 ++++++++++++++++++++++------------------ >> 1 file changed, 22 insertions(+), 18 deletions(-) >> >> diff --git a/translate-all.c b/translate-all.c >> index 8fa4378..0e11c70 100644 >> --- a/translate-all.c >> +++ b/translate-all.c >> @@ -72,10 +72,11 @@ >> #endif >> >> #define SMC_BITMAP_USE_THRESHOLD 10 >> +#define MAX_CPUS 256 >> >> typedef struct PageDesc { >> /* list of TBs intersecting this ram page */ >> - TranslationBlock *first_tb; >> + TranslationBlock *first_tb[MAX_CPUS]; > Do we really need to know this for every CPU, or just for > the one that's using this PageDesc? I am assuming we're going to make > the l1_map be per-CPU. Do we have any clue of which cpu is using this PageDesc? We did this like that because it is quite simple. > >> /* in order to optimize self modifying code, we count the number >> of lookups we do to a given page to use a bitmap */ >> unsigned int code_write_count; >> @@ -750,7 +751,7 @@ static inline void invalidate_page_bitmap(PageDesc *p) >> /* Set to NULL all the 'first_tb' fields in all PageDescs. */ >> static void page_flush_tb_1(int level, void **lp) >> { >> - int i; >> + int i, j; >> >> if (*lp == NULL) { >> return; >> @@ -759,7 +760,9 @@ static void page_flush_tb_1(int level, void **lp) >> PageDesc *pd = *lp; >> >> for (i = 0; i < V_L2_SIZE; ++i) { >> - pd[i].first_tb = NULL; >> + for (j = 0; j < MAX_CPUS; j++) { >> + pd[i].first_tb[j] = NULL; >> + } >> invalidate_page_bitmap(pd + i); >> } >> } else { >> @@ -937,12 +940,12 @@ void tb_phys_invalidate(TranslationBlock *tb, tb_page_addr_t page_addr) >> /* remove the TB from the page list */ >> if (tb->page_addr[0] != page_addr) { >> p = page_find(tb->page_addr[0] >> TARGET_PAGE_BITS); >> - tb_page_remove(&p->first_tb, tb); >> + tb_page_remove(&p->first_tb[current_cpu->cpu_index], tb); > Anything using current_cpu in this code is hugely suspect. > For instance cpu_restore_state() takes a CPUState pointer and > calls this function -- either it should be acting on just that > CPU (which might not be the current one) or on all CPUs. In > any case implicitly working on current_cpu here is wrong. > > Probably we need to look at the public-facing functions here > and decide which should have "operate on all CPUs" semantics > and which should have "operate on the CPU passed as a parameter" > and which "operate on the implicit current CPU". Ok so the idea would be to have eg a cpu mask parameter to know which cpu to invalidate/restore etc etc? Or just pointer and invalidate all if NULL? Thanks, Fred > > -- PMM