Re: [PATCH v4 2/3] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table

[Michael S. Tsirkin]

On Wed, Mar 02, 2022 at 10:44:03AM +0530, Ani Sinha wrote:
> On Wed, Mar 2, 2022 at 12:50 AM Liav Albani <liavalb@gmail.com> wrote:
> >
> >
> > On 3/1/22 11:52, Ani Sinha wrote:
> > >
> > > On Tue, 1 Mar 2022, Igor Mammedov wrote:
> > >
> > >> On Mon, 28 Feb 2022 22:17:32 +0200
> > >> Liav Albani <liavalb@gmail.com> wrote:
> > >>
> > >>> This can allow the guest OS to determine more easily if i8042 controller
> > >>> is present in the system or not, so it doesn't need to do probing of the
> > >>> controller, but just initialize it immediately, before enumerating the
> > >>> ACPI AML namespace.
> > >>>
> > >>> This change only applies to the x86/q35 machine type, as it uses FACP
> > >>> ACPI table with revision higher than 1, which should implement at least
> > >>> ACPI 2.0 features within the table, hence it can also set the IA-PC boot
> > >>> flags register according to the ACPI 2.0 specification.
> > >>>
> > >>> Signed-off-by: Liav Albani <liavalb@gmail.com>
> > >>> ---
> > >>>   hw/acpi/aml-build.c         | 11 ++++++++++-
> > >>>   hw/i386/acpi-build.c        |  9 +++++++++
> > >>>   hw/i386/acpi-microvm.c      |  9 +++++++++
> > >> commit message says it's q35 specific, so wy it touched microvm anc 
> > >> piix4?
> > > Igor is correct. Although I see that currently there are no 8042 devices
> > > for microvms, maybe we should be conservative and add the code to detect
> > > the device anyway. In that case, the change could affect microvms too when
> > > such devices get added in the future.
> > >
> > >
> > > echo -e "info qtree\r\nquit\r\n" | ./qemu-system-x86_64 -machine microvm
> > > -monitor stdio 2>/dev/null | grep 8042
> > >
> > > <empty>
> >
> > What about this?
> >
> > echo -e "info qtree\r\nquit\r\n" | qemu-system-x86_64 -machine microvm
> > -device i8042 -monitor stdio 2>/dev/null | grep 8042
> >
> > Or this?
> >
> > echo -e "info mtree\r\nquit\r\n" | qemu-system-x86_64 -machine microvm
> > -device i8042 -monitor stdio 2>/dev/null | grep 8042
> 
> On both occasions you are explicitly adding the device.

Yes of course. It seems a bit cleaner to have -device i8042 -monitor
stdio give us the correct ACPI table even if there's no pressing need
for this ATM, simply because it's not much more code, and because if we
don't we risk guests trying to work around incorrect ACPI tables.
Let us however do this in a patch by its own with proper
documentation and motivation.

-- 
MST