All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] cputlb: Various cleanups
@ 2020-01-09  2:48 Richard Henderson
  2020-01-09  2:48 ` [PATCH 1/9] cputlb: Merge tlb_table_flush_by_mmuidx into tlb_flush_one_mmuidx_locked Richard Henderson
                   ` (9 more replies)
  0 siblings, 10 replies; 37+ messages in thread
From: Richard Henderson @ 2020-01-09  2:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis

I had a conversation with Alistair Francis at KVM forum about
being able to represent ASIDs "properly".  This lead to the idea
that target-specific code might be able to cache TLBs outside of
the "main" NB_MMU_MODES -- possibly thousands of them.

This goes nowhere near that far.  But it does begin edging toward
the possibility of having a

    struct CPUTLBSaved {
        CPUTLBDesc d;
        CPUTLBDescFast f;
    };

by moving some of the most basic routines to use CPUTLBDesc and
CPUTLBDescFast directly instead of always using an mmu_idx.

I'm not sure how much time I'll have to go further along these
lines, but what I have so far still looks like a cleanup.


r~


Richard Henderson (9):
  cputlb: Merge tlb_table_flush_by_mmuidx into
    tlb_flush_one_mmuidx_locked
  cputlb: Make tlb_n_entries private to cputlb.c
  cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb
  cputlb: Hoist tlb portions in tlb_mmu_resize_locked
  cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked
  cputlb: Split out tlb_mmu_flush_locked
  cputlb: Partially merge tlb_dyn_init into tlb_init
  cputlb: Initialize tlbs as flushed
  cputlb: Hoist timestamp outside of loops over tlbs

 include/exec/cpu_ldst.h |   5 --
 accel/tcg/cputlb.c      | 120 +++++++++++++++++++++-------------------
 2 files changed, 64 insertions(+), 61 deletions(-)

-- 
2.20.1



^ permalink raw reply	[flat|nested] 37+ messages in thread

* [PATCH 1/9] cputlb: Merge tlb_table_flush_by_mmuidx into tlb_flush_one_mmuidx_locked
  2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
@ 2020-01-09  2:48 ` Richard Henderson
  2020-01-20  0:34   ` Alistair Francis
  2020-01-20 13:34   ` Alex Bennée
  2020-01-09  2:49 ` [PATCH 2/9] cputlb: Make tlb_n_entries private to cputlb.c Richard Henderson
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 37+ messages in thread
From: Richard Henderson @ 2020-01-09  2:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis

There is only one caller for tlb_table_flush_by_mmuidx.  Place
the result at the earlier line number, due to an expected user
in the near future.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index a991ea2964..1a81886e58 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -224,11 +224,16 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
     }
 }
 
-static inline void tlb_table_flush_by_mmuidx(CPUArchState *env, int mmu_idx)
+static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
 {
     tlb_mmu_resize_locked(env, mmu_idx);
-    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
     env_tlb(env)->d[mmu_idx].n_used_entries = 0;
+    env_tlb(env)->d[mmu_idx].large_page_addr = -1;
+    env_tlb(env)->d[mmu_idx].large_page_mask = -1;
+    env_tlb(env)->d[mmu_idx].vindex = 0;
+    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
+    memset(env_tlb(env)->d[mmu_idx].vtable, -1,
+           sizeof(env_tlb(env)->d[0].vtable));
 }
 
 static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
@@ -289,16 +294,6 @@ void tlb_flush_counts(size_t *pfull, size_t *ppart, size_t *pelide)
     *pelide = elide;
 }
 
-static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
-{
-    tlb_table_flush_by_mmuidx(env, mmu_idx);
-    env_tlb(env)->d[mmu_idx].large_page_addr = -1;
-    env_tlb(env)->d[mmu_idx].large_page_mask = -1;
-    env_tlb(env)->d[mmu_idx].vindex = 0;
-    memset(env_tlb(env)->d[mmu_idx].vtable, -1,
-           sizeof(env_tlb(env)->d[0].vtable));
-}
-
 static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
 {
     CPUArchState *env = cpu->env_ptr;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 2/9] cputlb: Make tlb_n_entries private to cputlb.c
  2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
  2020-01-09  2:48 ` [PATCH 1/9] cputlb: Merge tlb_table_flush_by_mmuidx into tlb_flush_one_mmuidx_locked Richard Henderson
@ 2020-01-09  2:49 ` Richard Henderson
  2020-01-20  0:36   ` Alistair Francis
                     ` (2 more replies)
  2020-01-09  2:49 ` [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb Richard Henderson
                   ` (7 subsequent siblings)
  9 siblings, 3 replies; 37+ messages in thread
From: Richard Henderson @ 2020-01-09  2:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis

There are no users of this function outside cputlb.c,
and its interface will change in the next patch.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu_ldst.h | 5 -----
 accel/tcg/cputlb.c      | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index a46116167c..53de19753a 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -234,11 +234,6 @@ static inline uintptr_t tlb_index(CPUArchState *env, uintptr_t mmu_idx,
     return (addr >> TARGET_PAGE_BITS) & size_mask;
 }
 
-static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
-{
-    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
-}
-
 /* Find the TLB entry corresponding to the mmu_idx + address pair.  */
 static inline CPUTLBEntry *tlb_entry(CPUArchState *env, uintptr_t mmu_idx,
                                      target_ulong addr)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 1a81886e58..e4a8ed9534 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -80,6 +80,11 @@ QEMU_BUILD_BUG_ON(sizeof(target_ulong) > sizeof(run_on_cpu_data));
 QEMU_BUILD_BUG_ON(NB_MMU_MODES > 16);
 #define ALL_MMUIDX_BITS ((1 << NB_MMU_MODES) - 1)
 
+static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
+{
+    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
+}
+
 static inline size_t sizeof_tlb(CPUArchState *env, uintptr_t mmu_idx)
 {
     return env_tlb(env)->f[mmu_idx].mask + (1 << CPU_TLB_ENTRY_BITS);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb
  2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
  2020-01-09  2:48 ` [PATCH 1/9] cputlb: Merge tlb_table_flush_by_mmuidx into tlb_flush_one_mmuidx_locked Richard Henderson
  2020-01-09  2:49 ` [PATCH 2/9] cputlb: Make tlb_n_entries private to cputlb.c Richard Henderson
@ 2020-01-09  2:49 ` Richard Henderson
  2020-01-20  0:37   ` Alistair Francis
                     ` (3 more replies)
  2020-01-09  2:49 ` [PATCH 4/9] cputlb: Hoist tlb portions in tlb_mmu_resize_locked Richard Henderson
                   ` (6 subsequent siblings)
  9 siblings, 4 replies; 37+ messages in thread
From: Richard Henderson @ 2020-01-09  2:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis

We do not need the entire CPUArchState to compute these values.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e4a8ed9534..49c605b6d8 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -80,14 +80,14 @@ QEMU_BUILD_BUG_ON(sizeof(target_ulong) > sizeof(run_on_cpu_data));
 QEMU_BUILD_BUG_ON(NB_MMU_MODES > 16);
 #define ALL_MMUIDX_BITS ((1 << NB_MMU_MODES) - 1)
 
-static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
+static inline size_t tlb_n_entries(CPUTLBDescFast *fast)
 {
-    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
+    return (fast->mask >> CPU_TLB_ENTRY_BITS) + 1;
 }
 
-static inline size_t sizeof_tlb(CPUArchState *env, uintptr_t mmu_idx)
+static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
 {
-    return env_tlb(env)->f[mmu_idx].mask + (1 << CPU_TLB_ENTRY_BITS);
+    return fast->mask + (1 << CPU_TLB_ENTRY_BITS);
 }
 
 static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
@@ -156,7 +156,7 @@ static void tlb_dyn_init(CPUArchState *env)
 static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
 {
     CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
-    size_t old_size = tlb_n_entries(env, mmu_idx);
+    size_t old_size = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
     size_t rate;
     size_t new_size = old_size;
     int64_t now = get_clock_realtime();
@@ -236,7 +236,8 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
     env_tlb(env)->d[mmu_idx].large_page_addr = -1;
     env_tlb(env)->d[mmu_idx].large_page_mask = -1;
     env_tlb(env)->d[mmu_idx].vindex = 0;
-    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
+    memset(env_tlb(env)->f[mmu_idx].table, -1,
+           sizeof_tlb(&env_tlb(env)->f[mmu_idx]));
     memset(env_tlb(env)->d[mmu_idx].vtable, -1,
            sizeof(env_tlb(env)->d[0].vtable));
 }
@@ -622,7 +623,7 @@ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length)
     qemu_spin_lock(&env_tlb(env)->c.lock);
     for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
         unsigned int i;
-        unsigned int n = tlb_n_entries(env, mmu_idx);
+        unsigned int n = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
 
         for (i = 0; i < n; i++) {
             tlb_reset_dirty_range_locked(&env_tlb(env)->f[mmu_idx].table[i],
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 4/9] cputlb: Hoist tlb portions in tlb_mmu_resize_locked
  2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
                   ` (2 preceding siblings ...)
  2020-01-09  2:49 ` [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb Richard Henderson
@ 2020-01-09  2:49 ` Richard Henderson
  2020-01-20  8:58   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2020-01-09  2:49 ` [PATCH 5/9] cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked Richard Henderson
                   ` (5 subsequent siblings)
  9 siblings, 3 replies; 37+ messages in thread
From: Richard Henderson @ 2020-01-09  2:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis

No functional change, but the smaller expressions make
the code easier to read.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 49c605b6d8..c7dc1dc85a 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -115,8 +115,8 @@ static void tlb_dyn_init(CPUArchState *env)
 
 /**
  * tlb_mmu_resize_locked() - perform TLB resize bookkeeping; resize if necessary
- * @env: CPU that owns the TLB
- * @mmu_idx: MMU index of the TLB
+ * @desc: The CPUTLBDesc portion of the TLB
+ * @fast: The CPUTLBDescFast portion of the same TLB
  *
  * Called with tlb_lock_held.
  *
@@ -153,10 +153,9 @@ static void tlb_dyn_init(CPUArchState *env)
  * high), since otherwise we are likely to have a significant amount of
  * conflict misses.
  */
-static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
+static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
 {
-    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
-    size_t old_size = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
+    size_t old_size = tlb_n_entries(fast);
     size_t rate;
     size_t new_size = old_size;
     int64_t now = get_clock_realtime();
@@ -198,14 +197,15 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
         return;
     }
 
-    g_free(env_tlb(env)->f[mmu_idx].table);
-    g_free(env_tlb(env)->d[mmu_idx].iotlb);
+    g_free(fast->table);
+    g_free(desc->iotlb);
 
     tlb_window_reset(desc, now, 0);
     /* desc->n_used_entries is cleared by the caller */
-    env_tlb(env)->f[mmu_idx].mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
-    env_tlb(env)->f[mmu_idx].table = g_try_new(CPUTLBEntry, new_size);
-    env_tlb(env)->d[mmu_idx].iotlb = g_try_new(CPUIOTLBEntry, new_size);
+    fast->mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
+    fast->table = g_try_new(CPUTLBEntry, new_size);
+    desc->iotlb = g_try_new(CPUIOTLBEntry, new_size);
+
     /*
      * If the allocations fail, try smaller sizes. We just freed some
      * memory, so going back to half of new_size has a good chance of working.
@@ -213,25 +213,24 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
      * allocations to fail though, so we progressively reduce the allocation
      * size, aborting if we cannot even allocate the smallest TLB we support.
      */
-    while (env_tlb(env)->f[mmu_idx].table == NULL ||
-           env_tlb(env)->d[mmu_idx].iotlb == NULL) {
+    while (fast->table == NULL || desc->iotlb == NULL) {
         if (new_size == (1 << CPU_TLB_DYN_MIN_BITS)) {
             error_report("%s: %s", __func__, strerror(errno));
             abort();
         }
         new_size = MAX(new_size >> 1, 1 << CPU_TLB_DYN_MIN_BITS);
-        env_tlb(env)->f[mmu_idx].mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
+        fast->mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
 
-        g_free(env_tlb(env)->f[mmu_idx].table);
-        g_free(env_tlb(env)->d[mmu_idx].iotlb);
-        env_tlb(env)->f[mmu_idx].table = g_try_new(CPUTLBEntry, new_size);
-        env_tlb(env)->d[mmu_idx].iotlb = g_try_new(CPUIOTLBEntry, new_size);
+        g_free(fast->table);
+        g_free(desc->iotlb);
+        fast->table = g_try_new(CPUTLBEntry, new_size);
+        desc->iotlb = g_try_new(CPUIOTLBEntry, new_size);
     }
 }
 
 static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
 {
-    tlb_mmu_resize_locked(env, mmu_idx);
+    tlb_mmu_resize_locked(&env_tlb(env)->d[mmu_idx], &env_tlb(env)->f[mmu_idx]);
     env_tlb(env)->d[mmu_idx].n_used_entries = 0;
     env_tlb(env)->d[mmu_idx].large_page_addr = -1;
     env_tlb(env)->d[mmu_idx].large_page_mask = -1;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 5/9] cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked
  2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
                   ` (3 preceding siblings ...)
  2020-01-09  2:49 ` [PATCH 4/9] cputlb: Hoist tlb portions in tlb_mmu_resize_locked Richard Henderson
@ 2020-01-09  2:49 ` Richard Henderson
  2020-01-20  8:58   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2020-01-09  2:49 ` [PATCH 6/9] cputlb: Split out tlb_mmu_flush_locked Richard Henderson
                   ` (4 subsequent siblings)
  9 siblings, 3 replies; 37+ messages in thread
From: Richard Henderson @ 2020-01-09  2:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis

No functional change, but the smaller expressions make
the code easier to read.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index c7dc1dc85a..eff427f137 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -230,15 +230,16 @@ static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
 
 static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
 {
-    tlb_mmu_resize_locked(&env_tlb(env)->d[mmu_idx], &env_tlb(env)->f[mmu_idx]);
-    env_tlb(env)->d[mmu_idx].n_used_entries = 0;
-    env_tlb(env)->d[mmu_idx].large_page_addr = -1;
-    env_tlb(env)->d[mmu_idx].large_page_mask = -1;
-    env_tlb(env)->d[mmu_idx].vindex = 0;
-    memset(env_tlb(env)->f[mmu_idx].table, -1,
-           sizeof_tlb(&env_tlb(env)->f[mmu_idx]));
-    memset(env_tlb(env)->d[mmu_idx].vtable, -1,
-           sizeof(env_tlb(env)->d[0].vtable));
+    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
+    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
+
+    tlb_mmu_resize_locked(desc, fast);
+    desc->n_used_entries = 0;
+    desc->large_page_addr = -1;
+    desc->large_page_mask = -1;
+    desc->vindex = 0;
+    memset(fast->table, -1, sizeof_tlb(fast));
+    memset(desc->vtable, -1, sizeof(desc->vtable));
 }
 
 static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 6/9] cputlb: Split out tlb_mmu_flush_locked
  2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
                   ` (4 preceding siblings ...)
  2020-01-09  2:49 ` [PATCH 5/9] cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked Richard Henderson
@ 2020-01-09  2:49 ` Richard Henderson
  2020-01-20  8:59   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2020-01-09  2:49 ` [PATCH 7/9] cputlb: Partially merge tlb_dyn_init into tlb_init Richard Henderson
                   ` (3 subsequent siblings)
  9 siblings, 3 replies; 37+ messages in thread
From: Richard Henderson @ 2020-01-09  2:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis

We will want to be able to flush a tlb without resizing.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index eff427f137..e60e501334 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -228,12 +228,8 @@ static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
     }
 }
 
-static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
+static void tlb_mmu_flush_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
 {
-    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
-    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
-
-    tlb_mmu_resize_locked(desc, fast);
     desc->n_used_entries = 0;
     desc->large_page_addr = -1;
     desc->large_page_mask = -1;
@@ -242,6 +238,15 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
     memset(desc->vtable, -1, sizeof(desc->vtable));
 }
 
+static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
+{
+    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
+    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
+
+    tlb_mmu_resize_locked(desc, fast);
+    tlb_mmu_flush_locked(desc, fast);
+}
+
 static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
 {
     env_tlb(env)->d[mmu_idx].n_used_entries++;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 7/9] cputlb: Partially merge tlb_dyn_init into tlb_init
  2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
                   ` (5 preceding siblings ...)
  2020-01-09  2:49 ` [PATCH 6/9] cputlb: Split out tlb_mmu_flush_locked Richard Henderson
@ 2020-01-09  2:49 ` Richard Henderson
  2020-01-20  9:01   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2020-01-09  2:49 ` [PATCH 8/9] cputlb: Initialize tlbs as flushed Richard Henderson
                   ` (2 subsequent siblings)
  9 siblings, 3 replies; 37+ messages in thread
From: Richard Henderson @ 2020-01-09  2:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis

Merge into the only caller, but at the same time split
out tlb_mmu_init to initialize a single tlb entry.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e60e501334..c7c34b185b 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -97,22 +97,6 @@ static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
     desc->window_max_entries = max_entries;
 }
 
-static void tlb_dyn_init(CPUArchState *env)
-{
-    int i;
-
-    for (i = 0; i < NB_MMU_MODES; i++) {
-        CPUTLBDesc *desc = &env_tlb(env)->d[i];
-        size_t n_entries = 1 << CPU_TLB_DYN_DEFAULT_BITS;
-
-        tlb_window_reset(desc, get_clock_realtime(), 0);
-        desc->n_used_entries = 0;
-        env_tlb(env)->f[i].mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
-        env_tlb(env)->f[i].table = g_new(CPUTLBEntry, n_entries);
-        env_tlb(env)->d[i].iotlb = g_new(CPUIOTLBEntry, n_entries);
-    }
-}
-
 /**
  * tlb_mmu_resize_locked() - perform TLB resize bookkeeping; resize if necessary
  * @desc: The CPUTLBDesc portion of the TLB
@@ -247,6 +231,17 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
     tlb_mmu_flush_locked(desc, fast);
 }
 
+static void tlb_mmu_init(CPUTLBDesc *desc, CPUTLBDescFast *fast, int64_t now)
+{
+    size_t n_entries = 1 << CPU_TLB_DYN_DEFAULT_BITS;
+
+    tlb_window_reset(desc, now, 0);
+    desc->n_used_entries = 0;
+    fast->mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
+    fast->table = g_new(CPUTLBEntry, n_entries);
+    desc->iotlb = g_new(CPUIOTLBEntry, n_entries);
+}
+
 static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
 {
     env_tlb(env)->d[mmu_idx].n_used_entries++;
@@ -260,13 +255,17 @@ static inline void tlb_n_used_entries_dec(CPUArchState *env, uintptr_t mmu_idx)
 void tlb_init(CPUState *cpu)
 {
     CPUArchState *env = cpu->env_ptr;
+    int64_t now = get_clock_realtime();
+    int i;
 
     qemu_spin_init(&env_tlb(env)->c.lock);
 
     /* Ensure that cpu_reset performs a full flush.  */
     env_tlb(env)->c.dirty = ALL_MMUIDX_BITS;
 
-    tlb_dyn_init(env);
+    for (i = 0; i < NB_MMU_MODES; i++) {
+        tlb_mmu_init(&env_tlb(env)->d[i], &env_tlb(env)->f[i], now);
+    }
 }
 
 /* flush_all_helper: run fn across all cpus
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 8/9] cputlb: Initialize tlbs as flushed
  2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
                   ` (6 preceding siblings ...)
  2020-01-09  2:49 ` [PATCH 7/9] cputlb: Partially merge tlb_dyn_init into tlb_init Richard Henderson
@ 2020-01-09  2:49 ` Richard Henderson
  2020-01-20 14:33   ` Alex Bennée
  2020-01-20 23:01   ` Alistair Francis
  2020-01-09  2:49 ` [PATCH 9/9] cputlb: Hoist timestamp outside of loops over tlbs Richard Henderson
  2020-01-20 23:03 ` [PATCH 0/9] cputlb: Various cleanups Alistair Francis
  9 siblings, 2 replies; 37+ messages in thread
From: Richard Henderson @ 2020-01-09  2:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis

There's little point in leaving these data structures half initialized,
and relying on a flush to be done during reset.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index c7c34b185b..761e9d44d7 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -240,6 +240,7 @@ static void tlb_mmu_init(CPUTLBDesc *desc, CPUTLBDescFast *fast, int64_t now)
     fast->mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
     fast->table = g_new(CPUTLBEntry, n_entries);
     desc->iotlb = g_new(CPUIOTLBEntry, n_entries);
+    tlb_mmu_flush_locked(desc, fast);
 }
 
 static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
@@ -260,8 +261,8 @@ void tlb_init(CPUState *cpu)
 
     qemu_spin_init(&env_tlb(env)->c.lock);
 
-    /* Ensure that cpu_reset performs a full flush.  */
-    env_tlb(env)->c.dirty = ALL_MMUIDX_BITS;
+    /* All tlbs are initialized flushed. */
+    env_tlb(env)->c.dirty = 0;
 
     for (i = 0; i < NB_MMU_MODES; i++) {
         tlb_mmu_init(&env_tlb(env)->d[i], &env_tlb(env)->f[i], now);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* [PATCH 9/9] cputlb: Hoist timestamp outside of loops over tlbs
  2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
                   ` (7 preceding siblings ...)
  2020-01-09  2:49 ` [PATCH 8/9] cputlb: Initialize tlbs as flushed Richard Henderson
@ 2020-01-09  2:49 ` Richard Henderson
  2020-01-20  9:02   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2020-01-20 23:03 ` [PATCH 0/9] cputlb: Various cleanups Alistair Francis
  9 siblings, 3 replies; 37+ messages in thread
From: Richard Henderson @ 2020-01-09  2:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: alistair.francis

Do not call get_clock_realtime() in tlb_mmu_resize_locked,
but hoist outside of any loop over a set of tlbs.  This is
only two (indirect) callers, tlb_flush_by_mmuidx_async_work
and tlb_flush_page_locked, so not onerous.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cputlb.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 761e9d44d7..9f6cb36921 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -137,12 +137,12 @@ static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
  * high), since otherwise we are likely to have a significant amount of
  * conflict misses.
  */
-static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
+static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast,
+                                  int64_t now)
 {
     size_t old_size = tlb_n_entries(fast);
     size_t rate;
     size_t new_size = old_size;
-    int64_t now = get_clock_realtime();
     int64_t window_len_ms = 100;
     int64_t window_len_ns = window_len_ms * 1000 * 1000;
     bool window_expired = now > desc->window_begin_ns + window_len_ns;
@@ -222,12 +222,13 @@ static void tlb_mmu_flush_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
     memset(desc->vtable, -1, sizeof(desc->vtable));
 }
 
-static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
+static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx,
+                                        int64_t now)
 {
     CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
     CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
 
-    tlb_mmu_resize_locked(desc, fast);
+    tlb_mmu_resize_locked(desc, fast, now);
     tlb_mmu_flush_locked(desc, fast);
 }
 
@@ -310,6 +311,7 @@ static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
     CPUArchState *env = cpu->env_ptr;
     uint16_t asked = data.host_int;
     uint16_t all_dirty, work, to_clean;
+    int64_t now = get_clock_realtime();
 
     assert_cpu_is_self(cpu);
 
@@ -324,7 +326,7 @@ static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
 
     for (work = to_clean; work != 0; work &= work - 1) {
         int mmu_idx = ctz32(work);
-        tlb_flush_one_mmuidx_locked(env, mmu_idx);
+        tlb_flush_one_mmuidx_locked(env, mmu_idx, now);
     }
 
     qemu_spin_unlock(&env_tlb(env)->c.lock);
@@ -446,7 +448,7 @@ static void tlb_flush_page_locked(CPUArchState *env, int midx,
         tlb_debug("forcing full flush midx %d ("
                   TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
                   midx, lp_addr, lp_mask);
-        tlb_flush_one_mmuidx_locked(env, midx);
+        tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime());
     } else {
         if (tlb_flush_entry_locked(tlb_entry(env, midx, page), page)) {
             tlb_n_used_entries_dec(env, midx);
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 37+ messages in thread

* Re: [PATCH 1/9] cputlb: Merge tlb_table_flush_by_mmuidx into tlb_flush_one_mmuidx_locked
  2020-01-09  2:48 ` [PATCH 1/9] cputlb: Merge tlb_table_flush_by_mmuidx into tlb_flush_one_mmuidx_locked Richard Henderson
@ 2020-01-20  0:34   ` Alistair Francis
  2020-01-20 13:34   ` Alex Bennée
  1 sibling, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-01-20  0:34 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers

On Thu, Jan 9, 2020 at 12:49 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> There is only one caller for tlb_table_flush_by_mmuidx.  Place
> the result at the earlier line number, due to an expected user
> in the near future.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cputlb.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index a991ea2964..1a81886e58 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -224,11 +224,16 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>      }
>  }
>
> -static inline void tlb_table_flush_by_mmuidx(CPUArchState *env, int mmu_idx)
> +static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>  {
>      tlb_mmu_resize_locked(env, mmu_idx);
> -    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
>      env_tlb(env)->d[mmu_idx].n_used_entries = 0;
> +    env_tlb(env)->d[mmu_idx].large_page_addr = -1;
> +    env_tlb(env)->d[mmu_idx].large_page_mask = -1;
> +    env_tlb(env)->d[mmu_idx].vindex = 0;
> +    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
> +    memset(env_tlb(env)->d[mmu_idx].vtable, -1,
> +           sizeof(env_tlb(env)->d[0].vtable));
>  }
>
>  static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
> @@ -289,16 +294,6 @@ void tlb_flush_counts(size_t *pfull, size_t *ppart, size_t *pelide)
>      *pelide = elide;
>  }
>
> -static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> -{
> -    tlb_table_flush_by_mmuidx(env, mmu_idx);
> -    env_tlb(env)->d[mmu_idx].large_page_addr = -1;
> -    env_tlb(env)->d[mmu_idx].large_page_mask = -1;
> -    env_tlb(env)->d[mmu_idx].vindex = 0;
> -    memset(env_tlb(env)->d[mmu_idx].vtable, -1,
> -           sizeof(env_tlb(env)->d[0].vtable));
> -}
> -
>  static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
>  {
>      CPUArchState *env = cpu->env_ptr;
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/9] cputlb: Make tlb_n_entries private to cputlb.c
  2020-01-09  2:49 ` [PATCH 2/9] cputlb: Make tlb_n_entries private to cputlb.c Richard Henderson
@ 2020-01-20  0:36   ` Alistair Francis
  2020-01-20  8:53   ` Philippe Mathieu-Daudé
  2020-01-20 13:34   ` Alex Bennée
  2 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-01-20  0:36 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers

On Thu, Jan 9, 2020 at 12:49 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> There are no users of this function outside cputlb.c,
> and its interface will change in the next patch.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/exec/cpu_ldst.h | 5 -----
>  accel/tcg/cputlb.c      | 5 +++++
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index a46116167c..53de19753a 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -234,11 +234,6 @@ static inline uintptr_t tlb_index(CPUArchState *env, uintptr_t mmu_idx,
>      return (addr >> TARGET_PAGE_BITS) & size_mask;
>  }
>
> -static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
> -{
> -    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
> -}
> -
>  /* Find the TLB entry corresponding to the mmu_idx + address pair.  */
>  static inline CPUTLBEntry *tlb_entry(CPUArchState *env, uintptr_t mmu_idx,
>                                       target_ulong addr)
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 1a81886e58..e4a8ed9534 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -80,6 +80,11 @@ QEMU_BUILD_BUG_ON(sizeof(target_ulong) > sizeof(run_on_cpu_data));
>  QEMU_BUILD_BUG_ON(NB_MMU_MODES > 16);
>  #define ALL_MMUIDX_BITS ((1 << NB_MMU_MODES) - 1)
>
> +static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
> +{
> +    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
> +}
> +
>  static inline size_t sizeof_tlb(CPUArchState *env, uintptr_t mmu_idx)
>  {
>      return env_tlb(env)->f[mmu_idx].mask + (1 << CPU_TLB_ENTRY_BITS);
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb
  2020-01-09  2:49 ` [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb Richard Henderson
@ 2020-01-20  0:37   ` Alistair Francis
  2020-01-20  8:54   ` Philippe Mathieu-Daudé
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-01-20  0:37 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers

On Thu, Jan 9, 2020 at 12:49 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We do not need the entire CPUArchState to compute these values.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cputlb.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e4a8ed9534..49c605b6d8 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -80,14 +80,14 @@ QEMU_BUILD_BUG_ON(sizeof(target_ulong) > sizeof(run_on_cpu_data));
>  QEMU_BUILD_BUG_ON(NB_MMU_MODES > 16);
>  #define ALL_MMUIDX_BITS ((1 << NB_MMU_MODES) - 1)
>
> -static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
> +static inline size_t tlb_n_entries(CPUTLBDescFast *fast)
>  {
> -    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
> +    return (fast->mask >> CPU_TLB_ENTRY_BITS) + 1;
>  }
>
> -static inline size_t sizeof_tlb(CPUArchState *env, uintptr_t mmu_idx)
> +static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
>  {
> -    return env_tlb(env)->f[mmu_idx].mask + (1 << CPU_TLB_ENTRY_BITS);
> +    return fast->mask + (1 << CPU_TLB_ENTRY_BITS);
>  }
>
>  static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
> @@ -156,7 +156,7 @@ static void tlb_dyn_init(CPUArchState *env)
>  static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>  {
>      CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> -    size_t old_size = tlb_n_entries(env, mmu_idx);
> +    size_t old_size = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
>      size_t rate;
>      size_t new_size = old_size;
>      int64_t now = get_clock_realtime();
> @@ -236,7 +236,8 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>      env_tlb(env)->d[mmu_idx].large_page_addr = -1;
>      env_tlb(env)->d[mmu_idx].large_page_mask = -1;
>      env_tlb(env)->d[mmu_idx].vindex = 0;
> -    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
> +    memset(env_tlb(env)->f[mmu_idx].table, -1,
> +           sizeof_tlb(&env_tlb(env)->f[mmu_idx]));
>      memset(env_tlb(env)->d[mmu_idx].vtable, -1,
>             sizeof(env_tlb(env)->d[0].vtable));
>  }
> @@ -622,7 +623,7 @@ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length)
>      qemu_spin_lock(&env_tlb(env)->c.lock);
>      for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
>          unsigned int i;
> -        unsigned int n = tlb_n_entries(env, mmu_idx);
> +        unsigned int n = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
>
>          for (i = 0; i < n; i++) {
>              tlb_reset_dirty_range_locked(&env_tlb(env)->f[mmu_idx].table[i],
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/9] cputlb: Make tlb_n_entries private to cputlb.c
  2020-01-09  2:49 ` [PATCH 2/9] cputlb: Make tlb_n_entries private to cputlb.c Richard Henderson
  2020-01-20  0:36   ` Alistair Francis
@ 2020-01-20  8:53   ` Philippe Mathieu-Daudé
  2020-01-20 13:34   ` Alex Bennée
  2 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-20  8:53 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: alistair.francis

On 1/9/20 3:49 AM, Richard Henderson wrote:
> There are no users of this function outside cputlb.c,
> and its interface will change in the next patch.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   include/exec/cpu_ldst.h | 5 -----
>   accel/tcg/cputlb.c      | 5 +++++
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index a46116167c..53de19753a 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -234,11 +234,6 @@ static inline uintptr_t tlb_index(CPUArchState *env, uintptr_t mmu_idx,
>       return (addr >> TARGET_PAGE_BITS) & size_mask;
>   }
>   
> -static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
> -{
> -    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
> -}
> -
>   /* Find the TLB entry corresponding to the mmu_idx + address pair.  */
>   static inline CPUTLBEntry *tlb_entry(CPUArchState *env, uintptr_t mmu_idx,
>                                        target_ulong addr)
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 1a81886e58..e4a8ed9534 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -80,6 +80,11 @@ QEMU_BUILD_BUG_ON(sizeof(target_ulong) > sizeof(run_on_cpu_data));
>   QEMU_BUILD_BUG_ON(NB_MMU_MODES > 16);
>   #define ALL_MMUIDX_BITS ((1 << NB_MMU_MODES) - 1)
>   
> +static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
> +{
> +    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
> +}
> +
>   static inline size_t sizeof_tlb(CPUArchState *env, uintptr_t mmu_idx)
>   {
>       return env_tlb(env)->f[mmu_idx].mask + (1 << CPU_TLB_ENTRY_BITS);
> 



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb
  2020-01-09  2:49 ` [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb Richard Henderson
  2020-01-20  0:37   ` Alistair Francis
@ 2020-01-20  8:54   ` Philippe Mathieu-Daudé
  2020-01-20 13:36   ` Alex Bennée
  2020-01-20 13:39   ` Alex Bennée
  3 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-20  8:54 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: alistair.francis

On 1/9/20 3:49 AM, Richard Henderson wrote:
> We do not need the entire CPUArchState to compute these values.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/tcg/cputlb.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e4a8ed9534..49c605b6d8 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -80,14 +80,14 @@ QEMU_BUILD_BUG_ON(sizeof(target_ulong) > sizeof(run_on_cpu_data));
>   QEMU_BUILD_BUG_ON(NB_MMU_MODES > 16);
>   #define ALL_MMUIDX_BITS ((1 << NB_MMU_MODES) - 1)
>   
> -static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
> +static inline size_t tlb_n_entries(CPUTLBDescFast *fast)
>   {
> -    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
> +    return (fast->mask >> CPU_TLB_ENTRY_BITS) + 1;
>   }
>   
> -static inline size_t sizeof_tlb(CPUArchState *env, uintptr_t mmu_idx)
> +static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
>   {
> -    return env_tlb(env)->f[mmu_idx].mask + (1 << CPU_TLB_ENTRY_BITS);
> +    return fast->mask + (1 << CPU_TLB_ENTRY_BITS);
>   }
>   
>   static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
> @@ -156,7 +156,7 @@ static void tlb_dyn_init(CPUArchState *env)
>   static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>   {
>       CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> -    size_t old_size = tlb_n_entries(env, mmu_idx);
> +    size_t old_size = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
>       size_t rate;
>       size_t new_size = old_size;
>       int64_t now = get_clock_realtime();
> @@ -236,7 +236,8 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>       env_tlb(env)->d[mmu_idx].large_page_addr = -1;
>       env_tlb(env)->d[mmu_idx].large_page_mask = -1;
>       env_tlb(env)->d[mmu_idx].vindex = 0;
> -    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
> +    memset(env_tlb(env)->f[mmu_idx].table, -1,
> +           sizeof_tlb(&env_tlb(env)->f[mmu_idx]));
>       memset(env_tlb(env)->d[mmu_idx].vtable, -1,
>              sizeof(env_tlb(env)->d[0].vtable));
>   }
> @@ -622,7 +623,7 @@ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length)
>       qemu_spin_lock(&env_tlb(env)->c.lock);
>       for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
>           unsigned int i;
> -        unsigned int n = tlb_n_entries(env, mmu_idx);
> +        unsigned int n = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
>   
>           for (i = 0; i < n; i++) {
>               tlb_reset_dirty_range_locked(&env_tlb(env)->f[mmu_idx].table[i],
> 



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 4/9] cputlb: Hoist tlb portions in tlb_mmu_resize_locked
  2020-01-09  2:49 ` [PATCH 4/9] cputlb: Hoist tlb portions in tlb_mmu_resize_locked Richard Henderson
@ 2020-01-20  8:58   ` Philippe Mathieu-Daudé
  2020-01-20 12:05   ` Alistair Francis
  2020-01-20 13:40   ` Alex Bennée
  2 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-20  8:58 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: alistair.francis

On 1/9/20 3:49 AM, Richard Henderson wrote:
> No functional change, but the smaller expressions make
> the code easier to read.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/tcg/cputlb.c | 35 +++++++++++++++++------------------
>   1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 49c605b6d8..c7dc1dc85a 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -115,8 +115,8 @@ static void tlb_dyn_init(CPUArchState *env)
>   
>   /**
>    * tlb_mmu_resize_locked() - perform TLB resize bookkeeping; resize if necessary
> - * @env: CPU that owns the TLB
> - * @mmu_idx: MMU index of the TLB
> + * @desc: The CPUTLBDesc portion of the TLB
> + * @fast: The CPUTLBDescFast portion of the same TLB
>    *
>    * Called with tlb_lock_held.
>    *
> @@ -153,10 +153,9 @@ static void tlb_dyn_init(CPUArchState *env)
>    * high), since otherwise we are likely to have a significant amount of
>    * conflict misses.
>    */
> -static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
> +static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>   {
> -    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> -    size_t old_size = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
> +    size_t old_size = tlb_n_entries(fast);
>       size_t rate;
>       size_t new_size = old_size;
>       int64_t now = get_clock_realtime();
> @@ -198,14 +197,15 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>           return;
>       }
>   
> -    g_free(env_tlb(env)->f[mmu_idx].table);
> -    g_free(env_tlb(env)->d[mmu_idx].iotlb);
> +    g_free(fast->table);
> +    g_free(desc->iotlb);
>   
>       tlb_window_reset(desc, now, 0);
>       /* desc->n_used_entries is cleared by the caller */
> -    env_tlb(env)->f[mmu_idx].mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
> -    env_tlb(env)->f[mmu_idx].table = g_try_new(CPUTLBEntry, new_size);
> -    env_tlb(env)->d[mmu_idx].iotlb = g_try_new(CPUIOTLBEntry, new_size);
> +    fast->mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
> +    fast->table = g_try_new(CPUTLBEntry, new_size);
> +    desc->iotlb = g_try_new(CPUIOTLBEntry, new_size);
> +
>       /*
>        * If the allocations fail, try smaller sizes. We just freed some
>        * memory, so going back to half of new_size has a good chance of working.
> @@ -213,25 +213,24 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>        * allocations to fail though, so we progressively reduce the allocation
>        * size, aborting if we cannot even allocate the smallest TLB we support.
>        */
> -    while (env_tlb(env)->f[mmu_idx].table == NULL ||
> -           env_tlb(env)->d[mmu_idx].iotlb == NULL) {
> +    while (fast->table == NULL || desc->iotlb == NULL) {
>           if (new_size == (1 << CPU_TLB_DYN_MIN_BITS)) {
>               error_report("%s: %s", __func__, strerror(errno));
>               abort();
>           }
>           new_size = MAX(new_size >> 1, 1 << CPU_TLB_DYN_MIN_BITS);
> -        env_tlb(env)->f[mmu_idx].mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
> +        fast->mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
>   
> -        g_free(env_tlb(env)->f[mmu_idx].table);
> -        g_free(env_tlb(env)->d[mmu_idx].iotlb);
> -        env_tlb(env)->f[mmu_idx].table = g_try_new(CPUTLBEntry, new_size);
> -        env_tlb(env)->d[mmu_idx].iotlb = g_try_new(CPUIOTLBEntry, new_size);
> +        g_free(fast->table);
> +        g_free(desc->iotlb);
> +        fast->table = g_try_new(CPUTLBEntry, new_size);
> +        desc->iotlb = g_try_new(CPUIOTLBEntry, new_size);
>       }
>   }
>   
>   static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>   {
> -    tlb_mmu_resize_locked(env, mmu_idx);
> +    tlb_mmu_resize_locked(&env_tlb(env)->d[mmu_idx], &env_tlb(env)->f[mmu_idx]);
>       env_tlb(env)->d[mmu_idx].n_used_entries = 0;
>       env_tlb(env)->d[mmu_idx].large_page_addr = -1;
>       env_tlb(env)->d[mmu_idx].large_page_mask = -1;
> 



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 5/9] cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked
  2020-01-09  2:49 ` [PATCH 5/9] cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked Richard Henderson
@ 2020-01-20  8:58   ` Philippe Mathieu-Daudé
  2020-01-20 13:41   ` Alex Bennée
  2020-01-20 22:56   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-20  8:58 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: alistair.francis

On 1/9/20 3:49 AM, Richard Henderson wrote:
> No functional change, but the smaller expressions make
> the code easier to read.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/tcg/cputlb.c | 19 ++++++++++---------
>   1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index c7dc1dc85a..eff427f137 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -230,15 +230,16 @@ static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>   
>   static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>   {
> -    tlb_mmu_resize_locked(&env_tlb(env)->d[mmu_idx], &env_tlb(env)->f[mmu_idx]);
> -    env_tlb(env)->d[mmu_idx].n_used_entries = 0;
> -    env_tlb(env)->d[mmu_idx].large_page_addr = -1;
> -    env_tlb(env)->d[mmu_idx].large_page_mask = -1;
> -    env_tlb(env)->d[mmu_idx].vindex = 0;
> -    memset(env_tlb(env)->f[mmu_idx].table, -1,
> -           sizeof_tlb(&env_tlb(env)->f[mmu_idx]));
> -    memset(env_tlb(env)->d[mmu_idx].vtable, -1,
> -           sizeof(env_tlb(env)->d[0].vtable));
> +    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> +    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
> +
> +    tlb_mmu_resize_locked(desc, fast);
> +    desc->n_used_entries = 0;
> +    desc->large_page_addr = -1;
> +    desc->large_page_mask = -1;
> +    desc->vindex = 0;
> +    memset(fast->table, -1, sizeof_tlb(fast));
> +    memset(desc->vtable, -1, sizeof(desc->vtable));
>   }
>   
>   static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
> 



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 6/9] cputlb: Split out tlb_mmu_flush_locked
  2020-01-09  2:49 ` [PATCH 6/9] cputlb: Split out tlb_mmu_flush_locked Richard Henderson
@ 2020-01-20  8:59   ` Philippe Mathieu-Daudé
  2020-01-20 13:41   ` Alex Bennée
  2020-01-20 22:58   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-20  8:59 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: alistair.francis

On 1/9/20 3:49 AM, Richard Henderson wrote:
> We will want to be able to flush a tlb without resizing.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/tcg/cputlb.c | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index eff427f137..e60e501334 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -228,12 +228,8 @@ static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>       }
>   }
>   
> -static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> +static void tlb_mmu_flush_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>   {
> -    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> -    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
> -
> -    tlb_mmu_resize_locked(desc, fast);
>       desc->n_used_entries = 0;
>       desc->large_page_addr = -1;
>       desc->large_page_mask = -1;
> @@ -242,6 +238,15 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>       memset(desc->vtable, -1, sizeof(desc->vtable));
>   }
>   
> +static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> +{
> +    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> +    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
> +
> +    tlb_mmu_resize_locked(desc, fast);
> +    tlb_mmu_flush_locked(desc, fast);
> +}
> +
>   static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
>   {
>       env_tlb(env)->d[mmu_idx].n_used_entries++;
> 



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 7/9] cputlb: Partially merge tlb_dyn_init into tlb_init
  2020-01-09  2:49 ` [PATCH 7/9] cputlb: Partially merge tlb_dyn_init into tlb_init Richard Henderson
@ 2020-01-20  9:01   ` Philippe Mathieu-Daudé
  2020-01-20 14:33   ` Alex Bennée
  2020-01-20 23:00   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-20  9:01 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: alistair.francis

On 1/9/20 3:49 AM, Richard Henderson wrote:
> Merge into the only caller, but at the same time split
> out tlb_mmu_init to initialize a single tlb entry.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/tcg/cputlb.c | 33 ++++++++++++++++-----------------
>   1 file changed, 16 insertions(+), 17 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e60e501334..c7c34b185b 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -97,22 +97,6 @@ static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
>       desc->window_max_entries = max_entries;
>   }
>   
> -static void tlb_dyn_init(CPUArchState *env)
> -{
> -    int i;
> -
> -    for (i = 0; i < NB_MMU_MODES; i++) {
> -        CPUTLBDesc *desc = &env_tlb(env)->d[i];
> -        size_t n_entries = 1 << CPU_TLB_DYN_DEFAULT_BITS;
> -
> -        tlb_window_reset(desc, get_clock_realtime(), 0);
> -        desc->n_used_entries = 0;
> -        env_tlb(env)->f[i].mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
> -        env_tlb(env)->f[i].table = g_new(CPUTLBEntry, n_entries);
> -        env_tlb(env)->d[i].iotlb = g_new(CPUIOTLBEntry, n_entries);
> -    }
> -}
> -
>   /**
>    * tlb_mmu_resize_locked() - perform TLB resize bookkeeping; resize if necessary
>    * @desc: The CPUTLBDesc portion of the TLB
> @@ -247,6 +231,17 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>       tlb_mmu_flush_locked(desc, fast);
>   }
>   
> +static void tlb_mmu_init(CPUTLBDesc *desc, CPUTLBDescFast *fast, int64_t now)
> +{
> +    size_t n_entries = 1 << CPU_TLB_DYN_DEFAULT_BITS;
> +
> +    tlb_window_reset(desc, now, 0);
> +    desc->n_used_entries = 0;
> +    fast->mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
> +    fast->table = g_new(CPUTLBEntry, n_entries);
> +    desc->iotlb = g_new(CPUIOTLBEntry, n_entries);
> +}
> +
>   static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
>   {
>       env_tlb(env)->d[mmu_idx].n_used_entries++;
> @@ -260,13 +255,17 @@ static inline void tlb_n_used_entries_dec(CPUArchState *env, uintptr_t mmu_idx)
>   void tlb_init(CPUState *cpu)
>   {
>       CPUArchState *env = cpu->env_ptr;
> +    int64_t now = get_clock_realtime();
> +    int i;
>   
>       qemu_spin_init(&env_tlb(env)->c.lock);
>   
>       /* Ensure that cpu_reset performs a full flush.  */
>       env_tlb(env)->c.dirty = ALL_MMUIDX_BITS;
>   
> -    tlb_dyn_init(env);
> +    for (i = 0; i < NB_MMU_MODES; i++) {
> +        tlb_mmu_init(&env_tlb(env)->d[i], &env_tlb(env)->f[i], now);
> +    }
>   }
>   
>   /* flush_all_helper: run fn across all cpus
> 



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 9/9] cputlb: Hoist timestamp outside of loops over tlbs
  2020-01-09  2:49 ` [PATCH 9/9] cputlb: Hoist timestamp outside of loops over tlbs Richard Henderson
@ 2020-01-20  9:02   ` Philippe Mathieu-Daudé
  2020-01-20 14:36   ` Alex Bennée
  2020-01-20 23:02   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-20  9:02 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: alistair.francis

On 1/9/20 3:49 AM, Richard Henderson wrote:
> Do not call get_clock_realtime() in tlb_mmu_resize_locked,
> but hoist outside of any loop over a set of tlbs.  This is
> only two (indirect) callers, tlb_flush_by_mmuidx_async_work
> and tlb_flush_page_locked, so not onerous.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   accel/tcg/cputlb.c | 14 ++++++++------
>   1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 761e9d44d7..9f6cb36921 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -137,12 +137,12 @@ static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
>    * high), since otherwise we are likely to have a significant amount of
>    * conflict misses.
>    */
> -static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
> +static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast,
> +                                  int64_t now)
>   {
>       size_t old_size = tlb_n_entries(fast);
>       size_t rate;
>       size_t new_size = old_size;
> -    int64_t now = get_clock_realtime();
>       int64_t window_len_ms = 100;
>       int64_t window_len_ns = window_len_ms * 1000 * 1000;
>       bool window_expired = now > desc->window_begin_ns + window_len_ns;
> @@ -222,12 +222,13 @@ static void tlb_mmu_flush_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>       memset(desc->vtable, -1, sizeof(desc->vtable));
>   }
>   
> -static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> +static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx,
> +                                        int64_t now)
>   {
>       CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
>       CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
>   
> -    tlb_mmu_resize_locked(desc, fast);
> +    tlb_mmu_resize_locked(desc, fast, now);
>       tlb_mmu_flush_locked(desc, fast);
>   }
>   
> @@ -310,6 +311,7 @@ static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
>       CPUArchState *env = cpu->env_ptr;
>       uint16_t asked = data.host_int;
>       uint16_t all_dirty, work, to_clean;
> +    int64_t now = get_clock_realtime();
>   
>       assert_cpu_is_self(cpu);
>   
> @@ -324,7 +326,7 @@ static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
>   
>       for (work = to_clean; work != 0; work &= work - 1) {
>           int mmu_idx = ctz32(work);
> -        tlb_flush_one_mmuidx_locked(env, mmu_idx);
> +        tlb_flush_one_mmuidx_locked(env, mmu_idx, now);
>       }
>   
>       qemu_spin_unlock(&env_tlb(env)->c.lock);
> @@ -446,7 +448,7 @@ static void tlb_flush_page_locked(CPUArchState *env, int midx,
>           tlb_debug("forcing full flush midx %d ("
>                     TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
>                     midx, lp_addr, lp_mask);
> -        tlb_flush_one_mmuidx_locked(env, midx);
> +        tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime());
>       } else {
>           if (tlb_flush_entry_locked(tlb_entry(env, midx, page), page)) {
>               tlb_n_used_entries_dec(env, midx);
> 



^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 4/9] cputlb: Hoist tlb portions in tlb_mmu_resize_locked
  2020-01-09  2:49 ` [PATCH 4/9] cputlb: Hoist tlb portions in tlb_mmu_resize_locked Richard Henderson
  2020-01-20  8:58   ` Philippe Mathieu-Daudé
@ 2020-01-20 12:05   ` Alistair Francis
  2020-01-20 13:40   ` Alex Bennée
  2 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-01-20 12:05 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers

On Thu, Jan 9, 2020 at 12:52 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> No functional change, but the smaller expressions make
> the code easier to read.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cputlb.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 49c605b6d8..c7dc1dc85a 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -115,8 +115,8 @@ static void tlb_dyn_init(CPUArchState *env)
>
>  /**
>   * tlb_mmu_resize_locked() - perform TLB resize bookkeeping; resize if necessary
> - * @env: CPU that owns the TLB
> - * @mmu_idx: MMU index of the TLB
> + * @desc: The CPUTLBDesc portion of the TLB
> + * @fast: The CPUTLBDescFast portion of the same TLB
>   *
>   * Called with tlb_lock_held.
>   *
> @@ -153,10 +153,9 @@ static void tlb_dyn_init(CPUArchState *env)
>   * high), since otherwise we are likely to have a significant amount of
>   * conflict misses.
>   */
> -static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
> +static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>  {
> -    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> -    size_t old_size = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
> +    size_t old_size = tlb_n_entries(fast);
>      size_t rate;
>      size_t new_size = old_size;
>      int64_t now = get_clock_realtime();
> @@ -198,14 +197,15 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>          return;
>      }
>
> -    g_free(env_tlb(env)->f[mmu_idx].table);
> -    g_free(env_tlb(env)->d[mmu_idx].iotlb);
> +    g_free(fast->table);
> +    g_free(desc->iotlb);
>
>      tlb_window_reset(desc, now, 0);
>      /* desc->n_used_entries is cleared by the caller */
> -    env_tlb(env)->f[mmu_idx].mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
> -    env_tlb(env)->f[mmu_idx].table = g_try_new(CPUTLBEntry, new_size);
> -    env_tlb(env)->d[mmu_idx].iotlb = g_try_new(CPUIOTLBEntry, new_size);
> +    fast->mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
> +    fast->table = g_try_new(CPUTLBEntry, new_size);
> +    desc->iotlb = g_try_new(CPUIOTLBEntry, new_size);
> +
>      /*
>       * If the allocations fail, try smaller sizes. We just freed some
>       * memory, so going back to half of new_size has a good chance of working.
> @@ -213,25 +213,24 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>       * allocations to fail though, so we progressively reduce the allocation
>       * size, aborting if we cannot even allocate the smallest TLB we support.
>       */
> -    while (env_tlb(env)->f[mmu_idx].table == NULL ||
> -           env_tlb(env)->d[mmu_idx].iotlb == NULL) {
> +    while (fast->table == NULL || desc->iotlb == NULL) {
>          if (new_size == (1 << CPU_TLB_DYN_MIN_BITS)) {
>              error_report("%s: %s", __func__, strerror(errno));
>              abort();
>          }
>          new_size = MAX(new_size >> 1, 1 << CPU_TLB_DYN_MIN_BITS);
> -        env_tlb(env)->f[mmu_idx].mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
> +        fast->mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
>
> -        g_free(env_tlb(env)->f[mmu_idx].table);
> -        g_free(env_tlb(env)->d[mmu_idx].iotlb);
> -        env_tlb(env)->f[mmu_idx].table = g_try_new(CPUTLBEntry, new_size);
> -        env_tlb(env)->d[mmu_idx].iotlb = g_try_new(CPUIOTLBEntry, new_size);
> +        g_free(fast->table);
> +        g_free(desc->iotlb);
> +        fast->table = g_try_new(CPUTLBEntry, new_size);
> +        desc->iotlb = g_try_new(CPUIOTLBEntry, new_size);
>      }
>  }
>
>  static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>  {
> -    tlb_mmu_resize_locked(env, mmu_idx);
> +    tlb_mmu_resize_locked(&env_tlb(env)->d[mmu_idx], &env_tlb(env)->f[mmu_idx]);
>      env_tlb(env)->d[mmu_idx].n_used_entries = 0;
>      env_tlb(env)->d[mmu_idx].large_page_addr = -1;
>      env_tlb(env)->d[mmu_idx].large_page_mask = -1;
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 1/9] cputlb: Merge tlb_table_flush_by_mmuidx into tlb_flush_one_mmuidx_locked
  2020-01-09  2:48 ` [PATCH 1/9] cputlb: Merge tlb_table_flush_by_mmuidx into tlb_flush_one_mmuidx_locked Richard Henderson
  2020-01-20  0:34   ` Alistair Francis
@ 2020-01-20 13:34   ` Alex Bennée
  1 sibling, 0 replies; 37+ messages in thread
From: Alex Bennée @ 2020-01-20 13:34 UTC (permalink / raw)
  To: Richard Henderson; +Cc: alistair.francis, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> There is only one caller for tlb_table_flush_by_mmuidx.  Place
> the result at the earlier line number, due to an expected user
> in the near future.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  accel/tcg/cputlb.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index a991ea2964..1a81886e58 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -224,11 +224,16 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>      }
>  }
>  
> -static inline void tlb_table_flush_by_mmuidx(CPUArchState *env, int mmu_idx)
> +static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>  {
>      tlb_mmu_resize_locked(env, mmu_idx);
> -    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
>      env_tlb(env)->d[mmu_idx].n_used_entries = 0;
> +    env_tlb(env)->d[mmu_idx].large_page_addr = -1;
> +    env_tlb(env)->d[mmu_idx].large_page_mask = -1;
> +    env_tlb(env)->d[mmu_idx].vindex = 0;
> +    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
> +    memset(env_tlb(env)->d[mmu_idx].vtable, -1,
> +           sizeof(env_tlb(env)->d[0].vtable));
>  }
>  
>  static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
> @@ -289,16 +294,6 @@ void tlb_flush_counts(size_t *pfull, size_t *ppart, size_t *pelide)
>      *pelide = elide;
>  }
>  
> -static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> -{
> -    tlb_table_flush_by_mmuidx(env, mmu_idx);
> -    env_tlb(env)->d[mmu_idx].large_page_addr = -1;
> -    env_tlb(env)->d[mmu_idx].large_page_mask = -1;
> -    env_tlb(env)->d[mmu_idx].vindex = 0;
> -    memset(env_tlb(env)->d[mmu_idx].vtable, -1,
> -           sizeof(env_tlb(env)->d[0].vtable));
> -}
> -
>  static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
>  {
>      CPUArchState *env = cpu->env_ptr;


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 2/9] cputlb: Make tlb_n_entries private to cputlb.c
  2020-01-09  2:49 ` [PATCH 2/9] cputlb: Make tlb_n_entries private to cputlb.c Richard Henderson
  2020-01-20  0:36   ` Alistair Francis
  2020-01-20  8:53   ` Philippe Mathieu-Daudé
@ 2020-01-20 13:34   ` Alex Bennée
  2 siblings, 0 replies; 37+ messages in thread
From: Alex Bennée @ 2020-01-20 13:34 UTC (permalink / raw)
  To: Richard Henderson; +Cc: alistair.francis, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> There are no users of this function outside cputlb.c,
> and its interface will change in the next patch.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  include/exec/cpu_ldst.h | 5 -----
>  accel/tcg/cputlb.c      | 5 +++++
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index a46116167c..53de19753a 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -234,11 +234,6 @@ static inline uintptr_t tlb_index(CPUArchState *env, uintptr_t mmu_idx,
>      return (addr >> TARGET_PAGE_BITS) & size_mask;
>  }
>  
> -static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
> -{
> -    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
> -}
> -
>  /* Find the TLB entry corresponding to the mmu_idx + address pair.  */
>  static inline CPUTLBEntry *tlb_entry(CPUArchState *env, uintptr_t mmu_idx,
>                                       target_ulong addr)
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 1a81886e58..e4a8ed9534 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -80,6 +80,11 @@ QEMU_BUILD_BUG_ON(sizeof(target_ulong) > sizeof(run_on_cpu_data));
>  QEMU_BUILD_BUG_ON(NB_MMU_MODES > 16);
>  #define ALL_MMUIDX_BITS ((1 << NB_MMU_MODES) - 1)
>  
> +static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
> +{
> +    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
> +}
> +
>  static inline size_t sizeof_tlb(CPUArchState *env, uintptr_t mmu_idx)
>  {
>      return env_tlb(env)->f[mmu_idx].mask + (1 << CPU_TLB_ENTRY_BITS);


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb
  2020-01-09  2:49 ` [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb Richard Henderson
  2020-01-20  0:37   ` Alistair Francis
  2020-01-20  8:54   ` Philippe Mathieu-Daudé
@ 2020-01-20 13:36   ` Alex Bennée
  2020-01-20 13:39   ` Alex Bennée
  3 siblings, 0 replies; 37+ messages in thread
From: Alex Bennée @ 2020-01-20 13:36 UTC (permalink / raw)
  To: Richard Henderson; +Cc: alistair.francis, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> We do not need the entire CPUArchState to compute these values.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  accel/tcg/cputlb.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e4a8ed9534..49c605b6d8 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -80,14 +80,14 @@ QEMU_BUILD_BUG_ON(sizeof(target_ulong) > sizeof(run_on_cpu_data));
>  QEMU_BUILD_BUG_ON(NB_MMU_MODES > 16);
>  #define ALL_MMUIDX_BITS ((1 << NB_MMU_MODES) - 1)
>  
> -static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
> +static inline size_t tlb_n_entries(CPUTLBDescFast *fast)
>  {
> -    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
> +    return (fast->mask >> CPU_TLB_ENTRY_BITS) + 1;
>  }
>  
> -static inline size_t sizeof_tlb(CPUArchState *env, uintptr_t mmu_idx)
> +static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
>  {
> -    return env_tlb(env)->f[mmu_idx].mask + (1 << CPU_TLB_ENTRY_BITS);
> +    return fast->mask + (1 << CPU_TLB_ENTRY_BITS);
>  }
>  
>  static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
> @@ -156,7 +156,7 @@ static void tlb_dyn_init(CPUArchState *env)
>  static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>  {
>      CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> -    size_t old_size = tlb_n_entries(env, mmu_idx);
> +    size_t old_size = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
>      size_t rate;
>      size_t new_size = old_size;
>      int64_t now = get_clock_realtime();
> @@ -236,7 +236,8 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>      env_tlb(env)->d[mmu_idx].large_page_addr = -1;
>      env_tlb(env)->d[mmu_idx].large_page_mask = -1;
>      env_tlb(env)->d[mmu_idx].vindex = 0;
> -    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
> +    memset(env_tlb(env)->f[mmu_idx].table, -1,
> +           sizeof_tlb(&env_tlb(env)->f[mmu_idx]));
>      memset(env_tlb(env)->d[mmu_idx].vtable, -1,
>             sizeof(env_tlb(env)->d[0].vtable));
>  }
> @@ -622,7 +623,7 @@ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length)
>      qemu_spin_lock(&env_tlb(env)->c.lock);
>      for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
>          unsigned int i;
> -        unsigned int n = tlb_n_entries(env, mmu_idx);
> +        unsigned int n = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
>  
>          for (i = 0; i < n; i++) {
>              tlb_reset_dirty_range_locked(&env_tlb(env)->f[mmu_idx].table[i],


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb
  2020-01-09  2:49 ` [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb Richard Henderson
                     ` (2 preceding siblings ...)
  2020-01-20 13:36   ` Alex Bennée
@ 2020-01-20 13:39   ` Alex Bennée
  3 siblings, 0 replies; 37+ messages in thread
From: Alex Bennée @ 2020-01-20 13:39 UTC (permalink / raw)
  To: Richard Henderson; +Cc: alistair.francis, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> We do not need the entire CPUArchState to compute these values.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  accel/tcg/cputlb.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e4a8ed9534..49c605b6d8 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -80,14 +80,14 @@ QEMU_BUILD_BUG_ON(sizeof(target_ulong) > sizeof(run_on_cpu_data));
>  QEMU_BUILD_BUG_ON(NB_MMU_MODES > 16);
>  #define ALL_MMUIDX_BITS ((1 << NB_MMU_MODES) - 1)
>  
> -static inline size_t tlb_n_entries(CPUArchState *env, uintptr_t mmu_idx)
> +static inline size_t tlb_n_entries(CPUTLBDescFast *fast)
>  {
> -    return (env_tlb(env)->f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS) + 1;
> +    return (fast->mask >> CPU_TLB_ENTRY_BITS) + 1;
>  }
>  
> -static inline size_t sizeof_tlb(CPUArchState *env, uintptr_t mmu_idx)
> +static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
>  {
> -    return env_tlb(env)->f[mmu_idx].mask + (1 << CPU_TLB_ENTRY_BITS);
> +    return fast->mask + (1 << CPU_TLB_ENTRY_BITS);
>  }
>  
>  static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
> @@ -156,7 +156,7 @@ static void tlb_dyn_init(CPUArchState *env)
>  static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>  {
>      CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> -    size_t old_size = tlb_n_entries(env, mmu_idx);
> +    size_t old_size = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
>      size_t rate;
>      size_t new_size = old_size;
>      int64_t now = get_clock_realtime();
> @@ -236,7 +236,8 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>      env_tlb(env)->d[mmu_idx].large_page_addr = -1;
>      env_tlb(env)->d[mmu_idx].large_page_mask = -1;
>      env_tlb(env)->d[mmu_idx].vindex = 0;
> -    memset(env_tlb(env)->f[mmu_idx].table, -1, sizeof_tlb(env, mmu_idx));
> +    memset(env_tlb(env)->f[mmu_idx].table, -1,
> +           sizeof_tlb(&env_tlb(env)->f[mmu_idx]));
>      memset(env_tlb(env)->d[mmu_idx].vtable, -1,
>             sizeof(env_tlb(env)->d[0].vtable));
>  }
> @@ -622,7 +623,7 @@ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length)
>      qemu_spin_lock(&env_tlb(env)->c.lock);
>      for (mmu_idx = 0; mmu_idx < NB_MMU_MODES; mmu_idx++) {
>          unsigned int i;
> -        unsigned int n = tlb_n_entries(env, mmu_idx);
> +        unsigned int n = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
>  
>          for (i = 0; i < n; i++) {
>              tlb_reset_dirty_range_locked(&env_tlb(env)->f[mmu_idx].table[i],


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 4/9] cputlb: Hoist tlb portions in tlb_mmu_resize_locked
  2020-01-09  2:49 ` [PATCH 4/9] cputlb: Hoist tlb portions in tlb_mmu_resize_locked Richard Henderson
  2020-01-20  8:58   ` Philippe Mathieu-Daudé
  2020-01-20 12:05   ` Alistair Francis
@ 2020-01-20 13:40   ` Alex Bennée
  2 siblings, 0 replies; 37+ messages in thread
From: Alex Bennée @ 2020-01-20 13:40 UTC (permalink / raw)
  To: Richard Henderson; +Cc: alistair.francis, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> No functional change, but the smaller expressions make
> the code easier to read.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  accel/tcg/cputlb.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 49c605b6d8..c7dc1dc85a 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -115,8 +115,8 @@ static void tlb_dyn_init(CPUArchState *env)
>  
>  /**
>   * tlb_mmu_resize_locked() - perform TLB resize bookkeeping; resize if necessary
> - * @env: CPU that owns the TLB
> - * @mmu_idx: MMU index of the TLB
> + * @desc: The CPUTLBDesc portion of the TLB
> + * @fast: The CPUTLBDescFast portion of the same TLB
>   *
>   * Called with tlb_lock_held.
>   *
> @@ -153,10 +153,9 @@ static void tlb_dyn_init(CPUArchState *env)
>   * high), since otherwise we are likely to have a significant amount of
>   * conflict misses.
>   */
> -static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
> +static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>  {
> -    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> -    size_t old_size = tlb_n_entries(&env_tlb(env)->f[mmu_idx]);
> +    size_t old_size = tlb_n_entries(fast);
>      size_t rate;
>      size_t new_size = old_size;
>      int64_t now = get_clock_realtime();
> @@ -198,14 +197,15 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>          return;
>      }
>  
> -    g_free(env_tlb(env)->f[mmu_idx].table);
> -    g_free(env_tlb(env)->d[mmu_idx].iotlb);
> +    g_free(fast->table);
> +    g_free(desc->iotlb);
>  
>      tlb_window_reset(desc, now, 0);
>      /* desc->n_used_entries is cleared by the caller */
> -    env_tlb(env)->f[mmu_idx].mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
> -    env_tlb(env)->f[mmu_idx].table = g_try_new(CPUTLBEntry, new_size);
> -    env_tlb(env)->d[mmu_idx].iotlb = g_try_new(CPUIOTLBEntry, new_size);
> +    fast->mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
> +    fast->table = g_try_new(CPUTLBEntry, new_size);
> +    desc->iotlb = g_try_new(CPUIOTLBEntry, new_size);
> +
>      /*
>       * If the allocations fail, try smaller sizes. We just freed some
>       * memory, so going back to half of new_size has a good chance of working.
> @@ -213,25 +213,24 @@ static void tlb_mmu_resize_locked(CPUArchState *env, int mmu_idx)
>       * allocations to fail though, so we progressively reduce the allocation
>       * size, aborting if we cannot even allocate the smallest TLB we support.
>       */
> -    while (env_tlb(env)->f[mmu_idx].table == NULL ||
> -           env_tlb(env)->d[mmu_idx].iotlb == NULL) {
> +    while (fast->table == NULL || desc->iotlb == NULL) {
>          if (new_size == (1 << CPU_TLB_DYN_MIN_BITS)) {
>              error_report("%s: %s", __func__, strerror(errno));
>              abort();
>          }
>          new_size = MAX(new_size >> 1, 1 << CPU_TLB_DYN_MIN_BITS);
> -        env_tlb(env)->f[mmu_idx].mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
> +        fast->mask = (new_size - 1) << CPU_TLB_ENTRY_BITS;
>  
> -        g_free(env_tlb(env)->f[mmu_idx].table);
> -        g_free(env_tlb(env)->d[mmu_idx].iotlb);
> -        env_tlb(env)->f[mmu_idx].table = g_try_new(CPUTLBEntry, new_size);
> -        env_tlb(env)->d[mmu_idx].iotlb = g_try_new(CPUIOTLBEntry, new_size);
> +        g_free(fast->table);
> +        g_free(desc->iotlb);
> +        fast->table = g_try_new(CPUTLBEntry, new_size);
> +        desc->iotlb = g_try_new(CPUIOTLBEntry, new_size);
>      }
>  }
>  
>  static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>  {
> -    tlb_mmu_resize_locked(env, mmu_idx);
> +    tlb_mmu_resize_locked(&env_tlb(env)->d[mmu_idx], &env_tlb(env)->f[mmu_idx]);
>      env_tlb(env)->d[mmu_idx].n_used_entries = 0;
>      env_tlb(env)->d[mmu_idx].large_page_addr = -1;
>      env_tlb(env)->d[mmu_idx].large_page_mask = -1;


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 5/9] cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked
  2020-01-09  2:49 ` [PATCH 5/9] cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked Richard Henderson
  2020-01-20  8:58   ` Philippe Mathieu-Daudé
@ 2020-01-20 13:41   ` Alex Bennée
  2020-01-20 22:56   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Alex Bennée @ 2020-01-20 13:41 UTC (permalink / raw)
  To: Richard Henderson; +Cc: alistair.francis, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> No functional change, but the smaller expressions make
> the code easier to read.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  accel/tcg/cputlb.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index c7dc1dc85a..eff427f137 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -230,15 +230,16 @@ static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>  
>  static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>  {
> -    tlb_mmu_resize_locked(&env_tlb(env)->d[mmu_idx], &env_tlb(env)->f[mmu_idx]);
> -    env_tlb(env)->d[mmu_idx].n_used_entries = 0;
> -    env_tlb(env)->d[mmu_idx].large_page_addr = -1;
> -    env_tlb(env)->d[mmu_idx].large_page_mask = -1;
> -    env_tlb(env)->d[mmu_idx].vindex = 0;
> -    memset(env_tlb(env)->f[mmu_idx].table, -1,
> -           sizeof_tlb(&env_tlb(env)->f[mmu_idx]));
> -    memset(env_tlb(env)->d[mmu_idx].vtable, -1,
> -           sizeof(env_tlb(env)->d[0].vtable));
> +    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> +    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
> +
> +    tlb_mmu_resize_locked(desc, fast);
> +    desc->n_used_entries = 0;
> +    desc->large_page_addr = -1;
> +    desc->large_page_mask = -1;
> +    desc->vindex = 0;
> +    memset(fast->table, -1, sizeof_tlb(fast));
> +    memset(desc->vtable, -1, sizeof(desc->vtable));
>  }
>  
>  static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 6/9] cputlb: Split out tlb_mmu_flush_locked
  2020-01-09  2:49 ` [PATCH 6/9] cputlb: Split out tlb_mmu_flush_locked Richard Henderson
  2020-01-20  8:59   ` Philippe Mathieu-Daudé
@ 2020-01-20 13:41   ` Alex Bennée
  2020-01-20 22:58   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Alex Bennée @ 2020-01-20 13:41 UTC (permalink / raw)
  To: Richard Henderson; +Cc: alistair.francis, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> We will want to be able to flush a tlb without resizing.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  accel/tcg/cputlb.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index eff427f137..e60e501334 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -228,12 +228,8 @@ static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>      }
>  }
>  
> -static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> +static void tlb_mmu_flush_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>  {
> -    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> -    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
> -
> -    tlb_mmu_resize_locked(desc, fast);
>      desc->n_used_entries = 0;
>      desc->large_page_addr = -1;
>      desc->large_page_mask = -1;
> @@ -242,6 +238,15 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>      memset(desc->vtable, -1, sizeof(desc->vtable));
>  }
>  
> +static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> +{
> +    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> +    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
> +
> +    tlb_mmu_resize_locked(desc, fast);
> +    tlb_mmu_flush_locked(desc, fast);
> +}
> +
>  static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
>  {
>      env_tlb(env)->d[mmu_idx].n_used_entries++;


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 7/9] cputlb: Partially merge tlb_dyn_init into tlb_init
  2020-01-09  2:49 ` [PATCH 7/9] cputlb: Partially merge tlb_dyn_init into tlb_init Richard Henderson
  2020-01-20  9:01   ` Philippe Mathieu-Daudé
@ 2020-01-20 14:33   ` Alex Bennée
  2020-01-20 23:00   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Alex Bennée @ 2020-01-20 14:33 UTC (permalink / raw)
  To: Richard Henderson; +Cc: alistair.francis, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> Merge into the only caller, but at the same time split
> out tlb_mmu_init to initialize a single tlb entry.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  accel/tcg/cputlb.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e60e501334..c7c34b185b 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -97,22 +97,6 @@ static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
>      desc->window_max_entries = max_entries;
>  }
>  
> -static void tlb_dyn_init(CPUArchState *env)
> -{
> -    int i;
> -
> -    for (i = 0; i < NB_MMU_MODES; i++) {
> -        CPUTLBDesc *desc = &env_tlb(env)->d[i];
> -        size_t n_entries = 1 << CPU_TLB_DYN_DEFAULT_BITS;
> -
> -        tlb_window_reset(desc, get_clock_realtime(), 0);
> -        desc->n_used_entries = 0;
> -        env_tlb(env)->f[i].mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
> -        env_tlb(env)->f[i].table = g_new(CPUTLBEntry, n_entries);
> -        env_tlb(env)->d[i].iotlb = g_new(CPUIOTLBEntry, n_entries);
> -    }
> -}
> -
>  /**
>   * tlb_mmu_resize_locked() - perform TLB resize bookkeeping; resize if necessary
>   * @desc: The CPUTLBDesc portion of the TLB
> @@ -247,6 +231,17 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>      tlb_mmu_flush_locked(desc, fast);
>  }
>  
> +static void tlb_mmu_init(CPUTLBDesc *desc, CPUTLBDescFast *fast, int64_t now)
> +{
> +    size_t n_entries = 1 << CPU_TLB_DYN_DEFAULT_BITS;
> +
> +    tlb_window_reset(desc, now, 0);
> +    desc->n_used_entries = 0;
> +    fast->mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
> +    fast->table = g_new(CPUTLBEntry, n_entries);
> +    desc->iotlb = g_new(CPUIOTLBEntry, n_entries);
> +}
> +
>  static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
>  {
>      env_tlb(env)->d[mmu_idx].n_used_entries++;
> @@ -260,13 +255,17 @@ static inline void tlb_n_used_entries_dec(CPUArchState *env, uintptr_t mmu_idx)
>  void tlb_init(CPUState *cpu)
>  {
>      CPUArchState *env = cpu->env_ptr;
> +    int64_t now = get_clock_realtime();
> +    int i;
>  
>      qemu_spin_init(&env_tlb(env)->c.lock);
>  
>      /* Ensure that cpu_reset performs a full flush.  */
>      env_tlb(env)->c.dirty = ALL_MMUIDX_BITS;
>  
> -    tlb_dyn_init(env);
> +    for (i = 0; i < NB_MMU_MODES; i++) {
> +        tlb_mmu_init(&env_tlb(env)->d[i], &env_tlb(env)->f[i], now);
> +    }
>  }
>  
>  /* flush_all_helper: run fn across all cpus


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 8/9] cputlb: Initialize tlbs as flushed
  2020-01-09  2:49 ` [PATCH 8/9] cputlb: Initialize tlbs as flushed Richard Henderson
@ 2020-01-20 14:33   ` Alex Bennée
  2020-01-20 23:01   ` Alistair Francis
  1 sibling, 0 replies; 37+ messages in thread
From: Alex Bennée @ 2020-01-20 14:33 UTC (permalink / raw)
  To: Richard Henderson; +Cc: alistair.francis, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> There's little point in leaving these data structures half initialized,
> and relying on a flush to be done during reset.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  accel/tcg/cputlb.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index c7c34b185b..761e9d44d7 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -240,6 +240,7 @@ static void tlb_mmu_init(CPUTLBDesc *desc, CPUTLBDescFast *fast, int64_t now)
>      fast->mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
>      fast->table = g_new(CPUTLBEntry, n_entries);
>      desc->iotlb = g_new(CPUIOTLBEntry, n_entries);
> +    tlb_mmu_flush_locked(desc, fast);
>  }
>  
>  static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
> @@ -260,8 +261,8 @@ void tlb_init(CPUState *cpu)
>  
>      qemu_spin_init(&env_tlb(env)->c.lock);
>  
> -    /* Ensure that cpu_reset performs a full flush.  */
> -    env_tlb(env)->c.dirty = ALL_MMUIDX_BITS;
> +    /* All tlbs are initialized flushed. */
> +    env_tlb(env)->c.dirty = 0;
>  
>      for (i = 0; i < NB_MMU_MODES; i++) {
>          tlb_mmu_init(&env_tlb(env)->d[i], &env_tlb(env)->f[i], now);


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 9/9] cputlb: Hoist timestamp outside of loops over tlbs
  2020-01-09  2:49 ` [PATCH 9/9] cputlb: Hoist timestamp outside of loops over tlbs Richard Henderson
  2020-01-20  9:02   ` Philippe Mathieu-Daudé
@ 2020-01-20 14:36   ` Alex Bennée
  2020-01-20 23:02   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Alex Bennée @ 2020-01-20 14:36 UTC (permalink / raw)
  To: Richard Henderson; +Cc: alistair.francis, qemu-devel


Richard Henderson <richard.henderson@linaro.org> writes:

> Do not call get_clock_realtime() in tlb_mmu_resize_locked,
> but hoist outside of any loop over a set of tlbs.  This is
> only two (indirect) callers, tlb_flush_by_mmuidx_async_work
> and tlb_flush_page_locked, so not onerous.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  accel/tcg/cputlb.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 761e9d44d7..9f6cb36921 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -137,12 +137,12 @@ static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
>   * high), since otherwise we are likely to have a significant amount of
>   * conflict misses.
>   */
> -static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
> +static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast,
> +                                  int64_t now)
>  {
>      size_t old_size = tlb_n_entries(fast);
>      size_t rate;
>      size_t new_size = old_size;
> -    int64_t now = get_clock_realtime();
>      int64_t window_len_ms = 100;
>      int64_t window_len_ns = window_len_ms * 1000 * 1000;
>      bool window_expired = now > desc->window_begin_ns + window_len_ns;
> @@ -222,12 +222,13 @@ static void tlb_mmu_flush_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>      memset(desc->vtable, -1, sizeof(desc->vtable));
>  }
>  
> -static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> +static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx,
> +                                        int64_t now)
>  {
>      CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
>      CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
>  
> -    tlb_mmu_resize_locked(desc, fast);
> +    tlb_mmu_resize_locked(desc, fast, now);
>      tlb_mmu_flush_locked(desc, fast);
>  }
>  
> @@ -310,6 +311,7 @@ static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
>      CPUArchState *env = cpu->env_ptr;
>      uint16_t asked = data.host_int;
>      uint16_t all_dirty, work, to_clean;
> +    int64_t now = get_clock_realtime();
>  
>      assert_cpu_is_self(cpu);
>  
> @@ -324,7 +326,7 @@ static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
>  
>      for (work = to_clean; work != 0; work &= work - 1) {
>          int mmu_idx = ctz32(work);
> -        tlb_flush_one_mmuidx_locked(env, mmu_idx);
> +        tlb_flush_one_mmuidx_locked(env, mmu_idx, now);
>      }
>  
>      qemu_spin_unlock(&env_tlb(env)->c.lock);
> @@ -446,7 +448,7 @@ static void tlb_flush_page_locked(CPUArchState *env, int midx,
>          tlb_debug("forcing full flush midx %d ("
>                    TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
>                    midx, lp_addr, lp_mask);
> -        tlb_flush_one_mmuidx_locked(env, midx);
> +        tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime());
>      } else {
>          if (tlb_flush_entry_locked(tlb_entry(env, midx, page), page)) {
>              tlb_n_used_entries_dec(env, midx);


-- 
Alex Bennée


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 5/9] cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked
  2020-01-09  2:49 ` [PATCH 5/9] cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked Richard Henderson
  2020-01-20  8:58   ` Philippe Mathieu-Daudé
  2020-01-20 13:41   ` Alex Bennée
@ 2020-01-20 22:56   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-01-20 22:56 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers

On Thu, Jan 9, 2020 at 12:55 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> No functional change, but the smaller expressions make
> the code easier to read.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cputlb.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index c7dc1dc85a..eff427f137 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -230,15 +230,16 @@ static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>
>  static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>  {
> -    tlb_mmu_resize_locked(&env_tlb(env)->d[mmu_idx], &env_tlb(env)->f[mmu_idx]);
> -    env_tlb(env)->d[mmu_idx].n_used_entries = 0;
> -    env_tlb(env)->d[mmu_idx].large_page_addr = -1;
> -    env_tlb(env)->d[mmu_idx].large_page_mask = -1;
> -    env_tlb(env)->d[mmu_idx].vindex = 0;
> -    memset(env_tlb(env)->f[mmu_idx].table, -1,
> -           sizeof_tlb(&env_tlb(env)->f[mmu_idx]));
> -    memset(env_tlb(env)->d[mmu_idx].vtable, -1,
> -           sizeof(env_tlb(env)->d[0].vtable));
> +    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> +    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
> +
> +    tlb_mmu_resize_locked(desc, fast);
> +    desc->n_used_entries = 0;
> +    desc->large_page_addr = -1;
> +    desc->large_page_mask = -1;
> +    desc->vindex = 0;
> +    memset(fast->table, -1, sizeof_tlb(fast));
> +    memset(desc->vtable, -1, sizeof(desc->vtable));
>  }
>
>  static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 6/9] cputlb: Split out tlb_mmu_flush_locked
  2020-01-09  2:49 ` [PATCH 6/9] cputlb: Split out tlb_mmu_flush_locked Richard Henderson
  2020-01-20  8:59   ` Philippe Mathieu-Daudé
  2020-01-20 13:41   ` Alex Bennée
@ 2020-01-20 22:58   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-01-20 22:58 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers

On Thu, Jan 9, 2020 at 12:52 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We will want to be able to flush a tlb without resizing.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cputlb.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index eff427f137..e60e501334 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -228,12 +228,8 @@ static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>      }
>  }
>
> -static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> +static void tlb_mmu_flush_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>  {
> -    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> -    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
> -
> -    tlb_mmu_resize_locked(desc, fast);
>      desc->n_used_entries = 0;
>      desc->large_page_addr = -1;
>      desc->large_page_mask = -1;
> @@ -242,6 +238,15 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>      memset(desc->vtable, -1, sizeof(desc->vtable));
>  }
>
> +static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> +{
> +    CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
> +    CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
> +
> +    tlb_mmu_resize_locked(desc, fast);
> +    tlb_mmu_flush_locked(desc, fast);
> +}
> +
>  static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
>  {
>      env_tlb(env)->d[mmu_idx].n_used_entries++;
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 7/9] cputlb: Partially merge tlb_dyn_init into tlb_init
  2020-01-09  2:49 ` [PATCH 7/9] cputlb: Partially merge tlb_dyn_init into tlb_init Richard Henderson
  2020-01-20  9:01   ` Philippe Mathieu-Daudé
  2020-01-20 14:33   ` Alex Bennée
@ 2020-01-20 23:00   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-01-20 23:00 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers

On Thu, Jan 9, 2020 at 12:52 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Merge into the only caller, but at the same time split
> out tlb_mmu_init to initialize a single tlb entry.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cputlb.c | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index e60e501334..c7c34b185b 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -97,22 +97,6 @@ static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
>      desc->window_max_entries = max_entries;
>  }
>
> -static void tlb_dyn_init(CPUArchState *env)
> -{
> -    int i;
> -
> -    for (i = 0; i < NB_MMU_MODES; i++) {
> -        CPUTLBDesc *desc = &env_tlb(env)->d[i];
> -        size_t n_entries = 1 << CPU_TLB_DYN_DEFAULT_BITS;
> -
> -        tlb_window_reset(desc, get_clock_realtime(), 0);
> -        desc->n_used_entries = 0;
> -        env_tlb(env)->f[i].mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
> -        env_tlb(env)->f[i].table = g_new(CPUTLBEntry, n_entries);
> -        env_tlb(env)->d[i].iotlb = g_new(CPUIOTLBEntry, n_entries);
> -    }
> -}
> -
>  /**
>   * tlb_mmu_resize_locked() - perform TLB resize bookkeeping; resize if necessary
>   * @desc: The CPUTLBDesc portion of the TLB
> @@ -247,6 +231,17 @@ static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
>      tlb_mmu_flush_locked(desc, fast);
>  }
>
> +static void tlb_mmu_init(CPUTLBDesc *desc, CPUTLBDescFast *fast, int64_t now)
> +{
> +    size_t n_entries = 1 << CPU_TLB_DYN_DEFAULT_BITS;
> +
> +    tlb_window_reset(desc, now, 0);
> +    desc->n_used_entries = 0;
> +    fast->mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
> +    fast->table = g_new(CPUTLBEntry, n_entries);
> +    desc->iotlb = g_new(CPUIOTLBEntry, n_entries);
> +}
> +
>  static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
>  {
>      env_tlb(env)->d[mmu_idx].n_used_entries++;
> @@ -260,13 +255,17 @@ static inline void tlb_n_used_entries_dec(CPUArchState *env, uintptr_t mmu_idx)
>  void tlb_init(CPUState *cpu)
>  {
>      CPUArchState *env = cpu->env_ptr;
> +    int64_t now = get_clock_realtime();
> +    int i;
>
>      qemu_spin_init(&env_tlb(env)->c.lock);
>
>      /* Ensure that cpu_reset performs a full flush.  */
>      env_tlb(env)->c.dirty = ALL_MMUIDX_BITS;
>
> -    tlb_dyn_init(env);
> +    for (i = 0; i < NB_MMU_MODES; i++) {
> +        tlb_mmu_init(&env_tlb(env)->d[i], &env_tlb(env)->f[i], now);
> +    }
>  }
>
>  /* flush_all_helper: run fn across all cpus
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 8/9] cputlb: Initialize tlbs as flushed
  2020-01-09  2:49 ` [PATCH 8/9] cputlb: Initialize tlbs as flushed Richard Henderson
  2020-01-20 14:33   ` Alex Bennée
@ 2020-01-20 23:01   ` Alistair Francis
  1 sibling, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-01-20 23:01 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers

On Thu, Jan 9, 2020 at 12:57 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> There's little point in leaving these data structures half initialized,
> and relying on a flush to be done during reset.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cputlb.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index c7c34b185b..761e9d44d7 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -240,6 +240,7 @@ static void tlb_mmu_init(CPUTLBDesc *desc, CPUTLBDescFast *fast, int64_t now)
>      fast->mask = (n_entries - 1) << CPU_TLB_ENTRY_BITS;
>      fast->table = g_new(CPUTLBEntry, n_entries);
>      desc->iotlb = g_new(CPUIOTLBEntry, n_entries);
> +    tlb_mmu_flush_locked(desc, fast);
>  }
>
>  static inline void tlb_n_used_entries_inc(CPUArchState *env, uintptr_t mmu_idx)
> @@ -260,8 +261,8 @@ void tlb_init(CPUState *cpu)
>
>      qemu_spin_init(&env_tlb(env)->c.lock);
>
> -    /* Ensure that cpu_reset performs a full flush.  */
> -    env_tlb(env)->c.dirty = ALL_MMUIDX_BITS;
> +    /* All tlbs are initialized flushed. */
> +    env_tlb(env)->c.dirty = 0;
>
>      for (i = 0; i < NB_MMU_MODES; i++) {
>          tlb_mmu_init(&env_tlb(env)->d[i], &env_tlb(env)->f[i], now);
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 9/9] cputlb: Hoist timestamp outside of loops over tlbs
  2020-01-09  2:49 ` [PATCH 9/9] cputlb: Hoist timestamp outside of loops over tlbs Richard Henderson
  2020-01-20  9:02   ` Philippe Mathieu-Daudé
  2020-01-20 14:36   ` Alex Bennée
@ 2020-01-20 23:02   ` Alistair Francis
  2 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-01-20 23:02 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers

On Thu, Jan 9, 2020 at 12:55 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Do not call get_clock_realtime() in tlb_mmu_resize_locked,
> but hoist outside of any loop over a set of tlbs.  This is
> only two (indirect) callers, tlb_flush_by_mmuidx_async_work
> and tlb_flush_page_locked, so not onerous.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  accel/tcg/cputlb.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 761e9d44d7..9f6cb36921 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -137,12 +137,12 @@ static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
>   * high), since otherwise we are likely to have a significant amount of
>   * conflict misses.
>   */
> -static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
> +static void tlb_mmu_resize_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast,
> +                                  int64_t now)
>  {
>      size_t old_size = tlb_n_entries(fast);
>      size_t rate;
>      size_t new_size = old_size;
> -    int64_t now = get_clock_realtime();
>      int64_t window_len_ms = 100;
>      int64_t window_len_ns = window_len_ms * 1000 * 1000;
>      bool window_expired = now > desc->window_begin_ns + window_len_ns;
> @@ -222,12 +222,13 @@ static void tlb_mmu_flush_locked(CPUTLBDesc *desc, CPUTLBDescFast *fast)
>      memset(desc->vtable, -1, sizeof(desc->vtable));
>  }
>
> -static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx)
> +static void tlb_flush_one_mmuidx_locked(CPUArchState *env, int mmu_idx,
> +                                        int64_t now)
>  {
>      CPUTLBDesc *desc = &env_tlb(env)->d[mmu_idx];
>      CPUTLBDescFast *fast = &env_tlb(env)->f[mmu_idx];
>
> -    tlb_mmu_resize_locked(desc, fast);
> +    tlb_mmu_resize_locked(desc, fast, now);
>      tlb_mmu_flush_locked(desc, fast);
>  }
>
> @@ -310,6 +311,7 @@ static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
>      CPUArchState *env = cpu->env_ptr;
>      uint16_t asked = data.host_int;
>      uint16_t all_dirty, work, to_clean;
> +    int64_t now = get_clock_realtime();
>
>      assert_cpu_is_self(cpu);
>
> @@ -324,7 +326,7 @@ static void tlb_flush_by_mmuidx_async_work(CPUState *cpu, run_on_cpu_data data)
>
>      for (work = to_clean; work != 0; work &= work - 1) {
>          int mmu_idx = ctz32(work);
> -        tlb_flush_one_mmuidx_locked(env, mmu_idx);
> +        tlb_flush_one_mmuidx_locked(env, mmu_idx, now);
>      }
>
>      qemu_spin_unlock(&env_tlb(env)->c.lock);
> @@ -446,7 +448,7 @@ static void tlb_flush_page_locked(CPUArchState *env, int midx,
>          tlb_debug("forcing full flush midx %d ("
>                    TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
>                    midx, lp_addr, lp_mask);
> -        tlb_flush_one_mmuidx_locked(env, midx);
> +        tlb_flush_one_mmuidx_locked(env, midx, get_clock_realtime());
>      } else {
>          if (tlb_flush_entry_locked(tlb_entry(env, midx, page), page)) {
>              tlb_n_used_entries_dec(env, midx);
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

* Re: [PATCH 0/9] cputlb: Various cleanups
  2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
                   ` (8 preceding siblings ...)
  2020-01-09  2:49 ` [PATCH 9/9] cputlb: Hoist timestamp outside of loops over tlbs Richard Henderson
@ 2020-01-20 23:03 ` Alistair Francis
  9 siblings, 0 replies; 37+ messages in thread
From: Alistair Francis @ 2020-01-20 23:03 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Alistair Francis, qemu-devel@nongnu.org Developers

On Thu, Jan 9, 2020 at 12:49 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> I had a conversation with Alistair Francis at KVM forum about
> being able to represent ASIDs "properly".  This lead to the idea
> that target-specific code might be able to cache TLBs outside of
> the "main" NB_MMU_MODES -- possibly thousands of them.
>
> This goes nowhere near that far.  But it does begin edging toward
> the possibility of having a
>
>     struct CPUTLBSaved {
>         CPUTLBDesc d;
>         CPUTLBDescFast f;
>     };
>
> by moving some of the most basic routines to use CPUTLBDesc and
> CPUTLBDescFast directly instead of always using an mmu_idx.
>
> I'm not sure how much time I'll have to go further along these
> lines, but what I have so far still looks like a cleanup.

Thanks for helping with this!

Unfortunately I haven't had a chance to dig into this myself.

Alistair

>
>
> r~
>
>
> Richard Henderson (9):
>   cputlb: Merge tlb_table_flush_by_mmuidx into
>     tlb_flush_one_mmuidx_locked
>   cputlb: Make tlb_n_entries private to cputlb.c
>   cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb
>   cputlb: Hoist tlb portions in tlb_mmu_resize_locked
>   cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked
>   cputlb: Split out tlb_mmu_flush_locked
>   cputlb: Partially merge tlb_dyn_init into tlb_init
>   cputlb: Initialize tlbs as flushed
>   cputlb: Hoist timestamp outside of loops over tlbs
>
>  include/exec/cpu_ldst.h |   5 --
>  accel/tcg/cputlb.c      | 120 +++++++++++++++++++++-------------------
>  2 files changed, 64 insertions(+), 61 deletions(-)
>
> --
> 2.20.1
>
>


^ permalink raw reply	[flat|nested] 37+ messages in thread

end of thread, other threads:[~2020-01-20 23:12 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-09  2:48 [PATCH 0/9] cputlb: Various cleanups Richard Henderson
2020-01-09  2:48 ` [PATCH 1/9] cputlb: Merge tlb_table_flush_by_mmuidx into tlb_flush_one_mmuidx_locked Richard Henderson
2020-01-20  0:34   ` Alistair Francis
2020-01-20 13:34   ` Alex Bennée
2020-01-09  2:49 ` [PATCH 2/9] cputlb: Make tlb_n_entries private to cputlb.c Richard Henderson
2020-01-20  0:36   ` Alistair Francis
2020-01-20  8:53   ` Philippe Mathieu-Daudé
2020-01-20 13:34   ` Alex Bennée
2020-01-09  2:49 ` [PATCH 3/9] cputlb: Pass CPUTLBDescFast to tlb_n_entries and sizeof_tlb Richard Henderson
2020-01-20  0:37   ` Alistair Francis
2020-01-20  8:54   ` Philippe Mathieu-Daudé
2020-01-20 13:36   ` Alex Bennée
2020-01-20 13:39   ` Alex Bennée
2020-01-09  2:49 ` [PATCH 4/9] cputlb: Hoist tlb portions in tlb_mmu_resize_locked Richard Henderson
2020-01-20  8:58   ` Philippe Mathieu-Daudé
2020-01-20 12:05   ` Alistair Francis
2020-01-20 13:40   ` Alex Bennée
2020-01-09  2:49 ` [PATCH 5/9] cputlb: Hoist tlb portions in tlb_flush_one_mmuidx_locked Richard Henderson
2020-01-20  8:58   ` Philippe Mathieu-Daudé
2020-01-20 13:41   ` Alex Bennée
2020-01-20 22:56   ` Alistair Francis
2020-01-09  2:49 ` [PATCH 6/9] cputlb: Split out tlb_mmu_flush_locked Richard Henderson
2020-01-20  8:59   ` Philippe Mathieu-Daudé
2020-01-20 13:41   ` Alex Bennée
2020-01-20 22:58   ` Alistair Francis
2020-01-09  2:49 ` [PATCH 7/9] cputlb: Partially merge tlb_dyn_init into tlb_init Richard Henderson
2020-01-20  9:01   ` Philippe Mathieu-Daudé
2020-01-20 14:33   ` Alex Bennée
2020-01-20 23:00   ` Alistair Francis
2020-01-09  2:49 ` [PATCH 8/9] cputlb: Initialize tlbs as flushed Richard Henderson
2020-01-20 14:33   ` Alex Bennée
2020-01-20 23:01   ` Alistair Francis
2020-01-09  2:49 ` [PATCH 9/9] cputlb: Hoist timestamp outside of loops over tlbs Richard Henderson
2020-01-20  9:02   ` Philippe Mathieu-Daudé
2020-01-20 14:36   ` Alex Bennée
2020-01-20 23:02   ` Alistair Francis
2020-01-20 23:03 ` [PATCH 0/9] cputlb: Various cleanups Alistair Francis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.