Re: [PATCH 15/24] accel/tcg: Restrict IcountDecr and CPUTLB to TCG

[Philippe Mathieu-Daudé]

On 29/4/24 00:14, Philippe Mathieu-Daudé wrote:
> IcountDecr union, the CPUTLB* structures and the
> "exec/tlb-common.h" header are only required for
> TCG.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/tlb-common.h | 4 ++++
>   include/hw/core/cpu.h     | 9 ++++++---
>   2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/include/exec/tlb-common.h b/include/exec/tlb-common.h
> index dc5a5faa0b..a529c9f056 100644
> --- a/include/exec/tlb-common.h
> +++ b/include/exec/tlb-common.h
> @@ -19,6 +19,10 @@
>   #ifndef EXEC_TLB_COMMON_H
>   #define EXEC_TLB_COMMON_H 1
>   
> +#ifndef CONFIG_TCG
> +#error Can only include this header with TCG
> +#endif
> +
>   #define CPU_TLB_ENTRY_BITS 5
>   
>   /* Minimalized TLB entry for use by TCG fast path. */
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index ef8b85b6fe..dc28920bcc 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -27,7 +27,6 @@
>   #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"
>   #include "qemu/rcu_queue.h"
> @@ -256,6 +255,9 @@ typedef struct CPUTLBEntryFull {
>       } extra;
>   } CPUTLBEntryFull;
>   
> +#ifdef CONFIG_TCG
> +#include "exec/tlb-common.h"
> +
>   /*
>    * Data elements that are per MMU mode, minus the bits accessed by
>    * the TCG fast path.
> @@ -311,11 +313,9 @@ typedef struct CPUTLBCommon {
>    * negative offsets are at the end of the struct.
>    */
>   typedef struct CPUTLB {
> -#ifdef CONFIG_TCG
>       CPUTLBCommon c;
>       CPUTLBDesc d[NB_MMU_MODES];
>       CPUTLBDescFast f[NB_MMU_MODES];
> -#endif
>   } CPUTLB;
>   
>   /*
> @@ -337,6 +337,7 @@ typedef union IcountDecr {
>   #endif
>       } u16;
>   } IcountDecr;
> +#endif
>   
>   /**
>    * CPUNegativeOffsetState: Elements of CPUState most efficiently accessed
> @@ -346,6 +347,7 @@ typedef union IcountDecr {
>    * @plugin_state: per-CPU plugin state
>    */
>   typedef struct CPUNegativeOffsetState {
> +#ifdef CONFIG_TCG
>       CPUTLB tlb;
>   #ifdef CONFIG_PLUGIN
>       /*
> @@ -356,6 +358,7 @@ typedef struct CPUNegativeOffsetState {
>   #endif
>       IcountDecr icount_decr;
>       bool can_do_io;
> +#endif
>   } CPUNegativeOffsetState;

We also need:

-- >8 --
@@ -1110,6 +1110,7 @@

+#ifdef CONFIG_TCG
 /**
  * cpu_plugin_mem_cbs_enabled() - are plugin memory callbacks enabled?
  * @cs: CPUState pointer
@@ -1126,6 +1127,7 @@ static inline bool 
cpu_plugin_mem_cbs_enabled(const CPUState *cpu)
     return false;
 #endif
 }
+#endif

---

Otherwise this inlined function fails to build when TCG is disabled...

But since it is only used from accel/tcg/, I'll move it to
accel/tcg/internal-common.h in a preliminary patch instead:

-- >8 --
Author: Philippe Mathieu-Daudé <philmd@linaro.org>
Date:   Mon Apr 29 16:01:18 2024 +0200

    accel/tcg: Restrict cpu_plugin_mem_cbs_enabled() to TCG

    So far cpu_plugin_mem_cbs_enabled() is only called from
    TCG, so reduce it to accel/tcg/.

    Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

diff --git a/accel/tcg/internal-common.h b/accel/tcg/internal-common.h
index df317e7496..867426500f 100644
--- a/accel/tcg/internal-common.h
+++ b/accel/tcg/internal-common.h
@@ -26,0 +27,17 @@ static inline bool cpu_in_serial_context(CPUState *cs)
+/**
+ * cpu_plugin_mem_cbs_enabled() - are plugin memory callbacks enabled?
+ * @cs: CPUState pointer
+ *
+ * The memory callbacks are installed if a plugin has instrumented an
+ * instruction for memory. This can be useful to know if you want to
+ * force a slow path for a series of memory accesses.
+ */
+static inline bool cpu_plugin_mem_cbs_enabled(const CPUState *cpu)
+{
+#ifdef CONFIG_PLUGIN
+    return !!cpu->neg.plugin_mem_cbs;
+#else
+    return false;
+#endif
+}
+
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index ef8b85b6fe..24ad52af7d 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1110,17 +1109,0 @@ void cpu_watchpoint_remove_all(CPUState *cpu, int 
mask);
-/**
- * cpu_plugin_mem_cbs_enabled() - are plugin memory callbacks enabled?
- * @cs: CPUState pointer
- *
- * The memory callbacks are installed if a plugin has instrumented an
- * instruction for memory. This can be useful to know if you want to
- * force a slow path for a series of memory accesses.
- */
-static inline bool cpu_plugin_mem_cbs_enabled(const CPUState *cpu)
-{
-#ifdef CONFIG_PLUGIN
-    return !!cpu->neg.plugin_mem_cbs;
-#else
-    return false;
-#endif
-}
-
---