[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/3] QEMU cpus.c refactoring part1
From: |
Paolo Bonzini |
Subject: |
Re: [PATCH 0/3] QEMU cpus.c refactoring part1 |
Date: |
Fri, 3 Jul 2020 19:21:44 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 |
On 02/07/20 08:27, Claudio Fontana wrote:
> Hi Alex, Markus, Paolo,
>
> maybe this could be queued in one of your queues?
Nevermind, Thunderbird was acting funny. Queued all now.
Paolo
> Thanks a lot,
>
> Claudio
>
> On 6/29/20 11:35 AM, Claudio Fontana wrote:
>> Motivation and higher level steps:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg04628.html
>>
>> Previous series: [RFC RESEND v7 0/4] QEMU cpus.c refactoring
>>
>> This series is already reviewed, and is a split of the first three patches
>> from the previous series (RFC). The forth and last of the previous series
>> will then be posted separately.
>>
>> PREVIOUS DISCUSSIONS:
>>
>> * should we reorder patches or moves inside patches to avoid code going
>> from cpus.c to softmmu/cpus.c and then again to softmmu/somethingelse.c ?
>> (Philippe)
>>
>> * some questions about headers in include/softmmu (Philippe)
>>
>>
>> [SPLIT into TWO series, changed from RFC to PATCH]
>> ----
>>
>> v6 -> v7:
>>
>> * rebased changes on top of Pavel Dovgalyuk changes to dma-helpers.c
>> "icount: make dma reads deterministic"
>>
>> ----
>>
>> v5 -> v6:
>>
>> * rebased changes on top of Emilio G. Cota changes to cpus.c
>> "cpu: convert queued work to a QSIMPLEQ"
>>
>> * keep a pointer in cpus.c instead of a copy of CpusAccel
>> (Alex)
>>
>> ----
>>
>>
>> v4 -> v5: rebase on latest master
>>
>> * rebased changes on top of roman series to remove one of the extra states
>> for hvf.
>> (Is the result now functional for HVF?)
>>
>> * rebased changes on top of icount changes and fixes to icount_configure and
>> the new shift vmstate. (Markus)
>>
>> v3 -> v4:
>>
>> * overall: added copyright headers to all files that were missing them
>> (used copyright and license of the module the stuff was extracted from).
>> For the new interface files, added SUSE LLC.
>>
>> * 1/4 (move softmmu only files from root):
>>
>> MAINTAINERS: moved softmmu/cpus.c to its final location (from patch 2)
>>
>> * 2/4 (cpu-throttle):
>>
>> MAINTAINERS (to patch 1),
>> copyright Fabrice Bellard and license from cpus.c
>>
>> * 3/4 (cpu-timers, icount):
>>
>> - MAINTAINERS: add cpu-timers.c and icount.c to Paolo
>>
>> - break very long lines (patchew)
>>
>> - add copyright SUSE LLC, GPLv2 to cpu-timers.h
>>
>> - add copyright Fabrice Bellard and license from cpus.c to timers-state.h
>> as it is lifted from cpus.c
>>
>> - vl.c: in configure_accelerators bail out if icount_enabled()
>> and !tcg_enabled() as qtest does not enable icount anymore.
>>
>> * 4/4 (accel stuff to accel):
>>
>> - add copyright SUSE LLC to files that mostly only consist of the
>> new interface. Add whatever copyright was in the accelerator code
>> if instead they mostly consist of accelerator code.
>>
>> - change a comment to mention the result of the AccelClass experiment
>>
>> - moved qtest accelerator into accel/qtest/ , make it like the others.
>>
>> - rename xxx-cpus-interface to xxx-cpus (remove "interface" from names)
>>
>> - rename accel_int to cpus_accel
>>
>> - rename CpusAccel functions from cpu_synchronize_* to synchronize_*
>>
>>
>> --------
>>
>> 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
>>
>> Claudio Fontana (3):
>> softmmu: move softmmu only files from root
>> cpu-throttle: new module, extracted from cpus.c
>> cpu-timers, icount: new modules
>>
>> MAINTAINERS | 15 +-
>> Makefile.target | 7 +-
>> accel/qtest.c | 6 +-
>> accel/tcg/cpu-exec.c | 43 +-
>> accel/tcg/tcg-all.c | 7 +-
>> accel/tcg/translate-all.c | 3 +-
>> dma-helpers.c | 4 +-
>> docs/replay.txt | 6 +-
>> exec.c | 4 -
>> hw/core/ptimer.c | 8 +-
>> 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 | 68 +++
>> include/sysemu/cpu-timers.h | 81 +++
>> include/sysemu/cpus.h | 12 +-
>> 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 | 284 +++++++++
>> cpus.c => softmmu/cpus.c | 839
>> +--------------------------
>> softmmu/icount.c | 499 ++++++++++++++++
>> 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 | 69 +++
>> softmmu/vl.c | 8 +-
>> stubs/Makefile.objs | 3 +-
>> stubs/clock-warp.c | 4 +-
>> stubs/cpu-get-clock.c | 3 +-
>> stubs/cpu-get-icount.c | 21 -
>> 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/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 +-
>> 51 files changed, 1343 insertions(+), 985 deletions(-)
>> 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
>> rename cpus.c => softmmu/cpus.c (59%)
>> 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/icount.c
>> create mode 100644 stubs/qemu-timer-notify-cb.c
>>
>