Re: [PATCH v6 13/21] accel/tcg: Add nofault parameter to get_page_addr_code_hostp

[Alistair Francis]

On Fri, Aug 19, 2022 at 1:36 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/exec/exec-all.h | 10 +++++-----
>  accel/tcg/cputlb.c      |  8 ++++----
>  accel/tcg/plugin-gen.c  |  4 ++--
>  accel/tcg/user-exec.c   |  4 ++--
>  4 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 9f35e3b7a9..7a6dc44d86 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -599,6 +599,8 @@ struct MemoryRegionSection *iotlb_to_section(CPUState 
> *cpu,
>   * get_page_addr_code_hostp()
>   * @env: CPUArchState
>   * @addr: guest virtual address of guest code
> + * @nofault: do not raise an exception
> + * @hostp: output for host pointer
>   *
>   * See get_page_addr_code() (full-system version) for documentation on the
>   * return value.
> @@ -607,10 +609,10 @@ struct MemoryRegionSection *iotlb_to_section(CPUState 
> *cpu,
>   * If the return value is -1, sets *@hostp to NULL. Otherwise, sets *@hostp
>   * to the host address where @addr's content is kept.
>   *
> - * Note: this function can trigger an exception.
> + * Note: Unless @nofault, this function can trigger an exception.
>   */
>  tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> -                                        void **hostp);
> +                                        bool nofault, void **hostp);
>
>  /**
>   * get_page_addr_code()
> @@ -620,13 +622,11 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState 
> *env, target_ulong addr,
>   * If we cannot translate and execute from the entire RAM page, or if
>   * the region is not backed by RAM, returns -1. Otherwise, returns the
>   * ram_addr_t corresponding to the guest code at @addr.
> - *
> - * Note: this function can trigger an exception.
>   */
>  static inline tb_page_addr_t get_page_addr_code(CPUArchState *env,
>                                                  target_ulong addr)
>  {
> -    return get_page_addr_code_hostp(env, addr, NULL);
> +    return get_page_addr_code_hostp(env, addr, true, NULL);
>  }
>
>  #if defined(CONFIG_USER_ONLY)
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 2dc2affa12..ae7b40dd51 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1644,16 +1644,16 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr 
> addr,
>   * of RAM.  This will force us to execute by loading and translating
>   * one insn at a time, without caching.
>   *
> - * NOTE: This function will trigger an exception if the page is
> - * not executable.
> + * NOTE: Unless @nofault, this function will trigger an exception
> + * if the page is not executable.
>   */
>  tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> -                                        void **hostp)
> +                                        bool nofault, void **hostp)
>  {
>      void *p;
>
>      (void)probe_access_internal(env, addr, 1, MMU_INST_FETCH,
> -                                cpu_mmu_index(env, true), true, &p, 0);
> +                                cpu_mmu_index(env, true), nofault, &p, 0);
>      if (p == NULL) {
>          return -1;
>      }
> diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
> index 3d0b101e34..8377c15383 100644
> --- a/accel/tcg/plugin-gen.c
> +++ b/accel/tcg/plugin-gen.c
> @@ -872,7 +872,7 @@ bool plugin_gen_tb_start(CPUState *cpu, const 
> TranslationBlock *tb, bool mem_onl
>
>          ptb->vaddr = tb->pc;
>          ptb->vaddr2 = -1;
> -        get_page_addr_code_hostp(cpu->env_ptr, tb->pc, &ptb->haddr1);
> +        get_page_addr_code_hostp(cpu->env_ptr, tb->pc, true, &ptb->haddr1);
>          ptb->haddr2 = NULL;
>          ptb->mem_only = mem_only;
>
> @@ -902,7 +902,7 @@ void plugin_gen_insn_start(CPUState *cpu, const 
> DisasContextBase *db)
>          unlikely((db->pc_next & TARGET_PAGE_MASK) !=
>                   (db->pc_first & TARGET_PAGE_MASK))) {
>          get_page_addr_code_hostp(cpu->env_ptr, db->pc_next,
> -                                 &ptb->haddr2);
> +                                 true, &ptb->haddr2);
>          ptb->vaddr2 = db->pc_next;
>      }
>      if (likely(ptb->vaddr2 == -1)) {
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 58edd33896..e7fec960c2 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -197,11 +197,11 @@ void *probe_access(CPUArchState *env, target_ulong 
> addr, int size,
>  }
>
>  tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, target_ulong addr,
> -                                        void **hostp)
> +                                        bool nofault, void **hostp)
>  {
>      int flags;
>
> -    flags = probe_access_internal(env, addr, 1, MMU_INST_FETCH, true, 0);
> +    flags = probe_access_internal(env, addr, 1, MMU_INST_FETCH, nofault, 0);
>      if (unlikely(flags)) {
>          return -1;
>      }
> --
> 2.34.1
>
>