| Message ID | 20250527074224.1197793-25-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Tue, 27 May 2025 09:40:26 +0200 Eric Auger <eric.auger@redhat.com> wrote: > From: Gustavo Romero <gustavo.romero@linaro.org> > > ACPI PCI hotplug is now turned on by default so we need to change the > existing tests to keep it off. However, even setting the ACPI PCI > hotplug off in the existing tests, there will be changes in the ACPI > tables because the _OSC method was modified, hence in the next patch of > this series the blobs are updated accordingly. > > Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> > Signed-off-by: Eric Auger <eric.auger@redhat.com> it would be better to test whatever default we end up with. (like x86) > > --- > > [Eric] also added acpi-pcihp=off to test_acpi_aarch64_virt_tcg_numamem > --- > tests/qtest/bios-tables-test.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c > index 0a333ec435..6379dba714 100644 > --- a/tests/qtest/bios-tables-test.c > +++ b/tests/qtest/bios-tables-test.c > @@ -1626,7 +1626,7 @@ static void test_acpi_aarch64_virt_tcg_memhp(void) > }; > > data.variant = ".memhp"; > - test_acpi_one(" -machine nvdimm=on" > + test_acpi_one(" -machine nvdimm=on,acpi-pcihp=off" > " -cpu cortex-a57" > " -m 256M,slots=3,maxmem=1G" > " -object memory-backend-ram,id=ram0,size=128M" > @@ -1747,7 +1747,8 @@ static void test_acpi_aarch64_virt_tcg_numamem(void) > }; > > data.variant = ".numamem"; > - test_acpi_one(" -cpu cortex-a57" > + test_acpi_one(" -machine acpi-pcihp=off" > + " -cpu cortex-a57" > " -object memory-backend-ram,id=ram0,size=128M" > " -numa node,memdev=ram0", > &data); > @@ -1775,7 +1776,8 @@ static void test_acpi_aarch64_virt_tcg_pxb(void) > * to solve the conflicts. > */ > data.variant = ".pxb"; > - test_acpi_one(" -device pcie-root-port,chassis=1,id=pci.1" > + test_acpi_one(" -machine acpi-pcihp=off" > + " -device pcie-root-port,chassis=1,id=pci.1" > " -device virtio-scsi-pci,id=scsi0,bus=pci.1" > " -drive file=" > "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2," > @@ -1846,7 +1848,7 @@ static void test_acpi_aarch64_virt_tcg_acpi_hmat(void) > > data.variant = ".acpihmatvirt"; > > - test_acpi_one(" -machine hmat=on" > + test_acpi_one(" -machine hmat=on,acpi-pcihp=off" > " -cpu cortex-a57" > " -smp 4,sockets=2" > " -m 384M" > @@ -2123,6 +2125,7 @@ static void test_acpi_aarch64_virt_tcg(void) > data.smbios_cpu_max_speed = 2900; > data.smbios_cpu_curr_speed = 2700; > test_acpi_one("-cpu cortex-a57 " > + "-machine acpi-pcihp=off " > "-smbios type=4,max-speed=2900,current-speed=2700", &data); > free_test_data(&data); > } > @@ -2142,6 +2145,7 @@ static void test_acpi_aarch64_virt_tcg_topology(void) > }; > > test_acpi_one("-cpu cortex-a57 " > + "-machine acpi-pcihp=off " > "-smp sockets=1,clusters=2,cores=2,threads=2", &data); > free_test_data(&data); > } > @@ -2227,6 +2231,7 @@ static void test_acpi_aarch64_virt_viot(void) > }; > > test_acpi_one("-cpu cortex-a57 " > + "-machine acpi-pcihp=off " > "-device virtio-iommu-pci", &data); > free_test_data(&data); > }
Hi Igor, On 5/28/25 11:38 AM, Igor Mammedov wrote: > On Tue, 27 May 2025 09:40:26 +0200 > Eric Auger <eric.auger@redhat.com> wrote: > >> From: Gustavo Romero <gustavo.romero@linaro.org> >> >> ACPI PCI hotplug is now turned on by default so we need to change the >> existing tests to keep it off. However, even setting the ACPI PCI >> hotplug off in the existing tests, there will be changes in the ACPI >> tables because the _OSC method was modified, hence in the next patch of >> this series the blobs are updated accordingly. >> >> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> > it would be better to test whatever default we end up with. > (like x86) See my question on patch 2's comment. We intended to have tests for both modes (legacy and acpi pcihp). Gustavo added some new tests for the new default, namely acpi pcihp. Now I did not really understand your point about keeping legacy mode as a default. Thanks Eric > >> --- >> >> [Eric] also added acpi-pcihp=off to test_acpi_aarch64_virt_tcg_numamem >> --- >> tests/qtest/bios-tables-test.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c >> index 0a333ec435..6379dba714 100644 >> --- a/tests/qtest/bios-tables-test.c >> +++ b/tests/qtest/bios-tables-test.c >> @@ -1626,7 +1626,7 @@ static void test_acpi_aarch64_virt_tcg_memhp(void) >> }; >> >> data.variant = ".memhp"; >> - test_acpi_one(" -machine nvdimm=on" >> + test_acpi_one(" -machine nvdimm=on,acpi-pcihp=off" >> " -cpu cortex-a57" >> " -m 256M,slots=3,maxmem=1G" >> " -object memory-backend-ram,id=ram0,size=128M" >> @@ -1747,7 +1747,8 @@ static void test_acpi_aarch64_virt_tcg_numamem(void) >> }; >> >> data.variant = ".numamem"; >> - test_acpi_one(" -cpu cortex-a57" >> + test_acpi_one(" -machine acpi-pcihp=off" >> + " -cpu cortex-a57" >> " -object memory-backend-ram,id=ram0,size=128M" >> " -numa node,memdev=ram0", >> &data); >> @@ -1775,7 +1776,8 @@ static void test_acpi_aarch64_virt_tcg_pxb(void) >> * to solve the conflicts. >> */ >> data.variant = ".pxb"; >> - test_acpi_one(" -device pcie-root-port,chassis=1,id=pci.1" >> + test_acpi_one(" -machine acpi-pcihp=off" >> + " -device pcie-root-port,chassis=1,id=pci.1" >> " -device virtio-scsi-pci,id=scsi0,bus=pci.1" >> " -drive file=" >> "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2," >> @@ -1846,7 +1848,7 @@ static void test_acpi_aarch64_virt_tcg_acpi_hmat(void) >> >> data.variant = ".acpihmatvirt"; >> >> - test_acpi_one(" -machine hmat=on" >> + test_acpi_one(" -machine hmat=on,acpi-pcihp=off" >> " -cpu cortex-a57" >> " -smp 4,sockets=2" >> " -m 384M" >> @@ -2123,6 +2125,7 @@ static void test_acpi_aarch64_virt_tcg(void) >> data.smbios_cpu_max_speed = 2900; >> data.smbios_cpu_curr_speed = 2700; >> test_acpi_one("-cpu cortex-a57 " >> + "-machine acpi-pcihp=off " >> "-smbios type=4,max-speed=2900,current-speed=2700", &data); >> free_test_data(&data); >> } >> @@ -2142,6 +2145,7 @@ static void test_acpi_aarch64_virt_tcg_topology(void) >> }; >> >> test_acpi_one("-cpu cortex-a57 " >> + "-machine acpi-pcihp=off " >> "-smp sockets=1,clusters=2,cores=2,threads=2", &data); >> free_test_data(&data); >> } >> @@ -2227,6 +2231,7 @@ static void test_acpi_aarch64_virt_viot(void) >> }; >> >> test_acpi_one("-cpu cortex-a57 " >> + "-machine acpi-pcihp=off " >> "-device virtio-iommu-pci", &data); >> free_test_data(&data); >> }
On Wed, 28 May 2025 11:48:20 +0200 Eric Auger <eric.auger@redhat.com> wrote: > Hi Igor, > > On 5/28/25 11:38 AM, Igor Mammedov wrote: > > On Tue, 27 May 2025 09:40:26 +0200 > > Eric Auger <eric.auger@redhat.com> wrote: > > > >> From: Gustavo Romero <gustavo.romero@linaro.org> > >> > >> ACPI PCI hotplug is now turned on by default so we need to change the > >> existing tests to keep it off. However, even setting the ACPI PCI > >> hotplug off in the existing tests, there will be changes in the ACPI > >> tables because the _OSC method was modified, hence in the next patch of > >> this series the blobs are updated accordingly. > >> > >> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> > >> Signed-off-by: Eric Auger <eric.auger@redhat.com> > > it would be better to test whatever default we end up with. > > (like x86) > > See my question on patch 2's comment. We intended to have tests for both > modes (legacy and acpi pcihp). Gustavo added some new tests for the new > default, namely acpi pcihp. Now I did not really understand your point > about keeping legacy mode as a default. default legacy wrt tests are orthogonal things. Just decide what default should be in #2 and then make sure existing tests work with that. Default (majority) tests shouldn't include CLI option a for a feature 'acpi-pcihp', only legacy tests should have it. Given bios-tables-test is heavy load on CI for x86 we have only few test cases to check no pcihp (legacy), the same should be done for ARM. Then on top add a few acpi hotplug tests to snapshot DSDT with hotplug hierarchy built in. ex: test_acpi_piix4_tcg_bridge or test_acpi_piix4_no_acpi_pci_hotplug. > > Thanks > > Eric > > > >> --- > >> > >> [Eric] also added acpi-pcihp=off to test_acpi_aarch64_virt_tcg_numamem > >> --- > >> tests/qtest/bios-tables-test.c | 13 +++++++++---- > >> 1 file changed, 9 insertions(+), 4 deletions(-) > >> > >> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c > >> index 0a333ec435..6379dba714 100644 > >> --- a/tests/qtest/bios-tables-test.c > >> +++ b/tests/qtest/bios-tables-test.c > >> @@ -1626,7 +1626,7 @@ static void test_acpi_aarch64_virt_tcg_memhp(void) > >> }; > >> > >> data.variant = ".memhp"; > >> - test_acpi_one(" -machine nvdimm=on" > >> + test_acpi_one(" -machine nvdimm=on,acpi-pcihp=off" > >> " -cpu cortex-a57" > >> " -m 256M,slots=3,maxmem=1G" > >> " -object memory-backend-ram,id=ram0,size=128M" > >> @@ -1747,7 +1747,8 @@ static void test_acpi_aarch64_virt_tcg_numamem(void) > >> }; > >> > >> data.variant = ".numamem"; > >> - test_acpi_one(" -cpu cortex-a57" > >> + test_acpi_one(" -machine acpi-pcihp=off" > >> + " -cpu cortex-a57" > >> " -object memory-backend-ram,id=ram0,size=128M" > >> " -numa node,memdev=ram0", > >> &data); > >> @@ -1775,7 +1776,8 @@ static void test_acpi_aarch64_virt_tcg_pxb(void) > >> * to solve the conflicts. > >> */ > >> data.variant = ".pxb"; > >> - test_acpi_one(" -device pcie-root-port,chassis=1,id=pci.1" > >> + test_acpi_one(" -machine acpi-pcihp=off" > >> + " -device pcie-root-port,chassis=1,id=pci.1" > >> " -device virtio-scsi-pci,id=scsi0,bus=pci.1" > >> " -drive file=" > >> "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2," > >> @@ -1846,7 +1848,7 @@ static void test_acpi_aarch64_virt_tcg_acpi_hmat(void) > >> > >> data.variant = ".acpihmatvirt"; > >> > >> - test_acpi_one(" -machine hmat=on" > >> + test_acpi_one(" -machine hmat=on,acpi-pcihp=off" > >> " -cpu cortex-a57" > >> " -smp 4,sockets=2" > >> " -m 384M" > >> @@ -2123,6 +2125,7 @@ static void test_acpi_aarch64_virt_tcg(void) > >> data.smbios_cpu_max_speed = 2900; > >> data.smbios_cpu_curr_speed = 2700; > >> test_acpi_one("-cpu cortex-a57 " > >> + "-machine acpi-pcihp=off " > >> "-smbios type=4,max-speed=2900,current-speed=2700", &data); > >> free_test_data(&data); > >> } > >> @@ -2142,6 +2145,7 @@ static void test_acpi_aarch64_virt_tcg_topology(void) > >> }; > >> > >> test_acpi_one("-cpu cortex-a57 " > >> + "-machine acpi-pcihp=off " > >> "-smp sockets=1,clusters=2,cores=2,threads=2", &data); > >> free_test_data(&data); > >> } > >> @@ -2227,6 +2231,7 @@ static void test_acpi_aarch64_virt_viot(void) > >> }; > >> > >> test_acpi_one("-cpu cortex-a57 " > >> + "-machine acpi-pcihp=off " > >> "-device virtio-iommu-pci", &data); > >> free_test_data(&data); > >> } >
Hi Igor, On 5/28/25 06:38, Igor Mammedov wrote: > On Tue, 27 May 2025 09:40:26 +0200 > Eric Auger <eric.auger@redhat.com> wrote: > >> From: Gustavo Romero <gustavo.romero@linaro.org> >> >> ACPI PCI hotplug is now turned on by default so we need to change the >> existing tests to keep it off. However, even setting the ACPI PCI >> hotplug off in the existing tests, there will be changes in the ACPI >> tables because the _OSC method was modified, hence in the next patch of >> this series the blobs are updated accordingly. >> >> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> >> Signed-off-by: Eric Auger <eric.auger@redhat.com> > > it would be better to test whatever default we end up with. > (like x86) hmm maybe there is a confusion here, Igor. We are actually planning what you said. This patch and the other two in this series related to the bios-tables-test (i.e., patches 8/25 and 10/25) are for actually making the current (legacy) test pass, since the new default as per this series will be acpi-pcihp=on. That's why here we're adapting the current test here to have acpi-pcihp=off. The new test that will test for acpi-pcihp=on (the new default) is not in this series and we decided to merge it separate. It's in the patch 4/5 and 5/5 of the follow series: https://mail.gnu.org/archive/html/qemu-devel/2025-05/msg05828.html 4/5 https://mail.gnu.org/archive/html/qemu-devel/2025-05/msg05827.html 5/5 Cheers, Gustavo >> >> --- >> >> [Eric] also added acpi-pcihp=off to test_acpi_aarch64_virt_tcg_numamem >> --- >> tests/qtest/bios-tables-test.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c >> index 0a333ec435..6379dba714 100644 >> --- a/tests/qtest/bios-tables-test.c >> +++ b/tests/qtest/bios-tables-test.c >> @@ -1626,7 +1626,7 @@ static void test_acpi_aarch64_virt_tcg_memhp(void) >> }; >> >> data.variant = ".memhp"; >> - test_acpi_one(" -machine nvdimm=on" >> + test_acpi_one(" -machine nvdimm=on,acpi-pcihp=off" >> " -cpu cortex-a57" >> " -m 256M,slots=3,maxmem=1G" >> " -object memory-backend-ram,id=ram0,size=128M" >> @@ -1747,7 +1747,8 @@ static void test_acpi_aarch64_virt_tcg_numamem(void) >> }; >> >> data.variant = ".numamem"; >> - test_acpi_one(" -cpu cortex-a57" >> + test_acpi_one(" -machine acpi-pcihp=off" >> + " -cpu cortex-a57" >> " -object memory-backend-ram,id=ram0,size=128M" >> " -numa node,memdev=ram0", >> &data); >> @@ -1775,7 +1776,8 @@ static void test_acpi_aarch64_virt_tcg_pxb(void) >> * to solve the conflicts. >> */ >> data.variant = ".pxb"; >> - test_acpi_one(" -device pcie-root-port,chassis=1,id=pci.1" >> + test_acpi_one(" -machine acpi-pcihp=off" >> + " -device pcie-root-port,chassis=1,id=pci.1" >> " -device virtio-scsi-pci,id=scsi0,bus=pci.1" >> " -drive file=" >> "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2," >> @@ -1846,7 +1848,7 @@ static void test_acpi_aarch64_virt_tcg_acpi_hmat(void) >> >> data.variant = ".acpihmatvirt"; >> >> - test_acpi_one(" -machine hmat=on" >> + test_acpi_one(" -machine hmat=on,acpi-pcihp=off" >> " -cpu cortex-a57" >> " -smp 4,sockets=2" >> " -m 384M" >> @@ -2123,6 +2125,7 @@ static void test_acpi_aarch64_virt_tcg(void) >> data.smbios_cpu_max_speed = 2900; >> data.smbios_cpu_curr_speed = 2700; >> test_acpi_one("-cpu cortex-a57 " >> + "-machine acpi-pcihp=off " >> "-smbios type=4,max-speed=2900,current-speed=2700", &data); >> free_test_data(&data); >> } >> @@ -2142,6 +2145,7 @@ static void test_acpi_aarch64_virt_tcg_topology(void) >> }; >> >> test_acpi_one("-cpu cortex-a57 " >> + "-machine acpi-pcihp=off " >> "-smp sockets=1,clusters=2,cores=2,threads=2", &data); >> free_test_data(&data); >> } >> @@ -2227,6 +2231,7 @@ static void test_acpi_aarch64_virt_viot(void) >> }; >> >> test_acpi_one("-cpu cortex-a57 " >> + "-machine acpi-pcihp=off " >> "-device virtio-iommu-pci", &data); >> free_test_data(&data); >> } >
On Wed, 28 May 2025 09:41:15 -0300 Gustavo Romero <gustavo.romero@linaro.org> wrote: > Hi Igor, > > On 5/28/25 06:38, Igor Mammedov wrote: > > On Tue, 27 May 2025 09:40:26 +0200 > > Eric Auger <eric.auger@redhat.com> wrote: > > > >> From: Gustavo Romero <gustavo.romero@linaro.org> > >> > >> ACPI PCI hotplug is now turned on by default so we need to change the > >> existing tests to keep it off. However, even setting the ACPI PCI > >> hotplug off in the existing tests, there will be changes in the ACPI > >> tables because the _OSC method was modified, hence in the next patch of > >> this series the blobs are updated accordingly. > >> > >> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> > >> Signed-off-by: Eric Auger <eric.auger@redhat.com> > > > > it would be better to test whatever default we end up with. > > (like x86) > > hmm maybe there is a confusion here, Igor. We are actually planning what you perhaps, see my reply to Eric about my expectations wrt tests. (i.e. default tests shouldn't have any explicit CLI options, instead it should follow whitelist blobs/set new default patch/update blobs pattern) > said. This patch and the other two in this series related to the bios-tables-test > (i.e., patches 8/25 and 10/25) are for actually making the current (legacy) test pass, > since the new default as per this series will be acpi-pcihp=on. That's why here we're > adapting the current test here to have acpi-pcihp=off. > > The new test that will test for acpi-pcihp=on (the new default) is not in this series > and we decided to merge it separate. It's in the patch 4/5 and 5/5 of the follow series: > > https://mail.gnu.org/archive/html/qemu-devel/2025-05/msg05828.html 4/5 > https://mail.gnu.org/archive/html/qemu-devel/2025-05/msg05827.html 5/5 > > > Cheers, > Gustavo > > >> > >> --- > >> > >> [Eric] also added acpi-pcihp=off to test_acpi_aarch64_virt_tcg_numamem > >> --- > >> tests/qtest/bios-tables-test.c | 13 +++++++++---- > >> 1 file changed, 9 insertions(+), 4 deletions(-) > >> > >> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c > >> index 0a333ec435..6379dba714 100644 > >> --- a/tests/qtest/bios-tables-test.c > >> +++ b/tests/qtest/bios-tables-test.c > >> @@ -1626,7 +1626,7 @@ static void test_acpi_aarch64_virt_tcg_memhp(void) > >> }; > >> > >> data.variant = ".memhp"; > >> - test_acpi_one(" -machine nvdimm=on" > >> + test_acpi_one(" -machine nvdimm=on,acpi-pcihp=off" > >> " -cpu cortex-a57" > >> " -m 256M,slots=3,maxmem=1G" > >> " -object memory-backend-ram,id=ram0,size=128M" > >> @@ -1747,7 +1747,8 @@ static void test_acpi_aarch64_virt_tcg_numamem(void) > >> }; > >> > >> data.variant = ".numamem"; > >> - test_acpi_one(" -cpu cortex-a57" > >> + test_acpi_one(" -machine acpi-pcihp=off" > >> + " -cpu cortex-a57" > >> " -object memory-backend-ram,id=ram0,size=128M" > >> " -numa node,memdev=ram0", > >> &data); > >> @@ -1775,7 +1776,8 @@ static void test_acpi_aarch64_virt_tcg_pxb(void) > >> * to solve the conflicts. > >> */ > >> data.variant = ".pxb"; > >> - test_acpi_one(" -device pcie-root-port,chassis=1,id=pci.1" > >> + test_acpi_one(" -machine acpi-pcihp=off" > >> + " -device pcie-root-port,chassis=1,id=pci.1" > >> " -device virtio-scsi-pci,id=scsi0,bus=pci.1" > >> " -drive file=" > >> "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2," > >> @@ -1846,7 +1848,7 @@ static void test_acpi_aarch64_virt_tcg_acpi_hmat(void) > >> > >> data.variant = ".acpihmatvirt"; > >> > >> - test_acpi_one(" -machine hmat=on" > >> + test_acpi_one(" -machine hmat=on,acpi-pcihp=off" > >> " -cpu cortex-a57" > >> " -smp 4,sockets=2" > >> " -m 384M" > >> @@ -2123,6 +2125,7 @@ static void test_acpi_aarch64_virt_tcg(void) > >> data.smbios_cpu_max_speed = 2900; > >> data.smbios_cpu_curr_speed = 2700; > >> test_acpi_one("-cpu cortex-a57 " > >> + "-machine acpi-pcihp=off " > >> "-smbios type=4,max-speed=2900,current-speed=2700", &data); > >> free_test_data(&data); > >> } > >> @@ -2142,6 +2145,7 @@ static void test_acpi_aarch64_virt_tcg_topology(void) > >> }; > >> > >> test_acpi_one("-cpu cortex-a57 " > >> + "-machine acpi-pcihp=off " > >> "-smp sockets=1,clusters=2,cores=2,threads=2", &data); > >> free_test_data(&data); > >> } > >> @@ -2227,6 +2231,7 @@ static void test_acpi_aarch64_virt_viot(void) > >> }; > >> > >> test_acpi_one("-cpu cortex-a57 " > >> + "-machine acpi-pcihp=off " > >> "-device virtio-iommu-pci", &data); > >> free_test_data(&data); > >> } > > >
Hi Igor, On 5/28/25 10:02, Igor Mammedov wrote: > On Wed, 28 May 2025 09:41:15 -0300 > Gustavo Romero <gustavo.romero@linaro.org> wrote: > >> Hi Igor, >> >> On 5/28/25 06:38, Igor Mammedov wrote: >>> On Tue, 27 May 2025 09:40:26 +0200 >>> Eric Auger <eric.auger@redhat.com> wrote: >>> >>>> From: Gustavo Romero <gustavo.romero@linaro.org> >>>> >>>> ACPI PCI hotplug is now turned on by default so we need to change the >>>> existing tests to keep it off. However, even setting the ACPI PCI >>>> hotplug off in the existing tests, there will be changes in the ACPI >>>> tables because the _OSC method was modified, hence in the next patch of >>>> this series the blobs are updated accordingly. >>>> >>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org> >>>> Signed-off-by: Eric Auger <eric.auger@redhat.com> >>> >>> it would be better to test whatever default we end up with. >>> (like x86) >> >> hmm maybe there is a confusion here, Igor. We are actually planning what you > > perhaps, see my reply to Eric about my expectations wrt tests. Yip, I read it before my reply here. > (i.e. default tests shouldn't have any explicit CLI options, > instead it should follow whitelist blobs/set new default patch/update blobs pattern) I see. I agree with that. But this patch is not about the new test. The new test is _not_ in this series. Patches 8/25, 10/25, and 24/25 are _not_ about the new test but about adapting the _legacy tests_ (native acpi) to the situation when ACPI HP becomes the default, because this series makes acpi-pcihp=on the default, hence the CLI option "acpi-pcihp=off" added to them. An update to the blobs are also necessary because of the change in _OSC method, even when acpi-pcihp=off. >> said. This patch and the other two in this series related to the bios-tables-test >> (i.e., patches 8/25 and 10/25) are for actually making the current (legacy) test pass, >> since the new default as per this series will be acpi-pcihp=on. That's why here we're >> adapting the current test here to have acpi-pcihp=off. >> >> The new test that will test for acpi-pcihp=on (the new default) is not in this series >> and we decided to merge it separate. It's in the patch 4/5 and 5/5 of the follow series: We're doing the "blobs/set new default patch/update blobs pattern" in the new test, which we can merge later, once this series is merged, no? The step "set new default" then will not be necessary because the new test will be merged separate, after this series, so when acpi-pcihp=on is already the default. Please note that although we're using acpi-pcihp=on in the new test, it's not necessary, we can dropped this option, making it implicit as you say, and it will work. This is the new test: >> https://mail.gnu.org/archive/html/qemu-devel/2025-05/msg05828.html 4/5 >> https://mail.gnu.org/archive/html/qemu-devel/2025-05/msg05827.html 5/5 Thus, there are to "acts" of modifying the bios-tables-test: 1) Adapt the current tests to when acpi-pcihp=on becomes the default (hence the addition to them of "acpi-pcihp=off". There is also the need to update the blobs, but it's because of the _OSC method change in DSDT table, which will change anyways, even with "acpi-pcihp=off¨, hence the need for patch 10/25 in this series. This is _done is this series_. 2) Add a new test for testing the default (i.e. acpi-pcihp-on). It follows what you're saying above: "follow whitelist blobs/set new default patch/update blobs pattern", because we can drop the acpi-pcihp-on option from the CLI in this test without any prejudice to test. While the step "set new default patch" was actually done in 1). Cheers, Gustavo >> https://mail.gnu.org/archive/html/qemu-devel/2025-05/msg05828.html 4/5 >> https://mail.gnu.org/archive/html/qemu-devel/2025-05/msg05827.html 5/5 >> >> >> Cheers, >> Gustavo >> >>>> >>>> --- >>>> >>>> [Eric] also added acpi-pcihp=off to test_acpi_aarch64_virt_tcg_numamem >>>> --- >>>> tests/qtest/bios-tables-test.c | 13 +++++++++---- >>>> 1 file changed, 9 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c >>>> index 0a333ec435..6379dba714 100644 >>>> --- a/tests/qtest/bios-tables-test.c >>>> +++ b/tests/qtest/bios-tables-test.c >>>> @@ -1626,7 +1626,7 @@ static void test_acpi_aarch64_virt_tcg_memhp(void) >>>> }; >>>> >>>> data.variant = ".memhp"; >>>> - test_acpi_one(" -machine nvdimm=on" >>>> + test_acpi_one(" -machine nvdimm=on,acpi-pcihp=off" >>>> " -cpu cortex-a57" >>>> " -m 256M,slots=3,maxmem=1G" >>>> " -object memory-backend-ram,id=ram0,size=128M" >>>> @@ -1747,7 +1747,8 @@ static void test_acpi_aarch64_virt_tcg_numamem(void) >>>> }; >>>> >>>> data.variant = ".numamem"; >>>> - test_acpi_one(" -cpu cortex-a57" >>>> + test_acpi_one(" -machine acpi-pcihp=off" >>>> + " -cpu cortex-a57" >>>> " -object memory-backend-ram,id=ram0,size=128M" >>>> " -numa node,memdev=ram0", >>>> &data); >>>> @@ -1775,7 +1776,8 @@ static void test_acpi_aarch64_virt_tcg_pxb(void) >>>> * to solve the conflicts. >>>> */ >>>> data.variant = ".pxb"; >>>> - test_acpi_one(" -device pcie-root-port,chassis=1,id=pci.1" >>>> + test_acpi_one(" -machine acpi-pcihp=off" >>>> + " -device pcie-root-port,chassis=1,id=pci.1" >>>> " -device virtio-scsi-pci,id=scsi0,bus=pci.1" >>>> " -drive file=" >>>> "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2," >>>> @@ -1846,7 +1848,7 @@ static void test_acpi_aarch64_virt_tcg_acpi_hmat(void) >>>> >>>> data.variant = ".acpihmatvirt"; >>>> >>>> - test_acpi_one(" -machine hmat=on" >>>> + test_acpi_one(" -machine hmat=on,acpi-pcihp=off" >>>> " -cpu cortex-a57" >>>> " -smp 4,sockets=2" >>>> " -m 384M" >>>> @@ -2123,6 +2125,7 @@ static void test_acpi_aarch64_virt_tcg(void) >>>> data.smbios_cpu_max_speed = 2900; >>>> data.smbios_cpu_curr_speed = 2700; >>>> test_acpi_one("-cpu cortex-a57 " >>>> + "-machine acpi-pcihp=off " >>>> "-smbios type=4,max-speed=2900,current-speed=2700", &data); >>>> free_test_data(&data); >>>> } >>>> @@ -2142,6 +2145,7 @@ static void test_acpi_aarch64_virt_tcg_topology(void) >>>> }; >>>> >>>> test_acpi_one("-cpu cortex-a57 " >>>> + "-machine acpi-pcihp=off " >>>> "-smp sockets=1,clusters=2,cores=2,threads=2", &data); >>>> free_test_data(&data); >>>> } >>>> @@ -2227,6 +2231,7 @@ static void test_acpi_aarch64_virt_viot(void) >>>> }; >>>> >>>> test_acpi_one("-cpu cortex-a57 " >>>> + "-machine acpi-pcihp=off " >>>> "-device virtio-iommu-pci", &data); >>>> free_test_data(&data); >>>> } >>> >> >
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c index 0a333ec435..6379dba714 100644 --- a/tests/qtest/bios-tables-test.c +++ b/tests/qtest/bios-tables-test.c @@ -1626,7 +1626,7 @@ static void test_acpi_aarch64_virt_tcg_memhp(void) }; data.variant = ".memhp"; - test_acpi_one(" -machine nvdimm=on" + test_acpi_one(" -machine nvdimm=on,acpi-pcihp=off" " -cpu cortex-a57" " -m 256M,slots=3,maxmem=1G" " -object memory-backend-ram,id=ram0,size=128M" @@ -1747,7 +1747,8 @@ static void test_acpi_aarch64_virt_tcg_numamem(void) }; data.variant = ".numamem"; - test_acpi_one(" -cpu cortex-a57" + test_acpi_one(" -machine acpi-pcihp=off" + " -cpu cortex-a57" " -object memory-backend-ram,id=ram0,size=128M" " -numa node,memdev=ram0", &data); @@ -1775,7 +1776,8 @@ static void test_acpi_aarch64_virt_tcg_pxb(void) * to solve the conflicts. */ data.variant = ".pxb"; - test_acpi_one(" -device pcie-root-port,chassis=1,id=pci.1" + test_acpi_one(" -machine acpi-pcihp=off" + " -device pcie-root-port,chassis=1,id=pci.1" " -device virtio-scsi-pci,id=scsi0,bus=pci.1" " -drive file=" "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2," @@ -1846,7 +1848,7 @@ static void test_acpi_aarch64_virt_tcg_acpi_hmat(void) data.variant = ".acpihmatvirt"; - test_acpi_one(" -machine hmat=on" + test_acpi_one(" -machine hmat=on,acpi-pcihp=off" " -cpu cortex-a57" " -smp 4,sockets=2" " -m 384M" @@ -2123,6 +2125,7 @@ static void test_acpi_aarch64_virt_tcg(void) data.smbios_cpu_max_speed = 2900; data.smbios_cpu_curr_speed = 2700; test_acpi_one("-cpu cortex-a57 " + "-machine acpi-pcihp=off " "-smbios type=4,max-speed=2900,current-speed=2700", &data); free_test_data(&data); } @@ -2142,6 +2145,7 @@ static void test_acpi_aarch64_virt_tcg_topology(void) }; test_acpi_one("-cpu cortex-a57 " + "-machine acpi-pcihp=off " "-smp sockets=1,clusters=2,cores=2,threads=2", &data); free_test_data(&data); } @@ -2227,6 +2231,7 @@ static void test_acpi_aarch64_virt_viot(void) }; test_acpi_one("-cpu cortex-a57 " + "-machine acpi-pcihp=off " "-device virtio-iommu-pci", &data); free_test_data(&data); }