[PATCH 00/24] include/exec: Rework (part 2)

[PATCH 00/24] include/exec: Rework (part 2)

[Philippe Mathieu-Daudé]

Reordering exec/, second round. Mostly splitting:
- TCG LDST API (to "exec/cpu_ldst.h")
- CPU TLB APIs (to "exec/cputlb.h")
from "exec/cpu-all.h" and "exec/exec-all.h", and
reducing inclusions of "hw/core/cpu.h".

Based-on: <20240417182806.69446-1-philmd@linaro.org>
or https://gitlab.com/philmd/qemu/-/commits/exec-next

Philippe Mathieu-Daudé (24):
  exec: Declare MMUAccessType type in 'mmu-access-type.h' header
  exec: Declare CPUBreakpoint/CPUWatchpoint type in 'breakpoint.h'
    header
  hw/core: Avoid including the full 'hw/core/cpu.h' in 'tcg-cpu-ops.h'
  exec: Restrict TCG specific declarations of 'cputlb.h'
  exec: Restrict 'cpu_ldst.h' to TCG accelerator
  exec: Have guest_addr_valid() methods take abi_ptr/size_t arguments
  exec: Un-inline tlb_vaddr_to_host() and declare it in 'exec/cputlb.h'
  physmem: Move TCG CPU IOTLB methods around
  physmem: Restrict TCG CPU IOTLB code to TCG accel
  exec: Reduce tlb_set_dirty() declaration scope
  exec: Move tlb_reset_dirty*() declarations to 'exec/cputlb.h'
  exec: Move CPUTLBEntry helpers to cputlb.c
  target/sparc: Replace abi_ulong by uint32_t for TARGET_ABI32
  exec: Rename 'exec/user/guest-base.h' as 'user/guest-base.h'
  exec: Restrict inclusion of 'user/guest-base.h'
  exec: Move GUEST_ADDR_MAX definition to 'cpu_ldst.h'
  exec: Include missing 'qemu/log-for-trace.h' header in 'exec/log.h'
  plugins: Include missing 'qemu/bitmap.h' header
  gdbstub: Avoid including 'cpu.h' in 'gdbstub/helpers.h'
  hw/xtensa: Include missing 'exec/cpu-common.h' in 'bootparam.h'
  plugins: Un-inline qemu_plugin_disable_mem_helpers()
  exec: Remove 'exec/tswap.h' from 'exec/cpu-all.h'
  exec: Remove 'disas/disas.h' from 'exec/log.h'
  exec: Remove unnecessary inclusions of 'hw/core/cpu.h'

 hw/xtensa/bootparam.h            |   3 +
 include/exec/breakpoint.h        |  23 ++++++
 include/exec/cpu-all.h           |  39 +---------
 include/exec/cpu-defs.h          |   1 -
 include/exec/cpu_ldst.h          | 122 ++++++++++---------------------
 include/exec/cputlb.h            |  58 ++++++++++++++-
 include/exec/exec-all.h          |  26 +------
 include/exec/log.h               |   2 +-
 include/exec/mmu-access-type.h   |  18 +++++
 include/exec/ram_addr.h          |   1 +
 include/exec/user/guest-base.h   |  12 ---
 include/gdbstub/helpers.h        |   3 +-
 include/hw/boards.h              |   1 -
 include/hw/core/cpu.h            |  24 +-----
 include/hw/core/tcg-cpu-ops.h    |   6 +-
 include/hw/ppc/openpic.h         |   1 -
 include/qemu/plugin.h            |   7 +-
 include/sysemu/hw_accel.h        |   1 -
 include/user/guest-base.h        |  18 +++++
 target/arm/internals.h           |   1 +
 target/ppc/internal.h            |   1 +
 target/riscv/debug.h             |   2 +
 accel/stubs/tcg-stub.c           |   4 -
 accel/tcg/cpu-exec.c             |   1 -
 accel/tcg/cputlb.c               |  53 +++++++++++++-
 accel/tcg/translator.c           |   1 +
 accel/tcg/user-exec.c            |   7 ++
 bsd-user/main.c                  |   1 +
 hw/arm/boot.c                    |   1 +
 hw/arm/npcm7xx.c                 |   1 +
 hw/mips/fuloong2e.c              |   1 +
 hw/mips/malta.c                  |   1 +
 hw/ppc/sam460ex.c                |   1 +
 hw/ppc/spapr.c                   |   1 +
 hw/ppc/virtex_ml507.c            |   1 +
 hw/sh4/r2d.c                     |   1 +
 linux-user/elfload.c             |   1 +
 linux-user/main.c                |   1 +
 plugins/core.c                   |   5 ++
 system/physmem.c                 |  58 ++++++++-------
 target/arm/gdbstub.c             |   1 +
 target/arm/tcg/helper-a64.c      |   1 +
 target/avr/gdbstub.c             |   1 +
 target/avr/translate.c           |   1 +
 target/hexagon/translate.c       |   1 +
 target/loongarch/tcg/translate.c |   1 +
 target/riscv/vector_helper.c     |   1 +
 target/rx/translate.c            |   1 +
 target/sparc/gdbstub.c           |   2 +-
 target/sparc/mmu_helper.c        |   1 +
 target/tricore/gdbstub.c         |   1 +
 target/xtensa/xtensa-semi.c      |   1 +
 tcg/tcg.c                        |   3 +-
 53 files changed, 300 insertions(+), 226 deletions(-)
 create mode 100644 include/exec/breakpoint.h
 create mode 100644 include/exec/mmu-access-type.h
 delete mode 100644 include/exec/user/guest-base.h
 create mode 100644 include/user/guest-base.h

-- 
2.41.0

[PATCH 01/24] exec: Declare MMUAccessType type in 'mmu-access-type.h' header

[Philippe Mathieu-Daudé]

The MMUAccessType enum is declared in "hw/core/cpu.h".
"hw/core/cpu.h" contains declarations related to CPUState
and CPUClass. Some source files only require MMUAccessType
and don't need to pull in all CPU* declarations. In order
to simplify, create a new "exec/mmu-access-type.h" header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu_ldst.h        |  1 +
 include/exec/exec-all.h        |  1 +
 include/exec/mmu-access-type.h | 18 ++++++++++++++++++
 include/hw/core/cpu.h          |  8 +-------
 4 files changed, 21 insertions(+), 7 deletions(-)
 create mode 100644 include/exec/mmu-access-type.h

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 64e0319996..5b99666702 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -64,6 +64,7 @@
 
 #include "exec/memopidx.h"
 #include "exec/abi_ptr.h"
+#include "exec/mmu-access-type.h"
 #include "qemu/int128.h"
 #include "cpu.h"
 
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index e932d364b6..5cddfea01b 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -25,6 +25,7 @@
 #include "exec/abi_ptr.h"
 #include "exec/cpu_ldst.h"
 #endif
+#include "exec/mmu-access-type.h"
 #include "exec/translation-block.h"
 #include "qemu/clang-tsa.h"
 
diff --git a/include/exec/mmu-access-type.h b/include/exec/mmu-access-type.h
new file mode 100644
index 0000000000..28bbb05b94
--- /dev/null
+++ b/include/exec/mmu-access-type.h
@@ -0,0 +1,18 @@
+/*
+ * QEMU MMU Access type definitions
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef EXEC_MMU_ACCESS_TYPE_H
+#define EXEC_MMU_ACCESS_TYPE_H
+
+typedef enum MMUAccessType {
+    MMU_DATA_LOAD  = 0,
+    MMU_DATA_STORE = 1,
+    MMU_INST_FETCH = 2
+#define MMU_ACCESS_COUNT 3
+} MMUAccessType;
+
+#endif
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 3a66d8581e..5f0422493b 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -25,6 +25,7 @@
 #include "exec/hwaddr.h"
 #include "exec/vaddr.h"
 #include "exec/memattrs.h"
+#include "exec/mmu-access-type.h"
 #include "exec/tlb-common.h"
 #include "qapi/qapi-types-run-state.h"
 #include "qemu/bitmap.h"
@@ -80,13 +81,6 @@ DECLARE_CLASS_CHECKERS(CPUClass, CPU,
     typedef struct ArchCPU CpuInstanceType; \
     OBJECT_DECLARE_TYPE(ArchCPU, CpuClassType, CPU_MODULE_OBJ_NAME);
 
-typedef enum MMUAccessType {
-    MMU_DATA_LOAD  = 0,
-    MMU_DATA_STORE = 1,
-    MMU_INST_FETCH = 2
-#define MMU_ACCESS_COUNT 3
-} MMUAccessType;
-
 typedef struct CPUWatchpoint CPUWatchpoint;
 
 /* see accel-cpu.h */
-- 
2.41.0

Re: [PATCH 01/24] exec: Declare MMUAccessType type in 'mmu-access-type.h' header

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> The MMUAccessType enum is declared in "hw/core/cpu.h".
> "hw/core/cpu.h" contains declarations related to CPUState
> and CPUClass. Some source files only require MMUAccessType
> and don't need to pull in all CPU* declarations. In order
> to simplify, create a new "exec/mmu-access-type.h" header.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/exec/cpu_ldst.h        |  1 +
>   include/exec/exec-all.h        |  1 +
>   include/exec/mmu-access-type.h | 18 ++++++++++++++++++
>   include/hw/core/cpu.h          |  8 +-------
>   4 files changed, 21 insertions(+), 7 deletions(-)
>   create mode 100644 include/exec/mmu-access-type.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 02/24] exec: Declare CPUBreakpoint/CPUWatchpoint type in 'breakpoint.h' header

[Philippe Mathieu-Daudé]

The CPUBreakpoint and CPUWatchpoint structures are declared
in "hw/core/cpu.h", which contains declarations related to
CPUState and CPUClass. Some source files only require the
BP/WP definitions and don't need to pull in all CPU* API.
In order to simplify, create a new "exec/breakpoint.h" header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/breakpoint.h | 23 +++++++++++++++++++++++
 include/hw/core/cpu.h     | 16 +---------------
 target/arm/internals.h    |  1 +
 target/ppc/internal.h     |  1 +
 target/riscv/debug.h      |  2 ++
 5 files changed, 28 insertions(+), 15 deletions(-)
 create mode 100644 include/exec/breakpoint.h

diff --git a/include/exec/breakpoint.h b/include/exec/breakpoint.h
new file mode 100644
index 0000000000..45038263e3
--- /dev/null
+++ b/include/exec/breakpoint.h
@@ -0,0 +1,23 @@
+#ifndef EXEC_BREAKPOINT_H
+#define EXEC_BREAKPOINT_H
+
+#include "qemu/queue.h"
+#include "exec/vaddr.h"
+#include "exec/memattrs.h" //MemTxAttrs
+
+typedef struct CPUBreakpoint {
+    vaddr pc;
+    int flags; /* BP_* */
+    QTAILQ_ENTRY(CPUBreakpoint) entry;
+} CPUBreakpoint;
+
+typedef struct CPUWatchpoint {
+    vaddr vaddr;
+    vaddr len;
+    vaddr hitaddr;
+    MemTxAttrs hitattrs;
+    int flags; /* BP_* */
+    QTAILQ_ENTRY(CPUWatchpoint) entry;
+} CPUWatchpoint;
+
+#endif
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 5f0422493b..6f5a16e0fc 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -22,6 +22,7 @@
 
 #include "hw/qdev-core.h"
 #include "disas/dis-asm.h"
+#include "exec/breakpoint.h"
 #include "exec/hwaddr.h"
 #include "exec/vaddr.h"
 #include "exec/memattrs.h"
@@ -347,21 +348,6 @@ typedef struct CPUNegativeOffsetState {
     bool can_do_io;
 } CPUNegativeOffsetState;
 
-typedef struct CPUBreakpoint {
-    vaddr pc;
-    int flags; /* BP_* */
-    QTAILQ_ENTRY(CPUBreakpoint) entry;
-} CPUBreakpoint;
-
-struct CPUWatchpoint {
-    vaddr vaddr;
-    vaddr len;
-    vaddr hitaddr;
-    MemTxAttrs hitattrs;
-    int flags; /* BP_* */
-    QTAILQ_ENTRY(CPUWatchpoint) entry;
-};
-
 struct KVMState;
 struct kvm_run;
 
diff --git a/target/arm/internals.h b/target/arm/internals.h
index dd3da211a3..a7c5ec1849 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -25,6 +25,7 @@
 #ifndef TARGET_ARM_INTERNALS_H
 #define TARGET_ARM_INTERNALS_H
 
+#include "exec/breakpoint.h"
 #include "hw/registerfields.h"
 #include "tcg/tcg-gvec-desc.h"
 #include "syndrome.h"
diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index 5b20ecbd33..601c0b533f 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -18,6 +18,7 @@
 #ifndef PPC_INTERNAL_H
 #define PPC_INTERNAL_H
 
+#include "exec/breakpoint.h"
 #include "hw/registerfields.h"
 
 /* PM instructions */
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 5794aa6ee5..c347863578 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -22,6 +22,8 @@
 #ifndef RISCV_DEBUG_H
 #define RISCV_DEBUG_H
 
+#include "exec/breakpoint.h"
+
 #define RV_MAX_TRIGGERS         2
 
 /* register index of tdata CSRs */
-- 
2.41.0

Re: [PATCH 02/24] exec: Declare CPUBreakpoint/CPUWatchpoint type in 'breakpoint.h' header

[Philippe Mathieu-Daudé]

On 18/4/24 21:25, Philippe Mathieu-Daudé wrote:
> The CPUBreakpoint and CPUWatchpoint structures are declared
> in "hw/core/cpu.h", which contains declarations related to
> CPUState and CPUClass. Some source files only require the
> BP/WP definitions and don't need to pull in all CPU* API.
> In order to simplify, create a new "exec/breakpoint.h" header.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/breakpoint.h | 23 +++++++++++++++++++++++
>   include/hw/core/cpu.h     | 16 +---------------
>   target/arm/internals.h    |  1 +
>   target/ppc/internal.h     |  1 +
>   target/riscv/debug.h      |  2 ++
>   5 files changed, 28 insertions(+), 15 deletions(-)
>   create mode 100644 include/exec/breakpoint.h
> 
> diff --git a/include/exec/breakpoint.h b/include/exec/breakpoint.h
> new file mode 100644
> index 0000000000..45038263e3
> --- /dev/null
> +++ b/include/exec/breakpoint.h
> @@ -0,0 +1,23 @@

Oops, forgot:

+/*
+ * QEMU breakpoint & watchpoint definitions
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */

> +#ifndef EXEC_BREAKPOINT_H
> +#define EXEC_BREAKPOINT_H
> +
> +#include "qemu/queue.h"
> +#include "exec/vaddr.h"
> +#include "exec/memattrs.h" //MemTxAttrs

Also forgot to remove that comment.

> +
> +typedef struct CPUBreakpoint {
> +    vaddr pc;
> +    int flags; /* BP_* */
> +    QTAILQ_ENTRY(CPUBreakpoint) entry;
> +} CPUBreakpoint;
> +
> +typedef struct CPUWatchpoint {
> +    vaddr vaddr;
> +    vaddr len;
> +    vaddr hitaddr;
> +    MemTxAttrs hitattrs;
> +    int flags; /* BP_* */
> +    QTAILQ_ENTRY(CPUWatchpoint) entry;
> +} CPUWatchpoint;
> +
> +#endif

Re: [PATCH 02/24] exec: Declare CPUBreakpoint/CPUWatchpoint type in 'breakpoint.h' header

[Anton Johansson via]

On 18/04/24, Philippe Mathieu-Daudé wrote:
> On 18/4/24 21:25, Philippe Mathieu-Daudé wrote:
> > The CPUBreakpoint and CPUWatchpoint structures are declared
> > in "hw/core/cpu.h", which contains declarations related to
> > CPUState and CPUClass. Some source files only require the
> > BP/WP definitions and don't need to pull in all CPU* API.
> > In order to simplify, create a new "exec/breakpoint.h" header.
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   include/exec/breakpoint.h | 23 +++++++++++++++++++++++
> >   include/hw/core/cpu.h     | 16 +---------------
> >   target/arm/internals.h    |  1 +
> >   target/ppc/internal.h     |  1 +
> >   target/riscv/debug.h      |  2 ++
> >   5 files changed, 28 insertions(+), 15 deletions(-)
> >   create mode 100644 include/exec/breakpoint.h

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 02/24] exec: Declare CPUBreakpoint/CPUWatchpoint type in 'breakpoint.h' header

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> The CPUBreakpoint and CPUWatchpoint structures are declared
> in "hw/core/cpu.h", which contains declarations related to
> CPUState and CPUClass. Some source files only require the
> BP/WP definitions and don't need to pull in all CPU* API.
> In order to simplify, create a new "exec/breakpoint.h" header.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/exec/breakpoint.h | 23 +++++++++++++++++++++++
>   include/hw/core/cpu.h     | 16 +---------------
>   target/arm/internals.h    |  1 +
>   target/ppc/internal.h     |  1 +
>   target/riscv/debug.h      |  2 ++
>   5 files changed, 28 insertions(+), 15 deletions(-)
>   create mode 100644 include/exec/breakpoint.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 03/24] hw/core: Avoid including the full 'hw/core/cpu.h' in 'tcg-cpu-ops.h'

[Philippe Mathieu-Daudé]

Only include what is required, avoiding the full
CPUState API from the huge "hw/core/cpu.h" header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/core/tcg-cpu-ops.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index dc1f16a977..9387d38748 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -10,7 +10,11 @@
 #ifndef TCG_CPU_OPS_H
 #define TCG_CPU_OPS_H
 
-#include "hw/core/cpu.h"
+#include "exec/breakpoint.h"
+#include "exec/hwaddr.h"
+#include "exec/memattrs.h"
+#include "exec/mmu-access-type.h"
+#include "exec/vaddr.h"
 
 struct TCGCPUOps {
     /**
-- 
2.41.0

Re: [PATCH 03/24] hw/core: Avoid including the full 'hw/core/cpu.h' in 'tcg-cpu-ops.h'

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> Only include what is required, avoiding the full
> CPUState API from the huge "hw/core/cpu.h" header.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/hw/core/tcg-cpu-ops.h | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 04/24] exec: Restrict TCG specific declarations of 'cputlb.h'

[Philippe Mathieu-Daudé]

Avoid TCG specific declarations being used from non-TCG accelerators.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cputlb.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index 6da1462c4f..ef18642a32 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -22,9 +22,14 @@
 
 #include "exec/cpu-common.h"
 
+#ifdef CONFIG_TCG
+
 #if !defined(CONFIG_USER_ONLY)
 /* cputlb.c */
 void tlb_protect_code(ram_addr_t ram_addr);
 void tlb_unprotect_code(ram_addr_t ram_addr);
 #endif
+
+#endif /* CONFIG_TCG */
+
 #endif
-- 
2.41.0

Re: [PATCH 04/24] exec: Restrict TCG specific declarations of 'cputlb.h'

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> Avoid TCG specific declarations being used from non-TCG accelerators.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/exec/cputlb.h | 5 +++++
>   1 file changed, 5 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 05/24] exec: Restrict 'cpu_ldst.h' to TCG accelerator

[Philippe Mathieu-Daudé]

"exec/cpu_ldst.h" is specific to TCG, do not allow its
inclusion from other accelerators.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu_ldst.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 5b99666702..f3c2a3ca74 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -1,5 +1,5 @@
 /*
- *  Software MMU support
+ *  Software MMU support (per-target)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -62,6 +62,10 @@
 #ifndef CPU_LDST_H
 #define CPU_LDST_H
 
+#ifndef CONFIG_TCG
+#error Can only include this header with TCG
+#endif
+
 #include "exec/memopidx.h"
 #include "exec/abi_ptr.h"
 #include "exec/mmu-access-type.h"
-- 
2.41.0

Re: [PATCH 05/24] exec: Restrict 'cpu_ldst.h' to TCG accelerator

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> "exec/cpu_ldst.h" is specific to TCG, do not allow its
> inclusion from other accelerators.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/exec/cpu_ldst.h | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 06/24] exec: Have guest_addr_valid() methods take abi_ptr/size_t arguments

[Philippe Mathieu-Daudé]

abi_ulong is target specific, replace by abi_ptr which isn't.
Use size_t for the @len type.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu_ldst.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index f3c2a3ca74..63186b07e4 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -92,12 +92,12 @@ static inline void *g2h(CPUState *cs, abi_ptr x)
     return g2h_untagged(cpu_untagged_addr(cs, x));
 }
 
-static inline bool guest_addr_valid_untagged(abi_ulong x)
+static inline bool guest_addr_valid_untagged(abi_ptr x)
 {
     return x <= GUEST_ADDR_MAX;
 }
 
-static inline bool guest_range_valid_untagged(abi_ulong start, abi_ulong len)
+static inline bool guest_range_valid_untagged(abi_ptr start, size_t len)
 {
     return len - 1 <= GUEST_ADDR_MAX && start <= GUEST_ADDR_MAX - len + 1;
 }
-- 
2.41.0

Re: [PATCH 06/24] exec: Have guest_addr_valid() methods take abi_ptr/size_t arguments

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> abi_ulong is target specific, replace by abi_ptr which isn't.
> Use size_t for the @len type.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/cpu_ldst.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index f3c2a3ca74..63186b07e4 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -92,12 +92,12 @@ static inline void *g2h(CPUState *cs, abi_ptr x)
>       return g2h_untagged(cpu_untagged_addr(cs, x));
>   }
>   
> -static inline bool guest_addr_valid_untagged(abi_ulong x)
> +static inline bool guest_addr_valid_untagged(abi_ptr x)
>   {
>       return x <= GUEST_ADDR_MAX;
>   }
>   
> -static inline bool guest_range_valid_untagged(abi_ulong start, abi_ulong len)
> +static inline bool guest_range_valid_untagged(abi_ptr start, size_t len)

No, this needs to remain a large type for 64-bit guest on 32-bit host.
(When will that ever die...)


r~

[PATCH 07/24] exec: Un-inline tlb_vaddr_to_host() and declare it in 'exec/cputlb.h'

[Philippe Mathieu-Daudé]

Declare tlb_vaddr_to_host() in "exec/cputlb.h" with the CPU TLB
API. Un-inline the user emulation definition to avoid including
"exec/cpu_ldst.h" (which declares g2h) in "exec/cputlb.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu_ldst.h      | 24 ------------------------
 include/exec/cputlb.h        | 18 ++++++++++++++++++
 accel/tcg/user-exec.c        |  7 +++++++
 target/arm/tcg/helper-a64.c  |  1 +
 target/riscv/vector_helper.c |  1 +
 target/sparc/mmu_helper.c    |  1 +
 6 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 63186b07e4..7032949dba 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -418,28 +418,4 @@ static inline int cpu_ldsw_code(CPUArchState *env, abi_ptr addr)
     return (int16_t)cpu_lduw_code(env, addr);
 }
 
-/**
- * tlb_vaddr_to_host:
- * @env: CPUArchState
- * @addr: guest virtual address to look up
- * @access_type: 0 for read, 1 for write, 2 for execute
- * @mmu_idx: MMU index to use for lookup
- *
- * Look up the specified guest virtual index in the TCG softmmu TLB.
- * If we can translate a host virtual address suitable for direct RAM
- * access, without causing a guest exception, then return it.
- * Otherwise (TLB entry is for an I/O access, guest software
- * TLB fill required, etc) return NULL.
- */
-#ifdef CONFIG_USER_ONLY
-static inline void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
-                                      MMUAccessType access_type, int mmu_idx)
-{
-    return g2h(env_cpu(env), addr);
-}
-#else
-void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
-                        MMUAccessType access_type, int mmu_idx);
-#endif
-
 #endif /* CPU_LDST_H */
diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index ef18642a32..173eb98b9a 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -20,10 +20,28 @@
 #ifndef CPUTLB_H
 #define CPUTLB_H
 
+#include "exec/abi_ptr.h"
 #include "exec/cpu-common.h"
+#include "exec/mmu-access-type.h"
 
 #ifdef CONFIG_TCG
 
+/**
+ * tlb_vaddr_to_host:
+ * @env: CPUArchState
+ * @addr: guest virtual address to look up
+ * @access_type: 0 for read, 1 for write, 2 for execute
+ * @mmu_idx: MMU index to use for lookup
+ *
+ * Look up the specified guest virtual index in the TCG softmmu TLB.
+ * If we can translate a host virtual address suitable for direct RAM
+ * access, without causing a guest exception, then return it.
+ * Otherwise (TLB entry is for an I/O access, guest software
+ * TLB fill required, etc) return NULL.
+ */
+void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
+                        MMUAccessType access_type, int mmu_idx);
+
 #if !defined(CONFIG_USER_ONLY)
 /* cputlb.c */
 void tlb_protect_code(ram_addr_t ram_addr);
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 1c621477ad..54b35588b9 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -23,6 +23,7 @@
 #include "tcg/tcg.h"
 #include "qemu/bitops.h"
 #include "qemu/rcu.h"
+#include "exec/cputlb.h"
 #include "exec/cpu_ldst.h"
 #include "exec/translate-all.h"
 #include "exec/helper-proto.h"
@@ -138,6 +139,12 @@ bool handle_sigsegv_accerr_write(CPUState *cpu, sigset_t *old_set,
     }
 }
 
+void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
+                        MMUAccessType access_type, int mmu_idx)
+{
+    return g2h(env_cpu(env), addr);
+}
+
 typedef struct PageFlagsNode {
     struct rcu_head rcu;
     IntervalTreeNode itree;
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index ebaa7f00df..9b3ae06207 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -29,6 +29,7 @@
 #include "internals.h"
 #include "qemu/crc32c.h"
 #include "exec/exec-all.h"
+#include "exec/cputlb.h"
 #include "exec/cpu_ldst.h"
 #include "qemu/int128.h"
 #include "qemu/atomic128.h"
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index fa139040f8..d3d9c33597 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -22,6 +22,7 @@
 #include "cpu.h"
 #include "exec/memop.h"
 #include "exec/exec-all.h"
+#include "exec/cputlb.h"
 #include "exec/cpu_ldst.h"
 #include "exec/helper-proto.h"
 #include "fpu/softfloat.h"
diff --git a/target/sparc/mmu_helper.c b/target/sparc/mmu_helper.c
index ad1591d9fd..e79a33367a 100644
--- a/target/sparc/mmu_helper.c
+++ b/target/sparc/mmu_helper.c
@@ -21,6 +21,7 @@
 #include "qemu/log.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
+#include "exec/cputlb.h"
 #include "qemu/qemu-print.h"
 #include "trace.h"
 
-- 
2.41.0

Re: [PATCH 07/24] exec: Un-inline tlb_vaddr_to_host() and declare it in 'exec/cputlb.h'

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> Declare tlb_vaddr_to_host() in "exec/cputlb.h" with the CPU TLB
> API. Un-inline the user emulation definition to avoid including
> "exec/cpu_ldst.h" (which declares g2h) in "exec/cputlb.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/cpu_ldst.h      | 24 ------------------------
>   include/exec/cputlb.h        | 18 ++++++++++++++++++
>   accel/tcg/user-exec.c        |  7 +++++++
>   target/arm/tcg/helper-a64.c  |  1 +
>   target/riscv/vector_helper.c |  1 +
>   target/sparc/mmu_helper.c    |  1 +
>   6 files changed, 28 insertions(+), 24 deletions(-)
> 
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index 63186b07e4..7032949dba 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -418,28 +418,4 @@ static inline int cpu_ldsw_code(CPUArchState *env, abi_ptr addr)
>       return (int16_t)cpu_lduw_code(env, addr);
>   }
>   
> -/**
> - * tlb_vaddr_to_host:
> - * @env: CPUArchState
> - * @addr: guest virtual address to look up
> - * @access_type: 0 for read, 1 for write, 2 for execute
> - * @mmu_idx: MMU index to use for lookup
> - *
> - * Look up the specified guest virtual index in the TCG softmmu TLB.
> - * If we can translate a host virtual address suitable for direct RAM
> - * access, without causing a guest exception, then return it.
> - * Otherwise (TLB entry is for an I/O access, guest software
> - * TLB fill required, etc) return NULL.
> - */
> -#ifdef CONFIG_USER_ONLY
> -static inline void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
> -                                      MMUAccessType access_type, int mmu_idx)
> -{
> -    return g2h(env_cpu(env), addr);
> -}
> -#else
> -void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
> -                        MMUAccessType access_type, int mmu_idx);
> -#endif
> -
>   #endif /* CPU_LDST_H */
> diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
> index ef18642a32..173eb98b9a 100644
> --- a/include/exec/cputlb.h
> +++ b/include/exec/cputlb.h
> @@ -20,10 +20,28 @@
>   #ifndef CPUTLB_H
>   #define CPUTLB_H
>   
> +#include "exec/abi_ptr.h"
>   #include "exec/cpu-common.h"
> +#include "exec/mmu-access-type.h"
>   
>   #ifdef CONFIG_TCG
>   
> +/**
> + * tlb_vaddr_to_host:
> + * @env: CPUArchState
> + * @addr: guest virtual address to look up
> + * @access_type: 0 for read, 1 for write, 2 for execute
> + * @mmu_idx: MMU index to use for lookup
> + *
> + * Look up the specified guest virtual index in the TCG softmmu TLB.
> + * If we can translate a host virtual address suitable for direct RAM
> + * access, without causing a guest exception, then return it.
> + * Otherwise (TLB entry is for an I/O access, guest software
> + * TLB fill required, etc) return NULL.
> + */
> +void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
> +                        MMUAccessType access_type, int mmu_idx);
> +

Why have you chosen cputlb.h, when the other probe functions are in exec-all.h?

Alternately, we only have two users remaining, which could be migrated to the newer 
interfaces...


r~

Re: [PATCH 07/24] exec: Un-inline tlb_vaddr_to_host() and declare it in 'exec/cputlb.h'

[Anton Johansson via]

On 18/04/24, Philippe Mathieu-Daudé wrote:
> Declare tlb_vaddr_to_host() in "exec/cputlb.h" with the CPU TLB
> API. Un-inline the user emulation definition to avoid including
> "exec/cpu_ldst.h" (which declares g2h) in "exec/cputlb.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/exec/cpu_ldst.h      | 24 ------------------------
>  include/exec/cputlb.h        | 18 ++++++++++++++++++
>  accel/tcg/user-exec.c        |  7 +++++++
>  target/arm/tcg/helper-a64.c  |  1 +
>  target/riscv/vector_helper.c |  1 +
>  target/sparc/mmu_helper.c    |  1 +
>  6 files changed, 28 insertions(+), 24 deletions(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 07/24] exec: Un-inline tlb_vaddr_to_host() and declare it in 'exec/cputlb.h'

[Anton Johansson via]

On 25/04/24, Anton Johansson via wrote:
> On 18/04/24, Philippe Mathieu-Daudé wrote:
> > Declare tlb_vaddr_to_host() in "exec/cputlb.h" with the CPU TLB
> > API. Un-inline the user emulation definition to avoid including
> > "exec/cpu_ldst.h" (which declares g2h) in "exec/cputlb.h".
> > 
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >  include/exec/cpu_ldst.h      | 24 ------------------------
> >  include/exec/cputlb.h        | 18 ++++++++++++++++++
> >  accel/tcg/user-exec.c        |  7 +++++++
> >  target/arm/tcg/helper-a64.c  |  1 +
> >  target/riscv/vector_helper.c |  1 +
> >  target/sparc/mmu_helper.c    |  1 +
> >  6 files changed, 28 insertions(+), 24 deletions(-)
> 
> Reviewed-by: Anton Johansson <anjo@rev.ng>
> 

Missed existing review, my bad!

[PATCH 08/24] physmem: Move TCG CPU IOTLB methods around

[Philippe Mathieu-Daudé]

The next commit will restrict TCG specific code in physmem.c
using some #ifdef'ry. In order to keep it simple, move
iotlb_to_section() and memory_region_section_get_iotlb()
around close together.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/physmem.c | 50 ++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/system/physmem.c b/system/physmem.c
index dd7b222942..cf6334f3a3 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -736,6 +736,31 @@ translate_fail:
     return &d->map.sections[PHYS_SECTION_UNASSIGNED];
 }
 
+MemoryRegionSection *iotlb_to_section(CPUState *cpu,
+                                      hwaddr index, MemTxAttrs attrs)
+{
+    int asidx = cpu_asidx_from_attrs(cpu, attrs);
+    CPUAddressSpace *cpuas = &cpu->cpu_ases[asidx];
+    AddressSpaceDispatch *d = cpuas->memory_dispatch;
+    int section_index = index & ~TARGET_PAGE_MASK;
+    MemoryRegionSection *ret;
+
+    assert(section_index < d->map.sections_nb);
+    ret = d->map.sections + section_index;
+    assert(ret->mr);
+    assert(ret->mr->ops);
+
+    return ret;
+}
+
+/* Called from RCU critical section */
+hwaddr memory_region_section_get_iotlb(CPUState *cpu,
+                                       MemoryRegionSection *section)
+{
+    AddressSpaceDispatch *d = flatview_to_dispatch(section->fv);
+    return section - d->map.sections;
+}
+
 void cpu_address_space_init(CPUState *cpu, int asidx,
                             const char *prefix, MemoryRegion *mr)
 {
@@ -957,14 +982,6 @@ bool cpu_physical_memory_snapshot_get_dirty(DirtyBitmapSnapshot *snap,
     return false;
 }
 
-/* Called from RCU critical section */
-hwaddr memory_region_section_get_iotlb(CPUState *cpu,
-                                       MemoryRegionSection *section)
-{
-    AddressSpaceDispatch *d = flatview_to_dispatch(section->fv);
-    return section - d->map.sections;
-}
-
 static int subpage_register(subpage_t *mmio, uint32_t start, uint32_t end,
                             uint16_t section);
 static subpage_t *subpage_init(FlatView *fv, hwaddr base);
@@ -2434,23 +2451,6 @@ static uint16_t dummy_section(PhysPageMap *map, FlatView *fv, MemoryRegion *mr)
     return phys_section_add(map, &section);
 }
 
-MemoryRegionSection *iotlb_to_section(CPUState *cpu,
-                                      hwaddr index, MemTxAttrs attrs)
-{
-    int asidx = cpu_asidx_from_attrs(cpu, attrs);
-    CPUAddressSpace *cpuas = &cpu->cpu_ases[asidx];
-    AddressSpaceDispatch *d = cpuas->memory_dispatch;
-    int section_index = index & ~TARGET_PAGE_MASK;
-    MemoryRegionSection *ret;
-
-    assert(section_index < d->map.sections_nb);
-    ret = d->map.sections + section_index;
-    assert(ret->mr);
-    assert(ret->mr->ops);
-
-    return ret;
-}
-
 static void io_mem_init(void)
 {
     memory_region_init_io(&io_mem_unassigned, NULL, &unassigned_mem_ops, NULL,
-- 
2.41.0

Re: [PATCH 08/24] physmem: Move TCG CPU IOTLB methods around

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> The next commit will restrict TCG specific code in physmem.c
> using some #ifdef'ry. In order to keep it simple, move
> iotlb_to_section() and memory_region_section_get_iotlb()
> around close together.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   system/physmem.c | 50 ++++++++++++++++++++++++------------------------
>   1 file changed, 25 insertions(+), 25 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 09/24] physmem: Restrict TCG CPU IOTLB code to TCG accel

[Philippe Mathieu-Daudé]

Restrict iotlb_to_section(), address_space_translate_for_iotlb()
and memory_region_section_get_iotlb() to TCG. Declare them in
"exec/cputlb.h". Declare iotlb_to_section() using the
MemoryRegionSection typedef.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cputlb.h   | 25 +++++++++++++++++++++++--
 include/exec/exec-all.h | 21 ---------------------
 system/physmem.c        |  6 ++++++
 3 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index 173eb98b9a..3594f904b4 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -21,6 +21,8 @@
 #define CPUTLB_H
 
 #include "exec/abi_ptr.h"
+#include "exec/hwaddr.h"
+#include "exec/memattrs.h"
 #include "exec/cpu-common.h"
 #include "exec/mmu-access-type.h"
 
@@ -43,10 +45,29 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
                         MMUAccessType access_type, int mmu_idx);
 
 #if !defined(CONFIG_USER_ONLY)
-/* cputlb.c */
+
 void tlb_protect_code(ram_addr_t ram_addr);
 void tlb_unprotect_code(ram_addr_t ram_addr);
-#endif
+
+/**
+ * iotlb_to_section:
+ * @cpu: CPU performing the access
+ * @index: TCG CPU IOTLB entry
+ *
+ * Given a TCG CPU IOTLB entry, return the MemoryRegionSection that
+ * it refers to. @index will have been initially created and returned
+ * by memory_region_section_get_iotlb().
+ */
+MemoryRegionSection *iotlb_to_section(CPUState *cpu,
+                                      hwaddr index, MemTxAttrs attrs);
+MemoryRegionSection *
+address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr,
+                                  hwaddr *xlat, hwaddr *plen,
+                                  MemTxAttrs attrs, int *prot);
+hwaddr memory_region_section_get_iotlb(CPUState *cpu,
+                                       MemoryRegionSection *section);
+
+#endif /* CONFIG_USER_ONLY */
 
 #endif /* CONFIG_TCG */
 
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 5cddfea01b..e4cff08db4 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -536,21 +536,6 @@ extern __thread uintptr_t tci_tb_ptr;
    smaller than 4 bytes, so we don't worry about special-casing this.  */
 #define GETPC_ADJ   2
 
-#if !defined(CONFIG_USER_ONLY)
-
-/**
- * iotlb_to_section:
- * @cpu: CPU performing the access
- * @index: TCG CPU IOTLB entry
- *
- * Given a TCG CPU IOTLB entry, return the MemoryRegionSection that
- * it refers to. @index will have been initially created and returned
- * by memory_region_section_get_iotlb().
- */
-struct MemoryRegionSection *iotlb_to_section(CPUState *cpu,
-                                             hwaddr index, MemTxAttrs attrs);
-#endif
-
 /**
  * get_page_addr_code_hostp()
  * @env: CPUArchState
@@ -659,12 +644,6 @@ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
 void tlb_set_dirty(CPUState *cpu, vaddr addr);
 void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length);
 
-MemoryRegionSection *
-address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr,
-                                  hwaddr *xlat, hwaddr *plen,
-                                  MemTxAttrs attrs, int *prot);
-hwaddr memory_region_section_get_iotlb(CPUState *cpu,
-                                       MemoryRegionSection *section);
 #endif
 
 #endif
diff --git a/system/physmem.c b/system/physmem.c
index cf6334f3a3..38d3ede9f9 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -31,6 +31,7 @@
 #endif /* CONFIG_TCG */
 
 #include "exec/exec-all.h"
+#include "exec/cputlb.h"
 #include "exec/target_page.h"
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
@@ -585,6 +586,7 @@ typedef struct TCGIOMMUNotifier {
     bool active;
 } TCGIOMMUNotifier;
 
+#ifdef CONFIG_TCG
 static void tcg_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
 {
     TCGIOMMUNotifier *notifier = container_of(n, TCGIOMMUNotifier, n);
@@ -648,6 +650,7 @@ static void tcg_register_iommu_notifier(CPUState *cpu,
         notifier->active = true;
     }
 }
+#endif
 
 void tcg_iommu_free_notifier_list(CPUState *cpu)
 {
@@ -668,6 +671,7 @@ void tcg_iommu_init_notifier_list(CPUState *cpu)
     cpu->iommu_notifiers = g_array_new(false, true, sizeof(TCGIOMMUNotifier *));
 }
 
+#ifdef CONFIG_TCG
 /* Called from RCU critical section */
 MemoryRegionSection *
 address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr orig_addr,
@@ -761,6 +765,8 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
     return section - d->map.sections;
 }
 
+#endif /* CONFIG_TCG */
+
 void cpu_address_space_init(CPUState *cpu, int asidx,
                             const char *prefix, MemoryRegion *mr)
 {
-- 
2.41.0

Re: [PATCH 09/24] physmem: Restrict TCG CPU IOTLB code to TCG accel

[Anton Johansson via]

On 18/04/24, Philippe Mathieu-Daudé wrote:
> Restrict iotlb_to_section(), address_space_translate_for_iotlb()
> and memory_region_section_get_iotlb() to TCG. Declare them in
> "exec/cputlb.h". Declare iotlb_to_section() using the
> MemoryRegionSection typedef.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/exec/cputlb.h   | 25 +++++++++++++++++++++++--
>  include/exec/exec-all.h | 21 ---------------------
>  system/physmem.c        |  6 ++++++
>  3 files changed, 29 insertions(+), 23 deletions(-)
> 
> diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
> index 173eb98b9a..3594f904b4 100644
> --- a/include/exec/cputlb.h
> +++ b/include/exec/cputlb.h
> @@ -21,6 +21,8 @@
>  #define CPUTLB_H
>  
>  #include "exec/abi_ptr.h"
> +#include "exec/hwaddr.h"
> +#include "exec/memattrs.h"
>  #include "exec/cpu-common.h"
>  #include "exec/mmu-access-type.h"
>  
> @@ -43,10 +45,29 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
>                          MMUAccessType access_type, int mmu_idx);
>  
>  #if !defined(CONFIG_USER_ONLY)
> -/* cputlb.c */
> +
>  void tlb_protect_code(ram_addr_t ram_addr);
>  void tlb_unprotect_code(ram_addr_t ram_addr);
> -#endif
> +
> +/**
> + * iotlb_to_section:
> + * @cpu: CPU performing the access
> + * @index: TCG CPU IOTLB entry
> + *
> + * Given a TCG CPU IOTLB entry, return the MemoryRegionSection that
> + * it refers to. @index will have been initially created and returned
> + * by memory_region_section_get_iotlb().
> + */
> +MemoryRegionSection *iotlb_to_section(CPUState *cpu,
> +                                      hwaddr index, MemTxAttrs attrs);
> +MemoryRegionSection *
> +address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr,
> +                                  hwaddr *xlat, hwaddr *plen,
> +                                  MemTxAttrs attrs, int *prot);
> +hwaddr memory_region_section_get_iotlb(CPUState *cpu,
> +                                       MemoryRegionSection *section);
> +
> +#endif /* CONFIG_USER_ONLY */

!CONFIG_USER_ONLY

>  
>  #endif /* CONFIG_TCG */
>  
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 5cddfea01b..e4cff08db4 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -536,21 +536,6 @@ extern __thread uintptr_t tci_tb_ptr;
>     smaller than 4 bytes, so we don't worry about special-casing this.  */
>  #define GETPC_ADJ   2
>  
> -#if !defined(CONFIG_USER_ONLY)
> -
> -/**
> - * iotlb_to_section:
> - * @cpu: CPU performing the access
> - * @index: TCG CPU IOTLB entry
> - *
> - * Given a TCG CPU IOTLB entry, return the MemoryRegionSection that
> - * it refers to. @index will have been initially created and returned
> - * by memory_region_section_get_iotlb().
> - */
> -struct MemoryRegionSection *iotlb_to_section(CPUState *cpu,
> -                                             hwaddr index, MemTxAttrs attrs);
> -#endif
> -
>  /**
>   * get_page_addr_code_hostp()
>   * @env: CPUArchState
> @@ -659,12 +644,6 @@ void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
>  void tlb_set_dirty(CPUState *cpu, vaddr addr);
>  void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length);
>  
> -MemoryRegionSection *
> -address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr,
> -                                  hwaddr *xlat, hwaddr *plen,
> -                                  MemTxAttrs attrs, int *prot);
> -hwaddr memory_region_section_get_iotlb(CPUState *cpu,
> -                                       MemoryRegionSection *section);
>  #endif
>  
>  #endif
> diff --git a/system/physmem.c b/system/physmem.c
> index cf6334f3a3..38d3ede9f9 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -31,6 +31,7 @@
>  #endif /* CONFIG_TCG */
>  
>  #include "exec/exec-all.h"
> +#include "exec/cputlb.h"
>  #include "exec/target_page.h"
>  #include "hw/qdev-core.h"
>  #include "hw/qdev-properties.h"
> @@ -585,6 +586,7 @@ typedef struct TCGIOMMUNotifier {
>      bool active;
>  } TCGIOMMUNotifier;
>  
> +#ifdef CONFIG_TCG
>  static void tcg_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
>  {
>      TCGIOMMUNotifier *notifier = container_of(n, TCGIOMMUNotifier, n);
> @@ -648,6 +650,7 @@ static void tcg_register_iommu_notifier(CPUState *cpu,
>          notifier->active = true;
>      }
>  }
> +#endif
>  
>  void tcg_iommu_free_notifier_list(CPUState *cpu)
>  {
> @@ -668,6 +671,7 @@ void tcg_iommu_init_notifier_list(CPUState *cpu)
>      cpu->iommu_notifiers = g_array_new(false, true, sizeof(TCGIOMMUNotifier *));
>  }
>  
> +#ifdef CONFIG_TCG
>  /* Called from RCU critical section */
>  MemoryRegionSection *
>  address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr orig_addr,
> @@ -761,6 +765,8 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
>      return section - d->map.sections;
>  }
>  
> +#endif /* CONFIG_TCG */

I might be missing some context here but are the other iotlb functions
#ifdef CONFIG_TCG'd by some other patch?

Otherwise,
Reviewed-by: Anton Johansson <anjo@rev.ng>

[PATCH 10/24] exec: Reduce tlb_set_dirty() declaration scope

[Philippe Mathieu-Daudé]

tlb_set_dirty() is only used in accel/tcg/cputlb.c,
where it is defined. Declare it statically, removing
the stub.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/exec-all.h | 1 -
 accel/stubs/tcg-stub.c  | 4 ----
 accel/tcg/cputlb.c      | 2 +-
 3 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index e4cff08db4..778c82ba8e 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -641,7 +641,6 @@ static inline void mmap_unlock(void) {}
 #define WITH_MMAP_LOCK_GUARD()
 
 void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
-void tlb_set_dirty(CPUState *cpu, vaddr addr);
 void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length);
 
 #endif
diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
index 8a496a2a6f..dd890d6cf6 100644
--- a/accel/stubs/tcg-stub.c
+++ b/accel/stubs/tcg-stub.c
@@ -18,10 +18,6 @@ void tb_flush(CPUState *cpu)
 {
 }
 
-void tlb_set_dirty(CPUState *cpu, vaddr vaddr)
-{
-}
-
 int probe_access_flags(CPUArchState *env, vaddr addr, int size,
                        MMUAccessType access_type, int mmu_idx,
                        bool nonfault, void **phost, uintptr_t retaddr)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 93b1ca810b..e16d02a62c 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1039,7 +1039,7 @@ static inline void tlb_set_dirty1_locked(CPUTLBEntry *tlb_entry,
 
 /* update the TLB corresponding to virtual page vaddr
    so that it is no longer dirty */
-void tlb_set_dirty(CPUState *cpu, vaddr addr)
+static void tlb_set_dirty(CPUState *cpu, vaddr addr)
 {
     int mmu_idx;
 
-- 
2.41.0

Re: [PATCH 10/24] exec: Reduce tlb_set_dirty() declaration scope

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> tlb_set_dirty() is only used in accel/tcg/cputlb.c,
> where it is defined. Declare it statically, removing
> the stub.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/exec/exec-all.h | 1 -
>   accel/stubs/tcg-stub.c  | 4 ----
>   accel/tcg/cputlb.c      | 2 +-
>   3 files changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 11/24] exec: Move tlb_reset_dirty*() declarations to 'exec/cputlb.h'

[Philippe Mathieu-Daudé]

Declare tlb_reset_dirty() and tlb_reset_dirty_range_all()
in "exec/cputlb.h". Restrict tlb_reset_dirty_range_all()
to TCG accel.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cputlb.h   | 12 +++++++++++-
 include/exec/exec-all.h |  3 ---
 include/exec/ram_addr.h |  1 +
 system/physmem.c        |  2 ++
 4 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index 3594f904b4..dc92befb93 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -49,6 +49,9 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
 void tlb_protect_code(ram_addr_t ram_addr);
 void tlb_unprotect_code(ram_addr_t ram_addr);
 
+void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
+void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length);
+
 /**
  * iotlb_to_section:
  * @cpu: CPU performing the access
@@ -69,6 +72,13 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
 
 #endif /* CONFIG_USER_ONLY */
 
-#endif /* CONFIG_TCG */
+#else /* !CONFIG_TCG */
+
+static inline void tlb_reset_dirty_range_all(ram_addr_t start,
+                                             ram_addr_t length)
+{
+}
+
+#endif /* !CONFIG_TCG */
 
 #endif
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 778c82ba8e..6f46015ab4 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -640,9 +640,6 @@ static inline void mmap_lock(void) {}
 static inline void mmap_unlock(void) {}
 #define WITH_MMAP_LOCK_GUARD()
 
-void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
-void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length);
-
 #endif
 
 #endif
diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 3fc83587c0..f06ae9b516 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -26,6 +26,7 @@
 #include "exec/ramlist.h"
 #include "exec/ramblock.h"
 #include "exec/exec-all.h"
+#include "cputlb.h"
 #include "qemu/rcu.h"
 
 extern uint64_t total_dirty_pages;
diff --git a/system/physmem.c b/system/physmem.c
index 38d3ede9f9..7a7876a375 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -850,6 +850,7 @@ found:
     return block;
 }
 
+#ifdef CONFIG_TCG
 void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length)
 {
     CPUState *cpu;
@@ -869,6 +870,7 @@ void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length)
         tlb_reset_dirty(cpu, start1, length);
     }
 }
+#endif
 
 /* Note: start and end must be within the same ram block.  */
 bool cpu_physical_memory_test_and_clear_dirty(ram_addr_t start,
-- 
2.41.0

Re: [PATCH 11/24] exec: Move tlb_reset_dirty*() declarations to 'exec/cputlb.h'

[Anton Johansson via]

On 18/04/24, Philippe Mathieu-Daudé wrote:
> Declare tlb_reset_dirty() and tlb_reset_dirty_range_all()
> in "exec/cputlb.h". Restrict tlb_reset_dirty_range_all()
> to TCG accel.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/exec/cputlb.h   | 12 +++++++++++-
>  include/exec/exec-all.h |  3 ---
>  include/exec/ram_addr.h |  1 +
>  system/physmem.c        |  2 ++
>  4 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
> index 3594f904b4..dc92befb93 100644
> --- a/include/exec/cputlb.h
> +++ b/include/exec/cputlb.h
> @@ -49,6 +49,9 @@ void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
>  void tlb_protect_code(ram_addr_t ram_addr);
>  void tlb_unprotect_code(ram_addr_t ram_addr);
>  
> +void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
> +void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length);
> +
>  /**
>   * iotlb_to_section:
>   * @cpu: CPU performing the access
> @@ -69,6 +72,13 @@ hwaddr memory_region_section_get_iotlb(CPUState *cpu,
>  
>  #endif /* CONFIG_USER_ONLY */
>  
> -#endif /* CONFIG_TCG */
> +#else /* !CONFIG_TCG */
> +
> +static inline void tlb_reset_dirty_range_all(ram_addr_t start,
> +                                             ram_addr_t length)
> +{
> +}
> +
> +#endif /* !CONFIG_TCG */

The only use of tlb_reset_dirty_range_all() is in
cpu_physical_memory_dirty_bits_cleared() surrounded by an
if (tcg_enabled()).  Would stubbing the latter function be better?

>  
>  #endif
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 778c82ba8e..6f46015ab4 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -640,9 +640,6 @@ static inline void mmap_lock(void) {}
>  static inline void mmap_unlock(void) {}
>  #define WITH_MMAP_LOCK_GUARD()
>  
> -void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
> -void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length);
> -
>  #endif
>  
>  #endif
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 3fc83587c0..f06ae9b516 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -26,6 +26,7 @@
>  #include "exec/ramlist.h"
>  #include "exec/ramblock.h"
>  #include "exec/exec-all.h"
> +#include "cputlb.h"
>  #include "qemu/rcu.h"
>  
>  extern uint64_t total_dirty_pages;
> diff --git a/system/physmem.c b/system/physmem.c
> index 38d3ede9f9..7a7876a375 100644
> --- a/system/physmem.c
> +++ b/system/physmem.c
> @@ -850,6 +850,7 @@ found:
>      return block;
>  }
>  
> +#ifdef CONFIG_TCG
>  void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length)
>  {
>      CPUState *cpu;
> @@ -869,6 +870,7 @@ void tlb_reset_dirty_range_all(ram_addr_t start, ram_addr_t length)
>          tlb_reset_dirty(cpu, start1, length);
>      }
>  }
> +#endif

This function only depends on qemu_get_ramblock which is statically
defined in physmem.c although it doesnt depend on anything in physmem.c
as far as I can tell. Thoughts on moving qemu_get_ramblock to a common
.c or .h and moving the tlb_reset_dirty_range_all definition to
cputlb.c?

[PATCH 12/24] exec: Move CPUTLBEntry helpers to cputlb.c

[Philippe Mathieu-Daudé]

The following CPUTLBEntry helpers are only used in accel/tcg/cputlb.c:
  - tlb_index()
  - tlb_entry()
  - tlb_read_idx()
  - tlb_addr_write()

Move them to this file, allowing to remove the huge "cpu.h" header
inclusion from "exec/cpu_ldst.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu_ldst.h | 55 -----------------------------------------
 accel/tcg/cputlb.c      | 51 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 55 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 7032949dba..2c5a0a5c81 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -70,7 +70,6 @@
 #include "exec/abi_ptr.h"
 #include "exec/mmu-access-type.h"
 #include "qemu/int128.h"
-#include "cpu.h"
 
 #if defined(CONFIG_USER_ONLY)
 
@@ -294,60 +293,6 @@ Int128 cpu_atomic_cmpxchgo_be_mmu(CPUArchState *env, abi_ptr addr,
                                   Int128 cmpv, Int128 newv,
                                   MemOpIdx oi, uintptr_t retaddr);
 
-#if !defined(CONFIG_USER_ONLY)
-
-#include "tcg/oversized-guest.h"
-
-static inline uint64_t tlb_read_idx(const CPUTLBEntry *entry,
-                                    MMUAccessType access_type)
-{
-    /* Do not rearrange the CPUTLBEntry structure members. */
-    QEMU_BUILD_BUG_ON(offsetof(CPUTLBEntry, addr_read) !=
-                      MMU_DATA_LOAD * sizeof(uint64_t));
-    QEMU_BUILD_BUG_ON(offsetof(CPUTLBEntry, addr_write) !=
-                      MMU_DATA_STORE * sizeof(uint64_t));
-    QEMU_BUILD_BUG_ON(offsetof(CPUTLBEntry, addr_code) !=
-                      MMU_INST_FETCH * sizeof(uint64_t));
-
-#if TARGET_LONG_BITS == 32
-    /* Use qatomic_read, in case of addr_write; only care about low bits. */
-    const uint32_t *ptr = (uint32_t *)&entry->addr_idx[access_type];
-    ptr += HOST_BIG_ENDIAN;
-    return qatomic_read(ptr);
-#else
-    const uint64_t *ptr = &entry->addr_idx[access_type];
-# if TCG_OVERSIZED_GUEST
-    return *ptr;
-# else
-    /* ofs might correspond to .addr_write, so use qatomic_read */
-    return qatomic_read(ptr);
-# endif
-#endif
-}
-
-static inline uint64_t tlb_addr_write(const CPUTLBEntry *entry)
-{
-    return tlb_read_idx(entry, MMU_DATA_STORE);
-}
-
-/* Find the TLB index corresponding to the mmu_idx + address pair.  */
-static inline uintptr_t tlb_index(CPUState *cpu, uintptr_t mmu_idx,
-                                  vaddr addr)
-{
-    uintptr_t size_mask = cpu->neg.tlb.f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS;
-
-    return (addr >> TARGET_PAGE_BITS) & size_mask;
-}
-
-/* Find the TLB entry corresponding to the mmu_idx + address pair.  */
-static inline CPUTLBEntry *tlb_entry(CPUState *cpu, uintptr_t mmu_idx,
-                                     vaddr addr)
-{
-    return &cpu->neg.tlb.f[mmu_idx].table[tlb_index(cpu, mmu_idx, addr)];
-}
-
-#endif /* !defined(CONFIG_USER_ONLY) */
-
 #if TARGET_BIG_ENDIAN
 # define cpu_lduw_data        cpu_lduw_be_data
 # define cpu_ldsw_data        cpu_ldsw_be_data
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e16d02a62c..953c437ba9 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -27,6 +27,9 @@
 #include "exec/tb-flush.h"
 #include "exec/memory-internal.h"
 #include "exec/ram_addr.h"
+#include "exec/mmu-access-type.h"
+#include "exec/tlb-common.h"
+#include "exec/vaddr.h"
 #include "tcg/tcg.h"
 #include "qemu/error-report.h"
 #include "exec/log.h"
@@ -95,6 +98,54 @@ static inline size_t sizeof_tlb(CPUTLBDescFast *fast)
     return fast->mask + (1 << CPU_TLB_ENTRY_BITS);
 }
 
+static inline uint64_t tlb_read_idx(const CPUTLBEntry *entry,
+                                    MMUAccessType access_type)
+{
+    /* Do not rearrange the CPUTLBEntry structure members. */
+    QEMU_BUILD_BUG_ON(offsetof(CPUTLBEntry, addr_read) !=
+                      MMU_DATA_LOAD * sizeof(uint64_t));
+    QEMU_BUILD_BUG_ON(offsetof(CPUTLBEntry, addr_write) !=
+                      MMU_DATA_STORE * sizeof(uint64_t));
+    QEMU_BUILD_BUG_ON(offsetof(CPUTLBEntry, addr_code) !=
+                      MMU_INST_FETCH * sizeof(uint64_t));
+
+#if TARGET_LONG_BITS == 32
+    /* Use qatomic_read, in case of addr_write; only care about low bits. */
+    const uint32_t *ptr = (uint32_t *)&entry->addr_idx[access_type];
+    ptr += HOST_BIG_ENDIAN;
+    return qatomic_read(ptr);
+#else
+    const uint64_t *ptr = &entry->addr_idx[access_type];
+# if TCG_OVERSIZED_GUEST
+    return *ptr;
+# else
+    /* ofs might correspond to .addr_write, so use qatomic_read */
+    return qatomic_read(ptr);
+# endif
+#endif
+}
+
+static inline uint64_t tlb_addr_write(const CPUTLBEntry *entry)
+{
+    return tlb_read_idx(entry, MMU_DATA_STORE);
+}
+
+/* Find the TLB index corresponding to the mmu_idx + address pair.  */
+static inline uintptr_t tlb_index(CPUState *cpu, uintptr_t mmu_idx,
+                                  vaddr addr)
+{
+    uintptr_t size_mask = cpu->neg.tlb.f[mmu_idx].mask >> CPU_TLB_ENTRY_BITS;
+
+    return (addr >> TARGET_PAGE_BITS) & size_mask;
+}
+
+/* Find the TLB entry corresponding to the mmu_idx + address pair.  */
+static inline CPUTLBEntry *tlb_entry(CPUState *cpu, uintptr_t mmu_idx,
+                                     vaddr addr)
+{
+    return &cpu->neg.tlb.f[mmu_idx].table[tlb_index(cpu, mmu_idx, addr)];
+}
+
 static void tlb_window_reset(CPUTLBDesc *desc, int64_t ns,
                              size_t max_entries)
 {
-- 
2.41.0

Re: [PATCH 12/24] exec: Move CPUTLBEntry helpers to cputlb.c

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> The following CPUTLBEntry helpers are only used in accel/tcg/cputlb.c:
>    - tlb_index()
>    - tlb_entry()
>    - tlb_read_idx()
>    - tlb_addr_write()
> 
> Move them to this file, allowing to remove the huge "cpu.h" header
> inclusion from "exec/cpu_ldst.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/exec/cpu_ldst.h | 55 -----------------------------------------
>   accel/tcg/cputlb.c      | 51 ++++++++++++++++++++++++++++++++++++++
>   2 files changed, 51 insertions(+), 55 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 13/24] target/sparc: Replace abi_ulong by uint32_t for TARGET_ABI32

[Philippe Mathieu-Daudé]

We have abi_ulong == uint32_t for the 32-bit ABI.
Use the generic type to avoid to depend on the
"exec/user/abitypes.h" header.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sparc/gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/sparc/gdbstub.c b/target/sparc/gdbstub.c
index 07ea81ab5f..ec0036e9ef 100644
--- a/target/sparc/gdbstub.c
+++ b/target/sparc/gdbstub.c
@@ -108,7 +108,7 @@ int sparc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
     SPARCCPU *cpu = SPARC_CPU(cs);
     CPUSPARCState *env = &cpu->env;
 #if defined(TARGET_ABI32)
-    abi_ulong tmp;
+    uint32_t tmp;
 
     tmp = ldl_p(mem_buf);
 #else
-- 
2.41.0

Re: [PATCH 13/24] target/sparc: Replace abi_ulong by uint32_t for TARGET_ABI32

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> We have abi_ulong == uint32_t for the 32-bit ABI.
> Use the generic type to avoid to depend on the
> "exec/user/abitypes.h" header.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   target/sparc/gdbstub.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 14/24] exec: Rename 'exec/user/guest-base.h' as 'user/guest-base.h'

[Philippe Mathieu-Daudé]

The include/user/ directory contains the user-emulation
specific headers. Move guest-base.h there too.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu-all.h               | 2 +-
 include/{exec => }/user/guest-base.h | 4 ++--
 tcg/tcg.c                            | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
 rename include/{exec => }/user/guest-base.h (72%)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 78848f018c..027f19e052 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -65,7 +65,7 @@
 
 #if defined(CONFIG_USER_ONLY)
 #include "exec/user/abitypes.h"
-#include "exec/user/guest-base.h"
+#include "user/guest-base.h"
 
 extern bool have_guest_base;
 
diff --git a/include/exec/user/guest-base.h b/include/user/guest-base.h
similarity index 72%
rename from include/exec/user/guest-base.h
rename to include/user/guest-base.h
index afe2ab7fbb..1e42bca5db 100644
--- a/include/exec/user/guest-base.h
+++ b/include/user/guest-base.h
@@ -4,8 +4,8 @@
  *  Copyright (c) 2003 Fabrice Bellard
  */
 
-#ifndef EXEC_USER_GUEST_BASE_H
-#define EXEC_USER_GUEST_BASE_H
+#ifndef USER_GUEST_BASE_H
+#define USER_GUEST_BASE_H
 
 extern uintptr_t guest_base;
 
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 0c0bb9d169..6a32656cd4 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -57,7 +57,7 @@
 #include "tcg-internal.h"
 #include "tcg/perf.h"
 #ifdef CONFIG_USER_ONLY
-#include "exec/user/guest-base.h"
+#include "user/guest-base.h"
 #endif
 
 /* Forward declarations for functions declared in tcg-target.c.inc and
-- 
2.41.0

Re: [PATCH 14/24] exec: Rename 'exec/user/guest-base.h' as 'user/guest-base.h'

[Anton Johansson via]

On 18/04/24, Philippe Mathieu-Daudé wrote:
> The include/user/ directory contains the user-emulation
> specific headers. Move guest-base.h there too.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/exec/cpu-all.h               | 2 +-
>  include/{exec => }/user/guest-base.h | 4 ++--
>  tcg/tcg.c                            | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>  rename include/{exec => }/user/guest-base.h (72%)

Patch looks good, but the difference between user/ and exec/user is not
entirely clear to me :)

Reviewed-by: Anton Johansson <anjo@rev.ng>

Re: [PATCH 14/24] exec: Rename 'exec/user/guest-base.h' as 'user/guest-base.h'

[Philippe Mathieu-Daudé]

On 25/4/24 09:39, Anton Johansson wrote:
> On 18/04/24, Philippe Mathieu-Daudé wrote:
>> The include/user/ directory contains the user-emulation
>> specific headers. Move guest-base.h there too.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/exec/cpu-all.h               | 2 +-
>>   include/{exec => }/user/guest-base.h | 4 ++--
>>   tcg/tcg.c                            | 2 +-
>>   3 files changed, 4 insertions(+), 4 deletions(-)
>>   rename include/{exec => }/user/guest-base.h (72%)
> 
> Patch looks good, but the difference between user/ and exec/user is not
> entirely clear to me :)

Neither it is for me. In next batch (part 3) all headers are
moved to user/.

> Reviewed-by: Anton Johansson <anjo@rev.ng>

Thanks!

[PATCH 15/24] exec: Restrict inclusion of 'user/guest-base.h'

[Philippe Mathieu-Daudé]

Declare 'have_guest_base' in "user/guest-base.h".

Very few files require this header, so explicitly include
it there instead of "exec/cpu-all.h" which is used in many
source files.

Assert this user-specific header is only included from user
emulation.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20231211212003.21686-23-philmd@linaro.org>
---
 include/exec/cpu-all.h    | 3 ---
 include/exec/cpu_ldst.h   | 2 ++
 include/user/guest-base.h | 6 ++++++
 bsd-user/main.c           | 1 +
 linux-user/elfload.c      | 1 +
 linux-user/main.c         | 1 +
 6 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 027f19e052..e75ec13cd0 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -65,9 +65,6 @@
 
 #if defined(CONFIG_USER_ONLY)
 #include "exec/user/abitypes.h"
-#include "user/guest-base.h"
-
-extern bool have_guest_base;
 
 /*
  * If non-zero, the guest virtual address space is a contiguous subset
diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 2c5a0a5c81..6d47d3114a 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -73,6 +73,8 @@
 
 #if defined(CONFIG_USER_ONLY)
 
+#include "user/guest-base.h"
+
 #ifndef TARGET_TAGGED_ADDRESSES
 static inline abi_ptr cpu_untagged_addr(CPUState *cs, abi_ptr x)
 {
diff --git a/include/user/guest-base.h b/include/user/guest-base.h
index 1e42bca5db..055c1d14fe 100644
--- a/include/user/guest-base.h
+++ b/include/user/guest-base.h
@@ -7,6 +7,12 @@
 #ifndef USER_GUEST_BASE_H
 #define USER_GUEST_BASE_H
 
+#ifndef CONFIG_USER_ONLY
+#error Cannot include this header from system emulation
+#endif
+
 extern uintptr_t guest_base;
 
+extern bool have_guest_base;
+
 #endif
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 01b313756e..29a629d877 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -36,6 +36,7 @@
 #include "qemu/help_option.h"
 #include "qemu/module.h"
 #include "exec/exec-all.h"
+#include "user/guest-base.h"
 #include "tcg/startup.h"
 #include "qemu/timer.h"
 #include "qemu/envlist.h"
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index fc60c01d57..9ed4baa2ac 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -8,6 +8,7 @@
 
 #include "qemu.h"
 #include "user/tswap-target.h"
+#include "user/guest-base.h"
 #include "user-internals.h"
 #include "signal-common.h"
 #include "loader.h"
diff --git a/linux-user/main.c b/linux-user/main.c
index 149e35432e..94e4c47f05 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -38,6 +38,7 @@
 #include "qemu/help_option.h"
 #include "qemu/module.h"
 #include "qemu/plugin.h"
+#include "user/guest-base.h"
 #include "exec/exec-all.h"
 #include "exec/gdbstub.h"
 #include "gdbstub/user.h"
-- 
2.41.0

Re: [PATCH 15/24] exec: Restrict inclusion of 'user/guest-base.h'

[Anton Johansson via]

On 18/04/24, Philippe Mathieu-Daudé wrote:
> Declare 'have_guest_base' in "user/guest-base.h".
> 
> Very few files require this header, so explicitly include
> it there instead of "exec/cpu-all.h" which is used in many
> source files.
> 
> Assert this user-specific header is only included from user
> emulation.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Message-Id: <20231211212003.21686-23-philmd@linaro.org>
> ---
>  include/exec/cpu-all.h    | 3 ---
>  include/exec/cpu_ldst.h   | 2 ++
>  include/user/guest-base.h | 6 ++++++
>  bsd-user/main.c           | 1 +
>  linux-user/elfload.c      | 1 +
>  linux-user/main.c         | 1 +
>  6 files changed, 11 insertions(+), 3 deletions(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

[PATCH 16/24] exec: Move GUEST_ADDR_MAX definition to 'cpu_ldst.h'

[Philippe Mathieu-Daudé]

GUEST_ADDR_MAX is only used in accel/tcg/user-exec.c
and "exec/cpu_ldst.h". Move it from "exec/cpu-all.h"
to "exec/cpu_ldst.h" so we can avoid including
"exec/cpu-all.h" in accel/tcg/user-exec.c.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu-all.h  | 34 +---------------------------------
 include/exec/cpu_ldst.h | 30 ++++++++++++++++++++++++++++++
 accel/tcg/cpu-exec.c    |  1 -
 3 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index e75ec13cd0..554b937ddb 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -63,39 +63,7 @@
 
 /* MMU memory access macros */
 
-#if defined(CONFIG_USER_ONLY)
-#include "exec/user/abitypes.h"
-
-/*
- * If non-zero, the guest virtual address space is a contiguous subset
- * of the host virtual address space, i.e. '-R reserved_va' is in effect
- * either from the command-line or by default.  The value is the last
- * byte of the guest address space e.g. UINT32_MAX.
- *
- * If zero, the host and guest virtual address spaces are intermingled.
- */
-extern unsigned long reserved_va;
-
-/*
- * Limit the guest addresses as best we can.
- *
- * When not using -R reserved_va, we cannot really limit the guest
- * to less address space than the host.  For 32-bit guests, this
- * acts as a sanity check that we're not giving the guest an address
- * that it cannot even represent.  For 64-bit guests... the address
- * might not be what the real kernel would give, but it is at least
- * representable in the guest.
- *
- * TODO: Improve address allocation to avoid this problem, and to
- * avoid setting bits at the top of guest addresses that might need
- * to be used for tags.
- */
-#define GUEST_ADDR_MAX_                                                 \
-    ((MIN_CONST(TARGET_VIRT_ADDR_SPACE_BITS, TARGET_ABI_BITS) <= 32) ?  \
-     UINT32_MAX : ~0ul)
-#define GUEST_ADDR_MAX    (reserved_va ? : GUEST_ADDR_MAX_)
-
-#else
+#if !defined(CONFIG_USER_ONLY)
 
 #include "exec/hwaddr.h"
 
diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 6d47d3114a..07649a01c1 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -73,8 +73,38 @@
 
 #if defined(CONFIG_USER_ONLY)
 
+#include "user/abitypes.h"
 #include "user/guest-base.h"
 
+/*
+ * If non-zero, the guest virtual address space is a contiguous subset
+ * of the host virtual address space, i.e. '-R reserved_va' is in effect
+ * either from the command-line or by default.  The value is the last
+ * byte of the guest address space e.g. UINT32_MAX.
+ *
+ * If zero, the host and guest virtual address spaces are intermingled.
+ */
+extern unsigned long reserved_va;
+
+/*
+ * Limit the guest addresses as best we can.
+ *
+ * When not using -R reserved_va, we cannot really limit the guest
+ * to less address space than the host.  For 32-bit guests, this
+ * acts as a sanity check that we're not giving the guest an address
+ * that it cannot even represent.  For 64-bit guests... the address
+ * might not be what the real kernel would give, but it is at least
+ * representable in the guest.
+ *
+ * TODO: Improve address allocation to avoid this problem, and to
+ * avoid setting bits at the top of guest addresses that might need
+ * to be used for tags.
+ */
+#define GUEST_ADDR_MAX_                                                 \
+    ((MIN_CONST(TARGET_VIRT_ADDR_SPACE_BITS, TARGET_ABI_BITS) <= 32) ?  \
+     UINT32_MAX : ~0ul)
+#define GUEST_ADDR_MAX    (reserved_va ? : GUEST_ADDR_MAX_)
+
 #ifndef TARGET_TAGGED_ADDRESSES
 static inline abi_ptr cpu_untagged_addr(CPUState *cs, abi_ptr x)
 {
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 225e5fbd3e..7bfd558027 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -31,7 +31,6 @@
 #include "exec/log.h"
 #include "qemu/main-loop.h"
 #include "sysemu/cpus.h"
-#include "exec/cpu-all.h"
 #include "sysemu/cpu-timers.h"
 #include "exec/replay-core.h"
 #include "sysemu/tcg.h"
-- 
2.41.0

Re: [PATCH 16/24] exec: Move GUEST_ADDR_MAX definition to 'cpu_ldst.h'

[Anton Johansson via]

On 18/04/24, Philippe Mathieu-Daudé wrote:
> GUEST_ADDR_MAX is only used in accel/tcg/user-exec.c
> and "exec/cpu_ldst.h". Move it from "exec/cpu-all.h"
> to "exec/cpu_ldst.h" so we can avoid including
> "exec/cpu-all.h" in accel/tcg/user-exec.c.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/exec/cpu-all.h  | 34 +---------------------------------
>  include/exec/cpu_ldst.h | 30 ++++++++++++++++++++++++++++++
>  accel/tcg/cpu-exec.c    |  1 -
>  3 files changed, 31 insertions(+), 34 deletions(-)

Reviewed-by: Anton Johansson <anjo@rev.ng>

[PATCH 17/24] exec: Include missing 'qemu/log-for-trace.h' header in 'exec/log.h'

[Philippe Mathieu-Daudé]

"exec/log.h" accesses the qemu_loglevel variable,
which is declared in "qemu/log-for-trace.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/log.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/exec/log.h b/include/exec/log.h
index 4a7375a45f..e0ff778a10 100644
--- a/include/exec/log.h
+++ b/include/exec/log.h
@@ -2,6 +2,7 @@
 #define QEMU_EXEC_LOG_H
 
 #include "qemu/log.h"
+#include "qemu/log-for-trace.h"
 #include "hw/core/cpu.h"
 #include "disas/disas.h"
 
-- 
2.41.0

Re: [PATCH 17/24] exec: Include missing 'qemu/log-for-trace.h' header in 'exec/log.h'

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> "exec/log.h" accesses the qemu_loglevel variable,
> which is declared in "qemu/log-for-trace.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/log.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/include/exec/log.h b/include/exec/log.h
> index 4a7375a45f..e0ff778a10 100644
> --- a/include/exec/log.h
> +++ b/include/exec/log.h
> @@ -2,6 +2,7 @@
>   #define QEMU_EXEC_LOG_H
>   
>   #include "qemu/log.h"
> +#include "qemu/log-for-trace.h"
>   #include "hw/core/cpu.h"
>   #include "disas/disas.h"
>   

I disagree: qemu/log.h is the main file; log-for-trace.h was split out for other usage. 
That shouldn't mean that log-for-trace.h needs to be spread around.


r~

Re: [PATCH 17/24] exec: Include missing 'qemu/log-for-trace.h' header in 'exec/log.h'

[Philippe Mathieu-Daudé]

On 21/4/24 18:44, Richard Henderson wrote:
> On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
>> "exec/log.h" accesses the qemu_loglevel variable,
>> which is declared in "qemu/log-for-trace.h".
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/exec/log.h | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/include/exec/log.h b/include/exec/log.h
>> index 4a7375a45f..e0ff778a10 100644
>> --- a/include/exec/log.h
>> +++ b/include/exec/log.h
>> @@ -2,6 +2,7 @@
>>   #define QEMU_EXEC_LOG_H
>>   #include "qemu/log.h"
>> +#include "qemu/log-for-trace.h"
>>   #include "hw/core/cpu.h"
>>   #include "disas/disas.h"
> 
> I disagree: qemu/log.h is the main file; log-for-trace.h was split out 
> for other usage. That shouldn't mean that log-for-trace.h needs to be 
> spread around.

Good point, I haven't noticed we can just move the qemu_loglevel
declaration.

Re: [PATCH 17/24] exec: Include missing 'qemu/log-for-trace.h' header in 'exec/log.h'

[Philippe Mathieu-Daudé]

On 22/4/24 11:05, Philippe Mathieu-Daudé wrote:
> On 21/4/24 18:44, Richard Henderson wrote:
>> On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
>>> "exec/log.h" accesses the qemu_loglevel variable,
>>> which is declared in "qemu/log-for-trace.h".
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   include/exec/log.h | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/exec/log.h b/include/exec/log.h
>>> index 4a7375a45f..e0ff778a10 100644
>>> --- a/include/exec/log.h
>>> +++ b/include/exec/log.h
>>> @@ -2,6 +2,7 @@
>>>   #define QEMU_EXEC_LOG_H
>>>   #include "qemu/log.h"
>>> +#include "qemu/log-for-trace.h"
>>>   #include "hw/core/cpu.h"
>>>   #include "disas/disas.h"
>>
>> I disagree: qemu/log.h is the main file; log-for-trace.h was split out 
>> for other usage. That shouldn't mean that log-for-trace.h needs to be 
>> spread around.
> 
> Good point, I haven't noticed we can just move the qemu_loglevel
> declaration.

Now I'm confused, I don't remember why I ended doing that, since
"exec/log.h" includes "qemu/log.h" itself including
"qemu/log-for-trace.h.

I now notice commit be0aa7ac89 ("log-for-trace.h: Split out
parts of log.h used by trace.h"):

     A persistent build problem we see is where a source file
     accidentally omits the #include of log.h. This slips through
     local developer testing because if you configure with the
     default (log) trace backend trace.h will pull in log.h for you.
     Compilation fails only if some other backend is selected.

     To make this error cause a compile failure regardless of
     the configured trace backend, split out the parts of log.h
     that trace.h requires into a new log-for-trace.h header.
     Since almost all manual uses of the log.h functions will
     use constants or functions which aren't in log-for-trace.h,
     this will let us catch missing #include "qemu/log.h" more
     consistently.

Let's drop this patch.

Thanks,

Phil.

[PATCH 18/24] plugins: Include missing 'qemu/bitmap.h' header

[Philippe Mathieu-Daudé]

"qemu/plugin.h" uses DECLARE_BITMAP(), which is
declared in "qemu/bitmap.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/plugin.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 12a96cea2a..41db748eda 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -13,6 +13,7 @@
 #include "qemu/queue.h"
 #include "qemu/option.h"
 #include "qemu/plugin-event.h"
+#include "qemu/bitmap.h"
 #include "exec/memopidx.h"
 #include "hw/core/cpu.h"
 
-- 
2.41.0

Re: [PATCH 18/24] plugins: Include missing 'qemu/bitmap.h' header

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> "qemu/plugin.h" uses DECLARE_BITMAP(), which is
> declared in "qemu/bitmap.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/qemu/plugin.h | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 18/24] plugins: Include missing 'qemu/bitmap.h' header

[Pierrick Bouvier]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> "qemu/plugin.h" uses DECLARE_BITMAP(), which is
> declared in "qemu/bitmap.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/qemu/plugin.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index 12a96cea2a..41db748eda 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -13,6 +13,7 @@
>   #include "qemu/queue.h"
>   #include "qemu/option.h"
>   #include "qemu/plugin-event.h"
> +#include "qemu/bitmap.h"
>   #include "exec/memopidx.h"
>   #include "hw/core/cpu.h"
>   

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

[PATCH 19/24] gdbstub: Avoid including 'cpu.h' in 'gdbstub/helpers.h'

[Philippe Mathieu-Daudé]

We only need the "exec/tswap.h" and "cpu-param.h" headers.
Only include "cpu.h" in the target gdbstub.c source files.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/gdbstub/helpers.h | 3 ++-
 target/avr/gdbstub.c      | 1 +
 target/tricore/gdbstub.c  | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/gdbstub/helpers.h b/include/gdbstub/helpers.h
index 6277a858a1..26140ef1ac 100644
--- a/include/gdbstub/helpers.h
+++ b/include/gdbstub/helpers.h
@@ -16,7 +16,8 @@
 #error "gdbstub helpers should only be included by target specific code"
 #endif
 
-#include "cpu.h"
+#include "exec/tswap.h"
+#include "cpu-param.h"
 
 /*
  * The GDB remote protocol transfers values in target byte order. As
diff --git a/target/avr/gdbstub.c b/target/avr/gdbstub.c
index 2eeee2bf4e..d6d3c1479b 100644
--- a/target/avr/gdbstub.c
+++ b/target/avr/gdbstub.c
@@ -20,6 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "gdbstub/helpers.h"
+#include "cpu.h"
 
 int avr_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
 {
diff --git a/target/tricore/gdbstub.c b/target/tricore/gdbstub.c
index f9309c5e27..29a70051ff 100644
--- a/target/tricore/gdbstub.c
+++ b/target/tricore/gdbstub.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "gdbstub/helpers.h"
+#include "cpu.h"
 
 
 #define LCX_REGNUM         32
-- 
2.41.0

Re: [PATCH 19/24] gdbstub: Avoid including 'cpu.h' in 'gdbstub/helpers.h'

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> We only need the "exec/tswap.h" and "cpu-param.h" headers.
> Only include "cpu.h" in the target gdbstub.c source files.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/gdbstub/helpers.h | 3 ++-
>   target/avr/gdbstub.c      | 1 +
>   target/tricore/gdbstub.c  | 1 +
>   3 files changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 20/24] hw/xtensa: Include missing 'exec/cpu-common.h' in 'bootparam.h'

[Philippe Mathieu-Daudé]

cpu_physical_memory_write() is declared in "exec/cpu-common.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/xtensa/bootparam.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/xtensa/bootparam.h b/hw/xtensa/bootparam.h
index ade7891ec5..f57ff850bc 100644
--- a/hw/xtensa/bootparam.h
+++ b/hw/xtensa/bootparam.h
@@ -1,6 +1,8 @@
 #ifndef HW_XTENSA_BOOTPARAM_H
 #define HW_XTENSA_BOOTPARAM_H
 
+#include "exec/cpu-common.h"
+
 #define BP_TAG_COMMAND_LINE     0x1001  /* command line (0-terminated string)*/
 #define BP_TAG_INITRD           0x1002  /* ramdisk addr and size (bp_meminfo) */
 #define BP_TAG_MEMORY           0x1003  /* memory addr and size (bp_meminfo) */
-- 
2.41.0

Re: [PATCH 20/24] hw/xtensa: Include missing 'exec/cpu-common.h' in 'bootparam.h'

[Anton Johansson via]

On 18/04/24, Philippe Mathieu-Daudé wrote:
> cpu_physical_memory_write() is declared in "exec/cpu-common.h".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/xtensa/bootparam.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/xtensa/bootparam.h b/hw/xtensa/bootparam.h
> index ade7891ec5..f57ff850bc 100644
> --- a/hw/xtensa/bootparam.h
> +++ b/hw/xtensa/bootparam.h
> @@ -1,6 +1,8 @@
>  #ifndef HW_XTENSA_BOOTPARAM_H
>  #define HW_XTENSA_BOOTPARAM_H
>  
> +#include "exec/cpu-common.h"
> +
>  #define BP_TAG_COMMAND_LINE     0x1001  /* command line (0-terminated string)*/
>  #define BP_TAG_INITRD           0x1002  /* ramdisk addr and size (bp_meminfo) */
>  #define BP_TAG_MEMORY           0x1003  /* memory addr and size (bp_meminfo) */
> -- 
> 2.41.0
> 

Reviewed-by: Anton Johansson <anjo@rev.ng>

[PATCH 21/24] plugins: Un-inline qemu_plugin_disable_mem_helpers()

[Philippe Mathieu-Daudé]

"qemu/plugin.h" only include the huge "hw/core/cpu.h"
because qemu_plugin_disable_mem_helpers() accesses
CPUState::plugin_mem_cbs. In order to avoid including
it, un-inline qemu_plugin_disable_mem_helpers().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/qemu/plugin.h | 6 +-----
 plugins/core.c        | 5 +++++
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index 41db748eda..28467c4a28 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -15,7 +15,6 @@
 #include "qemu/plugin-event.h"
 #include "qemu/bitmap.h"
 #include "exec/memopidx.h"
-#include "hw/core/cpu.h"
 
 /*
  * Option parsing/processing.
@@ -227,10 +226,7 @@ void qemu_plugin_atexit_cb(void);
 
 void qemu_plugin_add_dyn_cb_arr(GArray *arr);
 
-static inline void qemu_plugin_disable_mem_helpers(CPUState *cpu)
-{
-    cpu->plugin_mem_cbs = NULL;
-}
+void qemu_plugin_disable_mem_helpers(CPUState *cpu);
 
 /**
  * qemu_plugin_user_exit(): clean-up callbacks before calling exit callbacks
diff --git a/plugins/core.c b/plugins/core.c
index 11ca20e626..b25092abb5 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -493,6 +493,11 @@ void exec_inline_op(struct qemu_plugin_dyn_cb *cb, int cpu_index)
     }
 }
 
+void qemu_plugin_disable_mem_helpers(CPUState *cpu)
+{
+    cpu->plugin_mem_cbs = NULL;
+}
+
 void qemu_plugin_vcpu_mem_cb(CPUState *cpu, uint64_t vaddr,
                              MemOpIdx oi, enum qemu_plugin_mem_rw rw)
 {
-- 
2.41.0

Re: [PATCH 21/24] plugins: Un-inline qemu_plugin_disable_mem_helpers()

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> "qemu/plugin.h" only include the huge "hw/core/cpu.h"
> because qemu_plugin_disable_mem_helpers() accesses
> CPUState::plugin_mem_cbs. In order to avoid including
> it, un-inline qemu_plugin_disable_mem_helpers().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/qemu/plugin.h | 6 +-----
>   plugins/core.c        | 5 +++++
>   2 files changed, 6 insertions(+), 5 deletions(-)


I don't see how this helps.
All of the places that use qemu_plugin_disable_mem_helpers also have cpu.h.


r~

[PATCH 22/24] exec: Remove 'exec/tswap.h' from 'exec/cpu-all.h'

[Philippe Mathieu-Daudé]

"exec/cpu-all.h" doesn't require "exec/tswap.h". Remove it,
including it in the sources when required.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/xtensa/bootparam.h       | 1 +
 include/exec/cpu-all.h      | 1 -
 accel/tcg/translator.c      | 1 +
 hw/arm/boot.c               | 1 +
 hw/arm/npcm7xx.c            | 1 +
 hw/mips/fuloong2e.c         | 1 +
 hw/mips/malta.c             | 1 +
 hw/ppc/sam460ex.c           | 1 +
 hw/ppc/spapr.c              | 1 +
 hw/ppc/virtex_ml507.c       | 1 +
 hw/sh4/r2d.c                | 1 +
 target/arm/gdbstub.c        | 1 +
 target/xtensa/xtensa-semi.c | 1 +
 13 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/xtensa/bootparam.h b/hw/xtensa/bootparam.h
index f57ff850bc..e1d47b503c 100644
--- a/hw/xtensa/bootparam.h
+++ b/hw/xtensa/bootparam.h
@@ -1,6 +1,7 @@
 #ifndef HW_XTENSA_BOOTPARAM_H
 #define HW_XTENSA_BOOTPARAM_H
 
+#include "exec/tswap.h"
 #include "exec/cpu-common.h"
 
 #define BP_TAG_COMMAND_LINE     0x1001  /* command line (0-terminated string)*/
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 554b937ddb..cfbf51822c 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -21,7 +21,6 @@
 
 #include "exec/cpu-common.h"
 #include "exec/memory.h"
-#include "exec/tswap.h"
 #include "hw/core/cpu.h"
 
 /* some important defines:
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 6832e55135..85950377d9 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -12,6 +12,7 @@
 #include "qemu/error-report.h"
 #include "exec/exec-all.h"
 #include "exec/translator.h"
+#include "exec/tswap.h"
 #include "exec/cpu_ldst.h"
 #include "exec/plugin-gen.h"
 #include "tcg/tcg-op-common.h"
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 84ea6a807a..93945a1a15 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -22,6 +22,7 @@
 #include "sysemu/reset.h"
 #include "hw/loader.h"
 #include "elf.h"
+#include "exec/tswap.h"
 #include "sysemu/device_tree.h"
 #include "qemu/config-file.h"
 #include "qemu/option.h"
diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
index cc68b5d8f1..1ef303415b 100644
--- a/hw/arm/npcm7xx.c
+++ b/hw/arm/npcm7xx.c
@@ -27,6 +27,7 @@
 #include "qemu/units.h"
 #include "sysemu/sysemu.h"
 #include "target/arm/cpu-qom.h"
+#include "exec/tswap.h"
 
 /*
  * This covers the whole MMIO space. We'll use this to catch any MMIO accesses
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index a45aac368c..1d0613a76f 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -40,6 +40,7 @@
 #include "sysemu/reset.h"
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
+#include "exec/tswap.h"
 
 #define ENVP_PADDR              0x2000
 #define ENVP_VADDR              cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index af74008c82..3dca0f100c 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -56,6 +56,7 @@
 #include "semihosting/semihost.h"
 #include "hw/mips/cps.h"
 #include "hw/qdev-clock.h"
+#include "exec/tswap.h"
 #include "target/mips/internal.h"
 #include "trace.h"
 #include "cpu.h"
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index d42b677898..abc02f0817 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -24,6 +24,7 @@
 #include "hw/loader.h"
 #include "elf.h"
 #include "exec/memory.h"
+#include "exec/tswap.h"
 #include "ppc440.h"
 #include "hw/pci-host/ppc4xx.h"
 #include "hw/block/flash.h"
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e9bc97fee0..b4b1f43983 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -74,6 +74,7 @@
 #include "hw/virtio/virtio-scsi.h"
 #include "hw/virtio/vhost-scsi-common.h"
 
+#include "exec/tswap.h"
 #include "exec/ram_addr.h"
 #include "hw/usb.h"
 #include "qemu/config-file.h"
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index d02f330650..fd23afebf5 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -38,6 +38,7 @@
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/option.h"
+#include "exec/tswap.h"
 
 #include "hw/intc/ppc-uic.h"
 #include "hw/ppc/ppc.h"
diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index e5ac6751bd..5f4420f534 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -43,6 +43,7 @@
 #include "hw/loader.h"
 #include "hw/usb.h"
 #include "hw/block/flash.h"
+#include "exec/tswap.h"
 
 #define FLASH_BASE 0x00000000
 #define FLASH_SIZE (16 * MiB)
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index a3bb73cfa7..f2b001afdd 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -20,6 +20,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/gdbstub.h"
+#include "exec/tswap.h"
 #include "gdbstub/helpers.h"
 #include "sysemu/tcg.h"
 #include "internals.h"
diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
index fa21b7e11f..dbc42d1587 100644
--- a/target/xtensa/xtensa-semi.c
+++ b/target/xtensa/xtensa-semi.c
@@ -29,6 +29,7 @@
 #include "cpu.h"
 #include "chardev/char-fe.h"
 #include "exec/helper-proto.h"
+#include "exec/tswap.h"
 #include "semihosting/semihost.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
-- 
2.41.0

Re: [PATCH 22/24] exec: Remove 'exec/tswap.h' from 'exec/cpu-all.h'

[Harsh Prateek Bora]

On 4/19/24 00:55, Philippe Mathieu-Daudé wrote:
> "exec/cpu-all.h" doesn't require "exec/tswap.h". Remove it,
> including it in the sources when required.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/xtensa/bootparam.h       | 1 +
>   include/exec/cpu-all.h      | 1 -
>   accel/tcg/translator.c      | 1 +
>   hw/arm/boot.c               | 1 +
>   hw/arm/npcm7xx.c            | 1 +
>   hw/mips/fuloong2e.c         | 1 +
>   hw/mips/malta.c             | 1 +
>   hw/ppc/sam460ex.c           | 1 +
>   hw/ppc/spapr.c              | 1 +

For spapr:
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

>   hw/ppc/virtex_ml507.c       | 1 +
>   hw/sh4/r2d.c                | 1 +
>   target/arm/gdbstub.c        | 1 +
>   target/xtensa/xtensa-semi.c | 1 +
>   13 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xtensa/bootparam.h b/hw/xtensa/bootparam.h
> index f57ff850bc..e1d47b503c 100644
> --- a/hw/xtensa/bootparam.h
> +++ b/hw/xtensa/bootparam.h
> @@ -1,6 +1,7 @@
>   #ifndef HW_XTENSA_BOOTPARAM_H
>   #define HW_XTENSA_BOOTPARAM_H
>   
> +#include "exec/tswap.h"
>   #include "exec/cpu-common.h"
>   
>   #define BP_TAG_COMMAND_LINE     0x1001  /* command line (0-terminated string)*/
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 554b937ddb..cfbf51822c 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -21,7 +21,6 @@
>   
>   #include "exec/cpu-common.h"
>   #include "exec/memory.h"
> -#include "exec/tswap.h"
>   #include "hw/core/cpu.h"
>   
>   /* some important defines:
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 6832e55135..85950377d9 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -12,6 +12,7 @@
>   #include "qemu/error-report.h"
>   #include "exec/exec-all.h"
>   #include "exec/translator.h"
> +#include "exec/tswap.h"
>   #include "exec/cpu_ldst.h"
>   #include "exec/plugin-gen.h"
>   #include "tcg/tcg-op-common.h"
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 84ea6a807a..93945a1a15 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -22,6 +22,7 @@
>   #include "sysemu/reset.h"
>   #include "hw/loader.h"
>   #include "elf.h"
> +#include "exec/tswap.h"
>   #include "sysemu/device_tree.h"
>   #include "qemu/config-file.h"
>   #include "qemu/option.h"
> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> index cc68b5d8f1..1ef303415b 100644
> --- a/hw/arm/npcm7xx.c
> +++ b/hw/arm/npcm7xx.c
> @@ -27,6 +27,7 @@
>   #include "qemu/units.h"
>   #include "sysemu/sysemu.h"
>   #include "target/arm/cpu-qom.h"
> +#include "exec/tswap.h"
>   
>   /*
>    * This covers the whole MMIO space. We'll use this to catch any MMIO accesses
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index a45aac368c..1d0613a76f 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -40,6 +40,7 @@
>   #include "sysemu/reset.h"
>   #include "sysemu/sysemu.h"
>   #include "qemu/error-report.h"
> +#include "exec/tswap.h"
>   
>   #define ENVP_PADDR              0x2000
>   #define ENVP_VADDR              cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index af74008c82..3dca0f100c 100644
> --- a/hw/mips/malta.c
> +++ b/hw/mips/malta.c
> @@ -56,6 +56,7 @@
>   #include "semihosting/semihost.h"
>   #include "hw/mips/cps.h"
>   #include "hw/qdev-clock.h"
> +#include "exec/tswap.h"
>   #include "target/mips/internal.h"
>   #include "trace.h"
>   #include "cpu.h"
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index d42b677898..abc02f0817 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -24,6 +24,7 @@
>   #include "hw/loader.h"
>   #include "elf.h"
>   #include "exec/memory.h"
> +#include "exec/tswap.h"
>   #include "ppc440.h"
>   #include "hw/pci-host/ppc4xx.h"
>   #include "hw/block/flash.h"
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index e9bc97fee0..b4b1f43983 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -74,6 +74,7 @@
>   #include "hw/virtio/virtio-scsi.h"
>   #include "hw/virtio/vhost-scsi-common.h"
>   
> +#include "exec/tswap.h"
>   #include "exec/ram_addr.h"
>   #include "hw/usb.h"
>   #include "qemu/config-file.h"
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index d02f330650..fd23afebf5 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -38,6 +38,7 @@
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
>   #include "qemu/option.h"
> +#include "exec/tswap.h"
>   
>   #include "hw/intc/ppc-uic.h"
>   #include "hw/ppc/ppc.h"
> diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
> index e5ac6751bd..5f4420f534 100644
> --- a/hw/sh4/r2d.c
> +++ b/hw/sh4/r2d.c
> @@ -43,6 +43,7 @@
>   #include "hw/loader.h"
>   #include "hw/usb.h"
>   #include "hw/block/flash.h"
> +#include "exec/tswap.h"
>   
>   #define FLASH_BASE 0x00000000
>   #define FLASH_SIZE (16 * MiB)
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index a3bb73cfa7..f2b001afdd 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -20,6 +20,7 @@
>   #include "qemu/osdep.h"
>   #include "cpu.h"
>   #include "exec/gdbstub.h"
> +#include "exec/tswap.h"
>   #include "gdbstub/helpers.h"
>   #include "sysemu/tcg.h"
>   #include "internals.h"
> diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
> index fa21b7e11f..dbc42d1587 100644
> --- a/target/xtensa/xtensa-semi.c
> +++ b/target/xtensa/xtensa-semi.c
> @@ -29,6 +29,7 @@
>   #include "cpu.h"
>   #include "chardev/char-fe.h"
>   #include "exec/helper-proto.h"
> +#include "exec/tswap.h"
>   #include "semihosting/semihost.h"
>   #include "qapi/error.h"
>   #include "qemu/log.h"

[PATCH 23/24] exec: Remove 'disas/disas.h' from 'exec/log.h'

[Philippe Mathieu-Daudé]

"exec/log.h" doesn't require "disas/disas.h". Remove it,
including it in the sources when required.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/log.h               | 1 -
 target/avr/translate.c           | 1 +
 target/hexagon/translate.c       | 1 +
 target/loongarch/tcg/translate.c | 1 +
 target/rx/translate.c            | 1 +
 tcg/tcg.c                        | 1 +
 6 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/exec/log.h b/include/exec/log.h
index e0ff778a10..ee33981aab 100644
--- a/include/exec/log.h
+++ b/include/exec/log.h
@@ -4,7 +4,6 @@
 #include "qemu/log.h"
 #include "qemu/log-for-trace.h"
 #include "hw/core/cpu.h"
-#include "disas/disas.h"
 
 /* cpu_dump_state() logging functions: */
 /**
diff --git a/target/avr/translate.c b/target/avr/translate.c
index 87e2bd5ef1..b2005f3cbe 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -29,6 +29,7 @@
 #include "exec/helper-gen.h"
 #include "exec/log.h"
 #include "exec/translator.h"
+#include "disas/disas.h"
 
 #define HELPER_H "helper.h"
 #include "exec/helper-info.c.inc"
diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 47a870f42d..6bec38404e 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -25,6 +25,7 @@
 #include "exec/translation-block.h"
 #include "exec/cpu_ldst.h"
 #include "exec/log.h"
+#include "disas/disas.h"
 #include "internal.h"
 #include "attribs.h"
 #include "insn.h"
diff --git a/target/loongarch/tcg/translate.c b/target/loongarch/tcg/translate.c
index 7567712655..ae87a68c69 100644
--- a/target/loongarch/tcg/translate.c
+++ b/target/loongarch/tcg/translate.c
@@ -14,6 +14,7 @@
 #include "exec/helper-proto.h"
 #include "exec/helper-gen.h"
 #include "exec/log.h"
+#include "disas/disas.h"
 #include "qemu/qemu-print.h"
 #include "fpu/softfloat.h"
 #include "translate.h"
diff --git a/target/rx/translate.c b/target/rx/translate.c
index f6e9e0ec90..4259ad6d74 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -27,6 +27,7 @@
 #include "exec/helper-gen.h"
 #include "exec/translator.h"
 #include "exec/log.h"
+#include "disas/disas.h"
 
 #define HELPER_H "helper.h"
 #include "exec/helper-info.c.inc"
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 6a32656cd4..e7e0ab6846 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -51,6 +51,7 @@
 #endif
 
 #include "elf.h"
+#include "disas/disas.h"
 #include "exec/log.h"
 #include "tcg/tcg-ldst.h"
 #include "tcg/tcg-temp-internal.h"
-- 
2.41.0

Re: [PATCH 23/24] exec: Remove 'disas/disas.h' from 'exec/log.h'

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> "exec/log.h" doesn't require "disas/disas.h". Remove it,
> including it in the sources when required.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/log.h               | 1 -
>   target/avr/translate.c           | 1 +
>   target/hexagon/translate.c       | 1 +
>   target/loongarch/tcg/translate.c | 1 +
>   target/rx/translate.c            | 1 +
>   tcg/tcg.c                        | 1 +
>   6 files changed, 5 insertions(+), 1 deletion(-)

Most of these additions are obviated by
https://patchew.org/QEMU/20240405102459.462551-1-richard.henderson@linaro.org/


r~

[PATCH 24/24] exec: Remove unnecessary inclusions of 'hw/core/cpu.h'

[Philippe Mathieu-Daudé]

When "hw/core/cpu.h" is not required, remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/cpu-all.h    | 1 -
 include/exec/cpu-defs.h   | 1 -
 include/hw/boards.h       | 1 -
 include/hw/ppc/openpic.h  | 1 -
 include/sysemu/hw_accel.h | 1 -
 5 files changed, 5 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index cfbf51822c..03963442a0 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -21,7 +21,6 @@
 
 #include "exec/cpu-common.h"
 #include "exec/memory.h"
-#include "hw/core/cpu.h"
 
 /* some important defines:
  *
diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h
index 0dbef3010c..2b88db71a8 100644
--- a/include/exec/cpu-defs.h
+++ b/include/exec/cpu-defs.h
@@ -29,7 +29,6 @@
 #include "exec/hwaddr.h"
 #endif
 #include "exec/memattrs.h"
-#include "hw/core/cpu.h"
 
 #include "cpu-param.h"
 
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 8b8f6d5c00..2daa23082f 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -9,7 +9,6 @@
 #include "qapi/qapi-types-machine.h"
 #include "qemu/module.h"
 #include "qom/object.h"
-#include "hw/core/cpu.h"
 
 #define TYPE_MACHINE_SUFFIX "-machine"
 
diff --git a/include/hw/ppc/openpic.h b/include/hw/ppc/openpic.h
index 9c6af8e207..8fe3a5902f 100644
--- a/include/hw/ppc/openpic.h
+++ b/include/hw/ppc/openpic.h
@@ -2,7 +2,6 @@
 #define OPENPIC_H
 
 #include "hw/sysbus.h"
-#include "hw/core/cpu.h"
 #include "qom/object.h"
 
 #define MAX_CPU     32
diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h
index c71b77e71f..6aca3c6178 100644
--- a/include/sysemu/hw_accel.h
+++ b/include/sysemu/hw_accel.h
@@ -11,7 +11,6 @@
 #ifndef QEMU_HW_ACCEL_H
 #define QEMU_HW_ACCEL_H
 
-#include "hw/core/cpu.h"
 #include "sysemu/kvm.h"
 #include "sysemu/hvf.h"
 #include "sysemu/whpx.h"
-- 
2.41.0

Re: [PATCH 24/24] exec: Remove unnecessary inclusions of 'hw/core/cpu.h'

[Richard Henderson]

On 4/18/24 12:25, Philippe Mathieu-Daudé wrote:
> When "hw/core/cpu.h" is not required, remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   include/exec/cpu-all.h    | 1 -
>   include/exec/cpu-defs.h   | 1 -
>   include/hw/boards.h       | 1 -
>   include/hw/ppc/openpic.h  | 1 -
>   include/sysemu/hw_accel.h | 1 -
>   5 files changed, 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 00/24] include/exec: Rework (part 2)

[Philippe Mathieu-Daudé]

On 18/4/24 21:24, Philippe Mathieu-Daudé wrote:
> Reordering exec/, second round. Mostly splitting:
> - TCG LDST API (to "exec/cpu_ldst.h")
> - CPU TLB APIs (to "exec/cputlb.h")
> from "exec/cpu-all.h" and "exec/exec-all.h", and
> reducing inclusions of "hw/core/cpu.h".
> 
> Based-on: <20240417182806.69446-1-philmd@linaro.org>
> or https://gitlab.com/philmd/qemu/-/commits/exec-next

And this series is available as
https://gitlab.com/philmd/qemu/-/commits/exec-rework-part2/

> Philippe Mathieu-Daudé (24):
>    exec: Declare MMUAccessType type in 'mmu-access-type.h' header
>    exec: Declare CPUBreakpoint/CPUWatchpoint type in 'breakpoint.h'
>      header
>    hw/core: Avoid including the full 'hw/core/cpu.h' in 'tcg-cpu-ops.h'
>    exec: Restrict TCG specific declarations of 'cputlb.h'
>    exec: Restrict 'cpu_ldst.h' to TCG accelerator
>    exec: Have guest_addr_valid() methods take abi_ptr/size_t arguments
>    exec: Un-inline tlb_vaddr_to_host() and declare it in 'exec/cputlb.h'
>    physmem: Move TCG CPU IOTLB methods around
>    physmem: Restrict TCG CPU IOTLB code to TCG accel
>    exec: Reduce tlb_set_dirty() declaration scope
>    exec: Move tlb_reset_dirty*() declarations to 'exec/cputlb.h'
>    exec: Move CPUTLBEntry helpers to cputlb.c
>    target/sparc: Replace abi_ulong by uint32_t for TARGET_ABI32
>    exec: Rename 'exec/user/guest-base.h' as 'user/guest-base.h'
>    exec: Restrict inclusion of 'user/guest-base.h'
>    exec: Move GUEST_ADDR_MAX definition to 'cpu_ldst.h'
>    exec: Include missing 'qemu/log-for-trace.h' header in 'exec/log.h'
>    plugins: Include missing 'qemu/bitmap.h' header
>    gdbstub: Avoid including 'cpu.h' in 'gdbstub/helpers.h'
>    hw/xtensa: Include missing 'exec/cpu-common.h' in 'bootparam.h'
>    plugins: Un-inline qemu_plugin_disable_mem_helpers()
>    exec: Remove 'exec/tswap.h' from 'exec/cpu-all.h'
>    exec: Remove 'disas/disas.h' from 'exec/log.h'
>    exec: Remove unnecessary inclusions of 'hw/core/cpu.h'