[v3] cpu: Clarify overloading of reset QOM methods

[PATCH v3 0/2] cpu: Clarify overloading of reset QOM methods

Greg Kurz posted 2 patches 4 years, 4 months ago

Diff against v1 v2
Download series mbox

Test asan failed

Test checkpatch failed

Test FreeBSD failed

Test docker-mingw@fedora failed

Test docker-clang@ubuntu failed

Test docker-quick@centos7 failed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/157650846660.354886.16810288202617432561.stgit@bahia.lan

Maintainers: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, David Hildenbrand <david@redhat.com>, Chris Wulff <crwulff@gmail.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>, Anthony Green <green@moxielogic.com>, Marek Vasut <marex@denx.de>, Aleksandar Markovic <amarkovic@wavecomp.com>, Laurent Vivier <laurent@vivier.eu>, Richard Henderson <rth@twiddle.net>, Alistair Francis <Alistair.Francis@wdc.com>, Stafford Horne <shorne@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>, David Gibson <david@gibson.dropbear.id.au>, Artyom Tarasenko <atar4qemu@gmail.com>, Michael Walle <michael@walle.cc>, Peter Maydell <peter.maydell@linaro.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Max Filippov <jcmvbkbc@gmail.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Cornelia Huck <cohuck@redhat.com>

hw/core/cpu.c                   |    8 ++++++++
include/hw/core/cpu.h           |    4 ++++
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 +--
20 files changed, 30 insertions(+), 36 deletions(-)

Expand all Fold all

[PATCH v3 0/2] cpu: Clarify overloading of reset QOM methods

Posted by Greg Kurz 4 years, 4 months ago

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 v3:
- drop 'cpu: Introduce CPUReset callback typedef' patch which isn't needed
  and makes the code less clear. This changes the declaration of the helper
  in 'cpu: Introduce cpu_class_set_parent_reset()', but it is minor so I
  keep the Reviewed-by and Acked-by tags.

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 (2):
      cpu: Introduce cpu_class_set_parent_reset()
      cpu: Use cpu_class_set_parent_reset()


 hw/core/cpu.c                   |    8 ++++++++
 include/hw/core/cpu.h           |    4 ++++
 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 +--
 20 files changed, 30 insertions(+), 36 deletions(-)

Re: [PATCH v3 0/2] cpu: Clarify overloading of reset QOM methods

Posted by Paolo Bonzini 4 years, 3 months ago

On 16/12/19 16:01, 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 v3:
> - drop 'cpu: Introduce CPUReset callback typedef' patch which isn't needed
>   and makes the code less clear. This changes the declaration of the helper
>   in 'cpu: Introduce cpu_class_set_parent_reset()', but it is minor so I
>   keep the Reviewed-by and Acked-by tags.
> 
> 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 (2):
>       cpu: Introduce cpu_class_set_parent_reset()
>       cpu: Use cpu_class_set_parent_reset()
> 
> 
>  hw/core/cpu.c                   |    8 ++++++++
>  include/hw/core/cpu.h           |    4 ++++
>  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 +--
>  20 files changed, 30 insertions(+), 36 deletions(-)
> 

Queued, thanks.

Paolo