Re: [Qemu-devel] [PATCH v3 3/3] cputlb: drop flush_global flag from tlb_flush

2017-01-13 Thread Alex Bennée 

Richard Henderson  writes:

> On 01/12/2017 08:03 AM, Alex Bennée wrote:
>> And I immediately realise I missed out:
>>
>>> Signed-off-by: Alex Bennée 
>>> Reviewed-by: Richard Henderson 
>> [DG: ppc portions]
>> Acked-by: David Gibson 
>>
>> Do you want me to re-post or can you apply when you take the patches?
>>
>
> I also don't mind if you send the pull request.

If Peter is OK with that?

In that case I now have a few more tags to add and I can roll a PULL REQ.

--
Alex Bennée

Re: [Qemu-devel] [PATCH v3 3/3] cputlb: drop flush_global flag from tlb_flush

2017-01-12 Thread David Gibson 
On Thu, Jan 12, 2017 at 03:47:31PM +, 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 
> Reviewed-by: Richard Henderson 

For ppc:

Acked-by: David Gibson 

> ---
>  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 47835c1dc1..401a9127c2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -544,7 +544,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;
>  }
> @@ -2426,7 +2426,7 @@ static void tcg_commit(MemoryListener *listener)
>   */
>  d = atomic_rcu_read(>as->dispatch);
>  atomic_rcu_set(>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
> ---

Re: [Qemu-devel] [PATCH v3 3/3] cputlb: drop flush_global flag from tlb_flush

2017-01-12 Thread Richard Henderson 
On 01/12/2017 08:03 AM, Alex Bennée wrote:
> And I immediately realise I missed out:
> 
>> Signed-off-by: Alex Bennée 
>> Reviewed-by: Richard Henderson 
> [DG: ppc portions]
> Acked-by: David Gibson 
> 
> Do you want me to re-post or can you apply when you take the patches?
> 

I also don't mind if you send the pull request.


r~

Re: [Qemu-devel] [PATCH v3 3/3] cputlb: drop flush_global flag from tlb_flush

2017-01-12 Thread Alex Bennée 

Alex Bennée  writes:

> 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.
>

And I immediately realise I missed out:

> Signed-off-by: Alex Bennée 
> Reviewed-by: Richard Henderson 
[DG: ppc portions]
Acked-by: David Gibson 

Do you want me to re-post or can you apply when you take the patches?

> ---
>  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 47835c1dc1..401a9127c2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -544,7 +544,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;
>  }
> @@ -2426,7 +2426,7 @@ static void tcg_commit(MemoryListener *listener)
>   */
>  d = atomic_rcu_read(>as->dispatch);
>  atomic_rcu_set(>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