qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/2] Clean-up tlb_flush and cpu reset functions
@ 2016-12-15 12:36 Alex Bennée
  2016-12-15 12:36 ` [Qemu-devel] [PATCH v2 1/2] qom/cpu: move tlb_flush to cpu_common_reset Alex Bennée
  2016-12-15 12:36 ` [Qemu-devel] [PATCH v2 2/2] cputlb: drop flush_global flag from tlb_flush Alex Bennée
  0 siblings, 2 replies; 8+ messages in thread
From: Alex Bennée @ 2016-12-15 12:36 UTC (permalink / raw)
  To: rth; +Cc: qemu-devel, Alex Bennée

Hi Richard,

Nothing much changed in this except:

  - one minor patchew checkpatch.pl fix
  - added your r-b

So I reckon this is ready to go in once the tree re-opens?

Alex Bennée (2):
  qom/cpu: move tlb_flush to cpu_common_reset
  cputlb: drop flush_global flag from tlb_flush

 cputlb.c                           | 21 ++++++---------------
 exec.c                             |  4 ++--
 hw/sh4/sh7750.c                    |  2 +-
 include/exec/exec-all.h            | 14 ++++++--------
 qom/cpu.c                          | 10 ++++++++--
 target-alpha/cpu.c                 |  2 +-
 target-alpha/sys_helper.c          |  2 +-
 target-arm/cpu.c                   |  5 ++---
 target-arm/cpu.h                   |  5 ++++-
 target-arm/helper.c                | 26 +++++++++++++-------------
 target-cris/cpu.c                  |  3 +--
 target-cris/cpu.h                  |  9 ++++++---
 target-i386/cpu.c                  |  2 --
 target-i386/cpu.h                  |  6 ++++--
 target-i386/fpu_helper.c           |  2 +-
 target-i386/helper.c               |  8 ++++----
 target-i386/machine.c              |  2 +-
 target-i386/misc_helper.c          |  2 +-
 target-i386/svm_helper.c           |  2 +-
 target-lm32/cpu.c                  |  3 +--
 target-lm32/cpu.h                  |  3 +++
 target-m68k/cpu.c                  |  3 +--
 target-m68k/cpu.h                  |  3 +++
 target-microblaze/cpu.c            |  3 +--
 target-microblaze/cpu.h            |  3 +++
 target-microblaze/mmu.c            |  2 +-
 target-mips/cpu.c                  |  3 +--
 target-mips/cpu.h                  |  5 ++++-
 target-mips/helper.c               |  6 +++---
 target-mips/op_helper.c            |  8 ++++----
 target-moxie/cpu.c                 |  4 +---
 target-moxie/cpu.h                 |  3 +++
 target-openrisc/cpu.c              |  9 +--------
 target-openrisc/cpu.h              |  3 +++
 target-openrisc/interrupt.c        |  2 +-
 target-openrisc/interrupt_helper.c |  2 +-
 target-openrisc/sys_helper.c       |  2 +-
 target-ppc/helper_regs.h           |  4 ++--
 target-ppc/misc_helper.c           |  4 ++--
 target-ppc/mmu_helper.c            | 32 ++++++++++++++++----------------
 target-ppc/translate_init.c        |  3 ---
 target-s390x/cpu.c                 |  7 ++-----
 target-s390x/cpu.h                 |  5 +++--
 target-s390x/gdbstub.c             |  2 +-
 target-s390x/mem_helper.c          |  8 ++++----
 target-sh4/cpu.c                   |  3 +--
 target-sh4/cpu.h                   |  3 +++
 target-sh4/helper.c                |  2 +-
 target-sparc/cpu.c                 |  3 +--
 target-sparc/cpu.h                 |  3 +++
 target-sparc/ldst_helper.c         | 12 ++++++------
 target-tilegx/cpu.c                |  3 +--
 target-tilegx/cpu.h                |  3 +++
 target-tricore/cpu.c               |  2 --
 target-unicore32/cpu.c             |  2 +-
 target-unicore32/helper.c          |  2 +-
 target-xtensa/op_helper.c          |  2 +-
 57 files changed, 151 insertions(+), 148 deletions(-)

-- 
2.11.0

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

* [Qemu-devel] [PATCH v2 1/2] qom/cpu: move tlb_flush to cpu_common_reset
  2016-12-15 12:36 [Qemu-devel] [PATCH v2 0/2] Clean-up tlb_flush and cpu reset functions Alex Bennée
@ 2016-12-15 12:36 ` Alex Bennée
  2016-12-16  4:21   ` David Gibson
  2016-12-18 20:46   ` Eduardo Habkost
  2016-12-15 12:36 ` [Qemu-devel] [PATCH v2 2/2] cputlb: drop flush_global flag from tlb_flush Alex Bennée
  1 sibling, 2 replies; 8+ messages in thread
From: Alex Bennée @ 2016-12-15 12:36 UTC (permalink / raw)
  To: rth
  Cc: qemu-devel, Alex Bennée, Peter Maydell, Edgar E. Iglesias,
	Paolo Bonzini, Eduardo Habkost, Michael Walle, Laurent Vivier,
	Aurelien Jarno, Yongbok Kim, Anthony Green, Jia Liu, David Gibson,
	Alexander Graf, Mark Cave-Ayland, Artyom Tarasenko,
	Bastian Koppelmann, open list:ARM, open list:PowerPC

It is a common thing amongst the various cpu reset functions want to
flush the SoftMMU's TLB entries. This is done either by calling
tlb_flush directly or by way of a general memset of the CPU
structure (sometimes both).

This moves the tlb_flush call to the common reset function and
additionally ensures it is only done for the CONFIG_SOFTMMU case and
when tcg is enabled.

In some target cases we add an empty end_of_reset_fields structure to the
target vCPU structure so have a clear end point for any memset which
is resetting value in the structure before CPU_COMMON (where the TLB
structures are).

While this is a nice clean-up in general it is also a precursor for
changes coming to cputlb for MTTCG where the clearing of entries
can't be done arbitrarily across vCPUs. Currently the cpu_reset
function is usually called from the context of another vCPU as the
architectural power up sequence is run. By using the cputlb API
functions we can ensure the right behaviour in the future.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 qom/cpu.c                   | 10 ++++++++--
 target-arm/cpu.c            |  5 ++---
 target-arm/cpu.h            |  5 ++++-
 target-cris/cpu.c           |  3 +--
 target-cris/cpu.h           |  9 ++++++---
 target-i386/cpu.c           |  2 --
 target-i386/cpu.h           |  6 ++++--
 target-lm32/cpu.c           |  3 +--
 target-lm32/cpu.h           |  3 +++
 target-m68k/cpu.c           |  3 +--
 target-m68k/cpu.h           |  3 +++
 target-microblaze/cpu.c     |  3 +--
 target-microblaze/cpu.h     |  3 +++
 target-mips/cpu.c           |  3 +--
 target-mips/cpu.h           |  3 +++
 target-moxie/cpu.c          |  4 +---
 target-moxie/cpu.h          |  3 +++
 target-openrisc/cpu.c       |  9 +--------
 target-openrisc/cpu.h       |  3 +++
 target-ppc/translate_init.c |  3 ---
 target-s390x/cpu.c          |  7 ++-----
 target-s390x/cpu.h          |  5 +++--
 target-sh4/cpu.c            |  3 +--
 target-sh4/cpu.h            |  3 +++
 target-sparc/cpu.c          |  3 +--
 target-sparc/cpu.h          |  3 +++
 target-tilegx/cpu.c         |  3 +--
 target-tilegx/cpu.h         |  3 +++
 target-tricore/cpu.c        |  2 --
 29 files changed, 66 insertions(+), 52 deletions(-)

diff --git a/qom/cpu.c b/qom/cpu.c
index 03d9190f8c..61ee0cb88c 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -270,8 +270,14 @@ static void cpu_common_reset(CPUState *cpu)
     cpu->exception_index = -1;
     cpu->crash_occurred = false;
 
-    for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
-        atomic_set(&cpu->tb_jmp_cache[i], NULL);
+    if (tcg_enabled()) {
+        for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
+            atomic_set(&cpu->tb_jmp_cache[i], NULL);
+        }
+
+#ifdef CONFIG_SOFTMMU
+        tlb_flush(cpu, 0);
+#endif
     }
 }
 
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 99f0dbebb9..fb05d2e4ec 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -122,7 +122,8 @@ static void arm_cpu_reset(CPUState *s)
 
     acc->parent_reset(s);
 
-    memset(env, 0, offsetof(CPUARMState, features));
+    memset(env, 0, offsetof(CPUARMState, end_reset_fields));
+
     g_hash_table_foreach(cpu->cp_regs, cp_reg_reset, cpu);
     g_hash_table_foreach(cpu->cp_regs, cp_reg_check_reset, cpu);
 
@@ -226,8 +227,6 @@ static void arm_cpu_reset(CPUState *s)
                               &env->vfp.fp_status);
     set_float_detect_tininess(float_tininess_before_rounding,
                               &env->vfp.standard_fp_status);
-    tlb_flush(s, 1);
-
 #ifndef CONFIG_USER_ONLY
     if (kvm_enabled()) {
         kvm_arm_reset_vcpu(cpu);
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index ca5c849ed6..53e9d55adf 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -491,9 +491,12 @@ typedef struct CPUARMState {
     struct CPUBreakpoint *cpu_breakpoint[16];
     struct CPUWatchpoint *cpu_watchpoint[16];
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 
-    /* These fields after the common ones so they are preserved on reset.  */
+    /* Fields after CPU_COMMON are preserved across CPU reset. */
 
     /* Internal CPU feature flags.  */
     uint64_t features;
diff --git a/target-cris/cpu.c b/target-cris/cpu.c
index 2e9ab9700e..5f766f09d6 100644
--- a/target-cris/cpu.c
+++ b/target-cris/cpu.c
@@ -52,9 +52,8 @@ static void cris_cpu_reset(CPUState *s)
     ccc->parent_reset(s);
 
     vr = env->pregs[PR_VR];
-    memset(env, 0, offsetof(CPUCRISState, load_info));
+    memset(env, 0, offsetof(CPUCRISState, end_reset_fields));
     env->pregs[PR_VR] = vr;
-    tlb_flush(s, 1);
 
 #if defined(CONFIG_USER_ONLY)
     /* start in user mode with interrupts enabled.  */
diff --git a/target-cris/cpu.h b/target-cris/cpu.h
index 43d5f9d1da..920e1c33ba 100644
--- a/target-cris/cpu.h
+++ b/target-cris/cpu.h
@@ -167,10 +167,13 @@ typedef struct CPUCRISState {
 	 */
         TLBSet tlbsets[2][4][16];
 
-	CPU_COMMON
+        /* Fields up to this point are cleared by a CPU reset */
+        struct {} end_reset_fields;
 
-    /* Members from load_info on are preserved across resets.  */
-    void *load_info;
+        CPU_COMMON
+
+        /* Members from load_info on are preserved across resets.  */
+        void *load_info;
 } CPUCRISState;
 
 /**
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index de1f30eeda..810893952a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2815,8 +2815,6 @@ static void x86_cpu_reset(CPUState *s)
 
     memset(env, 0, offsetof(CPUX86State, end_reset_fields));
 
-    tlb_flush(s, 1);
-
     env->old_exception = -1;
 
     /* init to reset state */
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index c605724022..95ed91d8d1 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -1119,10 +1119,12 @@ typedef struct CPUX86State {
     uint8_t nmi_injected;
     uint8_t nmi_pending;
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 
-    /* Fields from here on are preserved across CPU reset. */
-    struct {} end_reset_fields;
+    /* Fields after CPU_COMMON are preserved across CPU reset. */
 
     /* processor features (e.g. for CPUID insn) */
     /* Minimum level/xlevel/xlevel2, based on CPU model + features */
diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
index 8d939a7779..2b8c36b6d0 100644
--- a/target-lm32/cpu.c
+++ b/target-lm32/cpu.c
@@ -128,10 +128,9 @@ static void lm32_cpu_reset(CPUState *s)
     lcc->parent_reset(s);
 
     /* reset cpu state */
-    memset(env, 0, offsetof(CPULM32State, eba));
+    memset(env, 0, offsetof(CPULM32State, end_reset_fields));
 
     lm32_cpu_init_cfg_reg(cpu);
-    tlb_flush(s, 1);
 }
 
 static void lm32_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
diff --git a/target-lm32/cpu.h b/target-lm32/cpu.h
index d8a3515244..1d972cb26b 100644
--- a/target-lm32/cpu.h
+++ b/target-lm32/cpu.h
@@ -165,6 +165,9 @@ struct CPULM32State {
     struct CPUBreakpoint *cpu_breakpoint[4];
     struct CPUWatchpoint *cpu_watchpoint[4];
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 
     /* Fields from here on are preserved across CPU reset. */
diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
index ba17480098..fa10b6e4cd 100644
--- a/target-m68k/cpu.c
+++ b/target-m68k/cpu.c
@@ -52,7 +52,7 @@ static void m68k_cpu_reset(CPUState *s)
 
     mcc->parent_reset(s);
 
-    memset(env, 0, offsetof(CPUM68KState, features));
+    memset(env, 0, offsetof(CPUM68KState, end_reset_fields));
 #if !defined(CONFIG_USER_ONLY)
     env->sr = 0x2700;
 #endif
@@ -61,7 +61,6 @@ static void m68k_cpu_reset(CPUState *s)
     cpu_m68k_set_ccr(env, 0);
     /* TODO: We should set PC from the interrupt vector.  */
     env->pc = 0;
-    tlb_flush(s, 1);
 }
 
 static void m68k_cpu_disas_set_info(CPUState *s, disassemble_info *info)
diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
index 6dfb54eb70..8e7b51b756 100644
--- a/target-m68k/cpu.h
+++ b/target-m68k/cpu.h
@@ -115,6 +115,9 @@ typedef struct CPUM68KState {
 
     uint32_t qregs[MAX_QREGS];
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 
     /* Fields from here on are preserved across CPU reset. */
diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
index 389c7b691e..3d58869716 100644
--- a/target-microblaze/cpu.c
+++ b/target-microblaze/cpu.c
@@ -103,9 +103,8 @@ static void mb_cpu_reset(CPUState *s)
 
     mcc->parent_reset(s);
 
-    memset(env, 0, offsetof(CPUMBState, pvr));
+    memset(env, 0, offsetof(CPUMBState, end_reset_fields));
     env->res_addr = RES_ADDR_NONE;
-    tlb_flush(s, 1);
 
     /* Disable stack protector.  */
     env->shr = ~0;
diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
index beb75ffd26..bf6963bcb7 100644
--- a/target-microblaze/cpu.h
+++ b/target-microblaze/cpu.h
@@ -267,6 +267,9 @@ struct CPUMBState {
     struct microblaze_mmu mmu;
 #endif
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 
     /* These fields are preserved on reset.  */
diff --git a/target-mips/cpu.c b/target-mips/cpu.c
index 65ca607f88..1bb66b7a5a 100644
--- a/target-mips/cpu.c
+++ b/target-mips/cpu.c
@@ -100,8 +100,7 @@ static void mips_cpu_reset(CPUState *s)
 
     mcc->parent_reset(s);
 
-    memset(env, 0, offsetof(CPUMIPSState, mvp));
-    tlb_flush(s, 1);
+    memset(env, 0, offsetof(CPUMIPSState, end_reset_fields));
 
     cpu_state_reset(env);
 
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 5182dc74ff..3146a6017d 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -607,6 +607,9 @@ struct CPUMIPSState {
     uint32_t CP0_TCStatus_rw_bitmask; /* Read/write bits in CP0_TCStatus */
     int insn_flags; /* Supported instruction set */
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 
     /* Fields from here on are preserved across CPU reset. */
diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
index b0be4a7551..927b1a1e44 100644
--- a/target-moxie/cpu.c
+++ b/target-moxie/cpu.c
@@ -45,10 +45,8 @@ static void moxie_cpu_reset(CPUState *s)
 
     mcc->parent_reset(s);
 
-    memset(env, 0, sizeof(CPUMoxieState));
+    memset(env, 0, offsetof(CPUMoxieState, end_reset_fields));
     env->pc = 0x1000;
-
-    tlb_flush(s, 1);
 }
 
 static void moxie_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
diff --git a/target-moxie/cpu.h b/target-moxie/cpu.h
index 3e880facf4..8991aaef9a 100644
--- a/target-moxie/cpu.h
+++ b/target-moxie/cpu.h
@@ -56,6 +56,9 @@ typedef struct CPUMoxieState {
 
     void *irq[8];
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 
 } CPUMoxieState;
diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
index 698e87bb25..422139d29f 100644
--- a/target-openrisc/cpu.c
+++ b/target-openrisc/cpu.c
@@ -44,14 +44,7 @@ static void openrisc_cpu_reset(CPUState *s)
 
     occ->parent_reset(s);
 
-#ifndef CONFIG_USER_ONLY
-    memset(&cpu->env, 0, offsetof(CPUOpenRISCState, tlb));
-#else
-    memset(&cpu->env, 0, offsetof(CPUOpenRISCState, irq));
-#endif
-
-    tlb_flush(s, 1);
-    /*tb_flush(&cpu->env);    FIXME: Do we need it?  */
+    memset(&cpu->env, 0, offsetof(CPUOpenRISCState, end_reset_fields));
 
     cpu->env.pc = 0x100;
     cpu->env.sr = SR_FO | SR_SM;
diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
index aaf153579a..508ef568b4 100644
--- a/target-openrisc/cpu.h
+++ b/target-openrisc/cpu.h
@@ -300,6 +300,9 @@ typedef struct CPUOpenRISCState {
                                  in solt so far.  */
     uint32_t btaken;          /* the SR_F bit */
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 
     /* Fields from here on are preserved across CPU reset. */
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 626e03186c..4ff987226e 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -10415,9 +10415,6 @@ static void ppc_cpu_reset(CPUState *s)
         }
         env->spr[i] = spr->default_value;
     }
-
-    /* Flush all TLBs */
-    tlb_flush(s, 1);
 }
 
 #ifndef CONFIG_USER_ONLY
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 0a39d31237..066dcd17df 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -82,7 +82,6 @@ static void s390_cpu_reset(CPUState *s)
     scc->parent_reset(s);
     cpu->env.sigp_order = 0;
     s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
-    tlb_flush(s, 1);
 }
 
 /* S390CPUClass::initial_reset() */
@@ -94,7 +93,7 @@ static void s390_cpu_initial_reset(CPUState *s)
 
     s390_cpu_reset(s);
     /* initial reset does not touch regs,fregs and aregs */
-    memset(&env->fpc, 0, offsetof(CPUS390XState, cpu_num) -
+    memset(&env->fpc, 0, offsetof(CPUS390XState, end_reset_fields) -
                          offsetof(CPUS390XState, fpc));
 
     /* architectured initial values for CR 0 and 14 */
@@ -118,7 +117,6 @@ static void s390_cpu_initial_reset(CPUState *s)
     if (kvm_enabled()) {
         kvm_s390_reset_vcpu(cpu);
     }
-    tlb_flush(s, 1);
 }
 
 /* CPUClass:reset() */
@@ -133,7 +131,7 @@ static void s390_cpu_full_reset(CPUState *s)
     cpu->env.sigp_order = 0;
     s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
 
-    memset(env, 0, offsetof(CPUS390XState, cpu_num));
+    memset(env, 0, offsetof(CPUS390XState, end_reset_fields));
 
     /* architectured initial values for CR 0 and 14 */
     env->cregs[0] = CR0_RESET;
@@ -156,7 +154,6 @@ static void s390_cpu_full_reset(CPUState *s)
     if (kvm_enabled()) {
         kvm_s390_reset_vcpu(cpu);
     }
-    tlb_flush(s, 1);
 }
 
 #if !defined(CONFIG_USER_ONLY)
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index fd36a25cf5..058ddad83a 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -139,9 +139,10 @@ typedef struct CPUS390XState {
 
     uint8_t riccb[64];
 
-    CPU_COMMON
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
 
-    /* reset does memset(0) up to here */
+    CPU_COMMON
 
     uint32_t cpu_num;
     uint32_t machine_type;
diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
index a38f6a6ded..9a481c35dc 100644
--- a/target-sh4/cpu.c
+++ b/target-sh4/cpu.c
@@ -56,8 +56,7 @@ static void superh_cpu_reset(CPUState *s)
 
     scc->parent_reset(s);
 
-    memset(env, 0, offsetof(CPUSH4State, id));
-    tlb_flush(s, 1);
+    memset(env, 0, offsetof(CPUSH4State, end_reset_fields));
 
     env->pc = 0xA0000000;
 #if defined(CONFIG_USER_ONLY)
diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
index 478ab55868..cad8989f7e 100644
--- a/target-sh4/cpu.h
+++ b/target-sh4/cpu.h
@@ -175,6 +175,9 @@ typedef struct CPUSH4State {
 
     uint32_t ldst;
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 
     /* Fields from here on are preserved over CPU reset. */
diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
index 4e07b92fbd..d6583f1c2a 100644
--- a/target-sparc/cpu.c
+++ b/target-sparc/cpu.c
@@ -36,8 +36,7 @@ static void sparc_cpu_reset(CPUState *s)
 
     scc->parent_reset(s);
 
-    memset(env, 0, offsetof(CPUSPARCState, version));
-    tlb_flush(s, 1);
+    memset(env, 0, offsetof(CPUSPARCState, end_reset_fields));
     env->cwp = 0;
 #ifndef TARGET_SPARC64
     env->wim = 1;
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 5fb0ed1aad..601c018a05 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -419,6 +419,9 @@ struct CPUSPARCState {
     /* NOTE: we allow 8 more registers to handle wrapping */
     target_ulong regbase[MAX_NWINDOWS * 16 + 8];
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 
     /* Fields from here on are preserved across CPU reset. */
diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
index 454793f94a..d90e38e88c 100644
--- a/target-tilegx/cpu.c
+++ b/target-tilegx/cpu.c
@@ -84,8 +84,7 @@ static void tilegx_cpu_reset(CPUState *s)
 
     tcc->parent_reset(s);
 
-    memset(env, 0, sizeof(CPUTLGState));
-    tlb_flush(s, 1);
+    memset(env, 0, offsetof(CPUTLGState, end_reset_fields));
 }
 
 static void tilegx_cpu_realizefn(DeviceState *dev, Error **errp)
diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
index 1735427233..f32be49f65 100644
--- a/target-tilegx/cpu.h
+++ b/target-tilegx/cpu.h
@@ -97,6 +97,9 @@ typedef struct CPUTLGState {
     uint32_t sigcode;                  /* Signal code */
 #endif
 
+    /* Fields up to this point are cleared by a CPU reset */
+    struct {} end_reset_fields;
+
     CPU_COMMON
 } CPUTLGState;
 
diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
index 785b76bd3a..08f50e2ba7 100644
--- a/target-tricore/cpu.c
+++ b/target-tricore/cpu.c
@@ -53,8 +53,6 @@ static void tricore_cpu_reset(CPUState *s)
 
     tcc->parent_reset(s);
 
-    tlb_flush(s, 1);
-
     cpu_state_reset(env);
 }
 
-- 
2.11.0

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

* [Qemu-devel] [PATCH v2 2/2] cputlb: drop flush_global flag from tlb_flush
  2016-12-15 12:36 [Qemu-devel] [PATCH v2 0/2] Clean-up tlb_flush and cpu reset functions Alex Bennée
  2016-12-15 12:36 ` [Qemu-devel] [PATCH v2 1/2] qom/cpu: move tlb_flush to cpu_common_reset Alex Bennée
@ 2016-12-15 12:36 ` Alex Bennée
  2016-12-16  4:21   ` David Gibson
  1 sibling, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2016-12-15 12:36 UTC (permalink / raw)
  To: rth
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, Peter Crosthwaite,
	Aurelien Jarno, Peter Maydell, Eduardo Habkost, Edgar E. Iglesias,
	Yongbok Kim, Jia Liu, David Gibson, Alexander Graf,
	Mark Cave-Ayland, Artyom Tarasenko, Guan Xuetao, Max Filippov,
	open list:ARM, open list:PowerPC

We have never has the concept of global TLB entries which would avoid
the flush so we never actually use this flag. Drop it and make clear
that tlb_flush is the sledge-hammer it has always been.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 cputlb.c                           | 21 ++++++---------------
 exec.c                             |  4 ++--
 hw/sh4/sh7750.c                    |  2 +-
 include/exec/exec-all.h            | 14 ++++++--------
 target-alpha/cpu.c                 |  2 +-
 target-alpha/sys_helper.c          |  2 +-
 target-arm/helper.c                | 26 +++++++++++++-------------
 target-i386/fpu_helper.c           |  2 +-
 target-i386/helper.c               |  8 ++++----
 target-i386/machine.c              |  2 +-
 target-i386/misc_helper.c          |  2 +-
 target-i386/svm_helper.c           |  2 +-
 target-microblaze/mmu.c            |  2 +-
 target-mips/cpu.h                  |  2 +-
 target-mips/helper.c               |  6 +++---
 target-mips/op_helper.c            |  8 ++++----
 target-openrisc/interrupt.c        |  2 +-
 target-openrisc/interrupt_helper.c |  2 +-
 target-openrisc/sys_helper.c       |  2 +-
 target-ppc/helper_regs.h           |  4 ++--
 target-ppc/misc_helper.c           |  4 ++--
 target-ppc/mmu_helper.c            | 32 ++++++++++++++++----------------
 target-s390x/gdbstub.c             |  2 +-
 target-s390x/mem_helper.c          |  8 ++++----
 target-sh4/helper.c                |  2 +-
 target-sparc/ldst_helper.c         | 12 ++++++------
 target-unicore32/cpu.c             |  2 +-
 target-unicore32/helper.c          |  2 +-
 target-xtensa/op_helper.c          |  2 +-
 29 files changed, 85 insertions(+), 96 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 813279f3bc..6c39927455 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -60,24 +60,15 @@
 /* statistics */
 int tlb_flush_count;
 
-/* NOTE:
- * If flush_global is true (the usual case), flush all tlb entries.
- * If flush_global is false, flush (at least) all tlb entries not
- * marked global.
- *
- * Since QEMU doesn't currently implement a global/not-global flag
- * for tlb entries, at the moment tlb_flush() will also flush all
- * tlb entries in the flush_global == false case. This is OK because
- * CPU architectures generally permit an implementation to drop
- * entries from the TLB at any time, so flushing more entries than
- * required is only an efficiency issue, not a correctness issue.
+/* This is OK because CPU architectures generally permit an
+ * implementation to drop entries from the TLB at any time, so
+ * flushing more entries than required is only an efficiency issue,
+ * not a correctness issue.
  */
-void tlb_flush(CPUState *cpu, int flush_global)
+void tlb_flush(CPUState *cpu)
 {
     CPUArchState *env = cpu->env_ptr;
 
-    tlb_debug("(%d)\n", flush_global);
-
     memset(env->tlb_table, -1, sizeof(env->tlb_table));
     memset(env->tlb_v_table, -1, sizeof(env->tlb_v_table));
     memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
@@ -144,7 +135,7 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr)
                   TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
                   env->tlb_flush_addr, env->tlb_flush_mask);
 
-        tlb_flush(cpu, 1);
+        tlb_flush(cpu);
         return;
     }
 
diff --git a/exec.c b/exec.c
index 08c558eecf..57128539b4 100644
--- a/exec.c
+++ b/exec.c
@@ -511,7 +511,7 @@ static int cpu_common_post_load(void *opaque, int version_id)
     /* 0x01 was CPU_INTERRUPT_EXIT. This line can be removed when the
        version_id is increased. */
     cpu->interrupt_request &= ~0x01;
-    tlb_flush(cpu, 1);
+    tlb_flush(cpu);
 
     return 0;
 }
@@ -2393,7 +2393,7 @@ static void tcg_commit(MemoryListener *listener)
      */
     d = atomic_rcu_read(&cpuas->as->dispatch);
     atomic_rcu_set(&cpuas->memory_dispatch, d);
-    tlb_flush(cpuas->cpu, 1);
+    tlb_flush(cpuas->cpu);
 }
 
 void address_space_init_dispatch(AddressSpace *as)
diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
index 3132d559d7..166e4bd947 100644
--- a/hw/sh4/sh7750.c
+++ b/hw/sh4/sh7750.c
@@ -417,7 +417,7 @@ static void sh7750_mem_writel(void *opaque, hwaddr addr,
     case SH7750_PTEH_A7:
         /* If asid changes, clear all registered tlb entries. */
         if ((s->cpu->env.pteh & 0xff) != (mem_value & 0xff)) {
-            tlb_flush(CPU(s->cpu), 1);
+            tlb_flush(CPU(s->cpu));
         }
         s->cpu->env.pteh = mem_value;
         return;
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index a8c13cee66..bbc9478a50 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -95,15 +95,13 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr);
 /**
  * tlb_flush:
  * @cpu: CPU whose TLB should be flushed
- * @flush_global: ignored
  *
- * Flush the entire TLB for the specified CPU.
- * The flush_global flag is in theory an indicator of whether the whole
- * TLB should be flushed, or only those entries not marked global.
- * In practice QEMU does not implement any global/not global flag for
- * TLB entries, and the argument is ignored.
+ * Flush the entire TLB for the specified CPU. Most CPU architectures
+ * allow the implementation to drop entries from the TLB at any time
+ * so this is generally safe. If more selective flushing is required
+ * use one of the other functions for efficiency.
  */
-void tlb_flush(CPUState *cpu, int flush_global);
+void tlb_flush(CPUState *cpu);
 /**
  * tlb_flush_page_by_mmuidx:
  * @cpu: CPU whose TLB should be flushed
@@ -165,7 +163,7 @@ static inline void tlb_flush_page(CPUState *cpu, target_ulong addr)
 {
 }
 
-static inline void tlb_flush(CPUState *cpu, int flush_global)
+static inline void tlb_flush(CPUState *cpu)
 {
 }
 
diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
index 30d77ce71c..b4f97983e5 100644
--- a/target-alpha/cpu.c
+++ b/target-alpha/cpu.c
@@ -273,7 +273,7 @@ static void alpha_cpu_initfn(Object *obj)
     CPUAlphaState *env = &cpu->env;
 
     cs->env_ptr = env;
-    tlb_flush(cs, 1);
+    tlb_flush(cs);
 
     alpha_translate_init();
 
diff --git a/target-alpha/sys_helper.c b/target-alpha/sys_helper.c
index bec1e178be..652195de6f 100644
--- a/target-alpha/sys_helper.c
+++ b/target-alpha/sys_helper.c
@@ -44,7 +44,7 @@ uint64_t helper_load_pcc(CPUAlphaState *env)
 #ifndef CONFIG_USER_ONLY
 void helper_tbia(CPUAlphaState *env)
 {
-    tlb_flush(CPU(alpha_env_get_cpu(env)), 1);
+    tlb_flush(CPU(alpha_env_get_cpu(env)));
 }
 
 void helper_tbis(CPUAlphaState *env, uint64_t p)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index b5b65caadf..0b2f68956e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -464,7 +464,7 @@ static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
     ARMCPU *cpu = arm_env_get_cpu(env);
 
     raw_write(env, ri, value);
-    tlb_flush(CPU(cpu), 1); /* Flush TLB as domain not tracked in TLB */
+    tlb_flush(CPU(cpu)); /* Flush TLB as domain not tracked in TLB */
 }
 
 static void fcse_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
@@ -475,7 +475,7 @@ static void fcse_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
         /* Unlike real hardware the qemu TLB uses virtual addresses,
          * not modified virtual addresses, so this causes a TLB flush.
          */
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
         raw_write(env, ri, value);
     }
 }
@@ -491,7 +491,7 @@ static void contextidr_write(CPUARMState *env, const ARMCPRegInfo *ri,
          * format) this register includes the ASID, so do a TLB flush.
          * For PMSA it is purely a process ID and no action is needed.
          */
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
     }
     raw_write(env, ri, value);
 }
@@ -502,7 +502,7 @@ static void tlbiall_write(CPUARMState *env, const ARMCPRegInfo *ri,
     /* Invalidate all (TLBIALL) */
     ARMCPU *cpu = arm_env_get_cpu(env);
 
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 static void tlbimva_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -520,7 +520,7 @@ static void tlbiasid_write(CPUARMState *env, const ARMCPRegInfo *ri,
     /* Invalidate by ASID (TLBIASID) */
     ARMCPU *cpu = arm_env_get_cpu(env);
 
-    tlb_flush(CPU(cpu), value == 0);
+    tlb_flush(CPU(cpu));
 }
 
 static void tlbimvaa_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -539,7 +539,7 @@ static void tlbiall_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
     CPUState *other_cs;
 
     CPU_FOREACH(other_cs) {
-        tlb_flush(other_cs, 1);
+        tlb_flush(other_cs);
     }
 }
 
@@ -549,7 +549,7 @@ static void tlbiasid_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
     CPUState *other_cs;
 
     CPU_FOREACH(other_cs) {
-        tlb_flush(other_cs, value == 0);
+        tlb_flush(other_cs);
     }
 }
 
@@ -2310,7 +2310,7 @@ static void pmsav7_write(CPUARMState *env, const ARMCPRegInfo *ri,
     }
 
     u32p += env->cp15.c6_rgnr;
-    tlb_flush(CPU(cpu), 1); /* Mappings may have changed - purge! */
+    tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
     *u32p = value;
 }
 
@@ -2455,7 +2455,7 @@ static void vmsa_ttbcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
         /* With LPAE the TTBCR could result in a change of ASID
          * via the TTBCR.A1 bit, so do a TLB flush.
          */
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
     }
     vmsa_ttbcr_raw_write(env, ri, value);
 }
@@ -2479,7 +2479,7 @@ static void vmsa_tcr_el1_write(CPUARMState *env, const ARMCPRegInfo *ri,
     TCR *tcr = raw_ptr(env, ri);
 
     /* For AArch64 the A1 bit could result in a change of ASID, so TLB flush. */
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
     tcr->raw_tcr = value;
 }
 
@@ -2492,7 +2492,7 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     if (cpreg_field_is_64bit(ri)) {
         ARMCPU *cpu = arm_env_get_cpu(env);
 
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
     }
     raw_write(env, ri, value);
 }
@@ -3160,7 +3160,7 @@ static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
     raw_write(env, ri, value);
     /* ??? Lots of these bits are not implemented.  */
     /* This may enable/disable the MMU, so do a TLB flush.  */
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 static CPAccessResult fpexc32_access(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3628,7 +3628,7 @@ static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
      * HCR_DC Disables stage1 and enables stage2 translation
      */
     if ((raw_read(env, ri) ^ value) & (HCR_VM | HCR_PTW | HCR_DC)) {
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
     }
     raw_write(env, ri, value);
 }
diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c
index 2049a8c01d..66474ad98e 100644
--- a/target-i386/fpu_helper.c
+++ b/target-i386/fpu_helper.c
@@ -1465,7 +1465,7 @@ void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
         }
         if (env->pkru != old_pkru) {
             CPUState *cs = CPU(x86_env_get_cpu(env));
-            tlb_flush(cs, 1);
+            tlb_flush(cs);
         }
     }
 }
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 4ecc0912a4..20a6dfca2d 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -586,7 +586,7 @@ void x86_cpu_set_a20(X86CPU *cpu, int a20_state)
 
         /* when a20 is changed, all the MMU mappings are invalid, so
            we must flush everything */
-        tlb_flush(cs, 1);
+        tlb_flush(cs);
         env->a20_mask = ~(1 << 20) | (a20_state << 20);
     }
 }
@@ -599,7 +599,7 @@ void cpu_x86_update_cr0(CPUX86State *env, uint32_t new_cr0)
     qemu_log_mask(CPU_LOG_MMU, "CR0 update: CR0=0x%08x\n", new_cr0);
     if ((new_cr0 & (CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK)) !=
         (env->cr[0] & (CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK))) {
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
     }
 
 #ifdef TARGET_X86_64
@@ -641,7 +641,7 @@ void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3)
     if (env->cr[0] & CR0_PG_MASK) {
         qemu_log_mask(CPU_LOG_MMU,
                         "CR3 update: CR3=" TARGET_FMT_lx "\n", new_cr3);
-        tlb_flush(CPU(cpu), 0);
+        tlb_flush(CPU(cpu));
     }
 }
 
@@ -656,7 +656,7 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
     if ((new_cr4 ^ env->cr[4]) &
         (CR4_PGE_MASK | CR4_PAE_MASK | CR4_PSE_MASK |
          CR4_SMEP_MASK | CR4_SMAP_MASK)) {
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
     }
 
     /* Clear bits we're going to recompute.  */
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 760f82b6c7..e002b4fc6d 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -387,7 +387,7 @@ static int cpu_post_load(void *opaque, int version_id)
         env->dr[7] = dr7 & ~(DR7_GLOBAL_BP_MASK | DR7_LOCAL_BP_MASK);
         cpu_x86_update_dr7(env, dr7);
     }
-    tlb_flush(cs, 1);
+    tlb_flush(cs);
 
     if (tcg_enabled()) {
         cpu_smm_update(cpu);
diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
index 3f666b4b87..5029efef47 100644
--- a/target-i386/misc_helper.c
+++ b/target-i386/misc_helper.c
@@ -635,5 +635,5 @@ void helper_wrpkru(CPUX86State *env, uint32_t ecx, uint64_t val)
     }
 
     env->pkru = val;
-    tlb_flush(cs, 1);
+    tlb_flush(cs);
 }
diff --git a/target-i386/svm_helper.c b/target-i386/svm_helper.c
index 782b3f12f0..210f6aa7b5 100644
--- a/target-i386/svm_helper.c
+++ b/target-i386/svm_helper.c
@@ -289,7 +289,7 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
         break;
     case TLB_CONTROL_FLUSH_ALL_ASID:
         /* FIXME: this is not 100% correct but should work for now */
-        tlb_flush(cs, 1);
+        tlb_flush(cs);
         break;
     }
 
diff --git a/target-microblaze/mmu.c b/target-microblaze/mmu.c
index a22a496ebb..a0f06758f8 100644
--- a/target-microblaze/mmu.c
+++ b/target-microblaze/mmu.c
@@ -255,7 +255,7 @@ void mmu_write(CPUMBState *env, uint32_t rn, uint32_t v)
             /* Changes to the zone protection reg flush the QEMU TLB.
                Fortunately, these are very uncommon.  */
             if (v != env->mmu.regs[rn]) {
-                tlb_flush(CPU(cpu), 1);
+                tlb_flush(CPU(cpu));
             }
             env->mmu.regs[rn] = v;
             break;
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 3146a6017d..e1c78f55ec 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -1054,7 +1054,7 @@ static inline void compute_hflags(CPUMIPSState *env)
     }
 }
 
-void cpu_mips_tlb_flush(CPUMIPSState *env, int flush_global);
+void cpu_mips_tlb_flush(CPUMIPSState *env);
 void sync_c0_status(CPUMIPSState *env, CPUMIPSState *cpu, int tc);
 void cpu_mips_store_status(CPUMIPSState *env, target_ulong val);
 void cpu_mips_store_cause(CPUMIPSState *env, target_ulong val);
diff --git a/target-mips/helper.c b/target-mips/helper.c
index c864b15b97..d2e77958fd 100644
--- a/target-mips/helper.c
+++ b/target-mips/helper.c
@@ -223,12 +223,12 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
     return ret;
 }
 
-void cpu_mips_tlb_flush(CPUMIPSState *env, int flush_global)
+void cpu_mips_tlb_flush(CPUMIPSState *env)
 {
     MIPSCPU *cpu = mips_env_get_cpu(env);
 
     /* Flush qemu's TLB and discard all shadowed entries.  */
-    tlb_flush(CPU(cpu), flush_global);
+    tlb_flush(CPU(cpu));
     env->tlb->tlb_in_use = env->tlb->nb_tlb;
 }
 
@@ -290,7 +290,7 @@ void cpu_mips_store_status(CPUMIPSState *env, target_ulong val)
 #if defined(TARGET_MIPS64)
     if ((env->CP0_Status ^ old) & (old & (7 << CP0St_UX))) {
         /* Access to at least one of the 64-bit segments has been disabled */
-        cpu_mips_tlb_flush(env, 1);
+        cpu_mips_tlb_flush(env);
     }
 #endif
     if (env->CP0_Config3 & (1 << CP0C3_MT)) {
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 7af4c2f084..047d11e423 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1431,7 +1431,7 @@ void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1)
     /* If the ASID changes, flush qemu's TLB.  */
     if ((old & env->CP0_EntryHi_ASID_mask) !=
         (val & env->CP0_EntryHi_ASID_mask)) {
-        cpu_mips_tlb_flush(env, 1);
+        cpu_mips_tlb_flush(env);
     }
 }
 
@@ -2021,7 +2021,7 @@ void r4k_helper_tlbinv(CPUMIPSState *env)
             tlb->EHINV = 1;
         }
     }
-    cpu_mips_tlb_flush(env, 1);
+    cpu_mips_tlb_flush(env);
 }
 
 void r4k_helper_tlbinvf(CPUMIPSState *env)
@@ -2031,7 +2031,7 @@ void r4k_helper_tlbinvf(CPUMIPSState *env)
     for (idx = 0; idx < env->tlb->nb_tlb; idx++) {
         env->tlb->mmu.r4k.tlb[idx].EHINV = 1;
     }
-    cpu_mips_tlb_flush(env, 1);
+    cpu_mips_tlb_flush(env);
 }
 
 void r4k_helper_tlbwi(CPUMIPSState *env)
@@ -2145,7 +2145,7 @@ void r4k_helper_tlbr(CPUMIPSState *env)
 
     /* If this will change the current ASID, flush qemu's TLB.  */
     if (ASID != tlb->ASID)
-        cpu_mips_tlb_flush (env, 1);
+        cpu_mips_tlb_flush(env);
 
     r4k_mips_tlb_flush_extra(env, env->tlb->nb_tlb);
 
diff --git a/target-openrisc/interrupt.c b/target-openrisc/interrupt.c
index 5fe3f11ffc..e43fc84ef7 100644
--- a/target-openrisc/interrupt.c
+++ b/target-openrisc/interrupt.c
@@ -45,7 +45,7 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
 
     /* For machine-state changed between user-mode and supervisor mode,
        we need flush TLB when we enter&exit EXCP.  */
-    tlb_flush(cs, 1);
+    tlb_flush(cs);
 
     env->esr = env->sr;
     env->sr &= ~SR_DME;
diff --git a/target-openrisc/interrupt_helper.c b/target-openrisc/interrupt_helper.c
index 116f9109a7..0ed5146e8d 100644
--- a/target-openrisc/interrupt_helper.c
+++ b/target-openrisc/interrupt_helper.c
@@ -53,7 +53,7 @@ void HELPER(rfe)(CPUOpenRISCState *env)
     }
 
     if (need_flush_tlb) {
-        tlb_flush(cs, 1);
+        tlb_flush(cs);
     }
 #endif
     cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
diff --git a/target-openrisc/sys_helper.c b/target-openrisc/sys_helper.c
index a719e452be..daea902856 100644
--- a/target-openrisc/sys_helper.c
+++ b/target-openrisc/sys_helper.c
@@ -47,7 +47,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
     case TO_SPR(0, 17): /* SR */
         if ((env->sr & (SR_IME | SR_DME | SR_SM)) ^
             (rb & (SR_IME | SR_DME | SR_SM))) {
-            tlb_flush(cs, 1);
+            tlb_flush(cs);
         }
         env->sr = rb;
         env->sr |= SR_FO;      /* FO is const equal to 1 */
diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
index 62138163a5..2627a70176 100644
--- a/target-ppc/helper_regs.h
+++ b/target-ppc/helper_regs.h
@@ -161,7 +161,7 @@ static inline void check_tlb_flush(CPUPPCState *env, bool global)
 {
     CPUState *cs = CPU(ppc_env_get_cpu(env));
     if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
-        tlb_flush(cs, 1);
+        tlb_flush(cs);
         env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
     }
 
@@ -176,7 +176,7 @@ static inline void check_tlb_flush(CPUPPCState *env, bool global)
                 CPUPPCState *other_env = &cpu->env;
 
                 other_env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
-                tlb_flush(other_cs, 1);
+                tlb_flush(other_cs);
             }
         }
         env->tlb_need_flush &= ~TLB_NEED_GLOBAL_FLUSH;
diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
index 1e6e705a4e..ab432bafaf 100644
--- a/target-ppc/misc_helper.c
+++ b/target-ppc/misc_helper.c
@@ -85,7 +85,7 @@ void helper_store_sdr1(CPUPPCState *env, target_ulong val)
     if (!env->external_htab) {
         if (env->spr[SPR_SDR1] != val) {
             ppc_store_sdr1(env, val);
-            tlb_flush(CPU(cpu), 1);
+            tlb_flush(CPU(cpu));
         }
     }
 }
@@ -114,7 +114,7 @@ void helper_store_403_pbr(CPUPPCState *env, uint32_t num, target_ulong value)
     if (likely(env->pb[num] != value)) {
         env->pb[num] = value;
         /* Should be optimized */
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
     }
 }
 
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index d09fc0a85f..f746f53615 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -248,7 +248,7 @@ static inline void ppc6xx_tlb_invalidate_all(CPUPPCState *env)
         tlb = &env->tlb.tlb6[nr];
         pte_invalidate(&tlb->pte0);
     }
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 static inline void ppc6xx_tlb_invalidate_virt2(CPUPPCState *env,
@@ -661,7 +661,7 @@ static inline void ppc4xx_tlb_invalidate_all(CPUPPCState *env)
         tlb = &env->tlb.tlbe[i];
         tlb->prot &= ~PAGE_VALID;
     }
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
@@ -863,7 +863,7 @@ static void booke206_flush_tlb(CPUPPCState *env, int flags,
         tlb += booke206_tlb_size(env, i);
     }
 
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 static hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
@@ -1769,7 +1769,7 @@ void helper_store_ibatu(CPUPPCState *env, uint32_t nr, target_ulong value)
 #if !defined(FLUSH_ALL_TLBS)
         do_invalidate_BAT(env, env->IBAT[0][nr], mask);
 #else
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
 #endif
     }
 }
@@ -1804,7 +1804,7 @@ void helper_store_dbatu(CPUPPCState *env, uint32_t nr, target_ulong value)
 #if !defined(FLUSH_ALL_TLBS)
         do_invalidate_BAT(env, env->DBAT[0][nr], mask);
 #else
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
 #endif
     }
 }
@@ -1852,7 +1852,7 @@ void helper_store_601_batu(CPUPPCState *env, uint32_t nr, target_ulong value)
         }
 #if defined(FLUSH_ALL_TLBS)
         if (do_inval) {
-            tlb_flush(CPU(cpu), 1);
+            tlb_flush(CPU(cpu));
         }
 #endif
     }
@@ -1892,7 +1892,7 @@ void helper_store_601_batl(CPUPPCState *env, uint32_t nr, target_ulong value)
         env->DBAT[1][nr] = value;
 #if defined(FLUSH_ALL_TLBS)
         if (do_inval) {
-            tlb_flush(CPU(cpu), 1);
+            tlb_flush(CPU(cpu));
         }
 #endif
     }
@@ -1921,7 +1921,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
         cpu_abort(CPU(cpu), "MPC8xx MMU model is not implemented\n");
         break;
     case POWERPC_MMU_BOOKE:
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
         break;
     case POWERPC_MMU_BOOKE206:
         booke206_flush_tlb(env, -1, 0);
@@ -1937,7 +1937,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
     case POWERPC_MMU_2_07a:
 #endif /* defined(TARGET_PPC64) */
         env->tlb_need_flush = 0;
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
         break;
     default:
         /* XXX: TODO */
@@ -2433,13 +2433,13 @@ void helper_440_tlbwe(CPUPPCState *env, uint32_t word, target_ulong entry,
         }
         tlb->PID = env->spr[SPR_440_MMUCR] & 0x000000FF;
         if (do_flush_tlbs) {
-            tlb_flush(CPU(cpu), 1);
+            tlb_flush(CPU(cpu));
         }
         break;
     case 1:
         RPN = value & 0xFFFFFC0F;
         if ((tlb->prot & PAGE_VALID) && tlb->RPN != RPN) {
-            tlb_flush(CPU(cpu), 1);
+            tlb_flush(CPU(cpu));
         }
         tlb->RPN = RPN;
         break;
@@ -2555,7 +2555,7 @@ void helper_booke_setpid(CPUPPCState *env, uint32_t pidn, target_ulong pid)
 
     env->spr[pidn] = pid;
     /* changing PIDs mean we're in a different address space now */
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 void helper_booke206_tlbwe(CPUPPCState *env)
@@ -2650,7 +2650,7 @@ void helper_booke206_tlbwe(CPUPPCState *env)
     if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
         tlb_flush_page(CPU(cpu), tlb->mas2 & MAS2_EPN_MASK);
     } else {
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
     }
 }
 
@@ -2775,7 +2775,7 @@ void helper_booke206_tlbivax(CPUPPCState *env, target_ulong address)
         /* flush TLB1 entries */
         booke206_invalidate_ea_tlb(env, 1, address);
         CPU_FOREACH(cs) {
-            tlb_flush(cs, 1);
+            tlb_flush(cs);
         }
     } else {
         /* flush TLB0 entries */
@@ -2811,7 +2811,7 @@ void helper_booke206_tlbilx1(CPUPPCState *env, target_ulong address)
         }
         tlb += booke206_tlb_size(env, i);
     }
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 void helper_booke206_tlbilx3(CPUPPCState *env, target_ulong address)
@@ -2852,7 +2852,7 @@ void helper_booke206_tlbilx3(CPUPPCState *env, target_ulong address)
             tlb->mas1 &= ~MAS1_VALID;
         }
     }
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 void helper_booke206_tlbflush(CPUPPCState *env, target_ulong type)
diff --git a/target-s390x/gdbstub.c b/target-s390x/gdbstub.c
index 3d223dec97..ea4dc22eeb 100644
--- a/target-s390x/gdbstub.c
+++ b/target-s390x/gdbstub.c
@@ -199,7 +199,7 @@ static int cpu_write_c_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
     case S390_C0_REGNUM ... S390_C15_REGNUM:
         env->cregs[n] = ldtul_p(mem_buf);
         if (tcg_enabled()) {
-            tlb_flush(ENV_GET_CPU(env), 1);
+            tlb_flush(ENV_GET_CPU(env));
         }
         cpu_synchronize_post_init(ENV_GET_CPU(env));
         return 8;
diff --git a/target-s390x/mem_helper.c b/target-s390x/mem_helper.c
index 99bc5e2834..675aba2e44 100644
--- a/target-s390x/mem_helper.c
+++ b/target-s390x/mem_helper.c
@@ -872,7 +872,7 @@ void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
         s390_cpu_recompute_watchpoints(CPU(cpu));
     }
 
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
@@ -900,7 +900,7 @@ void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
         s390_cpu_recompute_watchpoints(CPU(cpu));
     }
 
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 void HELPER(stctg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
@@ -1036,7 +1036,7 @@ uint32_t HELPER(csp)(CPUS390XState *env, uint32_t r1, uint64_t r2)
         cpu_stl_data(env, a2, env->regs[(r1 + 1) & 15]);
         if (r2 & 0x3) {
             /* flush TLB / ALB */
-            tlb_flush(CPU(cpu), 1);
+            tlb_flush(CPU(cpu));
         }
         cc = 0;
     } else {
@@ -1121,7 +1121,7 @@ void HELPER(ptlb)(CPUS390XState *env)
 {
     S390CPU *cpu = s390_env_get_cpu(env);
 
-    tlb_flush(CPU(cpu), 1);
+    tlb_flush(CPU(cpu));
 }
 
 /* load using real address */
diff --git a/target-sh4/helper.c b/target-sh4/helper.c
index a33ac697c5..036c5ca56c 100644
--- a/target-sh4/helper.c
+++ b/target-sh4/helper.c
@@ -583,7 +583,7 @@ void cpu_load_tlb(CPUSH4State * env)
         entry->v = 0;
     }
 
-    tlb_flush(CPU(sh_env_get_cpu(s)), 1);
+    tlb_flush(CPU(sh_env_get_cpu(s)));
 }
 
 uint32_t cpu_sh4_read_mmaped_itlb_addr(CPUSH4State *s,
diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
index de7d53ae20..a0171f73f7 100644
--- a/target-sparc/ldst_helper.c
+++ b/target-sparc/ldst_helper.c
@@ -816,7 +816,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
             case 2: /* flush region (16M) */
             case 3: /* flush context (4G) */
             case 4: /* flush entire */
-                tlb_flush(CPU(cpu), 1);
+                tlb_flush(CPU(cpu));
                 break;
             default:
                 break;
@@ -841,7 +841,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
                    are invalid in normal mode.  */
                 if ((oldreg ^ env->mmuregs[reg])
                     & (MMU_NF | env->def->mmu_bm)) {
-                    tlb_flush(CPU(cpu), 1);
+                    tlb_flush(CPU(cpu));
                 }
                 break;
             case 1: /* Context Table Pointer Register */
@@ -852,7 +852,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
                 if (oldreg != env->mmuregs[reg]) {
                     /* we flush when the MMU context changes because
                        QEMU has no MMU context support */
-                    tlb_flush(CPU(cpu), 1);
+                    tlb_flush(CPU(cpu));
                 }
                 break;
             case 3: /* Synchronous Fault Status Register with Clear */
@@ -1509,13 +1509,13 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val,
                 env->dmmu.mmu_primary_context = val;
                 /* can be optimized to only flush MMU_USER_IDX
                    and MMU_KERNEL_IDX entries */
-                tlb_flush(CPU(cpu), 1);
+                tlb_flush(CPU(cpu));
                 break;
             case 2: /* Secondary context */
                 env->dmmu.mmu_secondary_context = val;
                 /* can be optimized to only flush MMU_USER_SECONDARY_IDX
                    and MMU_KERNEL_SECONDARY_IDX entries */
-                tlb_flush(CPU(cpu), 1);
+                tlb_flush(CPU(cpu));
                 break;
             case 5: /* TSB access */
                 DPRINTF_MMU("dmmu TSB write: 0x%016" PRIx64 " -> 0x%016"
@@ -1654,7 +1654,7 @@ void sparc_cpu_unassigned_access(CPUState *cs, hwaddr addr,
     /* flush neverland mappings created during no-fault mode,
        so the sequential MMU faults report proper fault types */
     if (env->mmuregs[0] & MMU_NF) {
-        tlb_flush(cs, 1);
+        tlb_flush(cs);
     }
 }
 #else
diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
index c169972b59..c9b78ce68e 100644
--- a/target-unicore32/cpu.c
+++ b/target-unicore32/cpu.c
@@ -133,7 +133,7 @@ static void uc32_cpu_initfn(Object *obj)
     env->regs[31] = 0x03000000;
 #endif
 
-    tlb_flush(cs, 1);
+    tlb_flush(cs);
 
     if (tcg_enabled() && !inited) {
         inited = true;
diff --git a/target-unicore32/helper.c b/target-unicore32/helper.c
index d603bde237..9454efa665 100644
--- a/target-unicore32/helper.c
+++ b/target-unicore32/helper.c
@@ -116,7 +116,7 @@ void helper_cp0_set(CPUUniCore32State *env, uint32_t val, uint32_t creg,
     case 6:
         if ((cop <= 6) && (cop >= 2)) {
             /* invalid all tlb */
-            tlb_flush(CPU(cpu), 1);
+            tlb_flush(CPU(cpu));
             return;
         }
         break;
diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c
index 0a4b2147bc..63c89f80c5 100644
--- a/target-xtensa/op_helper.c
+++ b/target-xtensa/op_helper.c
@@ -492,7 +492,7 @@ void HELPER(wsr_rasid)(CPUXtensaState *env, uint32_t v)
     v = (v & 0xffffff00) | 0x1;
     if (v != env->sregs[RASID]) {
         env->sregs[RASID] = v;
-        tlb_flush(CPU(cpu), 1);
+        tlb_flush(CPU(cpu));
     }
 }
 
-- 
2.11.0

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

* Re: [Qemu-devel] [PATCH v2 1/2] qom/cpu: move tlb_flush to cpu_common_reset
  2016-12-15 12:36 ` [Qemu-devel] [PATCH v2 1/2] qom/cpu: move tlb_flush to cpu_common_reset Alex Bennée
@ 2016-12-16  4:21   ` David Gibson
  2016-12-18 20:46   ` Eduardo Habkost
  1 sibling, 0 replies; 8+ messages in thread
From: David Gibson @ 2016-12-16  4:21 UTC (permalink / raw)
  To: Alex Bennée
  Cc: rth, qemu-devel, Peter Maydell, Edgar E. Iglesias, Paolo Bonzini,
	Eduardo Habkost, Michael Walle, Laurent Vivier, Aurelien Jarno,
	Yongbok Kim, Anthony Green, Jia Liu, Alexander Graf,
	Mark Cave-Ayland, Artyom Tarasenko, Bastian Koppelmann,
	open list:ARM, open list:PowerPC

[-- Attachment #1: Type: text/plain, Size: 19156 bytes --]

On Thu, Dec 15, 2016 at 12:36:55PM +0000, Alex Bennée wrote:
> It is a common thing amongst the various cpu reset functions want to
> flush the SoftMMU's TLB entries. This is done either by calling
> tlb_flush directly or by way of a general memset of the CPU
> structure (sometimes both).
> 
> This moves the tlb_flush call to the common reset function and
> additionally ensures it is only done for the CONFIG_SOFTMMU case and
> when tcg is enabled.
> 
> In some target cases we add an empty end_of_reset_fields structure to the
> target vCPU structure so have a clear end point for any memset which
> is resetting value in the structure before CPU_COMMON (where the TLB
> structures are).
> 
> While this is a nice clean-up in general it is also a precursor for
> changes coming to cputlb for MTTCG where the clearing of entries
> can't be done arbitrarily across vCPUs. Currently the cpu_reset
> function is usually called from the context of another vCPU as the
> architectural power up sequence is run. By using the cputlb API
> functions we can ensure the right behaviour in the future.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Richard Henderson <rth@twiddle.net>

target-ppc portions

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  qom/cpu.c                   | 10 ++++++++--
>  target-arm/cpu.c            |  5 ++---
>  target-arm/cpu.h            |  5 ++++-
>  target-cris/cpu.c           |  3 +--
>  target-cris/cpu.h           |  9 ++++++---
>  target-i386/cpu.c           |  2 --
>  target-i386/cpu.h           |  6 ++++--
>  target-lm32/cpu.c           |  3 +--
>  target-lm32/cpu.h           |  3 +++
>  target-m68k/cpu.c           |  3 +--
>  target-m68k/cpu.h           |  3 +++
>  target-microblaze/cpu.c     |  3 +--
>  target-microblaze/cpu.h     |  3 +++
>  target-mips/cpu.c           |  3 +--
>  target-mips/cpu.h           |  3 +++
>  target-moxie/cpu.c          |  4 +---
>  target-moxie/cpu.h          |  3 +++
>  target-openrisc/cpu.c       |  9 +--------
>  target-openrisc/cpu.h       |  3 +++
>  target-ppc/translate_init.c |  3 ---
>  target-s390x/cpu.c          |  7 ++-----
>  target-s390x/cpu.h          |  5 +++--
>  target-sh4/cpu.c            |  3 +--
>  target-sh4/cpu.h            |  3 +++
>  target-sparc/cpu.c          |  3 +--
>  target-sparc/cpu.h          |  3 +++
>  target-tilegx/cpu.c         |  3 +--
>  target-tilegx/cpu.h         |  3 +++
>  target-tricore/cpu.c        |  2 --
>  29 files changed, 66 insertions(+), 52 deletions(-)
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 03d9190f8c..61ee0cb88c 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -270,8 +270,14 @@ static void cpu_common_reset(CPUState *cpu)
>      cpu->exception_index = -1;
>      cpu->crash_occurred = false;
>  
> -    for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
> -        atomic_set(&cpu->tb_jmp_cache[i], NULL);
> +    if (tcg_enabled()) {
> +        for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
> +            atomic_set(&cpu->tb_jmp_cache[i], NULL);
> +        }
> +
> +#ifdef CONFIG_SOFTMMU
> +        tlb_flush(cpu, 0);
> +#endif
>      }
>  }
>  
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 99f0dbebb9..fb05d2e4ec 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -122,7 +122,8 @@ static void arm_cpu_reset(CPUState *s)
>  
>      acc->parent_reset(s);
>  
> -    memset(env, 0, offsetof(CPUARMState, features));
> +    memset(env, 0, offsetof(CPUARMState, end_reset_fields));
> +
>      g_hash_table_foreach(cpu->cp_regs, cp_reg_reset, cpu);
>      g_hash_table_foreach(cpu->cp_regs, cp_reg_check_reset, cpu);
>  
> @@ -226,8 +227,6 @@ static void arm_cpu_reset(CPUState *s)
>                                &env->vfp.fp_status);
>      set_float_detect_tininess(float_tininess_before_rounding,
>                                &env->vfp.standard_fp_status);
> -    tlb_flush(s, 1);
> -
>  #ifndef CONFIG_USER_ONLY
>      if (kvm_enabled()) {
>          kvm_arm_reset_vcpu(cpu);
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index ca5c849ed6..53e9d55adf 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -491,9 +491,12 @@ typedef struct CPUARMState {
>      struct CPUBreakpoint *cpu_breakpoint[16];
>      struct CPUWatchpoint *cpu_watchpoint[16];
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
> -    /* These fields after the common ones so they are preserved on reset.  */
> +    /* Fields after CPU_COMMON are preserved across CPU reset. */
>  
>      /* Internal CPU feature flags.  */
>      uint64_t features;
> diff --git a/target-cris/cpu.c b/target-cris/cpu.c
> index 2e9ab9700e..5f766f09d6 100644
> --- a/target-cris/cpu.c
> +++ b/target-cris/cpu.c
> @@ -52,9 +52,8 @@ static void cris_cpu_reset(CPUState *s)
>      ccc->parent_reset(s);
>  
>      vr = env->pregs[PR_VR];
> -    memset(env, 0, offsetof(CPUCRISState, load_info));
> +    memset(env, 0, offsetof(CPUCRISState, end_reset_fields));
>      env->pregs[PR_VR] = vr;
> -    tlb_flush(s, 1);
>  
>  #if defined(CONFIG_USER_ONLY)
>      /* start in user mode with interrupts enabled.  */
> diff --git a/target-cris/cpu.h b/target-cris/cpu.h
> index 43d5f9d1da..920e1c33ba 100644
> --- a/target-cris/cpu.h
> +++ b/target-cris/cpu.h
> @@ -167,10 +167,13 @@ typedef struct CPUCRISState {
>  	 */
>          TLBSet tlbsets[2][4][16];
>  
> -	CPU_COMMON
> +        /* Fields up to this point are cleared by a CPU reset */
> +        struct {} end_reset_fields;
>  
> -    /* Members from load_info on are preserved across resets.  */
> -    void *load_info;
> +        CPU_COMMON
> +
> +        /* Members from load_info on are preserved across resets.  */
> +        void *load_info;
>  } CPUCRISState;
>  
>  /**
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index de1f30eeda..810893952a 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2815,8 +2815,6 @@ static void x86_cpu_reset(CPUState *s)
>  
>      memset(env, 0, offsetof(CPUX86State, end_reset_fields));
>  
> -    tlb_flush(s, 1);
> -
>      env->old_exception = -1;
>  
>      /* init to reset state */
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index c605724022..95ed91d8d1 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -1119,10 +1119,12 @@ typedef struct CPUX86State {
>      uint8_t nmi_injected;
>      uint8_t nmi_pending;
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
> -    /* Fields from here on are preserved across CPU reset. */
> -    struct {} end_reset_fields;
> +    /* Fields after CPU_COMMON are preserved across CPU reset. */
>  
>      /* processor features (e.g. for CPUID insn) */
>      /* Minimum level/xlevel/xlevel2, based on CPU model + features */
> diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
> index 8d939a7779..2b8c36b6d0 100644
> --- a/target-lm32/cpu.c
> +++ b/target-lm32/cpu.c
> @@ -128,10 +128,9 @@ static void lm32_cpu_reset(CPUState *s)
>      lcc->parent_reset(s);
>  
>      /* reset cpu state */
> -    memset(env, 0, offsetof(CPULM32State, eba));
> +    memset(env, 0, offsetof(CPULM32State, end_reset_fields));
>  
>      lm32_cpu_init_cfg_reg(cpu);
> -    tlb_flush(s, 1);
>  }
>  
>  static void lm32_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
> diff --git a/target-lm32/cpu.h b/target-lm32/cpu.h
> index d8a3515244..1d972cb26b 100644
> --- a/target-lm32/cpu.h
> +++ b/target-lm32/cpu.h
> @@ -165,6 +165,9 @@ struct CPULM32State {
>      struct CPUBreakpoint *cpu_breakpoint[4];
>      struct CPUWatchpoint *cpu_watchpoint[4];
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
>      /* Fields from here on are preserved across CPU reset. */
> diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c
> index ba17480098..fa10b6e4cd 100644
> --- a/target-m68k/cpu.c
> +++ b/target-m68k/cpu.c
> @@ -52,7 +52,7 @@ static void m68k_cpu_reset(CPUState *s)
>  
>      mcc->parent_reset(s);
>  
> -    memset(env, 0, offsetof(CPUM68KState, features));
> +    memset(env, 0, offsetof(CPUM68KState, end_reset_fields));
>  #if !defined(CONFIG_USER_ONLY)
>      env->sr = 0x2700;
>  #endif
> @@ -61,7 +61,6 @@ static void m68k_cpu_reset(CPUState *s)
>      cpu_m68k_set_ccr(env, 0);
>      /* TODO: We should set PC from the interrupt vector.  */
>      env->pc = 0;
> -    tlb_flush(s, 1);
>  }
>  
>  static void m68k_cpu_disas_set_info(CPUState *s, disassemble_info *info)
> diff --git a/target-m68k/cpu.h b/target-m68k/cpu.h
> index 6dfb54eb70..8e7b51b756 100644
> --- a/target-m68k/cpu.h
> +++ b/target-m68k/cpu.h
> @@ -115,6 +115,9 @@ typedef struct CPUM68KState {
>  
>      uint32_t qregs[MAX_QREGS];
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
>      /* Fields from here on are preserved across CPU reset. */
> diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c
> index 389c7b691e..3d58869716 100644
> --- a/target-microblaze/cpu.c
> +++ b/target-microblaze/cpu.c
> @@ -103,9 +103,8 @@ static void mb_cpu_reset(CPUState *s)
>  
>      mcc->parent_reset(s);
>  
> -    memset(env, 0, offsetof(CPUMBState, pvr));
> +    memset(env, 0, offsetof(CPUMBState, end_reset_fields));
>      env->res_addr = RES_ADDR_NONE;
> -    tlb_flush(s, 1);
>  
>      /* Disable stack protector.  */
>      env->shr = ~0;
> diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h
> index beb75ffd26..bf6963bcb7 100644
> --- a/target-microblaze/cpu.h
> +++ b/target-microblaze/cpu.h
> @@ -267,6 +267,9 @@ struct CPUMBState {
>      struct microblaze_mmu mmu;
>  #endif
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
>      /* These fields are preserved on reset.  */
> diff --git a/target-mips/cpu.c b/target-mips/cpu.c
> index 65ca607f88..1bb66b7a5a 100644
> --- a/target-mips/cpu.c
> +++ b/target-mips/cpu.c
> @@ -100,8 +100,7 @@ static void mips_cpu_reset(CPUState *s)
>  
>      mcc->parent_reset(s);
>  
> -    memset(env, 0, offsetof(CPUMIPSState, mvp));
> -    tlb_flush(s, 1);
> +    memset(env, 0, offsetof(CPUMIPSState, end_reset_fields));
>  
>      cpu_state_reset(env);
>  
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index 5182dc74ff..3146a6017d 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -607,6 +607,9 @@ struct CPUMIPSState {
>      uint32_t CP0_TCStatus_rw_bitmask; /* Read/write bits in CP0_TCStatus */
>      int insn_flags; /* Supported instruction set */
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
>      /* Fields from here on are preserved across CPU reset. */
> diff --git a/target-moxie/cpu.c b/target-moxie/cpu.c
> index b0be4a7551..927b1a1e44 100644
> --- a/target-moxie/cpu.c
> +++ b/target-moxie/cpu.c
> @@ -45,10 +45,8 @@ static void moxie_cpu_reset(CPUState *s)
>  
>      mcc->parent_reset(s);
>  
> -    memset(env, 0, sizeof(CPUMoxieState));
> +    memset(env, 0, offsetof(CPUMoxieState, end_reset_fields));
>      env->pc = 0x1000;
> -
> -    tlb_flush(s, 1);
>  }
>  
>  static void moxie_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
> diff --git a/target-moxie/cpu.h b/target-moxie/cpu.h
> index 3e880facf4..8991aaef9a 100644
> --- a/target-moxie/cpu.h
> +++ b/target-moxie/cpu.h
> @@ -56,6 +56,9 @@ typedef struct CPUMoxieState {
>  
>      void *irq[8];
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
>  } CPUMoxieState;
> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
> index 698e87bb25..422139d29f 100644
> --- a/target-openrisc/cpu.c
> +++ b/target-openrisc/cpu.c
> @@ -44,14 +44,7 @@ static void openrisc_cpu_reset(CPUState *s)
>  
>      occ->parent_reset(s);
>  
> -#ifndef CONFIG_USER_ONLY
> -    memset(&cpu->env, 0, offsetof(CPUOpenRISCState, tlb));
> -#else
> -    memset(&cpu->env, 0, offsetof(CPUOpenRISCState, irq));
> -#endif
> -
> -    tlb_flush(s, 1);
> -    /*tb_flush(&cpu->env);    FIXME: Do we need it?  */
> +    memset(&cpu->env, 0, offsetof(CPUOpenRISCState, end_reset_fields));
>  
>      cpu->env.pc = 0x100;
>      cpu->env.sr = SR_FO | SR_SM;
> diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
> index aaf153579a..508ef568b4 100644
> --- a/target-openrisc/cpu.h
> +++ b/target-openrisc/cpu.h
> @@ -300,6 +300,9 @@ typedef struct CPUOpenRISCState {
>                                   in solt so far.  */
>      uint32_t btaken;          /* the SR_F bit */
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
>      /* Fields from here on are preserved across CPU reset. */
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 626e03186c..4ff987226e 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -10415,9 +10415,6 @@ static void ppc_cpu_reset(CPUState *s)
>          }
>          env->spr[i] = spr->default_value;
>      }
> -
> -    /* Flush all TLBs */
> -    tlb_flush(s, 1);
>  }
>  
>  #ifndef CONFIG_USER_ONLY
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 0a39d31237..066dcd17df 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -82,7 +82,6 @@ static void s390_cpu_reset(CPUState *s)
>      scc->parent_reset(s);
>      cpu->env.sigp_order = 0;
>      s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
> -    tlb_flush(s, 1);
>  }
>  
>  /* S390CPUClass::initial_reset() */
> @@ -94,7 +93,7 @@ static void s390_cpu_initial_reset(CPUState *s)
>  
>      s390_cpu_reset(s);
>      /* initial reset does not touch regs,fregs and aregs */
> -    memset(&env->fpc, 0, offsetof(CPUS390XState, cpu_num) -
> +    memset(&env->fpc, 0, offsetof(CPUS390XState, end_reset_fields) -
>                           offsetof(CPUS390XState, fpc));
>  
>      /* architectured initial values for CR 0 and 14 */
> @@ -118,7 +117,6 @@ static void s390_cpu_initial_reset(CPUState *s)
>      if (kvm_enabled()) {
>          kvm_s390_reset_vcpu(cpu);
>      }
> -    tlb_flush(s, 1);
>  }
>  
>  /* CPUClass:reset() */
> @@ -133,7 +131,7 @@ static void s390_cpu_full_reset(CPUState *s)
>      cpu->env.sigp_order = 0;
>      s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
>  
> -    memset(env, 0, offsetof(CPUS390XState, cpu_num));
> +    memset(env, 0, offsetof(CPUS390XState, end_reset_fields));
>  
>      /* architectured initial values for CR 0 and 14 */
>      env->cregs[0] = CR0_RESET;
> @@ -156,7 +154,6 @@ static void s390_cpu_full_reset(CPUState *s)
>      if (kvm_enabled()) {
>          kvm_s390_reset_vcpu(cpu);
>      }
> -    tlb_flush(s, 1);
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index fd36a25cf5..058ddad83a 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -139,9 +139,10 @@ typedef struct CPUS390XState {
>  
>      uint8_t riccb[64];
>  
> -    CPU_COMMON
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
>  
> -    /* reset does memset(0) up to here */
> +    CPU_COMMON
>  
>      uint32_t cpu_num;
>      uint32_t machine_type;
> diff --git a/target-sh4/cpu.c b/target-sh4/cpu.c
> index a38f6a6ded..9a481c35dc 100644
> --- a/target-sh4/cpu.c
> +++ b/target-sh4/cpu.c
> @@ -56,8 +56,7 @@ static void superh_cpu_reset(CPUState *s)
>  
>      scc->parent_reset(s);
>  
> -    memset(env, 0, offsetof(CPUSH4State, id));
> -    tlb_flush(s, 1);
> +    memset(env, 0, offsetof(CPUSH4State, end_reset_fields));
>  
>      env->pc = 0xA0000000;
>  #if defined(CONFIG_USER_ONLY)
> diff --git a/target-sh4/cpu.h b/target-sh4/cpu.h
> index 478ab55868..cad8989f7e 100644
> --- a/target-sh4/cpu.h
> +++ b/target-sh4/cpu.h
> @@ -175,6 +175,9 @@ typedef struct CPUSH4State {
>  
>      uint32_t ldst;
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
>      /* Fields from here on are preserved over CPU reset. */
> diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
> index 4e07b92fbd..d6583f1c2a 100644
> --- a/target-sparc/cpu.c
> +++ b/target-sparc/cpu.c
> @@ -36,8 +36,7 @@ static void sparc_cpu_reset(CPUState *s)
>  
>      scc->parent_reset(s);
>  
> -    memset(env, 0, offsetof(CPUSPARCState, version));
> -    tlb_flush(s, 1);
> +    memset(env, 0, offsetof(CPUSPARCState, end_reset_fields));
>      env->cwp = 0;
>  #ifndef TARGET_SPARC64
>      env->wim = 1;
> diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
> index 5fb0ed1aad..601c018a05 100644
> --- a/target-sparc/cpu.h
> +++ b/target-sparc/cpu.h
> @@ -419,6 +419,9 @@ struct CPUSPARCState {
>      /* NOTE: we allow 8 more registers to handle wrapping */
>      target_ulong regbase[MAX_NWINDOWS * 16 + 8];
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
>      /* Fields from here on are preserved across CPU reset. */
> diff --git a/target-tilegx/cpu.c b/target-tilegx/cpu.c
> index 454793f94a..d90e38e88c 100644
> --- a/target-tilegx/cpu.c
> +++ b/target-tilegx/cpu.c
> @@ -84,8 +84,7 @@ static void tilegx_cpu_reset(CPUState *s)
>  
>      tcc->parent_reset(s);
>  
> -    memset(env, 0, sizeof(CPUTLGState));
> -    tlb_flush(s, 1);
> +    memset(env, 0, offsetof(CPUTLGState, end_reset_fields));
>  }
>  
>  static void tilegx_cpu_realizefn(DeviceState *dev, Error **errp)
> diff --git a/target-tilegx/cpu.h b/target-tilegx/cpu.h
> index 1735427233..f32be49f65 100644
> --- a/target-tilegx/cpu.h
> +++ b/target-tilegx/cpu.h
> @@ -97,6 +97,9 @@ typedef struct CPUTLGState {
>      uint32_t sigcode;                  /* Signal code */
>  #endif
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  } CPUTLGState;
>  
> diff --git a/target-tricore/cpu.c b/target-tricore/cpu.c
> index 785b76bd3a..08f50e2ba7 100644
> --- a/target-tricore/cpu.c
> +++ b/target-tricore/cpu.c
> @@ -53,8 +53,6 @@ static void tricore_cpu_reset(CPUState *s)
>  
>      tcc->parent_reset(s);
>  
> -    tlb_flush(s, 1);
> -
>      cpu_state_reset(env);
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [Qemu-devel] [PATCH v2 2/2] cputlb: drop flush_global flag from tlb_flush
  2016-12-15 12:36 ` [Qemu-devel] [PATCH v2 2/2] cputlb: drop flush_global flag from tlb_flush Alex Bennée
@ 2016-12-16  4:21   ` David Gibson
  0 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2016-12-16  4:21 UTC (permalink / raw)
  To: Alex Bennée
  Cc: rth, qemu-devel, Paolo Bonzini, Peter Crosthwaite, Aurelien Jarno,
	Peter Maydell, Eduardo Habkost, Edgar E. Iglesias, Yongbok Kim,
	Jia Liu, Alexander Graf, Mark Cave-Ayland, Artyom Tarasenko,
	Guan Xuetao, Max Filippov, open list:ARM, open list:PowerPC

[-- Attachment #1: Type: text/plain, Size: 33962 bytes --]

On Thu, Dec 15, 2016 at 12:36:56PM +0000, Alex Bennée wrote:
> We have never has the concept of global TLB entries which would avoid
> the flush so we never actually use this flag. Drop it and make clear
> that tlb_flush is the sledge-hammer it has always been.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Richard Henderson <rth@twiddle.net>

ppc portions

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  cputlb.c                           | 21 ++++++---------------
>  exec.c                             |  4 ++--
>  hw/sh4/sh7750.c                    |  2 +-
>  include/exec/exec-all.h            | 14 ++++++--------
>  target-alpha/cpu.c                 |  2 +-
>  target-alpha/sys_helper.c          |  2 +-
>  target-arm/helper.c                | 26 +++++++++++++-------------
>  target-i386/fpu_helper.c           |  2 +-
>  target-i386/helper.c               |  8 ++++----
>  target-i386/machine.c              |  2 +-
>  target-i386/misc_helper.c          |  2 +-
>  target-i386/svm_helper.c           |  2 +-
>  target-microblaze/mmu.c            |  2 +-
>  target-mips/cpu.h                  |  2 +-
>  target-mips/helper.c               |  6 +++---
>  target-mips/op_helper.c            |  8 ++++----
>  target-openrisc/interrupt.c        |  2 +-
>  target-openrisc/interrupt_helper.c |  2 +-
>  target-openrisc/sys_helper.c       |  2 +-
>  target-ppc/helper_regs.h           |  4 ++--
>  target-ppc/misc_helper.c           |  4 ++--
>  target-ppc/mmu_helper.c            | 32 ++++++++++++++++----------------
>  target-s390x/gdbstub.c             |  2 +-
>  target-s390x/mem_helper.c          |  8 ++++----
>  target-sh4/helper.c                |  2 +-
>  target-sparc/ldst_helper.c         | 12 ++++++------
>  target-unicore32/cpu.c             |  2 +-
>  target-unicore32/helper.c          |  2 +-
>  target-xtensa/op_helper.c          |  2 +-
>  29 files changed, 85 insertions(+), 96 deletions(-)
> 
> diff --git a/cputlb.c b/cputlb.c
> index 813279f3bc..6c39927455 100644
> --- a/cputlb.c
> +++ b/cputlb.c
> @@ -60,24 +60,15 @@
>  /* statistics */
>  int tlb_flush_count;
>  
> -/* NOTE:
> - * If flush_global is true (the usual case), flush all tlb entries.
> - * If flush_global is false, flush (at least) all tlb entries not
> - * marked global.
> - *
> - * Since QEMU doesn't currently implement a global/not-global flag
> - * for tlb entries, at the moment tlb_flush() will also flush all
> - * tlb entries in the flush_global == false case. This is OK because
> - * CPU architectures generally permit an implementation to drop
> - * entries from the TLB at any time, so flushing more entries than
> - * required is only an efficiency issue, not a correctness issue.
> +/* This is OK because CPU architectures generally permit an
> + * implementation to drop entries from the TLB at any time, so
> + * flushing more entries than required is only an efficiency issue,
> + * not a correctness issue.
>   */
> -void tlb_flush(CPUState *cpu, int flush_global)
> +void tlb_flush(CPUState *cpu)
>  {
>      CPUArchState *env = cpu->env_ptr;
>  
> -    tlb_debug("(%d)\n", flush_global);
> -
>      memset(env->tlb_table, -1, sizeof(env->tlb_table));
>      memset(env->tlb_v_table, -1, sizeof(env->tlb_v_table));
>      memset(cpu->tb_jmp_cache, 0, sizeof(cpu->tb_jmp_cache));
> @@ -144,7 +135,7 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr)
>                    TARGET_FMT_lx "/" TARGET_FMT_lx ")\n",
>                    env->tlb_flush_addr, env->tlb_flush_mask);
>  
> -        tlb_flush(cpu, 1);
> +        tlb_flush(cpu);
>          return;
>      }
>  
> diff --git a/exec.c b/exec.c
> index 08c558eecf..57128539b4 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -511,7 +511,7 @@ static int cpu_common_post_load(void *opaque, int version_id)
>      /* 0x01 was CPU_INTERRUPT_EXIT. This line can be removed when the
>         version_id is increased. */
>      cpu->interrupt_request &= ~0x01;
> -    tlb_flush(cpu, 1);
> +    tlb_flush(cpu);
>  
>      return 0;
>  }
> @@ -2393,7 +2393,7 @@ static void tcg_commit(MemoryListener *listener)
>       */
>      d = atomic_rcu_read(&cpuas->as->dispatch);
>      atomic_rcu_set(&cpuas->memory_dispatch, d);
> -    tlb_flush(cpuas->cpu, 1);
> +    tlb_flush(cpuas->cpu);
>  }
>  
>  void address_space_init_dispatch(AddressSpace *as)
> diff --git a/hw/sh4/sh7750.c b/hw/sh4/sh7750.c
> index 3132d559d7..166e4bd947 100644
> --- a/hw/sh4/sh7750.c
> +++ b/hw/sh4/sh7750.c
> @@ -417,7 +417,7 @@ static void sh7750_mem_writel(void *opaque, hwaddr addr,
>      case SH7750_PTEH_A7:
>          /* If asid changes, clear all registered tlb entries. */
>          if ((s->cpu->env.pteh & 0xff) != (mem_value & 0xff)) {
> -            tlb_flush(CPU(s->cpu), 1);
> +            tlb_flush(CPU(s->cpu));
>          }
>          s->cpu->env.pteh = mem_value;
>          return;
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index a8c13cee66..bbc9478a50 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -95,15 +95,13 @@ void tlb_flush_page(CPUState *cpu, target_ulong addr);
>  /**
>   * tlb_flush:
>   * @cpu: CPU whose TLB should be flushed
> - * @flush_global: ignored
>   *
> - * Flush the entire TLB for the specified CPU.
> - * The flush_global flag is in theory an indicator of whether the whole
> - * TLB should be flushed, or only those entries not marked global.
> - * In practice QEMU does not implement any global/not global flag for
> - * TLB entries, and the argument is ignored.
> + * Flush the entire TLB for the specified CPU. Most CPU architectures
> + * allow the implementation to drop entries from the TLB at any time
> + * so this is generally safe. If more selective flushing is required
> + * use one of the other functions for efficiency.
>   */
> -void tlb_flush(CPUState *cpu, int flush_global);
> +void tlb_flush(CPUState *cpu);
>  /**
>   * tlb_flush_page_by_mmuidx:
>   * @cpu: CPU whose TLB should be flushed
> @@ -165,7 +163,7 @@ static inline void tlb_flush_page(CPUState *cpu, target_ulong addr)
>  {
>  }
>  
> -static inline void tlb_flush(CPUState *cpu, int flush_global)
> +static inline void tlb_flush(CPUState *cpu)
>  {
>  }
>  
> diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c
> index 30d77ce71c..b4f97983e5 100644
> --- a/target-alpha/cpu.c
> +++ b/target-alpha/cpu.c
> @@ -273,7 +273,7 @@ static void alpha_cpu_initfn(Object *obj)
>      CPUAlphaState *env = &cpu->env;
>  
>      cs->env_ptr = env;
> -    tlb_flush(cs, 1);
> +    tlb_flush(cs);
>  
>      alpha_translate_init();
>  
> diff --git a/target-alpha/sys_helper.c b/target-alpha/sys_helper.c
> index bec1e178be..652195de6f 100644
> --- a/target-alpha/sys_helper.c
> +++ b/target-alpha/sys_helper.c
> @@ -44,7 +44,7 @@ uint64_t helper_load_pcc(CPUAlphaState *env)
>  #ifndef CONFIG_USER_ONLY
>  void helper_tbia(CPUAlphaState *env)
>  {
> -    tlb_flush(CPU(alpha_env_get_cpu(env)), 1);
> +    tlb_flush(CPU(alpha_env_get_cpu(env)));
>  }
>  
>  void helper_tbis(CPUAlphaState *env, uint64_t p)
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index b5b65caadf..0b2f68956e 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -464,7 +464,7 @@ static void dacr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>      ARMCPU *cpu = arm_env_get_cpu(env);
>  
>      raw_write(env, ri, value);
> -    tlb_flush(CPU(cpu), 1); /* Flush TLB as domain not tracked in TLB */
> +    tlb_flush(CPU(cpu)); /* Flush TLB as domain not tracked in TLB */
>  }
>  
>  static void fcse_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
> @@ -475,7 +475,7 @@ static void fcse_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>          /* Unlike real hardware the qemu TLB uses virtual addresses,
>           * not modified virtual addresses, so this causes a TLB flush.
>           */
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>          raw_write(env, ri, value);
>      }
>  }
> @@ -491,7 +491,7 @@ static void contextidr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>           * format) this register includes the ASID, so do a TLB flush.
>           * For PMSA it is purely a process ID and no action is needed.
>           */
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>      }
>      raw_write(env, ri, value);
>  }
> @@ -502,7 +502,7 @@ static void tlbiall_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      /* Invalidate all (TLBIALL) */
>      ARMCPU *cpu = arm_env_get_cpu(env);
>  
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  static void tlbimva_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -520,7 +520,7 @@ static void tlbiasid_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      /* Invalidate by ASID (TLBIASID) */
>      ARMCPU *cpu = arm_env_get_cpu(env);
>  
> -    tlb_flush(CPU(cpu), value == 0);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  static void tlbimvaa_write(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -539,7 +539,7 @@ static void tlbiall_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      CPUState *other_cs;
>  
>      CPU_FOREACH(other_cs) {
> -        tlb_flush(other_cs, 1);
> +        tlb_flush(other_cs);
>      }
>  }
>  
> @@ -549,7 +549,7 @@ static void tlbiasid_is_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      CPUState *other_cs;
>  
>      CPU_FOREACH(other_cs) {
> -        tlb_flush(other_cs, value == 0);
> +        tlb_flush(other_cs);
>      }
>  }
>  
> @@ -2310,7 +2310,7 @@ static void pmsav7_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      }
>  
>      u32p += env->cp15.c6_rgnr;
> -    tlb_flush(CPU(cpu), 1); /* Mappings may have changed - purge! */
> +    tlb_flush(CPU(cpu)); /* Mappings may have changed - purge! */
>      *u32p = value;
>  }
>  
> @@ -2455,7 +2455,7 @@ static void vmsa_ttbcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>          /* With LPAE the TTBCR could result in a change of ASID
>           * via the TTBCR.A1 bit, so do a TLB flush.
>           */
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>      }
>      vmsa_ttbcr_raw_write(env, ri, value);
>  }
> @@ -2479,7 +2479,7 @@ static void vmsa_tcr_el1_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      TCR *tcr = raw_ptr(env, ri);
>  
>      /* For AArch64 the A1 bit could result in a change of ASID, so TLB flush. */
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>      tcr->raw_tcr = value;
>  }
>  
> @@ -2492,7 +2492,7 @@ static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      if (cpreg_field_is_64bit(ri)) {
>          ARMCPU *cpu = arm_env_get_cpu(env);
>  
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>      }
>      raw_write(env, ri, value);
>  }
> @@ -3160,7 +3160,7 @@ static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>      raw_write(env, ri, value);
>      /* ??? Lots of these bits are not implemented.  */
>      /* This may enable/disable the MMU, so do a TLB flush.  */
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  static CPAccessResult fpexc32_access(CPUARMState *env, const ARMCPRegInfo *ri,
> @@ -3628,7 +3628,7 @@ static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
>       * HCR_DC Disables stage1 and enables stage2 translation
>       */
>      if ((raw_read(env, ri) ^ value) & (HCR_VM | HCR_PTW | HCR_DC)) {
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>      }
>      raw_write(env, ri, value);
>  }
> diff --git a/target-i386/fpu_helper.c b/target-i386/fpu_helper.c
> index 2049a8c01d..66474ad98e 100644
> --- a/target-i386/fpu_helper.c
> +++ b/target-i386/fpu_helper.c
> @@ -1465,7 +1465,7 @@ void helper_xrstor(CPUX86State *env, target_ulong ptr, uint64_t rfbm)
>          }
>          if (env->pkru != old_pkru) {
>              CPUState *cs = CPU(x86_env_get_cpu(env));
> -            tlb_flush(cs, 1);
> +            tlb_flush(cs);
>          }
>      }
>  }
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 4ecc0912a4..20a6dfca2d 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -586,7 +586,7 @@ void x86_cpu_set_a20(X86CPU *cpu, int a20_state)
>  
>          /* when a20 is changed, all the MMU mappings are invalid, so
>             we must flush everything */
> -        tlb_flush(cs, 1);
> +        tlb_flush(cs);
>          env->a20_mask = ~(1 << 20) | (a20_state << 20);
>      }
>  }
> @@ -599,7 +599,7 @@ void cpu_x86_update_cr0(CPUX86State *env, uint32_t new_cr0)
>      qemu_log_mask(CPU_LOG_MMU, "CR0 update: CR0=0x%08x\n", new_cr0);
>      if ((new_cr0 & (CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK)) !=
>          (env->cr[0] & (CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK))) {
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>      }
>  
>  #ifdef TARGET_X86_64
> @@ -641,7 +641,7 @@ void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3)
>      if (env->cr[0] & CR0_PG_MASK) {
>          qemu_log_mask(CPU_LOG_MMU,
>                          "CR3 update: CR3=" TARGET_FMT_lx "\n", new_cr3);
> -        tlb_flush(CPU(cpu), 0);
> +        tlb_flush(CPU(cpu));
>      }
>  }
>  
> @@ -656,7 +656,7 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
>      if ((new_cr4 ^ env->cr[4]) &
>          (CR4_PGE_MASK | CR4_PAE_MASK | CR4_PSE_MASK |
>           CR4_SMEP_MASK | CR4_SMAP_MASK)) {
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>      }
>  
>      /* Clear bits we're going to recompute.  */
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 760f82b6c7..e002b4fc6d 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -387,7 +387,7 @@ static int cpu_post_load(void *opaque, int version_id)
>          env->dr[7] = dr7 & ~(DR7_GLOBAL_BP_MASK | DR7_LOCAL_BP_MASK);
>          cpu_x86_update_dr7(env, dr7);
>      }
> -    tlb_flush(cs, 1);
> +    tlb_flush(cs);
>  
>      if (tcg_enabled()) {
>          cpu_smm_update(cpu);
> diff --git a/target-i386/misc_helper.c b/target-i386/misc_helper.c
> index 3f666b4b87..5029efef47 100644
> --- a/target-i386/misc_helper.c
> +++ b/target-i386/misc_helper.c
> @@ -635,5 +635,5 @@ void helper_wrpkru(CPUX86State *env, uint32_t ecx, uint64_t val)
>      }
>  
>      env->pkru = val;
> -    tlb_flush(cs, 1);
> +    tlb_flush(cs);
>  }
> diff --git a/target-i386/svm_helper.c b/target-i386/svm_helper.c
> index 782b3f12f0..210f6aa7b5 100644
> --- a/target-i386/svm_helper.c
> +++ b/target-i386/svm_helper.c
> @@ -289,7 +289,7 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
>          break;
>      case TLB_CONTROL_FLUSH_ALL_ASID:
>          /* FIXME: this is not 100% correct but should work for now */
> -        tlb_flush(cs, 1);
> +        tlb_flush(cs);
>          break;
>      }
>  
> diff --git a/target-microblaze/mmu.c b/target-microblaze/mmu.c
> index a22a496ebb..a0f06758f8 100644
> --- a/target-microblaze/mmu.c
> +++ b/target-microblaze/mmu.c
> @@ -255,7 +255,7 @@ void mmu_write(CPUMBState *env, uint32_t rn, uint32_t v)
>              /* Changes to the zone protection reg flush the QEMU TLB.
>                 Fortunately, these are very uncommon.  */
>              if (v != env->mmu.regs[rn]) {
> -                tlb_flush(CPU(cpu), 1);
> +                tlb_flush(CPU(cpu));
>              }
>              env->mmu.regs[rn] = v;
>              break;
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index 3146a6017d..e1c78f55ec 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -1054,7 +1054,7 @@ static inline void compute_hflags(CPUMIPSState *env)
>      }
>  }
>  
> -void cpu_mips_tlb_flush(CPUMIPSState *env, int flush_global);
> +void cpu_mips_tlb_flush(CPUMIPSState *env);
>  void sync_c0_status(CPUMIPSState *env, CPUMIPSState *cpu, int tc);
>  void cpu_mips_store_status(CPUMIPSState *env, target_ulong val);
>  void cpu_mips_store_cause(CPUMIPSState *env, target_ulong val);
> diff --git a/target-mips/helper.c b/target-mips/helper.c
> index c864b15b97..d2e77958fd 100644
> --- a/target-mips/helper.c
> +++ b/target-mips/helper.c
> @@ -223,12 +223,12 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
>      return ret;
>  }
>  
> -void cpu_mips_tlb_flush(CPUMIPSState *env, int flush_global)
> +void cpu_mips_tlb_flush(CPUMIPSState *env)
>  {
>      MIPSCPU *cpu = mips_env_get_cpu(env);
>  
>      /* Flush qemu's TLB and discard all shadowed entries.  */
> -    tlb_flush(CPU(cpu), flush_global);
> +    tlb_flush(CPU(cpu));
>      env->tlb->tlb_in_use = env->tlb->nb_tlb;
>  }
>  
> @@ -290,7 +290,7 @@ void cpu_mips_store_status(CPUMIPSState *env, target_ulong val)
>  #if defined(TARGET_MIPS64)
>      if ((env->CP0_Status ^ old) & (old & (7 << CP0St_UX))) {
>          /* Access to at least one of the 64-bit segments has been disabled */
> -        cpu_mips_tlb_flush(env, 1);
> +        cpu_mips_tlb_flush(env);
>      }
>  #endif
>      if (env->CP0_Config3 & (1 << CP0C3_MT)) {
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index 7af4c2f084..047d11e423 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -1431,7 +1431,7 @@ void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1)
>      /* If the ASID changes, flush qemu's TLB.  */
>      if ((old & env->CP0_EntryHi_ASID_mask) !=
>          (val & env->CP0_EntryHi_ASID_mask)) {
> -        cpu_mips_tlb_flush(env, 1);
> +        cpu_mips_tlb_flush(env);
>      }
>  }
>  
> @@ -2021,7 +2021,7 @@ void r4k_helper_tlbinv(CPUMIPSState *env)
>              tlb->EHINV = 1;
>          }
>      }
> -    cpu_mips_tlb_flush(env, 1);
> +    cpu_mips_tlb_flush(env);
>  }
>  
>  void r4k_helper_tlbinvf(CPUMIPSState *env)
> @@ -2031,7 +2031,7 @@ void r4k_helper_tlbinvf(CPUMIPSState *env)
>      for (idx = 0; idx < env->tlb->nb_tlb; idx++) {
>          env->tlb->mmu.r4k.tlb[idx].EHINV = 1;
>      }
> -    cpu_mips_tlb_flush(env, 1);
> +    cpu_mips_tlb_flush(env);
>  }
>  
>  void r4k_helper_tlbwi(CPUMIPSState *env)
> @@ -2145,7 +2145,7 @@ void r4k_helper_tlbr(CPUMIPSState *env)
>  
>      /* If this will change the current ASID, flush qemu's TLB.  */
>      if (ASID != tlb->ASID)
> -        cpu_mips_tlb_flush (env, 1);
> +        cpu_mips_tlb_flush(env);
>  
>      r4k_mips_tlb_flush_extra(env, env->tlb->nb_tlb);
>  
> diff --git a/target-openrisc/interrupt.c b/target-openrisc/interrupt.c
> index 5fe3f11ffc..e43fc84ef7 100644
> --- a/target-openrisc/interrupt.c
> +++ b/target-openrisc/interrupt.c
> @@ -45,7 +45,7 @@ void openrisc_cpu_do_interrupt(CPUState *cs)
>  
>      /* For machine-state changed between user-mode and supervisor mode,
>         we need flush TLB when we enter&exit EXCP.  */
> -    tlb_flush(cs, 1);
> +    tlb_flush(cs);
>  
>      env->esr = env->sr;
>      env->sr &= ~SR_DME;
> diff --git a/target-openrisc/interrupt_helper.c b/target-openrisc/interrupt_helper.c
> index 116f9109a7..0ed5146e8d 100644
> --- a/target-openrisc/interrupt_helper.c
> +++ b/target-openrisc/interrupt_helper.c
> @@ -53,7 +53,7 @@ void HELPER(rfe)(CPUOpenRISCState *env)
>      }
>  
>      if (need_flush_tlb) {
> -        tlb_flush(cs, 1);
> +        tlb_flush(cs);
>      }
>  #endif
>      cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
> diff --git a/target-openrisc/sys_helper.c b/target-openrisc/sys_helper.c
> index a719e452be..daea902856 100644
> --- a/target-openrisc/sys_helper.c
> +++ b/target-openrisc/sys_helper.c
> @@ -47,7 +47,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
>      case TO_SPR(0, 17): /* SR */
>          if ((env->sr & (SR_IME | SR_DME | SR_SM)) ^
>              (rb & (SR_IME | SR_DME | SR_SM))) {
> -            tlb_flush(cs, 1);
> +            tlb_flush(cs);
>          }
>          env->sr = rb;
>          env->sr |= SR_FO;      /* FO is const equal to 1 */
> diff --git a/target-ppc/helper_regs.h b/target-ppc/helper_regs.h
> index 62138163a5..2627a70176 100644
> --- a/target-ppc/helper_regs.h
> +++ b/target-ppc/helper_regs.h
> @@ -161,7 +161,7 @@ static inline void check_tlb_flush(CPUPPCState *env, bool global)
>  {
>      CPUState *cs = CPU(ppc_env_get_cpu(env));
>      if (env->tlb_need_flush & TLB_NEED_LOCAL_FLUSH) {
> -        tlb_flush(cs, 1);
> +        tlb_flush(cs);
>          env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
>      }
>  
> @@ -176,7 +176,7 @@ static inline void check_tlb_flush(CPUPPCState *env, bool global)
>                  CPUPPCState *other_env = &cpu->env;
>  
>                  other_env->tlb_need_flush &= ~TLB_NEED_LOCAL_FLUSH;
> -                tlb_flush(other_cs, 1);
> +                tlb_flush(other_cs);
>              }
>          }
>          env->tlb_need_flush &= ~TLB_NEED_GLOBAL_FLUSH;
> diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
> index 1e6e705a4e..ab432bafaf 100644
> --- a/target-ppc/misc_helper.c
> +++ b/target-ppc/misc_helper.c
> @@ -85,7 +85,7 @@ void helper_store_sdr1(CPUPPCState *env, target_ulong val)
>      if (!env->external_htab) {
>          if (env->spr[SPR_SDR1] != val) {
>              ppc_store_sdr1(env, val);
> -            tlb_flush(CPU(cpu), 1);
> +            tlb_flush(CPU(cpu));
>          }
>      }
>  }
> @@ -114,7 +114,7 @@ void helper_store_403_pbr(CPUPPCState *env, uint32_t num, target_ulong value)
>      if (likely(env->pb[num] != value)) {
>          env->pb[num] = value;
>          /* Should be optimized */
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>      }
>  }
>  
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index d09fc0a85f..f746f53615 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -248,7 +248,7 @@ static inline void ppc6xx_tlb_invalidate_all(CPUPPCState *env)
>          tlb = &env->tlb.tlb6[nr];
>          pte_invalidate(&tlb->pte0);
>      }
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  static inline void ppc6xx_tlb_invalidate_virt2(CPUPPCState *env,
> @@ -661,7 +661,7 @@ static inline void ppc4xx_tlb_invalidate_all(CPUPPCState *env)
>          tlb = &env->tlb.tlbe[i];
>          tlb->prot &= ~PAGE_VALID;
>      }
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
> @@ -863,7 +863,7 @@ static void booke206_flush_tlb(CPUPPCState *env, int flags,
>          tlb += booke206_tlb_size(env, i);
>      }
>  
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  static hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
> @@ -1769,7 +1769,7 @@ void helper_store_ibatu(CPUPPCState *env, uint32_t nr, target_ulong value)
>  #if !defined(FLUSH_ALL_TLBS)
>          do_invalidate_BAT(env, env->IBAT[0][nr], mask);
>  #else
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>  #endif
>      }
>  }
> @@ -1804,7 +1804,7 @@ void helper_store_dbatu(CPUPPCState *env, uint32_t nr, target_ulong value)
>  #if !defined(FLUSH_ALL_TLBS)
>          do_invalidate_BAT(env, env->DBAT[0][nr], mask);
>  #else
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>  #endif
>      }
>  }
> @@ -1852,7 +1852,7 @@ void helper_store_601_batu(CPUPPCState *env, uint32_t nr, target_ulong value)
>          }
>  #if defined(FLUSH_ALL_TLBS)
>          if (do_inval) {
> -            tlb_flush(CPU(cpu), 1);
> +            tlb_flush(CPU(cpu));
>          }
>  #endif
>      }
> @@ -1892,7 +1892,7 @@ void helper_store_601_batl(CPUPPCState *env, uint32_t nr, target_ulong value)
>          env->DBAT[1][nr] = value;
>  #if defined(FLUSH_ALL_TLBS)
>          if (do_inval) {
> -            tlb_flush(CPU(cpu), 1);
> +            tlb_flush(CPU(cpu));
>          }
>  #endif
>      }
> @@ -1921,7 +1921,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
>          cpu_abort(CPU(cpu), "MPC8xx MMU model is not implemented\n");
>          break;
>      case POWERPC_MMU_BOOKE:
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>          break;
>      case POWERPC_MMU_BOOKE206:
>          booke206_flush_tlb(env, -1, 0);
> @@ -1937,7 +1937,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
>      case POWERPC_MMU_2_07a:
>  #endif /* defined(TARGET_PPC64) */
>          env->tlb_need_flush = 0;
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>          break;
>      default:
>          /* XXX: TODO */
> @@ -2433,13 +2433,13 @@ void helper_440_tlbwe(CPUPPCState *env, uint32_t word, target_ulong entry,
>          }
>          tlb->PID = env->spr[SPR_440_MMUCR] & 0x000000FF;
>          if (do_flush_tlbs) {
> -            tlb_flush(CPU(cpu), 1);
> +            tlb_flush(CPU(cpu));
>          }
>          break;
>      case 1:
>          RPN = value & 0xFFFFFC0F;
>          if ((tlb->prot & PAGE_VALID) && tlb->RPN != RPN) {
> -            tlb_flush(CPU(cpu), 1);
> +            tlb_flush(CPU(cpu));
>          }
>          tlb->RPN = RPN;
>          break;
> @@ -2555,7 +2555,7 @@ void helper_booke_setpid(CPUPPCState *env, uint32_t pidn, target_ulong pid)
>  
>      env->spr[pidn] = pid;
>      /* changing PIDs mean we're in a different address space now */
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  void helper_booke206_tlbwe(CPUPPCState *env)
> @@ -2650,7 +2650,7 @@ void helper_booke206_tlbwe(CPUPPCState *env)
>      if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
>          tlb_flush_page(CPU(cpu), tlb->mas2 & MAS2_EPN_MASK);
>      } else {
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>      }
>  }
>  
> @@ -2775,7 +2775,7 @@ void helper_booke206_tlbivax(CPUPPCState *env, target_ulong address)
>          /* flush TLB1 entries */
>          booke206_invalidate_ea_tlb(env, 1, address);
>          CPU_FOREACH(cs) {
> -            tlb_flush(cs, 1);
> +            tlb_flush(cs);
>          }
>      } else {
>          /* flush TLB0 entries */
> @@ -2811,7 +2811,7 @@ void helper_booke206_tlbilx1(CPUPPCState *env, target_ulong address)
>          }
>          tlb += booke206_tlb_size(env, i);
>      }
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  void helper_booke206_tlbilx3(CPUPPCState *env, target_ulong address)
> @@ -2852,7 +2852,7 @@ void helper_booke206_tlbilx3(CPUPPCState *env, target_ulong address)
>              tlb->mas1 &= ~MAS1_VALID;
>          }
>      }
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  void helper_booke206_tlbflush(CPUPPCState *env, target_ulong type)
> diff --git a/target-s390x/gdbstub.c b/target-s390x/gdbstub.c
> index 3d223dec97..ea4dc22eeb 100644
> --- a/target-s390x/gdbstub.c
> +++ b/target-s390x/gdbstub.c
> @@ -199,7 +199,7 @@ static int cpu_write_c_reg(CPUS390XState *env, uint8_t *mem_buf, int n)
>      case S390_C0_REGNUM ... S390_C15_REGNUM:
>          env->cregs[n] = ldtul_p(mem_buf);
>          if (tcg_enabled()) {
> -            tlb_flush(ENV_GET_CPU(env), 1);
> +            tlb_flush(ENV_GET_CPU(env));
>          }
>          cpu_synchronize_post_init(ENV_GET_CPU(env));
>          return 8;
> diff --git a/target-s390x/mem_helper.c b/target-s390x/mem_helper.c
> index 99bc5e2834..675aba2e44 100644
> --- a/target-s390x/mem_helper.c
> +++ b/target-s390x/mem_helper.c
> @@ -872,7 +872,7 @@ void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>          s390_cpu_recompute_watchpoints(CPU(cpu));
>      }
>  
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
> @@ -900,7 +900,7 @@ void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
>          s390_cpu_recompute_watchpoints(CPU(cpu));
>      }
>  
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  void HELPER(stctg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
> @@ -1036,7 +1036,7 @@ uint32_t HELPER(csp)(CPUS390XState *env, uint32_t r1, uint64_t r2)
>          cpu_stl_data(env, a2, env->regs[(r1 + 1) & 15]);
>          if (r2 & 0x3) {
>              /* flush TLB / ALB */
> -            tlb_flush(CPU(cpu), 1);
> +            tlb_flush(CPU(cpu));
>          }
>          cc = 0;
>      } else {
> @@ -1121,7 +1121,7 @@ void HELPER(ptlb)(CPUS390XState *env)
>  {
>      S390CPU *cpu = s390_env_get_cpu(env);
>  
> -    tlb_flush(CPU(cpu), 1);
> +    tlb_flush(CPU(cpu));
>  }
>  
>  /* load using real address */
> diff --git a/target-sh4/helper.c b/target-sh4/helper.c
> index a33ac697c5..036c5ca56c 100644
> --- a/target-sh4/helper.c
> +++ b/target-sh4/helper.c
> @@ -583,7 +583,7 @@ void cpu_load_tlb(CPUSH4State * env)
>          entry->v = 0;
>      }
>  
> -    tlb_flush(CPU(sh_env_get_cpu(s)), 1);
> +    tlb_flush(CPU(sh_env_get_cpu(s)));
>  }
>  
>  uint32_t cpu_sh4_read_mmaped_itlb_addr(CPUSH4State *s,
> diff --git a/target-sparc/ldst_helper.c b/target-sparc/ldst_helper.c
> index de7d53ae20..a0171f73f7 100644
> --- a/target-sparc/ldst_helper.c
> +++ b/target-sparc/ldst_helper.c
> @@ -816,7 +816,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>              case 2: /* flush region (16M) */
>              case 3: /* flush context (4G) */
>              case 4: /* flush entire */
> -                tlb_flush(CPU(cpu), 1);
> +                tlb_flush(CPU(cpu));
>                  break;
>              default:
>                  break;
> @@ -841,7 +841,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>                     are invalid in normal mode.  */
>                  if ((oldreg ^ env->mmuregs[reg])
>                      & (MMU_NF | env->def->mmu_bm)) {
> -                    tlb_flush(CPU(cpu), 1);
> +                    tlb_flush(CPU(cpu));
>                  }
>                  break;
>              case 1: /* Context Table Pointer Register */
> @@ -852,7 +852,7 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>                  if (oldreg != env->mmuregs[reg]) {
>                      /* we flush when the MMU context changes because
>                         QEMU has no MMU context support */
> -                    tlb_flush(CPU(cpu), 1);
> +                    tlb_flush(CPU(cpu));
>                  }
>                  break;
>              case 3: /* Synchronous Fault Status Register with Clear */
> @@ -1509,13 +1509,13 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, target_ulong val,
>                  env->dmmu.mmu_primary_context = val;
>                  /* can be optimized to only flush MMU_USER_IDX
>                     and MMU_KERNEL_IDX entries */
> -                tlb_flush(CPU(cpu), 1);
> +                tlb_flush(CPU(cpu));
>                  break;
>              case 2: /* Secondary context */
>                  env->dmmu.mmu_secondary_context = val;
>                  /* can be optimized to only flush MMU_USER_SECONDARY_IDX
>                     and MMU_KERNEL_SECONDARY_IDX entries */
> -                tlb_flush(CPU(cpu), 1);
> +                tlb_flush(CPU(cpu));
>                  break;
>              case 5: /* TSB access */
>                  DPRINTF_MMU("dmmu TSB write: 0x%016" PRIx64 " -> 0x%016"
> @@ -1654,7 +1654,7 @@ void sparc_cpu_unassigned_access(CPUState *cs, hwaddr addr,
>      /* flush neverland mappings created during no-fault mode,
>         so the sequential MMU faults report proper fault types */
>      if (env->mmuregs[0] & MMU_NF) {
> -        tlb_flush(cs, 1);
> +        tlb_flush(cs);
>      }
>  }
>  #else
> diff --git a/target-unicore32/cpu.c b/target-unicore32/cpu.c
> index c169972b59..c9b78ce68e 100644
> --- a/target-unicore32/cpu.c
> +++ b/target-unicore32/cpu.c
> @@ -133,7 +133,7 @@ static void uc32_cpu_initfn(Object *obj)
>      env->regs[31] = 0x03000000;
>  #endif
>  
> -    tlb_flush(cs, 1);
> +    tlb_flush(cs);
>  
>      if (tcg_enabled() && !inited) {
>          inited = true;
> diff --git a/target-unicore32/helper.c b/target-unicore32/helper.c
> index d603bde237..9454efa665 100644
> --- a/target-unicore32/helper.c
> +++ b/target-unicore32/helper.c
> @@ -116,7 +116,7 @@ void helper_cp0_set(CPUUniCore32State *env, uint32_t val, uint32_t creg,
>      case 6:
>          if ((cop <= 6) && (cop >= 2)) {
>              /* invalid all tlb */
> -            tlb_flush(CPU(cpu), 1);
> +            tlb_flush(CPU(cpu));
>              return;
>          }
>          break;
> diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c
> index 0a4b2147bc..63c89f80c5 100644
> --- a/target-xtensa/op_helper.c
> +++ b/target-xtensa/op_helper.c
> @@ -492,7 +492,7 @@ void HELPER(wsr_rasid)(CPUXtensaState *env, uint32_t v)
>      v = (v & 0xffffff00) | 0x1;
>      if (v != env->sregs[RASID]) {
>          env->sregs[RASID] = v;
> -        tlb_flush(CPU(cpu), 1);
> +        tlb_flush(CPU(cpu));
>      }
>  }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [Qemu-devel] [PATCH v2 1/2] qom/cpu: move tlb_flush to cpu_common_reset
  2016-12-15 12:36 ` [Qemu-devel] [PATCH v2 1/2] qom/cpu: move tlb_flush to cpu_common_reset Alex Bennée
  2016-12-16  4:21   ` David Gibson
@ 2016-12-18 20:46   ` Eduardo Habkost
  2017-01-09 15:05     ` Alex Bennée
  1 sibling, 1 reply; 8+ messages in thread
From: Eduardo Habkost @ 2016-12-18 20:46 UTC (permalink / raw)
  To: Alex Bennée
  Cc: rth, qemu-devel, Peter Maydell, Edgar E. Iglesias, Paolo Bonzini,
	Michael Walle, Laurent Vivier, Aurelien Jarno, Yongbok Kim,
	Anthony Green, Jia Liu, David Gibson, Alexander Graf,
	Mark Cave-Ayland, Artyom Tarasenko, Bastian Koppelmann,
	open list:ARM, open list:PowerPC

On Thu, Dec 15, 2016 at 12:36:55PM +0000, Alex Bennée wrote:
> It is a common thing amongst the various cpu reset functions want to
> flush the SoftMMU's TLB entries. This is done either by calling
> tlb_flush directly or by way of a general memset of the CPU
> structure (sometimes both).
> 
> This moves the tlb_flush call to the common reset function and
> additionally ensures it is only done for the CONFIG_SOFTMMU case and
> when tcg is enabled.
> 
> In some target cases we add an empty end_of_reset_fields structure to the
> target vCPU structure so have a clear end point for any memset which
> is resetting value in the structure before CPU_COMMON (where the TLB
> structures are).
> 
> While this is a nice clean-up in general it is also a precursor for
> changes coming to cputlb for MTTCG where the clearing of entries
> can't be done arbitrarily across vCPUs. Currently the cpu_reset
> function is usually called from the context of another vCPU as the
> architectural power up sequence is run. By using the cputlb API
> functions we can ensure the right behaviour in the future.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> ---
>  qom/cpu.c                   | 10 ++++++++--
>  target-arm/cpu.c            |  5 ++---
>  target-arm/cpu.h            |  5 ++++-
>  target-cris/cpu.c           |  3 +--
>  target-cris/cpu.h           |  9 ++++++---
>  target-i386/cpu.c           |  2 --
>  target-i386/cpu.h           |  6 ++++--
>  target-lm32/cpu.c           |  3 +--
>  target-lm32/cpu.h           |  3 +++
>  target-m68k/cpu.c           |  3 +--
>  target-m68k/cpu.h           |  3 +++
>  target-microblaze/cpu.c     |  3 +--
>  target-microblaze/cpu.h     |  3 +++
>  target-mips/cpu.c           |  3 +--
>  target-mips/cpu.h           |  3 +++
>  target-moxie/cpu.c          |  4 +---
>  target-moxie/cpu.h          |  3 +++
>  target-openrisc/cpu.c       |  9 +--------
>  target-openrisc/cpu.h       |  3 +++
>  target-ppc/translate_init.c |  3 ---
>  target-s390x/cpu.c          |  7 ++-----
>  target-s390x/cpu.h          |  5 +++--
>  target-sh4/cpu.c            |  3 +--
>  target-sh4/cpu.h            |  3 +++
>  target-sparc/cpu.c          |  3 +--
>  target-sparc/cpu.h          |  3 +++
>  target-tilegx/cpu.c         |  3 +--
>  target-tilegx/cpu.h         |  3 +++
>  target-tricore/cpu.c        |  2 --
>  29 files changed, 66 insertions(+), 52 deletions(-)
> 
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 03d9190f8c..61ee0cb88c 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -270,8 +270,14 @@ static void cpu_common_reset(CPUState *cpu)
>      cpu->exception_index = -1;
>      cpu->crash_occurred = false;
>  
> -    for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
> -        atomic_set(&cpu->tb_jmp_cache[i], NULL);
> +    if (tcg_enabled()) {
> +        for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
> +            atomic_set(&cpu->tb_jmp_cache[i], NULL);
> +        }
> +
> +#ifdef CONFIG_SOFTMMU
> +        tlb_flush(cpu, 0);
> +#endif

The patch is changing 3 things at the same time:

1) Moving the tb_jmp_cache reset inside if (tcg_enabled())
2) Moving the tlb_flush() call to cpu_common_reset()
3) Moving the tlb_flush() call inside if (tcg_enabled())

Are we 100% sure there's no non-TCG looking looking at tlb_*
fields or calling tlb_*() functions anywhere? Isn't it better to
add the tcg_enabled() check in a separate patch?


>      }
>  }
>  
[...]
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index de1f30eeda..810893952a 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2815,8 +2815,6 @@ static void x86_cpu_reset(CPUState *s)
>  
>      memset(env, 0, offsetof(CPUX86State, end_reset_fields));
>  
> -    tlb_flush(s, 1);
> -
>      env->old_exception = -1;
>  
>      /* init to reset state */
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index c605724022..95ed91d8d1 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -1119,10 +1119,12 @@ typedef struct CPUX86State {
>      uint8_t nmi_injected;
>      uint8_t nmi_pending;
>  
> +    /* Fields up to this point are cleared by a CPU reset */
> +    struct {} end_reset_fields;
> +
>      CPU_COMMON
>  
> -    /* Fields from here on are preserved across CPU reset. */
> -    struct {} end_reset_fields;
> +    /* Fields after CPU_COMMON are preserved across CPU reset. */
>  
>      /* processor features (e.g. for CPUID insn) */
>      /* Minimum level/xlevel/xlevel2, based on CPU model + features */

Do you have plans to move the tlb_* fields from CPUArchState to
CPUState and remove CPU_COMMON completely?


> diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
> index 8d939a7779..2b8c36b6d0 100644
> --- a/target-lm32/cpu.c
> +++ b/target-lm32/cpu.c
> @@ -128,10 +128,9 @@ static void lm32_cpu_reset(CPUState *s)
>      lcc->parent_reset(s);
>  
>      /* reset cpu state */
> -    memset(env, 0, offsetof(CPULM32State, eba));
> +    memset(env, 0, offsetof(CPULM32State, end_reset_fields));
>  
>      lm32_cpu_init_cfg_reg(cpu);
> -    tlb_flush(s, 1);
>  }
>  
[...]

-- 
Eduardo

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

* Re: [Qemu-devel] [PATCH v2 1/2] qom/cpu: move tlb_flush to cpu_common_reset
  2016-12-18 20:46   ` Eduardo Habkost
@ 2017-01-09 15:05     ` Alex Bennée
  2017-01-11 17:00       ` Eduardo Habkost
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2017-01-09 15:05 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: rth, qemu-devel, Peter Maydell, Edgar E. Iglesias, Paolo Bonzini,
	Michael Walle, Laurent Vivier, Aurelien Jarno, Yongbok Kim,
	Anthony Green, Jia Liu, David Gibson, Alexander Graf,
	Mark Cave-Ayland, Artyom Tarasenko, Bastian Koppelmann,
	open list:ARM, open list:PowerPC


Eduardo Habkost <ehabkost@redhat.com> writes:

> On Thu, Dec 15, 2016 at 12:36:55PM +0000, Alex Bennée wrote:
>> It is a common thing amongst the various cpu reset functions want to
>> flush the SoftMMU's TLB entries. This is done either by calling
>> tlb_flush directly or by way of a general memset of the CPU
>> structure (sometimes both).
>>
>> This moves the tlb_flush call to the common reset function and
>> additionally ensures it is only done for the CONFIG_SOFTMMU case and
>> when tcg is enabled.
>>
>> In some target cases we add an empty end_of_reset_fields structure to the
>> target vCPU structure so have a clear end point for any memset which
>> is resetting value in the structure before CPU_COMMON (where the TLB
>> structures are).
>>
>> While this is a nice clean-up in general it is also a precursor for
>> changes coming to cputlb for MTTCG where the clearing of entries
>> can't be done arbitrarily across vCPUs. Currently the cpu_reset
>> function is usually called from the context of another vCPU as the
>> architectural power up sequence is run. By using the cputlb API
>> functions we can ensure the right behaviour in the future.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Richard Henderson <rth@twiddle.net>
>> ---
>>  qom/cpu.c                   | 10 ++++++++--
>>  target-arm/cpu.c            |  5 ++---
>>  target-arm/cpu.h            |  5 ++++-
>>  target-cris/cpu.c           |  3 +--
>>  target-cris/cpu.h           |  9 ++++++---
>>  target-i386/cpu.c           |  2 --
>>  target-i386/cpu.h           |  6 ++++--
>>  target-lm32/cpu.c           |  3 +--
>>  target-lm32/cpu.h           |  3 +++
>>  target-m68k/cpu.c           |  3 +--
>>  target-m68k/cpu.h           |  3 +++
>>  target-microblaze/cpu.c     |  3 +--
>>  target-microblaze/cpu.h     |  3 +++
>>  target-mips/cpu.c           |  3 +--
>>  target-mips/cpu.h           |  3 +++
>>  target-moxie/cpu.c          |  4 +---
>>  target-moxie/cpu.h          |  3 +++
>>  target-openrisc/cpu.c       |  9 +--------
>>  target-openrisc/cpu.h       |  3 +++
>>  target-ppc/translate_init.c |  3 ---
>>  target-s390x/cpu.c          |  7 ++-----
>>  target-s390x/cpu.h          |  5 +++--
>>  target-sh4/cpu.c            |  3 +--
>>  target-sh4/cpu.h            |  3 +++
>>  target-sparc/cpu.c          |  3 +--
>>  target-sparc/cpu.h          |  3 +++
>>  target-tilegx/cpu.c         |  3 +--
>>  target-tilegx/cpu.h         |  3 +++
>>  target-tricore/cpu.c        |  2 --
>>  29 files changed, 66 insertions(+), 52 deletions(-)
>>
>> diff --git a/qom/cpu.c b/qom/cpu.c
>> index 03d9190f8c..61ee0cb88c 100644
>> --- a/qom/cpu.c
>> +++ b/qom/cpu.c
>> @@ -270,8 +270,14 @@ static void cpu_common_reset(CPUState *cpu)
>>      cpu->exception_index = -1;
>>      cpu->crash_occurred = false;
>>
>> -    for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
>> -        atomic_set(&cpu->tb_jmp_cache[i], NULL);
>> +    if (tcg_enabled()) {
>> +        for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
>> +            atomic_set(&cpu->tb_jmp_cache[i], NULL);
>> +        }
>> +
>> +#ifdef CONFIG_SOFTMMU
>> +        tlb_flush(cpu, 0);
>> +#endif
>
> The patch is changing 3 things at the same time:
>
> 1) Moving the tb_jmp_cache reset inside if (tcg_enabled())

The tb_jump_cache only ever contains TranslationBlocks which are TCG
only. Any access outside of TCG would be confusing stuff.

> 2) Moving the tlb_flush() call to cpu_common_reset()

This is the main purpose of the patch.

> 3) Moving the tlb_flush() call inside if (tcg_enabled())
>
> Are we 100% sure there's no non-TCG looking looking at tlb_*
> fields or calling tlb_*() functions anywhere? Isn't it better to
> add the tcg_enabled() check in a separate patch?

The tlb_* functions are NOP in-lines for linux-user mode. Otherwise the
table is only accessed by generated SoftMMU code and the associated
helpers. AFAICT the KVM migration code uses the memory sub-system to
track the dirty state of pages so shouldn't be looking at those fields.

I seemed sensible to clean it up all in one patch, however I could split
it into two:

  1) move tlb_flush (with CONFIG_SOFTMMU) to qom/cpu.c
  2) wrap whole thing in tcg_enabled()

Would that be OK?

--
Alex Bennée

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

* Re: [Qemu-devel] [PATCH v2 1/2] qom/cpu: move tlb_flush to cpu_common_reset
  2017-01-09 15:05     ` Alex Bennée
@ 2017-01-11 17:00       ` Eduardo Habkost
  0 siblings, 0 replies; 8+ messages in thread
From: Eduardo Habkost @ 2017-01-11 17:00 UTC (permalink / raw)
  To: Alex Bennée
  Cc: rth, qemu-devel, Peter Maydell, Edgar E. Iglesias, Paolo Bonzini,
	Michael Walle, Laurent Vivier, Aurelien Jarno, Yongbok Kim,
	Anthony Green, Jia Liu, David Gibson, Alexander Graf,
	Mark Cave-Ayland, Artyom Tarasenko, Bastian Koppelmann,
	open list:ARM, open list:PowerPC

On Mon, Jan 09, 2017 at 03:05:11PM +0000, Alex Bennée wrote:
> 
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Thu, Dec 15, 2016 at 12:36:55PM +0000, Alex Bennée wrote:
> >> It is a common thing amongst the various cpu reset functions want to
> >> flush the SoftMMU's TLB entries. This is done either by calling
> >> tlb_flush directly or by way of a general memset of the CPU
> >> structure (sometimes both).
> >>
> >> This moves the tlb_flush call to the common reset function and
> >> additionally ensures it is only done for the CONFIG_SOFTMMU case and
> >> when tcg is enabled.
> >>
> >> In some target cases we add an empty end_of_reset_fields structure to the
> >> target vCPU structure so have a clear end point for any memset which
> >> is resetting value in the structure before CPU_COMMON (where the TLB
> >> structures are).
> >>
> >> While this is a nice clean-up in general it is also a precursor for
> >> changes coming to cputlb for MTTCG where the clearing of entries
> >> can't be done arbitrarily across vCPUs. Currently the cpu_reset
> >> function is usually called from the context of another vCPU as the
> >> architectural power up sequence is run. By using the cputlb API
> >> functions we can ensure the right behaviour in the future.
> >>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >> Reviewed-by: Richard Henderson <rth@twiddle.net>
> >> ---
> >>  qom/cpu.c                   | 10 ++++++++--
> >>  target-arm/cpu.c            |  5 ++---
> >>  target-arm/cpu.h            |  5 ++++-
> >>  target-cris/cpu.c           |  3 +--
> >>  target-cris/cpu.h           |  9 ++++++---
> >>  target-i386/cpu.c           |  2 --
> >>  target-i386/cpu.h           |  6 ++++--
> >>  target-lm32/cpu.c           |  3 +--
> >>  target-lm32/cpu.h           |  3 +++
> >>  target-m68k/cpu.c           |  3 +--
> >>  target-m68k/cpu.h           |  3 +++
> >>  target-microblaze/cpu.c     |  3 +--
> >>  target-microblaze/cpu.h     |  3 +++
> >>  target-mips/cpu.c           |  3 +--
> >>  target-mips/cpu.h           |  3 +++
> >>  target-moxie/cpu.c          |  4 +---
> >>  target-moxie/cpu.h          |  3 +++
> >>  target-openrisc/cpu.c       |  9 +--------
> >>  target-openrisc/cpu.h       |  3 +++
> >>  target-ppc/translate_init.c |  3 ---
> >>  target-s390x/cpu.c          |  7 ++-----
> >>  target-s390x/cpu.h          |  5 +++--
> >>  target-sh4/cpu.c            |  3 +--
> >>  target-sh4/cpu.h            |  3 +++
> >>  target-sparc/cpu.c          |  3 +--
> >>  target-sparc/cpu.h          |  3 +++
> >>  target-tilegx/cpu.c         |  3 +--
> >>  target-tilegx/cpu.h         |  3 +++
> >>  target-tricore/cpu.c        |  2 --
> >>  29 files changed, 66 insertions(+), 52 deletions(-)
> >>
> >> diff --git a/qom/cpu.c b/qom/cpu.c
> >> index 03d9190f8c..61ee0cb88c 100644
> >> --- a/qom/cpu.c
> >> +++ b/qom/cpu.c
> >> @@ -270,8 +270,14 @@ static void cpu_common_reset(CPUState *cpu)
> >>      cpu->exception_index = -1;
> >>      cpu->crash_occurred = false;
> >>
> >> -    for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
> >> -        atomic_set(&cpu->tb_jmp_cache[i], NULL);
> >> +    if (tcg_enabled()) {
> >> +        for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
> >> +            atomic_set(&cpu->tb_jmp_cache[i], NULL);
> >> +        }
> >> +
> >> +#ifdef CONFIG_SOFTMMU
> >> +        tlb_flush(cpu, 0);
> >> +#endif
> >
> > The patch is changing 3 things at the same time:
> >
> > 1) Moving the tb_jmp_cache reset inside if (tcg_enabled())
> 
> The tb_jump_cache only ever contains TranslationBlocks which are TCG
> only. Any access outside of TCG would be confusing stuff.
> 
> > 2) Moving the tlb_flush() call to cpu_common_reset()
> 
> This is the main purpose of the patch.
> 
> > 3) Moving the tlb_flush() call inside if (tcg_enabled())
> >
> > Are we 100% sure there's no non-TCG looking looking at tlb_*
> > fields or calling tlb_*() functions anywhere? Isn't it better to
> > add the tcg_enabled() check in a separate patch?
> 
> The tlb_* functions are NOP in-lines for linux-user mode. Otherwise the
> table is only accessed by generated SoftMMU code and the associated
> helpers. AFAICT the KVM migration code uses the memory sub-system to
> track the dirty state of pages so shouldn't be looking at those fields.
> 
> I seemed sensible to clean it up all in one patch, however I could split
> it into two:
> 
>   1) move tlb_flush (with CONFIG_SOFTMMU) to qom/cpu.c
>   2) wrap whole thing in tcg_enabled()
> 
> Would that be OK?

Sounds better to me. This is very likely to be safe, but it's
always better to separate trivial code movements from other
changes that could have unexpected side-effects or trigger hidden
bugs.

-- 
Eduardo

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

end of thread, other threads:[~2017-01-11 17:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-15 12:36 [Qemu-devel] [PATCH v2 0/2] Clean-up tlb_flush and cpu reset functions Alex Bennée
2016-12-15 12:36 ` [Qemu-devel] [PATCH v2 1/2] qom/cpu: move tlb_flush to cpu_common_reset Alex Bennée
2016-12-16  4:21   ` David Gibson
2016-12-18 20:46   ` Eduardo Habkost
2017-01-09 15:05     ` Alex Bennée
2017-01-11 17:00       ` Eduardo Habkost
2016-12-15 12:36 ` [Qemu-devel] [PATCH v2 2/2] cputlb: drop flush_global flag from tlb_flush Alex Bennée
2016-12-16  4:21   ` David Gibson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).