Re: [PATCH v2 00/13] microvm: add acpi support

[Sergio Lopez <slp@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3871 bytes --]

On Tue, May 05, 2020 at 04:16:00PM +0200, Philippe Mathieu-Daudé wrote:
> On 5/5/20 4:04 PM, Michael S. Tsirkin wrote:
> > On Tue, May 05, 2020 at 03:42:52PM +0200, Gerd Hoffmann wrote:
> > > 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.
> > 
> > Questions
> > 
> > - what's the tradeoff in startup time?
> > - what should be the default?
> > 
> > Based on above I'd be inclined to say default should stay no acpi and
> > users should enable acpi with an option.
> 
> As this machine was added to have the least minimum hardware required, I'd
> keep the default with no ACPI and have user requiring it to use an option.
> My 2 cents obviously.

I also share this opinion. And I would prefer it to be a machine type
option, defaulting to "off", but I guess that's a matter of taste.

Sergio.

> > 
> > > 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.
> > > 
> > > cheers,
> > >    Gerd
> > > 
> > > Gerd Hoffmann (13):
> > >    acpi: make build_madt() more generic.
> > >    acpi: create acpi-common.c and move madt code
> > >    acpi: madt: skip pci override on pci-less systems (microvm)
> > >    acpi: move acpi_build_facs to acpi-common.c
> > >    acpi: move acpi_init_common_fadt_data to acpi-common.c
> > >    acpi: move acpi_align_size to acpi-common.h
> > >    acpi: fadt: add hw-reduced sleep register support
> > >    acpi: generic event device for x86
> > >    microvm: add minimal acpi support
> > >    microvm: disable virtio-mmio cmdline hack
> > >    microvm: add acpi_dsdt_add_virtio() for x86
> > >    microvm: make virtio irq base runtime configurable
> > >    microvm/acpi: use GSI 16-23 for virtio
> > > 
> > >   hw/i386/acpi-common.h                  |  38 ++++
> > >   hw/i386/acpi-microvm.h                 |   6 +
> > >   include/hw/acpi/acpi-defs.h            |   2 +
> > >   include/hw/acpi/generic_event_device.h |  10 +
> > >   include/hw/i386/microvm.h              |  10 +-
> > >   hw/acpi/aml-build.c                    |   4 +-
> > >   hw/i386/acpi-build.c                   | 198 +-------------------
> > >   hw/i386/acpi-common.c                  | 206 ++++++++++++++++++++
> > >   hw/i386/acpi-microvm.c                 | 249 +++++++++++++++++++++++++
> > >   hw/i386/generic_event_device_x86.c     | 114 +++++++++++
> > >   hw/i386/microvm.c                      |  36 +++-
> > >   hw/i386/Kconfig                        |   1 +
> > >   hw/i386/Makefile.objs                  |   3 +
> > >   13 files changed, 676 insertions(+), 201 deletions(-)
> > >   create mode 100644 hw/i386/acpi-common.h
> > >   create mode 100644 hw/i386/acpi-microvm.h
> > >   create mode 100644 hw/i386/acpi-common.c
> > >   create mode 100644 hw/i386/acpi-microvm.c
> > >   create mode 100644 hw/i386/generic_event_device_x86.c
> > > 
> > > -- 
> > > 2.18.4
> > 
> > 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]