| Message ID | 20210706131729.30749-4-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | avocado-qemu: New SMMUv3 and intel IOMMU tests | expand |
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()
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()
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?
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.
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 --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()