diff mbox series

[for-10.0] tests/functional: Convert the intel_iommu avocado test

Message ID 20241206181728.1241169-1-thuth@redhat.com (mailing list archive)
State New
Headers show
Series [for-10.0] tests/functional: Convert the intel_iommu avocado test | expand

Commit Message

Thomas Huth Dec. 6, 2024, 6:17 p.m. UTC
Convert the intel_iommu test to the new functional framework.
This test needs some changes since we neither support the old 'LinuxTest'
class in the functional framework yet, nor a way to use SSH for running
commands in the guest. So we now directly download a Fedora kernel and
initrd and set up the serial console for executing the commands and for
looking for the results.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 MAINTAINERS                                   |   1 +
 tests/functional/meson.build                  |   1 +
 .../test_intel_iommu.py}                      | 119 ++++++++----------
 3 files changed, 51 insertions(+), 70 deletions(-)
 rename tests/{avocado/intel_iommu.py => functional/test_intel_iommu.py} (41%)
 mode change 100644 => 100755

Comments

CLEMENT MATHIEU--DRIF Dec. 9, 2024, 6:39 a.m. UTC | #1
Hi Thomas,

Reviewed-by: Clément Mathieu--Drif<clement.mathieu--drif@eviden.com>

Thanks
cmd

On 06/12/2024 19:17, Thomas Huth wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>
>
> Convert the intel_iommu test to the new functional framework.
> This test needs some changes since we neither support the old 'LinuxTest'
> class in the functional framework yet, nor a way to use SSH for running
> commands in the guest. So we now directly download a Fedora kernel and
> initrd and set up the serial console for executing the commands and for
> looking for the results.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   MAINTAINERS                                   |   1 +
>   tests/functional/meson.build                  |   1 +
>   .../test_intel_iommu.py}                      | 119 ++++++++----------
>   3 files changed, 51 insertions(+), 70 deletions(-)
>   rename tests/{avocado/intel_iommu.py => functional/test_intel_iommu.py} (41%)
>   mode change 100644 => 100755
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a62659b330..2ca452dbf9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3679,6 +3679,7 @@ S: Supported
>   F: hw/i386/intel_iommu.c
>   F: hw/i386/intel_iommu_internal.h
>   F: include/hw/i386/intel_iommu.h
> +F: tests/functional/test_intel_iommu.py
>
>   AMD-Vi Emulation
>   S: Orphan
> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> index 30c3eda7e4..dfc95fd904 100644
> --- a/tests/functional/meson.build
> +++ b/tests/functional/meson.build
> @@ -238,6 +238,7 @@ tests_x86_64_system_quick = [
>
>   tests_x86_64_system_thorough = [
>     'acpi_bits',
> +  'intel_iommu',
>     'x86_64_tuxrun',
>     'linux_initrd',
>     'multiprocess',
> diff --git a/tests/avocado/intel_iommu.py b/tests/functional/test_intel_iommu.py
> old mode 100644
> new mode 100755
> similarity index 41%
> rename from tests/avocado/intel_iommu.py
> rename to tests/functional/test_intel_iommu.py
> index 992583fa7d..6e47b1e9de
> --- a/tests/avocado/intel_iommu.py
> +++ b/tests/functional/test_intel_iommu.py
> @@ -1,3 +1,5 @@
> +#!/usr/bin/env python3
> +#
>   # INTEL_IOMMU Functional tests
>   #
>   # Copyright (c) 2021 Red Hat, Inc.
> @@ -7,116 +9,93 @@
>   #
>   # 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 skipUnless
> -from avocado_qemu.linuxtest import LinuxTest
> +from qemu_test import LinuxKernelTest, Asset, exec_command_and_wait_for_pattern
> +
> +class IntelIOMMU(LinuxKernelTest):
>
> -@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on GitLab')
> -class IntelIOMMU(LinuxTest):
> -    """
> -    :avocado: tags=arch:x86_64
> -    :avocado: tags=distro:fedora
> -    :avocado: tags=distro_version:31
> -    :avocado: tags=machine:q35
> -    :avocado: tags=accel:kvm
> -    :avocado: tags=intel_iommu
> -    :avocado: tags=flaky
> -    """
> +    ASSET_KERNEL = Asset(
> +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> +         'releases/39/Server/x86_64/os/images/pxeboot/vmlinuz'),
> +        '5f2ef0de47f8d79d5ee9bf8b0ee6d5ba4d987c2f9a16b8b511a7c69e53931fe3')
> +
> +    ASSET_INITRD = Asset(
> +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> +         'releases/39/Server/x86_64/os/images/pxeboot/initrd.img'),
> +        '5bc29e2d872ceeb39a9698d42da3fb0afd7583dc7180de05a6b78bcc726674bb')
>
>       IOMMU_ADDON = ',iommu_platform=on,disable-modern=off,disable-legacy=on'
> +    default_kernel_params = 'console=ttyS0 rd.rescue quiet '
>       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,' +
> -                         'drive=drv0,id=virtio-disk0,bootindex=1,'
> -                         'werror=stop,rerror=stop' + self.IOMMU_ADDON)
> -        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
> -        self.vm.add_args('-drive',
> -                         'file=%s,if=none,cache=writethrough,id=drv0' % path)
> -
> -    def setUp(self):
> -        super(IntelIOMMU, self).setUp(None, 'virtio-net-pci' + self.IOMMU_ADDON)
> -
>       def add_common_args(self):
>           self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
>           self.vm.add_args('-object',
>                            'rng-random,id=rng0,filename=/dev/urandom')
> +        self.vm.add_args('-device', 'virtio-net-pci' + self.IOMMU_ADDON)
> +        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
> +        self.vm.add_args("-m", "1G")
>
> -    def common_vm_setup(self, custom_kernel=None):
> +    def common_vm_setup(self):
> +        self.set_machine('q35')
>           self.require_accelerator("kvm")
>           self.add_common_args()
>           self.vm.add_args("-accel", "kvm")
>
> -        if custom_kernel is None:
> -            return
> -
> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
> -        self.kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> -        self.initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> +        self.kernel_path = self.ASSET_KERNEL.fetch()
> +        self.initrd_path = self.ASSET_INITRD.fetch()
> +        self.kernel_params = self.default_kernel_params
>
>       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('dmesg | grep -e DMAR -e IOMMU')
> -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
> -        self.ssh_command('dnf -y install numactl-devel')
> +        self.vm.set_console()
> +        self.vm.launch()
> +        self.wait_for_console_pattern('(or press Control-D to continue):')
> +        prompt = ':/root#'
> +        exec_command_and_wait_for_pattern(self, '', prompt)
> +        exec_command_and_wait_for_pattern(self, 'cat /proc/cmdline',
> +                                          'intel_iommu=on')
> +        self.wait_for_console_pattern(prompt)
> +        exec_command_and_wait_for_pattern(self, 'dmesg | grep DMAR:',
> +                                          'IOMMU enabled')
> +        self.wait_for_console_pattern(prompt)
> +        exec_command_and_wait_for_pattern(self,
> +                                    'find /sys/kernel/iommu_groups/ -type l',
> +                                    'devices/0000:00:')
> +        self.wait_for_console_pattern(prompt)
>
>       def test_intel_iommu(self):
> -        """
> -        :avocado: tags=intel_iommu_intremap
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>           self.vm.add_args('-device', 'intel-iommu,intremap=on')
>           self.vm.add_args('-machine', 'kernel_irqchip=split')
> -
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on')
> +        self.kernel_params += 'intel_iommu=on'
>           self.run_and_check()
>
>       def test_intel_iommu_strict(self):
> -        """
> -        :avocado: tags=intel_iommu_strict
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>           self.vm.add_args('-device', 'intel-iommu,intremap=on')
>           self.vm.add_args('-machine', 'kernel_irqchip=split')
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on,strict')
> +        self.kernel_params += 'intel_iommu=on,strict'
>           self.run_and_check()
>
>       def test_intel_iommu_strict_cm(self):
> -        """
> -        :avocado: tags=intel_iommu_strict_cm
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>           self.vm.add_args('-device', 'intel-iommu,intremap=on,caching-mode=on')
>           self.vm.add_args('-machine', 'kernel_irqchip=split')
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on,strict')
> +        self.kernel_params += 'intel_iommu=on,strict'
>           self.run_and_check()
>
>       def test_intel_iommu_pt(self):
> -        """
> -        :avocado: tags=intel_iommu_pt
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>           self.vm.add_args('-device', 'intel-iommu,intremap=on')
>           self.vm.add_args('-machine', 'kernel_irqchip=split')
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on iommu=pt')
> +        self.kernel_params += 'intel_iommu=on iommu=pt'
>           self.run_and_check()
> +
> +if __name__ == '__main__':
> +    LinuxKernelTest.main()
> --
> 2.47.1
>
Cédric Le Goater Dec. 9, 2024, 6:45 a.m. UTC | #2
On 12/6/24 19:17, Thomas Huth wrote:
> Convert the intel_iommu test to the new functional framework.
> This test needs some changes since we neither support the old 'LinuxTest'
> class in the functional framework yet, nor a way to use SSH for running
> commands in the guest. So we now directly download a Fedora kernel and
> initrd and set up the serial console for executing the commands and for
> looking for the results.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>


Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   MAINTAINERS                                   |   1 +
>   tests/functional/meson.build                  |   1 +
>   .../test_intel_iommu.py}                      | 119 ++++++++----------
>   3 files changed, 51 insertions(+), 70 deletions(-)
>   rename tests/{avocado/intel_iommu.py => functional/test_intel_iommu.py} (41%)
>   mode change 100644 => 100755
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a62659b330..2ca452dbf9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3679,6 +3679,7 @@ S: Supported
>   F: hw/i386/intel_iommu.c
>   F: hw/i386/intel_iommu_internal.h
>   F: include/hw/i386/intel_iommu.h
> +F: tests/functional/test_intel_iommu.py
>   
>   AMD-Vi Emulation
>   S: Orphan
> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> index 30c3eda7e4..dfc95fd904 100644
> --- a/tests/functional/meson.build
> +++ b/tests/functional/meson.build
> @@ -238,6 +238,7 @@ tests_x86_64_system_quick = [
>   
>   tests_x86_64_system_thorough = [
>     'acpi_bits',
> +  'intel_iommu',
>     'x86_64_tuxrun',
>     'linux_initrd',
>     'multiprocess',
> diff --git a/tests/avocado/intel_iommu.py b/tests/functional/test_intel_iommu.py
> old mode 100644
> new mode 100755
> similarity index 41%
> rename from tests/avocado/intel_iommu.py
> rename to tests/functional/test_intel_iommu.py
> index 992583fa7d..6e47b1e9de
> --- a/tests/avocado/intel_iommu.py
> +++ b/tests/functional/test_intel_iommu.py
> @@ -1,3 +1,5 @@
> +#!/usr/bin/env python3
> +#
>   # INTEL_IOMMU Functional tests
>   #
>   # Copyright (c) 2021 Red Hat, Inc.
> @@ -7,116 +9,93 @@
>   #
>   # 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 skipUnless
> -from avocado_qemu.linuxtest import LinuxTest
> +from qemu_test import LinuxKernelTest, Asset, exec_command_and_wait_for_pattern
> +
> +class IntelIOMMU(LinuxKernelTest):
>   
> -@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on GitLab')
> -class IntelIOMMU(LinuxTest):
> -    """
> -    :avocado: tags=arch:x86_64
> -    :avocado: tags=distro:fedora
> -    :avocado: tags=distro_version:31
> -    :avocado: tags=machine:q35
> -    :avocado: tags=accel:kvm
> -    :avocado: tags=intel_iommu
> -    :avocado: tags=flaky
> -    """
> +    ASSET_KERNEL = Asset(
> +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> +         'releases/39/Server/x86_64/os/images/pxeboot/vmlinuz'),
> +        '5f2ef0de47f8d79d5ee9bf8b0ee6d5ba4d987c2f9a16b8b511a7c69e53931fe3')
> +
> +    ASSET_INITRD = Asset(
> +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> +         'releases/39/Server/x86_64/os/images/pxeboot/initrd.img'),
> +        '5bc29e2d872ceeb39a9698d42da3fb0afd7583dc7180de05a6b78bcc726674bb')
>   
>       IOMMU_ADDON = ',iommu_platform=on,disable-modern=off,disable-legacy=on'
> +    default_kernel_params = 'console=ttyS0 rd.rescue quiet '
>       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,' +
> -                         'drive=drv0,id=virtio-disk0,bootindex=1,'
> -                         'werror=stop,rerror=stop' + self.IOMMU_ADDON)
> -        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
> -        self.vm.add_args('-drive',
> -                         'file=%s,if=none,cache=writethrough,id=drv0' % path)
> -
> -    def setUp(self):
> -        super(IntelIOMMU, self).setUp(None, 'virtio-net-pci' + self.IOMMU_ADDON)
> -
>       def add_common_args(self):
>           self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
>           self.vm.add_args('-object',
>                            'rng-random,id=rng0,filename=/dev/urandom')
> +        self.vm.add_args('-device', 'virtio-net-pci' + self.IOMMU_ADDON)
> +        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
> +        self.vm.add_args("-m", "1G")
>   
> -    def common_vm_setup(self, custom_kernel=None):
> +    def common_vm_setup(self):
> +        self.set_machine('q35')
>           self.require_accelerator("kvm")
>           self.add_common_args()
>           self.vm.add_args("-accel", "kvm")
>   
> -        if custom_kernel is None:
> -            return
> -
> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
> -        self.kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> -        self.initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> +        self.kernel_path = self.ASSET_KERNEL.fetch()
> +        self.initrd_path = self.ASSET_INITRD.fetch()
> +        self.kernel_params = self.default_kernel_params
>   
>       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('dmesg | grep -e DMAR -e IOMMU')
> -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
> -        self.ssh_command('dnf -y install numactl-devel')
> +        self.vm.set_console()
> +        self.vm.launch()
> +        self.wait_for_console_pattern('(or press Control-D to continue):')
> +        prompt = ':/root#'
> +        exec_command_and_wait_for_pattern(self, '', prompt)
> +        exec_command_and_wait_for_pattern(self, 'cat /proc/cmdline',
> +                                          'intel_iommu=on')
> +        self.wait_for_console_pattern(prompt)
> +        exec_command_and_wait_for_pattern(self, 'dmesg | grep DMAR:',
> +                                          'IOMMU enabled')
> +        self.wait_for_console_pattern(prompt)
> +        exec_command_and_wait_for_pattern(self,
> +                                    'find /sys/kernel/iommu_groups/ -type l',
> +                                    'devices/0000:00:')
> +        self.wait_for_console_pattern(prompt)
>   
>       def test_intel_iommu(self):
> -        """
> -        :avocado: tags=intel_iommu_intremap
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>           self.vm.add_args('-device', 'intel-iommu,intremap=on')
>           self.vm.add_args('-machine', 'kernel_irqchip=split')
> -
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on')
> +        self.kernel_params += 'intel_iommu=on'
>           self.run_and_check()
>   
>       def test_intel_iommu_strict(self):
> -        """
> -        :avocado: tags=intel_iommu_strict
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>           self.vm.add_args('-device', 'intel-iommu,intremap=on')
>           self.vm.add_args('-machine', 'kernel_irqchip=split')
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on,strict')
> +        self.kernel_params += 'intel_iommu=on,strict'
>           self.run_and_check()
>   
>       def test_intel_iommu_strict_cm(self):
> -        """
> -        :avocado: tags=intel_iommu_strict_cm
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>           self.vm.add_args('-device', 'intel-iommu,intremap=on,caching-mode=on')
>           self.vm.add_args('-machine', 'kernel_irqchip=split')
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on,strict')
> +        self.kernel_params += 'intel_iommu=on,strict'
>           self.run_and_check()
>   
>       def test_intel_iommu_pt(self):
> -        """
> -        :avocado: tags=intel_iommu_pt
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>           self.vm.add_args('-device', 'intel-iommu,intremap=on')
>           self.vm.add_args('-machine', 'kernel_irqchip=split')
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on iommu=pt')
> +        self.kernel_params += 'intel_iommu=on iommu=pt'
>           self.run_and_check()
> +
> +if __name__ == '__main__':
> +    LinuxKernelTest.main()
Eric Auger Dec. 9, 2024, 8:12 a.m. UTC | #3
Hi Thomas,

On 12/6/24 19:17, Thomas Huth wrote:
> Convert the intel_iommu test to the new functional framework.
> This test needs some changes since we neither support the old 'LinuxTest'
> class in the functional framework yet, nor a way to use SSH for running
> commands in the guest. So we now directly download a Fedora kernel and
> initrd and set up the serial console for executing the commands and for
> looking for the results.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  MAINTAINERS                                   |   1 +
>  tests/functional/meson.build                  |   1 +
>  .../test_intel_iommu.py}                      | 119 ++++++++----------
>  3 files changed, 51 insertions(+), 70 deletions(-)
>  rename tests/{avocado/intel_iommu.py => functional/test_intel_iommu.py} (41%)
>  mode change 100644 => 100755
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a62659b330..2ca452dbf9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3679,6 +3679,7 @@ S: Supported
>  F: hw/i386/intel_iommu.c
>  F: hw/i386/intel_iommu_internal.h
>  F: include/hw/i386/intel_iommu.h
> +F: tests/functional/test_intel_iommu.py
>  
>  AMD-Vi Emulation
>  S: Orphan
> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> index 30c3eda7e4..dfc95fd904 100644
> --- a/tests/functional/meson.build
> +++ b/tests/functional/meson.build
> @@ -238,6 +238,7 @@ tests_x86_64_system_quick = [
>  
>  tests_x86_64_system_thorough = [
>    'acpi_bits',
> +  'intel_iommu',
>    'x86_64_tuxrun',
>    'linux_initrd',
>    'multiprocess',
> diff --git a/tests/avocado/intel_iommu.py b/tests/functional/test_intel_iommu.py
> old mode 100644
> new mode 100755
> similarity index 41%
> rename from tests/avocado/intel_iommu.py
> rename to tests/functional/test_intel_iommu.py
> index 992583fa7d..6e47b1e9de
> --- a/tests/avocado/intel_iommu.py
> +++ b/tests/functional/test_intel_iommu.py
> @@ -1,3 +1,5 @@
> +#!/usr/bin/env python3
> +#
>  # INTEL_IOMMU Functional tests
>  #
>  # Copyright (c) 2021 Red Hat, Inc.
> @@ -7,116 +9,93 @@
>  #
>  # 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 skipUnless
> -from avocado_qemu.linuxtest import LinuxTest
> +from qemu_test import LinuxKernelTest, Asset, exec_command_and_wait_for_pattern
> +
> +class IntelIOMMU(LinuxKernelTest):
>  
> -@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on GitLab')
> -class IntelIOMMU(LinuxTest):
> -    """
> -    :avocado: tags=arch:x86_64
> -    :avocado: tags=distro:fedora
> -    :avocado: tags=distro_version:31
> -    :avocado: tags=machine:q35
> -    :avocado: tags=accel:kvm
> -    :avocado: tags=intel_iommu
> -    :avocado: tags=flaky
> -    """
> +    ASSET_KERNEL = Asset(
> +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> +         'releases/39/Server/x86_64/os/images/pxeboot/vmlinuz'),
> +        '5f2ef0de47f8d79d5ee9bf8b0ee6d5ba4d987c2f9a16b8b511a7c69e53931fe3')
> +
> +    ASSET_INITRD = Asset(
> +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> +         'releases/39/Server/x86_64/os/images/pxeboot/initrd.img'),
> +        '5bc29e2d872ceeb39a9698d42da3fb0afd7583dc7180de05a6b78bcc726674bb')
>  
>      IOMMU_ADDON = ',iommu_platform=on,disable-modern=off,disable-legacy=on'
> +    default_kernel_params = 'console=ttyS0 rd.rescue quiet '
>      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,' +
> -                         'drive=drv0,id=virtio-disk0,bootindex=1,'
> -                         'werror=stop,rerror=stop' + self.IOMMU_ADDON)
> -        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
> -        self.vm.add_args('-drive',
> -                         'file=%s,if=none,cache=writethrough,id=drv0' % path)
> -
> -    def setUp(self):
> -        super(IntelIOMMU, self).setUp(None, 'virtio-net-pci' + self.IOMMU_ADDON)
> -
>      def add_common_args(self):
>          self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
>          self.vm.add_args('-object',
>                           'rng-random,id=rng0,filename=/dev/urandom')
> +        self.vm.add_args('-device', 'virtio-net-pci' + self.IOMMU_ADDON)
> +        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
> +        self.vm.add_args("-m", "1G")
>  
> -    def common_vm_setup(self, custom_kernel=None):
> +    def common_vm_setup(self):
> +        self.set_machine('q35')
>          self.require_accelerator("kvm")
>          self.add_common_args()
>          self.vm.add_args("-accel", "kvm")
>  
> -        if custom_kernel is None:
> -            return
> -
> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
> -        self.kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> -        self.initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> +        self.kernel_path = self.ASSET_KERNEL.fetch()
> +        self.initrd_path = self.ASSET_INITRD.fetch()
> +        self.kernel_params = self.default_kernel_params
>  
>      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('dmesg | grep -e DMAR -e IOMMU')
> -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
> -        self.ssh_command('dnf -y install numactl-devel')
I understand you cannot use ssh yet but the bulk of the test was the
execution of the dnf install meaning we lose the main substance of it
through the conversion.

Eric
> +        self.vm.set_console()
> +        self.vm.launch()
> +        self.wait_for_console_pattern('(or press Control-D to continue):')
> +        prompt = ':/root#'
> +        exec_command_and_wait_for_pattern(self, '', prompt)
> +        exec_command_and_wait_for_pattern(self, 'cat /proc/cmdline',
> +                                          'intel_iommu=on')
> +        self.wait_for_console_pattern(prompt)
> +        exec_command_and_wait_for_pattern(self, 'dmesg | grep DMAR:',
> +                                          'IOMMU enabled')
> +        self.wait_for_console_pattern(prompt)
> +        exec_command_and_wait_for_pattern(self,
> +                                    'find /sys/kernel/iommu_groups/ -type l',
> +                                    'devices/0000:00:')
> +        self.wait_for_console_pattern(prompt)
>  
>      def test_intel_iommu(self):
> -        """
> -        :avocado: tags=intel_iommu_intremap
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>          self.vm.add_args('-device', 'intel-iommu,intremap=on')
>          self.vm.add_args('-machine', 'kernel_irqchip=split')
> -
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on')
> +        self.kernel_params += 'intel_iommu=on'
>          self.run_and_check()
>  
>      def test_intel_iommu_strict(self):
> -        """
> -        :avocado: tags=intel_iommu_strict
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>          self.vm.add_args('-device', 'intel-iommu,intremap=on')
>          self.vm.add_args('-machine', 'kernel_irqchip=split')
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on,strict')
> +        self.kernel_params += 'intel_iommu=on,strict'
>          self.run_and_check()
>  
>      def test_intel_iommu_strict_cm(self):
> -        """
> -        :avocado: tags=intel_iommu_strict_cm
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>          self.vm.add_args('-device', 'intel-iommu,intremap=on,caching-mode=on')
>          self.vm.add_args('-machine', 'kernel_irqchip=split')
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on,strict')
> +        self.kernel_params += 'intel_iommu=on,strict'
>          self.run_and_check()
>  
>      def test_intel_iommu_pt(self):
> -        """
> -        :avocado: tags=intel_iommu_pt
> -        """
> -
> -        self.common_vm_setup(True)
> +        self.common_vm_setup()
>          self.vm.add_args('-device', 'intel-iommu,intremap=on')
>          self.vm.add_args('-machine', 'kernel_irqchip=split')
> -        self.kernel_params = (self.distro.default_kernel_params +
> -                              ' quiet intel_iommu=on iommu=pt')
> +        self.kernel_params += 'intel_iommu=on iommu=pt'
>          self.run_and_check()
> +
> +if __name__ == '__main__':
> +    LinuxKernelTest.main()
Thomas Huth Dec. 9, 2024, 8:26 a.m. UTC | #4
On 09/12/2024 09.12, Eric Auger wrote:
> Hi Thomas,
> 
> On 12/6/24 19:17, Thomas Huth wrote:
>> Convert the intel_iommu test to the new functional framework.
>> This test needs some changes since we neither support the old 'LinuxTest'
>> class in the functional framework yet, nor a way to use SSH for running
>> commands in the guest. So we now directly download a Fedora kernel and
>> initrd and set up the serial console for executing the commands and for
>> looking for the results.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   MAINTAINERS                                   |   1 +
>>   tests/functional/meson.build                  |   1 +
>>   .../test_intel_iommu.py}                      | 119 ++++++++----------
>>   3 files changed, 51 insertions(+), 70 deletions(-)
>>   rename tests/{avocado/intel_iommu.py => functional/test_intel_iommu.py} (41%)
>>   mode change 100644 => 100755
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a62659b330..2ca452dbf9 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3679,6 +3679,7 @@ S: Supported
>>   F: hw/i386/intel_iommu.c
>>   F: hw/i386/intel_iommu_internal.h
>>   F: include/hw/i386/intel_iommu.h
>> +F: tests/functional/test_intel_iommu.py
>>   
>>   AMD-Vi Emulation
>>   S: Orphan
>> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
>> index 30c3eda7e4..dfc95fd904 100644
>> --- a/tests/functional/meson.build
>> +++ b/tests/functional/meson.build
>> @@ -238,6 +238,7 @@ tests_x86_64_system_quick = [
>>   
>>   tests_x86_64_system_thorough = [
>>     'acpi_bits',
>> +  'intel_iommu',
>>     'x86_64_tuxrun',
>>     'linux_initrd',
>>     'multiprocess',
>> diff --git a/tests/avocado/intel_iommu.py b/tests/functional/test_intel_iommu.py
>> old mode 100644
>> new mode 100755
>> similarity index 41%
>> rename from tests/avocado/intel_iommu.py
>> rename to tests/functional/test_intel_iommu.py
>> index 992583fa7d..6e47b1e9de
>> --- a/tests/avocado/intel_iommu.py
>> +++ b/tests/functional/test_intel_iommu.py
>> @@ -1,3 +1,5 @@
>> +#!/usr/bin/env python3
>> +#
>>   # INTEL_IOMMU Functional tests
>>   #
>>   # Copyright (c) 2021 Red Hat, Inc.
>> @@ -7,116 +9,93 @@
>>   #
>>   # 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 skipUnless
>> -from avocado_qemu.linuxtest import LinuxTest
>> +from qemu_test import LinuxKernelTest, Asset, exec_command_and_wait_for_pattern
>> +
>> +class IntelIOMMU(LinuxKernelTest):
>>   
>> -@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on GitLab')
>> -class IntelIOMMU(LinuxTest):
>> -    """
>> -    :avocado: tags=arch:x86_64
>> -    :avocado: tags=distro:fedora
>> -    :avocado: tags=distro_version:31
>> -    :avocado: tags=machine:q35
>> -    :avocado: tags=accel:kvm
>> -    :avocado: tags=intel_iommu
>> -    :avocado: tags=flaky
>> -    """
>> +    ASSET_KERNEL = Asset(
>> +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
>> +         'releases/39/Server/x86_64/os/images/pxeboot/vmlinuz'),
>> +        '5f2ef0de47f8d79d5ee9bf8b0ee6d5ba4d987c2f9a16b8b511a7c69e53931fe3')
>> +
>> +    ASSET_INITRD = Asset(
>> +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
>> +         'releases/39/Server/x86_64/os/images/pxeboot/initrd.img'),
>> +        '5bc29e2d872ceeb39a9698d42da3fb0afd7583dc7180de05a6b78bcc726674bb')
>>   
>>       IOMMU_ADDON = ',iommu_platform=on,disable-modern=off,disable-legacy=on'
>> +    default_kernel_params = 'console=ttyS0 rd.rescue quiet '
>>       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,' +
>> -                         'drive=drv0,id=virtio-disk0,bootindex=1,'
>> -                         'werror=stop,rerror=stop' + self.IOMMU_ADDON)
>> -        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
>> -        self.vm.add_args('-drive',
>> -                         'file=%s,if=none,cache=writethrough,id=drv0' % path)
>> -
>> -    def setUp(self):
>> -        super(IntelIOMMU, self).setUp(None, 'virtio-net-pci' + self.IOMMU_ADDON)
>> -
>>       def add_common_args(self):
>>           self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
>>           self.vm.add_args('-object',
>>                            'rng-random,id=rng0,filename=/dev/urandom')
>> +        self.vm.add_args('-device', 'virtio-net-pci' + self.IOMMU_ADDON)
>> +        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
>> +        self.vm.add_args("-m", "1G")
>>   
>> -    def common_vm_setup(self, custom_kernel=None):
>> +    def common_vm_setup(self):
>> +        self.set_machine('q35')
>>           self.require_accelerator("kvm")
>>           self.add_common_args()
>>           self.vm.add_args("-accel", "kvm")
>>   
>> -        if custom_kernel is None:
>> -            return
>> -
>> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
>> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
>> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
>> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
>> -        self.kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>> -        self.initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
>> +        self.kernel_path = self.ASSET_KERNEL.fetch()
>> +        self.initrd_path = self.ASSET_INITRD.fetch()
>> +        self.kernel_params = self.default_kernel_params
>>   
>>       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('dmesg | grep -e DMAR -e IOMMU')
>> -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
>> -        self.ssh_command('dnf -y install numactl-devel')
> I understand you cannot use ssh yet but the bulk of the test was the
> execution of the dnf install meaning we lose the main substance of it
> through the conversion.

Ah, I see, so this was exercising the virtio-net device with the IOMMU ... 
and I already wondered why there was this "dnf install" at the end without 
doing anything with  the numactl-devel package ... (a comment would have 
been helpful here)

Ok, then please disregard this version of the patch, I guess I have to 
tackle this in a different way...

  Thomas
Eric Auger Dec. 9, 2024, 8:58 a.m. UTC | #5
On 12/9/24 09:26, Thomas Huth wrote:
> On 09/12/2024 09.12, Eric Auger wrote:
>> Hi Thomas,
>>
>> On 12/6/24 19:17, Thomas Huth wrote:
>>> Convert the intel_iommu test to the new functional framework.
>>> This test needs some changes since we neither support the old
>>> 'LinuxTest'
>>> class in the functional framework yet, nor a way to use SSH for running
>>> commands in the guest. So we now directly download a Fedora kernel and
>>> initrd and set up the serial console for executing the commands and for
>>> looking for the results.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   MAINTAINERS                                   |   1 +
>>>   tests/functional/meson.build                  |   1 +
>>>   .../test_intel_iommu.py}                      | 119
>>> ++++++++----------
>>>   3 files changed, 51 insertions(+), 70 deletions(-)
>>>   rename tests/{avocado/intel_iommu.py =>
>>> functional/test_intel_iommu.py} (41%)
>>>   mode change 100644 => 100755
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index a62659b330..2ca452dbf9 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -3679,6 +3679,7 @@ S: Supported
>>>   F: hw/i386/intel_iommu.c
>>>   F: hw/i386/intel_iommu_internal.h
>>>   F: include/hw/i386/intel_iommu.h
>>> +F: tests/functional/test_intel_iommu.py
>>>     AMD-Vi Emulation
>>>   S: Orphan
>>> diff --git a/tests/functional/meson.build
>>> b/tests/functional/meson.build
>>> index 30c3eda7e4..dfc95fd904 100644
>>> --- a/tests/functional/meson.build
>>> +++ b/tests/functional/meson.build
>>> @@ -238,6 +238,7 @@ tests_x86_64_system_quick = [
>>>     tests_x86_64_system_thorough = [
>>>     'acpi_bits',
>>> +  'intel_iommu',
>>>     'x86_64_tuxrun',
>>>     'linux_initrd',
>>>     'multiprocess',
>>> diff --git a/tests/avocado/intel_iommu.py
>>> b/tests/functional/test_intel_iommu.py
>>> old mode 100644
>>> new mode 100755
>>> similarity index 41%
>>> rename from tests/avocado/intel_iommu.py
>>> rename to tests/functional/test_intel_iommu.py
>>> index 992583fa7d..6e47b1e9de
>>> --- a/tests/avocado/intel_iommu.py
>>> +++ b/tests/functional/test_intel_iommu.py
>>> @@ -1,3 +1,5 @@
>>> +#!/usr/bin/env python3
>>> +#
>>>   # INTEL_IOMMU Functional tests
>>>   #
>>>   # Copyright (c) 2021 Red Hat, Inc.
>>> @@ -7,116 +9,93 @@
>>>   #
>>>   # 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 skipUnless
>>> -from avocado_qemu.linuxtest import LinuxTest
>>> +from qemu_test import LinuxKernelTest, Asset,
>>> exec_command_and_wait_for_pattern
>>> +
>>> +class IntelIOMMU(LinuxKernelTest):
>>>   -@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable
>>> on GitLab')
>>> -class IntelIOMMU(LinuxTest):
>>> -    """
>>> -    :avocado: tags=arch:x86_64
>>> -    :avocado: tags=distro:fedora
>>> -    :avocado: tags=distro_version:31
>>> -    :avocado: tags=machine:q35
>>> -    :avocado: tags=accel:kvm
>>> -    :avocado: tags=intel_iommu
>>> -    :avocado: tags=flaky
>>> -    """
>>> +    ASSET_KERNEL = Asset(
>>> +       
>>> ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
>>> +         'releases/39/Server/x86_64/os/images/pxeboot/vmlinuz'),
>>> +       
>>> '5f2ef0de47f8d79d5ee9bf8b0ee6d5ba4d987c2f9a16b8b511a7c69e53931fe3')
>>> +
>>> +    ASSET_INITRD = Asset(
>>> +       
>>> ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
>>> +         'releases/39/Server/x86_64/os/images/pxeboot/initrd.img'),
>>> +       
>>> '5bc29e2d872ceeb39a9698d42da3fb0afd7583dc7180de05a6b78bcc726674bb')
>>>         IOMMU_ADDON =
>>> ',iommu_platform=on,disable-modern=off,disable-legacy=on'
>>> +    default_kernel_params = 'console=ttyS0 rd.rescue quiet '
>>>       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,' +
>>> -                         'drive=drv0,id=virtio-disk0,bootindex=1,'
>>> -                         'werror=stop,rerror=stop' + self.IOMMU_ADDON)
>>> -        self.vm.add_args('-device', 'virtio-gpu-pci' +
>>> self.IOMMU_ADDON)
>>> -        self.vm.add_args('-drive',
>>> -                        
>>> 'file=%s,if=none,cache=writethrough,id=drv0' % path)
>>> -
>>> -    def setUp(self):
>>> -        super(IntelIOMMU, self).setUp(None, 'virtio-net-pci' +
>>> self.IOMMU_ADDON)
>>> -
>>>       def add_common_args(self):
>>>           self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
>>>           self.vm.add_args('-object',
>>>                            'rng-random,id=rng0,filename=/dev/urandom')
>>> +        self.vm.add_args('-device', 'virtio-net-pci' +
>>> self.IOMMU_ADDON)
>>> +        self.vm.add_args('-device', 'virtio-gpu-pci' +
>>> self.IOMMU_ADDON)
>>> +        self.vm.add_args("-m", "1G")
>>>   -    def common_vm_setup(self, custom_kernel=None):
>>> +    def common_vm_setup(self):
>>> +        self.set_machine('q35')
>>>           self.require_accelerator("kvm")
>>>           self.add_common_args()
>>>           self.vm.add_args("-accel", "kvm")
>>>   -        if custom_kernel is None:
>>> -            return
>>> -
>>> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
>>> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
>>> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
>>> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
>>> -        self.kernel_path = self.fetch_asset(kernel_url,
>>> asset_hash=kernel_hash)
>>> -        self.initrd_path = self.fetch_asset(initrd_url,
>>> asset_hash=initrd_hash)
>>> +        self.kernel_path = self.ASSET_KERNEL.fetch()
>>> +        self.initrd_path = self.ASSET_INITRD.fetch()
>>> +        self.kernel_params = self.default_kernel_params
>>>         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('dmesg | grep -e DMAR -e IOMMU')
>>> -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
>>> -        self.ssh_command('dnf -y install numactl-devel')
>> I understand you cannot use ssh yet but the bulk of the test was the
>> execution of the dnf install meaning we lose the main substance of it
>> through the conversion.
>
> Ah, I see, so this was exercising the virtio-net device with the IOMMU
> ... and I already wondered why there was this "dnf install" at the end
> without doing anything with  the numactl-devel package ... (a comment
> would have been helpful here)
yes you got it. The point was to exercise the protected virtio-net device.
>
> Ok, then please disregard this version of the patch, I guess I have to
> tackle this in a different way...
np

Eric
>
>  Thomas
>
Daniel P. Berrangé Dec. 9, 2024, 9:09 a.m. UTC | #6
On Mon, Dec 09, 2024 at 09:26:21AM +0100, Thomas Huth wrote:
> On 09/12/2024 09.12, Eric Auger wrote:
> > Hi Thomas,
> > 
> > On 12/6/24 19:17, Thomas Huth wrote:
> > > Convert the intel_iommu test to the new functional framework.
> > > This test needs some changes since we neither support the old 'LinuxTest'
> > > class in the functional framework yet, nor a way to use SSH for running
> > > commands in the guest. So we now directly download a Fedora kernel and
> > > initrd and set up the serial console for executing the commands and for
> > > looking for the results.
> > > 
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > ---
> > >   MAINTAINERS                                   |   1 +
> > >   tests/functional/meson.build                  |   1 +
> > >   .../test_intel_iommu.py}                      | 119 ++++++++----------
> > >   3 files changed, 51 insertions(+), 70 deletions(-)
> > >   rename tests/{avocado/intel_iommu.py => functional/test_intel_iommu.py} (41%)
> > >   mode change 100644 => 100755
> > > 
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index a62659b330..2ca452dbf9 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -3679,6 +3679,7 @@ S: Supported
> > >   F: hw/i386/intel_iommu.c
> > >   F: hw/i386/intel_iommu_internal.h
> > >   F: include/hw/i386/intel_iommu.h
> > > +F: tests/functional/test_intel_iommu.py
> > >   AMD-Vi Emulation
> > >   S: Orphan
> > > diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> > > index 30c3eda7e4..dfc95fd904 100644
> > > --- a/tests/functional/meson.build
> > > +++ b/tests/functional/meson.build
> > > @@ -238,6 +238,7 @@ tests_x86_64_system_quick = [
> > >   tests_x86_64_system_thorough = [
> > >     'acpi_bits',
> > > +  'intel_iommu',
> > >     'x86_64_tuxrun',
> > >     'linux_initrd',
> > >     'multiprocess',
> > > diff --git a/tests/avocado/intel_iommu.py b/tests/functional/test_intel_iommu.py
> > > old mode 100644
> > > new mode 100755
> > > similarity index 41%
> > > rename from tests/avocado/intel_iommu.py
> > > rename to tests/functional/test_intel_iommu.py
> > > index 992583fa7d..6e47b1e9de
> > > --- a/tests/avocado/intel_iommu.py
> > > +++ b/tests/functional/test_intel_iommu.py
> > > @@ -1,3 +1,5 @@
> > > +#!/usr/bin/env python3
> > > +#
> > >   # INTEL_IOMMU Functional tests
> > >   #
> > >   # Copyright (c) 2021 Red Hat, Inc.
> > > @@ -7,116 +9,93 @@
> > >   #
> > >   # 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 skipUnless
> > > -from avocado_qemu.linuxtest import LinuxTest
> > > +from qemu_test import LinuxKernelTest, Asset, exec_command_and_wait_for_pattern
> > > +
> > > +class IntelIOMMU(LinuxKernelTest):
> > > -@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on GitLab')
> > > -class IntelIOMMU(LinuxTest):
> > > -    """
> > > -    :avocado: tags=arch:x86_64
> > > -    :avocado: tags=distro:fedora
> > > -    :avocado: tags=distro_version:31
> > > -    :avocado: tags=machine:q35
> > > -    :avocado: tags=accel:kvm
> > > -    :avocado: tags=intel_iommu
> > > -    :avocado: tags=flaky
> > > -    """
> > > +    ASSET_KERNEL = Asset(
> > > +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> > > +         'releases/39/Server/x86_64/os/images/pxeboot/vmlinuz'),
> > > +        '5f2ef0de47f8d79d5ee9bf8b0ee6d5ba4d987c2f9a16b8b511a7c69e53931fe3')
> > > +
> > > +    ASSET_INITRD = Asset(
> > > +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
> > > +         'releases/39/Server/x86_64/os/images/pxeboot/initrd.img'),
> > > +        '5bc29e2d872ceeb39a9698d42da3fb0afd7583dc7180de05a6b78bcc726674bb')
> > >       IOMMU_ADDON = ',iommu_platform=on,disable-modern=off,disable-legacy=on'
> > > +    default_kernel_params = 'console=ttyS0 rd.rescue quiet '
> > >       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,' +
> > > -                         'drive=drv0,id=virtio-disk0,bootindex=1,'
> > > -                         'werror=stop,rerror=stop' + self.IOMMU_ADDON)
> > > -        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
> > > -        self.vm.add_args('-drive',
> > > -                         'file=%s,if=none,cache=writethrough,id=drv0' % path)
> > > -
> > > -    def setUp(self):
> > > -        super(IntelIOMMU, self).setUp(None, 'virtio-net-pci' + self.IOMMU_ADDON)
> > > -
> > >       def add_common_args(self):
> > >           self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
> > >           self.vm.add_args('-object',
> > >                            'rng-random,id=rng0,filename=/dev/urandom')
> > > +        self.vm.add_args('-device', 'virtio-net-pci' + self.IOMMU_ADDON)
> > > +        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
> > > +        self.vm.add_args("-m", "1G")
> > > -    def common_vm_setup(self, custom_kernel=None):
> > > +    def common_vm_setup(self):
> > > +        self.set_machine('q35')
> > >           self.require_accelerator("kvm")
> > >           self.add_common_args()
> > >           self.vm.add_args("-accel", "kvm")
> > > -        if custom_kernel is None:
> > > -            return
> > > -
> > > -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
> > > -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
> > > -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
> > > -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
> > > -        self.kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> > > -        self.initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
> > > +        self.kernel_path = self.ASSET_KERNEL.fetch()
> > > +        self.initrd_path = self.ASSET_INITRD.fetch()
> > > +        self.kernel_params = self.default_kernel_params
> > >       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('dmesg | grep -e DMAR -e IOMMU')
> > > -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
> > > -        self.ssh_command('dnf -y install numactl-devel')
> > I understand you cannot use ssh yet but the bulk of the test was the
> > execution of the dnf install meaning we lose the main substance of it
> > through the conversion.
> 
> Ah, I see, so this was exercising the virtio-net device with the IOMMU ...
> and I already wondered why there was this "dnf install" at the end without
> doing anything with  the numactl-devel package ... (a comment would have
> been helpful here)

FYI, I find 'dnf instal' to be a *highly* undesirable thing todo in
our test functional. Its performance is highly non-deterministic
depending on what mirror you happen to get sent to, such that it could
easily push us over the timeouts. It is also susceptible to periodic
broken mirrors, and instability around time of Fefdora EOL. I can't
remember if it was this test case, or a different one, but I've seen
problems before in avocado with 'dnf install'.

If we want to test working networking, then can we arrange for something
more simple & targetted to run, with better worst case performance.

With regards,
Daniel
Thomas Huth Dec. 9, 2024, 9:25 a.m. UTC | #7
On 09/12/2024 10.09, Daniel P. Berrangé wrote:
> On Mon, Dec 09, 2024 at 09:26:21AM +0100, Thomas Huth wrote:
>> On 09/12/2024 09.12, Eric Auger wrote:
>>> Hi Thomas,
>>>
>>> On 12/6/24 19:17, Thomas Huth wrote:
>>>> Convert the intel_iommu test to the new functional framework.
>>>> This test needs some changes since we neither support the old 'LinuxTest'
>>>> class in the functional framework yet, nor a way to use SSH for running
>>>> commands in the guest. So we now directly download a Fedora kernel and
>>>> initrd and set up the serial console for executing the commands and for
>>>> looking for the results.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>    MAINTAINERS                                   |   1 +
>>>>    tests/functional/meson.build                  |   1 +
>>>>    .../test_intel_iommu.py}                      | 119 ++++++++----------
>>>>    3 files changed, 51 insertions(+), 70 deletions(-)
>>>>    rename tests/{avocado/intel_iommu.py => functional/test_intel_iommu.py} (41%)
>>>>    mode change 100644 => 100755
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index a62659b330..2ca452dbf9 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -3679,6 +3679,7 @@ S: Supported
>>>>    F: hw/i386/intel_iommu.c
>>>>    F: hw/i386/intel_iommu_internal.h
>>>>    F: include/hw/i386/intel_iommu.h
>>>> +F: tests/functional/test_intel_iommu.py
>>>>    AMD-Vi Emulation
>>>>    S: Orphan
>>>> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
>>>> index 30c3eda7e4..dfc95fd904 100644
>>>> --- a/tests/functional/meson.build
>>>> +++ b/tests/functional/meson.build
>>>> @@ -238,6 +238,7 @@ tests_x86_64_system_quick = [
>>>>    tests_x86_64_system_thorough = [
>>>>      'acpi_bits',
>>>> +  'intel_iommu',
>>>>      'x86_64_tuxrun',
>>>>      'linux_initrd',
>>>>      'multiprocess',
>>>> diff --git a/tests/avocado/intel_iommu.py b/tests/functional/test_intel_iommu.py
>>>> old mode 100644
>>>> new mode 100755
>>>> similarity index 41%
>>>> rename from tests/avocado/intel_iommu.py
>>>> rename to tests/functional/test_intel_iommu.py
>>>> index 992583fa7d..6e47b1e9de
>>>> --- a/tests/avocado/intel_iommu.py
>>>> +++ b/tests/functional/test_intel_iommu.py
>>>> @@ -1,3 +1,5 @@
>>>> +#!/usr/bin/env python3
>>>> +#
>>>>    # INTEL_IOMMU Functional tests
>>>>    #
>>>>    # Copyright (c) 2021 Red Hat, Inc.
>>>> @@ -7,116 +9,93 @@
>>>>    #
>>>>    # 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 skipUnless
>>>> -from avocado_qemu.linuxtest import LinuxTest
>>>> +from qemu_test import LinuxKernelTest, Asset, exec_command_and_wait_for_pattern
>>>> +
>>>> +class IntelIOMMU(LinuxKernelTest):
>>>> -@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on GitLab')
>>>> -class IntelIOMMU(LinuxTest):
>>>> -    """
>>>> -    :avocado: tags=arch:x86_64
>>>> -    :avocado: tags=distro:fedora
>>>> -    :avocado: tags=distro_version:31
>>>> -    :avocado: tags=machine:q35
>>>> -    :avocado: tags=accel:kvm
>>>> -    :avocado: tags=intel_iommu
>>>> -    :avocado: tags=flaky
>>>> -    """
>>>> +    ASSET_KERNEL = Asset(
>>>> +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
>>>> +         'releases/39/Server/x86_64/os/images/pxeboot/vmlinuz'),
>>>> +        '5f2ef0de47f8d79d5ee9bf8b0ee6d5ba4d987c2f9a16b8b511a7c69e53931fe3')
>>>> +
>>>> +    ASSET_INITRD = Asset(
>>>> +        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
>>>> +         'releases/39/Server/x86_64/os/images/pxeboot/initrd.img'),
>>>> +        '5bc29e2d872ceeb39a9698d42da3fb0afd7583dc7180de05a6b78bcc726674bb')
>>>>        IOMMU_ADDON = ',iommu_platform=on,disable-modern=off,disable-legacy=on'
>>>> +    default_kernel_params = 'console=ttyS0 rd.rescue quiet '
>>>>        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,' +
>>>> -                         'drive=drv0,id=virtio-disk0,bootindex=1,'
>>>> -                         'werror=stop,rerror=stop' + self.IOMMU_ADDON)
>>>> -        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
>>>> -        self.vm.add_args('-drive',
>>>> -                         'file=%s,if=none,cache=writethrough,id=drv0' % path)
>>>> -
>>>> -    def setUp(self):
>>>> -        super(IntelIOMMU, self).setUp(None, 'virtio-net-pci' + self.IOMMU_ADDON)
>>>> -
>>>>        def add_common_args(self):
>>>>            self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
>>>>            self.vm.add_args('-object',
>>>>                             'rng-random,id=rng0,filename=/dev/urandom')
>>>> +        self.vm.add_args('-device', 'virtio-net-pci' + self.IOMMU_ADDON)
>>>> +        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
>>>> +        self.vm.add_args("-m", "1G")
>>>> -    def common_vm_setup(self, custom_kernel=None):
>>>> +    def common_vm_setup(self):
>>>> +        self.set_machine('q35')
>>>>            self.require_accelerator("kvm")
>>>>            self.add_common_args()
>>>>            self.vm.add_args("-accel", "kvm")
>>>> -        if custom_kernel is None:
>>>> -            return
>>>> -
>>>> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
>>>> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
>>>> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
>>>> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
>>>> -        self.kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
>>>> -        self.initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
>>>> +        self.kernel_path = self.ASSET_KERNEL.fetch()
>>>> +        self.initrd_path = self.ASSET_INITRD.fetch()
>>>> +        self.kernel_params = self.default_kernel_params
>>>>        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('dmesg | grep -e DMAR -e IOMMU')
>>>> -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
>>>> -        self.ssh_command('dnf -y install numactl-devel')
>>> I understand you cannot use ssh yet but the bulk of the test was the
>>> execution of the dnf install meaning we lose the main substance of it
>>> through the conversion.
>>
>> Ah, I see, so this was exercising the virtio-net device with the IOMMU ...
>> and I already wondered why there was this "dnf install" at the end without
>> doing anything with  the numactl-devel package ... (a comment would have
>> been helpful here)
> 
> FYI, I find 'dnf instal' to be a *highly* undesirable thing todo in
> our test functional. Its performance is highly non-deterministic
> depending on what mirror you happen to get sent to, such that it could
> easily push us over the timeouts. It is also susceptible to periodic
> broken mirrors, and instability around time of Fefdora EOL. I can't
> remember if it was this test case, or a different one, but I've seen
> problems before in avocado with 'dnf install'.
> 
> If we want to test working networking, then can we arrange for something
> more simple & targetted to run, with better worst case performance.

That might also be the reason why the avocado test was marked with 
"QEMU_TEST_FLAKY_TESTS" ... I think we might want to replace the "dnf 
install" with some local network file transfer between the guest and the 
host instead. I'll have a look at it when I find some spare time for v2 of 
my patch.

  Thomas
Eric Auger Dec. 9, 2024, 10:24 a.m. UTC | #8
On 12/9/24 10:25, Thomas Huth wrote:
> On 09/12/2024 10.09, Daniel P. Berrangé wrote:
>> On Mon, Dec 09, 2024 at 09:26:21AM +0100, Thomas Huth wrote:
>>> On 09/12/2024 09.12, Eric Auger wrote:
>>>> Hi Thomas,
>>>>
>>>> On 12/6/24 19:17, Thomas Huth wrote:
>>>>> Convert the intel_iommu test to the new functional framework.
>>>>> This test needs some changes since we neither support the old
>>>>> 'LinuxTest'
>>>>> class in the functional framework yet, nor a way to use SSH for
>>>>> running
>>>>> commands in the guest. So we now directly download a Fedora kernel
>>>>> and
>>>>> initrd and set up the serial console for executing the commands
>>>>> and for
>>>>> looking for the results.
>>>>>
>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>> ---
>>>>>    MAINTAINERS                                   |   1 +
>>>>>    tests/functional/meson.build                  |   1 +
>>>>>    .../test_intel_iommu.py}                      | 119
>>>>> ++++++++----------
>>>>>    3 files changed, 51 insertions(+), 70 deletions(-)
>>>>>    rename tests/{avocado/intel_iommu.py =>
>>>>> functional/test_intel_iommu.py} (41%)
>>>>>    mode change 100644 => 100755
>>>>>
>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>> index a62659b330..2ca452dbf9 100644
>>>>> --- a/MAINTAINERS
>>>>> +++ b/MAINTAINERS
>>>>> @@ -3679,6 +3679,7 @@ S: Supported
>>>>>    F: hw/i386/intel_iommu.c
>>>>>    F: hw/i386/intel_iommu_internal.h
>>>>>    F: include/hw/i386/intel_iommu.h
>>>>> +F: tests/functional/test_intel_iommu.py
>>>>>    AMD-Vi Emulation
>>>>>    S: Orphan
>>>>> diff --git a/tests/functional/meson.build
>>>>> b/tests/functional/meson.build
>>>>> index 30c3eda7e4..dfc95fd904 100644
>>>>> --- a/tests/functional/meson.build
>>>>> +++ b/tests/functional/meson.build
>>>>> @@ -238,6 +238,7 @@ tests_x86_64_system_quick = [
>>>>>    tests_x86_64_system_thorough = [
>>>>>      'acpi_bits',
>>>>> +  'intel_iommu',
>>>>>      'x86_64_tuxrun',
>>>>>      'linux_initrd',
>>>>>      'multiprocess',
>>>>> diff --git a/tests/avocado/intel_iommu.py
>>>>> b/tests/functional/test_intel_iommu.py
>>>>> old mode 100644
>>>>> new mode 100755
>>>>> similarity index 41%
>>>>> rename from tests/avocado/intel_iommu.py
>>>>> rename to tests/functional/test_intel_iommu.py
>>>>> index 992583fa7d..6e47b1e9de
>>>>> --- a/tests/avocado/intel_iommu.py
>>>>> +++ b/tests/functional/test_intel_iommu.py
>>>>> @@ -1,3 +1,5 @@
>>>>> +#!/usr/bin/env python3
>>>>> +#
>>>>>    # INTEL_IOMMU Functional tests
>>>>>    #
>>>>>    # Copyright (c) 2021 Red Hat, Inc.
>>>>> @@ -7,116 +9,93 @@
>>>>>    #
>>>>>    # 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 skipUnless
>>>>> -from avocado_qemu.linuxtest import LinuxTest
>>>>> +from qemu_test import LinuxKernelTest, Asset,
>>>>> exec_command_and_wait_for_pattern
>>>>> +
>>>>> +class IntelIOMMU(LinuxKernelTest):
>>>>> -@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable
>>>>> on GitLab')
>>>>> -class IntelIOMMU(LinuxTest):
>>>>> -    """
>>>>> -    :avocado: tags=arch:x86_64
>>>>> -    :avocado: tags=distro:fedora
>>>>> -    :avocado: tags=distro_version:31
>>>>> -    :avocado: tags=machine:q35
>>>>> -    :avocado: tags=accel:kvm
>>>>> -    :avocado: tags=intel_iommu
>>>>> -    :avocado: tags=flaky
>>>>> -    """
>>>>> +    ASSET_KERNEL = Asset(
>>>>> +       
>>>>> ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
>>>>> +         'releases/39/Server/x86_64/os/images/pxeboot/vmlinuz'),
>>>>> +       
>>>>> '5f2ef0de47f8d79d5ee9bf8b0ee6d5ba4d987c2f9a16b8b511a7c69e53931fe3')
>>>>> +
>>>>> +    ASSET_INITRD = Asset(
>>>>> +       
>>>>> ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
>>>>> +         'releases/39/Server/x86_64/os/images/pxeboot/initrd.img'),
>>>>> +       
>>>>> '5bc29e2d872ceeb39a9698d42da3fb0afd7583dc7180de05a6b78bcc726674bb')
>>>>>        IOMMU_ADDON =
>>>>> ',iommu_platform=on,disable-modern=off,disable-legacy=on'
>>>>> +    default_kernel_params = 'console=ttyS0 rd.rescue quiet '
>>>>>        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,' +
>>>>> -                         'drive=drv0,id=virtio-disk0,bootindex=1,'
>>>>> -                         'werror=stop,rerror=stop' +
>>>>> self.IOMMU_ADDON)
>>>>> -        self.vm.add_args('-device', 'virtio-gpu-pci' +
>>>>> self.IOMMU_ADDON)
>>>>> -        self.vm.add_args('-drive',
>>>>> -                        
>>>>> 'file=%s,if=none,cache=writethrough,id=drv0' % path)
>>>>> -
>>>>> -    def setUp(self):
>>>>> -        super(IntelIOMMU, self).setUp(None, 'virtio-net-pci' +
>>>>> self.IOMMU_ADDON)
>>>>> -
>>>>>        def add_common_args(self):
>>>>>            self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
>>>>>            self.vm.add_args('-object',
>>>>>                            
>>>>> 'rng-random,id=rng0,filename=/dev/urandom')
>>>>> +        self.vm.add_args('-device', 'virtio-net-pci' +
>>>>> self.IOMMU_ADDON)
>>>>> +        self.vm.add_args('-device', 'virtio-gpu-pci' +
>>>>> self.IOMMU_ADDON)
>>>>> +        self.vm.add_args("-m", "1G")
>>>>> -    def common_vm_setup(self, custom_kernel=None):
>>>>> +    def common_vm_setup(self):
>>>>> +        self.set_machine('q35')
>>>>>            self.require_accelerator("kvm")
>>>>>            self.add_common_args()
>>>>>            self.vm.add_args("-accel", "kvm")
>>>>> -        if custom_kernel is None:
>>>>> -            return
>>>>> -
>>>>> -        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
>>>>> -        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
>>>>> -        initrd_url = self.distro.pxeboot_url + 'initrd.img'
>>>>> -        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
>>>>> -        self.kernel_path = self.fetch_asset(kernel_url,
>>>>> asset_hash=kernel_hash)
>>>>> -        self.initrd_path = self.fetch_asset(initrd_url,
>>>>> asset_hash=initrd_hash)
>>>>> +        self.kernel_path = self.ASSET_KERNEL.fetch()
>>>>> +        self.initrd_path = self.ASSET_INITRD.fetch()
>>>>> +        self.kernel_params = self.default_kernel_params
>>>>>        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('dmesg | grep -e DMAR -e IOMMU')
>>>>> -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
>>>>> -        self.ssh_command('dnf -y install numactl-devel')
>>>> I understand you cannot use ssh yet but the bulk of the test was the
>>>> execution of the dnf install meaning we lose the main substance of it
>>>> through the conversion.
>>>
>>> Ah, I see, so this was exercising the virtio-net device with the
>>> IOMMU ...
>>> and I already wondered why there was this "dnf install" at the end
>>> without
>>> doing anything with  the numactl-devel package ... (a comment would
>>> have
>>> been helpful here)
>>
>> FYI, I find 'dnf instal' to be a *highly* undesirable thing todo in
>> our test functional. Its performance is highly non-deterministic
>> depending on what mirror you happen to get sent to, such that it could
>> easily push us over the timeouts. It is also susceptible to periodic
>> broken mirrors, and instability around time of Fefdora EOL. I can't
>> remember if it was this test case, or a different one, but I've seen
>> problems before in avocado with 'dnf install'.
>>
>> If we want to test working networking, then can we arrange for something
>> more simple & targetted to run, with better worst case performance.
>
> That might also be the reason why the avocado test was marked with
> "QEMU_TEST_FLAKY_TESTS" ... I think we might want to replace the "dnf
> install" with some local network file transfer between the guest and
> the host instead. I'll have a look at it when I find some spare time
> for v2 of my patch.

I think the actual reason of QEMU_TEST_FLAKY_TESTS presence is the test
was too long to execute. But indeed if we can get rid of the dnf install
and exercise the network a little bit, that's the best solution.

Thanks

Eric
>
>  Thomas
>
Philippe Mathieu-Daudé Dec. 9, 2024, 4:31 p.m. UTC | #9
On 9/12/24 10:09, Daniel P. Berrangé wrote:
> On Mon, Dec 09, 2024 at 09:26:21AM +0100, Thomas Huth wrote:
>> On 09/12/2024 09.12, Eric Auger wrote:
>>> Hi Thomas,
>>>
>>> On 12/6/24 19:17, Thomas Huth wrote:
>>>> Convert the intel_iommu test to the new functional framework.
>>>> This test needs some changes since we neither support the old 'LinuxTest'
>>>> class in the functional framework yet, nor a way to use SSH for running
>>>> commands in the guest. So we now directly download a Fedora kernel and
>>>> initrd and set up the serial console for executing the commands and for
>>>> looking for the results.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>> ---
>>>>    MAINTAINERS                                   |   1 +
>>>>    tests/functional/meson.build                  |   1 +
>>>>    .../test_intel_iommu.py}                      | 119 ++++++++----------
>>>>    3 files changed, 51 insertions(+), 70 deletions(-)
>>>>    rename tests/{avocado/intel_iommu.py => functional/test_intel_iommu.py} (41%)
>>>>    mode change 100644 => 100755


>>>> -        self.launch_and_wait()
>>>> -        self.ssh_command('cat /proc/cmdline')
>>>> -        self.ssh_command('dmesg | grep -e DMAR -e IOMMU')
>>>> -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
>>>> -        self.ssh_command('dnf -y install numactl-devel')
>>> I understand you cannot use ssh yet but the bulk of the test was the
>>> execution of the dnf install meaning we lose the main substance of it
>>> through the conversion.
>>
>> Ah, I see, so this was exercising the virtio-net device with the IOMMU ...
>> and I already wondered why there was this "dnf install" at the end without
>> doing anything with  the numactl-devel package ... (a comment would have
>> been helpful here)
> 
> FYI, I find 'dnf instal' to be a *highly* undesirable thing todo in
> our test functional. Its performance is highly non-deterministic
> depending on what mirror you happen to get sent to, such that it could
> easily push us over the timeouts. It is also susceptible to periodic
> broken mirrors, and instability around time of Fefdora EOL. I can't
> remember if it was this test case, or a different one, but I've seen
> problems before in avocado with 'dnf install'.
> 
> If we want to test working networking, then can we arrange for something
> more simple & targetted to run, with better worst case performance.

Could we use 2 virtio-net interfaces inter-connected and stress with
https://linux.die.net/man/1/ab ?
Daniel P. Berrangé Dec. 9, 2024, 4:37 p.m. UTC | #10
On Mon, Dec 09, 2024 at 05:31:37PM +0100, Philippe Mathieu-Daudé wrote:
> On 9/12/24 10:09, Daniel P. Berrangé wrote:
> > On Mon, Dec 09, 2024 at 09:26:21AM +0100, Thomas Huth wrote:
> > > On 09/12/2024 09.12, Eric Auger wrote:
> > > > Hi Thomas,
> > > > 
> > > > On 12/6/24 19:17, Thomas Huth wrote:
> > > > > Convert the intel_iommu test to the new functional framework.
> > > > > This test needs some changes since we neither support the old 'LinuxTest'
> > > > > class in the functional framework yet, nor a way to use SSH for running
> > > > > commands in the guest. So we now directly download a Fedora kernel and
> > > > > initrd and set up the serial console for executing the commands and for
> > > > > looking for the results.
> > > > > 
> > > > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > > > ---
> > > > >    MAINTAINERS                                   |   1 +
> > > > >    tests/functional/meson.build                  |   1 +
> > > > >    .../test_intel_iommu.py}                      | 119 ++++++++----------
> > > > >    3 files changed, 51 insertions(+), 70 deletions(-)
> > > > >    rename tests/{avocado/intel_iommu.py => functional/test_intel_iommu.py} (41%)
> > > > >    mode change 100644 => 100755
> 
> 
> > > > > -        self.launch_and_wait()
> > > > > -        self.ssh_command('cat /proc/cmdline')
> > > > > -        self.ssh_command('dmesg | grep -e DMAR -e IOMMU')
> > > > > -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
> > > > > -        self.ssh_command('dnf -y install numactl-devel')
> > > > I understand you cannot use ssh yet but the bulk of the test was the
> > > > execution of the dnf install meaning we lose the main substance of it
> > > > through the conversion.
> > > 
> > > Ah, I see, so this was exercising the virtio-net device with the IOMMU ...
> > > and I already wondered why there was this "dnf install" at the end without
> > > doing anything with  the numactl-devel package ... (a comment would have
> > > been helpful here)
> > 
> > FYI, I find 'dnf instal' to be a *highly* undesirable thing todo in
> > our test functional. Its performance is highly non-deterministic
> > depending on what mirror you happen to get sent to, such that it could
> > easily push us over the timeouts. It is also susceptible to periodic
> > broken mirrors, and instability around time of Fefdora EOL. I can't
> > remember if it was this test case, or a different one, but I've seen
> > problems before in avocado with 'dnf install'.
> > 
> > If we want to test working networking, then can we arrange for something
> > more simple & targetted to run, with better worst case performance.
> 
> Could we use 2 virtio-net interfaces inter-connected and stress with
> https://linux.die.net/man/1/ab ?

Do we actually need to stress this ? IMHO for a functional tests we just
need to prove that the device is working at a fairly basic level.
"wget example.com"

With regards,
Daniel
Thomas Huth Dec. 9, 2024, 5:26 p.m. UTC | #11
On 09/12/2024 17.37, Daniel P. Berrangé wrote:
> On Mon, Dec 09, 2024 at 05:31:37PM +0100, Philippe Mathieu-Daudé wrote:
>> On 9/12/24 10:09, Daniel P. Berrangé wrote:
>>> On Mon, Dec 09, 2024 at 09:26:21AM +0100, Thomas Huth wrote:
>>>> On 09/12/2024 09.12, Eric Auger wrote:
>>>>> Hi Thomas,
>>>>>
>>>>> On 12/6/24 19:17, Thomas Huth wrote:
>>>>>> Convert the intel_iommu test to the new functional framework.
>>>>>> This test needs some changes since we neither support the old 'LinuxTest'
>>>>>> class in the functional framework yet, nor a way to use SSH for running
>>>>>> commands in the guest. So we now directly download a Fedora kernel and
>>>>>> initrd and set up the serial console for executing the commands and for
>>>>>> looking for the results.
>>>>>>
>>>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>>>> ---
>>>>>>     MAINTAINERS                                   |   1 +
>>>>>>     tests/functional/meson.build                  |   1 +
>>>>>>     .../test_intel_iommu.py}                      | 119 ++++++++----------
>>>>>>     3 files changed, 51 insertions(+), 70 deletions(-)
>>>>>>     rename tests/{avocado/intel_iommu.py => functional/test_intel_iommu.py} (41%)
>>>>>>     mode change 100644 => 100755
>>
>>
>>>>>> -        self.launch_and_wait()
>>>>>> -        self.ssh_command('cat /proc/cmdline')
>>>>>> -        self.ssh_command('dmesg | grep -e DMAR -e IOMMU')
>>>>>> -        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
>>>>>> -        self.ssh_command('dnf -y install numactl-devel')
>>>>> I understand you cannot use ssh yet but the bulk of the test was the
>>>>> execution of the dnf install meaning we lose the main substance of it
>>>>> through the conversion.
>>>>
>>>> Ah, I see, so this was exercising the virtio-net device with the IOMMU ...
>>>> and I already wondered why there was this "dnf install" at the end without
>>>> doing anything with  the numactl-devel package ... (a comment would have
>>>> been helpful here)
>>>
>>> FYI, I find 'dnf instal' to be a *highly* undesirable thing todo in
>>> our test functional. Its performance is highly non-deterministic
>>> depending on what mirror you happen to get sent to, such that it could
>>> easily push us over the timeouts. It is also susceptible to periodic
>>> broken mirrors, and instability around time of Fefdora EOL. I can't
>>> remember if it was this test case, or a different one, but I've seen
>>> problems before in avocado with 'dnf install'.
>>>
>>> If we want to test working networking, then can we arrange for something
>>> more simple & targetted to run, with better worst case performance.
>>
>> Could we use 2 virtio-net interfaces inter-connected and stress with
>> https://linux.die.net/man/1/ab ?

The problem is that this tool is not available in the guest image by 
default, so we'd need to dial with "dnf install" during runtime again, 
relying on the availability of a third party server... so that would not be 
any better than the "dnf install" that was in this test here before, I think.

> Do we actually need to stress this ? IMHO for a functional tests we just
> need to prove that the device is working at a fairly basic level.
> "wget example.com"

My current plan is to start a HTTP server in the guest ("python3 -m 
http.server 8080" is your friend), and then to download some megabytes from 
the guest to the host to see whether we can transfer them successfully.

  Thomas
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index a62659b330..2ca452dbf9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3679,6 +3679,7 @@  S: Supported
 F: hw/i386/intel_iommu.c
 F: hw/i386/intel_iommu_internal.h
 F: include/hw/i386/intel_iommu.h
+F: tests/functional/test_intel_iommu.py
 
 AMD-Vi Emulation
 S: Orphan
diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index 30c3eda7e4..dfc95fd904 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -238,6 +238,7 @@  tests_x86_64_system_quick = [
 
 tests_x86_64_system_thorough = [
   'acpi_bits',
+  'intel_iommu',
   'x86_64_tuxrun',
   'linux_initrd',
   'multiprocess',
diff --git a/tests/avocado/intel_iommu.py b/tests/functional/test_intel_iommu.py
old mode 100644
new mode 100755
similarity index 41%
rename from tests/avocado/intel_iommu.py
rename to tests/functional/test_intel_iommu.py
index 992583fa7d..6e47b1e9de
--- a/tests/avocado/intel_iommu.py
+++ b/tests/functional/test_intel_iommu.py
@@ -1,3 +1,5 @@ 
+#!/usr/bin/env python3
+#
 # INTEL_IOMMU Functional tests
 #
 # Copyright (c) 2021 Red Hat, Inc.
@@ -7,116 +9,93 @@ 
 #
 # 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 skipUnless
-from avocado_qemu.linuxtest import LinuxTest
+from qemu_test import LinuxKernelTest, Asset, exec_command_and_wait_for_pattern
+
+class IntelIOMMU(LinuxKernelTest):
 
-@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on GitLab')
-class IntelIOMMU(LinuxTest):
-    """
-    :avocado: tags=arch:x86_64
-    :avocado: tags=distro:fedora
-    :avocado: tags=distro_version:31
-    :avocado: tags=machine:q35
-    :avocado: tags=accel:kvm
-    :avocado: tags=intel_iommu
-    :avocado: tags=flaky
-    """
+    ASSET_KERNEL = Asset(
+        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
+         'releases/39/Server/x86_64/os/images/pxeboot/vmlinuz'),
+        '5f2ef0de47f8d79d5ee9bf8b0ee6d5ba4d987c2f9a16b8b511a7c69e53931fe3')
+
+    ASSET_INITRD = Asset(
+        ('https://archives.fedoraproject.org/pub/archive/fedora/linux/'
+         'releases/39/Server/x86_64/os/images/pxeboot/initrd.img'),
+        '5bc29e2d872ceeb39a9698d42da3fb0afd7583dc7180de05a6b78bcc726674bb')
 
     IOMMU_ADDON = ',iommu_platform=on,disable-modern=off,disable-legacy=on'
+    default_kernel_params = 'console=ttyS0 rd.rescue quiet '
     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,' +
-                         'drive=drv0,id=virtio-disk0,bootindex=1,'
-                         'werror=stop,rerror=stop' + self.IOMMU_ADDON)
-        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
-        self.vm.add_args('-drive',
-                         'file=%s,if=none,cache=writethrough,id=drv0' % path)
-
-    def setUp(self):
-        super(IntelIOMMU, self).setUp(None, 'virtio-net-pci' + self.IOMMU_ADDON)
-
     def add_common_args(self):
         self.vm.add_args('-device', 'virtio-rng-pci,rng=rng0')
         self.vm.add_args('-object',
                          'rng-random,id=rng0,filename=/dev/urandom')
+        self.vm.add_args('-device', 'virtio-net-pci' + self.IOMMU_ADDON)
+        self.vm.add_args('-device', 'virtio-gpu-pci' + self.IOMMU_ADDON)
+        self.vm.add_args("-m", "1G")
 
-    def common_vm_setup(self, custom_kernel=None):
+    def common_vm_setup(self):
+        self.set_machine('q35')
         self.require_accelerator("kvm")
         self.add_common_args()
         self.vm.add_args("-accel", "kvm")
 
-        if custom_kernel is None:
-            return
-
-        kernel_url = self.distro.pxeboot_url + 'vmlinuz'
-        kernel_hash = '5b6f6876e1b5bda314f93893271da0d5777b1f3c'
-        initrd_url = self.distro.pxeboot_url + 'initrd.img'
-        initrd_hash = 'dd0340a1b39bd28f88532babd4581c67649ec5b1'
-        self.kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
-        self.initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
+        self.kernel_path = self.ASSET_KERNEL.fetch()
+        self.initrd_path = self.ASSET_INITRD.fetch()
+        self.kernel_params = self.default_kernel_params
 
     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('dmesg | grep -e DMAR -e IOMMU')
-        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
-        self.ssh_command('dnf -y install numactl-devel')
+        self.vm.set_console()
+        self.vm.launch()
+        self.wait_for_console_pattern('(or press Control-D to continue):')
+        prompt = ':/root#'
+        exec_command_and_wait_for_pattern(self, '', prompt)
+        exec_command_and_wait_for_pattern(self, 'cat /proc/cmdline',
+                                          'intel_iommu=on')
+        self.wait_for_console_pattern(prompt)
+        exec_command_and_wait_for_pattern(self, 'dmesg | grep DMAR:',
+                                          'IOMMU enabled')
+        self.wait_for_console_pattern(prompt)
+        exec_command_and_wait_for_pattern(self,
+                                    'find /sys/kernel/iommu_groups/ -type l',
+                                    'devices/0000:00:')
+        self.wait_for_console_pattern(prompt)
 
     def test_intel_iommu(self):
-        """
-        :avocado: tags=intel_iommu_intremap
-        """
-
-        self.common_vm_setup(True)
+        self.common_vm_setup()
         self.vm.add_args('-device', 'intel-iommu,intremap=on')
         self.vm.add_args('-machine', 'kernel_irqchip=split')
-
-        self.kernel_params = (self.distro.default_kernel_params +
-                              ' quiet intel_iommu=on')
+        self.kernel_params += 'intel_iommu=on'
         self.run_and_check()
 
     def test_intel_iommu_strict(self):
-        """
-        :avocado: tags=intel_iommu_strict
-        """
-
-        self.common_vm_setup(True)
+        self.common_vm_setup()
         self.vm.add_args('-device', 'intel-iommu,intremap=on')
         self.vm.add_args('-machine', 'kernel_irqchip=split')
-        self.kernel_params = (self.distro.default_kernel_params +
-                              ' quiet intel_iommu=on,strict')
+        self.kernel_params += 'intel_iommu=on,strict'
         self.run_and_check()
 
     def test_intel_iommu_strict_cm(self):
-        """
-        :avocado: tags=intel_iommu_strict_cm
-        """
-
-        self.common_vm_setup(True)
+        self.common_vm_setup()
         self.vm.add_args('-device', 'intel-iommu,intremap=on,caching-mode=on')
         self.vm.add_args('-machine', 'kernel_irqchip=split')
-        self.kernel_params = (self.distro.default_kernel_params +
-                              ' quiet intel_iommu=on,strict')
+        self.kernel_params += 'intel_iommu=on,strict'
         self.run_and_check()
 
     def test_intel_iommu_pt(self):
-        """
-        :avocado: tags=intel_iommu_pt
-        """
-
-        self.common_vm_setup(True)
+        self.common_vm_setup()
         self.vm.add_args('-device', 'intel-iommu,intremap=on')
         self.vm.add_args('-machine', 'kernel_irqchip=split')
-        self.kernel_params = (self.distro.default_kernel_params +
-                              ' quiet intel_iommu=on iommu=pt')
+        self.kernel_params += 'intel_iommu=on iommu=pt'
         self.run_and_check()
+
+if __name__ == '__main__':
+    LinuxKernelTest.main()