Re: [PATCH 1/2] accel/tcg: Introduce memset_ra, memmove_ra

[Peter Maydell]

On Wed, 3 Jul 2024 at 00:43, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Add wrappers that set and clear helper_retaddr around the
> host memory operation.  This cannot fail for system mode,
> but might raise SIGSEGV for user mode.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/cpu_ldst.h | 40 ++++++++++++++++++++++++++++++++++++++++
>  accel/tcg/user-exec.c   | 22 ++++++++++++++++++++++
>  2 files changed, 62 insertions(+)
>
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index 71009f84f5..baf4f9367d 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -379,4 +379,44 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
>                          MMUAccessType access_type, int mmu_idx);
>  #endif
>
> +/**
> + * memset_ra:
> + * @p: host pointer
> + * @c: data
> + * @n: length
> + * @ra: unwind return address
> + *
> + * Like system memset(p,c,n), except manages @ra for the calling
> + * helper in the event of a signal.  To be used with the result
> + * of tlb_vaddr_to_host to resolve the host pointer.
> + */
> +#ifdef CONFIG_USER_ONLY
> +void *memset_ra(void *p, int c, size_t n, uintptr_t ra);
> +#else
> +static inline void *memset_ra(void *p, int c, size_t n, uintptr_t ra)
> +{
> +    return memset(p, c, n);
> +}
> +#endif
> +
> +/**
> + * memmove_ra:
> + * @d: host destination pointer
> + * @s: host source pointer
> + * @n: length
> + * @ra: unwind return address
> + *
> + * Like system memmove(d,s,n), except manages @ra for the calling
> + * helper in the event of a signal.  To be used with the result of
> + * tlb_vaddr_to_host to resolve the host pointer.
> + */
> +#ifdef CONFIG_USER_ONLY
> +void *memmove_ra(void *d, const void *s, size_t n, uintptr_t ra);
> +#else
> +static inline void *memmove_ra(void *d, const void *s, size_t n, uintptr_t 
> ra)
> +{
> +    return memmove(d, s, n);
> +}
> +#endif

I guess these make sense. I feel like they're a function where
the caller needs to be quite careful about what they're doing
(e.g. not to use them in a way that the memmove or memset
would cross a page boundary if other guest register state needs
to be kept in sync with the reported fault address), but I
can't think of a useful non-architecture-specific warning that
would be worth putting in the doc comments.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM