SRIOV: add new test case of dma_translation for vIOMMU #5937

meinaLi · 2024-10-11T06:02:38Z

Automate new dma_translation attribute for vIOMMU:
VIRT-302220 - [vIOMMU][intel] Start VM with intel iommu and dma_translation attribute

meinaLi · 2024-10-11T06:03:00Z

# avocado run --vt-type libvirt --test-runner=runner --vt-machine-type q35 vIOMMU.iommu_device_settings..intel
JOB ID     : 758b6911ec2381a2f01846ed9ab84c0bae470a4f
JOB LOG    : /var/log/avocado/job-results/job-2024-10-10T22.41-758b691/job.log
 (01/16) type_specific.io-github-autotest-libvirt.vIOMMU.iommu_device_settings.e1000e.virtio:PASS (40.62 s)
 (02/16) type_specific.io-github-autotest-libvirt.vIOMMU.iommu_device_settings.e1000e.intel: PASS (165.87 s)
 (03/16) type_specific.io-github-autotest-libvirt.vIOMMU.iommu_device_settings.rtl8139.virtio: PASS (41.68 s)
......
 (16/16) type_specific.io-github-autotest-libvirt.vIOMMU.iommu_device_settings.pcie_root_port_from_expander_bus.intel: PASS (276.34 s)
Not opening readers, lock_server_running not locked
Not opening readers, lock_server_running not locked
RESULTS    : PASS 16 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /var/log/avocado/job-results/job-2024-10-10T22.41-758b691/results.html
JOB TIME   : 1900.18 s

# avocado run --vt-type libvirt --test-runner=runner --vt-machine-type q35 vIOMMU.attach_iommu_device..intel
JOB ID     : bb1c851cd27ddcfecb9357d64bf30ba07b033cb3
JOB LOG    : /var/log/avocado/job-results/job-2024-10-10T23.28-bb1c851/job.log
 (1/4) type_specific.io-github-autotest-libvirt.vIOMMU.attach_iommu_device.cold_plug.virtio: PASS (8.14 s)
 (2/4) type_specific.io-github-autotest-libvirt.vIOMMU.attach_iommu_device.cold_plug.intel: PASS (8.45 s)
 (3/4) type_specific.io-github-autotest-libvirt.vIOMMU.attach_iommu_device.hot_plug.virtio: PASS (28.88 s)
 (4/4) type_specific.io-github-autotest-libvirt.vIOMMU.attach_iommu_device.hot_plug.intel: PASS (37.56 s)
RESULTS    : PASS 4 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /var/log/avocado/job-results/job-2024-10-10T23.28-bb1c851/results.html
JOB TIME   : 84.85 s

chunfuwen · 2024-10-15T08:32:55Z

libvirt/tests/src/sriov/vIOMMU/attach_iommu_device.py

@@ -33,6 +33,8 @@ def run(test, params, env):
                vmxml.features = features
                vmxml.sync()
            err_msg = ''
+        if libvirt_version.version_compare(10, 7, 0) and iommu_dict['model'] == 'intel':
+            iommu_dict['driver']['dma_translation'] = 'on'


dma_translation is new feature, so maybe create separate case under intel variant of ./libvirt/tests/cfg/sriov/vIOMMU/attach_iommu_device.cfg , otherwise it is easy to ignore this feature if still under previous test case.

I've considered whether we need to add a new case. But after the evaluation I gave up because:

Keep the same attributes strategy as before in plan.

This new attribute is not so important because before it was enabled default.
Certainly it can also reduce our debug workloads.

chunfuwen

lgtm

meinaLi · 2024-10-23T07:03:28Z

@Yingshun Could you help review this PR? Thanks.

Yingshun

As discussed offline, it's better to add it to intel iommu cases or create a new one.

meinaLi · 2024-10-31T06:28:02Z

# avocado run --vt-type libvirt --test-runner=runner --vt-machine-type q35 vIOMMU.intel_iommu_with_dma_translation
JOB ID     : d23c0f2bac019be9d0c4153cb80a230cb4250c40
JOB LOG    : /var/lib/avocado/job-results/job-2024-10-31T01.29-d23c0f2/job.log
 (1/3) type_specific.io-github-autotest-libvirt.vIOMMU.intel_iommu_with_dma_translation.enable_dma_translation: PASS (209.04 s)
 (2/3) type_specific.io-github-autotest-libvirt.vIOMMU.intel_iommu_with_dma_translation.disable_dma_translation: PASS (208.03 s)
 (3/3) type_specific.io-github-autotest-libvirt.vIOMMU.intel_iommu_with_dma_translation.disable_dma_translation_with_more_vcpus: PASS (841.30 s)
Not opening readers, lock_server_running not locked
Not opening readers, lock_server_running not locked
RESULTS    : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /var/lib/avocado/job-results/job-2024-10-31T01.29-d23c0f2/results.html
JOB TIME   : 1259.86 s

Yingshun · 2024-11-01T10:48:44Z

libvirt/tests/src/sriov/vIOMMU/intel_iommu_with_dma_translation.py

+    iommu_dict = eval(params.get("iommu_dict", "{}") % dma_translation)
+    with_more_vcpus = "yes" == params.get("with_more_vcpus", "no")
+    test_obj = viommu_base.VIOMMUTest(vm, test, params)
+    libvirt_version.is_libvirt_feature_supported(params)


It's covered in viommu_base.VIOMMUTest(), plz remove it.

Removed it now.

Yingshun · 2024-11-01T10:53:05Z

libvirt/tests/cfg/sriov/vIOMMU/intel_iommu_with_dma_translation.cfg

+    type = intel_iommu_with_dma_translation
+    start_vm = "yes"
+    enable_guest_iommu = "yes"
+    iommu_dict = {'model': 'intel', 'driver': {'intremap': 'on', 'caching_mode': 'on', 'iotlb': 'on', 'dma_translation': '%s'}}


I think it's better to move this line to the end of this file and use the variant 'dma_translation' to show the iommu settings for each tests:
iommu_dict = {'model': 'intel', 'driver': {'intremap': 'on', 'caching_mode': 'on', 'iotlb': 'on', 'dma_translation': 'dma_translation'}}

Yes, it can also be at the end of this file.

Yingshun · 2024-11-01T11:00:32Z

libvirt/tests/src/sriov/vIOMMU/intel_iommu_with_dma_translation.py

+        iommu_dict['driver'].update(eim_dict)
+        test.log.info("TEST STEP1: Prepare guest xml with intel iommu device.")
+        test_obj.setup_iommu_test(iommu_dict=iommu_dict)
+        vmxml = vm_xml.VMXML.new_from_dumpxml(vm_name)


It's only used for L27-29, I think it can be moved into the if statement.

Moved it now.

Yingshun · 2024-11-01T11:02:42Z

libvirt/tests/src/sriov/vIOMMU/intel_iommu_with_dma_translation.py

+            vmxml.vcpu = int(guest_vcpus)
+            vmxml.sync()
+        test.log.info("TEST STEP2: Start the guest.")
+        virsh.start(vm_name, debug=True)


It's better to use vm.start which will raise exceptions if the startup fails.

Changed to use vm.start().

Yingshun · 2024-11-01T11:04:35Z

libvirt/tests/src/sriov/vIOMMU/intel_iommu_with_dma_translation.py

+        test.log.info("TEST STEP3: Check the message for iommu group and DMA.")
+        vm.cleanup_serial_console()
+        vm.create_serial_console()
+        vm_session = vm.wait_for_serial_login(timeout=6000)


Why do we expect the timeout to be so long? 6000s is too long in my opinion, what do you think?

Because when we start guest with 256 vCPUs, it may take long time to login the guest. Based on this scenario, I extended the timeout.

Yingshun · 2024-11-01T11:13:05Z

libvirt/tests/src/sriov/vIOMMU/intel_iommu_with_dma_translation.py

+        if dma_translation == "on" and (not dma_status or iommu_status):
+            test.fail("Can't get expected message result when enabling dma_translation.")
+        if dma_translation == "off" and (dma_status or not iommu_status):
+            test.fail("Can't get expected message result when disabling dma_translation.")


It's better to log the outputs of above 2 shell commands - 'dmesg | xxx'

Updated now.

Yingshun · 2024-11-01T11:13:54Z

libvirt/tests/src/sriov/vIOMMU/intel_iommu_with_dma_translation.py

+        if with_more_vcpus:
+            s, o = vm_session.cmd_status_output("cat /proc/cpuinfo | grep processor| wc -l")
+            if int(o) != int(guest_vcpus):
+                test.fail("Can't get expected vCPU number, the actual number is %s" % type(o))


type(o) to 'o'?

Yes, just a mistake in my debugging. I forgot to restore it before.

Automate new dma_translation attribute for vIOMMU: VIRT-302220 - [vIOMMU][intel] Start VM with intel iommu and dma_translation attribute Signed-off-by: Meina Li <[email protected]>

chunfuwen reviewed Oct 15, 2024

View reviewed changes

chunfuwen approved these changes Oct 23, 2024

View reviewed changes

Yingshun requested changes Oct 25, 2024

View reviewed changes

meinaLi force-pushed the viommu branch 2 times, most recently from 2c4386a to c718500 Compare October 31, 2024 06:09

meinaLi changed the title ~~SRIOV: add new test scenarios for vIOMMU~~ SRIOV: add new test case of dma_translation for vIOMMU Oct 31, 2024

Yingshun reviewed Nov 1, 2024

View reviewed changes

meinaLi force-pushed the viommu branch 2 times, most recently from 14ecc69 to 282b647 Compare November 5, 2024 02:55

meinaLi requested a review from Yingshun November 5, 2024 03:04

meinaLi force-pushed the viommu branch from 282b647 to 9e5bf64 Compare November 7, 2024 05:50

SRIOV: add new test case of dma_translation for vIOMMU

644e55c

Automate new dma_translation attribute for vIOMMU: VIRT-302220 - [vIOMMU][intel] Start VM with intel iommu and dma_translation attribute Signed-off-by: Meina Li <[email protected]>

meinaLi force-pushed the viommu branch from 9e5bf64 to 644e55c Compare November 7, 2024 05:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SRIOV: add new test case of dma_translation for vIOMMU #5937

SRIOV: add new test case of dma_translation for vIOMMU #5937

meinaLi commented Oct 11, 2024 •

edited

Loading

meinaLi commented Oct 11, 2024

chunfuwen Oct 15, 2024

meinaLi Oct 22, 2024

chunfuwen left a comment

meinaLi commented Oct 23, 2024

Yingshun left a comment

meinaLi commented Oct 31, 2024

Yingshun Nov 1, 2024

meinaLi Nov 5, 2024

Yingshun Nov 1, 2024

meinaLi Nov 5, 2024

Yingshun Nov 1, 2024

meinaLi Nov 5, 2024

Yingshun Nov 1, 2024

meinaLi Nov 5, 2024

Yingshun Nov 1, 2024

meinaLi Nov 5, 2024

Yingshun Nov 1, 2024

meinaLi Nov 5, 2024

Yingshun Nov 1, 2024

meinaLi Nov 5, 2024

SRIOV: add new test case of dma_translation for vIOMMU #5937

Are you sure you want to change the base?

SRIOV: add new test case of dma_translation for vIOMMU #5937

Conversation

meinaLi commented Oct 11, 2024 • edited Loading

meinaLi commented Oct 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chunfuwen left a comment

Choose a reason for hiding this comment

meinaLi commented Oct 23, 2024

Yingshun left a comment

Choose a reason for hiding this comment

meinaLi commented Oct 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meinaLi commented Oct 11, 2024 •

edited

Loading