mbox series

[v5,00/20] microvm: add acpi support

Message ID 20200707125356.32450-1-kraxel@redhat.com
Headers show
Series microvm: add acpi support | expand

Message

Gerd Hoffmann July 7, 2020, 12:53 p.m. UTC
I know that not supporting ACPI in microvm is intentional.  If you still
don't want ACPI this is perfectly fine, you can use the usual -no-acpi
switch to toggle ACPI support.

These are the advantages you are going to loose then:

  (1) virtio-mmio device discovery without command line hacks (tweaking
      the command line is a problem when not using direct kernel boot).
  (2) Better IO-APIC support, we can use IRQ lines 16-23.
  (3) ACPI power button (aka powerdown request) works.
  (4) machine poweroff (aka S5 state) works.

Together with seabios patches for virtio-mmio support this allows to
boot standard fedora images (cloud, coreos, workstation live) with the
microvm machine type.

git branch for testing (including updated seabios):
	https://git.kraxel.org/cgit/qemu/log/?h=sirius/microvm

changes in v2:
  * some acpi cleanups are an separate patch series now.
  * switched to hw reduced acpi & generic event device.
  * misc fixes here and there.

changes in v3:
  * depeds on "[PATCH v6 00/16] acpi: i386 tweaks" series.
  * renamed qboot to qboot.bin
  * updated seabios to master branch snapshot.
    - this version boots fine with rtc=off
  * generic event device tweaks (Igor's comments).
  * make SMP work.
  * add RfC patches to turn off acpi by default for microvm.
  * misc fixes here and there.

changes in v4:
  * rebase to latest master
    - this also depends on the pending seabios update
  * drop some patches which got cherry-picked.
  * wire up cpu hotplug (also does coldplug cpu init).
  * add microvm acpi test case.
  * dropped RfC patches to turn off acpi by default for microvm.
  * misc fixes here and there.

changes in v5:
  * rebase to latest master (seabios dependency is merged meanwhile)
  * fix DSDT version (mst).
  * add comments referencing the ACPI spec for easier review (mst).

take care,
  Gerd

Gerd Hoffmann (20):
  microvm: name qboot binary qboot.rom
  seabios: add microvm config, update build rules
  seabios: add bios-microvm.bin binary
  acpi: ged: add control regs
  acpi: ged: add x86 device variant.
  acpi: move acpi_dsdt_add_power_button() to ged
  microvm: make virtio irq base runtime configurable
  microvm/acpi: add minimal acpi support
  microvm/acpi: add acpi_dsdt_add_virtio() for x86
  microvm/acpi: use GSI 16-23 for virtio
  microvm/acpi: use seabios with acpi=on
  microvm/acpi: disable virtio-mmio cmdline hack
  x86: constify x86_machine_is_*_enabled
  x86: move acpi_dev from pc/microvm
  x86: move cpu plug from pc to x86
  microvm: wire up hotplug
  tests/acpi: allow microvm test data updates.
  tests/acpi: allow override blkdev
  tests/acpi: add microvm test
  tests/acpi: update expected data files for microvm

 hw/i386/acpi-microvm.h                 |   8 +
 include/hw/acpi/generic_event_device.h |  12 ++
 include/hw/i386/microvm.h              |  10 +-
 include/hw/i386/pc.h                   |   1 -
 include/hw/i386/x86.h                  |  11 +-
 hw/acpi/generic_event_device.c         |  52 +++++
 hw/arm/virt-acpi-build.c               |   8 -
 hw/i386/acpi-build.c                   |   2 +-
 hw/i386/acpi-microvm.c                 | 248 +++++++++++++++++++++++
 hw/i386/generic_event_device_x86.c     |  36 ++++
 hw/i386/microvm.c                      | 105 +++++++++-
 hw/i386/pc.c                           | 260 ++-----------------------
 hw/i386/pc_piix.c                      |   2 +-
 hw/i386/pc_q35.c                       |   2 +-
 hw/i386/x86.c                          | 226 ++++++++++++++++++++-
 tests/qtest/bios-tables-test.c         |  21 +-
 hw/i386/Kconfig                        |   1 +
 hw/i386/Makefile.objs                  |   2 +
 pc-bios/bios-microvm.bin               | Bin 65536 -> 131072 bytes
 pc-bios/qboot.rom                      | Bin 0 -> 65536 bytes
 roms/Makefile                          |  11 +-
 roms/config.seabios-microvm            |  26 +++
 tests/data/acpi/microvm/APIC           | Bin 0 -> 70 bytes
 tests/data/acpi/microvm/DSDT           | Bin 0 -> 365 bytes
 tests/data/acpi/microvm/FACP           | Bin 0 -> 268 bytes
 25 files changed, 775 insertions(+), 269 deletions(-)
 create mode 100644 hw/i386/acpi-microvm.h
 create mode 100644 hw/i386/acpi-microvm.c
 create mode 100644 hw/i386/generic_event_device_x86.c
 create mode 100644 pc-bios/qboot.rom
 create mode 100644 roms/config.seabios-microvm
 create mode 100644 tests/data/acpi/microvm/APIC
 create mode 100644 tests/data/acpi/microvm/DSDT
 create mode 100644 tests/data/acpi/microvm/FACP

Comments

no-reply@patchew.org July 7, 2020, 1:34 p.m. UTC | #1
Patchew URL: https://patchew.org/QEMU/20200707125356.32450-1-kraxel@redhat.com/



Hi,

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

Type: series
Message-id: 20200707125356.32450-1-kraxel@redhat.com
Subject: [PATCH v5 00/20] microvm: add acpi support

=== 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
 * [new tag]         patchew/20200707132116.26207-1-f4bug@amsat.org -> patchew/20200707132116.26207-1-f4bug@amsat.org
Switched to a new branch 'test'
d5b6102 tests/acpi: update expected data files for microvm
4016e67 tests/acpi: add microvm test
7a0b05e tests/acpi: allow override blkdev
9855e16 tests/acpi: allow microvm test data updates.
e96b4cf microvm: wire up hotplug
c0f5744 x86: move cpu plug from pc to x86
d7dc51c x86: move acpi_dev from pc/microvm
c98bafc x86: constify x86_machine_is_*_enabled
9bf3547 microvm/acpi: disable virtio-mmio cmdline hack
c725465 microvm/acpi: use seabios with acpi=on
856eae6 microvm/acpi: use GSI 16-23 for virtio
4b04f91 microvm/acpi: add acpi_dsdt_add_virtio() for x86
1d5e0bf microvm/acpi: add minimal acpi support
6ec826e microvm: make virtio irq base runtime configurable
c3ca775 acpi: move acpi_dsdt_add_power_button() to ged
7d7d83c acpi: ged: add x86 device variant.
18cd04d acpi: ged: add control regs
c64763a seabios: add bios-microvm.bin binary
91801f8 seabios: add microvm config, update build rules
31f51e5 microvm: name qboot binary qboot.rom

=== OUTPUT BEGIN ===
1/20 Checking commit 31f51e53edea (microvm: name qboot binary qboot.rom)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
rename from pc-bios/bios-microvm.bin

total: 0 errors, 1 warnings, 35 lines checked

Patch 1/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/20 Checking commit 91801f804550 (seabios: add microvm config, update build rules)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100644

total: 0 errors, 1 warnings, 39 lines checked

Patch 2/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/20 Checking commit c64763a56c6a (seabios: add bios-microvm.bin binary)
4/20 Checking commit 18cd04d78b56 (acpi: ged: add control regs)
5/20 Checking commit 7d7d83cd1c2c (acpi: ged: add x86 device variant.)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#25: 
new file mode 100644

total: 0 errors, 1 warnings, 51 lines checked

Patch 5/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/20 Checking commit c3ca775f6e35 (acpi: move acpi_dsdt_add_power_button() to ged)
7/20 Checking commit 6ec826ec31e9 (microvm: make virtio irq base runtime configurable)
8/20 Checking commit 1d5e0bff7fc1 (microvm/acpi: add minimal acpi support)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#34: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#39: FILE: hw/i386/acpi-microvm.c:1:
+/* Support for generating ACPI tables and passing them to Guests

WARNING: Block comments use a leading /* on a separate line
#163: FILE: hw/i386/acpi-microvm.c:125:
+    table_offsets = g_array_new(false, true /* clear */,

WARNING: Block comments use a leading /* on a separate line
#167: FILE: hw/i386/acpi-microvm.c:129:
+                             64 /* Ensure FACS is aligned */,

WARNING: Block comments use a leading /* on a separate line
#168: FILE: hw/i386/acpi-microvm.c:130:
+                             false /* high memory */);

total: 0 errors, 5 warnings, 311 lines checked

Patch 8/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/20 Checking commit 4b04f911df7a (microvm/acpi: add acpi_dsdt_add_virtio() for x86)
10/20 Checking commit 856eae6a8af7 (microvm/acpi: use GSI 16-23 for virtio)
11/20 Checking commit c7254650d200 (microvm/acpi: use seabios with acpi=on)
12/20 Checking commit 9bf3547b321c (microvm/acpi: disable virtio-mmio cmdline hack)
13/20 Checking commit c98bafce4745 (x86: constify x86_machine_is_*_enabled)
14/20 Checking commit d7dc51c13d40 (x86: move acpi_dev from pc/microvm)
15/20 Checking commit c0f57441ad6c (x86: move cpu plug from pc to x86)
ERROR: space required before the open parenthesis '('
#344: FILE: hw/i386/x86.c:187:
+    if(!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {

WARNING: Block comments use a leading /* on a separate line
#431: FILE: hw/i386/x86.c:274:
+    /* if 'address' properties socket-id/core-id/thread-id are not set, set them

WARNING: Block comments use a leading /* on a separate line
#434: FILE: hw/i386/x86.c:277:
+    /* TODO: move socket_id/core_id/thread_id checks into x86_cpu_realizefn()

WARNING: Block comments use a trailing */ on a separate line
#436: FILE: hw/i386/x86.c:279:
+     * CPUState::nr_cores and CPUState::nr_threads fields instead of globals */

WARNING: Block comments use a leading /* on a separate line
#484: FILE: hw/i386/x86.c:327:
+        /* If the number of CPUs can't be represented in 8 bits, the

WARNING: Block comments use a leading /* on a separate line
#502: FILE: hw/i386/x86.c:345:
+/* returns pointer to CPUArchId descriptor that matches CPU's apic_id

total: 1 errors, 5 warnings, 533 lines checked

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

16/20 Checking commit e96b4cfdddca (microvm: wire up hotplug)
17/20 Checking commit 9855e16bac52 (tests/acpi: allow microvm test data updates.)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#13: 
new file mode 100644

total: 0 errors, 1 warnings, 4 lines checked

Patch 17/20 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
18/20 Checking commit 7a0b05e95471 (tests/acpi: allow override blkdev)
19/20 Checking commit 4016e6747755 (tests/acpi: add microvm test)
20/20 Checking commit d5b610245ac6 (tests/acpi: update expected data files for microvm)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200707125356.32450-1-kraxel@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
no-reply@patchew.org July 7, 2020, 1:35 p.m. UTC | #2
Patchew URL: https://patchew.org/QEMU/20200707125356.32450-1-kraxel@redhat.com/



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 ===

acpi-test: Warning! DSDT binary file mismatch. Actual [aml:/tmp/aml-YB3SN0], Expected [aml:tests/data/acpi/microvm/DSDT].
See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files.
to see ASL diff between mismatched files install IASL, rebuild QEMU from scratch and re-run tests with V=1 environment variable set**
ERROR:/tmp/qemu-test/src/tests/qtest/bios-tables-test.c:495:test_acpi_asl: assertion failed: (all_tables_match)
ERROR - Bail out! ERROR:/tmp/qemu-test/src/tests/qtest/bios-tables-test.c:495:test_acpi_asl: assertion failed: (all_tables_match)
make: *** [check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs....
  TEST    iotest-qcow2: 030
  TEST    iotest-qcow2: 031
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=8f5f41e386b845a9a0f5edc2ade231c7', '-u', '1003', '--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/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-dd7_unsg/src/docker-src.2020-07-07-09.19.21.10455:/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=8f5f41e386b845a9a0f5edc2ade231c7
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-dd7_unsg/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    15m42.335s
user    0m8.981s


The full log is available at
http://patchew.org/logs/20200707125356.32450-1-kraxel@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com