[PATCH v5 0/4] avocado-qemu: New SMMUv3 and intel IOMMU tests

[PATCH v5 0/4] avocado-qemu: New SMMUv3 and intel IOMMU tests

[Eric Auger]

This series adds ARM SMMU and Intel IOMMU functional
tests using Fedora cloud-init images.

ARM SMMU tests feature guests with and without RIL
(range invalidation support) using respectively fedora 33
and 31.  For each, we test the protection of virtio-net-pci
and virtio-block-pci devices. Also strict=no and passthrough
modes are tested. So there is a total of 6 tests.

The series applies on top of Cleber's series:
- [PATCH 0/3] Acceptance Tests: support choosing specific

History:
v4 -> v5:
- William's patch put before my patches
- some import cleanups
- added @skipIf(os.getenv('GITLAB_CI'), 'Running on GitLab')
- Rename into IntelIOMMU Class
- Added R-b/T-b

v3 -> v4:
- I added Wainer's refactoring of KNOWN_DISTROS
into a class (last patch) and took into account his comments.

v2 -> v3:
- Added Intel IOMMU tests were added. Different
operating modes are tested such as strict, caching mode, pt.

Best Regards

Eric

The series and its dependencies can be found at:
https://github.com/eauger/qemu/tree/avocado-qemu-v5

Eric Auger (2):
  avocado_qemu: Add SMMUv3 tests
  avocado_qemu: Add Intel iommu tests

Willian Rampazzo (2):
  avocado_qemu: Fix KNOWN_DISTROS map into the LinuxDistro class
  Acceptance Tests: Add default kernel params and pxeboot url to the
    KNOWN_DISTROS collection

 tests/acceptance/avocado_qemu/__init__.py | 125 ++++++++++++++------
 tests/acceptance/intel_iommu.py           | 115 +++++++++++++++++++
 tests/acceptance/smmu.py                  | 133 ++++++++++++++++++++++
 3 files changed, 340 insertions(+), 33 deletions(-)
 create mode 100644 tests/acceptance/intel_iommu.py
 create mode 100644 tests/acceptance/smmu.py

-- 
2.26.3

[PATCH v5 1/4] avocado_qemu: Fix KNOWN_DISTROS map into the LinuxDistro class

[Eric Auger]

From: Willian Rampazzo <willianr@redhat.com>

As the KNOWN_DISTROS grows, more loosely methods will be created in
the avocado_qemu/__init__.py file.

Let's refactor the code so that KNOWN_DISTROS and related methods are
packaged in a class

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 tests/acceptance/avocado_qemu/__init__.py | 74 +++++++++++++----------
 1 file changed, 42 insertions(+), 32 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index 81ac90bebb..af93cd63ea 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -299,29 +299,43 @@ def ssh_command(self, command):
                          f'Guest command failed: {command}')
         return stdout_lines, stderr_lines
 
+class LinuxDistro:
+    """Represents a Linux distribution
 
-#: A collection of known distros and their respective image checksum
-KNOWN_DISTROS = {
-    'fedora': {
-        '31': {
-            'x86_64':
-            {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
-            'aarch64':
-            {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'},
-            'ppc64':
-            {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
-            's390x':
-            {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'},
+    Holds information of known distros.
+    """
+    #: A collection of known distros and their respective image checksum
+    KNOWN_DISTROS = {
+        'fedora': {
+            '31': {
+                'x86_64':
+                {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
+                'ppc64':
+                {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
+                's390x':
+                {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'},
             }
         }
     }
 
+    def __init__(self, name, version, arch):
+        self.name = name
+        self.version = version
+        self.arch = arch
+        try:
+            self._info = self.KNOWN_DISTROS.get(name).get(version).get(arch)
+        except AttributeError:
+            # Unknown distro
+            self._info = {}
 
-def get_known_distro_checksum(distro, distro_version, arch):
-    try:
-        return KNOWN_DISTROS.get(distro).get(distro_version).get(arch).get('checksum')
-    except AttributeError:
-        return None
+    @property
+    def checksum(self):
+        """Gets the cloud-image file checksum"""
+        return self._info.get('checksum', None)
+
+    @checksum.setter
+    def checksum(self, value):
+        self._info['checksum'] = value
 
 
 class LinuxTest(Test, LinuxSSHMixIn):
@@ -332,24 +346,24 @@ class LinuxTest(Test, LinuxSSHMixIn):
     """
 
     timeout = 900
-    distro_checksum = None
+    distro = None
     username = 'root'
     password = 'password'
 
     def _set_distro(self):
-        distro = self.params.get(
+        distro_name = self.params.get(
             'distro',
             default=self._get_unique_tag_val('distro'))
-        if not distro:
-            distro = 'fedora'
-        self.distro = distro
+        if not distro_name:
+            distro_name = 'fedora'
 
         distro_version = self.params.get(
             'distro_version',
             default=self._get_unique_tag_val('distro_version'))
         if not distro_version:
             distro_version = '31'
-        self.distro_version = distro_version
+
+        self.distro = LinuxDistro(distro_name, distro_version, self.arch)
 
         # The distro checksum behaves differently than distro name and
         # version. First, it does not respect a tag with the same
@@ -358,13 +372,9 @@ def _set_distro(self):
         # order of precedence is: parameter, attribute and then value
         # from KNOWN_DISTROS.
         distro_checksum = self.params.get('distro_checksum',
-                                          default=self.distro_checksum)
-        if not distro_checksum:
-            distro_checksum = get_known_distro_checksum(self.distro,
-                                                        self.distro_version,
-                                                        self.arch)
+                                          default=None)
         if distro_checksum:
-            self.distro_checksum = distro_checksum
+            self.distro.checksum = distro_checksum
 
     def setUp(self, ssh_pubkey=None, network_device_type='virtio-net'):
         super(LinuxTest, self).setUp()
@@ -406,14 +416,14 @@ def download_boot(self):
         self.log.info('Downloading/preparing boot image')
         # Fedora 31 only provides ppc64le images
         image_arch = self.arch
-        if self.distro == 'fedora':
+        if self.distro.name == 'fedora':
             if image_arch == 'ppc64':
                 image_arch = 'ppc64le'
 
         try:
             boot = vmimage.get(
-                self.distro, arch=image_arch, version=self.distro_version,
-                checksum=self.distro_checksum,
+                self.distro.name, arch=image_arch, version=self.distro.version,
+                checksum=self.distro.checksum,
                 algorithm='sha256',
                 cache_dir=self.cache_dirs[0],
                 snapshot_dir=self.workdir)
-- 
2.26.3

[PATCH v5 2/4] Acceptance Tests: Add default kernel params and pxeboot url to the KNOWN_DISTROS collection

[Eric Auger]

From: Willian Rampazzo <willianr@redhat.com>

When running LinuxTests we may need to run the guest with
custom params. It is practical to store the pxeboot URL
and the default kernel params so that the
tests just need to fetch those and augment the kernel params.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>

---

v4 -> v5:
- fixed some indentation issues (William)
- added William's R-b
---
 tests/acceptance/avocado_qemu/__init__.py | 53 ++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
index af93cd63ea..ee354eb458 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -309,12 +309,51 @@ class LinuxDistro:
         'fedora': {
             '31': {
                 'x86_64':
-                {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
+                {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0',
+                'pxeboot_url': 'https://archives.fedoraproject.org/'
+                               'pub/archive/fedora/linux/releases/31/'
+                               'Everything/x86_64/os/images/pxeboot/',
+                'kernel_params': 'root=UUID=b1438b9b-2cab-4065-a99a-08a96687f73c'
+                                 ' ro no_timer_check net.ifnames=0'
+                                 ' console=tty1 console=ttyS0,115200n8'
+                },
+                'aarch64':
+                {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49',
+                'pxeboot_url': 'https://archives.fedoraproject.org/'
+                               'pub/archive/fedora/linux/releases/31/'
+                               'Everything/aarch64/os/images/pxeboot/',
+                'kernel_params': 'root=UUID=b6950a44-9f3c-4076-a9c2-355e8475b0a7'
+                                 ' ro earlyprintk=pl011,0x9000000'
+                                 ' ignore_loglevel no_timer_check'
+                                 ' printk.time=1 rd_NO_PLYMOUTH'
+                                 ' console=ttyAMA0'
+                },
                 'ppc64':
                 {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
                 's390x':
                 {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'},
-            }
+            },
+            '32': {
+                'aarch64':
+                {'checksum': 'b367755c664a2d7a26955bbfff985855adfa2ca15e908baf15b4b176d68d3967',
+                'pxeboot_url': 'http://dl.fedoraproject.org/pub/fedora/linux/'
+                               'releases/32/Server/aarch64/os/images/pxeboot/',
+                'kernel_params': 'root=UUID=3df75b65-be8d-4db4-8655-14d95c0e90c5'
+                                 ' ro no_timer_check net.ifnames=0'
+                                 ' console=tty1 console=ttyS0,115200n8',
+                },
+            },
+            '33': {
+                'aarch64':
+                {'checksum': 'e7f75cdfd523fe5ac2ca9eeece68edc1a81f386a17f969c1d1c7c87031008a6b',
+                'pxeboot_url': 'http://dl.fedoraproject.org/pub/fedora/linux/'
+                               'releases/33/Server/aarch64/os/images/pxeboot/',
+                'kernel_params': 'root=UUID=d20b3ffa-6397-4a63-a734-1126a0208f8a'
+                                 ' ro no_timer_check net.ifnames=0'
+                                 ' console=tty1 console=ttyS0,115200n8'
+                                 ' console=tty0',
+                 },
+            },
         }
     }
 
@@ -337,6 +376,16 @@ def checksum(self):
     def checksum(self, value):
         self._info['checksum'] = value
 
+    @property
+    def pxeboot_url(self):
+        """Gets the repository url where pxeboot files can be found"""
+        return self._info.get('pxeboot_url', None)
+
+    @property
+    def default_kernel_params(self):
+        """Gets the default kernel parameters"""
+        return self._info.get('kernel_params', None)
+
 
 class LinuxTest(Test, LinuxSSHMixIn):
     """Facilitates having a cloud-image Linux based available.
-- 
2.26.3

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

[Eric Auger]

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

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()
-- 
2.26.3

[PATCH v5 4/4] avocado_qemu: Add Intel iommu tests

[Eric Auger]

Add Intel IOMMU functional tests based on fedora 31.
Different configs are checked:
- strict
- caching mode, strict
- passthrough.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Willian Rampazzo <willianr@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

---

v4 -> v5:
- import cleanup (William)
- IntelIOMMU becomes the class name (William)
- Added William's Sob
---
 tests/acceptance/intel_iommu.py | 115 ++++++++++++++++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 tests/acceptance/intel_iommu.py

diff --git a/tests/acceptance/intel_iommu.py b/tests/acceptance/intel_iommu.py
new file mode 100644
index 0000000000..ad10491d89
--- /dev/null
+++ b/tests/acceptance/intel_iommu.py
@@ -0,0 +1,115 @@
+# INTEL_IOMMU 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
+
+@skipIf(os.getenv('GITLAB_CI'), 'Running 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
+    """
+
+    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('-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')
+
+    def common_vm_setup(self, custom_kernel=None):
+        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'
+        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('dmesg | grep -e DMAR -e IOMMU')
+        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
+        self.ssh_command('dnf -y install numactl-devel')
+
+    def test_intel_iommu(self):
+        """
+        :avocado: tags=intel_iommu_intremap
+        """
+
+        self.common_vm_setup(True)
+        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.run_and_check()
+
+    def test_intel_iommu_strict(self):
+        """
+        :avocado: tags=intel_iommu_strict
+        """
+
+        self.common_vm_setup(True)
+        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.run_and_check()
+
+    def test_intel_iommu_strict_cm(self):
+        """
+        :avocado: tags=intel_iommu_strict_cm
+        """
+
+        self.common_vm_setup(True)
+        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.run_and_check()
+
+    def test_intel_iommu_pt(self):
+        """
+        :avocado: tags=intel_iommu_pt
+        """
+
+        self.common_vm_setup(True)
+        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.run_and_check()
-- 
2.26.3

Re: [PATCH v5 3/4] avocado_qemu: Add SMMUv3 tests

[Philippe Mathieu-Daudé]

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()

Re: [PATCH v5 3/4] avocado_qemu: Add SMMUv3 tests

[Eric Auger]

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()

Re: [PATCH v5 3/4] avocado_qemu: Add SMMUv3 tests

[Philippe Mathieu-Daudé]

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?

Re: [PATCH v5 3/4] avocado_qemu: Add SMMUv3 tests

[Willian Rampazzo]

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.

Re: [PATCH v5 3/4] avocado_qemu: Add SMMUv3 tests

[Philippe Mathieu-Daudé]

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 ;)

Re: [PATCH v5 1/4] avocado_qemu: Fix KNOWN_DISTROS map into the LinuxDistro class

[Cleber Rosa]

On 7/6/21 9:17 AM, Eric Auger wrote:
> From: Willian Rampazzo <willianr@redhat.com>
>
> As the KNOWN_DISTROS grows, more loosely methods will be created in
> the avocado_qemu/__init__.py file.
>
> Let's refactor the code so that KNOWN_DISTROS and related methods are
> packaged in a class
>
> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 74 +++++++++++++----------
>   1 file changed, 42 insertions(+), 32 deletions(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index 81ac90bebb..af93cd63ea 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -299,29 +299,43 @@ def ssh_command(self, command):
>                            f'Guest command failed: {command}')
>           return stdout_lines, stderr_lines
>   
> +class LinuxDistro:
> +    """Represents a Linux distribution
>   


I definitely like the idea.


> -#: A collection of known distros and their respective image checksum
> -KNOWN_DISTROS = {
> -    'fedora': {
> -        '31': {
> -            'x86_64':
> -            {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
> -            'aarch64':
> -            {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'},
> -            'ppc64':
> -            {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
> -            's390x':
> -            {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'},
> +    Holds information of known distros.
> +    """
> +    #: A collection of known distros and their respective image checksum
> +    KNOWN_DISTROS = {
> +        'fedora': {
> +            '31': {
> +                'x86_64':
> +                {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
> +                'ppc64':
> +                {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
> +                's390x':
> +                {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'},
>               }
>           }
>       }
>   
> +    def __init__(self, name, version, arch):
> +        self.name = name
> +        self.version = version
> +        self.arch = arch


This looks a lot like 
https://github.com/avocado-framework/avocado/blob/f0996dafefa412c77c221c2d1a6fafdcba1c97b7/avocado/utils/distro.py#L34 
, although admittedly, their goals are very different.


As a next step, in the future, I'd consider separating the data from the 
actual class and having it the LinuxDistro instances, helped by a 
registry.  Something like:


class LinuxDistroRegistry:

    def __init__(self):
       self.distros = set()

    def register(self, linux_distro):

       self.distros.add(linux_distro)

    def query(self, **kwargs):

       ...


registry = LinuxDistroRegistry()

registry.register(LinuxDistro('fedora', '31', 'x86_64', 'deadbeefdeadbeef'))

registry.register(LinuxDistro('fedora', '31', 'aarch64', 
'beefdeadbeefdead'))

checksum = registry.query(name='fedora', version='31', 
arch='x86_64').checksum


> +        try:
> +            self._info = self.KNOWN_DISTROS.get(name).get(version).get(arch)


The `AttributeError` that could be caught at the removed 
`get_known_distro_checksum()` function, could come from any of the 
`.get()`s returning `None`, which in turn would not have a `.get()` 
attribute.

But now, if there's a "name", then a "version", but no "arch" entry, 
this line will set `self._info` to `None`.  This is manifested if you 
try to run a test that tries to find an aarch64 distro, such as:

./tests/venv/bin/avocado run 
tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_tcg_gicv2


It will result in:


20:38:18 ERROR| Reproduced traceback from: 
/var/lib/users/cleber/build/qemu/tests/venv/lib64/python3.9/site-packages/avocado/core/test.py:756
20:38:18 ERROR| Traceback (most recent call last):
20:38:18 ERROR|   File 
"/var/lib/users/cleber/build/qemu/tests/acceptance/avocado_qemu/__init__.py", 
line 426, in download_boot
20:38:18 ERROR|     checksum=self.distro.checksum,
20:38:18 ERROR|   File 
"/var/lib/users/cleber/build/qemu/tests/acceptance/avocado_qemu/__init__.py", 
line 334, in checksum
20:38:18 ERROR|     return self._info.get('checksum', None)
20:38:18 ERROR| AttributeError: 'NoneType' object has no attribute 'get'
20:38:18 ERROR|
20:38:18 ERROR| During handling of the above exception, another 
exception occurred:
20:38:18 ERROR|
20:38:18 ERROR| Traceback (most recent call last):
20:38:18 ERROR|   File 
"/var/lib/users/cleber/build/qemu/tests/acceptance/avocado_qemu/__init__.py", 
line 387, in setUp
20:38:18 ERROR|     self.set_up_boot()
20:38:18 ERROR|   File 
"/var/lib/users/cleber/build/qemu/tests/acceptance/avocado_qemu/__init__.py", 
line 455, in set_up_boot
20:38:18 ERROR|     path = self.download_boot()
20:38:18 ERROR|   File 
"/var/lib/users/cleber/build/qemu/tests/acceptance/avocado_qemu/__init__.py", 
line 431, in download_boot
20:38:18 ERROR|     self.cancel('Failed to download/prepare boot image')
20:38:18 ERROR|   File 
"/var/lib/users/cleber/build/qemu/tests/venv/lib64/python3.9/site-packages/avocado/core/test.py", 
line 988, in cancel
20:38:18 ERROR|     raise exceptions.TestCancel(message)
20:38:18 ERROR| avocado.core.exceptions.TestCancel: Failed to 
download/prepare boot image


Cheers,

- Cleber.

Re: [PATCH v5 2/4] Acceptance Tests: Add default kernel params and pxeboot url to the KNOWN_DISTROS collection

[Cleber Rosa]

On 7/6/21 9:17 AM, Eric Auger wrote:
> From: Willian Rampazzo <willianr@redhat.com>
>
> When running LinuxTests we may need to run the guest with
> custom params. It is practical to store the pxeboot URL
> and the default kernel params so that the
> tests just need to fetch those and augment the kernel params.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Willian Rampazzo <willianr@redhat.com>
>
> ---
>
> v4 -> v5:
> - fixed some indentation issues (William)
> - added William's R-b
> ---
>   tests/acceptance/avocado_qemu/__init__.py | 53 ++++++++++++++++++++++-
>   1 file changed, 51 insertions(+), 2 deletions(-)
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py b/tests/acceptance/avocado_qemu/__init__.py
> index af93cd63ea..ee354eb458 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -309,12 +309,51 @@ class LinuxDistro:
>           'fedora': {
>               '31': {
>                   'x86_64':
> -                {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
> +                {'checksum': 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0',
> +                'pxeboot_url': 'https://archives.fedoraproject.org/'
> +                               'pub/archive/fedora/linux/releases/31/'
> +                               'Everything/x86_64/os/images/pxeboot/',
> +                'kernel_params': 'root=UUID=b1438b9b-2cab-4065-a99a-08a96687f73c'
> +                                 ' ro no_timer_check net.ifnames=0'
> +                                 ' console=tty1 console=ttyS0,115200n8'
> +                },
> +                'aarch64':
> +                {'checksum': '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49',


So this is what made it easy for me to catch the issue on the previous 
patch.  The aarch64 info should be present on the previous patch too, 
with this one adding the pxeboot_url, kernel_params, etc.


But, just to be extra clear, the actual issue of `self._info` being set 
to `None` on the previous patch still needs to be addressed.



> +                'pxeboot_url': 'https://archives.fedoraproject.org/'
> +                               'pub/archive/fedora/linux/releases/31/'
> +                               'Everything/aarch64/os/images/pxeboot/',
> +                'kernel_params': 'root=UUID=b6950a44-9f3c-4076-a9c2-355e8475b0a7'
> +                                 ' ro earlyprintk=pl011,0x9000000'
> +                                 ' ignore_loglevel no_timer_check'
> +                                 ' printk.time=1 rd_NO_PLYMOUTH'
> +                                 ' console=ttyAMA0'
> +                },
>                   'ppc64':
>                   {'checksum': '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
>                   's390x':
>                   {'checksum': '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'},
> -            }
> +            },
> +            '32': {
> +                'aarch64':
> +                {'checksum': 'b367755c664a2d7a26955bbfff985855adfa2ca15e908baf15b4b176d68d3967',
> +                'pxeboot_url': 'http://dl.fedoraproject.org/pub/fedora/linux/'
> +                               'releases/32/Server/aarch64/os/images/pxeboot/',
> +                'kernel_params': 'root=UUID=3df75b65-be8d-4db4-8655-14d95c0e90c5'
> +                                 ' ro no_timer_check net.ifnames=0'
> +                                 ' console=tty1 console=ttyS0,115200n8',
> +                },
> +            },
> +            '33': {
> +                'aarch64':
> +                {'checksum': 'e7f75cdfd523fe5ac2ca9eeece68edc1a81f386a17f969c1d1c7c87031008a6b',
> +                'pxeboot_url': 'http://dl.fedoraproject.org/pub/fedora/linux/'
> +                               'releases/33/Server/aarch64/os/images/pxeboot/',
> +                'kernel_params': 'root=UUID=d20b3ffa-6397-4a63-a734-1126a0208f8a'
> +                                 ' ro no_timer_check net.ifnames=0'
> +                                 ' console=tty1 console=ttyS0,115200n8'
> +                                 ' console=tty0',
> +                 },
> +            },
>           }
>       }
>   
> @@ -337,6 +376,16 @@ def checksum(self):
>       def checksum(self, value):
>           self._info['checksum'] = value
>   
> +    @property
> +    def pxeboot_url(self):
> +        """Gets the repository url where pxeboot files can be found"""
> +        return self._info.get('pxeboot_url', None)
> +
> +    @property
> +    def default_kernel_params(self):
> +        """Gets the default kernel parameters"""
> +        return self._info.get('kernel_params', None)
> +
>   
>   class LinuxTest(Test, LinuxSSHMixIn):
>       """Facilitates having a cloud-image Linux based available.


Everything else looks to me.


Thanks,

- Cleber.

Re: [PATCH v5 1/4] avocado_qemu: Fix KNOWN_DISTROS map into the LinuxDistro class

[Eric Auger]

Hi Wainer, William, Cleber,

On 7/8/21 3:17 AM, Cleber Rosa wrote:
>
> On 7/6/21 9:17 AM, Eric Auger wrote:
>> From: Willian Rampazzo <willianr@redhat.com>
>>
>> As the KNOWN_DISTROS grows, more loosely methods will be created in
>> the avocado_qemu/__init__.py file.
>>
>> Let's refactor the code so that KNOWN_DISTROS and related methods are
>> packaged in a class
>>
>> Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> ---
>>   tests/acceptance/avocado_qemu/__init__.py | 74 +++++++++++++----------
>>   1 file changed, 42 insertions(+), 32 deletions(-)
>>
>> diff --git a/tests/acceptance/avocado_qemu/__init__.py
>> b/tests/acceptance/avocado_qemu/__init__.py
>> index 81ac90bebb..af93cd63ea 100644
>> --- a/tests/acceptance/avocado_qemu/__init__.py
>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>> @@ -299,29 +299,43 @@ def ssh_command(self, command):
>>                            f'Guest command failed: {command}')
>>           return stdout_lines, stderr_lines
>>   +class LinuxDistro:
>> +    """Represents a Linux distribution
>>   
>
>
> I definitely like the idea.
>
>
>> -#: A collection of known distros and their respective image checksum
>> -KNOWN_DISTROS = {
>> -    'fedora': {
>> -        '31': {
>> -            'x86_64':
>> -            {'checksum':
>> 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
>> -            'aarch64':
>> -            {'checksum':
>> '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'},
>> -            'ppc64':
>> -            {'checksum':
>> '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
>> -            's390x':
>> -            {'checksum':
>> '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'},
>> +    Holds information of known distros.
>> +    """
>> +    #: A collection of known distros and their respective image
>> checksum
>> +    KNOWN_DISTROS = {
>> +        'fedora': {
>> +            '31': {
>> +                'x86_64':
>> +                {'checksum':
>> 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
>> +                'ppc64':
>> +                {'checksum':
>> '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
>> +                's390x':
>> +                {'checksum':
>> '4caaab5a434fd4d1079149a072fdc7891e354f834d355069ca982fdcaf5a122d'},
>>               }
>>           }
>>       }
>>   +    def __init__(self, name, version, arch):
>> +        self.name = name
>> +        self.version = version
>> +        self.arch = arch
>
>
> This looks a lot like
> https://github.com/avocado-framework/avocado/blob/f0996dafefa412c77c221c2d1a6fafdcba1c97b7/avocado/utils/distro.py#L34
> , although admittedly, their goals are very different.
>
>
> As a next step, in the future, I'd consider separating the data from
> the actual class and having it the LinuxDistro instances, helped by a
> registry.  Something like:
>
>
> class LinuxDistroRegistry:
>
>    def __init__(self):
>       self.distros = set()
>
>    def register(self, linux_distro):
>
>       self.distros.add(linux_distro)
>
>    def query(self, **kwargs):
>
>       ...
>
>
> registry = LinuxDistroRegistry()
>
> registry.register(LinuxDistro('fedora', '31', 'x86_64',
> 'deadbeefdeadbeef'))
>
> registry.register(LinuxDistro('fedora', '31', 'aarch64',
> 'beefdeadbeefdead'))
>
> checksum = registry.query(name='fedora', version='31',
> arch='x86_64').checksum
>
>
>> +        try:
>> +            self._info =
>> self.KNOWN_DISTROS.get(name).get(version).get(arch)
>
>
> The `AttributeError` that could be caught at the removed
> `get_known_distro_checksum()` function, could come from any of the
> `.get()`s returning `None`, which in turn would not have a `.get()`
> attribute.
>
> But now, if there's a "name", then a "version", but no "arch" entry,
> this line will set `self._info` to `None`.  This is manifested if you
> try to run a test that tries to find an aarch64 distro, such as:
>
> ./tests/venv/bin/avocado run
> tests/acceptance/boot_linux.py:BootLinuxAarch64.test_virt_tcg_gicv2
>
>
> It will result in:
>
>
> 20:38:18 ERROR| Reproduced traceback from:
> /var/lib/users/cleber/build/qemu/tests/venv/lib64/python3.9/site-packages/avocado/core/test.py:756
> 20:38:18 ERROR| Traceback (most recent call last):
> 20:38:18 ERROR|   File
> "/var/lib/users/cleber/build/qemu/tests/acceptance/avocado_qemu/__init__.py",
> line 426, in download_boot
> 20:38:18 ERROR|     checksum=self.distro.checksum,
> 20:38:18 ERROR|   File
> "/var/lib/users/cleber/build/qemu/tests/acceptance/avocado_qemu/__init__.py",
> line 334, in checksum
> 20:38:18 ERROR|     return self._info.get('checksum', None)
> 20:38:18 ERROR| AttributeError: 'NoneType' object has no attribute 'get'
> 20:38:18 ERROR|
> 20:38:18 ERROR| During handling of the above exception, another
> exception occurred:
> 20:38:18 ERROR|
> 20:38:18 ERROR| Traceback (most recent call last):
> 20:38:18 ERROR|   File
> "/var/lib/users/cleber/build/qemu/tests/acceptance/avocado_qemu/__init__.py",
> line 387, in setUp
> 20:38:18 ERROR|     self.set_up_boot()
> 20:38:18 ERROR|   File
> "/var/lib/users/cleber/build/qemu/tests/acceptance/avocado_qemu/__init__.py",
> line 455, in set_up_boot
> 20:38:18 ERROR|     path = self.download_boot()
> 20:38:18 ERROR|   File
> "/var/lib/users/cleber/build/qemu/tests/acceptance/avocado_qemu/__init__.py",
> line 431, in download_boot
> 20:38:18 ERROR|     self.cancel('Failed to download/prepare boot image')
> 20:38:18 ERROR|   File
> "/var/lib/users/cleber/build/qemu/tests/venv/lib64/python3.9/site-packages/avocado/core/test.py",
> line 988, in cancel
> 20:38:18 ERROR|     raise exceptions.TestCancel(message)
> 20:38:18 ERROR| avocado.core.exceptions.TestCancel: Failed to
> download/prepare boot image

I am not sufficiently expert on the test infra and python to be really
efficient fixing that. Can anyone help quickly to target the soft
freeze? Otherwise, today I will drop that patch and restore the code I
had in v4, just based on Cleber series. I think the refactoring can
happen later...

Thanks

Eric
>
>
> Cheers,
>
> - Cleber.
>

Re: [PATCH v5 1/4] avocado_qemu: Fix KNOWN_DISTROS map into the LinuxDistro class

[Cleber Rosa]

On 7/8/21 4:56 AM, Eric Auger wrote:
>
> I am not sufficiently expert on the test infra and python to be really
> efficient fixing that. Can anyone help quickly to target the soft
> freeze? Otherwise, today I will drop that patch and restore the code I
> had in v4, just based on Cleber series. I think the refactoring can
> happen later...


Hi Eric,


The following diff works for me:


diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index af93cd63ea..b3bed00062 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -310,6 +310,8 @@ class LinuxDistro:
              '31': {
                  'x86_64':
                  {'checksum': 
'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
+                'aarch64':
+                {'checksum': 
'1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'},
                  'ppc64':
                  {'checksum': 
'7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
                  's390x':
@@ -323,10 +325,11 @@ def __init__(self, name, version, arch):
          self.version = version
          self.arch = arch
          try:
-            self._info = 
self.KNOWN_DISTROS.get(name).get(version).get(arch)
+            info = self.KNOWN_DISTROS.get(name).get(version).get(arch)
          except AttributeError:
              # Unknown distro
-            self._info = {}
+            info = None
+        self._info = info or {}

      @property
      def checksum(self):


I've tested it with both existing and the newly introduced tests.


Cheers,

- Cleber.

Re: [PATCH v5 1/4] avocado_qemu: Fix KNOWN_DISTROS map into the LinuxDistro class

[Eric Auger]

Hi Cleber,

On 7/8/21 7:34 PM, Cleber Rosa wrote:
>
> On 7/8/21 4:56 AM, Eric Auger wrote:
>>
>> I am not sufficiently expert on the test infra and python to be really
>> efficient fixing that. Can anyone help quickly to target the soft
>> freeze? Otherwise, today I will drop that patch and restore the code I
>> had in v4, just based on Cleber series. I think the refactoring can
>> happen later...
>
>
> Hi Eric,
>
>
> The following diff works for me:
>
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py
> b/tests/acceptance/avocado_qemu/__init__.py
> index af93cd63ea..b3bed00062 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -310,6 +310,8 @@ class LinuxDistro:
>              '31': {
>                  'x86_64':
>                  {'checksum':
> 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
> +                'aarch64':
> +                {'checksum':
> '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'},
>                  'ppc64':
>                  {'checksum':
> '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
>                  's390x':
> @@ -323,10 +325,11 @@ def __init__(self, name, version, arch):
>          self.version = version
>          self.arch = arch
>          try:
> -            self._info =
> self.KNOWN_DISTROS.get(name).get(version).get(arch)
> +            info = self.KNOWN_DISTROS.get(name).get(version).get(arch)
>          except AttributeError:
>              # Unknown distro
> -            self._info = {}
> +            info = None
> +        self._info = info or {}
>
>      @property
>      def checksum(self):
>
>
> I've tested it with both existing and the newly introduced tests.

Thank you for the work! Do you plan to introduce it as a fixup or do I
need to respin?

Thanks

Eric
>
>
> Cheers,
>
> - Cleber.
>

Re: [PATCH v5 1/4] avocado_qemu: Fix KNOWN_DISTROS map into the LinuxDistro class

[Wainer dos Santos Moschetta]

On 7/8/21 2:34 PM, Cleber Rosa wrote:
>
> On 7/8/21 4:56 AM, Eric Auger wrote:
>>
>> I am not sufficiently expert on the test infra and python to be really
>> efficient fixing that. Can anyone help quickly to target the soft
>> freeze? Otherwise, today I will drop that patch and restore the code I
>> had in v4, just based on Cleber series. I think the refactoring can
>> happen later...
>
>
> Hi Eric,
>
>
> The following diff works for me:
>
>
> diff --git a/tests/acceptance/avocado_qemu/__init__.py 
> b/tests/acceptance/avocado_qemu/__init__.py
> index af93cd63ea..b3bed00062 100644
> --- a/tests/acceptance/avocado_qemu/__init__.py
> +++ b/tests/acceptance/avocado_qemu/__init__.py
> @@ -310,6 +310,8 @@ class LinuxDistro:
>              '31': {
>                  'x86_64':
>                  {'checksum': 
> 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
> +                'aarch64':
> +                {'checksum': 
> '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'},
>                  'ppc64':
>                  {'checksum': 
> '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
>                  's390x':
> @@ -323,10 +325,11 @@ def __init__(self, name, version, arch):
>          self.version = version
>          self.arch = arch
>          try:
> -            self._info = 
> self.KNOWN_DISTROS.get(name).get(version).get(arch)
> +            info = self.KNOWN_DISTROS.get(name).get(version).get(arch)
>          except AttributeError:
>              # Unknown distro
> -            self._info = {}
> +            info = None
> +        self._info = info or {}
>
>      @property
>      def checksum(self):
>
>

Thanks for that fix, Cleber.

Acked-by: Wainer dos Santos Moschetta <wainersm@redhat.com>


> I've tested it with both existing and the newly introduced tests.
>
>
> Cheers,
>
> - Cleber.
>

Re: [PATCH v5 4/4] avocado_qemu: Add Intel iommu tests

[Wainer dos Santos Moschetta]

On 7/6/21 10:17 AM, Eric Auger wrote:
> Add Intel IOMMU functional tests based on fedora 31.
> Different configs are checked:
> - strict
> - caching mode, strict
> - passthrough.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Willian Rampazzo <willianr@redhat.com>
> Acked-by: Peter Xu <peterx@redhat.com>
>
> ---
>
> v4 -> v5:
> - import cleanup (William)
> - IntelIOMMU becomes the class name (William)
> - Added William's Sob
> ---
>   tests/acceptance/intel_iommu.py | 115 ++++++++++++++++++++++++++++++++
>   1 file changed, 115 insertions(+)
>   create mode 100644 tests/acceptance/intel_iommu.py

I successfully ran this test.

Tested-by: Wainer dos Santos Moschetta <wainersm@redhat.com>

>
> diff --git a/tests/acceptance/intel_iommu.py b/tests/acceptance/intel_iommu.py
> new file mode 100644
> index 0000000000..ad10491d89
> --- /dev/null
> +++ b/tests/acceptance/intel_iommu.py
> @@ -0,0 +1,115 @@
> +# INTEL_IOMMU 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
> +
> +@skipIf(os.getenv('GITLAB_CI'), 'Running 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
> +    """
> +
> +    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('-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')
> +
> +    def common_vm_setup(self, custom_kernel=None):
> +        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'
> +        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('dmesg | grep -e DMAR -e IOMMU')
> +        self.ssh_command('find /sys/kernel/iommu_groups/ -type l')
> +        self.ssh_command('dnf -y install numactl-devel')
> +
> +    def test_intel_iommu(self):
> +        """
> +        :avocado: tags=intel_iommu_intremap
> +        """
> +
> +        self.common_vm_setup(True)
> +        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.run_and_check()
> +
> +    def test_intel_iommu_strict(self):
> +        """
> +        :avocado: tags=intel_iommu_strict
> +        """
> +
> +        self.common_vm_setup(True)
> +        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.run_and_check()
> +
> +    def test_intel_iommu_strict_cm(self):
> +        """
> +        :avocado: tags=intel_iommu_strict_cm
> +        """
> +
> +        self.common_vm_setup(True)
> +        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.run_and_check()
> +
> +    def test_intel_iommu_pt(self):
> +        """
> +        :avocado: tags=intel_iommu_pt
> +        """
> +
> +        self.common_vm_setup(True)
> +        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.run_and_check()

Re: [PATCH v5 1/4] avocado_qemu: Fix KNOWN_DISTROS map into the LinuxDistro class

[Cleber Rosa]

On 7/8/21 3:32 PM, Eric Auger wrote:
> Hi Cleber,
>
> On 7/8/21 7:34 PM, Cleber Rosa wrote:
>> On 7/8/21 4:56 AM, Eric Auger wrote:
>>> I am not sufficiently expert on the test infra and python to be really
>>> efficient fixing that. Can anyone help quickly to target the soft
>>> freeze? Otherwise, today I will drop that patch and restore the code I
>>> had in v4, just based on Cleber series. I think the refactoring can
>>> happen later...
>>
>> Hi Eric,
>>
>>
>> The following diff works for me:
>>
>>
>> diff --git a/tests/acceptance/avocado_qemu/__init__.py
>> b/tests/acceptance/avocado_qemu/__init__.py
>> index af93cd63ea..b3bed00062 100644
>> --- a/tests/acceptance/avocado_qemu/__init__.py
>> +++ b/tests/acceptance/avocado_qemu/__init__.py
>> @@ -310,6 +310,8 @@ class LinuxDistro:
>>               '31': {
>>                   'x86_64':
>>                   {'checksum':
>> 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0'},
>> +                'aarch64':
>> +                {'checksum':
>> '1e18d9c0cf734940c4b5d5ec592facaed2af0ad0329383d5639c997fdf16fe49'},
>>                   'ppc64':
>>                   {'checksum':
>> '7c3528b85a3df4b2306e892199a9e1e43f991c506f2cc390dc4efa2026ad2f58'},
>>                   's390x':
>> @@ -323,10 +325,11 @@ def __init__(self, name, version, arch):
>>           self.version = version
>>           self.arch = arch
>>           try:
>> -            self._info =
>> self.KNOWN_DISTROS.get(name).get(version).get(arch)
>> +            info = self.KNOWN_DISTROS.get(name).get(version).get(arch)
>>           except AttributeError:
>>               # Unknown distro
>> -            self._info = {}
>> +            info = None
>> +        self._info = info or {}
>>
>>       @property
>>       def checksum(self):
>>
>>
>> I've tested it with both existing and the newly introduced tests.
> Thank you for the work! Do you plan to introduce it as a fixup or do I
> need to respin?


Hi Eric,


Yes, I can add it as a fixup.


Thanks,

- Cleber.