[RFC v3 0/4] QEMU cpus.c refactoring

Claudio Fontana posted 4 patches 3 years, 11 months ago
Test docker-mingw@fedora passed
Test checkpatch failed
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200525145440.29728-1-cfontana@suse.de
Maintainers: Wenchao Wang <wenchao.wang@intel.com>, "Michael S. Tsirkin" <mst@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Thomas Huth <thuth@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Sunil Muthuswamy <sunilmut@microsoft.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Roman Bolshakov <r.bolshakov@yadro.com>, Colin Xu <colin.xu@intel.com>, Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>, Richard Henderson <rth@twiddle.net>, Laurent Vivier <lvivier@redhat.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Alistair Francis <Alistair.Francis@wdc.com>, Juan Quintela <quintela@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Palmer Dabbelt <palmer@dabbelt.com>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
MAINTAINERS                                  |   14 +-
Makefile.target                              |    7 +-
accel/kvm/Makefile.objs                      |    2 +
accel/kvm/kvm-all.c                          |   15 +-
accel/kvm/kvm-cpus-interface.c               |   94 ++
accel/kvm/kvm-cpus-interface.h               |    8 +
accel/qtest.c                                |   88 +-
accel/stubs/kvm-stub.c                       |    3 +-
accel/tcg/Makefile.objs                      |    1 +
accel/tcg/cpu-exec.c                         |   43 +-
accel/tcg/tcg-all.c                          |   19 +-
accel/tcg/tcg-cpus-interface.c               |  523 ++++++
accel/tcg/tcg-cpus-interface.h               |    8 +
accel/tcg/translate-all.c                    |    3 +-
cpus.c                                       | 2290 --------------------------
docs/replay.txt                              |    6 +-
exec.c                                       |    4 -
hw/core/cpu.c                                |    1 +
hw/core/ptimer.c                             |    6 +-
hw/i386/x86.c                                |    1 +
include/exec/cpu-all.h                       |    4 +
include/exec/exec-all.h                      |    4 +-
include/hw/core/cpu.h                        |   37 -
include/qemu/main-loop.h                     |    5 +
include/qemu/timer.h                         |   22 +-
include/sysemu/cpu-throttle.h                |   50 +
include/sysemu/cpu-timers.h                  |   72 +
include/sysemu/cpus.h                        |   44 +-
include/sysemu/hvf.h                         |    1 -
include/sysemu/hw_accel.h                    |   57 +-
include/sysemu/kvm.h                         |    2 +-
include/sysemu/qtest.h                       |    2 +
include/sysemu/replay.h                      |    4 +-
migration/migration.c                        |    1 +
migration/ram.c                              |    1 +
replay/replay.c                              |    6 +-
softmmu/Makefile.objs                        |   13 +
arch_init.c => softmmu/arch_init.c           |    0
balloon.c => softmmu/balloon.c               |    0
softmmu/cpu-throttle.c                       |  122 ++
softmmu/cpu-timers.c                         |  267 +++
softmmu/cpus.c                               |  741 +++++++++
softmmu/icount.c                             |  496 ++++++
ioport.c => softmmu/ioport.c                 |    0
memory.c => softmmu/memory.c                 |    0
memory_mapping.c => softmmu/memory_mapping.c |    0
qtest.c => softmmu/qtest.c                   |   34 +-
softmmu/timers-state.h                       |   45 +
softmmu/vl.c                                 |    8 +-
stubs/Makefile.objs                          |    4 +-
stubs/clock-warp.c                           |    4 +-
stubs/cpu-get-clock.c                        |    3 +-
stubs/cpu-get-icount.c                       |   21 -
stubs/cpu-synchronize-state.c                |   15 +
stubs/icount.c                               |   22 +
stubs/qemu-timer-notify-cb.c                 |    8 +
stubs/qtest.c                                |    5 +
target/alpha/translate.c                     |    3 +-
target/arm/helper.c                          |    7 +-
target/i386/Makefile.objs                    |    7 +-
target/i386/hax-all.c                        |    6 +-
target/i386/hax-cpus-interface.c             |   85 +
target/i386/hax-cpus-interface.h             |    8 +
target/i386/hax-i386.h                       |    2 +
target/i386/hax-posix.c                      |   12 +
target/i386/hax-windows.c                    |   20 +
target/i386/hvf/Makefile.objs                |    2 +-
target/i386/hvf/hvf-cpus-interface.c         |   92 ++
target/i386/hvf/hvf-cpus-interface.h         |    8 +
target/i386/hvf/hvf.c                        |    5 +-
target/i386/whpx-all.c                       |    3 +
target/i386/whpx-cpus-interface.c            |   96 ++
target/i386/whpx-cpus-interface.h            |    8 +
target/riscv/csr.c                           |    8 +-
tests/ptimer-test-stubs.c                    |    7 +-
tests/test-timed-average.c                   |    2 +-
util/main-loop.c                             |    4 +-
util/qemu-timer.c                            |   12 +-
78 files changed, 3136 insertions(+), 2517 deletions(-)
create mode 100644 accel/kvm/kvm-cpus-interface.c
create mode 100644 accel/kvm/kvm-cpus-interface.h
create mode 100644 accel/tcg/tcg-cpus-interface.c
create mode 100644 accel/tcg/tcg-cpus-interface.h
delete mode 100644 cpus.c
create mode 100644 include/sysemu/cpu-throttle.h
create mode 100644 include/sysemu/cpu-timers.h
rename arch_init.c => softmmu/arch_init.c (100%)
rename balloon.c => softmmu/balloon.c (100%)
create mode 100644 softmmu/cpu-throttle.c
create mode 100644 softmmu/cpu-timers.c
create mode 100644 softmmu/cpus.c
create mode 100644 softmmu/icount.c
rename ioport.c => softmmu/ioport.c (100%)
rename memory.c => softmmu/memory.c (100%)
rename memory_mapping.c => softmmu/memory_mapping.c (100%)
rename qtest.c => softmmu/qtest.c (95%)
create mode 100644 softmmu/timers-state.h
delete mode 100644 stubs/cpu-get-icount.c
create mode 100644 stubs/cpu-synchronize-state.c
create mode 100644 stubs/icount.c
create mode 100644 stubs/qemu-timer-notify-cb.c
create mode 100644 target/i386/hax-cpus-interface.c
create mode 100644 target/i386/hax-cpus-interface.h
create mode 100644 target/i386/hvf/hvf-cpus-interface.c
create mode 100644 target/i386/hvf/hvf-cpus-interface.h
create mode 100644 target/i386/whpx-cpus-interface.c
create mode 100644 target/i386/whpx-cpus-interface.h
[RFC v3 0/4] QEMU cpus.c refactoring
Posted by Claudio Fontana 3 years, 11 months ago
Motivation and higher level steps:

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

--------

v2 -> v3:

* turned into a 4 patch series, adding a first patch moving
  softmmu code currently in top_srcdir to softmmu/

* cpu-throttle: moved to softmmu/

* cpu-timers, icount:

  - moved to softmmu/

  - fixed assumption of qtest_enabled() => icount_enabled()
  causing the failure of check-qtest-arm goal, in test-arm-mptimer.c

  Fix is in hw/core/ptimer.c,

  where the artificial timeout rate limit should not be applied
  under qtest_enabled(), in a similar way to how it is not applied
  for icount_enabled().

* CpuAccelInterface: no change.


--------


v1 -> v2:

* 1/3 (cpu-throttle): provide a description in the commit message

* 2/3 (cpu-timers, icount): in this v2 separate icount from cpu-timers,
  as icount is actually TCG-specific. Only build it under CONFIG_TCG.

  To do this, qtest had to be detached from icount. To this end, a
  trivial global counter for qtest has been introduced.

* 3/3 (CpuAccelInterface): provided a description.

This is point 8) in that plan. The idea is to extract the unrelated parts
in cpus, and register interfaces from each single accelerator to the main
cpus module (cpus.c).

While doing this RFC, I noticed some assumptions about Windows being
either TCG or HAX (not considering WHPX) that might need to be revisited.
I added a comment there.

The thing builds successfully based on Linux cross-compilations for
windows/hax, windows/whpx, and I got a good build on Darwin/hvf.

Tests run successully for tcg and kvm configurations, but did not test on
windows or darwin.

Welcome your feedback and help on this,

Claudio

*** BLURB HERE ***

Claudio Fontana (4):
  softmmu: move softmmu only files from root
  cpu-throttle: new module, extracted from cpus.c
  cpu-timers, icount: new modules
  cpus: extract out accel-specific code to each accel

 MAINTAINERS                                  |   14 +-
 Makefile.target                              |    7 +-
 accel/kvm/Makefile.objs                      |    2 +
 accel/kvm/kvm-all.c                          |   15 +-
 accel/kvm/kvm-cpus-interface.c               |   94 ++
 accel/kvm/kvm-cpus-interface.h               |    8 +
 accel/qtest.c                                |   88 +-
 accel/stubs/kvm-stub.c                       |    3 +-
 accel/tcg/Makefile.objs                      |    1 +
 accel/tcg/cpu-exec.c                         |   43 +-
 accel/tcg/tcg-all.c                          |   19 +-
 accel/tcg/tcg-cpus-interface.c               |  523 ++++++
 accel/tcg/tcg-cpus-interface.h               |    8 +
 accel/tcg/translate-all.c                    |    3 +-
 cpus.c                                       | 2290 --------------------------
 docs/replay.txt                              |    6 +-
 exec.c                                       |    4 -
 hw/core/cpu.c                                |    1 +
 hw/core/ptimer.c                             |    6 +-
 hw/i386/x86.c                                |    1 +
 include/exec/cpu-all.h                       |    4 +
 include/exec/exec-all.h                      |    4 +-
 include/hw/core/cpu.h                        |   37 -
 include/qemu/main-loop.h                     |    5 +
 include/qemu/timer.h                         |   22 +-
 include/sysemu/cpu-throttle.h                |   50 +
 include/sysemu/cpu-timers.h                  |   72 +
 include/sysemu/cpus.h                        |   44 +-
 include/sysemu/hvf.h                         |    1 -
 include/sysemu/hw_accel.h                    |   57 +-
 include/sysemu/kvm.h                         |    2 +-
 include/sysemu/qtest.h                       |    2 +
 include/sysemu/replay.h                      |    4 +-
 migration/migration.c                        |    1 +
 migration/ram.c                              |    1 +
 replay/replay.c                              |    6 +-
 softmmu/Makefile.objs                        |   13 +
 arch_init.c => softmmu/arch_init.c           |    0
 balloon.c => softmmu/balloon.c               |    0
 softmmu/cpu-throttle.c                       |  122 ++
 softmmu/cpu-timers.c                         |  267 +++
 softmmu/cpus.c                               |  741 +++++++++
 softmmu/icount.c                             |  496 ++++++
 ioport.c => softmmu/ioport.c                 |    0
 memory.c => softmmu/memory.c                 |    0
 memory_mapping.c => softmmu/memory_mapping.c |    0
 qtest.c => softmmu/qtest.c                   |   34 +-
 softmmu/timers-state.h                       |   45 +
 softmmu/vl.c                                 |    8 +-
 stubs/Makefile.objs                          |    4 +-
 stubs/clock-warp.c                           |    4 +-
 stubs/cpu-get-clock.c                        |    3 +-
 stubs/cpu-get-icount.c                       |   21 -
 stubs/cpu-synchronize-state.c                |   15 +
 stubs/icount.c                               |   22 +
 stubs/qemu-timer-notify-cb.c                 |    8 +
 stubs/qtest.c                                |    5 +
 target/alpha/translate.c                     |    3 +-
 target/arm/helper.c                          |    7 +-
 target/i386/Makefile.objs                    |    7 +-
 target/i386/hax-all.c                        |    6 +-
 target/i386/hax-cpus-interface.c             |   85 +
 target/i386/hax-cpus-interface.h             |    8 +
 target/i386/hax-i386.h                       |    2 +
 target/i386/hax-posix.c                      |   12 +
 target/i386/hax-windows.c                    |   20 +
 target/i386/hvf/Makefile.objs                |    2 +-
 target/i386/hvf/hvf-cpus-interface.c         |   92 ++
 target/i386/hvf/hvf-cpus-interface.h         |    8 +
 target/i386/hvf/hvf.c                        |    5 +-
 target/i386/whpx-all.c                       |    3 +
 target/i386/whpx-cpus-interface.c            |   96 ++
 target/i386/whpx-cpus-interface.h            |    8 +
 target/riscv/csr.c                           |    8 +-
 tests/ptimer-test-stubs.c                    |    7 +-
 tests/test-timed-average.c                   |    2 +-
 util/main-loop.c                             |    4 +-
 util/qemu-timer.c                            |   12 +-
 78 files changed, 3136 insertions(+), 2517 deletions(-)
 create mode 100644 accel/kvm/kvm-cpus-interface.c
 create mode 100644 accel/kvm/kvm-cpus-interface.h
 create mode 100644 accel/tcg/tcg-cpus-interface.c
 create mode 100644 accel/tcg/tcg-cpus-interface.h
 delete mode 100644 cpus.c
 create mode 100644 include/sysemu/cpu-throttle.h
 create mode 100644 include/sysemu/cpu-timers.h
 rename arch_init.c => softmmu/arch_init.c (100%)
 rename balloon.c => softmmu/balloon.c (100%)
 create mode 100644 softmmu/cpu-throttle.c
 create mode 100644 softmmu/cpu-timers.c
 create mode 100644 softmmu/cpus.c
 create mode 100644 softmmu/icount.c
 rename ioport.c => softmmu/ioport.c (100%)
 rename memory.c => softmmu/memory.c (100%)
 rename memory_mapping.c => softmmu/memory_mapping.c (100%)
 rename qtest.c => softmmu/qtest.c (95%)
 create mode 100644 softmmu/timers-state.h
 delete mode 100644 stubs/cpu-get-icount.c
 create mode 100644 stubs/cpu-synchronize-state.c
 create mode 100644 stubs/icount.c
 create mode 100644 stubs/qemu-timer-notify-cb.c
 create mode 100644 target/i386/hax-cpus-interface.c
 create mode 100644 target/i386/hax-cpus-interface.h
 create mode 100644 target/i386/hvf/hvf-cpus-interface.c
 create mode 100644 target/i386/hvf/hvf-cpus-interface.h
 create mode 100644 target/i386/whpx-cpus-interface.c
 create mode 100644 target/i386/whpx-cpus-interface.h

-- 
2.16.4


Re: [RFC v3 0/4] QEMU cpus.c refactoring
Posted by no-reply@patchew.org 3 years, 10 months ago
Patchew URL: https://patchew.org/QEMU/20200525145440.29728-1-cfontana@suse.de/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20200525145440.29728-1-cfontana@suse.de
Subject: [RFC v3 0/4] QEMU cpus.c refactoring
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20200525111001.8147-1-f4bug@amsat.org -> patchew/20200525111001.8147-1-f4bug@amsat.org
Switched to a new branch 'test'
0f5db0e cpus: extract out accel-specific code to each accel
1b135cf cpu-timers, icount: new modules
cb5c834 cpu-throttle: new module, extracted from cpus.c
3d5a04d softmmu: move softmmu only files from root

=== OUTPUT BEGIN ===
1/4 Checking commit 3d5a04d99492 (softmmu: move softmmu only files from root)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#112: 
rename from arch_init.c

total: 0 errors, 1 warnings, 77 lines checked

Patch 1/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/4 Checking commit cb5c834770dd (cpu-throttle: new module, extracted from cpus.c)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#110: 
new file mode 100644

total: 0 errors, 1 warnings, 386 lines checked

Patch 2/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/4 Checking commit 1b135cfa40c8 (cpu-timers, icount: new modules)
WARNING: line over 80 characters
#94: FILE: accel/tcg/cpu-exec.c:107:
+            qemu_printf("Warning: The guest is now late by %.1f to %.1f seconds\n",

ERROR: line over 90 characters
#265: FILE: hw/core/ptimer.c:137:
+    if (s->enabled == 1 && (delta * period < 10000) && !icount_enabled() && !qtest_enabled()) {

ERROR: line over 90 characters
#274: FILE: hw/core/ptimer.c:220:
+            if (!oneshot && (s->delta * period < 10000) && !icount_enabled() && !qtest_enabled()) {

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#384: 
new file mode 100644

total: 2 errors, 2 warnings, 2331 lines checked

Patch 3/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/4 Checking commit 0f5db0e76127 (cpus: extract out accel-specific code to each accel)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#86: 
new file mode 100644

total: 0 errors, 1 warnings, 2463 lines checked

Patch 4/4 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200525145440.29728-1-cfontana@suse.de/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [RFC v3 0/4] QEMU cpus.c refactoring
Posted by Claudio Fontana 3 years, 10 months ago
Some comments on patchew output:

On 5/26/20 12:58 AM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200525145440.29728-1-cfontana@suse.de/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Message-id: 20200525145440.29728-1-cfontana@suse.de
> Subject: [RFC v3 0/4] QEMU cpus.c refactoring
> Type: series
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  - [tag update]      patchew/20200525111001.8147-1-f4bug@amsat.org -> patchew/20200525111001.8147-1-f4bug@amsat.org
> Switched to a new branch 'test'
> 0f5db0e cpus: extract out accel-specific code to each accel
> 1b135cf cpu-timers, icount: new modules
> cb5c834 cpu-throttle: new module, extracted from cpus.c
> 3d5a04d softmmu: move softmmu only files from root
> 
> === OUTPUT BEGIN ===
> 1/4 Checking commit 3d5a04d99492 (softmmu: move softmmu only files from root)
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #112: 
> rename from arch_init.c
> 
> total: 0 errors, 1 warnings, 77 lines checked


I am not sure who the maintainer of arch_init would be.

get_maintainer.pl: No maintainers found, printing recent contributors.


> 
> Patch 1/4 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 2/4 Checking commit cb5c834770dd (cpu-throttle: new module, extracted from cpus.c)
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #110: 
> new file mode 100644
> 
> total: 0 errors, 1 warnings, 386 lines checked

I assumed this would be Paolo, since he maintains related components (and the cpus.c it came from)


> 
> Patch 2/4 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 3/4 Checking commit 1b135cfa40c8 (cpu-timers, icount: new modules)
> WARNING: line over 80 characters
> #94: FILE: accel/tcg/cpu-exec.c:107:
> +            qemu_printf("Warning: The guest is now late by %.1f to %.1f seconds\n",



Here I would ignore the warning for the benefit of readability of the printf string.


> 
> ERROR: line over 90 characters
> #265: FILE: hw/core/ptimer.c:137:
> +    if (s->enabled == 1 && (delta * period < 10000) && !icount_enabled() && !qtest_enabled()) {


this I should fix


> 
> ERROR: line over 90 characters
> #274: FILE: hw/core/ptimer.c:220:
> +            if (!oneshot && (s->delta * period < 10000) && !icount_enabled() && !qtest_enabled()) {
> 

this I should fix

> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #384: 
> new file mode 100644
> 
> total: 2 errors, 2 warnings, 2331 lines checked
> 
> Patch 3/4 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 4/4 Checking commit 0f5db0e76127 (cpus: extract out accel-specific code to each accel)
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #86: 
> new file mode 100644
> 
> total: 0 errors, 1 warnings, 2463 lines checked
> 
> Patch 4/4 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> === OUTPUT END ===


I am adding the new accelerator interfaces to the respective accelerator maintainers.


> 
> Test command exited with code: 1
> 
> 
> The full log is available at
> http://patchew.org/logs/20200525145440.29728-1-cfontana@suse.de/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
>