On Wed, Mar 2, 2022 at 21:13 Liav Albani <liav...@gmail.com> wrote: > > On 3/2/22 14:42, Michael S. Tsirkin wrote: > > On Wed, Mar 02, 2022 at 10:44:03AM +0530, Ani Sinha wrote: > >> On Wed, Mar 2, 2022 at 12:50 AM Liav Albani <liav...@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 <liav...@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 <liav...@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. > > > So if I understand how to do this now - I should drop the code for the > MicroVM ACPI for now, letting only to change the Q35 FACP table, right?
Correct. Microvm changes can go in a separate patch. > So if that's the case I should send it in a separate patch? Yes. > > If that's the case, as suggested by you and Ani, I'll not add a separate > function to reduce code duplication as there's no such thing in such > case... Please add the function regardless. > >
