On Wed, May 25, 2022 at 06:32:26PM +0100, Mark Cave-Ayland wrote: > I was working away at some improvements for PS2 devices when I noticed that > one > small change to the instantiation of a PS2 mouse device caused a regression in > tests/qtest/bios-tables-test, specifically the /x86_64/acpi/q35/viot subtest. > > Closer examination of the failed test output showed the problem was that the > order of the PCI host bridge entries had changed within the table causing the > generated binary to fail to match the version in > tests/data/acpi/q35/VIOT.viot. > > The error occurs because there is no guarantee in the order of PCI host > bridges > being returned from object_child_foreach_recursive() used within > hw/acpi/viot.c's build_viot() function, so any change to the QOM tree can > potentially change the generated ACPI VIOT table ordering and cause the > regression tests to fail. > > Fortunately the solution is fairly easy: change build_viot() to build an array > of PCI host bridges and then sort them first before generating the final ACPI > VIOT table. I've chosen to sort the PCI host bridges based upon the min_bus > number which seems to work okay here. > > The changes in this patchset were heavily inspired by the build_iort() > function > in hw/arm/virt-acpi-build.c which already does the right thing here. Patches > 1-5 > make the required changes before patch 6 updates the VIOT binary to match the > updated ACPI VIOT table ordering.
Thanks for the fix, looks good to me and I did some light testing Reviewed-by: Jean-Philippe Brucker <jean-phili...@linaro.org> > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> > > v3: > - Rebase onto master > - Add Reviewed-by tag from Ani in patch 1 > - Declare struct viot_pci_host_range as const in enumerate_pci_host_bridges() > in patch 3 > - Add Reviewed-by tags for the series from Phil > > v2: > - Rebase onto master > - Rename pci_host_bridges() to enumerate_pci_host_bridges() in patch 1 > - Change return type of pci_host_range_compare() from int to gint in patch 5 > - Tweak subject line in patch 5: s/PCI host bus/PCI host bridge/ > - Add Acked-by and Reviewed-by tags from Ani > > > Mark Cave-Ayland (6): > hw/acpi/viot: rename build_pci_range_node() to > enumerate_pci_host_bridges() > hw/acpi/viot: move the individual PCI host bridge entry generation to > a new function > hw/acpi/viot: build array of PCI host bridges before generating VIOT > ACPI table > tests/acpi: virt: allow VIOT acpi table changes > hw/acpi/viot: sort VIOT ACPI table entries by PCI host bridge min_bus > tests/acpi: virt: update golden masters for VIOT > > hw/acpi/viot.c | 107 +++++++++++++++++++++------------- > tests/data/acpi/q35/VIOT.viot | Bin 112 -> 112 bytes > 2 files changed, 68 insertions(+), 39 deletions(-) > > -- > 2.20.1 >
