Re: [PATCH 14/20] accel/tcg: Move cpu_memory_rw_debug() user implementation to user-exec.c

Richard Henderson
2025-01-26T21:13:51Z

On 1/23/25 15:44, Philippe Mathieu-Daudé wrote:
> cpu_memory_rw_debug() system implementation is defined in
> system/physmem.c. Move the user one to accel/tcg/user-exec.c
> to simplify cpu-target.c maintenance.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> accel/tcg/user-exec.c | 92 +++++++++++++++++++++++++++++++++++++
> cpu-target.c | 102 +-----------------------------------------
> 2 files changed, 94 insertions(+), 100 deletions(-)
>
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index c4454100ad7..e7e99a46087 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -19,6 +19,8 @@
> #include "qemu/osdep.h"
> #include "accel/tcg/cpu-ops.h"
> #include "disas/disas.h"
> +#include "exec/vaddr.h"
> +#include "exec/tswap.h"
> #include "exec/exec-all.h"
> #include "tcg/tcg.h"
> #include "qemu/bitops.h"
> @@ -35,6 +37,7 @@
> #include "internal-common.h"
> #include "internal-target.h"
> #include "tb-internal.h"
> +#include "qemu.h"

What is required from *-user/qemu.h?
We really should not be including that in accel/tcg/.

> + if (flags & PAGE_WRITE) {
> + /* XXX: this code should not depend on lock_user */
> + p = lock_user(VERIFY_WRITE, addr, l, 0);

Ah, here it is, complete with comment.

Indeed, I don't think lock_user is required at all. page_get_flags() and g2h() are
sufficient.

> + mmap_lock();
> + tb_invalidate_phys_range(addr, addr + l - 1);
> + written = pwrite(fd, buf, l,
> + (off_t)(uintptr_t)g2h_untagged(addr));
> + mmap_unlock();

We probably want to own mmap_lock for the entire function.

r~