[for-5.0 PATCH v2 0/3] cpu: Clarify overloading of reset QOM methods

[for-5.0 PATCH v2 0/3] cpu: Clarify overloading of reset QOM methods

[Greg Kurz]

Each cpu subclass overloads the reset method of its parent class with
its own. But since it needs to call the parent method as well, it keeps
a parent_reset pointer to do so. This causes the same not very explicit
boiler plate to be duplicated all around the place:

    pcc->parent_reset = cc->reset;
    cc->reset = ppc_cpu_reset;

A similar concern was addressed some time back by Philippe Mathieu-Daudé
in qdev, with the addition of device_class_set_parent_reset() and friends:

https://git.qemu.org/?p=qemu.git;a=commit;h=46795cf2e2f6
https://git.qemu.org/?p=qemu.git;a=commit;h=bf853881690d

Follow the same approach with cpus.

Changes in v2:
- added Reviewed-by and Acked-by tags
- rebased on top of https://github.com/cohuck/qemu.git s390-next
  SHA1 dd6252f035a2

--
Greg

---

Greg Kurz (3):
      cpu: Introduce CPUReset callback typedef
      cpu: Introduce cpu_class_set_parent_reset()
      cpu: Use cpu_class_set_parent_reset()


 hw/core/cpu.c                   |    8 ++++++++
 include/hw/core/cpu.h           |    8 +++++++-
 target/alpha/cpu-qom.h          |    2 +-
 target/arm/cpu-qom.h            |    2 +-
 target/arm/cpu.c                |    3 +--
 target/cris/cpu-qom.h           |    2 +-
 target/cris/cpu.c               |    3 +--
 target/hppa/cpu-qom.h           |    2 +-
 target/i386/cpu-qom.h           |    2 +-
 target/i386/cpu.c               |    3 +--
 target/lm32/cpu-qom.h           |    2 +-
 target/lm32/cpu.c               |    3 +--
 target/m68k/cpu-qom.h           |    2 +-
 target/m68k/cpu.c               |    3 +--
 target/microblaze/cpu-qom.h     |    2 +-
 target/microblaze/cpu.c         |    3 +--
 target/mips/cpu-qom.h           |    2 +-
 target/mips/cpu.c               |    3 +--
 target/moxie/cpu.c              |    3 +--
 target/moxie/cpu.h              |    2 +-
 target/nios2/cpu.c              |    3 +--
 target/nios2/cpu.h              |    2 +-
 target/openrisc/cpu.c           |    3 +--
 target/openrisc/cpu.h           |    2 +-
 target/ppc/cpu-qom.h            |    2 +-
 target/ppc/translate_init.inc.c |    3 +--
 target/riscv/cpu.c              |    3 +--
 target/riscv/cpu.h              |    2 +-
 target/s390x/cpu-qom.h          |    2 +-
 target/s390x/cpu.c              |    3 +--
 target/sh4/cpu-qom.h            |    2 +-
 target/sh4/cpu.c                |    3 +--
 target/sparc/cpu-qom.h          |    2 +-
 target/sparc/cpu.c              |    3 +--
 target/tilegx/cpu.c             |    3 +--
 target/tilegx/cpu.h             |    2 +-
 target/tricore/cpu-qom.h        |    2 +-
 target/tricore/cpu.c            |    3 +--
 target/xtensa/cpu-qom.h         |    2 +-
 target/xtensa/cpu.c             |    3 +--
 40 files changed, 53 insertions(+), 57 deletions(-)

[for-5.0 PATCH v2 1/3] cpu: Introduce CPUReset callback typedef

[Greg Kurz]

Use it in include/hw/core/cpu.h and convert all targets to use it as
well with:

perl -pi \
 -e 's/void\s+\(\*(parent_reset)\)\(CPUState\s+\*\w+\)/CPUReset \1/;' \
 $(git ls-files 'target/*.h')

Signed-off-by: Greg Kurz <groug@kaod.org>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/hw/core/cpu.h       |    4 +++-
 target/alpha/cpu-qom.h      |    2 +-
 target/arm/cpu-qom.h        |    2 +-
 target/cris/cpu-qom.h       |    2 +-
 target/hppa/cpu-qom.h       |    2 +-
 target/i386/cpu-qom.h       |    2 +-
 target/lm32/cpu-qom.h       |    2 +-
 target/m68k/cpu-qom.h       |    2 +-
 target/microblaze/cpu-qom.h |    2 +-
 target/mips/cpu-qom.h       |    2 +-
 target/moxie/cpu.h          |    2 +-
 target/nios2/cpu.h          |    2 +-
 target/openrisc/cpu.h       |    2 +-
 target/ppc/cpu-qom.h        |    2 +-
 target/riscv/cpu.h          |    2 +-
 target/s390x/cpu-qom.h      |    2 +-
 target/sh4/cpu-qom.h        |    2 +-
 target/sparc/cpu-qom.h      |    2 +-
 target/tilegx/cpu.h         |    2 +-
 target/tricore/cpu-qom.h    |    2 +-
 target/xtensa/cpu-qom.h     |    2 +-
 21 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 77c6f0529903..047e3972ecaf 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -74,6 +74,8 @@ typedef struct CPUWatchpoint CPUWatchpoint;
 
 struct TranslationBlock;
 
+typedef void (*CPUReset)(CPUState *cpu);
+
 /**
  * CPUClass:
  * @class_by_name: Callback to map -cpu command line model name to an
@@ -165,7 +167,7 @@ typedef struct CPUClass {
     ObjectClass *(*class_by_name)(const char *cpu_model);
     void (*parse_features)(const char *typename, char *str, Error **errp);
 
-    void (*reset)(CPUState *cpu);
+    CPUReset reset;
     int reset_dump_flags;
     bool (*has_work)(CPUState *cpu);
     void (*do_interrupt)(CPUState *cpu);
diff --git a/target/alpha/cpu-qom.h b/target/alpha/cpu-qom.h
index 6f0a0adb9efa..0c974805481b 100644
--- a/target/alpha/cpu-qom.h
+++ b/target/alpha/cpu-qom.h
@@ -44,7 +44,7 @@ typedef struct AlphaCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } AlphaCPUClass;
 
 typedef struct AlphaCPU AlphaCPU;
diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 7f5b244bde35..aeaa84afcc9a 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -51,7 +51,7 @@ typedef struct ARMCPUClass {
 
     const ARMCPUInfo *info;
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } ARMCPUClass;
 
 typedef struct ARMCPU ARMCPU;
diff --git a/target/cris/cpu-qom.h b/target/cris/cpu-qom.h
index 308c1f95bdf6..079ffe6bda0a 100644
--- a/target/cris/cpu-qom.h
+++ b/target/cris/cpu-qom.h
@@ -45,7 +45,7 @@ typedef struct CRISCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 
     uint32_t vr;
 } CRISCPUClass;
diff --git a/target/hppa/cpu-qom.h b/target/hppa/cpu-qom.h
index 6367dc479391..5c129de148a8 100644
--- a/target/hppa/cpu-qom.h
+++ b/target/hppa/cpu-qom.h
@@ -44,7 +44,7 @@ typedef struct HPPACPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } HPPACPUClass;
 
 typedef struct HPPACPU HPPACPU;
diff --git a/target/i386/cpu-qom.h b/target/i386/cpu-qom.h
index 0efab2fc670f..1e962518e68e 100644
--- a/target/i386/cpu-qom.h
+++ b/target/i386/cpu-qom.h
@@ -71,7 +71,7 @@ typedef struct X86CPUClass {
 
     DeviceRealize parent_realize;
     DeviceUnrealize parent_unrealize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } X86CPUClass;
 
 typedef struct X86CPU X86CPU;
diff --git a/target/lm32/cpu-qom.h b/target/lm32/cpu-qom.h
index dc9ac9ac9f7b..e105a315aa3e 100644
--- a/target/lm32/cpu-qom.h
+++ b/target/lm32/cpu-qom.h
@@ -44,7 +44,7 @@ typedef struct LM32CPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } LM32CPUClass;
 
 typedef struct LM32CPU LM32CPU;
diff --git a/target/m68k/cpu-qom.h b/target/m68k/cpu-qom.h
index b56da8a21374..0a196775e5d1 100644
--- a/target/m68k/cpu-qom.h
+++ b/target/m68k/cpu-qom.h
@@ -44,7 +44,7 @@ typedef struct M68kCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } M68kCPUClass;
 
 typedef struct M68kCPU M68kCPU;
diff --git a/target/microblaze/cpu-qom.h b/target/microblaze/cpu-qom.h
index 49b07cc697b9..7a4ff4a11e33 100644
--- a/target/microblaze/cpu-qom.h
+++ b/target/microblaze/cpu-qom.h
@@ -44,7 +44,7 @@ typedef struct MicroBlazeCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } MicroBlazeCPUClass;
 
 typedef struct MicroBlazeCPU MicroBlazeCPU;
diff --git a/target/mips/cpu-qom.h b/target/mips/cpu-qom.h
index a430c0fe4bbf..818401a501cb 100644
--- a/target/mips/cpu-qom.h
+++ b/target/mips/cpu-qom.h
@@ -48,7 +48,7 @@ typedef struct MIPSCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
     const struct mips_def_t *cpu_def;
 } MIPSCPUClass;
 
diff --git a/target/moxie/cpu.h b/target/moxie/cpu.h
index 01dca548e5d5..20dafc80f6ac 100644
--- a/target/moxie/cpu.h
+++ b/target/moxie/cpu.h
@@ -69,7 +69,7 @@ typedef struct MoxieCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } MoxieCPUClass;
 
 /**
diff --git a/target/nios2/cpu.h b/target/nios2/cpu.h
index 361b06ffeb61..59a07a5d0ee0 100644
--- a/target/nios2/cpu.h
+++ b/target/nios2/cpu.h
@@ -50,7 +50,7 @@ typedef struct Nios2CPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } Nios2CPUClass;
 
 #define TARGET_HAS_ICE 1
diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
index 0ad02eab7946..d77976ccce7f 100644
--- a/target/openrisc/cpu.h
+++ b/target/openrisc/cpu.h
@@ -48,7 +48,7 @@ typedef struct OpenRISCCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } OpenRISCCPUClass;
 
 #define TARGET_INSN_START_EXTRA_WORDS 1
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index e499575dc873..9a20e5a1bfea 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -166,7 +166,7 @@ typedef struct PowerPCCPUClass {
 
     DeviceRealize parent_realize;
     DeviceUnrealize parent_unrealize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
     void (*parent_parse_features)(const char *type, char *str, Error **errp);
 
     uint32_t pvr;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index e59343e13c02..2246f95b3f33 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -185,7 +185,7 @@ typedef struct RISCVCPUClass {
     CPUClass parent_class;
     /*< public >*/
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } RISCVCPUClass;
 
 /**
diff --git a/target/s390x/cpu-qom.h b/target/s390x/cpu-qom.h
index dbe5346ec901..daa955f08907 100644
--- a/target/s390x/cpu-qom.h
+++ b/target/s390x/cpu-qom.h
@@ -61,7 +61,7 @@ typedef struct S390CPUClass {
     const char *desc;
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
     void (*load_normal)(CPUState *cpu);
     void (*reset)(CPUState *cpu, cpu_reset_type type);
 } S390CPUClass;
diff --git a/target/sh4/cpu-qom.h b/target/sh4/cpu-qom.h
index 0c56d055bada..35732a367427 100644
--- a/target/sh4/cpu-qom.h
+++ b/target/sh4/cpu-qom.h
@@ -51,7 +51,7 @@ typedef struct SuperHCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 
     uint32_t pvr;
     uint32_t prr;
diff --git a/target/sparc/cpu-qom.h b/target/sparc/cpu-qom.h
index 7442e2768e88..93165bd24f1c 100644
--- a/target/sparc/cpu-qom.h
+++ b/target/sparc/cpu-qom.h
@@ -49,7 +49,7 @@ typedef struct SPARCCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
     sparc_def_t *cpu_def;
 } SPARCCPUClass;
 
diff --git a/target/tilegx/cpu.h b/target/tilegx/cpu.h
index 9cbec247d238..68bd509898d4 100644
--- a/target/tilegx/cpu.h
+++ b/target/tilegx/cpu.h
@@ -118,7 +118,7 @@ typedef struct TileGXCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } TileGXCPUClass;
 
 /**
diff --git a/target/tricore/cpu-qom.h b/target/tricore/cpu-qom.h
index 7c1e130b4ede..f613452b00e0 100644
--- a/target/tricore/cpu-qom.h
+++ b/target/tricore/cpu-qom.h
@@ -36,7 +36,7 @@ typedef struct TriCoreCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 } TriCoreCPUClass;
 
 typedef struct TriCoreCPU TriCoreCPU;
diff --git a/target/xtensa/cpu-qom.h b/target/xtensa/cpu-qom.h
index 9ac54241bd69..685d7b8d823a 100644
--- a/target/xtensa/cpu-qom.h
+++ b/target/xtensa/cpu-qom.h
@@ -56,7 +56,7 @@ typedef struct XtensaCPUClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
-    void (*parent_reset)(CPUState *cpu);
+    CPUReset parent_reset;
 
     const XtensaConfig *config;
 } XtensaCPUClass;

[for-5.0 PATCH v2 2/3] cpu: Introduce cpu_class_set_parent_reset()

[Greg Kurz]

Similarly to what we already do with qdev, use a helper to overload the
reset QOM methods of the parent in children classes, for clarity.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/core/cpu.c         |    8 ++++++++
 include/hw/core/cpu.h |    4 ++++
 2 files changed, 12 insertions(+)

diff --git a/hw/core/cpu.c b/hw/core/cpu.c
index db1a03c6bbb3..6dad2c8488a9 100644
--- a/hw/core/cpu.c
+++ b/hw/core/cpu.c
@@ -239,6 +239,14 @@ void cpu_dump_statistics(CPUState *cpu, int flags)
     }
 }
 
+void cpu_class_set_parent_reset(CPUClass *cc,
+                                CPUReset child_reset,
+                                CPUReset *parent_reset)
+{
+    *parent_reset = cc->reset;
+    cc->reset = child_reset;
+}
+
 void cpu_reset(CPUState *cpu)
 {
     CPUClass *klass = CPU_GET_CLASS(cpu);
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 047e3972ecaf..6680f4b047f4 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -1137,6 +1137,10 @@ void cpu_exec_unrealizefn(CPUState *cpu);
  */
 bool target_words_bigendian(void);
 
+void cpu_class_set_parent_reset(CPUClass *cc,
+                                CPUReset child_reset,
+                                CPUReset *parent_reset);
+
 #ifdef NEED_CPU_H
 
 #ifdef CONFIG_SOFTMMU

[for-5.0 PATCH v2 3/3] cpu: Use cpu_class_set_parent_reset()

[Greg Kurz]

Convert all targets to use cpu_class_set_parent_reset() with the following
coccinelle script:

@@
type CPUParentClass;
CPUParentClass *pcc;
CPUClass *cc;
identifier parent_fn;
identifier child_fn;
@@
+cpu_class_set_parent_reset(cc, child_fn, &pcc->parent_fn);
-pcc->parent_fn = cc->reset;
...
-cc->reset = child_fn;

Signed-off-by: Greg Kurz <groug@kaod.org>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/arm/cpu.c                |    3 +--
 target/cris/cpu.c               |    3 +--
 target/i386/cpu.c               |    3 +--
 target/lm32/cpu.c               |    3 +--
 target/m68k/cpu.c               |    3 +--
 target/microblaze/cpu.c         |    3 +--
 target/mips/cpu.c               |    3 +--
 target/moxie/cpu.c              |    3 +--
 target/nios2/cpu.c              |    3 +--
 target/openrisc/cpu.c           |    3 +--
 target/ppc/translate_init.inc.c |    3 +--
 target/riscv/cpu.c              |    3 +--
 target/s390x/cpu.c              |    3 +--
 target/sh4/cpu.c                |    3 +--
 target/sparc/cpu.c              |    3 +--
 target/tilegx/cpu.c             |    3 +--
 target/tricore/cpu.c            |    3 +--
 target/xtensa/cpu.c             |    3 +--
 18 files changed, 18 insertions(+), 36 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 7a4ac9339bf9..712a9425fdf5 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2625,8 +2625,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
                                     &acc->parent_realize);
     dc->props = arm_cpu_properties;
 
-    acc->parent_reset = cc->reset;
-    cc->reset = arm_cpu_reset;
+    cpu_class_set_parent_reset(cc, arm_cpu_reset, &acc->parent_reset);
 
     cc->class_by_name = arm_cpu_class_by_name;
     cc->has_work = arm_cpu_has_work;
diff --git a/target/cris/cpu.c b/target/cris/cpu.c
index 7adfd6caf4ed..486675e3822f 100644
--- a/target/cris/cpu.c
+++ b/target/cris/cpu.c
@@ -256,8 +256,7 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data)
     device_class_set_parent_realize(dc, cris_cpu_realizefn,
                                     &ccc->parent_realize);
 
-    ccc->parent_reset = cc->reset;
-    cc->reset = cris_cpu_reset;
+    cpu_class_set_parent_reset(cc, cris_cpu_reset, &ccc->parent_reset);
 
     cc->class_by_name = cris_cpu_class_by_name;
     cc->has_work = cris_cpu_has_work;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 69f518a21a9b..57d36931725d 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7049,8 +7049,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
                                       &xcc->parent_unrealize);
     dc->props = x86_cpu_properties;
 
-    xcc->parent_reset = cc->reset;
-    cc->reset = x86_cpu_reset;
+    cpu_class_set_parent_reset(cc, x86_cpu_reset, &xcc->parent_reset);
     cc->reset_dump_flags = CPU_DUMP_FPU | CPU_DUMP_CCOP;
 
     cc->class_by_name = x86_cpu_class_by_name;
diff --git a/target/lm32/cpu.c b/target/lm32/cpu.c
index b35537de6285..687bf35e6588 100644
--- a/target/lm32/cpu.c
+++ b/target/lm32/cpu.c
@@ -218,8 +218,7 @@ static void lm32_cpu_class_init(ObjectClass *oc, void *data)
 
     device_class_set_parent_realize(dc, lm32_cpu_realizefn,
                                     &lcc->parent_realize);
-    lcc->parent_reset = cc->reset;
-    cc->reset = lm32_cpu_reset;
+    cpu_class_set_parent_reset(cc, lm32_cpu_reset, &lcc->parent_reset);
 
     cc->class_by_name = lm32_cpu_class_by_name;
     cc->has_work = lm32_cpu_has_work;
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index e6596de29c2c..176d95e6fcfb 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -257,8 +257,7 @@ static void m68k_cpu_class_init(ObjectClass *c, void *data)
 
     device_class_set_parent_realize(dc, m68k_cpu_realizefn,
                                     &mcc->parent_realize);
-    mcc->parent_reset = cc->reset;
-    cc->reset = m68k_cpu_reset;
+    cpu_class_set_parent_reset(cc, m68k_cpu_reset, &mcc->parent_reset);
 
     cc->class_by_name = m68k_cpu_class_by_name;
     cc->has_work = m68k_cpu_has_work;
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 9cfd7445e7da..71d88f603b2e 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -292,8 +292,7 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data)
 
     device_class_set_parent_realize(dc, mb_cpu_realizefn,
                                     &mcc->parent_realize);
-    mcc->parent_reset = cc->reset;
-    cc->reset = mb_cpu_reset;
+    cpu_class_set_parent_reset(cc, mb_cpu_reset, &mcc->parent_reset);
 
     cc->class_by_name = mb_cpu_class_by_name;
     cc->has_work = mb_cpu_has_work;
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index bbcf7ca4635c..6cd6b9650baa 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -189,8 +189,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
 
     device_class_set_parent_realize(dc, mips_cpu_realizefn,
                                     &mcc->parent_realize);
-    mcc->parent_reset = cc->reset;
-    cc->reset = mips_cpu_reset;
+    cpu_class_set_parent_reset(cc, mips_cpu_reset, &mcc->parent_reset);
 
     cc->class_by_name = mips_cpu_class_by_name;
     cc->has_work = mips_cpu_has_work;
diff --git a/target/moxie/cpu.c b/target/moxie/cpu.c
index 48996d0554f2..cf47bc709b54 100644
--- a/target/moxie/cpu.c
+++ b/target/moxie/cpu.c
@@ -101,8 +101,7 @@ static void moxie_cpu_class_init(ObjectClass *oc, void *data)
 
     device_class_set_parent_realize(dc, moxie_cpu_realizefn,
                                     &mcc->parent_realize);
-    mcc->parent_reset = cc->reset;
-    cc->reset = moxie_cpu_reset;
+    cpu_class_set_parent_reset(cc, moxie_cpu_reset, &mcc->parent_reset);
 
     cc->class_by_name = moxie_cpu_class_by_name;
 
diff --git a/target/nios2/cpu.c b/target/nios2/cpu.c
index ca9c7a6df5d1..bbdbc0c6fbf0 100644
--- a/target/nios2/cpu.c
+++ b/target/nios2/cpu.c
@@ -188,8 +188,7 @@ static void nios2_cpu_class_init(ObjectClass *oc, void *data)
     device_class_set_parent_realize(dc, nios2_cpu_realizefn,
                                     &ncc->parent_realize);
     dc->props = nios2_properties;
-    ncc->parent_reset = cc->reset;
-    cc->reset = nios2_cpu_reset;
+    cpu_class_set_parent_reset(cc, nios2_cpu_reset, &ncc->parent_reset);
 
     cc->class_by_name = nios2_cpu_class_by_name;
     cc->has_work = nios2_cpu_has_work;
diff --git a/target/openrisc/cpu.c b/target/openrisc/cpu.c
index 506aec6bfba5..5cd04dafab69 100644
--- a/target/openrisc/cpu.c
+++ b/target/openrisc/cpu.c
@@ -150,8 +150,7 @@ static void openrisc_cpu_class_init(ObjectClass *oc, void *data)
 
     device_class_set_parent_realize(dc, openrisc_cpu_realizefn,
                                     &occ->parent_realize);
-    occ->parent_reset = cc->reset;
-    cc->reset = openrisc_cpu_reset;
+    cpu_class_set_parent_reset(cc, openrisc_cpu_reset, &occ->parent_reset);
 
     cc->class_by_name = openrisc_cpu_class_by_name;
     cc->has_work = openrisc_cpu_has_work;
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index ba726dec4d00..e5773a99fffd 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -10614,8 +10614,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_always;
     dc->props = ppc_cpu_properties;
 
-    pcc->parent_reset = cc->reset;
-    cc->reset = ppc_cpu_reset;
+    cpu_class_set_parent_reset(cc, ppc_cpu_reset, &pcc->parent_reset);
 
     cc->class_by_name = ppc_cpu_class_by_name;
     pcc->parent_parse_features = cc->parse_features;
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d37861a4305b..d6f187272859 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -462,8 +462,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
     device_class_set_parent_realize(dc, riscv_cpu_realize,
                                     &mcc->parent_realize);
 
-    mcc->parent_reset = cc->reset;
-    cc->reset = riscv_cpu_reset;
+    cpu_class_set_parent_reset(cc, riscv_cpu_reset, &mcc->parent_reset);
 
     cc->class_by_name = riscv_cpu_class_by_name;
     cc->has_work = riscv_cpu_has_work;
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 625daeedd133..ca487f5fdd0d 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -456,12 +456,11 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
     dc->props = s390x_cpu_properties;
     dc->user_creatable = true;
 
-    scc->parent_reset = cc->reset;
+    cpu_class_set_parent_reset(cc, 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->reset = s390_cpu_reset_full;
     cc->class_by_name = s390_cpu_class_by_name,
     cc->has_work = s390_cpu_has_work;
 #ifdef CONFIG_TCG
diff --git a/target/sh4/cpu.c b/target/sh4/cpu.c
index d0a7707991fe..70c8d8170ff3 100644
--- a/target/sh4/cpu.c
+++ b/target/sh4/cpu.c
@@ -214,8 +214,7 @@ static void superh_cpu_class_init(ObjectClass *oc, void *data)
     device_class_set_parent_realize(dc, superh_cpu_realizefn,
                                     &scc->parent_realize);
 
-    scc->parent_reset = cc->reset;
-    cc->reset = superh_cpu_reset;
+    cpu_class_set_parent_reset(cc, superh_cpu_reset, &scc->parent_reset);
 
     cc->class_by_name = superh_cpu_class_by_name;
     cc->has_work = superh_cpu_has_work;
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index bc659295520f..9c306e52717e 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -859,8 +859,7 @@ static void sparc_cpu_class_init(ObjectClass *oc, void *data)
                                     &scc->parent_realize);
     dc->props = sparc_cpu_properties;
 
-    scc->parent_reset = cc->reset;
-    cc->reset = sparc_cpu_reset;
+    cpu_class_set_parent_reset(cc, sparc_cpu_reset, &scc->parent_reset);
 
     cc->class_by_name = sparc_cpu_class_by_name;
     cc->parse_features = sparc_cpu_parse_features;
diff --git a/target/tilegx/cpu.c b/target/tilegx/cpu.c
index 2b2a7ccc313f..cd422a0467a0 100644
--- a/target/tilegx/cpu.c
+++ b/target/tilegx/cpu.c
@@ -142,8 +142,7 @@ static void tilegx_cpu_class_init(ObjectClass *oc, void *data)
     device_class_set_parent_realize(dc, tilegx_cpu_realizefn,
                                     &tcc->parent_realize);
 
-    tcc->parent_reset = cc->reset;
-    cc->reset = tilegx_cpu_reset;
+    cpu_class_set_parent_reset(cc, tilegx_cpu_reset, &tcc->parent_reset);
 
     cc->class_by_name = tilegx_cpu_class_by_name;
     cc->has_work = tilegx_cpu_has_work;
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index df807c1d7437..85bc9f03a1ee 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -153,8 +153,7 @@ static void tricore_cpu_class_init(ObjectClass *c, void *data)
     device_class_set_parent_realize(dc, tricore_cpu_realizefn,
                                     &mcc->parent_realize);
 
-    mcc->parent_reset = cc->reset;
-    cc->reset = tricore_cpu_reset;
+    cpu_class_set_parent_reset(cc, tricore_cpu_reset, &mcc->parent_reset);
     cc->class_by_name = tricore_cpu_class_by_name;
     cc->has_work = tricore_cpu_has_work;
 
diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index c65dcf9dd782..4856aee8eca6 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -184,8 +184,7 @@ static void xtensa_cpu_class_init(ObjectClass *oc, void *data)
     device_class_set_parent_realize(dc, xtensa_cpu_realizefn,
                                     &xcc->parent_realize);
 
-    xcc->parent_reset = cc->reset;
-    cc->reset = xtensa_cpu_reset;
+    cpu_class_set_parent_reset(cc, xtensa_cpu_reset, &xcc->parent_reset);
 
     cc->class_by_name = xtensa_cpu_class_by_name;
     cc->has_work = xtensa_cpu_has_work;

Re: [for-5.0 PATCH v2 1/3] cpu: Introduce CPUReset callback typedef

[Cornelia Huck]

On Mon, 09 Dec 2019 18:55:18 +0100
Greg Kurz <groug@kaod.org> wrote:

> Use it in include/hw/core/cpu.h and convert all targets to use it as
> well with:
> 
> perl -pi \
>  -e 's/void\s+\(\*(parent_reset)\)\(CPUState\s+\*\w+\)/CPUReset \1/;' \
>  $(git ls-files 'target/*.h')
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/hw/core/cpu.h       |    4 +++-
>  target/alpha/cpu-qom.h      |    2 +-
>  target/arm/cpu-qom.h        |    2 +-
>  target/cris/cpu-qom.h       |    2 +-
>  target/hppa/cpu-qom.h       |    2 +-
>  target/i386/cpu-qom.h       |    2 +-
>  target/lm32/cpu-qom.h       |    2 +-
>  target/m68k/cpu-qom.h       |    2 +-
>  target/microblaze/cpu-qom.h |    2 +-
>  target/mips/cpu-qom.h       |    2 +-
>  target/moxie/cpu.h          |    2 +-
>  target/nios2/cpu.h          |    2 +-
>  target/openrisc/cpu.h       |    2 +-
>  target/ppc/cpu-qom.h        |    2 +-
>  target/riscv/cpu.h          |    2 +-
>  target/s390x/cpu-qom.h      |    2 +-
>  target/sh4/cpu-qom.h        |    2 +-
>  target/sparc/cpu-qom.h      |    2 +-
>  target/tilegx/cpu.h         |    2 +-
>  target/tricore/cpu-qom.h    |    2 +-
>  target/xtensa/cpu-qom.h     |    2 +-
>  21 files changed, 23 insertions(+), 21 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [for-5.0 PATCH v2 3/3] cpu: Use cpu_class_set_parent_reset()

[Cornelia Huck]

On Mon, 09 Dec 2019 18:55:30 +0100
Greg Kurz <groug@kaod.org> wrote:

> Convert all targets to use cpu_class_set_parent_reset() with the following
> coccinelle script:
> 
> @@
> type CPUParentClass;
> CPUParentClass *pcc;
> CPUClass *cc;
> identifier parent_fn;
> identifier child_fn;
> @@
> +cpu_class_set_parent_reset(cc, child_fn, &pcc->parent_fn);
> -pcc->parent_fn = cc->reset;
> ...
> -cc->reset = child_fn;
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/arm/cpu.c                |    3 +--
>  target/cris/cpu.c               |    3 +--
>  target/i386/cpu.c               |    3 +--
>  target/lm32/cpu.c               |    3 +--
>  target/m68k/cpu.c               |    3 +--
>  target/microblaze/cpu.c         |    3 +--
>  target/mips/cpu.c               |    3 +--
>  target/moxie/cpu.c              |    3 +--
>  target/nios2/cpu.c              |    3 +--
>  target/openrisc/cpu.c           |    3 +--
>  target/ppc/translate_init.inc.c |    3 +--
>  target/riscv/cpu.c              |    3 +--
>  target/s390x/cpu.c              |    3 +--
>  target/sh4/cpu.c                |    3 +--
>  target/sparc/cpu.c              |    3 +--
>  target/tilegx/cpu.c             |    3 +--
>  target/tricore/cpu.c            |    3 +--
>  target/xtensa/cpu.c             |    3 +--
>  18 files changed, 18 insertions(+), 36 deletions(-)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [for-5.0 PATCH v2 0/3] cpu: Clarify overloading of reset QOM methods

[Cornelia Huck]

On Mon, 09 Dec 2019 18:55:12 +0100
Greg Kurz <groug@kaod.org> wrote:

> Each cpu subclass overloads the reset method of its parent class with
> its own. But since it needs to call the parent method as well, it keeps
> a parent_reset pointer to do so. This causes the same not very explicit
> boiler plate to be duplicated all around the place:
> 
>     pcc->parent_reset = cc->reset;
>     cc->reset = ppc_cpu_reset;
> 
> A similar concern was addressed some time back by Philippe Mathieu-Daudé
> in qdev, with the addition of device_class_set_parent_reset() and friends:
> 
> https://git.qemu.org/?p=qemu.git;a=commit;h=46795cf2e2f6
> https://git.qemu.org/?p=qemu.git;a=commit;h=bf853881690d
> 
> Follow the same approach with cpus.
> 
> Changes in v2:
> - added Reviewed-by and Acked-by tags
> - rebased on top of https://github.com/cohuck/qemu.git s390-next
>   SHA1 dd6252f035a2

My apologies for the churn. I'll try to flush my queue ASAP after 5.0
development opens.

> 
> --
> Greg
> 
> ---
> 
> Greg Kurz (3):
>       cpu: Introduce CPUReset callback typedef
>       cpu: Introduce cpu_class_set_parent_reset()
>       cpu: Use cpu_class_set_parent_reset()
> 
> 
>  hw/core/cpu.c                   |    8 ++++++++
>  include/hw/core/cpu.h           |    8 +++++++-
>  target/alpha/cpu-qom.h          |    2 +-
>  target/arm/cpu-qom.h            |    2 +-
>  target/arm/cpu.c                |    3 +--
>  target/cris/cpu-qom.h           |    2 +-
>  target/cris/cpu.c               |    3 +--
>  target/hppa/cpu-qom.h           |    2 +-
>  target/i386/cpu-qom.h           |    2 +-
>  target/i386/cpu.c               |    3 +--
>  target/lm32/cpu-qom.h           |    2 +-
>  target/lm32/cpu.c               |    3 +--
>  target/m68k/cpu-qom.h           |    2 +-
>  target/m68k/cpu.c               |    3 +--
>  target/microblaze/cpu-qom.h     |    2 +-
>  target/microblaze/cpu.c         |    3 +--
>  target/mips/cpu-qom.h           |    2 +-
>  target/mips/cpu.c               |    3 +--
>  target/moxie/cpu.c              |    3 +--
>  target/moxie/cpu.h              |    2 +-
>  target/nios2/cpu.c              |    3 +--
>  target/nios2/cpu.h              |    2 +-
>  target/openrisc/cpu.c           |    3 +--
>  target/openrisc/cpu.h           |    2 +-
>  target/ppc/cpu-qom.h            |    2 +-
>  target/ppc/translate_init.inc.c |    3 +--
>  target/riscv/cpu.c              |    3 +--
>  target/riscv/cpu.h              |    2 +-
>  target/s390x/cpu-qom.h          |    2 +-
>  target/s390x/cpu.c              |    3 +--
>  target/sh4/cpu-qom.h            |    2 +-
>  target/sh4/cpu.c                |    3 +--
>  target/sparc/cpu-qom.h          |    2 +-
>  target/sparc/cpu.c              |    3 +--
>  target/tilegx/cpu.c             |    3 +--
>  target/tilegx/cpu.h             |    2 +-
>  target/tricore/cpu-qom.h        |    2 +-
>  target/tricore/cpu.c            |    3 +--
>  target/xtensa/cpu-qom.h         |    2 +-
>  target/xtensa/cpu.c             |    3 +--
>  40 files changed, 53 insertions(+), 57 deletions(-)
> 

Re: [for-5.0 PATCH v2 0/3] cpu: Clarify overloading of reset QOM methods

[Greg Kurz]

On Mon, 9 Dec 2019 19:21:43 +0100
Cornelia Huck <cohuck@redhat.com> wrote:

> On Mon, 09 Dec 2019 18:55:12 +0100
> Greg Kurz <groug@kaod.org> wrote:
> 
> > Each cpu subclass overloads the reset method of its parent class with
> > its own. But since it needs to call the parent method as well, it keeps
> > a parent_reset pointer to do so. This causes the same not very explicit
> > boiler plate to be duplicated all around the place:
> > 
> >     pcc->parent_reset = cc->reset;
> >     cc->reset = ppc_cpu_reset;
> > 
> > A similar concern was addressed some time back by Philippe Mathieu-Daudé
> > in qdev, with the addition of device_class_set_parent_reset() and friends:
> > 
> > https://git.qemu.org/?p=qemu.git;a=commit;h=46795cf2e2f6
> > https://git.qemu.org/?p=qemu.git;a=commit;h=bf853881690d
> > 
> > Follow the same approach with cpus.
> > 
> > Changes in v2:
> > - added Reviewed-by and Acked-by tags
> > - rebased on top of https://github.com/cohuck/qemu.git s390-next
> >   SHA1 dd6252f035a2
> 
> My apologies for the churn. I'll try to flush my queue ASAP after 5.0
> development opens.
> 

No problems. It is perfectly fine to queue patches during freeze :)

> > 
> > --
> > Greg
> > 
> > ---
> > 
> > Greg Kurz (3):
> >       cpu: Introduce CPUReset callback typedef
> >       cpu: Introduce cpu_class_set_parent_reset()
> >       cpu: Use cpu_class_set_parent_reset()
> > 
> > 
> >  hw/core/cpu.c                   |    8 ++++++++
> >  include/hw/core/cpu.h           |    8 +++++++-
> >  target/alpha/cpu-qom.h          |    2 +-
> >  target/arm/cpu-qom.h            |    2 +-
> >  target/arm/cpu.c                |    3 +--
> >  target/cris/cpu-qom.h           |    2 +-
> >  target/cris/cpu.c               |    3 +--
> >  target/hppa/cpu-qom.h           |    2 +-
> >  target/i386/cpu-qom.h           |    2 +-
> >  target/i386/cpu.c               |    3 +--
> >  target/lm32/cpu-qom.h           |    2 +-
> >  target/lm32/cpu.c               |    3 +--
> >  target/m68k/cpu-qom.h           |    2 +-
> >  target/m68k/cpu.c               |    3 +--
> >  target/microblaze/cpu-qom.h     |    2 +-
> >  target/microblaze/cpu.c         |    3 +--
> >  target/mips/cpu-qom.h           |    2 +-
> >  target/mips/cpu.c               |    3 +--
> >  target/moxie/cpu.c              |    3 +--
> >  target/moxie/cpu.h              |    2 +-
> >  target/nios2/cpu.c              |    3 +--
> >  target/nios2/cpu.h              |    2 +-
> >  target/openrisc/cpu.c           |    3 +--
> >  target/openrisc/cpu.h           |    2 +-
> >  target/ppc/cpu-qom.h            |    2 +-
> >  target/ppc/translate_init.inc.c |    3 +--
> >  target/riscv/cpu.c              |    3 +--
> >  target/riscv/cpu.h              |    2 +-
> >  target/s390x/cpu-qom.h          |    2 +-
> >  target/s390x/cpu.c              |    3 +--
> >  target/sh4/cpu-qom.h            |    2 +-
> >  target/sh4/cpu.c                |    3 +--
> >  target/sparc/cpu-qom.h          |    2 +-
> >  target/sparc/cpu.c              |    3 +--
> >  target/tilegx/cpu.c             |    3 +--
> >  target/tilegx/cpu.h             |    2 +-
> >  target/tricore/cpu-qom.h        |    2 +-
> >  target/tricore/cpu.c            |    3 +--
> >  target/xtensa/cpu-qom.h         |    2 +-
> >  target/xtensa/cpu.c             |    3 +--
> >  40 files changed, 53 insertions(+), 57 deletions(-)
> > 
> 

Re: [for-5.0 PATCH v2 1/3] cpu: Introduce CPUReset callback typedef

[Markus Armbruster]

Greg Kurz <groug@kaod.org> writes:

> Use it in include/hw/core/cpu.h and convert all targets to use it as
> well with:
>
> perl -pi \
>  -e 's/void\s+\(\*(parent_reset)\)\(CPUState\s+\*\w+\)/CPUReset \1/;' \
>  $(git ls-files 'target/*.h')
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/hw/core/cpu.h       |    4 +++-
>  target/alpha/cpu-qom.h      |    2 +-
>  target/arm/cpu-qom.h        |    2 +-
>  target/cris/cpu-qom.h       |    2 +-
>  target/hppa/cpu-qom.h       |    2 +-
>  target/i386/cpu-qom.h       |    2 +-
>  target/lm32/cpu-qom.h       |    2 +-
>  target/m68k/cpu-qom.h       |    2 +-
>  target/microblaze/cpu-qom.h |    2 +-
>  target/mips/cpu-qom.h       |    2 +-
>  target/moxie/cpu.h          |    2 +-
>  target/nios2/cpu.h          |    2 +-
>  target/openrisc/cpu.h       |    2 +-
>  target/ppc/cpu-qom.h        |    2 +-
>  target/riscv/cpu.h          |    2 +-
>  target/s390x/cpu-qom.h      |    2 +-
>  target/sh4/cpu-qom.h        |    2 +-
>  target/sparc/cpu-qom.h      |    2 +-
>  target/tilegx/cpu.h         |    2 +-
>  target/tricore/cpu-qom.h    |    2 +-
>  target/xtensa/cpu-qom.h     |    2 +-
>  21 files changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 77c6f0529903..047e3972ecaf 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -74,6 +74,8 @@ typedef struct CPUWatchpoint CPUWatchpoint;
>  
>  struct TranslationBlock;
>  
> +typedef void (*CPUReset)(CPUState *cpu);
> +
>  /**
>   * CPUClass:
>   * @class_by_name: Callback to map -cpu command line model name to an
> @@ -165,7 +167,7 @@ typedef struct CPUClass {
>      ObjectClass *(*class_by_name)(const char *cpu_model);
>      void (*parse_features)(const char *typename, char *str, Error **errp);
>  
> -    void (*reset)(CPUState *cpu);
> +    CPUReset reset;
>      int reset_dump_flags;
>      bool (*has_work)(CPUState *cpu);
>      void (*do_interrupt)(CPUState *cpu);
[...]

Opinion, not objection: such typedefs make the code less obvious.

Re: [for-5.0 PATCH v2 0/3] cpu: Clarify overloading of reset QOM methods

[David Hildenbrand]

On 09.12.19 18:55, Greg Kurz wrote:
> Each cpu subclass overloads the reset method of its parent class with
> its own. But since it needs to call the parent method as well, it keeps
> a parent_reset pointer to do so. This causes the same not very explicit
> boiler plate to be duplicated all around the place:
> 
>     pcc->parent_reset = cc->reset;
>     cc->reset = ppc_cpu_reset;
> 
> A similar concern was addressed some time back by Philippe Mathieu-Daudé
> in qdev, with the addition of device_class_set_parent_reset() and friends:
> 
> https://git.qemu.org/?p=qemu.git;a=commit;h=46795cf2e2f6
> https://git.qemu.org/?p=qemu.git;a=commit;h=bf853881690d
> 
> Follow the same approach with cpus.
> 
> Changes in v2:
> - added Reviewed-by and Acked-by tags
> - rebased on top of https://github.com/cohuck/qemu.git s390-next
>   SHA1 dd6252f035a2
> 
> --
> Greg
> 
> ---
> 
> Greg Kurz (3):
>       cpu: Introduce CPUReset callback typedef
>       cpu: Introduce cpu_class_set_parent_reset()
>       cpu: Use cpu_class_set_parent_reset()
> 
> 
>  hw/core/cpu.c                   |    8 ++++++++
>  include/hw/core/cpu.h           |    8 +++++++-
>  target/alpha/cpu-qom.h          |    2 +-
>  target/arm/cpu-qom.h            |    2 +-
>  target/arm/cpu.c                |    3 +--
>  target/cris/cpu-qom.h           |    2 +-
>  target/cris/cpu.c               |    3 +--
>  target/hppa/cpu-qom.h           |    2 +-
>  target/i386/cpu-qom.h           |    2 +-
>  target/i386/cpu.c               |    3 +--
>  target/lm32/cpu-qom.h           |    2 +-
>  target/lm32/cpu.c               |    3 +--
>  target/m68k/cpu-qom.h           |    2 +-
>  target/m68k/cpu.c               |    3 +--
>  target/microblaze/cpu-qom.h     |    2 +-
>  target/microblaze/cpu.c         |    3 +--
>  target/mips/cpu-qom.h           |    2 +-
>  target/mips/cpu.c               |    3 +--
>  target/moxie/cpu.c              |    3 +--
>  target/moxie/cpu.h              |    2 +-
>  target/nios2/cpu.c              |    3 +--
>  target/nios2/cpu.h              |    2 +-
>  target/openrisc/cpu.c           |    3 +--
>  target/openrisc/cpu.h           |    2 +-
>  target/ppc/cpu-qom.h            |    2 +-
>  target/ppc/translate_init.inc.c |    3 +--
>  target/riscv/cpu.c              |    3 +--
>  target/riscv/cpu.h              |    2 +-
>  target/s390x/cpu-qom.h          |    2 +-
>  target/s390x/cpu.c              |    3 +--
>  target/sh4/cpu-qom.h            |    2 +-
>  target/sh4/cpu.c                |    3 +--
>  target/sparc/cpu-qom.h          |    2 +-
>  target/sparc/cpu.c              |    3 +--
>  target/tilegx/cpu.c             |    3 +--
>  target/tilegx/cpu.h             |    2 +-
>  target/tricore/cpu-qom.h        |    2 +-
>  target/tricore/cpu.c            |    3 +--
>  target/xtensa/cpu-qom.h         |    2 +-
>  target/xtensa/cpu.c             |    3 +--
>  40 files changed, 53 insertions(+), 57 deletions(-)
> 

Acked-by: David Hildenbrand <david@redhat.com>

For all patches (this sorts out something that was bugging me a while
ago :) )

-- 
Thanks,

David / dhildenb

Re: [for-5.0 PATCH v2 1/3] cpu: Introduce CPUReset callback typedef

[Peter Maydell]

On Tue, 10 Dec 2019 at 10:39, Markus Armbruster <armbru@redhat.com> wrote:
>
> Greg Kurz <groug@kaod.org> writes:
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 77c6f0529903..047e3972ecaf 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -74,6 +74,8 @@ typedef struct CPUWatchpoint CPUWatchpoint;
> >
> >  struct TranslationBlock;
> >
> > +typedef void (*CPUReset)(CPUState *cpu);
> > +
> >  /**
> >   * CPUClass:
> >   * @class_by_name: Callback to map -cpu command line model name to an
> > @@ -165,7 +167,7 @@ typedef struct CPUClass {
> >      ObjectClass *(*class_by_name)(const char *cpu_model);
> >      void (*parse_features)(const char *typename, char *str, Error **errp);
> >
> > -    void (*reset)(CPUState *cpu);
> > +    CPUReset reset;
> >      int reset_dump_flags;
> >      bool (*has_work)(CPUState *cpu);
> >      void (*do_interrupt)(CPUState *cpu);
> [...]
>
> Opinion, not objection: such typedefs make the code less obvious.

It's particularly odd here where this class has several
methods but we've only chosen one to privilege with a typedef.

Personal preference: if you use a typedef, typedef the
function type, not the pointer-to-the-function-type.
But I would just leave it be.

thanks
-- PMM

Re: [for-5.0 PATCH v2 1/3] cpu: Introduce CPUReset callback typedef

[Greg Kurz]

On Tue, 10 Dec 2019 11:39:00 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Greg Kurz <groug@kaod.org> writes:
> 
> > Use it in include/hw/core/cpu.h and convert all targets to use it as
> > well with:
> >
> > perl -pi \
> >  -e 's/void\s+\(\*(parent_reset)\)\(CPUState\s+\*\w+\)/CPUReset \1/;' \
> >  $(git ls-files 'target/*.h')
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > Acked-by: David Gibson <david@gibson.dropbear.id.au>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  include/hw/core/cpu.h       |    4 +++-
> >  target/alpha/cpu-qom.h      |    2 +-
> >  target/arm/cpu-qom.h        |    2 +-
> >  target/cris/cpu-qom.h       |    2 +-
> >  target/hppa/cpu-qom.h       |    2 +-
> >  target/i386/cpu-qom.h       |    2 +-
> >  target/lm32/cpu-qom.h       |    2 +-
> >  target/m68k/cpu-qom.h       |    2 +-
> >  target/microblaze/cpu-qom.h |    2 +-
> >  target/mips/cpu-qom.h       |    2 +-
> >  target/moxie/cpu.h          |    2 +-
> >  target/nios2/cpu.h          |    2 +-
> >  target/openrisc/cpu.h       |    2 +-
> >  target/ppc/cpu-qom.h        |    2 +-
> >  target/riscv/cpu.h          |    2 +-
> >  target/s390x/cpu-qom.h      |    2 +-
> >  target/sh4/cpu-qom.h        |    2 +-
> >  target/sparc/cpu-qom.h      |    2 +-
> >  target/tilegx/cpu.h         |    2 +-
> >  target/tricore/cpu-qom.h    |    2 +-
> >  target/xtensa/cpu-qom.h     |    2 +-
> >  21 files changed, 23 insertions(+), 21 deletions(-)
> >
> > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > index 77c6f0529903..047e3972ecaf 100644
> > --- a/include/hw/core/cpu.h
> > +++ b/include/hw/core/cpu.h
> > @@ -74,6 +74,8 @@ typedef struct CPUWatchpoint CPUWatchpoint;
> >  
> >  struct TranslationBlock;
> >  
> > +typedef void (*CPUReset)(CPUState *cpu);
> > +
> >  /**
> >   * CPUClass:
> >   * @class_by_name: Callback to map -cpu command line model name to an
> > @@ -165,7 +167,7 @@ typedef struct CPUClass {
> >      ObjectClass *(*class_by_name)(const char *cpu_model);
> >      void (*parse_features)(const char *typename, char *str, Error **errp);
> >  
> > -    void (*reset)(CPUState *cpu);
> > +    CPUReset reset;
> >      int reset_dump_flags;
> >      bool (*has_work)(CPUState *cpu);
> >      void (*do_interrupt)(CPUState *cpu);
> [...]
> 
> Opinion, not objection: such typedefs make the code less obvious.
> 

I merely followed what we have in qdev, but I tend to agree. And, as
mentioned by Peter in another mail, it looks odd to only convert the
reset method.

Re: [for-5.0 PATCH v2 1/3] cpu: Introduce CPUReset callback typedef

[Greg Kurz]

On Tue, 10 Dec 2019 10:42:51 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Tue, 10 Dec 2019 at 10:39, Markus Armbruster <armbru@redhat.com> wrote:
> >
> > Greg Kurz <groug@kaod.org> writes:
> > > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> > > index 77c6f0529903..047e3972ecaf 100644
> > > --- a/include/hw/core/cpu.h
> > > +++ b/include/hw/core/cpu.h
> > > @@ -74,6 +74,8 @@ typedef struct CPUWatchpoint CPUWatchpoint;
> > >
> > >  struct TranslationBlock;
> > >
> > > +typedef void (*CPUReset)(CPUState *cpu);
> > > +
> > >  /**
> > >   * CPUClass:
> > >   * @class_by_name: Callback to map -cpu command line model name to an
> > > @@ -165,7 +167,7 @@ typedef struct CPUClass {
> > >      ObjectClass *(*class_by_name)(const char *cpu_model);
> > >      void (*parse_features)(const char *typename, char *str, Error **errp);
> > >
> > > -    void (*reset)(CPUState *cpu);
> > > +    CPUReset reset;
> > >      int reset_dump_flags;
> > >      bool (*has_work)(CPUState *cpu);
> > >      void (*do_interrupt)(CPUState *cpu);
> > [...]
> >
> > Opinion, not objection: such typedefs make the code less obvious.
> 
> It's particularly odd here where this class has several
> methods but we've only chosen one to privilege with a typedef.
> 

Yes, children classes don't do the overloading-and-call-the-parent for
other methods which was the initial motivation for the typedef.

> Personal preference: if you use a typedef, typedef the
> function type, not the pointer-to-the-function-type.
> But I would just leave it be.
> 

Thinking again, I'm not sure the typedef really helps here. Markus
doesn't like it either. I'll try without.

> thanks
> -- PMM