|
From: | Frederic Konrad |
Subject: | Re: [Qemu-devel] [RFC 02/10] use a different translation block list for each cpu. |
Date: | Mon, 02 Feb 2015 09:39:25 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
On 29/01/2015 16:24, Peter Maydell wrote:
On 16 January 2015 at 17:19, <address@hidden> wrote:From: KONRAD Frederic <address@hidden> We need a different TranslationBlock list for each core in case of multithread TCG. Signed-off-by: KONRAD Frederic <address@hidden> --- 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
[Prev in Thread] | Current Thread | [Next in Thread] |