[RFC PATCH 00/15] softmmu: Make various objects target agnostic

[RFC PATCH 00/15] softmmu: Make various objects target agnostic

[Philippe Mathieu-Daudé]

Few changes to speed up builds, removing 330 objects.

RFC because of:
- accel hax/whpx changes
- sysemu/memory_mapping target_ulong -> hwaddr
which i'm not sure of the impacts.

Philippe Mathieu-Daudé (15):
  accel/kvm: Add more stubs
  accel/whpx: Simplify #ifdef'ry
  accel/hax: Simplify #ifdef'ry
  accel: Only use TCG when building user-mode emulation
  accel/kvm: Simplify user-mode #ifdef'ry
  hw/acpi/memory_hotplug: Remove unused 'hw/acpi/pc-hotplug.h' header
  softmmu/globals: Remove unused 'hw/i386/*' headers
  softmmu/cpu-timers: Remove unused 'exec/exec-all.h' header
  softmmu/runstate: Clean headers
  exec/gdbstub: Make gdb_exit() / gdb_set_stop_cpu() target agnostic
  exec/cpu: Make address_space_init/reloading_memory_map target agnostic
  sysemu/kvm: Make kvm_on_sigbus() / kvm_on_sigbus_vcpu() target
    agnostic
  sysemu/memory_mapping: Become target-agnostic
  softmmu/cpus: Extract QMP command handlers to cpus-qmp.c
  softmmu: Build target-agnostic objects once

 include/exec/cpu-common.h       |  23 +++++++
 include/exec/exec-all.h         |  25 -------
 include/exec/gdbstub.h          |  23 ++++---
 include/sysemu/hax.h            |  14 +---
 include/sysemu/kvm.h            |   6 +-
 include/sysemu/memory_mapping.h |   3 +-
 include/sysemu/whpx.h           |  15 +----
 accel/stubs/hax-stub.c          |   5 ++
 accel/stubs/kvm-stub.c          |  11 +--
 accel/stubs/whpx-stub.c         |  21 ++++++
 hw/acpi/memory_hotplug.c        |   1 -
 softmmu/cpu-timers.c            |   1 -
 softmmu/cpus-qmp.c              | 115 ++++++++++++++++++++++++++++++++
 softmmu/cpus.c                  |  89 ------------------------
 softmmu/globals.c               |   2 -
 softmmu/memory_mapping.c        |   1 +
 softmmu/runstate.c              |   2 +-
 target/i386/hax/hax-all.c       |   2 +-
 target/i386/whpx/whpx-all.c     |   2 +-
 MAINTAINERS                     |   1 +
 accel/meson.build               |  10 +--
 accel/stubs/meson.build         |   1 +
 softmmu/meson.build             |  25 +++----
 23 files changed, 213 insertions(+), 185 deletions(-)
 create mode 100644 accel/stubs/whpx-stub.c
 create mode 100644 softmmu/cpus-qmp.c

-- 
2.26.3

[RFC PATCH 01/15] accel/kvm: Add more stubs

[Philippe Mathieu-Daudé]

To be able to make softmmu/cpus.c not target-specific, we need to
add two more KVM stubs, to avoid:

  /usr/bin/ld: libcommon.fa.p/softmmu_cpus.c.o: in function `cpu_thread_is_idle':
  softmmu/cpus.c:85: undefined reference to `kvm_halt_in_kernel_allowed'
  /usr/bin/ld: libcommon.fa.p/softmmu_cpus.c.o: in function `cpu_check_are_resettable':
  include/sysemu/hw_accel.h:28: undefined reference to `kvm_cpu_check_are_resettable'

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/stubs/kvm-stub.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
index 5b1d00a2224..6bda6c8c925 100644
--- a/accel/stubs/kvm-stub.c
+++ b/accel/stubs/kvm-stub.c
@@ -20,6 +20,7 @@
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
 bool kvm_async_interrupts_allowed;
+bool kvm_halt_in_kernel_allowed;
 bool kvm_eventfds_allowed;
 bool kvm_irqfds_allowed;
 bool kvm_resamplefds_allowed;
@@ -147,4 +148,10 @@ bool kvm_arm_supports_user_irq(void)
 {
     return false;
 }
+
+bool kvm_cpu_check_are_resettable(void)
+{
+    g_assert_not_reached();
+}
+
 #endif
-- 
2.26.3

[RFC PATCH 02/15] accel/whpx: Simplify #ifdef'ry

[Philippe Mathieu-Daudé]

whpx_apic_in_platform() is called from:

- pic_irq_request() in hw/i386/x86.c
- cpu_thread_is_idle() softmmu/cpus.c
- apic_get_class() in target/i386/cpu-sysemu.c

whpx_enabled() is called from:

- cpu_report_tpr_access() in target/i386/helper.c

By converting macros to a function, we can remove the
NEED_CPU_H dependency and build softmmu/cpus.c once
for all targets.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/sysemu/whpx.h       | 15 +--------------
 accel/stubs/whpx-stub.c     | 21 +++++++++++++++++++++
 target/i386/whpx/whpx-all.c |  2 +-
 MAINTAINERS                 |  1 +
 accel/stubs/meson.build     |  1 +
 5 files changed, 25 insertions(+), 15 deletions(-)
 create mode 100644 accel/stubs/whpx-stub.c

diff --git a/include/sysemu/whpx.h b/include/sysemu/whpx.h
index 2889fa2278b..b32f46f9ebf 100644
--- a/include/sysemu/whpx.h
+++ b/include/sysemu/whpx.h
@@ -13,20 +13,7 @@
 #ifndef QEMU_WHPX_H
 #define QEMU_WHPX_H
 
-#ifdef NEED_CPU_H
-
-#ifdef CONFIG_WHPX
-
-int whpx_enabled(void);
+bool whpx_enabled(void);
 bool whpx_apic_in_platform(void);
 
-#else /* CONFIG_WHPX */
-
-#define whpx_enabled() (0)
-#define whpx_apic_in_platform() (0)
-
-#endif /* CONFIG_WHPX */
-
-#endif /* NEED_CPU_H */
-
 #endif /* QEMU_WHPX_H */
diff --git a/accel/stubs/whpx-stub.c b/accel/stubs/whpx-stub.c
new file mode 100644
index 00000000000..794a992acc6
--- /dev/null
+++ b/accel/stubs/whpx-stub.c
@@ -0,0 +1,21 @@
+/*
+ * QEMU WHPX stub
+ *
+ * Copyright (c) 2019 Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/whpx.h"
+
+bool whpx_enabled(void)
+{
+    return false;
+}
+
+bool whpx_apic_in_platform(void)
+{
+    return false;
+}
diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
index f832f286ac3..6709a89f8f0 100644
--- a/target/i386/whpx/whpx-all.c
+++ b/target/i386/whpx/whpx-all.c
@@ -1862,7 +1862,7 @@ error:
     return ret;
 }
 
-int whpx_enabled(void)
+bool whpx_enabled(void)
 {
     return whpx_allowed;
 }
diff --git a/MAINTAINERS b/MAINTAINERS
index 78561a223f9..7877710e372 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -439,6 +439,7 @@ F: include/sysemu/hvf.h
 WHPX CPUs
 M: Sunil Muthuswamy <sunilmut@microsoft.com>
 S: Supported
+F: accel/stubs/whpx-stub.c
 F: target/i386/whpx/
 F: include/sysemu/whpx.h
 
diff --git a/accel/stubs/meson.build b/accel/stubs/meson.build
index 12dd1539afa..3fbe34e5bb3 100644
--- a/accel/stubs/meson.build
+++ b/accel/stubs/meson.build
@@ -2,3 +2,4 @@
 specific_ss.add(when: 'CONFIG_XEN', if_false: files('xen-stub.c'))
 specific_ss.add(when: 'CONFIG_KVM', if_false: files('kvm-stub.c'))
 specific_ss.add(when: 'CONFIG_TCG', if_false: files('tcg-stub.c'))
+specific_ss.add(when: 'CONFIG_WHPX', if_false: files('whpx-stub.c'))
-- 
2.26.3

[RFC PATCH 03/15] accel/hax: Simplify #ifdef'ry

[Philippe Mathieu-Daudé]

hax_enabled() is called from:

- qemu_init_board() in softmmu/vl.c
- qemu_wait_io_event() in softmmu/cpus.c
- apic_common_realize() in hw/intc/apic_common.c

By converting macros to a function, we can remove the
NEED_CPU_H dependency and build softmmu/cpus.c once
for all targets.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/sysemu/hax.h      | 14 +-------------
 accel/stubs/hax-stub.c    |  5 +++++
 target/i386/hax/hax-all.c |  2 +-
 3 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/include/sysemu/hax.h b/include/sysemu/hax.h
index 247f0661d12..bc165b23ae6 100644
--- a/include/sysemu/hax.h
+++ b/include/sysemu/hax.h
@@ -24,18 +24,6 @@
 
 int hax_sync_vcpus(void);
 
-#ifdef NEED_CPU_H
-
-#ifdef CONFIG_HAX
-
-int hax_enabled(void);
-
-#else /* CONFIG_HAX */
-
-#define hax_enabled() (0)
-
-#endif /* CONFIG_HAX */
-
-#endif /* NEED_CPU_H */
+bool hax_enabled(void);
 
 #endif /* QEMU_HAX_H */
diff --git a/accel/stubs/hax-stub.c b/accel/stubs/hax-stub.c
index 49077f88e3c..1a0370a4362 100644
--- a/accel/stubs/hax-stub.c
+++ b/accel/stubs/hax-stub.c
@@ -16,6 +16,11 @@
 #include "qemu/osdep.h"
 #include "sysemu/hax.h"
 
+bool hax_enabled(void)
+{
+    return false;
+}
+
 int hax_sync_vcpus(void)
 {
     return 0;
diff --git a/target/i386/hax/hax-all.c b/target/i386/hax/hax-all.c
index bf65ed6fa92..d99feef21d4 100644
--- a/target/i386/hax/hax-all.c
+++ b/target/i386/hax/hax-all.c
@@ -56,7 +56,7 @@ struct hax_state hax_global;
 static void hax_vcpu_sync_state(CPUArchState *env, int modified);
 static int hax_arch_get_registers(CPUArchState *env);
 
-int hax_enabled(void)
+bool hax_enabled(void)
 {
     return hax_allowed;
 }
-- 
2.26.3

[RFC PATCH 04/15] accel: Only use TCG when building user-mode emulation

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/meson.build | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/accel/meson.build b/accel/meson.build
index b44ba30c864..0e296911aea 100644
--- a/accel/meson.build
+++ b/accel/meson.build
@@ -2,11 +2,13 @@
 softmmu_ss.add(files('accel-softmmu.c'))
 user_ss.add(files('accel-user.c'))
 
-subdir('qtest')
-subdir('kvm')
 subdir('tcg')
-subdir('xen')
-subdir('stubs')
+if have_system
+  subdir('kvm')
+  subdir('qtest')
+  subdir('stubs')
+  subdir('xen')
+endif
 
 dummy_ss = ss.source_set()
 dummy_ss.add(files(
-- 
2.26.3

[RFC PATCH 05/15] accel/kvm: Simplify user-mode #ifdef'ry

[Philippe Mathieu-Daudé]

Now than we only build this stub with system emulation,
remove the user-mode #ifdef'ry.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 accel/stubs/kvm-stub.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
index 6bda6c8c925..6ae1ff62607 100644
--- a/accel/stubs/kvm-stub.c
+++ b/accel/stubs/kvm-stub.c
@@ -12,10 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "sysemu/kvm.h"
-
-#ifndef CONFIG_USER_ONLY
 #include "hw/pci/msi.h"
-#endif
 
 KVMState *kvm_state;
 bool kvm_kernel_irqchip;
@@ -81,7 +78,6 @@ int kvm_on_sigbus(int code, void *addr)
     return 1;
 }
 
-#ifndef CONFIG_USER_ONLY
 int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
 {
     return -ENOSYS;
@@ -153,5 +149,3 @@ bool kvm_cpu_check_are_resettable(void)
 {
     g_assert_not_reached();
 }
-
-#endif
-- 
2.26.3

[RFC PATCH 06/15] hw/acpi/memory_hotplug: Remove unused 'hw/acpi/pc-hotplug.h' header

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/acpi/memory_hotplug.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index af378894235..104c1abd4eb 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -1,6 +1,5 @@
 #include "qemu/osdep.h"
 #include "hw/acpi/memory_hotplug.h"
-#include "hw/acpi/pc-hotplug.h"
 #include "hw/mem/pc-dimm.h"
 #include "hw/qdev-core.h"
 #include "migration/vmstate.h"
-- 
2.26.3

[RFC PATCH 07/15] softmmu/globals: Remove unused 'hw/i386/*' headers

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/globals.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/softmmu/globals.c b/softmmu/globals.c
index 7d0fc811835..3ebd718e35d 100644
--- a/softmmu/globals.c
+++ b/softmmu/globals.c
@@ -25,8 +25,6 @@
 #include "qemu/osdep.h"
 #include "exec/cpu-common.h"
 #include "hw/display/vga.h"
-#include "hw/i386/pc.h"
-#include "hw/i386/x86.h"
 #include "hw/loader.h"
 #include "hw/xen/xen.h"
 #include "net/net.h"
-- 
2.26.3

[RFC PATCH 08/15] softmmu/cpu-timers: Remove unused 'exec/exec-all.h' header

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/cpu-timers.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/softmmu/cpu-timers.c b/softmmu/cpu-timers.c
index 34ddfa02f1e..204d946a172 100644
--- a/softmmu/cpu-timers.c
+++ b/softmmu/cpu-timers.c
@@ -28,7 +28,6 @@
 #include "migration/vmstate.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
-#include "exec/exec-all.h"
 #include "sysemu/cpus.h"
 #include "qemu/main-loop.h"
 #include "qemu/option.h"
-- 
2.26.3

[RFC PATCH 09/15] softmmu/runstate: Clean headers

[Philippe Mathieu-Daudé]

Add the missing 'qemu/log.h' header and remove the
unused 'exec/exec-all.h' one.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/runstate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/softmmu/runstate.c b/softmmu/runstate.c
index ce8977c6a29..ffd8ddf1341 100644
--- a/softmmu/runstate.c
+++ b/softmmu/runstate.c
@@ -30,7 +30,6 @@
 #include "crypto/cipher.h"
 #include "crypto/init.h"
 #include "exec/cpu-common.h"
-#include "exec/exec-all.h"
 #include "exec/gdbstub.h"
 #include "hw/boards.h"
 #include "migration/misc.h"
@@ -44,6 +43,7 @@
 #include "qemu-common.h"
 #include "qemu/error-report.h"
 #include "qemu/job.h"
+#include "qemu/log.h"
 #include "qemu/module.h"
 #include "qemu/plugin.h"
 #include "qemu/sockets.h"
-- 
2.26.3

[RFC PATCH 10/15] exec/gdbstub: Make gdb_exit() / gdb_set_stop_cpu() target agnostic

[Philippe Mathieu-Daudé]

gdb_exit() and gdb_set_stop_cpu() prototypes don't have to be
target specific. Remove this limitation to be able to build
softmmu/cpus.c and softmmu/runstate.c once for all targets.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/exec/gdbstub.h | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index a024a0350df..84b1f2ff2aa 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -45,17 +45,6 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...);
  */
 void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va);
 int use_gdb_syscalls(void);
-void gdb_set_stop_cpu(CPUState *cpu);
-
-/**
- * gdb_exit: exit gdb session, reporting inferior status
- * @code: exit code reported
- *
- * This closes the session and sends a final packet to GDB reporting
- * the exit status of the program. It also cleans up any connections
- * detritus before returning.
- */
-void gdb_exit(int code);
 
 #ifdef CONFIG_USER_ONLY
 /**
@@ -177,6 +166,18 @@ static inline uint8_t * gdb_get_reg_ptr(GByteArray *buf, int len)
  */
 int gdbserver_start(const char *port_or_device);
 
+/**
+ * gdb_exit: exit gdb session, reporting inferior status
+ * @code: exit code reported
+ *
+ * This closes the session and sends a final packet to GDB reporting
+ * the exit status of the program. It also cleans up any connections
+ * detritus before returning.
+ */
+void gdb_exit(int code);
+
+void gdb_set_stop_cpu(CPUState *cpu);
+
 /**
  * gdb_has_xml:
  * This is an ugly hack to cope with both new and old gdb.
-- 
2.26.3

[RFC PATCH 11/15] exec/cpu: Make address_space_init/reloading_memory_map target agnostic

[Philippe Mathieu-Daudé]

cpu_address_space_init() and cpu_reloading_memory_map() don't
have to be target specific. Remove this limitation to be able
to build softmmu/cpus.c once for all targets.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/exec/cpu-common.h | 23 +++++++++++++++++++++++
 include/exec/exec-all.h   | 25 -------------------------
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index ccabed4003a..a4fb6813206 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -68,6 +68,28 @@ void qemu_ram_unset_migratable(RAMBlock *rb);
 size_t qemu_ram_pagesize(RAMBlock *block);
 size_t qemu_ram_pagesize_largest(void);
 
+/**
+ * cpu_address_space_init:
+ * @cpu: CPU to add this address space to
+ * @asidx: integer index of this address space
+ * @prefix: prefix to be used as name of address space
+ * @mr: the root memory region of address space
+ *
+ * Add the specified address space to the CPU's cpu_ases list.
+ * The address space added with @asidx 0 is the one used for the
+ * convenience pointer cpu->as.
+ * The target-specific code which registers ASes is responsible
+ * for defining what semantics address space 0, 1, 2, etc have.
+ *
+ * Before the first call to this function, the caller must set
+ * cpu->num_ases to the total number of address spaces it needs
+ * to support.
+ *
+ * Note that with KVM only one address space is supported.
+ */
+void cpu_address_space_init(CPUState *cpu, int asidx,
+                            const char *prefix, MemoryRegion *mr);
+
 void cpu_physical_memory_rw(hwaddr addr, void *buf,
                             hwaddr len, bool is_write);
 static inline void cpu_physical_memory_read(hwaddr addr,
@@ -80,6 +102,7 @@ static inline void cpu_physical_memory_write(hwaddr addr,
 {
     cpu_physical_memory_rw(addr, (void *)buf, len, true);
 }
+void cpu_reloading_memory_map(void);
 void *cpu_physical_memory_map(hwaddr addr,
                               hwaddr *plen,
                               bool is_write);
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 6b036cae8f6..781ee62f395 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -84,31 +84,6 @@ static inline bool cpu_loop_exit_requested(CPUState *cpu)
     return (int32_t)qatomic_read(&cpu_neg(cpu)->icount_decr.u32) < 0;
 }
 
-#if !defined(CONFIG_USER_ONLY)
-void cpu_reloading_memory_map(void);
-/**
- * cpu_address_space_init:
- * @cpu: CPU to add this address space to
- * @asidx: integer index of this address space
- * @prefix: prefix to be used as name of address space
- * @mr: the root memory region of address space
- *
- * Add the specified address space to the CPU's cpu_ases list.
- * The address space added with @asidx 0 is the one used for the
- * convenience pointer cpu->as.
- * The target-specific code which registers ASes is responsible
- * for defining what semantics address space 0, 1, 2, etc have.
- *
- * Before the first call to this function, the caller must set
- * cpu->num_ases to the total number of address spaces it needs
- * to support.
- *
- * Note that with KVM only one address space is supported.
- */
-void cpu_address_space_init(CPUState *cpu, int asidx,
-                            const char *prefix, MemoryRegion *mr);
-#endif
-
 #if !defined(CONFIG_USER_ONLY) && defined(CONFIG_TCG)
 /* cputlb.c */
 /**
-- 
2.26.3

[RFC PATCH 12/15] sysemu/kvm: Make kvm_on_sigbus() / kvm_on_sigbus_vcpu() target agnostic

[Philippe Mathieu-Daudé]

kvm_on_sigbus() and kvm_on_sigbus_vcpu() prototypes don't have
to be target specific. Remove this limitation to be able to build
softmmu/cpus.c once for all targets.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/sysemu/kvm.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index a1ab1ee12d3..d9bced5f392 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -234,6 +234,9 @@ int kvm_has_intx_set_mask(void);
 bool kvm_arm_supports_user_irq(void);
 
 
+int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
+int kvm_on_sigbus(int code, void *addr);
+
 #ifdef NEED_CPU_H
 #include "cpu.h"
 
@@ -246,9 +249,6 @@ int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr,
 void kvm_remove_all_breakpoints(CPUState *cpu);
 int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap);
 
-int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
-int kvm_on_sigbus(int code, void *addr);
-
 /* internal API */
 
 int kvm_ioctl(KVMState *s, int type, ...);
-- 
2.26.3

[RFC PATCH 13/15] sysemu/memory_mapping: Become target-agnostic

[Philippe Mathieu-Daudé]

target_ulong is target-specific, while hwaddr isn't.

memory_mapping_list_add_merge_sorted() uses hwaddr arguments
anyway, so use the hwaddr type for MemoryMapping::virt_addr.

Remove the unnecessary "exec/cpu-defs.h" target-speficic header
from "memory_mapping.h" and use the target-agnostic "hw/core/cpu.h"
locally in memory_mapping.c.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/sysemu/memory_mapping.h | 3 +--
 softmmu/memory_mapping.c        | 1 +
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
index 4b20f1a639e..771dba21773 100644
--- a/include/sysemu/memory_mapping.h
+++ b/include/sysemu/memory_mapping.h
@@ -15,7 +15,6 @@
 #define MEMORY_MAPPING_H
 
 #include "qemu/queue.h"
-#include "exec/cpu-defs.h"
 #include "exec/memory.h"
 
 typedef struct GuestPhysBlock {
@@ -43,7 +42,7 @@ typedef struct GuestPhysBlockList {
 /* The physical and virtual address in the memory mapping are contiguous. */
 typedef struct MemoryMapping {
     hwaddr phys_addr;
-    target_ulong virt_addr;
+    hwaddr virt_addr;
     ram_addr_t length;
     QTAILQ_ENTRY(MemoryMapping) next;
 } MemoryMapping;
diff --git a/softmmu/memory_mapping.c b/softmmu/memory_mapping.c
index e7af2765466..7bb74df5a7a 100644
--- a/softmmu/memory_mapping.c
+++ b/softmmu/memory_mapping.c
@@ -17,6 +17,7 @@
 #include "sysemu/memory_mapping.h"
 #include "exec/memory.h"
 #include "exec/address-spaces.h"
+#include "hw/core/cpu.h"
 
 //#define DEBUG_GUEST_PHYS_REGION_ADD
 
-- 
2.26.3

[RFC PATCH 14/15] softmmu/cpus: Extract QMP command handlers to cpus-qmp.c

[Philippe Mathieu-Daudé]

qmp_memsave() and qmp_pmemsave() call cpu_memory_rw_debug()
and cpu_physical_memory_read(), which are target specific
prototypes. To be able to build softmmu/cpus.c once for
all targets, extract the QMP commands handlers to a new
file which will be built per target.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/cpus-qmp.c  | 115 ++++++++++++++++++++++++++++++++++++++++++++
 softmmu/cpus.c      |  89 ----------------------------------
 softmmu/meson.build |   1 +
 3 files changed, 116 insertions(+), 89 deletions(-)
 create mode 100644 softmmu/cpus-qmp.c

diff --git a/softmmu/cpus-qmp.c b/softmmu/cpus-qmp.c
new file mode 100644
index 00000000000..7b613028225
--- /dev/null
+++ b/softmmu/cpus-qmp.c
@@ -0,0 +1,115 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "exec/exec-all.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qerror.h"
+#include "qapi/qapi-commands-machine.h"
+#include "monitor/monitor.h"
+#include "hw/nmi.h"
+
+void qmp_memsave(int64_t addr, int64_t size, const char *filename,
+                 bool has_cpu, int64_t cpu_index, Error **errp)
+{
+    FILE *f;
+    uint32_t l;
+    CPUState *cpu;
+    uint8_t buf[1024];
+    int64_t orig_addr = addr, orig_size = size;
+
+    if (!has_cpu) {
+        cpu_index = 0;
+    }
+
+    cpu = qemu_get_cpu(cpu_index);
+    if (cpu == NULL) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cpu-index",
+                   "a CPU number");
+        return;
+    }
+
+    f = fopen(filename, "wb");
+    if (!f) {
+        error_setg_file_open(errp, errno, filename);
+        return;
+    }
+
+    while (size != 0) {
+        l = sizeof(buf);
+        if (l > size) {
+            l = size;
+        }
+        if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
+            error_setg(errp, "Invalid addr 0x%016" PRIx64 "/size %" PRId64
+                             " specified", orig_addr, orig_size);
+            goto exit;
+        }
+        if (fwrite(buf, 1, l, f) != l) {
+            error_setg(errp, QERR_IO_ERROR);
+            goto exit;
+        }
+        addr += l;
+        size -= l;
+    }
+
+exit:
+    fclose(f);
+}
+
+void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
+                  Error **errp)
+{
+    FILE *f;
+    uint32_t l;
+    uint8_t buf[1024];
+
+    f = fopen(filename, "wb");
+    if (!f) {
+        error_setg_file_open(errp, errno, filename);
+        return;
+    }
+
+    while (size != 0) {
+        l = sizeof(buf);
+        if (l > size) {
+            l = size;
+        }
+        cpu_physical_memory_read(addr, buf, l);
+        if (fwrite(buf, 1, l, f) != l) {
+            error_setg(errp, QERR_IO_ERROR);
+            goto exit;
+        }
+        addr += l;
+        size -= l;
+    }
+
+exit:
+    fclose(f);
+}
+
+void qmp_inject_nmi(Error **errp)
+{
+    nmi_monitor_handle(monitor_get_cpu_index(monitor_cur()), errp);
+}
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index a7ee431187a..e3810135166 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -24,20 +24,14 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
-#include "monitor/monitor.h"
 #include "qapi/error.h"
-#include "qapi/qapi-commands-machine.h"
-#include "qapi/qapi-commands-misc.h"
 #include "qapi/qapi-events-run-state.h"
-#include "qapi/qmp/qerror.h"
 #include "exec/gdbstub.h"
 #include "sysemu/hw_accel.h"
-#include "exec/exec-all.h"
 #include "qemu/thread.h"
 #include "qemu/plugin.h"
 #include "sysemu/cpus.h"
 #include "qemu/guest-random.h"
-#include "hw/nmi.h"
 #include "sysemu/replay.h"
 #include "sysemu/runstate.h"
 #include "sysemu/cpu-timers.h"
@@ -720,86 +714,3 @@ void list_cpus(const char *optarg)
     cpu_list();
 #endif
 }
-
-void qmp_memsave(int64_t addr, int64_t size, const char *filename,
-                 bool has_cpu, int64_t cpu_index, Error **errp)
-{
-    FILE *f;
-    uint32_t l;
-    CPUState *cpu;
-    uint8_t buf[1024];
-    int64_t orig_addr = addr, orig_size = size;
-
-    if (!has_cpu) {
-        cpu_index = 0;
-    }
-
-    cpu = qemu_get_cpu(cpu_index);
-    if (cpu == NULL) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cpu-index",
-                   "a CPU number");
-        return;
-    }
-
-    f = fopen(filename, "wb");
-    if (!f) {
-        error_setg_file_open(errp, errno, filename);
-        return;
-    }
-
-    while (size != 0) {
-        l = sizeof(buf);
-        if (l > size)
-            l = size;
-        if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
-            error_setg(errp, "Invalid addr 0x%016" PRIx64 "/size %" PRId64
-                             " specified", orig_addr, orig_size);
-            goto exit;
-        }
-        if (fwrite(buf, 1, l, f) != l) {
-            error_setg(errp, QERR_IO_ERROR);
-            goto exit;
-        }
-        addr += l;
-        size -= l;
-    }
-
-exit:
-    fclose(f);
-}
-
-void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
-                  Error **errp)
-{
-    FILE *f;
-    uint32_t l;
-    uint8_t buf[1024];
-
-    f = fopen(filename, "wb");
-    if (!f) {
-        error_setg_file_open(errp, errno, filename);
-        return;
-    }
-
-    while (size != 0) {
-        l = sizeof(buf);
-        if (l > size)
-            l = size;
-        cpu_physical_memory_read(addr, buf, l);
-        if (fwrite(buf, 1, l, f) != l) {
-            error_setg(errp, QERR_IO_ERROR);
-            goto exit;
-        }
-        addr += l;
-        size -= l;
-    }
-
-exit:
-    fclose(f);
-}
-
-void qmp_inject_nmi(Error **errp)
-{
-    nmi_monitor_handle(monitor_get_cpu_index(monitor_cur()), errp);
-}
-
diff --git a/softmmu/meson.build b/softmmu/meson.build
index d8e03018abf..5e578b20e6c 100644
--- a/softmmu/meson.build
+++ b/softmmu/meson.build
@@ -2,6 +2,7 @@
   'arch_init.c',
   'balloon.c',
   'cpus.c',
+  'cpus-qmp.c',
   'cpu-throttle.c',
   'datadir.c',
   'globals.c',
-- 
2.26.3

[RFC PATCH 15/15] softmmu: Build target-agnostic objects once

[Philippe Mathieu-Daudé]

Various softmmu objects aren't target specific. Move them
to the generic softmmu source set.

For our 31 softmmu targets, this is in total 330 objects
less to build!

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 softmmu/meson.build | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/softmmu/meson.build b/softmmu/meson.build
index 5e578b20e6c..f98a5972d23 100644
--- a/softmmu/meson.build
+++ b/softmmu/meson.build
@@ -1,21 +1,10 @@
 specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: [files(
   'arch_init.c',
-  'balloon.c',
-  'cpus.c',
   'cpus-qmp.c',
-  'cpu-throttle.c',
-  'datadir.c',
-  'globals.c',
-  'physmem.c',
   'ioport.c',
-  'rtc.c',
-  'runstate.c',
   'memory.c',
-  'memory_mapping.c',
+  'physmem.c',
   'qtest.c',
-  'vl.c',
-  'cpu-timers.c',
-  'runstate-action.c',
 )])
 
 specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TCG'], if_true: [files(
@@ -23,9 +12,20 @@
 )])
 
 softmmu_ss.add(files(
+  'balloon.c',
   'bootdevice.c',
+  'cpus.c',
+  'cpu-throttle.c',
+  'cpu-timers.c',
+  'datadir.c',
   'dma-helpers.c',
+  'globals.c',
+  'memory_mapping.c',
   'qdev-monitor.c',
+  'rtc.c',
+  'runstate-action.c',
+  'runstate.c',
+  'vl.c',
 ), sdl, libpmem, libdaxctl)
 
 softmmu_ss.add(when: 'CONFIG_TPM', if_true: files('tpm.c'))
-- 
2.26.3

Re: [RFC PATCH 10/15] exec/gdbstub: Make gdb_exit() / gdb_set_stop_cpu() target agnostic

[Philippe Mathieu-Daudé]

Forgot to Cc Alex...

On 5/17/21 1:55 PM, Philippe Mathieu-Daudé wrote:
> gdb_exit() and gdb_set_stop_cpu() prototypes don't have to be
> target specific. Remove this limitation to be able to build
> softmmu/cpus.c and softmmu/runstate.c once for all targets.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/exec/gdbstub.h | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index a024a0350df..84b1f2ff2aa 100644
> --- a/include/exec/gdbstub.h
> +++ b/include/exec/gdbstub.h
> @@ -45,17 +45,6 @@ void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...);
>   */
>  void gdb_do_syscallv(gdb_syscall_complete_cb cb, const char *fmt, va_list va);
>  int use_gdb_syscalls(void);
> -void gdb_set_stop_cpu(CPUState *cpu);
> -
> -/**
> - * gdb_exit: exit gdb session, reporting inferior status
> - * @code: exit code reported
> - *
> - * This closes the session and sends a final packet to GDB reporting
> - * the exit status of the program. It also cleans up any connections
> - * detritus before returning.
> - */
> -void gdb_exit(int code);
>  
>  #ifdef CONFIG_USER_ONLY
>  /**
> @@ -177,6 +166,18 @@ static inline uint8_t * gdb_get_reg_ptr(GByteArray *buf, int len)
>   */
>  int gdbserver_start(const char *port_or_device);
>  
> +/**
> + * gdb_exit: exit gdb session, reporting inferior status
> + * @code: exit code reported
> + *
> + * This closes the session and sends a final packet to GDB reporting
> + * the exit status of the program. It also cleans up any connections
> + * detritus before returning.
> + */
> +void gdb_exit(int code);
> +
> +void gdb_set_stop_cpu(CPUState *cpu);
> +
>  /**
>   * gdb_has_xml:
>   * This is an ugly hack to cope with both new and old gdb.
> 

Re: [RFC PATCH 01/15] accel/kvm: Add more stubs

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> To be able to make softmmu/cpus.c not target-specific, we need to
> add two more KVM stubs, to avoid:
> 
>    /usr/bin/ld: libcommon.fa.p/softmmu_cpus.c.o: in function `cpu_thread_is_idle':
>    softmmu/cpus.c:85: undefined reference to `kvm_halt_in_kernel_allowed'
>    /usr/bin/ld: libcommon.fa.p/softmmu_cpus.c.o: in function `cpu_check_are_resettable':
>    include/sysemu/hw_accel.h:28: undefined reference to `kvm_cpu_check_are_resettable'
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   accel/stubs/kvm-stub.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c
> index 5b1d00a2224..6bda6c8c925 100644
> --- a/accel/stubs/kvm-stub.c
> +++ b/accel/stubs/kvm-stub.c
> @@ -20,6 +20,7 @@
>   KVMState *kvm_state;
>   bool kvm_kernel_irqchip;
>   bool kvm_async_interrupts_allowed;
> +bool kvm_halt_in_kernel_allowed;
>   bool kvm_eventfds_allowed;
>   bool kvm_irqfds_allowed;
>   bool kvm_resamplefds_allowed;
> @@ -147,4 +148,10 @@ bool kvm_arm_supports_user_irq(void)
>   {
>       return false;
>   }
> +
> +bool kvm_cpu_check_are_resettable(void)
> +{
> +    g_assert_not_reached();
> +}
> +

It should be easy to turn cpus_are_resettable into an AccelOpsClass hook.

It's less obvious how to do that for cpu_thread_is_idle, but it's clear with 
the kvm and whpx checks that something is required.


r~

Re: [RFC PATCH 02/15] accel/whpx: Simplify #ifdef'ry

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> whpx_apic_in_platform() is called from:
> 
> - pic_irq_request() in hw/i386/x86.c
> - cpu_thread_is_idle() softmmu/cpus.c
> - apic_get_class() in target/i386/cpu-sysemu.c
> 
> whpx_enabled() is called from:
> 
> - cpu_report_tpr_access() in target/i386/helper.c
> 
> By converting macros to a function, we can remove the
> NEED_CPU_H dependency and build softmmu/cpus.c once
> for all targets.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   include/sysemu/whpx.h       | 15 +--------------
>   accel/stubs/whpx-stub.c     | 21 +++++++++++++++++++++
>   target/i386/whpx/whpx-all.c |  2 +-
>   MAINTAINERS                 |  1 +
>   accel/stubs/meson.build     |  1 +
>   5 files changed, 25 insertions(+), 15 deletions(-)
>   create mode 100644 accel/stubs/whpx-stub.c
> 
> diff --git a/include/sysemu/whpx.h b/include/sysemu/whpx.h
> index 2889fa2278b..b32f46f9ebf 100644
> --- a/include/sysemu/whpx.h
> +++ b/include/sysemu/whpx.h
> @@ -13,20 +13,7 @@
>   #ifndef QEMU_WHPX_H
>   #define QEMU_WHPX_H
>   
> -#ifdef NEED_CPU_H
> -
> -#ifdef CONFIG_WHPX
> -
> -int whpx_enabled(void);
> +bool whpx_enabled(void);
>   bool whpx_apic_in_platform(void);
>   
> -#else /* CONFIG_WHPX */
> -
> -#define whpx_enabled() (0)
> -#define whpx_apic_in_platform() (0)

I think the loss of whpx_enabled as false is unfortunate. I'd like to see this 
file mirror sysemu/kvm.h with CONFIG_WHPX_IS_POSSIBLE.

As I mentioned vs patch 1, whpx_apic_in_platform would be handled via 
AccelOpsClass with whatever hook we come up with for cpu_thread_is_idle.


r~

Re: [RFC PATCH 03/15] accel/hax: Simplify #ifdef'ry

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> -#ifdef NEED_CPU_H
> -
> -#ifdef CONFIG_HAX
> -
> -int hax_enabled(void);
> -
> -#else /* CONFIG_HAX */
> -
> -#define hax_enabled() (0)
> -
> -#endif /* CONFIG_HAX */
> -
> -#endif /* NEED_CPU_H */
> +bool hax_enabled(void);

Similarly with CONFIG_HAX_IS_POSSIBLE.


r~

Re: [RFC PATCH 04/15] accel: Only use TCG when building user-mode emulation

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   accel/meson.build | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)

I think the description should be clearer, since the restriction being applied 
is for system emulation.  How about

meson: Only build hw virtualization with system emulation

> 
> diff --git a/accel/meson.build b/accel/meson.build
> index b44ba30c864..0e296911aea 100644
> --- a/accel/meson.build
> +++ b/accel/meson.build
> @@ -2,11 +2,13 @@
>   softmmu_ss.add(files('accel-softmmu.c'))
>   user_ss.add(files('accel-user.c'))
>   
> -subdir('qtest')
> -subdir('kvm')
>   subdir('tcg')
> -subdir('xen')
> -subdir('stubs')
> +if have_system
> +  subdir('kvm')
> +  subdir('qtest')
> +  subdir('stubs')
> +  subdir('xen')
> +endif

The patch itself looks fine.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [RFC PATCH 05/15] accel/kvm: Simplify user-mode #ifdef'ry

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> Now than we only build this stub with system emulation,
> remove the user-mode #ifdef'ry.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   accel/stubs/kvm-stub.c | 6 ------
>   1 file changed, 6 deletions(-)

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

r~

Re: [RFC PATCH 06/15] hw/acpi/memory_hotplug: Remove unused 'hw/acpi/pc-hotplug.h' header

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   hw/acpi/memory_hotplug.c | 1 -
>   1 file changed, 1 deletion(-)

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

r~

Re: [RFC PATCH 07/15] softmmu/globals: Remove unused 'hw/i386/*' headers

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   softmmu/globals.c | 2 --
>   1 file changed, 2 deletions(-)

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

r~

Re: [RFC PATCH 08/15] softmmu/cpu-timers: Remove unused 'exec/exec-all.h' header

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   softmmu/cpu-timers.c | 1 -
>   1 file changed, 1 deletion(-)

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

r~

Re: [RFC PATCH 09/15] softmmu/runstate: Clean headers

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> Add the missing 'qemu/log.h' header and remove the
> unused 'exec/exec-all.h' one.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   softmmu/runstate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

r~

Re: [RFC PATCH 10/15] exec/gdbstub: Make gdb_exit() / gdb_set_stop_cpu() target agnostic

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> gdb_exit() and gdb_set_stop_cpu() prototypes don't have to be
> target specific. Remove this limitation to be able to build
> softmmu/cpus.c and softmmu/runstate.c once for all targets.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   include/exec/gdbstub.h | 23 ++++++++++++-----------
>   1 file changed, 12 insertions(+), 11 deletions(-)

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

r~

Re: [RFC PATCH 11/15] exec/cpu: Make address_space_init/reloading_memory_map target agnostic

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> cpu_address_space_init() and cpu_reloading_memory_map() don't
> have to be target specific. Remove this limitation to be able
> to build softmmu/cpus.c once for all targets.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   include/exec/cpu-common.h | 23 +++++++++++++++++++++++
>   include/exec/exec-all.h   | 25 -------------------------
>   2 files changed, 23 insertions(+), 25 deletions(-)

It's not clear to me why the declarations moved file, instead of just droppig 
the surrounding ifdef.

If there's a good reason, fine, but the reason belongs in the commit message.

r~

Re: [RFC PATCH 12/15] sysemu/kvm: Make kvm_on_sigbus() / kvm_on_sigbus_vcpu() target agnostic

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> kvm_on_sigbus() and kvm_on_sigbus_vcpu() prototypes don't have
> to be target specific. Remove this limitation to be able to build
> softmmu/cpus.c once for all targets.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   include/sysemu/kvm.h | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

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

r~

Re: [RFC PATCH 13/15] sysemu/memory_mapping: Become target-agnostic

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> target_ulong is target-specific, while hwaddr isn't.
> 
> memory_mapping_list_add_merge_sorted() uses hwaddr arguments
> anyway, so use the hwaddr type for MemoryMapping::virt_addr.
> 
> Remove the unnecessary "exec/cpu-defs.h" target-speficic header
> from "memory_mapping.h" and use the target-agnostic "hw/core/cpu.h"
> locally in memory_mapping.c.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   include/sysemu/memory_mapping.h | 3 +--
>   softmmu/memory_mapping.c        | 1 +
>   2 files changed, 2 insertions(+), 2 deletions(-)

Certainly hwaddr is now unconditionally 64-bit, so I think this would work 
fine.  I just wonder if uint64_t wouldn't be better, since virt_addr is not a 
"hardware" aka physical address (as per hwaddr.h).

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


r~

Re: [RFC PATCH 14/15] softmmu/cpus: Extract QMP command handlers to cpus-qmp.c

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> qmp_memsave() and qmp_pmemsave() call cpu_memory_rw_debug()
> and cpu_physical_memory_read(), which are target specific
> prototypes.

Is there any reason they should be?

In the short-term though,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [RFC PATCH 15/15] softmmu: Build target-agnostic objects once

[Richard Henderson]

On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
> Various softmmu objects aren't target specific. Move them
> to the generic softmmu source set.
> 
> For our 31 softmmu targets, this is in total 330 objects
> less to build!
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   softmmu/meson.build | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)

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

r~

Re: [RFC PATCH 11/15] exec/cpu: Make address_space_init/reloading_memory_map target agnostic

[Philippe Mathieu-Daudé]

On 5/26/21 9:01 PM, Richard Henderson wrote:
> On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
>> cpu_address_space_init() and cpu_reloading_memory_map() don't
>> have to be target specific. Remove this limitation to be able
>> to build softmmu/cpus.c once for all targets.
>>
>> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
>> ---
>>   include/exec/cpu-common.h | 23 +++++++++++++++++++++++
>>   include/exec/exec-all.h   | 25 -------------------------
>>   2 files changed, 23 insertions(+), 25 deletions(-)
> 
> It's not clear to me why the declarations moved file, instead of just
> droppig the surrounding ifdef.

hwaddr is target-specific, cpu_address_space_init don't uses it.

softmmu/cpus.c is target-agnostic but uses cpu_address_space_init().

Similarly with cpu_reloading_memory_map() in softmmu/physmem.c.

> If there's a good reason, fine, but the reason belongs in the commit
> message.

OK, I'll mention hwaddr.

Re: [RFC PATCH 14/15] softmmu/cpus: Extract QMP command handlers to cpus-qmp.c

[Philippe Mathieu-Daudé]

On 5/26/21 9:10 PM, Richard Henderson wrote:
> On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
>> qmp_memsave() and qmp_pmemsave() call cpu_memory_rw_debug()
>> and cpu_physical_memory_read(), which are target specific
>> prototypes.
> 
> Is there any reason they should be?

They use target_ulong. Should they use hwaddr instead?

> In the short-term though,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 
> 
> r~
> 

Re: [RFC PATCH 11/15] exec/cpu: Make address_space_init/reloading_memory_map target agnostic

[Richard Henderson]

On 5/26/21 2:32 PM, Philippe Mathieu-Daudé wrote:
> On 5/26/21 9:01 PM, Richard Henderson wrote:
>> On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
>>> cpu_address_space_init() and cpu_reloading_memory_map() don't
>>> have to be target specific. Remove this limitation to be able
>>> to build softmmu/cpus.c once for all targets.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
>>> ---
>>>    include/exec/cpu-common.h | 23 +++++++++++++++++++++++
>>>    include/exec/exec-all.h   | 25 -------------------------
>>>    2 files changed, 23 insertions(+), 25 deletions(-)
>>
>> It's not clear to me why the declarations moved file, instead of just
>> droppig the surrounding ifdef.
> 
> hwaddr is target-specific, cpu_address_space_init don't uses it.

hwaddr is not target specific.


r~

Re: [RFC PATCH 14/15] softmmu/cpus: Extract QMP command handlers to cpus-qmp.c

[Richard Henderson]

On 5/26/21 2:35 PM, Philippe Mathieu-Daudé wrote:
> On 5/26/21 9:10 PM, Richard Henderson wrote:
>> On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
>>> qmp_memsave() and qmp_pmemsave() call cpu_memory_rw_debug()
>>> and cpu_physical_memory_read(), which are target specific
>>> prototypes.
>>
>> Is there any reason they should be?
> 
> They use target_ulong. Should they use hwaddr instead?

cpu_physical_memory_* should use hwaddr.

cpu_memory_rw_debug uses a virtual address, and so could probably use uint64_t, 
as per my comment vs patch 13 about not using hwaddr for virtual addresses.


r~

Re: [RFC PATCH 11/15] exec/cpu: Make address_space_init/reloading_memory_map target agnostic

[Philippe Mathieu-Daudé via]

On 26/5/21 21:01, Richard Henderson wrote:
> On 5/17/21 4:55 AM, Philippe Mathieu-Daudé wrote:
>> cpu_address_space_init() and cpu_reloading_memory_map() don't
>> have to be target specific. Remove this limitation to be able
>> to build softmmu/cpus.c once for all targets.
>>
>> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
>> ---
>>   include/exec/cpu-common.h | 23 +++++++++++++++++++++++
>>   include/exec/exec-all.h   | 25 -------------------------
>>   2 files changed, 23 insertions(+), 25 deletions(-)
> 
> It's not clear to me why the declarations moved file, instead of just 
> droppig the surrounding ifdef.
> 
> If there's a good reason, fine, but the reason belongs in the commit 
> message.

What about:

'''
cpu_address_space_init() and cpu_reloading_memory_map() are
target-agnostic, but are declared in "exec/exec-all.h" which
contains target-specific declarations. Any target-agnostic
source including "exec/exec-all.h" becomes target-specific
and we have to compile it N times for the N targets built.
In order to avoid that, move the declarations to "exec/cpu-common.h"
which only contains target-agnostic declarations.
'''