On 16 January 2015 at 17:19, <fred.kon...@greensocs.com> wrote:
> From: KONRAD Frederic <fred.kon...@greensocs.com>
>
> We need a different TranslationBlock list for each core in case of multithread
> TCG.
>
> Signed-off-by: KONRAD Frederic <fred.kon...@greensocs.com>
> ---
> 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.
> /* 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".
-- PMM
