1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v2 0/7] Poison some more target-specific defines
  4. View patch

[Qemu-devel] [PATCH 5/7] cpu: Introduce a wrapper for tlb_flush() that can be used in common code

Thomas Huth posted 7 patches 8 years, 4 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 5/7] cpu: Introduce a wrapper for tlb_flush() that can be used in common code
Posted by Thomas Huth 8 years, 4 months ago 
Commit 1f5c00cfdb8114c ("qom/cpu: move tlb_flush to cpu_common_reset")
moved the call to tlb_flush() from the target-specific reset handlers
into the common code qom/cpu.c file, and protected the call with
"#ifdef CONFIG_SOFTMMU" to avoid that it is called for linux-user
only targets. But since qom/cpu.c is common code, CONFIG_SOFTMMU is
*never* defined here, so the tlb_flush() was simply never executed
anymore. Fix it by introducing a wrapper for tlb_flush() in a file
that is re-compiled for each target, i.e. in translate-all.c.

Fixes: 1f5c00cfdb8114c1e3a13426588ceb64f82c9ddb
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/exec/cpu-common.h | 2 ++
 qom/cpu.c                 | 5 ++---
 translate-all.c           | 8 ++++++++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 4d45a72..74341b1 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -28,6 +28,8 @@ void qemu_init_cpu_list(void);
 void cpu_list_lock(void);
 void cpu_list_unlock(void);
 
+void tcg_flush_softmmu_tlb(CPUState *cs);
+
 #if !defined(CONFIG_USER_ONLY)
 
 enum device_endian {
diff --git a/qom/cpu.c b/qom/cpu.c
index 5069876..303eb42 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -26,6 +26,7 @@
 #include "qemu/notify.h"
 #include "qemu/log.h"
 #include "exec/log.h"
+#include "exec/cpu-common.h"
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "hw/qdev-properties.h"
@@ -296,9 +297,7 @@ static void cpu_common_reset(CPUState *cpu)
             atomic_set(&cpu->tb_jmp_cache[i], NULL);
         }
 
-#ifdef CONFIG_SOFTMMU
-        tlb_flush(cpu, 0);
-#endif
+        tcg_flush_softmmu_tlb(cpu);
     }
 }
 
diff --git a/translate-all.c b/translate-all.c
index b3ee876..a45480f 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -2219,3 +2219,11 @@ int page_unprotect(target_ulong address, uintptr_t pc)
     return 0;
 }
 #endif /* CONFIG_USER_ONLY */
+
+/* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
+void tcg_flush_softmmu_tlb(CPUState *cs)
+{
+#ifdef CONFIG_SOFTMMU
+    tlb_flush(cs);
+#endif
+}
-- 
1.8.3.1
Re: [Qemu-devel] [PATCH 5/7] cpu: Introduce a wrapper for tlb_flush() that can be used in common code
Posted by Alex Bennée 8 years, 4 months ago 
Thomas Huth <thuth@redhat.com> writes:

> Commit 1f5c00cfdb8114c ("qom/cpu: move tlb_flush to cpu_common_reset")
> moved the call to tlb_flush() from the target-specific reset handlers
> into the common code qom/cpu.c file, and protected the call with
> "#ifdef CONFIG_SOFTMMU" to avoid that it is called for linux-user
> only targets. But since qom/cpu.c is common code, CONFIG_SOFTMMU is
> *never* defined here, so the tlb_flush() was simply never executed
> anymore. Fix it by introducing a wrapper for tlb_flush() in a file
> that is re-compiled for each target, i.e. in translate-all.c.
>
> Fixes: 1f5c00cfdb8114c1e3a13426588ceb64f82c9ddb
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  include/exec/cpu-common.h | 2 ++
>  qom/cpu.c                 | 5 ++---
>  translate-all.c           | 8 ++++++++
>  3 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 4d45a72..74341b1 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -28,6 +28,8 @@ void qemu_init_cpu_list(void);
>  void cpu_list_lock(void);
>  void cpu_list_unlock(void);
>
> +void tcg_flush_softmmu_tlb(CPUState *cs);
> +
>  #if !defined(CONFIG_USER_ONLY)
>
>  enum device_endian {
> diff --git a/qom/cpu.c b/qom/cpu.c
> index 5069876..303eb42 100644
> --- a/qom/cpu.c
> +++ b/qom/cpu.c
> @@ -26,6 +26,7 @@
>  #include "qemu/notify.h"
>  #include "qemu/log.h"
>  #include "exec/log.h"
> +#include "exec/cpu-common.h"
>  #include "qemu/error-report.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/qdev-properties.h"
> @@ -296,9 +297,7 @@ static void cpu_common_reset(CPUState *cpu)
>              atomic_set(&cpu->tb_jmp_cache[i], NULL);
>          }
>
> -#ifdef CONFIG_SOFTMMU
> -        tlb_flush(cpu, 0);
> -#endif
> +        tcg_flush_softmmu_tlb(cpu);
>      }
>  }
>
> diff --git a/translate-all.c b/translate-all.c
> index b3ee876..a45480f 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -2219,3 +2219,11 @@ int page_unprotect(target_ulong address, uintptr_t pc)
>      return 0;
>  }
>  #endif /* CONFIG_USER_ONLY */
> +
> +/* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
> +void tcg_flush_softmmu_tlb(CPUState *cs)
> +{
> +#ifdef CONFIG_SOFTMMU
> +    tlb_flush(cs);
> +#endif
> +}

Don't you usually have a empty inline for the stub in the headers so the
non-SoftMMU build can optimize away rather than link to an empty function?

--
Alex Bennée
Re: [Qemu-devel] [PATCH 5/7] cpu: Introduce a wrapper for tlb_flush() that can be used in common code
Posted by Paolo Bonzini 8 years, 4 months ago 

On 16/06/2017 19:07, Alex Bennée wrote:
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
>> Commit 1f5c00cfdb8114c ("qom/cpu: move tlb_flush to cpu_common_reset")
>> moved the call to tlb_flush() from the target-specific reset handlers
>> into the common code qom/cpu.c file, and protected the call with
>> "#ifdef CONFIG_SOFTMMU" to avoid that it is called for linux-user
>> only targets. But since qom/cpu.c is common code, CONFIG_SOFTMMU is
>> *never* defined here, so the tlb_flush() was simply never executed
>> anymore. Fix it by introducing a wrapper for tlb_flush() in a file
>> that is re-compiled for each target, i.e. in translate-all.c.
>>
>> Fixes: 1f5c00cfdb8114c1e3a13426588ceb64f82c9ddb
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  include/exec/cpu-common.h | 2 ++
>>  qom/cpu.c                 | 5 ++---
>>  translate-all.c           | 8 ++++++++
>>  3 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
>> index 4d45a72..74341b1 100644
>> --- a/include/exec/cpu-common.h
>> +++ b/include/exec/cpu-common.h
>> @@ -28,6 +28,8 @@ void qemu_init_cpu_list(void);
>>  void cpu_list_lock(void);
>>  void cpu_list_unlock(void);
>>
>> +void tcg_flush_softmmu_tlb(CPUState *cs);
>> +
>>  #if !defined(CONFIG_USER_ONLY)
>>
>>  enum device_endian {
>> diff --git a/qom/cpu.c b/qom/cpu.c
>> index 5069876..303eb42 100644
>> --- a/qom/cpu.c
>> +++ b/qom/cpu.c
>> @@ -26,6 +26,7 @@
>>  #include "qemu/notify.h"
>>  #include "qemu/log.h"
>>  #include "exec/log.h"
>> +#include "exec/cpu-common.h"
>>  #include "qemu/error-report.h"
>>  #include "sysemu/sysemu.h"
>>  #include "hw/qdev-properties.h"
>> @@ -296,9 +297,7 @@ static void cpu_common_reset(CPUState *cpu)
>>              atomic_set(&cpu->tb_jmp_cache[i], NULL);
>>          }
>>
>> -#ifdef CONFIG_SOFTMMU
>> -        tlb_flush(cpu, 0);
>> -#endif
>> +        tcg_flush_softmmu_tlb(cpu);
>>      }
>>  }
>>
>> diff --git a/translate-all.c b/translate-all.c
>> index b3ee876..a45480f 100644
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -2219,3 +2219,11 @@ int page_unprotect(target_ulong address, uintptr_t pc)
>>      return 0;
>>  }
>>  #endif /* CONFIG_USER_ONLY */
>> +
>> +/* This is a wrapper for common code that can not use CONFIG_SOFTMMU */
>> +void tcg_flush_softmmu_tlb(CPUState *cs)
>> +{
>> +#ifdef CONFIG_SOFTMMU
>> +    tlb_flush(cs);
>> +#endif
>> +}
> 
> Don't you usually have a empty inline for the stub in the headers so the
> non-SoftMMU build can optimize away rather than link to an empty function?

Optimizing reset doesn't seem worthwhile. :)

Paolo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.