Re: [PATCH 4/4] tcg: Map code_gen_buffer with PROT_BTI

[Philippe Mathieu-Daudé]

(Cc'ing Joelle)

On 16/8/23 16:25, Richard Henderson wrote:
> For linux aarch64 host supporting BTI, map the buffer
> to require BTI instructions at branch landing pads.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tcg/region.c | 39 ++++++++++++++++++++++++++++-----------
>   1 file changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/tcg/region.c b/tcg/region.c
> index 2b28ed3556..58cb68c6c8 100644
> --- a/tcg/region.c
> +++ b/tcg/region.c
> @@ -33,8 +33,19 @@
>   #include "tcg/tcg.h"
>   #include "exec/translation-block.h"
>   #include "tcg-internal.h"
> +#include "host/cpuinfo.h"
>   
>   
> +/*
> + * Local source-level compatibility with Unix.
> + * Used by tcg_region_init below.
> + */
> +#if defined(_WIN32)
> +#define PROT_READ   1
> +#define PROT_WRITE  2
> +#define PROT_EXEC   4
> +#endif
> +
>   struct tcg_region_tree {
>       QemuMutex lock;
>       QTree *tree;
> @@ -83,6 +94,16 @@ bool in_code_gen_buffer(const void *p)
>       return (size_t)(p - region.start_aligned) <= region.total_size;
>   }
>   
> +static int host_prot_read_exec(void)
> +{
> +#if defined(CONFIG_LINUX) && defined(HOST_AARCH64) && defined(PROT_BTI)
> +    if (cpuinfo & CPUINFO_BTI) {
> +        return PROT_READ | PROT_EXEC | PROT_BTI;
> +    }
> +#endif
> +    return PROT_READ | PROT_EXEC;
> +}
> +
>   #ifdef CONFIG_DEBUG_TCG
>   const void *tcg_splitwx_to_rx(void *rw)
>   {
> @@ -505,14 +526,6 @@ static int alloc_code_gen_buffer(size_t tb_size, int 
> splitwx, Error **errp)
>       return PROT_READ | PROT_WRITE;
>   }
>   #elif defined(_WIN32)
> -/*
> - * Local source-level compatibility with Unix.
> - * Used by tcg_region_init below.
> - */
> -#define PROT_READ   1
> -#define PROT_WRITE  2
> -#define PROT_EXEC   4
> -
>   static int alloc_code_gen_buffer(size_t size, int splitwx, Error **errp)
>   {
>       void *buf;
> @@ -567,7 +580,7 @@ static int alloc_code_gen_buffer_splitwx_memfd(size_t size, 
> Error **errp)
>           goto fail;
>       }
>   
> -    buf_rx = mmap(NULL, size, PROT_READ | PROT_EXEC, MAP_SHARED, fd, 0);
> +    buf_rx = mmap(NULL, size, host_prot_read_exec(), MAP_SHARED, fd, 0);
>       if (buf_rx == MAP_FAILED) {
>           goto fail_rx;
>       }
> @@ -642,7 +655,7 @@ static int alloc_code_gen_buffer_splitwx_vmremap(size_t 
> size, Error **errp)
>           return -1;
>       }
>   
> -    if (mprotect((void *)buf_rx, size, PROT_READ | PROT_EXEC) != 0) {
> +    if (mprotect((void *)buf_rx, size, host_prot_read_exec()) != 0) {
>           error_setg_errno(errp, errno, "mprotect for jit splitwx");
>           munmap((void *)buf_rx, size);
>           munmap((void *)buf_rw, size);
> @@ -805,7 +818,7 @@ void tcg_region_init(size_t tb_size, int splitwx, unsigned 
> max_cpus)
>       need_prot = PROT_READ | PROT_WRITE;
>   #ifndef CONFIG_TCG_INTERPRETER
>       if (tcg_splitwx_diff == 0) {
> -        need_prot |= PROT_EXEC;
> +        need_prot |= host_prot_read_exec();
>       }
>   #endif
>       for (size_t i = 0, n = region.n; i < n; i++) {
> @@ -820,7 +833,11 @@ void tcg_region_init(size_t tb_size, int splitwx, unsigned 
> max_cpus)
>               } else if (need_prot == (PROT_READ | PROT_WRITE)) {
>                   rc = qemu_mprotect_rw(start, end - start);
>               } else {
> +#ifdef CONFIG_POSIX
> +                rc = mprotect(start, end - start, need_prot);

Hmm this bypass the qemu_real_host_page_mask() checks in
qemu_mprotect__osdep(), but I guess this is acceptable.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +#else
>                   g_assert_not_reached();
> +#endif
>               }
>               if (rc) {
>                   error_setg_errno(&error_fatal, errno,