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

[Gerd Hoffmann]

On Tue, May 05, 2020 at 10:04:02AM -0400, 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?

In the noise.  0.28-0.29 seconds on my hardware to the "i8042: PNP: No
PS/2 controller found" message, no matter whenever acpi is on or off.
With "quiet" (acpi prints more and logging to the serial console is
slow).

At that point -no-acpi takes one second to figure the ps2 controller
really isn't there (as discussed before).

Another interesting difference is interrupt handling.

The -no-acpi version:

           CPU0       
  2:          0    XT-PIC      cascade
  4:        284   IO-APIC   4-edge      ttyS0
  8:          0   IO-APIC   8-edge      rtc0
 14:       5399   IO-APIC  14-edge      virtio1
 15:         58   IO-APIC  15-edge      virtio0
NMI:          0   Non-maskable interrupts
[ ... ]

The acpi version:

           CPU0       
  1:          0   IO-APIC   9-edge      ACPI:Ged
  2:        231   IO-APIC  23-fasteoi   virtio0
  3:       6291   IO-APIC  22-fasteoi   virtio1
  4:       1758   IO-APIC   4-edge      ttyS0
  5:          0   IO-APIC   8-edge      rtc0
NMI:          0   Non-maskable interrupts
[ ... ]

> - what should be the default?

IMO it makes sense to enable it by default.  You get working
power management.  You can boot stock cloud images (patched
seabios parses the dsdt to find virtio-mmio devices to boot
from virtio-mmio disks).

It's easier to leave behind legacy stuff:  The kernel trusts the
firmware and doesn't go into "trying harder to find ps2 kbd" mode.
Also what is this "cascade" thing in /proc/interrupts above? [1]

I expect dropping the rtc is easier with acpi too, the kernel probably
wouldn't try to find it then.  Right now seabios needs rtc cmos for
ram size probing, so I didn't test that yet.

On the other hand I don't really see any disadvantages.  The tables are
small ...

# find /sys/firmware/acpi/tables/ -type f | xargs ls -l
-r--------. 1 root root  70 May  6 06:48 /sys/firmware/acpi/tables/APIC
-r--------. 1 root root 472 May  6 06:48 /sys/firmware/acpi/tables/DSDT
-r--------. 1 root root 268 May  6 06:48 /sys/firmware/acpi/tables/FACP

... and simple (no methods) so you can hardly call that "bloat".

> Based on above I'd be inclined to say default should stay no acpi and
> users should enable acpi with an option.

I disagree, but I can live with off by default too.  We already have
acpi=OnOffAuto for X86MachineState, so it is just a matter of handling
microvm.acpi=auto accordingly in x86_machine_is_acpi_enabled().

take care,
  Gerd

[1] Rhetorical question, I know what it is. [2]
[2] I don't want remember though.