| Message ID | 20220701133515.137890-3-imammedo@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | acpi:pc/q35: minor PCI refactoring/cleanups | expand |
On Fri, Jul 01, 2022 at 09:35:00AM -0400, Igor Mammedov wrote: > HPET AML doesn't depend on piix4 nor q35, move code buiding it > to common scope to avoid duplication. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> Apropos, tests/data/acpi/rebuild-expected-aml.sh ignores the fact that some tables might be identical. Also, there's no way to reuse expected files between machines. And so we have: [qemu]$ find tests/data/acpi -type f -exec sha256sum '{}' ';'|sort 01fefc161dcd26564d86826df5c000b080a3442912165a1432dddd3fe9d1b78a tests/data/acpi/virt/APIC 01fefc161dcd26564d86826df5c000b080a3442912165a1432dddd3fe9d1b78a tests/data/acpi/virt/APIC.memhp 01fefc161dcd26564d86826df5c000b080a3442912165a1432dddd3fe9d1b78a tests/data/acpi/virt/APIC.numamem 034be54fda2de6c07e7568b0c2ed6e0a0936c235f1a4ae42dbad0771b8b451db tests/data/acpi/pc/NFIT.dimmpxm 034be54fda2de6c07e7568b0c2ed6e0a0936c235f1a4ae42dbad0771b8b451db tests/data/acpi/q35/NFIT.dimmpxm 0ecccad48c22fc54f86756ef5e9dbf5360544bcd50970e277a13422c5831b67c tests/data/acpi/q35/DSDT.memhp 10341f3d4e6822e578f0759e9c2cdf3ed24267e5a427cb2a31b63a028e208147 tests/data/acpi/virt/FACP 10341f3d4e6822e578f0759e9c2cdf3ed24267e5a427cb2a31b63a028e208147 tests/data/acpi/virt/FACP.memhp 10341f3d4e6822e578f0759e9c2cdf3ed24267e5a427cb2a31b63a028e208147 tests/data/acpi/virt/FACP.numamem 1a93fe13736fb8d51277ab086f7d7a3bbc6bcf946c68229c785ba8257ec8d865 tests/data/acpi/pc/SRAT.memhp 1a93fe13736fb8d51277ab086f7d7a3bbc6bcf946c68229c785ba8257ec8d865 tests/data/acpi/q35/SRAT.memhp 3f0fe229cae8032140af80d8e84105e1b3cc2307b5daa60548d0a00c3b5c0649 tests/data/acpi/pc/SLIT.cphp 3f0fe229cae8032140af80d8e84105e1b3cc2307b5daa60548d0a00c3b5c0649 tests/data/acpi/pc/SLIT.memhp 3f0fe229cae8032140af80d8e84105e1b3cc2307b5daa60548d0a00c3b5c0649 tests/data/acpi/q35/SLIT.cphp 3f0fe229cae8032140af80d8e84105e1b3cc2307b5daa60548d0a00c3b5c0649 tests/data/acpi/q35/SLIT.memhp 3f0fe229cae8032140af80d8e84105e1b3cc2307b5daa60548d0a00c3b5c0649 tests/data/acpi/virt/SLIT.memhp 3fb3115e2c3c626603dbb20ab49825295d952f171ed7901a7f464cead2f40800 tests/data/acpi/pc/FACS 3fb3115e2c3c626603dbb20ab49825295d952f171ed7901a7f464cead2f40800 tests/data/acpi/q35/FACS 43b225e2dc8f37c4a7d5af6452187097a4417b0aaa102a5a7000c4ba683946cb tests/data/acpi/virt/SPCR 43b225e2dc8f37c4a7d5af6452187097a4417b0aaa102a5a7000c4ba683946cb tests/data/acpi/virt/SPCR.memhp 43b225e2dc8f37c4a7d5af6452187097a4417b0aaa102a5a7000c4ba683946cb tests/data/acpi/virt/SPCR.numamem 48831ff36e0e0c1916a0a637854a44f861aee551fff75d486da31605caebd16f tests/data/acpi/rebuild-expected-aml.sh 498a52d92d615ba2c5c69d62296f4f21792543c8d52ccb9fade1c69cfd83fae8 tests/data/acpi/pc/APIC.cphp 498a52d92d615ba2c5c69d62296f4f21792543c8d52ccb9fade1c69cfd83fae8 tests/data/acpi/q35/APIC.cphp 49906fead65caf8e2b093dc193ec082772c7559a9a8fe2d20914bc44dc811716 tests/data/acpi/q35/DSDT.dimmpxm 5307211b50c65c8c05e4efb81b1d320e9208b5d3aac5a3d6bd37b9f47ef3bc4c tests/data/acpi/pc/DSDT.ipmikcs 532089625245db3aeb4bf3a88c13d054be1fb21a24599b9d1bc17832a416937a tests/data/acpi/pc/DSDT 5d3803c3e90296d0a4ea3b9501b0af926196f7288b1ff9160c5294141113e396 tests/data/acpi/q35/DSDT.numamem 5e4c0a107998d58be1ff10ffa08bff225f7f6efe3027095135d2e55bc15512fe tests/data/acpi/pc/DSDT.cphp 61402328e4de16de4b7409564225268ae522ad913d9ef92276611ef22d9013a0 tests/data/acpi/virt/SRAT.numamem 645efebd748df15bd2cf7f0186cc7aeaaf8b40ac4adea29d871ac323360fc82c tests/data/acpi/q35/FACP 6608617b0a5df135a12a3866e4e4239fbe77e3004ff4d6ea42f16879e69cf10f tests/data/acpi/pc/DSDT.dimmpxm 6bc5f8fb421eff6f00de89743a56a88c951f47180b5ad83ceaad69aaf9fad984 tests/data/acpi/q35/DSDT.ipmibt 73d031ef66864988d9be337684317128c2e45c75a2af4fc35f20d216d83a2efd tests/data/acpi/virt/DSDT 73d031ef66864988d9be337684317128c2e45c75a2af4fc35f20d216d83a2efd tests/data/acpi/virt/DSDT.numamem 79fd95d6904b66b0a99982c64a0d54ea75bfa0568e8f02e1dcd9cdca404a758f tests/data/acpi/pc/APIC 79fd95d6904b66b0a99982c64a0d54ea75bfa0568e8f02e1dcd9cdca404a758f tests/data/acpi/q35/APIC 836514ed3adca20b1085838a37688dc1427811b648a7cc4e2a0a0f2d0cc6c99f tests/data/acpi/pc/DSDT.numamem 84d4fd8e55b1785e8c22d1ea17a542266ca14af0c747115316cc7201df338e03 tests/data/acpi/q35/MCFG 879dd52ff293ddbc4d1f589482f882619b99816c4e362f19dab099d1a0e42da9 tests/data/acpi/pc/SRAT.numamem 879dd52ff293ddbc4d1f589482f882619b99816c4e362f19dab099d1a0e42da9 tests/data/acpi/q35/SRAT.numamem 87ec0a685c6bcb1e33af3e558431f038ecd33e4243dd68a4b14d4e97c3d5fb6d tests/data/acpi/q35/SSDT.dimmpxm 8b28d0ce160bf42a812871210141f5d509932682a959b579c0d4f8f43b03fb42 tests/data/acpi/virt/SRAT.memhp 90a603d2f099cc3e3c9bd9de2f8f1cdad5307a6d2f6d2e7951f5418c7f422908 tests/data/acpi/q35/DSDT.cphp 95d9002674201957e58be516daf29729c2ac8e926bfb7256c1bc56fb8dc6eb01 tests/data/acpi/virt/MCFG 95d9002674201957e58be516daf29729c2ac8e926bfb7256c1bc56fb8dc6eb01 tests/data/acpi/virt/MCFG.memhp 95d9002674201957e58be516daf29729c2ac8e926bfb7256c1bc56fb8dc6eb01 tests/data/acpi/virt/MCFG.numamem 9aa0f3568a9f1c25d4c12257831974af88236bcc4b21b90df78ebe95bbb97475 tests/data/acpi/pc/HPET 9aa0f3568a9f1c25d4c12257831974af88236bcc4b21b90df78ebe95bbb97475 tests/data/acpi/q35/HPET ac64d624eb666028565f4d508a80ae04ed7b48232c60c320de1beba4999ecb69 tests/data/acpi/virt/DSDT.memhp b65c1b10bc565814f0c0950abd40712fd098b88c88f37f9bb20355356dd2b04f tests/data/acpi/pc/DSDT.bridge bb1c9e9e87bf9c1f8c96eb1deb201abb8a177296f1daa4dd53a9450c94307b7f tests/data/acpi/pc/SRAT.cphp bb1c9e9e87bf9c1f8c96eb1deb201abb8a177296f1daa4dd53a9450c94307b7f tests/data/acpi/q35/SRAT.cphp c9864027de5fed8169f5a283f64f15b799f34d26f66e3f4131da5c5ce1ec6d5d tests/data/acpi/q35/DSDT ca48a8cf471fcdde73f0997700cb99a39e738257eb26b5509dfb81005a694b83 tests/data/acpi/pc/SSDT.dimmpxm ddfb6d6b50dd4645efac8358d86c7d2f310cbf0300823b68ef0d996e31f01c44 tests/data/acpi/virt/GTDT ddfb6d6b50dd4645efac8358d86c7d2f310cbf0300823b68ef0d996e31f01c44 tests/data/acpi/virt/GTDT.memhp ddfb6d6b50dd4645efac8358d86c7d2f310cbf0300823b68ef0d996e31f01c44 tests/data/acpi/virt/GTDT.numamem e641d8505ab15bfd7ec399431eb3cb709cf0e06e15c528cc7d5da389392c89da tests/data/acpi/q35/SRAT.mmio64 e7274a622cf4be3d991a8cedf4437c1976334a1332fac78aaeaa47a94d6e2697 tests/data/acpi/pc/DSDT.memhp e93a393836970c4f32fff86a1931b8e45ee4634e5c99168491f2c55823647fda tests/data/acpi/pc/APIC.dimmpxm e93a393836970c4f32fff86a1931b8e45ee4634e5c99168491f2c55823647fda tests/data/acpi/q35/APIC.dimmpxm ee7b4e121c41519501119b4b7bd52edcf1124fcf271b7dd3086f0038b182b947 tests/data/acpi/q35/DSDT.bridge f089eb8441987367f223fa5e5e9fec8c12d9557cf0c6d68d0e8372be4c39fa7c tests/data/acpi/pc/FACP fabfa7d0ed2367bf1499b1a1d0b7253cd08a298e2aa0be3ca8f0dae5150e71a9 tests/data/acpi/q35/DSDT.mmio64 fb7675409f2264b14e5859e9187e7d05811bcc0570444b18cded8ff49a3514c5 tests/data/acpi/pc/SRAT.dimmpxm fb7675409f2264b14e5859e9187e7d05811bcc0570444b18cded8ff49a3514c5 tests/data/acpi/q35/SRAT.dimmpxm It's easy to fix up duplications within virt. But I am not 100% sure how fix up duplication between q35 and pc. I think we should really fix qemu to allow pc-i440fx and pc-q35 as machine types. Right now: microvm microvm (i386) xenfv-4.2 Xen Fully-virtualized PC xenfv Xen Fully-virtualized PC (alias of xenfv-3.1) xenfv-3.1 Xen Fully-virtualized PC pc Standard PC (i440FX + PIIX, 1996) (alias of pc-i440fx-7.1) pc-i440fx-7.1 Standard PC (i440FX + PIIX, 1996) (default) pc-i440fx-7.0 Standard PC (i440FX + PIIX, 1996) pc-i440fx-6.2 Standard PC (i440FX + PIIX, 1996) pc-i440fx-6.1 Standard PC (i440FX + PIIX, 1996) pc-i440fx-6.0 Standard PC (i440FX + PIIX, 1996) pc-i440fx-5.2 Standard PC (i440FX + PIIX, 1996) pc-i440fx-5.1 Standard PC (i440FX + PIIX, 1996) pc-i440fx-5.0 Standard PC (i440FX + PIIX, 1996) pc-i440fx-4.2 Standard PC (i440FX + PIIX, 1996) pc-i440fx-4.1 Standard PC (i440FX + PIIX, 1996) pc-i440fx-4.0 Standard PC (i440FX + PIIX, 1996) pc-i440fx-3.1 Standard PC (i440FX + PIIX, 1996) pc-i440fx-3.0 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.9 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.8 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.7 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.6 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.5 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.4 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.3 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.2 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.12 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.11 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.10 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.1 Standard PC (i440FX + PIIX, 1996) pc-i440fx-2.0 Standard PC (i440FX + PIIX, 1996) pc-i440fx-1.7 Standard PC (i440FX + PIIX, 1996) (deprecated) pc-i440fx-1.6 Standard PC (i440FX + PIIX, 1996) (deprecated) pc-i440fx-1.5 Standard PC (i440FX + PIIX, 1996) (deprecated) pc-i440fx-1.4 Standard PC (i440FX + PIIX, 1996) (deprecated) q35 Standard PC (Q35 + ICH9, 2009) (alias of pc-q35-7.1) pc-q35-7.1 Standard PC (Q35 + ICH9, 2009) pc-q35-7.0 Standard PC (Q35 + ICH9, 2009) pc-q35-6.2 Standard PC (Q35 + ICH9, 2009) pc-q35-6.1 Standard PC (Q35 + ICH9, 2009) pc-q35-6.0 Standard PC (Q35 + ICH9, 2009) pc-q35-5.2 Standard PC (Q35 + ICH9, 2009) pc-q35-5.1 Standard PC (Q35 + ICH9, 2009) pc-q35-5.0 Standard PC (Q35 + ICH9, 2009) pc-q35-4.2 Standard PC (Q35 + ICH9, 2009) pc-q35-4.1 Standard PC (Q35 + ICH9, 2009) pc-q35-4.0.1 Standard PC (Q35 + ICH9, 2009) pc-q35-4.0 Standard PC (Q35 + ICH9, 2009) pc-q35-3.1 Standard PC (Q35 + ICH9, 2009) pc-q35-3.0 Standard PC (Q35 + ICH9, 2009) pc-q35-2.9 Standard PC (Q35 + ICH9, 2009) pc-q35-2.8 Standard PC (Q35 + ICH9, 2009) pc-q35-2.7 Standard PC (Q35 + ICH9, 2009) pc-q35-2.6 Standard PC (Q35 + ICH9, 2009) pc-q35-2.5 Standard PC (Q35 + ICH9, 2009) pc-q35-2.4 Standard PC (Q35 + ICH9, 2009) pc-q35-2.12 Standard PC (Q35 + ICH9, 2009) pc-q35-2.11 Standard PC (Q35 + ICH9, 2009) pc-q35-2.10 Standard PC (Q35 + ICH9, 2009) isapc ISA-only PC none empty machine x-remote Experimental remote machine xenpv Xen Para-virtualized PC which IMHO is a bit of a mess. Then we could maybe use the directory "pc" for files common to i440fx and q35. Maybe just teach the test to look under tests/data/acpi/x86 too? And I think we should teach tests/data/acpi/rebuild-expected-aml.sh to check for duplicates and at least warn the user.
On Fri, 1 Jul 2022 12:26:16 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Fri, Jul 01, 2022 at 09:35:00AM -0400, Igor Mammedov wrote: > > HPET AML doesn't depend on piix4 nor q35, move code buiding it > > to common scope to avoid duplication. > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > Apropos, tests/data/acpi/rebuild-expected-aml.sh ignores the > fact that some tables might be identical. Also, there's no > way to reuse expected files between machines. And so we have: > > > [qemu]$ find tests/data/acpi -type f -exec sha256sum '{}' ';'|sort [...] > > > It's easy to fix up duplications within virt. But I am not 100% sure how > fix up duplication between q35 and pc. [...] > Then we could maybe use the directory "pc" for files common to i440fx > and q35. Maybe just teach the test to look under tests/data/acpi/x86 > too? And I think we should teach tests/data/acpi/rebuild-expected-aml.sh > to check for duplicates and at least warn the user. Probably duplicates in 'virt' mostly due to combination of not knowing that there is a fallback lookup (which is hidden in the code) and simplistic way tests/data/acpi/rebuild-expected-aml.sh rebuilds tables. As you suggest, rebuild-expected-aml.sh can be improved to warn or even better drop duplicates if found. As for reusing tables between different machine types, alternatively we can add explicit remapping rules (possibly auto-generated) versus currently used implicit fallback approach.
On Thu, Jul 07, 2022 at 11:16:16AM +0200, Igor Mammedov wrote: > On Fri, 1 Jul 2022 12:26:16 -0400 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Fri, Jul 01, 2022 at 09:35:00AM -0400, Igor Mammedov wrote: > > > HPET AML doesn't depend on piix4 nor q35, move code buiding it > > > to common scope to avoid duplication. > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > Apropos, tests/data/acpi/rebuild-expected-aml.sh ignores the > > fact that some tables might be identical. Also, there's no > > way to reuse expected files between machines. And so we have: > > > > > > [qemu]$ find tests/data/acpi -type f -exec sha256sum '{}' ';'|sort > [...] > > > > > > > It's easy to fix up duplications within virt. But I am not 100% sure how > > fix up duplication between q35 and pc. > [...] > > > Then we could maybe use the directory "pc" for files common to i440fx > > and q35. Maybe just teach the test to look under tests/data/acpi/x86 > > too? And I think we should teach tests/data/acpi/rebuild-expected-aml.sh > > to check for duplicates and at least warn the user. > > Probably duplicates in 'virt' mostly due to combination of not knowing > that there is a fallback lookup (which is hidden in the code) > and simplistic way tests/data/acpi/rebuild-expected-aml.sh rebuilds tables. > > As you suggest, rebuild-expected-aml.sh can be improved to warn or even > better drop duplicates if found. Want to try? > As for reusing tables between different machine types, alternatively > we can add explicit remapping rules (possibly auto-generated) versus > currently used implicit fallback approach. My worry with this is that if a specific table needs to be split from the generic variant then user would have to hack the test code as opposed to just updating the tables, so the update can not be done automatically. Thoughts?
On Thu, 7 Jul 2022 06:08:35 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Jul 07, 2022 at 11:16:16AM +0200, Igor Mammedov wrote: > > On Fri, 1 Jul 2022 12:26:16 -0400 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Fri, Jul 01, 2022 at 09:35:00AM -0400, Igor Mammedov wrote: > > > > HPET AML doesn't depend on piix4 nor q35, move code buiding it > > > > to common scope to avoid duplication. > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > > > Apropos, tests/data/acpi/rebuild-expected-aml.sh ignores the > > > fact that some tables might be identical. Also, there's no > > > way to reuse expected files between machines. And so we have: > > > > > > > > > [qemu]$ find tests/data/acpi -type f -exec sha256sum '{}' ';'|sort > > [...] > > > > > > > > > > > It's easy to fix up duplications within virt. But I am not 100% sure how > > > fix up duplication between q35 and pc. > > [...] > > > > > Then we could maybe use the directory "pc" for files common to i440fx > > > and q35. Maybe just teach the test to look under tests/data/acpi/x86 > > > too? And I think we should teach tests/data/acpi/rebuild-expected-aml.sh > > > to check for duplicates and at least warn the user. > > > > Probably duplicates in 'virt' mostly due to combination of not knowing > > that there is a fallback lookup (which is hidden in the code) > > and simplistic way tests/data/acpi/rebuild-expected-aml.sh rebuilds tables. > > > > As you suggest, rebuild-expected-aml.sh can be improved to warn or even > > better drop duplicates if found. > > Want to try? I'll put it on my queue, after PCI refactorings > > > As for reusing tables between different machine types, alternatively > > we can add explicit remapping rules (possibly auto-generated) versus > > currently used implicit fallback approach. > > My worry with this is that if a specific table needs to be split from > the generic variant then user would have to hack the test code as opposed > to just updating the tables, so the update can not be done > automatically. Thoughts? I'll try to implement it and see if it's possible.
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index cad6f5ac41..69ead4b34a 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -1467,9 +1467,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(sb_scope, dev); aml_append(dsdt, sb_scope); - if (misc->has_hpet) { - build_hpet_aml(dsdt); - } build_piix4_isa_bridge(dsdt); if (pm->pcihp_bridge_en || pm->pcihp_root_en) { build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); @@ -1515,9 +1512,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(dsdt, sb_scope); - if (misc->has_hpet) { - build_hpet_aml(dsdt); - } build_q35_isa_bridge(dsdt); if (pm->pcihp_bridge_en) { build_x86_acpi_pci_hotplug(dsdt, pm->pcihp_io_base); @@ -1528,6 +1522,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, } } + if (misc->has_hpet) { + build_hpet_aml(dsdt); + } + if (vmbus_bridge) { sb_scope = aml_scope("_SB"); aml_append(sb_scope, build_vmbus_device_aml(vmbus_bridge));
HPET AML doesn't depend on piix4 nor q35, move code buiding it to common scope to avoid duplication. Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- hw/i386/acpi-build.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)