| Message ID | 20230206150416.4604-12-farosas@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qtests vs. default devices | expand |
On 06/02/2023 16.04, Fabiano Rosas wrote: > If we build with --without-default-devices, CONFIG_HPET and > CONFIG_PARALLEL are set to N, which makes the respective devices go > missing from acpi tables. > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > I currently don't see a way of allowing the tests to pass in the > absence of these two configs. As far as I understand, we would need to > have one set of expected table files (tests/data/acpi) for each > combination of machine vs. possible CONFIG that can be toggled. I think you're right ... maintaining tables for each combination does not scale. Disabling the test in that case is likely the best we can do here right now. > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build > index a930706a43..2829eda2c9 100644 > --- a/tests/qtest/meson.build > +++ b/tests/qtest/meson.build > @@ -78,7 +78,9 @@ qtests_i386 = \ > config_all_devices.has_key('CONFIG_Q35') and \ > config_all_devices.has_key('CONFIG_VIRTIO_PCI') and \ > slirp.found() ? ['virtio-net-failover'] : []) + \ > - (unpack_edk2_blobs ? ['bios-tables-test'] : []) + \ > + (unpack_edk2_blobs and \ > + config_all_devices.has_key('CONFIG_HPET') and \ > + config_all_devices.has_key('CONFIG_PARALLEL') ? ['bios-tables-test'] : []) + \ > qtests_pci + \ > qtests_cxl + \ > ['fdc-test', Reviewed-by: Thomas Huth <thuth@redhat.com>
On Tue, Feb 07, 2023 at 03:35:56PM +0100, Thomas Huth wrote: > On 06/02/2023 16.04, Fabiano Rosas wrote: > > If we build with --without-default-devices, CONFIG_HPET and > > CONFIG_PARALLEL are set to N, which makes the respective devices go > > missing from acpi tables. > > > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > > --- > > I currently don't see a way of allowing the tests to pass in the > > absence of these two configs. As far as I understand, we would need to > > have one set of expected table files (tests/data/acpi) for each > > combination of machine vs. possible CONFIG that can be toggled. > > I think you're right ... maintaining tables for each combination does not > scale. Disabling the test in that case is likely the best we can do here > right now. > > > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build > > index a930706a43..2829eda2c9 100644 > > --- a/tests/qtest/meson.build > > +++ b/tests/qtest/meson.build > > @@ -78,7 +78,9 @@ qtests_i386 = \ > > config_all_devices.has_key('CONFIG_Q35') and \ > > config_all_devices.has_key('CONFIG_VIRTIO_PCI') and \ > > slirp.found() ? ['virtio-net-failover'] : []) + \ > > - (unpack_edk2_blobs ? ['bios-tables-test'] : []) + \ > > + (unpack_edk2_blobs and \ > > + config_all_devices.has_key('CONFIG_HPET') and \ > > + config_all_devices.has_key('CONFIG_PARALLEL') ? ['bios-tables-test'] : []) + \ > > qtests_pci + \ > > qtests_cxl + \ > > ['fdc-test', > > Reviewed-by: Thomas Huth <thuth@redhat.com> One thing we could do is move this code to an SSDT by itself. Then there's two variants of e.g. HPET SSDT: with and without CONFIG_HPET. Needs ACPI work though. Igor what do you think? Worth it?
On Tue, 7 Feb 2023 09:42:45 -0500 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Tue, Feb 07, 2023 at 03:35:56PM +0100, Thomas Huth wrote: > > On 06/02/2023 16.04, Fabiano Rosas wrote: > > > If we build with --without-default-devices, CONFIG_HPET and > > > CONFIG_PARALLEL are set to N, which makes the respective devices go > > > missing from acpi tables. > > > > > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > > > --- > > > I currently don't see a way of allowing the tests to pass in the > > > absence of these two configs. As far as I understand, we would need to > > > have one set of expected table files (tests/data/acpi) for each > > > combination of machine vs. possible CONFIG that can be toggled. > > > > I think you're right ... maintaining tables for each combination does not > > scale. Disabling the test in that case is likely the best we can do here > > right now. > > > > > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build > > > index a930706a43..2829eda2c9 100644 > > > --- a/tests/qtest/meson.build > > > +++ b/tests/qtest/meson.build > > > @@ -78,7 +78,9 @@ qtests_i386 = \ > > > config_all_devices.has_key('CONFIG_Q35') and \ > > > config_all_devices.has_key('CONFIG_VIRTIO_PCI') and \ > > > slirp.found() ? ['virtio-net-failover'] : []) + \ > > > - (unpack_edk2_blobs ? ['bios-tables-test'] : []) + \ > > > + (unpack_edk2_blobs and \ > > > + config_all_devices.has_key('CONFIG_HPET') and \ > > > + config_all_devices.has_key('CONFIG_PARALLEL') ? ['bios-tables-test'] : []) + \ > > > qtests_pci + \ > > > qtests_cxl + \ > > > ['fdc-test', > > > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > > One thing we could do is move this code to an SSDT by itself. Then > there's two variants of e.g. HPET SSDT: with and without CONFIG_HPET. > Needs ACPI work though. Igor what do you think? Worth it? I'd go with just disabling test in this case. having dedicated ACPI tables for each permutation config changes might cause doesn't look to me as sustainable.
On Wed, Feb 08, 2023 at 11:52:38AM +0100, Igor Mammedov wrote: > On Tue, 7 Feb 2023 09:42:45 -0500 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Tue, Feb 07, 2023 at 03:35:56PM +0100, Thomas Huth wrote: > > > On 06/02/2023 16.04, Fabiano Rosas wrote: > > > > If we build with --without-default-devices, CONFIG_HPET and > > > > CONFIG_PARALLEL are set to N, which makes the respective devices go > > > > missing from acpi tables. > > > > > > > > Signed-off-by: Fabiano Rosas <farosas@suse.de> > > > > --- > > > > I currently don't see a way of allowing the tests to pass in the > > > > absence of these two configs. As far as I understand, we would need to > > > > have one set of expected table files (tests/data/acpi) for each > > > > combination of machine vs. possible CONFIG that can be toggled. > > > > > > I think you're right ... maintaining tables for each combination does not > > > scale. Disabling the test in that case is likely the best we can do here > > > right now. > > > > > > > diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build > > > > index a930706a43..2829eda2c9 100644 > > > > --- a/tests/qtest/meson.build > > > > +++ b/tests/qtest/meson.build > > > > @@ -78,7 +78,9 @@ qtests_i386 = \ > > > > config_all_devices.has_key('CONFIG_Q35') and \ > > > > config_all_devices.has_key('CONFIG_VIRTIO_PCI') and \ > > > > slirp.found() ? ['virtio-net-failover'] : []) + \ > > > > - (unpack_edk2_blobs ? ['bios-tables-test'] : []) + \ > > > > + (unpack_edk2_blobs and \ > > > > + config_all_devices.has_key('CONFIG_HPET') and \ > > > > + config_all_devices.has_key('CONFIG_PARALLEL') ? ['bios-tables-test'] : []) + \ > > > > qtests_pci + \ > > > > qtests_cxl + \ > > > > ['fdc-test', > > > > > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > > > > > One thing we could do is move this code to an SSDT by itself. Then > > there's two variants of e.g. HPET SSDT: with and without CONFIG_HPET. > > Needs ACPI work though. Igor what do you think? Worth it? > > I'd go with just disabling test in this case. ok for now. > having dedicated ACPI tables for each permutation config changes > might cause doesn't look to me as sustainable. > I feel I was unclear. What I am proposing is not that we add SSDT for each permutation. What I am saying is basically this: we have build_hpet_aml. call it from a separate SSDT. Now with HPET we check this expected SSDT. Without HPET we don't have this SSDT so nothing to check.
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index a930706a43..2829eda2c9 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -78,7 +78,9 @@ qtests_i386 = \ config_all_devices.has_key('CONFIG_Q35') and \ config_all_devices.has_key('CONFIG_VIRTIO_PCI') and \ slirp.found() ? ['virtio-net-failover'] : []) + \ - (unpack_edk2_blobs ? ['bios-tables-test'] : []) + \ + (unpack_edk2_blobs and \ + config_all_devices.has_key('CONFIG_HPET') and \ + config_all_devices.has_key('CONFIG_PARALLEL') ? ['bios-tables-test'] : []) + \ qtests_pci + \ qtests_cxl + \ ['fdc-test',
If we build with --without-default-devices, CONFIG_HPET and CONFIG_PARALLEL are set to N, which makes the respective devices go missing from acpi tables. Signed-off-by: Fabiano Rosas <farosas@suse.de> --- I currently don't see a way of allowing the tests to pass in the absence of these two configs. As far as I understand, we would need to have one set of expected table files (tests/data/acpi) for each combination of machine vs. possible CONFIG that can be toggled. Any ideas? --- tests/qtest/meson.build | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)