[PULL 31/33] tests/acpi: add test case for VIOT

There is a newer version of this series
[PULL 31/33] tests/acpi: add test case for VIOT
Posted by Peter Maydell 3 years, 4 months ago
From: Jean-Philippe Brucker <jean-philippe@linaro.org>

Add two test cases for VIOT, one on the q35 machine and the other on
virt. To test complex topologies the q35 test has two PCIe buses that
bypass the IOMMU (and are therefore not described by VIOT), and two
buses that are translated by virtio-iommu.

Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Message-id: 20211210170415.583179-7-jean-philippe@linaro.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/qtest/bios-tables-test.c | 38 ++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 258874167ef..58df53b15b5 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1465,6 +1465,42 @@ static void test_acpi_virt_tcg(void)
     free_test_data(&data);
 }
 
+static void test_acpi_q35_viot(void)
+{
+    test_data data = {
+        .machine = MACHINE_Q35,
+        .variant = ".viot",
+    };
+
+    /*
+     * To keep things interesting, two buses bypass the IOMMU.
+     * VIOT should only describes the other two buses.
+     */
+    test_acpi_one("-machine default_bus_bypass_iommu=on "
+                  "-device virtio-iommu-pci "
+                  "-device pxb-pcie,bus_nr=0x10,id=pcie.100,bus=pcie.0 "
+                  "-device pxb-pcie,bus_nr=0x20,id=pcie.200,bus=pcie.0,bypass_iommu=on "
+                  "-device pxb-pcie,bus_nr=0x30,id=pcie.300,bus=pcie.0",
+                  &data);
+    free_test_data(&data);
+}
+
+static void test_acpi_virt_viot(void)
+{
+    test_data data = {
+        .machine = "virt",
+        .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
+        .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
+        .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
+        .ram_start = 0x40000000ULL,
+        .scan_len = 128ULL * 1024 * 1024,
+    };
+
+    test_acpi_one("-cpu cortex-a57 "
+                  "-device virtio-iommu-pci", &data);
+    free_test_data(&data);
+}
+
 static void test_oem_fields(test_data *data)
 {
     int i;
@@ -1639,6 +1675,7 @@ int main(int argc, char *argv[])
             qtest_add_func("acpi/q35/kvm/xapic", test_acpi_q35_kvm_xapic);
             qtest_add_func("acpi/q35/kvm/dmar", test_acpi_q35_kvm_dmar);
         }
+        qtest_add_func("acpi/q35/viot", test_acpi_q35_viot);
     } else if (strcmp(arch, "aarch64") == 0) {
         if (has_tcg) {
             qtest_add_func("acpi/virt", test_acpi_virt_tcg);
@@ -1646,6 +1683,7 @@ int main(int argc, char *argv[])
             qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
             qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
             qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
+            qtest_add_func("acpi/virt/viot", test_acpi_virt_viot);
         }
     }
     ret = g_test_run();
-- 
2.25.1


Re: [PULL 31/33] tests/acpi: add test case for VIOT
Posted by Richard Henderson 3 years, 4 months ago
On 12/15/21 2:40 AM, Peter Maydell wrote:
> From: Jean-Philippe Brucker <jean-philippe@linaro.org>
> 
> Add two test cases for VIOT, one on the q35 machine and the other on
> virt. To test complex topologies the q35 test has two PCIe buses that
> bypass the IOMMU (and are therefore not described by VIOT), and two
> buses that are translated by virtio-iommu.
> 
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Message-id: 20211210170415.583179-7-jean-philippe@linaro.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   tests/qtest/bios-tables-test.c | 38 ++++++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)

I should have been more careful while applying.  The aarch64 host failure for this is not 
transient as I first assumed:

PASS 5 qtest-aarch64/bios-tables-test /aarch64/acpi/virt/oem-fields
qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
Broken pipe
ERROR qtest-aarch64/bios-tables-test - too few tests run (expected 6, got 5)
make: *** [Makefile.mtest:312: run-test-37] Error 1


r~

> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 258874167ef..58df53b15b5 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1465,6 +1465,42 @@ static void test_acpi_virt_tcg(void)
>       free_test_data(&data);
>   }
>   
> +static void test_acpi_q35_viot(void)
> +{
> +    test_data data = {
> +        .machine = MACHINE_Q35,
> +        .variant = ".viot",
> +    };
> +
> +    /*
> +     * To keep things interesting, two buses bypass the IOMMU.
> +     * VIOT should only describes the other two buses.
> +     */
> +    test_acpi_one("-machine default_bus_bypass_iommu=on "
> +                  "-device virtio-iommu-pci "
> +                  "-device pxb-pcie,bus_nr=0x10,id=pcie.100,bus=pcie.0 "
> +                  "-device pxb-pcie,bus_nr=0x20,id=pcie.200,bus=pcie.0,bypass_iommu=on "
> +                  "-device pxb-pcie,bus_nr=0x30,id=pcie.300,bus=pcie.0",
> +                  &data);
> +    free_test_data(&data);
> +}
> +
> +static void test_acpi_virt_viot(void)
> +{
> +    test_data data = {
> +        .machine = "virt",
> +        .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
> +        .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
> +        .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
> +        .ram_start = 0x40000000ULL,
> +        .scan_len = 128ULL * 1024 * 1024,
> +    };
> +
> +    test_acpi_one("-cpu cortex-a57 "
> +                  "-device virtio-iommu-pci", &data);
> +    free_test_data(&data);
> +}
> +
>   static void test_oem_fields(test_data *data)
>   {
>       int i;
> @@ -1639,6 +1675,7 @@ int main(int argc, char *argv[])
>               qtest_add_func("acpi/q35/kvm/xapic", test_acpi_q35_kvm_xapic);
>               qtest_add_func("acpi/q35/kvm/dmar", test_acpi_q35_kvm_dmar);
>           }
> +        qtest_add_func("acpi/q35/viot", test_acpi_q35_viot);
>       } else if (strcmp(arch, "aarch64") == 0) {
>           if (has_tcg) {
>               qtest_add_func("acpi/virt", test_acpi_virt_tcg);
> @@ -1646,6 +1683,7 @@ int main(int argc, char *argv[])
>               qtest_add_func("acpi/virt/memhp", test_acpi_virt_tcg_memhp);
>               qtest_add_func("acpi/virt/pxb", test_acpi_virt_tcg_pxb);
>               qtest_add_func("acpi/virt/oem-fields", test_acpi_oem_fields_virt);
> +            qtest_add_func("acpi/virt/viot", test_acpi_virt_viot);
>           }
>       }
>       ret = g_test_run();
> 


Re: [PULL 31/33] tests/acpi: add test case for VIOT
Posted by Jean-Philippe Brucker 3 years, 4 months ago
On Wed, Dec 15, 2021 at 04:59:10PM -0800, Richard Henderson wrote:
> On 12/15/21 2:40 AM, Peter Maydell wrote:
> > From: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > 
> > Add two test cases for VIOT, one on the q35 machine and the other on
> > virt. To test complex topologies the q35 test has two PCIe buses that
> > bypass the IOMMU (and are therefore not described by VIOT), and two
> > buses that are translated by virtio-iommu.
> > 
> > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > Message-id: 20211210170415.583179-7-jean-philippe@linaro.org
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   tests/qtest/bios-tables-test.c | 38 ++++++++++++++++++++++++++++++++++
> >   1 file changed, 38 insertions(+)
> 
> I should have been more careful while applying.  The aarch64 host failure
> for this is not transient as I first assumed:
> 
> PASS 5 qtest-aarch64/bios-tables-test /aarch64/acpi/virt/oem-fields
> qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
> Broken pipe
> ERROR qtest-aarch64/bios-tables-test - too few tests run (expected 6, got 5)
> make: *** [Makefile.mtest:312: run-test-37] Error 1

I'm guessing adding "tcg_only = true", like all other virt machine tests
in this file, should work around this, but I don't really understand the
problem because I can't reproduce it on my aarch64 host (I'm running
"configure --target-list=aarch64-softmmu" followed by "make -j10
check-qtest V=1" in a loop)

Does the attached patch fix it for you?

Thanks,
Jean

--- 8< ---
From 6da0e4d98022d173c79e3caab273b226617d5943 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
Date: Thu, 16 Dec 2021 09:15:06 +0000
Subject: [PATCH] tests/qtest/bios-tables-test: Only run VIOT test on TCG

The VIOT test does not always work under KVM on the virt machine:

  PASS 5 qtest-aarch64/bios-tables-test /aarch64/acpi/virt/oem-fields
  qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
  Broken pipe

Make it TCG only.

Reported-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 tests/qtest/bios-tables-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 58df53b15b..9a468e29eb 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1489,6 +1489,7 @@ static void test_acpi_virt_viot(void)
 {
     test_data data = {
         .machine = "virt",
+        .tcg_only = true,
         .uefi_fl1 = "pc-bios/edk2-aarch64-code.fd",
         .uefi_fl2 = "pc-bios/edk2-arm-vars.fd",
         .cd = "tests/data/uefi-boot-images/bios-tables-test.aarch64.iso.qcow2",
-- 
2.34.1


Re: [PULL 31/33] tests/acpi: add test case for VIOT
Posted by Peter Maydell 3 years, 4 months ago
On Thu, 16 Dec 2021 at 09:58, Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> On Wed, Dec 15, 2021 at 04:59:10PM -0800, Richard Henderson wrote:
> > On 12/15/21 2:40 AM, Peter Maydell wrote:
> > > From: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > >
> > > Add two test cases for VIOT, one on the q35 machine and the other on
> > > virt. To test complex topologies the q35 test has two PCIe buses that
> > > bypass the IOMMU (and are therefore not described by VIOT), and two
> > > buses that are translated by virtio-iommu.
> > >
> > > Reviewed-by: Eric Auger <eric.auger@redhat.com>
> > > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > > Message-id: 20211210170415.583179-7-jean-philippe@linaro.org
> > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > > ---
> > >   tests/qtest/bios-tables-test.c | 38 ++++++++++++++++++++++++++++++++++
> > >   1 file changed, 38 insertions(+)
> >
> > I should have been more careful while applying.  The aarch64 host failure
> > for this is not transient as I first assumed:
> >
> > PASS 5 qtest-aarch64/bios-tables-test /aarch64/acpi/virt/oem-fields
> > qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
> > Broken pipe
> > ERROR qtest-aarch64/bios-tables-test - too few tests run (expected 6, got 5)
> > make: *** [Makefile.mtest:312: run-test-37] Error 1
>
> I'm guessing adding "tcg_only = true", like all other virt machine tests
> in this file, should work around this, but I don't really understand the
> problem because I can't reproduce it on my aarch64 host (I'm running
> "configure --target-list=aarch64-softmmu" followed by "make -j10
> check-qtest V=1" in a loop)

What host are you testing on? The test case explicitly asks
for "-cpu cortex-a57", so it is only going to work with KVM
on hosts with a Cortex-A57.

Richard: given I'm off work for Christmas now, if Jean-Philippe's
suggested fix fixes this are you OK with just applying it directly
to un-break the CI ?

thanks
-- PMM

Re: [PULL 31/33] tests/acpi: add test case for VIOT
Posted by Richard Henderson 3 years, 4 months ago
On 12/16/21 3:28 AM, Peter Maydell wrote:
> On Thu, 16 Dec 2021 at 09:58, Jean-Philippe Brucker
> <jean-philippe@linaro.org> wrote:
>>
>> On Wed, Dec 15, 2021 at 04:59:10PM -0800, Richard Henderson wrote:
>>> On 12/15/21 2:40 AM, Peter Maydell wrote:
>>>> From: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>>>
>>>> Add two test cases for VIOT, one on the q35 machine and the other on
>>>> virt. To test complex topologies the q35 test has two PCIe buses that
>>>> bypass the IOMMU (and are therefore not described by VIOT), and two
>>>> buses that are translated by virtio-iommu.
>>>>
>>>> Reviewed-by: Eric Auger <eric.auger@redhat.com>
>>>> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>>>> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>>> Message-id: 20211210170415.583179-7-jean-philippe@linaro.org
>>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>>> ---
>>>>    tests/qtest/bios-tables-test.c | 38 ++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 38 insertions(+)
>>>
>>> I should have been more careful while applying.  The aarch64 host failure
>>> for this is not transient as I first assumed:
>>>
>>> PASS 5 qtest-aarch64/bios-tables-test /aarch64/acpi/virt/oem-fields
>>> qemu-system-aarch64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
>>> Broken pipe
>>> ERROR qtest-aarch64/bios-tables-test - too few tests run (expected 6, got 5)
>>> make: *** [Makefile.mtest:312: run-test-37] Error 1
>>
>> I'm guessing adding "tcg_only = true", like all other virt machine tests
>> in this file, should work around this, but I don't really understand the
>> problem because I can't reproduce it on my aarch64 host (I'm running
>> "configure --target-list=aarch64-softmmu" followed by "make -j10
>> check-qtest V=1" in a loop)
> 
> What host are you testing on? The test case explicitly asks
> for "-cpu cortex-a57", so it is only going to work with KVM
> on hosts with a Cortex-A57.
> 
> Richard: given I'm off work for Christmas now, if Jean-Philippe's
> suggested fix fixes this are you OK with just applying it directly
> to un-break the CI ?

Yes, it looks like it should fix the problem.  I'll run it through and apply directly.

After the new year we can look to see if -cpu max would not work just as well for most of 
these tests, so that kvm can run them as well.


r~