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

Richard Henderson posted 2 patches 5 months, 4 weeks ago
[PATCH 1/2] accel/tcg: Introduce memset_ra, memmove_ra
Posted by Richard Henderson 5 months, 4 weeks ago
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
+
 #endif /* CPU_LDST_H */
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 80d24540ed..a73da42e3a 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -1289,3 +1289,25 @@ static void *atomic_mmu_lookup(CPUState *cpu, vaddr addr, MemOpIdx oi,
 #define DATA_SIZE 16
 #include "atomic_template.h"
 #endif
+
+void *memset_ra(void *p, int c, size_t n, uintptr_t ra)
+{
+    void *r;
+
+    set_helper_retaddr(ra);
+    r = memset(p, c, n);
+    clear_helper_retaddr();
+
+    return r;
+}
+
+void *memmove_ra(void *d, const void *s, size_t n, uintptr_t ra)
+{
+    void *r;
+
+    set_helper_retaddr(ra);
+    r = memmove(d, s, n);
+    clear_helper_retaddr();
+
+    return r;
+}
-- 
2.34.1
Re: [PATCH 1/2] accel/tcg: Introduce memset_ra, memmove_ra
Posted by Peter Maydell 5 months, 3 weeks ago
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