diff mbox series

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

Message ID 20240428221450.26460-16-philmd@linaro.org (mailing list archive)
State New, archived
Headers show
Series exec: Rework around CPUState user fields (part 2) | expand

Commit Message

Philippe Mathieu-Daudé April 28, 2024, 10:14 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé April 29, 2024, 2:03 p.m. UTC | #1
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
-}
-
---
Richard Henderson April 29, 2024, 2:46 p.m. UTC | #2
On 4/29/24 07:03, Philippe Mathieu-Daudé wrote:
> 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>

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

r~
Richard Henderson April 29, 2024, 2:48 p.m. UTC | #3
On 4/28/24 15: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(-)

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

Though you might mention can_do_io also being restricted:

> @@ -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;
>   
>   struct KVMState;


r~
diff mbox series

Patch

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;
 
 struct KVMState;