mbox series

[v5,00/14] QEMU cpus.c refactoring part2

Message ID 20200812183250.9221-1-cfontana@suse.de (mailing list archive)
Headers show
Series QEMU cpus.c refactoring part2 | expand

Message

Claudio Fontana Aug. 12, 2020, 6:32 p.m. UTC
Motivation and higher level steps:

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

RFC v4 -> PATCH v5:

* in patch 2, move comment about cpus_get_elapsed_ticks from patch 3
  (Philippe)

* in patch 11-14, do not create separate xxx-int.h files,
  instead use the xxx-cpus.h files (Philippe)

RFC v3 -> v4:

* added patch 9: cleanup unneeded includes

* added patch 10: add handle_interrupt to the interface (Roman)

* added patch 11-14: remove accelerator specific internal functions
  from global includes (Roman)

* in patch 2, removed leftover "if hvf_enabled" hunk

* in patch 2, convert if (!tcg_enabled) with more punctual if (hax_enabled)
  when eating dummy APC

----

RFC v2 -> v3:

* provided defaults for all methods.
  Only create_vcpu_thread is now a mandatory field. (Paolo)

* separated new CpusAccel patch from its first user, new patch nr. 2:
  "cpus: prepare new CpusAccel cpu accelerator interface"

* new CpusAccel methods: get_virtual_clock and get_elapsed_ticks.
  (Paolo)

  In this series, get_virtual_clock has a separate implementation
  between TCG/icount and qtest,
  while get_elapsed_ticks only returns a virtual counter for icount.

  Looking for more comments in this area.

----

RFC v1 -> v2:

* split the cpus.c accelerator refactoring into 6 patches.

* other minor changes to be able to proceed step by step.

----

* Rebased on commit 255ae6e2158c743717bed76c9a2365ee4bcd326e,
"replay: notify the main loop when there are no instructions"

[SPLIT into part1 and part2]

----

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 (14):
  cpu-timers, icount: new modules
  cpus: prepare new CpusAccel cpu accelerator interface
  cpus: extract out TCG-specific code to accel/tcg
  cpus: extract out qtest-specific code to accel/qtest
  cpus: extract out kvm-specific code to accel/kvm
  cpus: extract out hax-specific code to target/i386/
  cpus: extract out whpx-specific code to target/i386/
  cpus: extract out hvf-specific code to target/i386/hvf/
  cpus: cleanup now unneeded includes
  cpus: add handle_interrupt to the CpusAccel interface
  hvf: remove hvf specific functions from global includes
  whpx: remove whpx specific functions from global includes
  hax: remove hax specific functions from global includes
  kvm: remove kvm specific functions from global includes

 MAINTAINERS                    |    5 +-
 accel/Makefile.objs            |    2 +-
 accel/kvm/Makefile.objs        |    2 +
 accel/kvm/kvm-all.c            |   14 +-
 accel/kvm/kvm-cpus.c           |   88 +++
 accel/kvm/kvm-cpus.h           |   24 +
 accel/qtest/Makefile.objs      |    2 +
 accel/qtest/qtest-cpus.c       |   91 +++
 accel/qtest/qtest-cpus.h       |   17 +
 accel/{ => qtest}/qtest.c      |   13 +-
 accel/stubs/Makefile.objs      |    2 -
 accel/stubs/hax-stub.c         |   10 -
 accel/stubs/hvf-stub.c         |   30 -
 accel/stubs/kvm-stub.c         |   23 -
 accel/stubs/whpx-stub.c        |   47 --
 accel/tcg/Makefile.objs        |    1 +
 accel/tcg/cpu-exec.c           |   43 +-
 accel/tcg/tcg-all.c            |   43 +-
 accel/tcg/tcg-cpus.c           |  569 ++++++++++++++
 accel/tcg/tcg-cpus.h           |   17 +
 accel/tcg/translate-all.c      |    3 +-
 dma-helpers.c                  |    4 +-
 docs/replay.txt                |    6 +-
 exec.c                         |    4 -
 hw/core/cpu.c                  |   14 +-
 hw/core/ptimer.c               |    8 +-
 hw/i386/x86.c                  |    3 +-
 include/exec/cpu-all.h         |    4 +
 include/exec/exec-all.h        |    4 +-
 include/hw/core/cpu.h          |   14 -
 include/qemu/timer.h           |   24 +-
 include/sysemu/cpu-timers.h    |   84 ++
 include/sysemu/cpus.h          |   50 +-
 include/sysemu/hax.h           |   17 -
 include/sysemu/hvf.h           |    8 -
 include/sysemu/hw_accel.h      |   69 +-
 include/sysemu/kvm.h           |    7 -
 include/sysemu/qtest.h         |    2 +
 include/sysemu/replay.h        |    4 +-
 include/sysemu/whpx.h          |   19 -
 replay/replay.c                |    6 +-
 softmmu/Makefile.objs          |    2 +
 softmmu/cpu-timers.c           |  279 +++++++
 softmmu/cpus.c                 | 1684 +++-------------------------------------
 softmmu/icount.c               |  497 ++++++++++++
 softmmu/qtest.c                |   34 +-
 softmmu/timers-state.h         |   69 ++
 softmmu/vl.c                   |   11 +-
 stubs/Makefile.objs            |    6 +-
 stubs/clock-warp.c             |    7 -
 stubs/cpu-get-clock.c          |    3 +-
 stubs/cpu-get-icount.c         |   21 -
 stubs/cpu-synchronize-state.c  |   15 +
 stubs/cpus-get-virtual-clock.c |    8 +
 stubs/icount.c                 |   52 ++
 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          |   17 +-
 target/i386/hax-cpus.c         |   84 ++
 target/i386/hax-cpus.h         |   33 +
 target/i386/hax-i386.h         |    2 +
 target/i386/hax-mem.c          |    2 +-
 target/i386/hax-posix.c        |   13 +-
 target/i386/hax-windows.c      |   22 +-
 target/i386/hax-windows.h      |    2 +
 target/i386/hvf/Makefile.objs  |    2 +-
 target/i386/hvf/hvf-cpus.c     |  131 ++++
 target/i386/hvf/hvf-cpus.h     |   25 +
 target/i386/hvf/hvf.c          |   12 +-
 target/i386/hvf/x86hvf.c       |    2 +
 target/i386/hvf/x86hvf.h       |    1 -
 target/i386/whpx-all.c         |   13 +-
 target/i386/whpx-cpus.c        |   96 +++
 target/i386/whpx-cpus.h        |   34 +
 target/riscv/csr.c             |    8 +-
 tests/ptimer-test-stubs.c      |    7 +-
 tests/test-timed-average.c     |    2 +-
 util/main-loop.c               |   12 +-
 util/qemu-timer.c              |   14 +-
 82 files changed, 2623 insertions(+), 2026 deletions(-)
 create mode 100644 accel/kvm/kvm-cpus.c
 create mode 100644 accel/kvm/kvm-cpus.h
 create mode 100644 accel/qtest/Makefile.objs
 create mode 100644 accel/qtest/qtest-cpus.c
 create mode 100644 accel/qtest/qtest-cpus.h
 rename accel/{ => qtest}/qtest.c (81%)
 delete mode 100644 accel/stubs/hvf-stub.c
 delete mode 100644 accel/stubs/whpx-stub.c
 create mode 100644 accel/tcg/tcg-cpus.c
 create mode 100644 accel/tcg/tcg-cpus.h
 create mode 100644 include/sysemu/cpu-timers.h
 create mode 100644 softmmu/cpu-timers.c
 create mode 100644 softmmu/icount.c
 create mode 100644 softmmu/timers-state.h
 delete mode 100644 stubs/clock-warp.c
 delete mode 100644 stubs/cpu-get-icount.c
 create mode 100644 stubs/cpu-synchronize-state.c
 create mode 100644 stubs/cpus-get-virtual-clock.c
 create mode 100644 stubs/icount.c
 create mode 100644 stubs/qemu-timer-notify-cb.c
 create mode 100644 target/i386/hax-cpus.c
 create mode 100644 target/i386/hax-cpus.h
 create mode 100644 target/i386/hvf/hvf-cpus.c
 create mode 100644 target/i386/hvf/hvf-cpus.h
 create mode 100644 target/i386/whpx-cpus.c
 create mode 100644 target/i386/whpx-cpus.h

Comments

no-reply@patchew.org Aug. 13, 2020, 2:16 p.m. UTC | #1
Patchew URL: https://patchew.org/QEMU/20200812183250.9221-1-cfontana@suse.de/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  TEST    check-unit: tests/test-char
Unexpected error in object_property_try_add() at /tmp/qemu-test/src/qom/object.c:1181:
attempt to add duplicate property 'serial-id' to object (type 'container')
ERROR test-char - too few tests run (expected 38, got 9)
make: *** [check-unit] Error 1
make: *** Waiting for unfinished jobs....
  TEST    check-qtest-x86_64: tests/qtest/hd-geo-test
qemu-system-aarch64: -accel kvm: invalid accelerator kvm
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=e2cd2d5f1f1d4eb6bc6bcd79e3d16404', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-2np4x1wq/src/docker-src.2020-08-13-10.00.40.11908:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=e2cd2d5f1f1d4eb6bc6bcd79e3d16404
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-2np4x1wq/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    15m49.276s
user    0m8.608s


The full log is available at
http://patchew.org/logs/20200812183250.9221-1-cfontana@suse.de/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Claudio Fontana Aug. 13, 2020, 4:06 p.m. UTC | #2
Any current infra work that could cause this failure?

I do not have problems when testing this set of commands locally,
for me it's all green.

Thanks,

Claudio

On 8/13/20 4:16 PM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200812183250.9221-1-cfontana@suse.de/
> 
> 
> 
> Hi,
> 
> This series failed the docker-quick@centos7 build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> make docker-image-centos7 V=1 NETWORK=1
> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
>   TEST    check-unit: tests/test-char
> Unexpected error in object_property_try_add() at /tmp/qemu-test/src/qom/object.c:1181:
> attempt to add duplicate property 'serial-id' to object (type 'container')
> ERROR test-char - too few tests run (expected 38, got 9)
> make: *** [check-unit] Error 1
> make: *** Waiting for unfinished jobs....
>   TEST    check-qtest-x86_64: tests/qtest/hd-geo-test
> qemu-system-aarch64: -accel kvm: invalid accelerator kvm
> ---
>     raise CalledProcessError(retcode, cmd)
> subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=e2cd2d5f1f1d4eb6bc6bcd79e3d16404', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-2np4x1wq/src/docker-src.2020-08-13-10.00.40.11908:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
> filter=--filter=label=com.qemu.instance.uuid=e2cd2d5f1f1d4eb6bc6bcd79e3d16404
> make[1]: *** [docker-run] Error 1
> make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-2np4x1wq/src'
> make: *** [docker-run-test-quick@centos7] Error 2
> 
> real    15m49.276s
> user    0m8.608s
> 
> 
> The full log is available at
> http://patchew.org/logs/20200812183250.9221-1-cfontana@suse.de/testing.docker-quick@centos7/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
>
Philippe Mathieu-Daudé Aug. 14, 2020, 7:49 a.m. UTC | #3
On 8/13/20 6:06 PM, Claudio Fontana wrote:
> Any current infra work that could cause this failure?
> 
> I do not have problems when testing this set of commands locally,
> for me it's all green.

Marc-André tried to debug this, but is having hard time reproducing.

> 
> Thanks,
> 
> Claudio
> 
> On 8/13/20 4:16 PM, no-reply@patchew.org wrote:
>> Patchew URL: https://patchew.org/QEMU/20200812183250.9221-1-cfontana@suse.de/
>>
>>
>>
>> Hi,
>>
>> This series failed the docker-quick@centos7 build test. Please find the testing commands and
>> their output below. If you have Docker installed, you can probably reproduce it
>> locally.
>>
>> === TEST SCRIPT BEGIN ===
>> #!/bin/bash
>> make docker-image-centos7 V=1 NETWORK=1
>> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
>> === TEST SCRIPT END ===
>>
>>   TEST    check-unit: tests/test-char
>> Unexpected error in object_property_try_add() at /tmp/qemu-test/src/qom/object.c:1181:
>> attempt to add duplicate property 'serial-id' to object (type 'container')
>> ERROR test-char - too few tests run (expected 38, got 9)
>> make: *** [check-unit] Error 1
>> make: *** Waiting for unfinished jobs....
>>   TEST    check-qtest-x86_64: tests/qtest/hd-geo-test
>> qemu-system-aarch64: -accel kvm: invalid accelerator kvm
>> ---
>>     raise CalledProcessError(retcode, cmd)
>> subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=e2cd2d5f1f1d4eb6bc6bcd79e3d16404', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-2np4x1wq/src/docker-src.2020-08-13-10.00.40.11908:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
>> filter=--filter=label=com.qemu.instance.uuid=e2cd2d5f1f1d4eb6bc6bcd79e3d16404
>> make[1]: *** [docker-run] Error 1
>> make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-2np4x1wq/src'
>> make: *** [docker-run-test-quick@centos7] Error 2
>>
>> real    15m49.276s
>> user    0m8.608s
>>
>>
>> The full log is available at
>> http://patchew.org/logs/20200812183250.9221-1-cfontana@suse.de/testing.docker-quick@centos7/?type=message.
>> ---
>> Email generated automatically by Patchew [https://patchew.org/].
>> Please send your feedback to patchew-devel@redhat.com
>>
>