diff mbox series

[v5,3/4] avocado_qemu: Add SMMUv3 tests

Message ID 20210706131729.30749-4-eric.auger@redhat.com (mailing list archive)
State New, archived
Headers show
Series avocado-qemu: New SMMUv3 and intel IOMMU tests | expand

Commit Message

Eric Auger July 6, 2021, 1:17 p.m. UTC
Add new tests checking the good behavior of the SMMUv3 protecting
2 virtio pci devices (block and net). We check the guest boots and
we are able to install a package. Different guest configs are tested:
standard, passthrough an strict=0. This is tested with both fedora 31 and
33. The former uses a 5.3 kernel without range invalidation whereas the
latter uses a 5.8 kernel that features range invalidation.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

---

v4 -> v5:
- Added the skipIf statement (William) and William's R-b
- added Wainer's R-b and T-b
---
 tests/acceptance/smmu.py | 133 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)
 create mode 100644 tests/acceptance/smmu.py

Comments

Philippe Mathieu-Daudé July 6, 2021, 1:34 p.m. UTC | #1
On 7/6/21 3:17 PM, Eric Auger wrote:
> Add new tests checking the good behavior of the SMMUv3 protecting
> 2 virtio pci devices (block and net). We check the guest boots and
> we are able to install a package. Different guest configs are tested:
> standard, passthrough an strict=0. This is tested with both fedora 31 and
> 33. The former uses a 5.3 kernel without range invalidation whereas the
> latter uses a 5.8 kernel that features range invalidation.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> 
> ---
> 
> v4 -> v5:
> - Added the skipIf statement (William) and William's R-b
> - added Wainer's R-b and T-b
> ---
>  tests/acceptance/smmu.py | 133 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 133 insertions(+)
>  create mode 100644 tests/acceptance/smmu.py

> +    def run_and_check(self):
> +        if self.kernel_path:
> +            self.vm.add_args('-kernel', self.kernel_path,
> +                             '-append', self.kernel_params,
> +                             '-initrd', self.initrd_path)
> +        self.launch_and_wait()

IIUC above is the 'run' part and below is the 'check' part?

So the check succeed if the VM booted, right?

> +        self.ssh_command('cat /proc/cmdline')
> +        self.ssh_command('dnf -y install numactl-devel')
> +
> +
> +    # 5.3 kernel without RIL #
> +
> +    def test_smmu_noril(self):
> +        """
> +        :avocado: tags=smmu_noril
> +        :avocado: tags=smmu_noril_tests
> +        :avocado: tags=distro_version:31
> +        """
> +        self.common_vm_setup()
> +        self.run_and_check()
Eric Auger July 6, 2021, 1:57 p.m. UTC | #2
Hi Philippe,

On 7/6/21 3:34 PM, Philippe Mathieu-Daudé wrote:
> On 7/6/21 3:17 PM, Eric Auger wrote:
>> Add new tests checking the good behavior of the SMMUv3 protecting
>> 2 virtio pci devices (block and net). We check the guest boots and
>> we are able to install a package. Different guest configs are tested:
>> standard, passthrough an strict=0. This is tested with both fedora 31 and
>> 33. The former uses a 5.3 kernel without range invalidation whereas the
>> latter uses a 5.8 kernel that features range invalidation.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
>> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>> Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>>
>> ---
>>
>> v4 -> v5:
>> - Added the skipIf statement (William) and William's R-b
>> - added Wainer's R-b and T-b
>> ---
>>  tests/acceptance/smmu.py | 133 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 133 insertions(+)
>>  create mode 100644 tests/acceptance/smmu.py
>> +    def run_and_check(self):
>> +        if self.kernel_path:
>> +            self.vm.add_args('-kernel', self.kernel_path,
>> +                             '-append', self.kernel_params,
>> +                             '-initrd', self.initrd_path)
>> +        self.launch_and_wait()
> IIUC above is the 'run' part and below is the 'check' part?
>
> So the check succeed if the VM booted, right?
>
>> +        self.ssh_command('cat /proc/cmdline')
>> +        self.ssh_command('dnf -y install numactl-devel')

For the test to succeed, the guests needs to boot, the ssh connection
needs to be established and  the dnf command needs to complete.
See launch_and_wait() in __init__.py

Thanks

Eric
>> +
>> +
>> +    # 5.3 kernel without RIL #
>> +
>> +    def test_smmu_noril(self):
>> +        """
>> +        :avocado: tags=smmu_noril
>> +        :avocado: tags=smmu_noril_tests
>> +        :avocado: tags=distro_version:31
>> +        """
>> +        self.common_vm_setup()
>> +        self.run_and_check()
Philippe Mathieu-Daudé July 6, 2021, 2:25 p.m. UTC | #3
On 7/6/21 3:57 PM, Eric Auger wrote:
> Hi Philippe,
> 
> On 7/6/21 3:34 PM, Philippe Mathieu-Daudé wrote:
>> On 7/6/21 3:17 PM, Eric Auger wrote:
>>> Add new tests checking the good behavior of the SMMUv3 protecting
>>> 2 virtio pci devices (block and net). We check the guest boots and
>>> we are able to install a package. Different guest configs are tested:
>>> standard, passthrough an strict=0. This is tested with both fedora 31 and
>>> 33. The former uses a 5.3 kernel without range invalidation whereas the
>>> latter uses a 5.8 kernel that features range invalidation.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
>>> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>>> Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>>>
>>> ---
>>>
>>> v4 -> v5:
>>> - Added the skipIf statement (William) and William's R-b
>>> - added Wainer's R-b and T-b
>>> ---
>>>  tests/acceptance/smmu.py | 133 +++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 133 insertions(+)
>>>  create mode 100644 tests/acceptance/smmu.py
>>> +    def run_and_check(self):
>>> +        if self.kernel_path:
>>> +            self.vm.add_args('-kernel', self.kernel_path,
>>> +                             '-append', self.kernel_params,
>>> +                             '-initrd', self.initrd_path)
>>> +        self.launch_and_wait()
>> IIUC above is the 'run' part and below is the 'check' part?
>>
>> So the check succeed if the VM booted, right?
>>
>>> +        self.ssh_command('cat /proc/cmdline')
>>> +        self.ssh_command('dnf -y install numactl-devel')
> 
> For the test to succeed, the guests needs to boot, the ssh connection
> needs to be established and  the dnf command needs to complete.
> See launch_and_wait() in __init__.py

OK. I see the Test class provide a .fail() method:

  fail(msg=None)

    Signals a test failure unconditionally, with msg or None
    for the error message.

but no .succeed() one...

Willian: Should we add a comment here mentioning the test
succeeded by that point? Would it be more explicit to add
.succeed() in avocado.Test or avocado_qemu.Test?
Willian Rampazzo July 6, 2021, 2:39 p.m. UTC | #4
On Tue, Jul 6, 2021 at 11:25 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 7/6/21 3:57 PM, Eric Auger wrote:
> > Hi Philippe,
> >
> > On 7/6/21 3:34 PM, Philippe Mathieu-Daudé wrote:
> >> On 7/6/21 3:17 PM, Eric Auger wrote:
> >>> Add new tests checking the good behavior of the SMMUv3 protecting
> >>> 2 virtio pci devices (block and net). We check the guest boots and
> >>> we are able to install a package. Different guest configs are tested:
> >>> standard, passthrough an strict=0. This is tested with both fedora 31 and
> >>> 33. The former uses a 5.3 kernel without range invalidation whereas the
> >>> latter uses a 5.8 kernel that features range invalidation.
> >>>
> >>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >>> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
> >>> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> >>> Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> >>>
> >>> ---
> >>>
> >>> v4 -> v5:
> >>> - Added the skipIf statement (William) and William's R-b
> >>> - added Wainer's R-b and T-b
> >>> ---
> >>>  tests/acceptance/smmu.py | 133 +++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 133 insertions(+)
> >>>  create mode 100644 tests/acceptance/smmu.py
> >>> +    def run_and_check(self):
> >>> +        if self.kernel_path:
> >>> +            self.vm.add_args('-kernel', self.kernel_path,
> >>> +                             '-append', self.kernel_params,
> >>> +                             '-initrd', self.initrd_path)
> >>> +        self.launch_and_wait()
> >> IIUC above is the 'run' part and below is the 'check' part?
> >>
> >> So the check succeed if the VM booted, right?
> >>
> >>> +        self.ssh_command('cat /proc/cmdline')
> >>> +        self.ssh_command('dnf -y install numactl-devel')
> >
> > For the test to succeed, the guests needs to boot, the ssh connection
> > needs to be established and  the dnf command needs to complete.
> > See launch_and_wait() in __init__.py
>
> OK. I see the Test class provide a .fail() method:
>
>   fail(msg=None)
>
>     Signals a test failure unconditionally, with msg or None
>     for the error message.
>
> but no .succeed() one...
>
> Willian: Should we add a comment here mentioning the test
> succeeded by that point? Would it be more explicit to add
> .succeed() in avocado.Test or avocado_qemu.Test?
>

If the test does not fail, it will succeed anyway.

I miss some assert statements in this test, telling what is really
tested, but as it won't run on CI, I don't see a problem adding it now
and extending it later.
Philippe Mathieu-Daudé July 6, 2021, 2:55 p.m. UTC | #5
On 7/6/21 4:39 PM, Willian Rampazzo wrote:
> On Tue, Jul 6, 2021 at 11:25 AM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> On 7/6/21 3:57 PM, Eric Auger wrote:
>>> Hi Philippe,
>>>
>>> On 7/6/21 3:34 PM, Philippe Mathieu-Daudé wrote:
>>>> On 7/6/21 3:17 PM, Eric Auger wrote:
>>>>> Add new tests checking the good behavior of the SMMUv3 protecting
>>>>> 2 virtio pci devices (block and net). We check the guest boots and
>>>>> we are able to install a package. Different guest configs are tested:
>>>>> standard, passthrough an strict=0. This is tested with both fedora 31 and
>>>>> 33. The former uses a 5.3 kernel without range invalidation whereas the
>>>>> latter uses a 5.8 kernel that features range invalidation.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>>> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
>>>>> Reviewed-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>>>>> Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>>>>>
>>>>> ---
>>>>>
>>>>> v4 -> v5:
>>>>> - Added the skipIf statement (William) and William's R-b
>>>>> - added Wainer's R-b and T-b
>>>>> ---
>>>>>  tests/acceptance/smmu.py | 133 +++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 133 insertions(+)
>>>>>  create mode 100644 tests/acceptance/smmu.py
>>>>> +    def run_and_check(self):
>>>>> +        if self.kernel_path:
>>>>> +            self.vm.add_args('-kernel', self.kernel_path,
>>>>> +                             '-append', self.kernel_params,
>>>>> +                             '-initrd', self.initrd_path)
>>>>> +        self.launch_and_wait()
>>>> IIUC above is the 'run' part and below is the 'check' part?
>>>>
>>>> So the check succeed if the VM booted, right?
>>>>
>>>>> +        self.ssh_command('cat /proc/cmdline')
>>>>> +        self.ssh_command('dnf -y install numactl-devel')
>>>
>>> For the test to succeed, the guests needs to boot, the ssh connection
>>> needs to be established and  the dnf command needs to complete.
>>> See launch_and_wait() in __init__.py
>>
>> OK. I see the Test class provide a .fail() method:
>>
>>   fail(msg=None)
>>
>>     Signals a test failure unconditionally, with msg or None
>>     for the error message.
>>
>> but no .succeed() one...
>>
>> Willian: Should we add a comment here mentioning the test
>> succeeded by that point? Would it be more explicit to add
>> .succeed() in avocado.Test or avocado_qemu.Test?
>>
> 
> If the test does not fail, it will succeed anyway.
> 
> I miss some assert statements in this test, telling what is really
> tested, but as it won't run on CI, I don't see a problem adding it now
> and extending it later.

Sure, but a one-line comment can be easily added by the maintainer
taking this series ;)
diff mbox series

Patch

diff --git a/tests/acceptance/smmu.py b/tests/acceptance/smmu.py
new file mode 100644
index 0000000000..b7ed980067
--- /dev/null
+++ b/tests/acceptance/smmu.py
@@ -0,0 +1,133 @@ 
+# SMMUv3 Functional tests
+#
+# Copyright (c) 2021 Red Hat, Inc.
+#
+# Author:
+#  Eric Auger <eric.auger@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+import os
+
+from avocado import skipIf
+from avocado_qemu import LinuxTest, BUILD_DIR
+
+@skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
+class SMMU(LinuxTest):
+    """
+    :avocado: tags=accel:kvm
+    :avocado: tags=cpu:host
+    :avocado: tags=arch:aarch64
+    :avocado: tags=machine:virt
+    :avocado: tags=distro:fedora
+    :avocado: tags=smmu
+    """
+
+    IOMMU_ADDON = ',iommu_platform=on,disable-modern=off,disable-legacy=on'
+    kernel_path = None
+    initrd_path = None
+    kernel_params = None
+
+    def set_up_boot(self):
+        path = self.download_boot()
+        self.vm.add_args('-device', 'virtio-blk-pci,bus=pcie.0,scsi=off,' +
+                         'drive=drv0,id=virtio-disk0,bootindex=1,'
+                         'werror=stop,rerror=stop' + self.IOMMU_ADDON)
+        self.vm.add_args('-drive',
+                         'file=%s,if=none,cache=writethrough,id=drv0' % path)
+
+    def setUp(self):
+        super(SMMU, self).setUp(None, 'virtio-net-pci' + self.IOMMU_ADDON)
+
+    def common_vm_setup(self, custom_kernel=False):
+        self.require_accelerator("kvm")
+        self.vm.add_args("-accel", "kvm")
+        self.vm.add_args("-cpu", "host")
+        self.vm.add_args("-machine", "iommu=smmuv3")
+        self.vm.add_args("-d", "guest_errors")
+        self.vm.add_args('-bios', os.path.join(BUILD_DIR, 'pc-bios',
+                         'edk2-aarch64-code.fd'))
+        self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
+        self.vm.add_args('-object',
+                         'rng-random,id=rng0,filename=/dev/urandom')
+
+        if custom_kernel is False:
+            return
+
+        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
+        initrd_url = self.distro.pxeboot_url + 'initrd.img'
+        self.kernel_path = self.fetch_asset(kernel_url)
+        self.initrd_path = self.fetch_asset(initrd_url)
+
+    def run_and_check(self):
+        if self.kernel_path:
+            self.vm.add_args('-kernel', self.kernel_path,
+                             '-append', self.kernel_params,
+                             '-initrd', self.initrd_path)
+        self.launch_and_wait()
+        self.ssh_command('cat /proc/cmdline')
+        self.ssh_command('dnf -y install numactl-devel')
+
+
+    # 5.3 kernel without RIL #
+
+    def test_smmu_noril(self):
+        """
+        :avocado: tags=smmu_noril
+        :avocado: tags=smmu_noril_tests
+        :avocado: tags=distro_version:31
+        """
+        self.common_vm_setup()
+        self.run_and_check()
+
+    def test_smmu_noril_passthrough(self):
+        """
+        :avocado: tags=smmu_noril_passthrough
+        :avocado: tags=smmu_noril_tests
+        :avocado: tags=distro_version:31
+        """
+        self.common_vm_setup(True)
+        self.kernel_params = self.distro.default_kernel_params + ' iommu.passthrough=on'
+        self.run_and_check()
+
+    def test_smmu_noril_nostrict(self):
+        """
+        :avocado: tags=smmu_noril_nostrict
+        :avocado: tags=smmu_noril_tests
+        :avocado: tags=distro_version:31
+        """
+        self.common_vm_setup(True)
+        self.kernel_params = self.distro.default_kernel_params + ' iommu.strict=0'
+        self.run_and_check()
+
+    # 5.8 kernel featuring range invalidation
+    # >= v5.7 kernel
+
+    def test_smmu_ril(self):
+        """
+        :avocado: tags=smmu_ril
+        :avocado: tags=smmu_ril_tests
+        :avocado: tags=distro_version:33
+        """
+        self.common_vm_setup()
+        self.run_and_check()
+
+    def test_smmu_ril_passthrough(self):
+        """
+        :avocado: tags=smmu_ril_passthrough
+        :avocado: tags=smmu_ril_tests
+        :avocado: tags=distro_version:33
+        """
+        self.common_vm_setup(True)
+        self.kernel_params = self.distro.default_kernel_params + ' iommu.passthrough=on'
+        self.run_and_check()
+
+    def test_smmu_ril_nostrict(self):
+        """
+        :avocado: tags=smmu_ril_nostrict
+        :avocado: tags=smmu_ril_tests
+        :avocado: tags=distro_version:33
+        """
+        self.common_vm_setup(True)
+        self.kernel_params = self.distro.default_kernel_params + ' iommu.strict=0'
+        self.run_and_check()