Re: [PATCH 04/18] accel/tcg: Declare mmap_[un]lock() in 'exec/page-protection.h'

[Pierrick Bouvier]

On 12/12/24 10:53, Philippe Mathieu-Daudé wrote:
> Move mmap_lock(), mmap_unlock() declarations and the
> WITH_MMAP_LOCK_GUARD() definition to 'exec/page-protection.h'.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/exec-all.h        | 17 -----------------
>   include/exec/page-protection.h | 24 ++++++++++++++++++++++++
>   accel/tcg/cpu-exec.c           |  1 +
>   accel/tcg/translate-all.c      |  1 +
>   accel/tcg/watchpoint.c         |  1 +
>   linux-user/flatload.c          |  1 +
>   target/arm/helper.c            |  1 +
>   7 files changed, 29 insertions(+), 17 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 2e4c4cc4b46..b5ea607cf45 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -27,7 +27,6 @@
>   #endif
>   #include "exec/mmu-access-type.h"
>   #include "exec/translation-block.h"
> -#include "qemu/clang-tsa.h"
>   
>   /**
>    * cpu_loop_exit_requested:
> @@ -520,18 +519,6 @@ static inline tb_page_addr_t 
> get_page_addr_code(CPUArchState *env,
>   }
>   
>   #if defined(CONFIG_USER_ONLY)
> -void TSA_NO_TSA mmap_lock(void);
> -void TSA_NO_TSA mmap_unlock(void);
> -bool have_mmap_lock(void);
> -
> -static inline void mmap_unlock_guard(void *unused)
> -{
> -    mmap_unlock();
> -}
> -
> -#define WITH_MMAP_LOCK_GUARD()                                            \
> -    for (int _mmap_lock_iter __attribute__((cleanup(mmap_unlock_guard)))  \
> -         = (mmap_lock(), 0); _mmap_lock_iter == 0; _mmap_lock_iter = 1)
>   
>   /**
>    * adjust_signal_pc:
> @@ -585,10 +572,6 @@ G_NORETURN void cpu_loop_exit_sigbus(CPUState *cpu, 
> target_ulong addr,
>                                        uintptr_t ra);
>   
>   #else
> -static inline void mmap_lock(void) {}
> -static inline void mmap_unlock(void) {}
> -#define WITH_MMAP_LOCK_GUARD()
> -
>   void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
>   void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length);
>   
> diff --git a/include/exec/page-protection.h b/include/exec/page-protection.h
> index c43231af8b5..bae3355f62c 100644
> --- a/include/exec/page-protection.h
> +++ b/include/exec/page-protection.h
> @@ -38,4 +38,28 @@
>    */
>   #define PAGE_PASSTHROUGH 0x0800
>   
> +#ifdef CONFIG_USER_ONLY
> +
> +#include "qemu/clang-tsa.h"
> +
> +void TSA_NO_TSA mmap_lock(void);
> +void TSA_NO_TSA mmap_unlock(void);
> +bool have_mmap_lock(void);
> +
> +static inline void mmap_unlock_guard(void *unused)
> +{
> +    mmap_unlock();
> +}
> +
> +#define WITH_MMAP_LOCK_GUARD() \
> +    for (int _mmap_lock_iter __attribute__((cleanup(mmap_unlock_guard))) \
> +         = (mmap_lock(), 0); _mmap_lock_iter == 0; _mmap_lock_iter = 1)
> +#else
> +
> +static inline void mmap_lock(void) {}
> +static inline void mmap_unlock(void) {}
> +#define WITH_MMAP_LOCK_GUARD()
> +
> +#endif /* !CONFIG_USER_ONLY */
> +
>   #endif
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index c13f4a7cbbf..f82870a1e2a 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -25,6 +25,7 @@
>   #include "trace.h"
>   #include "disas/disas.h"
>   #include "exec/exec-all.h"
> +#include "exec/page-protection.h"
>   #include "tcg/tcg.h"
>   #include "qemu/atomic.h"
>   #include "qemu/rcu.h"
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index a8b24be0b96..d586ac9bb1d 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -44,6 +44,7 @@
>   #endif
>   
>   #include "exec/cputlb.h"
> +#include "exec/page-protection.h"
>   #include "exec/translate-all.h"
>   #include "exec/translator.h"
>   #include "exec/tb-flush.h"
> diff --git a/accel/tcg/watchpoint.c b/accel/tcg/watchpoint.c
> index ca641eb95cf..8923301b8b8 100644
> --- a/accel/tcg/watchpoint.c
> +++ b/accel/tcg/watchpoint.c
> @@ -22,6 +22,7 @@
>   #include "qemu/error-report.h"
>   #include "exec/exec-all.h"
>   #include "exec/translate-all.h"
> +#include "exec/page-protection.h"
>   #include "system/tcg.h"
>   #include "system/replay.h"
>   #include "hw/core/tcg-cpu-ops.h"
> diff --git a/linux-user/flatload.c b/linux-user/flatload.c
> index 0e4be5bf44a..d5cb1830ddf 100644
> --- a/linux-user/flatload.c
> +++ b/linux-user/flatload.c
> @@ -34,6 +34,7 @@
>   #include "qemu/osdep.h"
>   
>   #include "qemu.h"
> +#include "exec/page-protection.h"
>   #include "user-internals.h"
>   #include "loader.h"
>   #include "user-mmap.h"
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 63fe4ba318b..c35cb0f6a54 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -13,6 +13,7 @@
>   #include "internals.h"
>   #include "cpu-features.h"
>   #include "exec/helper-proto.h"
> +#include "exec/page-protection.h"
>   #include "qemu/main-loop.h"
>   #include "qemu/timer.h"
>   #include "qemu/bitops.h"

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>