All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v1 0/5] s390x cleanup
@ 2021-03-22 19:15 Claudio Fontana
  2021-03-22 19:15 ` [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build Claudio Fontana
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Claudio Fontana @ 2021-03-22 19:15 UTC (permalink / raw)
  To: Cornelia Huck, Thomas Huth, Richard Henderson
  Cc: David Hildenbrand, qemu-devel, Halil Pasic,
	Christian Borntraeger, qemu-s390x, Claudio Fontana,
	Paolo Bonzini

Hi, I am starting a cleanup series for s390x,

with the goal of doing similar splits of KVM vs TCG,
sysemu vs user mode, as for the existing work on x86 and ARM.

S/390 target looks very good already, and seems much easier to work
with. I hope that with some patches it will be even better.

I will pile up more patches later on, but I start sharing something
here with these few RFC patches for your eyes,

they are based on the pre-requisite series:

https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg07461.html

Motivation and higher level steps:

https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg04628.html

Comments welcome, thanks,

Claudio

Claudio Fontana (5):
  hw/s390x: only build qemu-tod from the CONFIG_TCG build
  target/s390x: start moving TCG-only code to tcg/
  target/s390x: move sysemu-only code out to cpu-sysemu.c
  target/s390x: split cpu-dump from helper.c
  target/s390x: make helper.c sysemu-only

 include/hw/s390x/tod.h                        |   2 +-
 target/s390x/{internal.h => s390x-internal.h} |   6 +
 target/s390x/{ => tcg}/s390-tod.h             |   0
 target/s390x/{ => tcg}/tcg_s390x.h            |   0
 target/s390x/{ => tcg}/vec.h                  |   0
 hw/s390x/tod-qemu.c                           |   2 +-
 hw/s390x/tod.c                                |   9 +-
 target/s390x/arch_dump.c                      |   2 +-
 target/s390x/cpu-dump.c                       | 131 ++++++++
 target/s390x/cpu-sysemu.c                     | 304 ++++++++++++++++++
 target/s390x/cpu.c                            | 285 +---------------
 target/s390x/cpu_models.c                     |   2 +-
 target/s390x/diag.c                           |   2 +-
 target/s390x/gdbstub.c                        |   2 +-
 target/s390x/helper.c                         | 113 +------
 target/s390x/interrupt.c                      |   4 +-
 target/s390x/ioinst.c                         |   2 +-
 target/s390x/kvm.c                            |   2 +-
 target/s390x/machine.c                        |   4 +-
 target/s390x/mmu_helper.c                     |   2 +-
 target/s390x/sigp.c                           |   2 +-
 target/s390x/tcg-stub.c                       |  30 --
 target/s390x/{ => tcg}/cc_helper.c            |   2 +-
 target/s390x/{ => tcg}/crypto_helper.c        |   2 +-
 target/s390x/{ => tcg}/excp_helper.c          |   2 +-
 target/s390x/{ => tcg}/fpu_helper.c           |   2 +-
 target/s390x/{ => tcg}/int_helper.c           |   2 +-
 target/s390x/{ => tcg}/mem_helper.c           |   2 +-
 target/s390x/{ => tcg}/misc_helper.c          |   2 +-
 target/s390x/{ => tcg}/translate.c            |   2 +-
 target/s390x/{ => tcg}/vec_fpu_helper.c       |   2 +-
 target/s390x/{ => tcg}/vec_helper.c           |   2 +-
 target/s390x/{ => tcg}/vec_int_helper.c       |   0
 target/s390x/{ => tcg}/vec_string_helper.c    |   2 +-
 target/s390x/{ => tcg}/translate_vx.c.inc     |   0
 hw/s390x/meson.build                          |   5 +-
 target/s390x/meson.build                      |  21 +-
 target/s390x/{ => tcg}/insn-data.def          |   0
 target/s390x/{ => tcg}/insn-format.def        |   0
 target/s390x/tcg/meson.build                  |  14 +
 target/s390x/trace-events                     |   2 +-
 41 files changed, 512 insertions(+), 458 deletions(-)
 rename target/s390x/{internal.h => s390x-internal.h} (98%)
 rename target/s390x/{ => tcg}/s390-tod.h (100%)
 rename target/s390x/{ => tcg}/tcg_s390x.h (100%)
 rename target/s390x/{ => tcg}/vec.h (100%)
 create mode 100644 target/s390x/cpu-dump.c
 create mode 100644 target/s390x/cpu-sysemu.c
 delete mode 100644 target/s390x/tcg-stub.c
 rename target/s390x/{ => tcg}/cc_helper.c (99%)
 rename target/s390x/{ => tcg}/crypto_helper.c (98%)
 rename target/s390x/{ => tcg}/excp_helper.c (99%)
 rename target/s390x/{ => tcg}/fpu_helper.c (99%)
 rename target/s390x/{ => tcg}/int_helper.c (99%)
 rename target/s390x/{ => tcg}/mem_helper.c (99%)
 rename target/s390x/{ => tcg}/misc_helper.c (99%)
 rename target/s390x/{ => tcg}/translate.c (99%)
 rename target/s390x/{ => tcg}/vec_fpu_helper.c (99%)
 rename target/s390x/{ => tcg}/vec_helper.c (99%)
 rename target/s390x/{ => tcg}/vec_int_helper.c (100%)
 rename target/s390x/{ => tcg}/vec_string_helper.c (99%)
 rename target/s390x/{ => tcg}/translate_vx.c.inc (100%)
 rename target/s390x/{ => tcg}/insn-data.def (100%)
 rename target/s390x/{ => tcg}/insn-format.def (100%)
 create mode 100644 target/s390x/tcg/meson.build

-- 
2.26.2



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
  2021-03-22 19:15 [RFC v1 0/5] s390x cleanup Claudio Fontana
@ 2021-03-22 19:15 ` Claudio Fontana
  2021-03-31 11:07   ` Cornelia Huck
  2021-03-22 19:15 ` [RFC v1 2/5] target/s390x: start moving TCG-only code to tcg/ Claudio Fontana
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Claudio Fontana @ 2021-03-22 19:15 UTC (permalink / raw)
  To: Cornelia Huck, Thomas Huth, Richard Henderson
  Cc: David Hildenbrand, qemu-devel, Halil Pasic,
	Christian Borntraeger, qemu-s390x, Claudio Fontana,
	Paolo Bonzini

this allows to remove unneeded stubs for target/s390x.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 hw/s390x/tod.c       | 9 ++++++++-
 hw/s390x/meson.build | 5 ++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
index 3c2979175e..322732d7fd 100644
--- a/hw/s390x/tod.c
+++ b/hw/s390x/tod.c
@@ -14,6 +14,8 @@
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "sysemu/kvm.h"
+#include "sysemu/tcg.h"
+#include "sysemu/qtest.h"
 #include "migration/qemu-file-types.h"
 #include "migration/register.h"
 
@@ -23,8 +25,13 @@ void s390_init_tod(void)
 
     if (kvm_enabled()) {
         obj = object_new(TYPE_KVM_S390_TOD);
-    } else {
+    } else if (tcg_enabled()) {
         obj = object_new(TYPE_QEMU_S390_TOD);
+    } else if (qtest_enabled()) {
+        return;
+    } else {
+        warn_report("current accelerator not handled in s390_init_tod!");
+        return;
     }
     object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
     object_unref(obj);
diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
index 91495b5631..7f31f9e5d5 100644
--- a/hw/s390x/meson.build
+++ b/hw/s390x/meson.build
@@ -16,7 +16,6 @@ s390x_ss.add(files(
   'sclp.c',
   'sclpcpu.c',
   'sclpquiesce.c',
-  'tod-qemu.c',
   'tod.c',
 ))
 s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
@@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
   's390-stattrib-kvm.c',
   'pv.c',
 ))
+s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
+  'tod-qemu.c',
+))
+
 s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c'))
 s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c'))
 s390x_ss.add(when: 'CONFIG_VFIO', if_true: files('s390-pci-vfio.c'))
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC v1 2/5] target/s390x: start moving TCG-only code to tcg/
  2021-03-22 19:15 [RFC v1 0/5] s390x cleanup Claudio Fontana
  2021-03-22 19:15 ` [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build Claudio Fontana
@ 2021-03-22 19:15 ` Claudio Fontana
  2021-03-22 19:15 ` [RFC v1 3/5] target/s390x: move sysemu-only code out to cpu-sysemu.c Claudio Fontana
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Claudio Fontana @ 2021-03-22 19:15 UTC (permalink / raw)
  To: Cornelia Huck, Thomas Huth, Richard Henderson
  Cc: David Hildenbrand, qemu-devel, Halil Pasic,
	Christian Borntraeger, qemu-s390x, Claudio Fontana,
	Paolo Bonzini

move everything related to translate, as well as HELPER code in tcg/

mmu_helper.c stays put for now, as it contains both TCG and KVM code.

removed the tcg-stub.c file, as it seems unnecessary,
due to all calls being wrapped by if (tcg_enabled()),
and the prototype being available.

The internal.h file is renamed to s390x-internal.h, because from inside
tcg/ , the name collides with other files called the same way
(accel/tcg/internal.h).

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 include/hw/s390x/tod.h                        |  2 +-
 target/s390x/{internal.h => s390x-internal.h} |  0
 target/s390x/{ => tcg}/s390-tod.h             |  0
 target/s390x/{ => tcg}/tcg_s390x.h            |  0
 target/s390x/{ => tcg}/vec.h                  |  0
 hw/s390x/tod-qemu.c                           |  2 +-
 target/s390x/arch_dump.c                      |  2 +-
 target/s390x/cpu.c                            |  2 +-
 target/s390x/cpu_models.c                     |  2 +-
 target/s390x/diag.c                           |  2 +-
 target/s390x/gdbstub.c                        |  2 +-
 target/s390x/helper.c                         |  2 +-
 target/s390x/interrupt.c                      |  4 +--
 target/s390x/ioinst.c                         |  2 +-
 target/s390x/kvm.c                            |  2 +-
 target/s390x/machine.c                        |  4 +--
 target/s390x/mmu_helper.c                     |  2 +-
 target/s390x/sigp.c                           |  2 +-
 target/s390x/tcg-stub.c                       | 30 -------------------
 target/s390x/{ => tcg}/cc_helper.c            |  2 +-
 target/s390x/{ => tcg}/crypto_helper.c        |  2 +-
 target/s390x/{ => tcg}/excp_helper.c          |  2 +-
 target/s390x/{ => tcg}/fpu_helper.c           |  2 +-
 target/s390x/{ => tcg}/int_helper.c           |  2 +-
 target/s390x/{ => tcg}/mem_helper.c           |  2 +-
 target/s390x/{ => tcg}/misc_helper.c          |  2 +-
 target/s390x/{ => tcg}/translate.c            |  2 +-
 target/s390x/{ => tcg}/vec_fpu_helper.c       |  2 +-
 target/s390x/{ => tcg}/vec_helper.c           |  2 +-
 target/s390x/{ => tcg}/vec_int_helper.c       |  0
 target/s390x/{ => tcg}/vec_string_helper.c    |  2 +-
 target/s390x/{ => tcg}/translate_vx.c.inc     |  0
 target/s390x/meson.build                      | 17 ++---------
 target/s390x/{ => tcg}/insn-data.def          |  0
 target/s390x/{ => tcg}/insn-format.def        |  0
 target/s390x/tcg/meson.build                  | 14 +++++++++
 36 files changed, 43 insertions(+), 72 deletions(-)
 rename target/s390x/{internal.h => s390x-internal.h} (100%)
 rename target/s390x/{ => tcg}/s390-tod.h (100%)
 rename target/s390x/{ => tcg}/tcg_s390x.h (100%)
 rename target/s390x/{ => tcg}/vec.h (100%)
 delete mode 100644 target/s390x/tcg-stub.c
 rename target/s390x/{ => tcg}/cc_helper.c (99%)
 rename target/s390x/{ => tcg}/crypto_helper.c (98%)
 rename target/s390x/{ => tcg}/excp_helper.c (99%)
 rename target/s390x/{ => tcg}/fpu_helper.c (99%)
 rename target/s390x/{ => tcg}/int_helper.c (99%)
 rename target/s390x/{ => tcg}/mem_helper.c (99%)
 rename target/s390x/{ => tcg}/misc_helper.c (99%)
 rename target/s390x/{ => tcg}/translate.c (99%)
 rename target/s390x/{ => tcg}/vec_fpu_helper.c (99%)
 rename target/s390x/{ => tcg}/vec_helper.c (99%)
 rename target/s390x/{ => tcg}/vec_int_helper.c (100%)
 rename target/s390x/{ => tcg}/vec_string_helper.c (99%)
 rename target/s390x/{ => tcg}/translate_vx.c.inc (100%)
 rename target/s390x/{ => tcg}/insn-data.def (100%)
 rename target/s390x/{ => tcg}/insn-format.def (100%)
 create mode 100644 target/s390x/tcg/meson.build

diff --git a/include/hw/s390x/tod.h b/include/hw/s390x/tod.h
index ff3195a4bf..0935e85089 100644
--- a/include/hw/s390x/tod.h
+++ b/include/hw/s390x/tod.h
@@ -12,7 +12,7 @@
 #define HW_S390_TOD_H
 
 #include "hw/qdev-core.h"
-#include "target/s390x/s390-tod.h"
+#include "tcg/s390-tod.h"
 #include "qom/object.h"
 
 typedef struct S390TOD {
diff --git a/target/s390x/internal.h b/target/s390x/s390x-internal.h
similarity index 100%
rename from target/s390x/internal.h
rename to target/s390x/s390x-internal.h
diff --git a/target/s390x/s390-tod.h b/target/s390x/tcg/s390-tod.h
similarity index 100%
rename from target/s390x/s390-tod.h
rename to target/s390x/tcg/s390-tod.h
diff --git a/target/s390x/tcg_s390x.h b/target/s390x/tcg/tcg_s390x.h
similarity index 100%
rename from target/s390x/tcg_s390x.h
rename to target/s390x/tcg/tcg_s390x.h
diff --git a/target/s390x/vec.h b/target/s390x/tcg/vec.h
similarity index 100%
rename from target/s390x/vec.h
rename to target/s390x/tcg/vec.h
diff --git a/hw/s390x/tod-qemu.c b/hw/s390x/tod-qemu.c
index e91b9590f5..4b3e65050a 100644
--- a/hw/s390x/tod-qemu.c
+++ b/hw/s390x/tod-qemu.c
@@ -16,7 +16,7 @@
 #include "qemu/cutils.h"
 #include "qemu/module.h"
 #include "cpu.h"
-#include "tcg_s390x.h"
+#include "tcg/tcg_s390x.h"
 
 static void qemu_s390_tod_get(const S390TODState *td, S390TOD *tod,
                               Error **errp)
diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
index cc1330876b..08daf93ae1 100644
--- a/target/s390x/arch_dump.c
+++ b/target/s390x/arch_dump.c
@@ -13,7 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "elf.h"
 #include "sysemu/dump.h"
 
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index d35eb39a1b..ae054d264b 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -23,7 +23,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "kvm_s390x.h"
 #include "sysemu/kvm.h"
 #include "sysemu/reset.h"
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 050dcf2d42..4ff8cba7e5 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -12,7 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "kvm_s390x.h"
 #include "sysemu/kvm.h"
 #include "sysemu/tcg.h"
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index 1a48429564..86b7032b5b 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -14,7 +14,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "exec/address-spaces.h"
 #include "hw/watchdog/wdt_diag288.h"
 #include "sysemu/cpus.h"
diff --git a/target/s390x/gdbstub.c b/target/s390x/gdbstub.c
index d6fce5ff1e..1dbe2973f4 100644
--- a/target/s390x/gdbstub.c
+++ b/target/s390x/gdbstub.c
@@ -20,7 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "exec/exec-all.h"
 #include "exec/gdbstub.h"
 #include "qemu/bitops.h"
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 7678994feb..2254873cef 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -20,7 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "exec/gdbstub.h"
 #include "qemu/timer.h"
 #include "qemu/qemu-print.h"
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index 4cdbbc8849..d0e58d6e8d 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -11,12 +11,12 @@
 #include "qemu/log.h"
 #include "cpu.h"
 #include "kvm_s390x.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "exec/exec-all.h"
 #include "sysemu/kvm.h"
 #include "sysemu/tcg.h"
 #include "hw/s390x/ioinst.h"
-#include "tcg_s390x.h"
+#include "tcg/tcg_s390x.h"
 #if !defined(CONFIG_USER_ONLY)
 #include "hw/s390x/s390_flic.h"
 #endif
diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index 1ee11522e1..4eb0a7a9f8 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -12,7 +12,7 @@
 #include "qemu/osdep.h"
 
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "hw/s390x/ioinst.h"
 #include "trace.h"
 #include "hw/s390x/s390-pci-bus.h"
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 4fb3bbfef5..2a22cc69f6 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -26,7 +26,7 @@
 
 #include "qemu-common.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "kvm_s390x.h"
 #include "sysemu/kvm_int.h"
 #include "qemu/cutils.h"
diff --git a/target/s390x/machine.c b/target/s390x/machine.c
index 5b4e82f1ab..81a8a7ff99 100644
--- a/target/s390x/machine.c
+++ b/target/s390x/machine.c
@@ -16,10 +16,10 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "kvm_s390x.h"
 #include "migration/vmstate.h"
-#include "tcg_s390x.h"
+#include "tcg/tcg_s390x.h"
 #include "sysemu/kvm.h"
 #include "sysemu/tcg.h"
 
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index d492b23a17..52fdd86c63 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -19,7 +19,7 @@
 #include "qemu/error-report.h"
 #include "exec/address-spaces.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "kvm_s390x.h"
 #include "sysemu/kvm.h"
 #include "sysemu/tcg.h"
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index c604f17710..320dddbae2 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -10,7 +10,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "sysemu/hw_accel.h"
 #include "sysemu/runstate.h"
 #include "exec/address-spaces.h"
diff --git a/target/s390x/tcg-stub.c b/target/s390x/tcg-stub.c
deleted file mode 100644
index d22c898802..0000000000
--- a/target/s390x/tcg-stub.c
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * QEMU TCG support -- s390x specific function stubs.
- *
- * Copyright (C) 2018 Red Hat Inc
- *
- * Authors:
- *   David Hildenbrand <david@redhat.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- */
-
-#include "qemu/osdep.h"
-#include "qemu-common.h"
-#include "cpu.h"
-#include "tcg_s390x.h"
-
-void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque)
-{
-}
-void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env,
-                                              uint32_t code, uintptr_t ra)
-{
-    g_assert_not_reached();
-}
-void QEMU_NORETURN tcg_s390_data_exception(CPUS390XState *env, uint32_t dxc,
-                                           uintptr_t ra)
-{
-    g_assert_not_reached();
-}
diff --git a/target/s390x/cc_helper.c b/target/s390x/tcg/cc_helper.c
similarity index 99%
rename from target/s390x/cc_helper.c
rename to target/s390x/tcg/cc_helper.c
index e7039d0d18..f0663f7a3e 100644
--- a/target/s390x/cc_helper.c
+++ b/target/s390x/tcg/cc_helper.c
@@ -20,7 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "tcg_s390x.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
diff --git a/target/s390x/crypto_helper.c b/target/s390x/tcg/crypto_helper.c
similarity index 98%
rename from target/s390x/crypto_helper.c
rename to target/s390x/tcg/crypto_helper.c
index ff3fbc3950..138d9e7ad9 100644
--- a/target/s390x/crypto_helper.c
+++ b/target/s390x/tcg/crypto_helper.c
@@ -12,7 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "tcg_s390x.h"
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
diff --git a/target/s390x/excp_helper.c b/target/s390x/tcg/excp_helper.c
similarity index 99%
rename from target/s390x/excp_helper.c
rename to target/s390x/tcg/excp_helper.c
index c48cd6b46f..ecc830cea0 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/tcg/excp_helper.c
@@ -20,7 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "exec/helper-proto.h"
 #include "qemu/timer.h"
 #include "exec/exec-all.h"
diff --git a/target/s390x/fpu_helper.c b/target/s390x/tcg/fpu_helper.c
similarity index 99%
rename from target/s390x/fpu_helper.c
rename to target/s390x/tcg/fpu_helper.c
index f155bc048c..bc89ef7cc8 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/tcg/fpu_helper.c
@@ -20,7 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "tcg_s390x.h"
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
diff --git a/target/s390x/int_helper.c b/target/s390x/tcg/int_helper.c
similarity index 99%
rename from target/s390x/int_helper.c
rename to target/s390x/tcg/int_helper.c
index 658507dd6d..954542388a 100644
--- a/target/s390x/int_helper.c
+++ b/target/s390x/tcg/int_helper.c
@@ -20,7 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "tcg_s390x.h"
 #include "exec/exec-all.h"
 #include "qemu/host-utils.h"
diff --git a/target/s390x/mem_helper.c b/target/s390x/tcg/mem_helper.c
similarity index 99%
rename from target/s390x/mem_helper.c
rename to target/s390x/tcg/mem_helper.c
index 12e84a4285..a91791a1b7 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -20,7 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "tcg_s390x.h"
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
diff --git a/target/s390x/misc_helper.c b/target/s390x/tcg/misc_helper.c
similarity index 99%
rename from target/s390x/misc_helper.c
rename to target/s390x/tcg/misc_helper.c
index 7ea90d414a..33e6999e15 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -22,7 +22,7 @@
 #include "qemu/cutils.h"
 #include "qemu/main-loop.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "exec/memory.h"
 #include "qemu/host-utils.h"
 #include "exec/helper-proto.h"
diff --git a/target/s390x/translate.c b/target/s390x/tcg/translate.c
similarity index 99%
rename from target/s390x/translate.c
rename to target/s390x/tcg/translate.c
index 4f953ddfba..ac236888a0 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -30,7 +30,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "disas/disas.h"
 #include "exec/exec-all.h"
 #include "tcg/tcg-op.h"
diff --git a/target/s390x/vec_fpu_helper.c b/target/s390x/tcg/vec_fpu_helper.c
similarity index 99%
rename from target/s390x/vec_fpu_helper.c
rename to target/s390x/tcg/vec_fpu_helper.c
index c1564e819b..850fac721e 100644
--- a/target/s390x/vec_fpu_helper.c
+++ b/target/s390x/tcg/vec_fpu_helper.c
@@ -12,7 +12,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "vec.h"
 #include "tcg_s390x.h"
 #include "tcg/tcg-gvec-desc.h"
diff --git a/target/s390x/vec_helper.c b/target/s390x/tcg/vec_helper.c
similarity index 99%
rename from target/s390x/vec_helper.c
rename to target/s390x/tcg/vec_helper.c
index 986e7cc825..ddd0a8be5b 100644
--- a/target/s390x/vec_helper.c
+++ b/target/s390x/tcg/vec_helper.c
@@ -11,7 +11,7 @@
  */
 #include "qemu/osdep.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "vec.h"
 #include "tcg/tcg.h"
 #include "tcg/tcg-gvec-desc.h"
diff --git a/target/s390x/vec_int_helper.c b/target/s390x/tcg/vec_int_helper.c
similarity index 100%
rename from target/s390x/vec_int_helper.c
rename to target/s390x/tcg/vec_int_helper.c
diff --git a/target/s390x/vec_string_helper.c b/target/s390x/tcg/vec_string_helper.c
similarity index 99%
rename from target/s390x/vec_string_helper.c
rename to target/s390x/tcg/vec_string_helper.c
index c516c0ceeb..ac315eb095 100644
--- a/target/s390x/vec_string_helper.c
+++ b/target/s390x/tcg/vec_string_helper.c
@@ -12,7 +12,7 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "cpu.h"
-#include "internal.h"
+#include "s390x-internal.h"
 #include "vec.h"
 #include "tcg/tcg.h"
 #include "tcg/tcg-gvec-desc.h"
diff --git a/target/s390x/translate_vx.c.inc b/target/s390x/tcg/translate_vx.c.inc
similarity index 100%
rename from target/s390x/translate_vx.c.inc
rename to target/s390x/tcg/translate_vx.c.inc
diff --git a/target/s390x/meson.build b/target/s390x/meson.build
index 1219f64112..60d7f1b908 100644
--- a/target/s390x/meson.build
+++ b/target/s390x/meson.build
@@ -8,21 +8,6 @@ s390x_ss.add(files(
   'interrupt.c',
 ))
 
-s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
-  'cc_helper.c',
-  'crypto_helper.c',
-  'excp_helper.c',
-  'fpu_helper.c',
-  'int_helper.c',
-  'mem_helper.c',
-  'misc_helper.c',
-  'translate.c',
-  'vec_fpu_helper.c',
-  'vec_helper.c',
-  'vec_int_helper.c',
-  'vec_string_helper.c',
-), if_false: files('tcg-stub.c'))
-
 s390x_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c'))
 
 gen_features = executable('gen-features', 'gen-features.c', native: true,
@@ -60,6 +45,8 @@ endif
 
 s390x_user_ss = ss.source_set()
 
+subdir('tcg')
+
 target_arch += {'s390x': s390x_ss}
 target_softmmu_arch += {'s390x': s390x_softmmu_ss}
 target_user_arch += {'s390x': s390x_user_ss}
diff --git a/target/s390x/insn-data.def b/target/s390x/tcg/insn-data.def
similarity index 100%
rename from target/s390x/insn-data.def
rename to target/s390x/tcg/insn-data.def
diff --git a/target/s390x/insn-format.def b/target/s390x/tcg/insn-format.def
similarity index 100%
rename from target/s390x/insn-format.def
rename to target/s390x/tcg/insn-format.def
diff --git a/target/s390x/tcg/meson.build b/target/s390x/tcg/meson.build
new file mode 100644
index 0000000000..ee4e8fec77
--- /dev/null
+++ b/target/s390x/tcg/meson.build
@@ -0,0 +1,14 @@
+s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
+  'cc_helper.c',
+  'crypto_helper.c',
+  'excp_helper.c',
+  'fpu_helper.c',
+  'int_helper.c',
+  'mem_helper.c',
+  'misc_helper.c',
+  'translate.c',
+  'vec_fpu_helper.c',
+  'vec_helper.c',
+  'vec_int_helper.c',
+  'vec_string_helper.c',
+))
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC v1 3/5] target/s390x: move sysemu-only code out to cpu-sysemu.c
  2021-03-22 19:15 [RFC v1 0/5] s390x cleanup Claudio Fontana
  2021-03-22 19:15 ` [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build Claudio Fontana
  2021-03-22 19:15 ` [RFC v1 2/5] target/s390x: start moving TCG-only code to tcg/ Claudio Fontana
@ 2021-03-22 19:15 ` Claudio Fontana
  2021-03-22 19:15 ` [RFC v1 4/5] target/s390x: split cpu-dump from helper.c Claudio Fontana
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Claudio Fontana @ 2021-03-22 19:15 UTC (permalink / raw)
  To: Cornelia Huck, Thomas Huth, Richard Henderson
  Cc: David Hildenbrand, qemu-devel, Halil Pasic,
	Christian Borntraeger, qemu-s390x, Claudio Fontana,
	Paolo Bonzini

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 target/s390x/s390x-internal.h |   6 +
 target/s390x/cpu-sysemu.c     | 304 ++++++++++++++++++++++++++++++++++
 target/s390x/cpu.c            | 283 ++-----------------------------
 target/s390x/meson.build      |   1 +
 target/s390x/trace-events     |   2 +-
 5 files changed, 324 insertions(+), 272 deletions(-)
 create mode 100644 target/s390x/cpu-sysemu.c

diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
index 11515bb617..171ecd59fb 100644
--- a/target/s390x/s390x-internal.h
+++ b/target/s390x/s390x-internal.h
@@ -244,6 +244,12 @@ void load_psw(CPUS390XState *env, uint64_t mask, uint64_t addr);
 #ifndef CONFIG_USER_ONLY
 unsigned int s390_cpu_halt(S390CPU *cpu);
 void s390_cpu_unhalt(S390CPU *cpu);
+void s390_cpu_init_sysemu(Object *obj);
+bool s390_cpu_realize_sysemu(DeviceState *dev, Error **errp);
+void s390_cpu_finalize(Object *obj);
+void s390_cpu_class_init_sysemu(CPUClass *cc);
+void s390_cpu_machine_reset_cb(void *opaque);
+
 #else
 static inline unsigned int s390_cpu_halt(S390CPU *cpu)
 {
diff --git a/target/s390x/cpu-sysemu.c b/target/s390x/cpu-sysemu.c
new file mode 100644
index 0000000000..6081b7ef32
--- /dev/null
+++ b/target/s390x/cpu-sysemu.c
@@ -0,0 +1,304 @@
+/*
+ * QEMU S/390 CPU - System Emulation-only code
+ *
+ * Copyright (c) 2009 Ulrich Hecht
+ * Copyright (c) 2011 Alexander Graf
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ * Copyright (c) 2012 IBM Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "cpu.h"
+#include "s390x-internal.h"
+#include "kvm_s390x.h"
+#include "sysemu/kvm.h"
+#include "sysemu/reset.h"
+#include "qemu/timer.h"
+#include "trace.h"
+#include "qapi/qapi-visit-run-state.h"
+#include "sysemu/hw_accel.h"
+
+#include "hw/s390x/pv.h"
+#include "hw/boards.h"
+#include "sysemu/arch_init.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/tcg.h"
+
+/* S390CPUClass::load_normal() */
+static void s390_cpu_load_normal(CPUState *s)
+{
+    S390CPU *cpu = S390_CPU(s);
+    uint64_t spsw;
+
+    if (!s390_is_pv()) {
+        spsw = ldq_phys(s->as, 0);
+        cpu->env.psw.mask = spsw & PSW_MASK_SHORT_CTRL;
+        /*
+         * Invert short psw indication, so SIE will report a specification
+         * exception if it was not set.
+         */
+        cpu->env.psw.mask ^= PSW_MASK_SHORTPSW;
+        cpu->env.psw.addr = spsw & PSW_MASK_SHORT_ADDR;
+    } else {
+        /*
+         * Firmware requires us to set the load state before we set
+         * the cpu to operating on protected guests.
+         */
+        s390_cpu_set_state(S390_CPU_STATE_LOAD, cpu);
+    }
+    s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
+}
+
+void s390_cpu_machine_reset_cb(void *opaque)
+{
+    S390CPU *cpu = opaque;
+
+    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
+}
+
+static GuestPanicInformation *s390_cpu_get_crash_info(CPUState *cs)
+{
+    GuestPanicInformation *panic_info;
+    S390CPU *cpu = S390_CPU(cs);
+
+    cpu_synchronize_state(cs);
+    panic_info = g_malloc0(sizeof(GuestPanicInformation));
+
+    panic_info->type = GUEST_PANIC_INFORMATION_TYPE_S390;
+    panic_info->u.s390.core = cpu->env.core_id;
+    panic_info->u.s390.psw_mask = cpu->env.psw.mask;
+    panic_info->u.s390.psw_addr = cpu->env.psw.addr;
+    panic_info->u.s390.reason = cpu->env.crash_reason;
+
+    return panic_info;
+}
+
+static void s390_cpu_get_crash_info_qom(Object *obj, Visitor *v,
+                                        const char *name, void *opaque,
+                                        Error **errp)
+{
+    CPUState *cs = CPU(obj);
+    GuestPanicInformation *panic_info;
+
+    if (!cs->crash_occurred) {
+        error_setg(errp, "No crash occurred");
+        return;
+    }
+
+    panic_info = s390_cpu_get_crash_info(cs);
+
+    visit_type_GuestPanicInformation(v, "crash-information", &panic_info,
+                                     errp);
+    qapi_free_GuestPanicInformation(panic_info);
+}
+
+void s390_cpu_init_sysemu(Object *obj)
+{
+    CPUState *cs = CPU(obj);
+    S390CPU *cpu = S390_CPU(obj);
+
+    cs->start_powered_off = true;
+    object_property_add(obj, "crash-information", "GuestPanicInformation",
+                        s390_cpu_get_crash_info_qom, NULL, NULL, NULL);
+    cpu->env.tod_timer =
+        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
+    cpu->env.cpu_timer =
+        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
+    s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
+}
+
+bool s390_cpu_realize_sysemu(DeviceState *dev, Error **errp)
+{
+    S390CPU *cpu = S390_CPU(dev);
+    MachineState *ms = MACHINE(qdev_get_machine());
+    unsigned int max_cpus = ms->smp.max_cpus;
+
+    if (cpu->env.core_id >= max_cpus) {
+        error_setg(errp, "Unable to add CPU with core-id: %" PRIu32
+                   ", maximum core-id: %d", cpu->env.core_id,
+                   max_cpus - 1);
+        return false;
+    }
+
+    if (cpu_exists(cpu->env.core_id)) {
+        error_setg(errp, "Unable to add CPU with core-id: %" PRIu32
+                   ", it already exists", cpu->env.core_id);
+        return false;
+    }
+
+    /* sync cs->cpu_index and env->core_id. The latter is needed for TCG. */
+    CPU(cpu)->cpu_index = cpu->env.core_id;
+    return true;
+}
+
+void s390_cpu_finalize(Object *obj)
+{
+    S390CPU *cpu = S390_CPU(obj);
+
+    timer_free(cpu->env.tod_timer);
+    timer_free(cpu->env.cpu_timer);
+
+    qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu);
+    g_free(cpu->irqstate);
+}
+
+void s390_cpu_class_init_sysemu(CPUClass *cc)
+{
+    S390CPUClass *scc = S390_CPU_CLASS(cc);
+
+    scc->load_normal = s390_cpu_load_normal;
+    cc->get_phys_page_debug = s390_cpu_get_phys_page_debug;
+    cc->vmsd = &vmstate_s390_cpu;
+    cc->get_crash_info = s390_cpu_get_crash_info;
+    cc->write_elf64_note = s390_cpu_write_elf64_note;
+}
+
+static bool disabled_wait(CPUState *cpu)
+{
+    return cpu->halted && !(S390_CPU(cpu)->env.psw.mask &
+                            (PSW_MASK_IO | PSW_MASK_EXT | PSW_MASK_MCHECK));
+}
+
+static unsigned s390_count_running_cpus(void)
+{
+    CPUState *cpu;
+    int nr_running = 0;
+
+    CPU_FOREACH(cpu) {
+        uint8_t state = S390_CPU(cpu)->env.cpu_state;
+        if (state == S390_CPU_STATE_OPERATING ||
+            state == S390_CPU_STATE_LOAD) {
+            if (!disabled_wait(cpu)) {
+                nr_running++;
+            }
+        }
+    }
+
+    return nr_running;
+}
+
+unsigned int s390_cpu_halt(S390CPU *cpu)
+{
+    CPUState *cs = CPU(cpu);
+    trace_cpu_halt(cs->cpu_index);
+
+    if (!cs->halted) {
+        cs->halted = 1;
+        cs->exception_index = EXCP_HLT;
+    }
+
+    return s390_count_running_cpus();
+}
+
+void s390_cpu_unhalt(S390CPU *cpu)
+{
+    CPUState *cs = CPU(cpu);
+    trace_cpu_unhalt(cs->cpu_index);
+
+    if (cs->halted) {
+        cs->halted = 0;
+        cs->exception_index = -1;
+    }
+}
+
+unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
+ {
+    trace_cpu_set_state(CPU(cpu)->cpu_index, cpu_state);
+
+    switch (cpu_state) {
+    case S390_CPU_STATE_STOPPED:
+    case S390_CPU_STATE_CHECK_STOP:
+        /* halt the cpu for common infrastructure */
+        s390_cpu_halt(cpu);
+        break;
+    case S390_CPU_STATE_OPERATING:
+    case S390_CPU_STATE_LOAD:
+        /*
+         * Starting a CPU with a PSW WAIT bit set:
+         * KVM: handles this internally and triggers another WAIT exit.
+         * TCG: will actually try to continue to run. Don't unhalt, will
+         *      be done when the CPU actually has work (an interrupt).
+         */
+        if (!tcg_enabled() || !(cpu->env.psw.mask & PSW_MASK_WAIT)) {
+            s390_cpu_unhalt(cpu);
+        }
+        break;
+    default:
+        error_report("Requested CPU state is not a valid S390 CPU state: %u",
+                     cpu_state);
+        exit(1);
+    }
+    if (kvm_enabled() && cpu->env.cpu_state != cpu_state) {
+        kvm_s390_set_cpu_state(cpu, cpu_state);
+    }
+    cpu->env.cpu_state = cpu_state;
+
+    return s390_count_running_cpus();
+}
+
+int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit)
+{
+    if (kvm_enabled()) {
+        return kvm_s390_set_mem_limit(new_limit, hw_limit);
+    }
+    return 0;
+}
+
+void s390_set_max_pagesize(uint64_t pagesize, Error **errp)
+{
+    if (kvm_enabled()) {
+        kvm_s390_set_max_pagesize(pagesize, errp);
+    }
+}
+
+void s390_cmma_reset(void)
+{
+    if (kvm_enabled()) {
+        kvm_s390_cmma_reset();
+    }
+}
+
+int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,
+                                int vq, bool assign)
+{
+    if (kvm_enabled()) {
+        return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
+    } else {
+        return 0;
+    }
+}
+
+void s390_crypto_reset(void)
+{
+    if (kvm_enabled()) {
+        kvm_s390_crypto_reset();
+    }
+}
+
+void s390_enable_css_support(S390CPU *cpu)
+{
+    if (kvm_enabled()) {
+        kvm_s390_enable_css_support(cpu);
+    }
+}
+
+void s390_do_cpu_set_diag318(CPUState *cs, run_on_cpu_data arg)
+{
+    if (kvm_enabled()) {
+        kvm_s390_set_diag318(cs, arg.host_ulong);
+    }
+}
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index ae054d264b..59efe48bcd 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -27,22 +27,11 @@
 #include "kvm_s390x.h"
 #include "sysemu/kvm.h"
 #include "sysemu/reset.h"
-#include "qemu/timer.h"
-#include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "trace.h"
-#include "qapi/visitor.h"
 #include "qapi/qapi-types-machine.h"
-#include "qapi/qapi-visit-run-state.h"
 #include "sysemu/hw_accel.h"
 #include "hw/qdev-properties.h"
-#ifndef CONFIG_USER_ONLY
-#include "hw/s390x/pv.h"
-#include "hw/boards.h"
-#include "sysemu/arch_init.h"
-#include "sysemu/sysemu.h"
-#include "sysemu/tcg.h"
-#endif
 #include "fpu/softfloat-helpers.h"
 #include "disas/capstone.h"
 
@@ -73,33 +62,6 @@ static bool s390_cpu_has_work(CPUState *cs)
     return s390_cpu_has_int(cpu);
 }
 
-#if !defined(CONFIG_USER_ONLY)
-/* S390CPUClass::load_normal() */
-static void s390_cpu_load_normal(CPUState *s)
-{
-    S390CPU *cpu = S390_CPU(s);
-    uint64_t spsw;
-
-    if (!s390_is_pv()) {
-        spsw = ldq_phys(s->as, 0);
-        cpu->env.psw.mask = spsw & PSW_MASK_SHORT_CTRL;
-        /*
-         * Invert short psw indication, so SIE will report a specification
-         * exception if it was not set.
-         */
-        cpu->env.psw.mask ^= PSW_MASK_SHORTPSW;
-        cpu->env.psw.addr = spsw & PSW_MASK_SHORT_ADDR;
-    } else {
-        /*
-         * Firmware requires us to set the load state before we set
-         * the cpu to operating on protected guests.
-         */
-        s390_cpu_set_state(S390_CPU_STATE_LOAD, cpu);
-    }
-    s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
-}
-#endif
-
 /* S390CPUClass::reset() */
 static void s390_cpu_reset(CPUState *s, cpu_reset_type type)
 {
@@ -170,15 +132,6 @@ static void s390_cpu_reset(CPUState *s, cpu_reset_type type)
     }
 }
 
-#if !defined(CONFIG_USER_ONLY)
-static void s390_cpu_machine_reset_cb(void *opaque)
-{
-    S390CPU *cpu = opaque;
-
-    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, RUN_ON_CPU_NULL);
-}
-#endif
-
 static void s390_cpu_disas_set_info(CPUState *cpu, disassemble_info *info)
 {
     info->mach = bfd_mach_s390_64;
@@ -192,9 +145,6 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
     S390CPUClass *scc = S390_CPU_GET_CLASS(dev);
-#if !defined(CONFIG_USER_ONLY)
-    S390CPU *cpu = S390_CPU(dev);
-#endif
     Error *err = NULL;
 
     /* the model has to be realized before qemu_init_vcpu() due to kvm */
@@ -204,23 +154,9 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
 #if !defined(CONFIG_USER_ONLY)
-    MachineState *ms = MACHINE(qdev_get_machine());
-    unsigned int max_cpus = ms->smp.max_cpus;
-    if (cpu->env.core_id >= max_cpus) {
-        error_setg(&err, "Unable to add CPU with core-id: %" PRIu32
-                   ", maximum core-id: %d", cpu->env.core_id,
-                   max_cpus - 1);
+    if (!s390_cpu_realize_sysemu(dev, &err)) {
         goto out;
     }
-
-    if (cpu_exists(cpu->env.core_id)) {
-        error_setg(&err, "Unable to add CPU with core-id: %" PRIu32
-                   ", it already exists", cpu->env.core_id);
-        goto out;
-    }
-
-    /* sync cs->cpu_index and env->core_id. The latter is needed for TCG. */
-    cs->cpu_index = cpu->env.core_id;
 #endif
 
     cpu_exec_realizefn(cs, &err);
@@ -229,7 +165,7 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
 #if !defined(CONFIG_USER_ONLY)
-    qemu_register_reset(s390_cpu_machine_reset_cb, cpu);
+    qemu_register_reset(s390_cpu_machine_reset_cb, S390_CPU(dev));
 #endif
     s390_cpu_gdb_init(cs);
     qemu_init_vcpu(cs);
@@ -251,44 +187,6 @@ out:
     error_propagate(errp, err);
 }
 
-#if !defined(CONFIG_USER_ONLY)
-static GuestPanicInformation *s390_cpu_get_crash_info(CPUState *cs)
-{
-    GuestPanicInformation *panic_info;
-    S390CPU *cpu = S390_CPU(cs);
-
-    cpu_synchronize_state(cs);
-    panic_info = g_malloc0(sizeof(GuestPanicInformation));
-
-    panic_info->type = GUEST_PANIC_INFORMATION_TYPE_S390;
-    panic_info->u.s390.core = cpu->env.core_id;
-    panic_info->u.s390.psw_mask = cpu->env.psw.mask;
-    panic_info->u.s390.psw_addr = cpu->env.psw.addr;
-    panic_info->u.s390.reason = cpu->env.crash_reason;
-
-    return panic_info;
-}
-
-static void s390_cpu_get_crash_info_qom(Object *obj, Visitor *v,
-                                        const char *name, void *opaque,
-                                        Error **errp)
-{
-    CPUState *cs = CPU(obj);
-    GuestPanicInformation *panic_info;
-
-    if (!cs->crash_occurred) {
-        error_setg(errp, "No crash occurred");
-        return;
-    }
-
-    panic_info = s390_cpu_get_crash_info(cs);
-
-    visit_type_GuestPanicInformation(v, "crash-information", &panic_info,
-                                     errp);
-    qapi_free_GuestPanicInformation(panic_info);
-}
-#endif
-
 static void s390_cpu_initfn(Object *obj)
 {
     CPUState *cs = CPU(obj);
@@ -296,169 +194,12 @@ static void s390_cpu_initfn(Object *obj)
 
     cpu_set_cpustate_pointers(cpu);
     cs->exception_index = EXCP_HLT;
-#if !defined(CONFIG_USER_ONLY)
-    cs->start_powered_off = true;
-    object_property_add(obj, "crash-information", "GuestPanicInformation",
-                        s390_cpu_get_crash_info_qom, NULL, NULL, NULL);
-    cpu->env.tod_timer =
-        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
-    cpu->env.cpu_timer =
-        timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
-    s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
-#endif
-}
 
-static void s390_cpu_finalize(Object *obj)
-{
 #if !defined(CONFIG_USER_ONLY)
-    S390CPU *cpu = S390_CPU(obj);
-
-    timer_free(cpu->env.tod_timer);
-    timer_free(cpu->env.cpu_timer);
-
-    qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu);
-    g_free(cpu->irqstate);
+    s390_cpu_init_sysemu(obj);
 #endif
 }
 
-#if !defined(CONFIG_USER_ONLY)
-static bool disabled_wait(CPUState *cpu)
-{
-    return cpu->halted && !(S390_CPU(cpu)->env.psw.mask &
-                            (PSW_MASK_IO | PSW_MASK_EXT | PSW_MASK_MCHECK));
-}
-
-static unsigned s390_count_running_cpus(void)
-{
-    CPUState *cpu;
-    int nr_running = 0;
-
-    CPU_FOREACH(cpu) {
-        uint8_t state = S390_CPU(cpu)->env.cpu_state;
-        if (state == S390_CPU_STATE_OPERATING ||
-            state == S390_CPU_STATE_LOAD) {
-            if (!disabled_wait(cpu)) {
-                nr_running++;
-            }
-        }
-    }
-
-    return nr_running;
-}
-
-unsigned int s390_cpu_halt(S390CPU *cpu)
-{
-    CPUState *cs = CPU(cpu);
-    trace_cpu_halt(cs->cpu_index);
-
-    if (!cs->halted) {
-        cs->halted = 1;
-        cs->exception_index = EXCP_HLT;
-    }
-
-    return s390_count_running_cpus();
-}
-
-void s390_cpu_unhalt(S390CPU *cpu)
-{
-    CPUState *cs = CPU(cpu);
-    trace_cpu_unhalt(cs->cpu_index);
-
-    if (cs->halted) {
-        cs->halted = 0;
-        cs->exception_index = -1;
-    }
-}
-
-unsigned int s390_cpu_set_state(uint8_t cpu_state, S390CPU *cpu)
- {
-    trace_cpu_set_state(CPU(cpu)->cpu_index, cpu_state);
-
-    switch (cpu_state) {
-    case S390_CPU_STATE_STOPPED:
-    case S390_CPU_STATE_CHECK_STOP:
-        /* halt the cpu for common infrastructure */
-        s390_cpu_halt(cpu);
-        break;
-    case S390_CPU_STATE_OPERATING:
-    case S390_CPU_STATE_LOAD:
-        /*
-         * Starting a CPU with a PSW WAIT bit set:
-         * KVM: handles this internally and triggers another WAIT exit.
-         * TCG: will actually try to continue to run. Don't unhalt, will
-         *      be done when the CPU actually has work (an interrupt).
-         */
-        if (!tcg_enabled() || !(cpu->env.psw.mask & PSW_MASK_WAIT)) {
-            s390_cpu_unhalt(cpu);
-        }
-        break;
-    default:
-        error_report("Requested CPU state is not a valid S390 CPU state: %u",
-                     cpu_state);
-        exit(1);
-    }
-    if (kvm_enabled() && cpu->env.cpu_state != cpu_state) {
-        kvm_s390_set_cpu_state(cpu, cpu_state);
-    }
-    cpu->env.cpu_state = cpu_state;
-
-    return s390_count_running_cpus();
-}
-
-int s390_set_memory_limit(uint64_t new_limit, uint64_t *hw_limit)
-{
-    if (kvm_enabled()) {
-        return kvm_s390_set_mem_limit(new_limit, hw_limit);
-    }
-    return 0;
-}
-
-void s390_set_max_pagesize(uint64_t pagesize, Error **errp)
-{
-    if (kvm_enabled()) {
-        kvm_s390_set_max_pagesize(pagesize, errp);
-    }
-}
-
-void s390_cmma_reset(void)
-{
-    if (kvm_enabled()) {
-        kvm_s390_cmma_reset();
-    }
-}
-
-int s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch_id,
-                                int vq, bool assign)
-{
-    if (kvm_enabled()) {
-        return kvm_s390_assign_subch_ioeventfd(notifier, sch_id, vq, assign);
-    } else {
-        return 0;
-    }
-}
-
-void s390_crypto_reset(void)
-{
-    if (kvm_enabled()) {
-        kvm_s390_crypto_reset();
-    }
-}
-
-void s390_enable_css_support(S390CPU *cpu)
-{
-    if (kvm_enabled()) {
-        kvm_s390_enable_css_support(cpu);
-    }
-}
-
-void s390_do_cpu_set_diag318(CPUState *cs, run_on_cpu_data arg)
-{
-    if (kvm_enabled()) {
-        kvm_s390_set_diag318(cs, arg.host_ulong);
-    }
-}
-#endif
-
 static gchar *s390_gdb_arch_name(CPUState *cs)
 {
     return g_strdup("s390:64-bit");
@@ -505,9 +246,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
     dc->user_creatable = true;
 
     device_class_set_parent_reset(dc, s390_cpu_reset_full, &scc->parent_reset);
-#if !defined(CONFIG_USER_ONLY)
-    scc->load_normal = s390_cpu_load_normal;
-#endif
+
     scc->reset = s390_cpu_reset;
     cc->class_by_name = s390_cpu_class_by_name,
     cc->has_work = s390_cpu_has_work;
@@ -515,17 +254,15 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
     cc->set_pc = s390_cpu_set_pc;
     cc->gdb_read_register = s390_cpu_gdb_read_register;
     cc->gdb_write_register = s390_cpu_gdb_write_register;
-#ifndef CONFIG_USER_ONLY
-    cc->get_phys_page_debug = s390_cpu_get_phys_page_debug;
-    cc->vmsd = &vmstate_s390_cpu;
-    cc->get_crash_info = s390_cpu_get_crash_info;
-    cc->write_elf64_note = s390_cpu_write_elf64_note;
-#endif
     cc->disas_set_info = s390_cpu_disas_set_info;
     cc->gdb_num_core_regs = S390_NUM_CORE_REGS;
     cc->gdb_core_xml_file = "s390x-core64.xml";
     cc->gdb_arch_name = s390_gdb_arch_name;
 
+#ifndef CONFIG_USER_ONLY
+    s390_cpu_class_init_sysemu(cc);
+#endif /* CONFIG_USER_ONLY */
+
     s390_cpu_model_class_register_props(oc);
 
 #ifdef CONFIG_TCG
@@ -539,7 +276,11 @@ static const TypeInfo s390_cpu_type_info = {
     .instance_size = sizeof(S390CPU),
     .instance_align = __alignof__(S390CPU),
     .instance_init = s390_cpu_initfn,
+
+#ifndef CONFIG_USER_ONLY
     .instance_finalize = s390_cpu_finalize,
+#endif /* !CONFIG_USER_ONLY */
+
     .abstract = true,
     .class_size = sizeof(S390CPUClass),
     .class_init = s390_cpu_class_init,
diff --git a/target/s390x/meson.build b/target/s390x/meson.build
index 60d7f1b908..a73bae3dc5 100644
--- a/target/s390x/meson.build
+++ b/target/s390x/meson.build
@@ -28,6 +28,7 @@ s390x_softmmu_ss.add(files(
   'machine.c',
   'mmu_helper.c',
   'sigp.c',
+  'cpu-sysemu.c',
 ))
 
 # Newer kernels on s390 check for an S390_PGSTE program header and
diff --git a/target/s390x/trace-events b/target/s390x/trace-events
index fda1ee8220..e6c5fc1d03 100644
--- a/target/s390x/trace-events
+++ b/target/s390x/trace-events
@@ -16,7 +16,7 @@ kvm_clear_cmma(int rc) "CMMA: clearing with result code %d"
 kvm_failed_cpu_state_set(int cpu_index, uint8_t state, const char *msg) "Warning: Unable to set cpu %d state %" PRIu8 " to KVM: %s"
 kvm_assign_subch_ioeventfd(int fd, uint32_t addr, bool assign, int datamatch) "fd: %d sch: @0x%x assign: %d vq: %d"
 
-# cpu.c
+# cpu-sysemu.c
 cpu_set_state(int cpu_index, uint8_t state) "setting cpu %d state to %" PRIu8
 cpu_halt(int cpu_index) "halting cpu %d"
 cpu_unhalt(int cpu_index) "unhalting cpu %d"
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC v1 4/5] target/s390x: split cpu-dump from helper.c
  2021-03-22 19:15 [RFC v1 0/5] s390x cleanup Claudio Fontana
                   ` (2 preceding siblings ...)
  2021-03-22 19:15 ` [RFC v1 3/5] target/s390x: move sysemu-only code out to cpu-sysemu.c Claudio Fontana
@ 2021-03-22 19:15 ` Claudio Fontana
  2021-03-22 19:15 ` [RFC v1 5/5] target/s390x: make helper.c sysemu-only Claudio Fontana
  2021-03-23 19:30 ` [RFC v1 0/5] s390x cleanup Claudio Fontana
  5 siblings, 0 replies; 13+ messages in thread
From: Claudio Fontana @ 2021-03-22 19:15 UTC (permalink / raw)
  To: Cornelia Huck, Thomas Huth, Richard Henderson
  Cc: David Hildenbrand, qemu-devel, Halil Pasic,
	Christian Borntraeger, qemu-s390x, Claudio Fontana,
	Paolo Bonzini

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 target/s390x/cpu-dump.c  | 131 +++++++++++++++++++++++++++++++++++++++
 target/s390x/helper.c    | 107 --------------------------------
 target/s390x/meson.build |   1 +
 3 files changed, 132 insertions(+), 107 deletions(-)
 create mode 100644 target/s390x/cpu-dump.c

diff --git a/target/s390x/cpu-dump.c b/target/s390x/cpu-dump.c
new file mode 100644
index 0000000000..4170dec01a
--- /dev/null
+++ b/target/s390x/cpu-dump.c
@@ -0,0 +1,131 @@
+/*
+ * S/390 CPU dump to FILE
+ *
+ *  Copyright (c) 2009 Ulrich Hecht
+ *  Copyright (c) 2011 Alexander Graf
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "s390x-internal.h"
+#include "qemu/qemu-print.h"
+
+void s390_cpu_dump_state(CPUState *cs, FILE *f, int flags)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    CPUS390XState *env = &cpu->env;
+    int i;
+
+    if (env->cc_op > 3) {
+        qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64 " cc %15s\n",
+                     env->psw.mask, env->psw.addr, cc_name(env->cc_op));
+    } else {
+        qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64 " cc %02x\n",
+                     env->psw.mask, env->psw.addr, env->cc_op);
+    }
+
+    for (i = 0; i < 16; i++) {
+        qemu_fprintf(f, "R%02d=%016" PRIx64, i, env->regs[i]);
+        if ((i % 4) == 3) {
+            qemu_fprintf(f, "\n");
+        } else {
+            qemu_fprintf(f, " ");
+        }
+    }
+
+    if (flags & CPU_DUMP_FPU) {
+        if (s390_has_feat(S390_FEAT_VECTOR)) {
+            for (i = 0; i < 32; i++) {
+                qemu_fprintf(f, "V%02d=%016" PRIx64 "%016" PRIx64 "%c",
+                             i, env->vregs[i][0], env->vregs[i][1],
+                             i % 2 ? '\n' : ' ');
+            }
+        } else {
+            for (i = 0; i < 16; i++) {
+                qemu_fprintf(f, "F%02d=%016" PRIx64 "%c",
+                             i, *get_freg(env, i),
+                             (i % 4) == 3 ? '\n' : ' ');
+            }
+        }
+    }
+
+#ifndef CONFIG_USER_ONLY
+    for (i = 0; i < 16; i++) {
+        qemu_fprintf(f, "C%02d=%016" PRIx64, i, env->cregs[i]);
+        if ((i % 4) == 3) {
+            qemu_fprintf(f, "\n");
+        } else {
+            qemu_fprintf(f, " ");
+        }
+    }
+#endif
+
+#ifdef DEBUG_INLINE_BRANCHES
+    for (i = 0; i < CC_OP_MAX; i++) {
+        qemu_fprintf(f, "  %15s = %10ld\t%10ld\n", cc_name(i),
+                     inline_branch_miss[i], inline_branch_hit[i]);
+    }
+#endif
+
+    qemu_fprintf(f, "\n");
+}
+
+const char *cc_name(enum cc_op cc_op)
+{
+    static const char * const cc_names[] = {
+        [CC_OP_CONST0]    = "CC_OP_CONST0",
+        [CC_OP_CONST1]    = "CC_OP_CONST1",
+        [CC_OP_CONST2]    = "CC_OP_CONST2",
+        [CC_OP_CONST3]    = "CC_OP_CONST3",
+        [CC_OP_DYNAMIC]   = "CC_OP_DYNAMIC",
+        [CC_OP_STATIC]    = "CC_OP_STATIC",
+        [CC_OP_NZ]        = "CC_OP_NZ",
+        [CC_OP_ADDU]      = "CC_OP_ADDU",
+        [CC_OP_SUBU]      = "CC_OP_SUBU",
+        [CC_OP_LTGT_32]   = "CC_OP_LTGT_32",
+        [CC_OP_LTGT_64]   = "CC_OP_LTGT_64",
+        [CC_OP_LTUGTU_32] = "CC_OP_LTUGTU_32",
+        [CC_OP_LTUGTU_64] = "CC_OP_LTUGTU_64",
+        [CC_OP_LTGT0_32]  = "CC_OP_LTGT0_32",
+        [CC_OP_LTGT0_64]  = "CC_OP_LTGT0_64",
+        [CC_OP_ADD_64]    = "CC_OP_ADD_64",
+        [CC_OP_SUB_64]    = "CC_OP_SUB_64",
+        [CC_OP_ABS_64]    = "CC_OP_ABS_64",
+        [CC_OP_NABS_64]   = "CC_OP_NABS_64",
+        [CC_OP_ADD_32]    = "CC_OP_ADD_32",
+        [CC_OP_SUB_32]    = "CC_OP_SUB_32",
+        [CC_OP_ABS_32]    = "CC_OP_ABS_32",
+        [CC_OP_NABS_32]   = "CC_OP_NABS_32",
+        [CC_OP_COMP_32]   = "CC_OP_COMP_32",
+        [CC_OP_COMP_64]   = "CC_OP_COMP_64",
+        [CC_OP_TM_32]     = "CC_OP_TM_32",
+        [CC_OP_TM_64]     = "CC_OP_TM_64",
+        [CC_OP_NZ_F32]    = "CC_OP_NZ_F32",
+        [CC_OP_NZ_F64]    = "CC_OP_NZ_F64",
+        [CC_OP_NZ_F128]   = "CC_OP_NZ_F128",
+        [CC_OP_ICM]       = "CC_OP_ICM",
+        [CC_OP_SLA_32]    = "CC_OP_SLA_32",
+        [CC_OP_SLA_64]    = "CC_OP_SLA_64",
+        [CC_OP_FLOGR]     = "CC_OP_FLOGR",
+        [CC_OP_LCBB]      = "CC_OP_LCBB",
+        [CC_OP_VC]        = "CC_OP_VC",
+        [CC_OP_MULS_32]   = "CC_OP_MULS_32",
+        [CC_OP_MULS_64]   = "CC_OP_MULS_64",
+    };
+
+    return cc_names[cc_op];
+}
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 2254873cef..41ccc83d11 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -23,7 +23,6 @@
 #include "s390x-internal.h"
 #include "exec/gdbstub.h"
 #include "qemu/timer.h"
-#include "qemu/qemu-print.h"
 #include "hw/s390x/ioinst.h"
 #include "hw/s390x/pv.h"
 #include "sysemu/hw_accel.h"
@@ -324,109 +323,3 @@ int s390_store_adtl_status(S390CPU *cpu, hwaddr addr, hwaddr len)
     return 0;
 }
 #endif /* CONFIG_USER_ONLY */
-
-void s390_cpu_dump_state(CPUState *cs, FILE *f, int flags)
-{
-    S390CPU *cpu = S390_CPU(cs);
-    CPUS390XState *env = &cpu->env;
-    int i;
-
-    if (env->cc_op > 3) {
-        qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64 " cc %15s\n",
-                     env->psw.mask, env->psw.addr, cc_name(env->cc_op));
-    } else {
-        qemu_fprintf(f, "PSW=mask %016" PRIx64 " addr %016" PRIx64 " cc %02x\n",
-                     env->psw.mask, env->psw.addr, env->cc_op);
-    }
-
-    for (i = 0; i < 16; i++) {
-        qemu_fprintf(f, "R%02d=%016" PRIx64, i, env->regs[i]);
-        if ((i % 4) == 3) {
-            qemu_fprintf(f, "\n");
-        } else {
-            qemu_fprintf(f, " ");
-        }
-    }
-
-    if (flags & CPU_DUMP_FPU) {
-        if (s390_has_feat(S390_FEAT_VECTOR)) {
-            for (i = 0; i < 32; i++) {
-                qemu_fprintf(f, "V%02d=%016" PRIx64 "%016" PRIx64 "%c",
-                             i, env->vregs[i][0], env->vregs[i][1],
-                             i % 2 ? '\n' : ' ');
-            }
-        } else {
-            for (i = 0; i < 16; i++) {
-                qemu_fprintf(f, "F%02d=%016" PRIx64 "%c",
-                             i, *get_freg(env, i),
-                             (i % 4) == 3 ? '\n' : ' ');
-            }
-        }
-    }
-
-#ifndef CONFIG_USER_ONLY
-    for (i = 0; i < 16; i++) {
-        qemu_fprintf(f, "C%02d=%016" PRIx64, i, env->cregs[i]);
-        if ((i % 4) == 3) {
-            qemu_fprintf(f, "\n");
-        } else {
-            qemu_fprintf(f, " ");
-        }
-    }
-#endif
-
-#ifdef DEBUG_INLINE_BRANCHES
-    for (i = 0; i < CC_OP_MAX; i++) {
-        qemu_fprintf(f, "  %15s = %10ld\t%10ld\n", cc_name(i),
-                     inline_branch_miss[i], inline_branch_hit[i]);
-    }
-#endif
-
-    qemu_fprintf(f, "\n");
-}
-
-const char *cc_name(enum cc_op cc_op)
-{
-    static const char * const cc_names[] = {
-        [CC_OP_CONST0]    = "CC_OP_CONST0",
-        [CC_OP_CONST1]    = "CC_OP_CONST1",
-        [CC_OP_CONST2]    = "CC_OP_CONST2",
-        [CC_OP_CONST3]    = "CC_OP_CONST3",
-        [CC_OP_DYNAMIC]   = "CC_OP_DYNAMIC",
-        [CC_OP_STATIC]    = "CC_OP_STATIC",
-        [CC_OP_NZ]        = "CC_OP_NZ",
-        [CC_OP_ADDU]      = "CC_OP_ADDU",
-        [CC_OP_SUBU]      = "CC_OP_SUBU",
-        [CC_OP_LTGT_32]   = "CC_OP_LTGT_32",
-        [CC_OP_LTGT_64]   = "CC_OP_LTGT_64",
-        [CC_OP_LTUGTU_32] = "CC_OP_LTUGTU_32",
-        [CC_OP_LTUGTU_64] = "CC_OP_LTUGTU_64",
-        [CC_OP_LTGT0_32]  = "CC_OP_LTGT0_32",
-        [CC_OP_LTGT0_64]  = "CC_OP_LTGT0_64",
-        [CC_OP_ADD_64]    = "CC_OP_ADD_64",
-        [CC_OP_SUB_64]    = "CC_OP_SUB_64",
-        [CC_OP_ABS_64]    = "CC_OP_ABS_64",
-        [CC_OP_NABS_64]   = "CC_OP_NABS_64",
-        [CC_OP_ADD_32]    = "CC_OP_ADD_32",
-        [CC_OP_SUB_32]    = "CC_OP_SUB_32",
-        [CC_OP_ABS_32]    = "CC_OP_ABS_32",
-        [CC_OP_NABS_32]   = "CC_OP_NABS_32",
-        [CC_OP_COMP_32]   = "CC_OP_COMP_32",
-        [CC_OP_COMP_64]   = "CC_OP_COMP_64",
-        [CC_OP_TM_32]     = "CC_OP_TM_32",
-        [CC_OP_TM_64]     = "CC_OP_TM_64",
-        [CC_OP_NZ_F32]    = "CC_OP_NZ_F32",
-        [CC_OP_NZ_F64]    = "CC_OP_NZ_F64",
-        [CC_OP_NZ_F128]   = "CC_OP_NZ_F128",
-        [CC_OP_ICM]       = "CC_OP_ICM",
-        [CC_OP_SLA_32]    = "CC_OP_SLA_32",
-        [CC_OP_SLA_64]    = "CC_OP_SLA_64",
-        [CC_OP_FLOGR]     = "CC_OP_FLOGR",
-        [CC_OP_LCBB]      = "CC_OP_LCBB",
-        [CC_OP_VC]        = "CC_OP_VC",
-        [CC_OP_MULS_32]   = "CC_OP_MULS_32",
-        [CC_OP_MULS_64]   = "CC_OP_MULS_64",
-    };
-
-    return cc_names[cc_op];
-}
diff --git a/target/s390x/meson.build b/target/s390x/meson.build
index a73bae3dc5..6e1aa3b0cd 100644
--- a/target/s390x/meson.build
+++ b/target/s390x/meson.build
@@ -6,6 +6,7 @@ s390x_ss.add(files(
   'gdbstub.c',
   'helper.c',
   'interrupt.c',
+  'cpu-dump.c',
 ))
 
 s390x_ss.add(when: 'CONFIG_KVM', if_true: files('kvm.c'), if_false: files('kvm-stub.c'))
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC v1 5/5] target/s390x: make helper.c sysemu-only
  2021-03-22 19:15 [RFC v1 0/5] s390x cleanup Claudio Fontana
                   ` (3 preceding siblings ...)
  2021-03-22 19:15 ` [RFC v1 4/5] target/s390x: split cpu-dump from helper.c Claudio Fontana
@ 2021-03-22 19:15 ` Claudio Fontana
  2021-03-23 19:30 ` [RFC v1 0/5] s390x cleanup Claudio Fontana
  5 siblings, 0 replies; 13+ messages in thread
From: Claudio Fontana @ 2021-03-22 19:15 UTC (permalink / raw)
  To: Cornelia Huck, Thomas Huth, Richard Henderson
  Cc: David Hildenbrand, qemu-devel, Halil Pasic,
	Christian Borntraeger, qemu-s390x, Claudio Fontana,
	Paolo Bonzini

Now that we have moved cpu-dump functionality out of helper.c,
we can make the module sysemu-only.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 target/s390x/helper.c    | 4 ----
 target/s390x/meson.build | 2 +-
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 41ccc83d11..f246bec353 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -27,11 +27,8 @@
 #include "hw/s390x/pv.h"
 #include "sysemu/hw_accel.h"
 #include "sysemu/runstate.h"
-#ifndef CONFIG_USER_ONLY
 #include "sysemu/tcg.h"
-#endif
 
-#ifndef CONFIG_USER_ONLY
 void s390x_tod_timer(void *opaque)
 {
     cpu_inject_clock_comparator((S390CPU *) opaque);
@@ -322,4 +319,3 @@ int s390_store_adtl_status(S390CPU *cpu, hwaddr addr, hwaddr len)
     cpu_physical_memory_unmap(sa, len, 1, len);
     return 0;
 }
-#endif /* CONFIG_USER_ONLY */
diff --git a/target/s390x/meson.build b/target/s390x/meson.build
index 6e1aa3b0cd..bbcaede384 100644
--- a/target/s390x/meson.build
+++ b/target/s390x/meson.build
@@ -4,7 +4,6 @@ s390x_ss.add(files(
   'cpu_features.c',
   'cpu_models.c',
   'gdbstub.c',
-  'helper.c',
   'interrupt.c',
   'cpu-dump.c',
 ))
@@ -23,6 +22,7 @@ s390x_ss.add(gen_features_h)
 
 s390x_softmmu_ss = ss.source_set()
 s390x_softmmu_ss.add(files(
+  'helper.c',
   'arch_dump.c',
   'diag.c',
   'ioinst.c',
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC v1 0/5] s390x cleanup
  2021-03-22 19:15 [RFC v1 0/5] s390x cleanup Claudio Fontana
                   ` (4 preceding siblings ...)
  2021-03-22 19:15 ` [RFC v1 5/5] target/s390x: make helper.c sysemu-only Claudio Fontana
@ 2021-03-23 19:30 ` Claudio Fontana
  5 siblings, 0 replies; 13+ messages in thread
From: Claudio Fontana @ 2021-03-23 19:30 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 3/22/21 8:15 PM, Claudio Fontana wrote:
> Hi, I am starting a cleanup series for s390x,
> 
> with the goal of doing similar splits of KVM vs TCG,


One annoying thing is that it appears that the cpu accel model does not offer much in terms of refactoring
and code simplification opportunities for s390x as-is, in particular for KVM.

It is possible that with quite some rework of the cpu_models code we could gain something,
so I will give it a try still, but for now adding the accel-cpu object does not suddenly provide nice low hanging fruits in terms of simplifications.

There are instead some good opportunities in terms of SYSEMU vs USER mode splits/beautification I think.

Ciao,

Claudio

> sysemu vs user mode, as for the existing work on x86 and ARM.
> 
> S/390 target looks very good already, and seems much easier to work
> with. I hope that with some patches it will be even better.
> 
> I will pile up more patches later on, but I start sharing something
> here with these few RFC patches for your eyes,
> 
> they are based on the pre-requisite series:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg07461.html
> 
> Motivation and higher level steps:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg04628.html
> 
> Comments welcome, thanks,
> 
> Claudio
> 
> Claudio Fontana (5):
>   hw/s390x: only build qemu-tod from the CONFIG_TCG build
>   target/s390x: start moving TCG-only code to tcg/
>   target/s390x: move sysemu-only code out to cpu-sysemu.c
>   target/s390x: split cpu-dump from helper.c
>   target/s390x: make helper.c sysemu-only
> 
>  include/hw/s390x/tod.h                        |   2 +-
>  target/s390x/{internal.h => s390x-internal.h} |   6 +
>  target/s390x/{ => tcg}/s390-tod.h             |   0
>  target/s390x/{ => tcg}/tcg_s390x.h            |   0
>  target/s390x/{ => tcg}/vec.h                  |   0
>  hw/s390x/tod-qemu.c                           |   2 +-
>  hw/s390x/tod.c                                |   9 +-
>  target/s390x/arch_dump.c                      |   2 +-
>  target/s390x/cpu-dump.c                       | 131 ++++++++
>  target/s390x/cpu-sysemu.c                     | 304 ++++++++++++++++++
>  target/s390x/cpu.c                            | 285 +---------------
>  target/s390x/cpu_models.c                     |   2 +-
>  target/s390x/diag.c                           |   2 +-
>  target/s390x/gdbstub.c                        |   2 +-
>  target/s390x/helper.c                         | 113 +------
>  target/s390x/interrupt.c                      |   4 +-
>  target/s390x/ioinst.c                         |   2 +-
>  target/s390x/kvm.c                            |   2 +-
>  target/s390x/machine.c                        |   4 +-
>  target/s390x/mmu_helper.c                     |   2 +-
>  target/s390x/sigp.c                           |   2 +-
>  target/s390x/tcg-stub.c                       |  30 --
>  target/s390x/{ => tcg}/cc_helper.c            |   2 +-
>  target/s390x/{ => tcg}/crypto_helper.c        |   2 +-
>  target/s390x/{ => tcg}/excp_helper.c          |   2 +-
>  target/s390x/{ => tcg}/fpu_helper.c           |   2 +-
>  target/s390x/{ => tcg}/int_helper.c           |   2 +-
>  target/s390x/{ => tcg}/mem_helper.c           |   2 +-
>  target/s390x/{ => tcg}/misc_helper.c          |   2 +-
>  target/s390x/{ => tcg}/translate.c            |   2 +-
>  target/s390x/{ => tcg}/vec_fpu_helper.c       |   2 +-
>  target/s390x/{ => tcg}/vec_helper.c           |   2 +-
>  target/s390x/{ => tcg}/vec_int_helper.c       |   0
>  target/s390x/{ => tcg}/vec_string_helper.c    |   2 +-
>  target/s390x/{ => tcg}/translate_vx.c.inc     |   0
>  hw/s390x/meson.build                          |   5 +-
>  target/s390x/meson.build                      |  21 +-
>  target/s390x/{ => tcg}/insn-data.def          |   0
>  target/s390x/{ => tcg}/insn-format.def        |   0
>  target/s390x/tcg/meson.build                  |  14 +
>  target/s390x/trace-events                     |   2 +-
>  41 files changed, 512 insertions(+), 458 deletions(-)
>  rename target/s390x/{internal.h => s390x-internal.h} (98%)
>  rename target/s390x/{ => tcg}/s390-tod.h (100%)
>  rename target/s390x/{ => tcg}/tcg_s390x.h (100%)
>  rename target/s390x/{ => tcg}/vec.h (100%)
>  create mode 100644 target/s390x/cpu-dump.c
>  create mode 100644 target/s390x/cpu-sysemu.c
>  delete mode 100644 target/s390x/tcg-stub.c
>  rename target/s390x/{ => tcg}/cc_helper.c (99%)
>  rename target/s390x/{ => tcg}/crypto_helper.c (98%)
>  rename target/s390x/{ => tcg}/excp_helper.c (99%)
>  rename target/s390x/{ => tcg}/fpu_helper.c (99%)
>  rename target/s390x/{ => tcg}/int_helper.c (99%)
>  rename target/s390x/{ => tcg}/mem_helper.c (99%)
>  rename target/s390x/{ => tcg}/misc_helper.c (99%)
>  rename target/s390x/{ => tcg}/translate.c (99%)
>  rename target/s390x/{ => tcg}/vec_fpu_helper.c (99%)
>  rename target/s390x/{ => tcg}/vec_helper.c (99%)
>  rename target/s390x/{ => tcg}/vec_int_helper.c (100%)
>  rename target/s390x/{ => tcg}/vec_string_helper.c (99%)
>  rename target/s390x/{ => tcg}/translate_vx.c.inc (100%)
>  rename target/s390x/{ => tcg}/insn-data.def (100%)
>  rename target/s390x/{ => tcg}/insn-format.def (100%)
>  create mode 100644 target/s390x/tcg/meson.build
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
  2021-03-22 19:15 ` [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build Claudio Fontana
@ 2021-03-31 11:07   ` Cornelia Huck
  2021-04-19  9:11     ` Claudio Fontana
  0 siblings, 1 reply; 13+ messages in thread
From: Cornelia Huck @ 2021-03-31 11:07 UTC (permalink / raw)
  To: Claudio Fontana
  Cc: Thomas Huth, David Hildenbrand, Richard Henderson, qemu-devel,
	Halil Pasic, Christian Borntraeger, qemu-s390x, Paolo Bonzini

On Mon, 22 Mar 2021 20:15:47 +0100
Claudio Fontana <cfontana@suse.de> wrote:

> this allows to remove unneeded stubs for target/s390x.

This patch doesn't seem to remove any, though?

> 
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>  hw/s390x/tod.c       | 9 ++++++++-
>  hw/s390x/meson.build | 5 ++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
> index 3c2979175e..322732d7fd 100644
> --- a/hw/s390x/tod.c
> +++ b/hw/s390x/tod.c
> @@ -14,6 +14,8 @@
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
>  #include "sysemu/kvm.h"
> +#include "sysemu/tcg.h"
> +#include "sysemu/qtest.h"
>  #include "migration/qemu-file-types.h"
>  #include "migration/register.h"
>  
> @@ -23,8 +25,13 @@ void s390_init_tod(void)
>  
>      if (kvm_enabled()) {
>          obj = object_new(TYPE_KVM_S390_TOD);
> -    } else {
> +    } else if (tcg_enabled()) {
>          obj = object_new(TYPE_QEMU_S390_TOD);
> +    } else if (qtest_enabled()) {
> +        return;
> +    } else {
> +        warn_report("current accelerator not handled in s390_init_tod!");
> +        return;

I'm wondering whether this should be a fatal error.

>      }
>      object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
>      object_unref(obj);
> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
> index 91495b5631..7f31f9e5d5 100644
> --- a/hw/s390x/meson.build
> +++ b/hw/s390x/meson.build
> @@ -16,7 +16,6 @@ s390x_ss.add(files(
>    'sclp.c',
>    'sclpcpu.c',
>    'sclpquiesce.c',
> -  'tod-qemu.c',
>    'tod.c',
>  ))
>  s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>    's390-stattrib-kvm.c',
>    'pv.c',
>  ))
> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
> +  'tod-qemu.c',

Should we rename this to tod-tcg.c?

> +))
> +
>  s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c'))
>  s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c'))
>  s390x_ss.add(when: 'CONFIG_VFIO', if_true: files('s390-pci-vfio.c'))



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
  2021-03-31 11:07   ` Cornelia Huck
@ 2021-04-19  9:11     ` Claudio Fontana
  2021-04-19 16:12       ` Claudio Fontana
  0 siblings, 1 reply; 13+ messages in thread
From: Claudio Fontana @ 2021-04-19  9:11 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Thomas Huth, David Hildenbrand, Richard Henderson, qemu-devel,
	Halil Pasic, Christian Borntraeger, qemu-s390x, Paolo Bonzini

Hi Cornelia,

On 3/31/21 1:07 PM, Cornelia Huck wrote:
> On Mon, 22 Mar 2021 20:15:47 +0100
> Claudio Fontana <cfontana@suse.de> wrote:
> 
>> this allows to remove unneeded stubs for target/s390x.
> 
> This patch doesn't seem to remove any, though?

The next patch does... I'll split more the patches so it becomes clearer.

> 
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>>  hw/s390x/tod.c       | 9 ++++++++-
>>  hw/s390x/meson.build | 5 ++++-
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
>> index 3c2979175e..322732d7fd 100644
>> --- a/hw/s390x/tod.c
>> +++ b/hw/s390x/tod.c
>> @@ -14,6 +14,8 @@
>>  #include "qemu/error-report.h"
>>  #include "qemu/module.h"
>>  #include "sysemu/kvm.h"
>> +#include "sysemu/tcg.h"
>> +#include "sysemu/qtest.h"
>>  #include "migration/qemu-file-types.h"
>>  #include "migration/register.h"
>>  
>> @@ -23,8 +25,13 @@ void s390_init_tod(void)
>>  
>>      if (kvm_enabled()) {
>>          obj = object_new(TYPE_KVM_S390_TOD);
>> -    } else {
>> +    } else if (tcg_enabled()) {
>>          obj = object_new(TYPE_QEMU_S390_TOD);
>> +    } else if (qtest_enabled()) {
>> +        return;
>> +    } else {
>> +        warn_report("current accelerator not handled in s390_init_tod!");
>> +        return;
> 
> I'm wondering whether this should be a fatal error.

I would agree with that.

> 
>>      }
>>      object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
>>      object_unref(obj);
>> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
>> index 91495b5631..7f31f9e5d5 100644
>> --- a/hw/s390x/meson.build
>> +++ b/hw/s390x/meson.build
>> @@ -16,7 +16,6 @@ s390x_ss.add(files(
>>    'sclp.c',
>>    'sclpcpu.c',
>>    'sclpquiesce.c',
>> -  'tod-qemu.c',
>>    'tod.c',
>>  ))
>>  s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>>    's390-stattrib-kvm.c',
>>    'pv.c',
>>  ))
>> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
>> +  'tod-qemu.c',
> 
> Should we rename this to tod-tcg.c?

I think so.

> 
>> +))
>> +
>>  s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c'))
>>  s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c'))
>>  s390x_ss.add(when: 'CONFIG_VFIO', if_true: files('s390-pci-vfio.c'))
> 
> 

Will prepare a new version,

Thanks,

Claudio


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
  2021-04-19  9:11     ` Claudio Fontana
@ 2021-04-19 16:12       ` Claudio Fontana
  2021-04-19 16:20         ` Cornelia Huck
  0 siblings, 1 reply; 13+ messages in thread
From: Claudio Fontana @ 2021-04-19 16:12 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Thomas Huth, David Hildenbrand, Richard Henderson, qemu-devel,
	Halil Pasic, Christian Borntraeger, qemu-s390x, Paolo Bonzini

On 4/19/21 11:11 AM, Claudio Fontana wrote:
> Hi Cornelia,
> 
> On 3/31/21 1:07 PM, Cornelia Huck wrote:
>> On Mon, 22 Mar 2021 20:15:47 +0100
>> Claudio Fontana <cfontana@suse.de> wrote:
>>
>>> this allows to remove unneeded stubs for target/s390x.
>>
>> This patch doesn't seem to remove any, though?
> 
> The next patch does... I'll split more the patches so it becomes clearer.
> 
>>
>>>
>>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>>> ---
>>>  hw/s390x/tod.c       | 9 ++++++++-
>>>  hw/s390x/meson.build | 5 ++++-
>>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
>>> index 3c2979175e..322732d7fd 100644
>>> --- a/hw/s390x/tod.c
>>> +++ b/hw/s390x/tod.c
>>> @@ -14,6 +14,8 @@
>>>  #include "qemu/error-report.h"
>>>  #include "qemu/module.h"
>>>  #include "sysemu/kvm.h"
>>> +#include "sysemu/tcg.h"
>>> +#include "sysemu/qtest.h"
>>>  #include "migration/qemu-file-types.h"
>>>  #include "migration/register.h"
>>>  
>>> @@ -23,8 +25,13 @@ void s390_init_tod(void)
>>>  
>>>      if (kvm_enabled()) {
>>>          obj = object_new(TYPE_KVM_S390_TOD);
>>> -    } else {
>>> +    } else if (tcg_enabled()) {
>>>          obj = object_new(TYPE_QEMU_S390_TOD);
>>> +    } else if (qtest_enabled()) {
>>> +        return;
>>> +    } else {
>>> +        warn_report("current accelerator not handled in s390_init_tod!");
>>> +        return;
>>
>> I'm wondering whether this should be a fatal error.
> 
> I would agree with that.
> 
>>
>>>      }
>>>      object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
>>>      object_unref(obj);
>>> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
>>> index 91495b5631..7f31f9e5d5 100644
>>> --- a/hw/s390x/meson.build
>>> +++ b/hw/s390x/meson.build
>>> @@ -16,7 +16,6 @@ s390x_ss.add(files(
>>>    'sclp.c',
>>>    'sclpcpu.c',
>>>    'sclpquiesce.c',
>>> -  'tod-qemu.c',
>>>    'tod.c',
>>>  ))
>>>  s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>>> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>>>    's390-stattrib-kvm.c',
>>>    'pv.c',
>>>  ))
>>> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
>>> +  'tod-qemu.c',
>>
>> Should we rename this to tod-tcg.c?
> 
> I think so.

Here we are a bit limited though by the fact that the object is currently called:

include/hw/s390x/tod.h:26:#define TYPE_QEMU_S390_TOD TYPE_S390_TOD "-qemu"

So there might be a compatibility issue in trying to make this consistent, which would mean to replace this with:

#define TYPE_TCG_S390_TOD TYPE_S390_TOD "-tcg"

What do you think?


> 
>>
>>> +))
>>> +
>>>  s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c'))
>>>  s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c'))
>>>  s390x_ss.add(when: 'CONFIG_VFIO', if_true: files('s390-pci-vfio.c'))
>>
>>
> 
> Will prepare a new version,
> 
> Thanks,
> 
> Claudio
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
  2021-04-19 16:12       ` Claudio Fontana
@ 2021-04-19 16:20         ` Cornelia Huck
  2021-04-19 16:24           ` Claudio Fontana
  0 siblings, 1 reply; 13+ messages in thread
From: Cornelia Huck @ 2021-04-19 16:20 UTC (permalink / raw)
  To: Claudio Fontana
  Cc: Thomas Huth, David Hildenbrand, Richard Henderson, qemu-devel,
	Halil Pasic, Christian Borntraeger, qemu-s390x, Paolo Bonzini

On Mon, 19 Apr 2021 18:12:48 +0200
Claudio Fontana <cfontana@suse.de> wrote:

> On 4/19/21 11:11 AM, Claudio Fontana wrote:
> > Hi Cornelia,
> > 
> > On 3/31/21 1:07 PM, Cornelia Huck wrote:  
> >> On Mon, 22 Mar 2021 20:15:47 +0100
> >> Claudio Fontana <cfontana@suse.de> wrote:

> >>> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
> >>>    's390-stattrib-kvm.c',
> >>>    'pv.c',
> >>>  ))
> >>> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
> >>> +  'tod-qemu.c',  
> >>
> >> Should we rename this to tod-tcg.c?  
> > 
> > I think so.  
> 
> Here we are a bit limited though by the fact that the object is currently called:
> 
> include/hw/s390x/tod.h:26:#define TYPE_QEMU_S390_TOD TYPE_S390_TOD "-qemu"
> 
> So there might be a compatibility issue in trying to make this consistent, which would mean to replace this with:
> 
> #define TYPE_TCG_S390_TOD TYPE_S390_TOD "-tcg"
> 
> What do you think?

How visible is this? I don't think the TOD objects are instantiable by
the user.



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
  2021-04-19 16:20         ` Cornelia Huck
@ 2021-04-19 16:24           ` Claudio Fontana
  2021-04-19 16:30             ` Cornelia Huck
  0 siblings, 1 reply; 13+ messages in thread
From: Claudio Fontana @ 2021-04-19 16:24 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Thomas Huth, David Hildenbrand, Richard Henderson, qemu-devel,
	Halil Pasic, Christian Borntraeger, qemu-s390x, Paolo Bonzini

On 4/19/21 6:20 PM, Cornelia Huck wrote:
> On Mon, 19 Apr 2021 18:12:48 +0200
> Claudio Fontana <cfontana@suse.de> wrote:
> 
>> On 4/19/21 11:11 AM, Claudio Fontana wrote:
>>> Hi Cornelia,
>>>
>>> On 3/31/21 1:07 PM, Cornelia Huck wrote:  
>>>> On Mon, 22 Mar 2021 20:15:47 +0100
>>>> Claudio Fontana <cfontana@suse.de> wrote:
> 
>>>>> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>>>>>    's390-stattrib-kvm.c',
>>>>>    'pv.c',
>>>>>  ))
>>>>> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
>>>>> +  'tod-qemu.c',  
>>>>
>>>> Should we rename this to tod-tcg.c?  
>>>
>>> I think so.  
>>
>> Here we are a bit limited though by the fact that the object is currently called:
>>
>> include/hw/s390x/tod.h:26:#define TYPE_QEMU_S390_TOD TYPE_S390_TOD "-qemu"
>>
>> So there might be a compatibility issue in trying to make this consistent, which would mean to replace this with:
>>
>> #define TYPE_TCG_S390_TOD TYPE_S390_TOD "-tcg"
>>
>> What do you think?
> 
> How visible is this? I don't think the TOD objects are instantiable by
> the user.
> 

I just remember we were very conservative with the object hierarchy on x86, personally I am fine with the change.
I will add this change then, I'd ask for people with concerns about this to speak up:

(Paolo?) 

Ciao,

CLaudio



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
  2021-04-19 16:24           ` Claudio Fontana
@ 2021-04-19 16:30             ` Cornelia Huck
  0 siblings, 0 replies; 13+ messages in thread
From: Cornelia Huck @ 2021-04-19 16:30 UTC (permalink / raw)
  To: Claudio Fontana
  Cc: Thomas Huth, David Hildenbrand, Richard Henderson, qemu-devel,
	Halil Pasic, Christian Borntraeger, qemu-s390x, Paolo Bonzini

On Mon, 19 Apr 2021 18:24:34 +0200
Claudio Fontana <cfontana@suse.de> wrote:

> On 4/19/21 6:20 PM, Cornelia Huck wrote:
> > On Mon, 19 Apr 2021 18:12:48 +0200
> > Claudio Fontana <cfontana@suse.de> wrote:
> >   
> >> On 4/19/21 11:11 AM, Claudio Fontana wrote:  
> >>> Hi Cornelia,
> >>>
> >>> On 3/31/21 1:07 PM, Cornelia Huck wrote:    
> >>>> On Mon, 22 Mar 2021 20:15:47 +0100
> >>>> Claudio Fontana <cfontana@suse.de> wrote:  
> >   
> >>>>> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
> >>>>>    's390-stattrib-kvm.c',
> >>>>>    'pv.c',
> >>>>>  ))
> >>>>> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
> >>>>> +  'tod-qemu.c',    
> >>>>
> >>>> Should we rename this to tod-tcg.c?    
> >>>
> >>> I think so.    
> >>
> >> Here we are a bit limited though by the fact that the object is currently called:
> >>
> >> include/hw/s390x/tod.h:26:#define TYPE_QEMU_S390_TOD TYPE_S390_TOD "-qemu"
> >>
> >> So there might be a compatibility issue in trying to make this consistent, which would mean to replace this with:
> >>
> >> #define TYPE_TCG_S390_TOD TYPE_S390_TOD "-tcg"
> >>
> >> What do you think?  
> > 
> > How visible is this? I don't think the TOD objects are instantiable by
> > the user.
> >   
> 
> I just remember we were very conservative with the object hierarchy on x86, personally I am fine with the change.
> I will add this change then, I'd ask for people with concerns about this to speak up:
> 
> (Paolo?) 
> 
> Ciao,
> 
> CLaudio
> 

It was more an argument against changing it, because most people won't
see it anyway :)



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-04-19 16:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 19:15 [RFC v1 0/5] s390x cleanup Claudio Fontana
2021-03-22 19:15 ` [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build Claudio Fontana
2021-03-31 11:07   ` Cornelia Huck
2021-04-19  9:11     ` Claudio Fontana
2021-04-19 16:12       ` Claudio Fontana
2021-04-19 16:20         ` Cornelia Huck
2021-04-19 16:24           ` Claudio Fontana
2021-04-19 16:30             ` Cornelia Huck
2021-03-22 19:15 ` [RFC v1 2/5] target/s390x: start moving TCG-only code to tcg/ Claudio Fontana
2021-03-22 19:15 ` [RFC v1 3/5] target/s390x: move sysemu-only code out to cpu-sysemu.c Claudio Fontana
2021-03-22 19:15 ` [RFC v1 4/5] target/s390x: split cpu-dump from helper.c Claudio Fontana
2021-03-22 19:15 ` [RFC v1 5/5] target/s390x: make helper.c sysemu-only Claudio Fontana
2021-03-23 19:30 ` [RFC v1 0/5] s390x cleanup Claudio Fontana

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.