Re: [PATCH 1/2] accel/tcg: Introduce memset_ra, memmove_ra
On Wed, 3 Jul 2024 at 00:43, Richard Henderson
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
> ---
> 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
thanks
-- PMM
[PATCH 1/2] accel/tcg: Introduce memset_ra, memmove_ra
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
---
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
