Re: [PATCH 00/24] exec: Rework around CPUState user fields (part 2)

[Philippe Mathieu-Daudé]

On 29/4/24 00:14, Philippe Mathieu-Daudé wrote:
> Finish extracting TCG fields from CPUState:
> - Extract tcg_cpu_exit() from cpu_exit()
> - Introduce AccelOpsClass::exit_vcpu_thread()
> - cpu_exit() calls exit_vcpu_thread=tcg_cpu_exit for TCG
> - Forward declare TaskState and more uses of get_task_state()
> - Introduce TCG AccelCPUState
> - Move TCG specific fields from CPUState to AccelCPUState
> - Restrict "exec/tlb-common.h" to TCG
> - Restrict iommu_notifiers, icount to system emulation
>
> Based-on: <20240428214915.10339-1-philmd@linaro.org>

The CPUState changes (part 1 & 2) can be resumed as:

$ git diff master.. -- include/hw/core/cpu.h accel/tcg/vcpu-state.h
-- >8 --
diff --git a/accel/tcg/vcpu-state.h b/accel/tcg/vcpu-state.h
new file mode 100644
index 0000000000..9bb8afac57
--- /dev/null
+++ b/accel/tcg/vcpu-state.h
@@ -0,0 +1,45 @@
+/**
+ * AccelCPUState:
+ * @cflags: Pre-computed cflags for this cpu.
+ * @icount_extra: Instructions until next timer event.
+ * @mem_io_pc: Host Program Counter at which the memory was accessed.
+ */
+struct AccelCPUState {
+    uint32_t cflags;
+    uint32_t cflags_next_tb;
+
+    sigjmp_buf jmp_env;
+    CPUJumpCache tb_jmp_cache;
+
+#ifdef CONFIG_USER_ONLY
+    TaskState *ts;
+#else
+    int64_t icount_budget;
+    int64_t icount_extra;
+
+    uintptr_t mem_io_pc;
+
+    /* track IOMMUs whose translations we've cached in the TCG TLB */
+    GArray *iommu_notifiers;
+#endif
+};
+#endif
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 46b99a7ea5..bdcb09b464 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -337,15 +337,28 @@ typedef union IcountDecr {
-/*
- * Elements of CPUState most efficiently accessed from CPUArchState,
- * via small negative offsets.
+/**
+ * CPUNegativeOffsetState: Elements of CPUState most efficiently accessed
+ *                         from CPUArchState, via small negative offsets.
+ * @can_do_io: True if memory-mapped IO is allowed.
+ * @plugin_mem_cbs: active plugin memory callbacks
+ * @plugin_state: per-CPU plugin state
  */
 typedef struct CPUNegativeOffsetState {
+#ifdef CONFIG_TCG
     CPUTLB tlb;
+#ifdef CONFIG_PLUGIN
+    /*
+     * The callback pointer are accessed via TCG (see 
gen_empty_mem_helper).
+     */
+    GArray *plugin_mem_cbs;
+    CPUPluginState *plugin_state;
+#endif
     IcountDecr icount_decr;
     bool can_do_io;
+#endif
 } CPUNegativeOffsetState;

 struct KVMState;
@@ -383,9 +396,8 @@ struct qemu_work_item;
  *   to a cluster this will be UNASSIGNED_CLUSTER_INDEX; otherwise it will
  *   be the same as the cluster-id property of the CPU object's 
TYPE_CPU_CLUSTER
  *   QOM parent.
- *   Under TCG this value is propagated to @tcg_cflags.
+ *   Under TCG this value is propagated to @accel->cflags.
  *   See TranslationBlock::TCG CF_CLUSTER_MASK.
- * @tcg_cflags: Pre-computed cflags for this cpu.
  * @nr_cores: Number of cores within this CPU package.
  * @nr_threads: Number of threads within this CPU core.
  * @running: #true if CPU is currently running (lockless).
@@ -399,8 +411,6 @@ struct qemu_work_item;
  * @unplug: Indicates a pending CPU unplug request.
  * @crash_occurred: Indicates the OS reported a crash (panic) for this CPU
  * @singlestep_enabled: Flags for single-stepping.
- * @icount_extra: Instructions until next timer event.
- * @neg.can_do_io: True if memory-mapped IO is allowed.
  * @cpu_ases: Pointer to array of CPUAddressSpaces (which define the
  *            AddressSpaces this CPU has)
  * @num_ases: number of CPUAddressSpaces in @cpu_ases
@@ -411,13 +421,10 @@ struct qemu_work_item;
  * @gdb_num_g_regs: Number of registers in GDB 'g' packets.
  * @node: QTAILQ of CPUs sharing TB cache.
  * @opaque: User data.
- * @mem_io_pc: Host Program Counter at which the memory was accessed.
  * @accel: Pointer to accelerator specific state.
  * @kvm_fd: vCPU file descriptor for KVM.
  * @work_mutex: Lock to prevent multiple access to @work_list.
  * @work_list: List of pending asynchronous work.
- * @plugin_mem_cbs: active plugin memory callbacks
- * @plugin_state: per-CPU plugin state
  * @ignore_memory_transaction_failures: Cached copy of the MachineState
  *    flag of the same name: allows the board to suppress calling of the
  *    CPU do_transaction_failed hook function.
@@ -460,14 +467,10 @@ struct CPUState {
     bool crash_occurred;
     bool exit_request;
     int exclusive_context_count;
-    uint32_t cflags_next_tb;
     /* updates protected by BQL */
     uint32_t interrupt_request;
     int singlestep_enabled;
-    int64_t icount_budget;
-    int64_t icount_extra;
     uint64_t random_seed;
-    sigjmp_buf jmp_env;

     QemuMutex work_mutex;
     QSIMPLEQ_HEAD(, qemu_work_item) work_list;
@@ -477,8 +480,6 @@ struct CPUState {
     AddressSpace *as;
     MemoryRegion *memory;

-    CPUJumpCache *tb_jmp_cache;
-
     GArray *gdb_regs;
     int gdb_num_regs;
     int gdb_num_g_regs;
@@ -490,12 +491,9 @@ struct CPUState {
     QTAILQ_HEAD(, CPUWatchpoint) watchpoints;
     CPUWatchpoint *watchpoint_hit;

-    void *opaque;
-
     /* In order to avoid passing too many arguments to the MMIO helpers,
      * we store some rarely used information in the CPU context.
      */
-    uintptr_t mem_io_pc;

     /* Only used in KVM */
     int kvm_fd;
@@ -510,19 +508,9 @@ struct CPUState {
     /* Use by accel-block: CPU is executing an ioctl() */
     QemuLockCnt in_ioctl_lock;

-#ifdef CONFIG_PLUGIN
-    /*
-     * The callback pointer stays in the main CPUState as it is
-     * accessed via TCG (see gen_empty_mem_helper).
-     */
-    GArray *plugin_mem_cbs;
-    CPUPluginState *plugin_state;
-#endif
-
     /* TODO Move common fields from CPUArchState here. */
     int cpu_index;
     int cluster_index;
-    uint32_t tcg_cflags;
     uint32_t halted;
     int32_t exception_index;

@@ -544,9 +532,6 @@ struct CPUState {
     /* Used for user-only emulation of prctl(PR_SET_UNALIGN). */
     bool prctl_unalign_sigbus;

-    /* track IOMMUs whose translations we've cached in the TCG TLB */
-    GArray *iommu_notifiers;
-
     /*
      * MUST BE LAST in order to minimize the displacement to CPUArchState.
      */
---