[PATCH v2 0/3] Enable avocado testing for Xen guests

[PATCH v2 0/3] Enable avocado testing for Xen guests

[David Woodhouse]

I'd love to test other operating systems too (FreeBSD does work if I 
abuse the stuff in tests/vm/ and add a Xen guest mode). But this is
enough for now to help catch regressions on the QEMU side.

Especially the polling for the event channel GSI deassertion is likely 
to be fragile, and in the not-so-distant future I really do want to add 
an explicit EOI hook in the interrupt controllers; as discussed, VFIO 
INTx would benefit from that too.

When the guest kernel boots with 4 CPUs, it ends up initially targeting 
the event channel GSI to CPU#2, and then testing the migration.

Since nobody evidently cared about the I/OAPIC migration bug in the 7
years since it was introduced with the first split-irqchip support,
that fix might as well stay here in this series.

v2: https://git.infradead.org/users/dwmw2/qemu.git/shortlog/refs/heads/xenfv-test-2

 • Use xen-disk now that the PR adding support for that has been submitted

 • Include the I/O APIC migration fix, so that we can use -smp 4 

v1: https://lore.kernel.org/qemu-devel/20230303125126.1269861-1-dwmw2@infradead.org/
    https://git.infradead.org/users/dwmw2/qemu.git/shortlog/refs/heads/xenfv-test

David Woodhouse (3):
      hw/intc/ioapic: Update KVM routes before redelivering IRQ, on RTE update
      tests/avocado: Add Fedora 34 distro, including kernel/initrd checksums
      tests/avocado: Test Xen guest support under KVM

 hw/intc/ioapic.c                       |   3 +-
 tests/avocado/avocado_qemu/__init__.py |  27 ++++++
 tests/avocado/xen_guest.py             | 147 +++++++++++++++++++++++++++++++++
 3 files changed, 175 insertions(+), 2 deletions(-)
 create mode 100644 tests/avocado/xen_guest.py

[PATCH v2 1/3] hw/intc/ioapic: Update KVM routes before redelivering IRQ, on RTE update

[David Woodhouse]

A Linux guest will perform IRQ migration after the IRQ has happened,
updating the RTE to point to the new destination CPU and then unmasking
the interrupt.

However, when the guest updates the RTE, ioapic_mem_write() calls
ioapic_service(), which redelivers the pending level interrupt via
kvm_set_irq(), *before* calling ioapic_update_kvm_routes() which sets
the new target CPU.

Thus, the IRQ which is supposed to go to the new target CPU is instead
misdelivered to the previous target. An example where the guest kernel
is attempting to migrate from CPU#2 to CPU#0 shows:

xenstore_read tx 0 path control/platform-feature-xs_reset_watches
ioapic_set_irq vector: 11 level: 1
ioapic_set_remote_irr set remote irr for pin 11
ioapic_service: trigger KVM IRQ 11
[    0.523627] The affinity mask was 0-3 and the handler is on 2
ioapic_mem_write ioapic mem write addr 0x0 regsel: 0x27 size 0x4 val 0x26
ioapic_update_kvm_routes: update KVM route for IRQ 11: fee02000 8021
ioapic_mem_write ioapic mem write addr 0x10 regsel: 0x26 size 0x4 val 0x18021
xenstore_reset_watches
ioapic_set_irq vector: 11 level: 1
ioapic_mem_read ioapic mem read addr 0x10 regsel: 0x26 size 0x4 retval 0x1c021
[    0.524569] ioapic_ack_level IRQ 11 moveit = 1
ioapic_eoi_broadcast EOI broadcast for vector 33
ioapic_clear_remote_irr clear remote irr for pin 11 vector 33
ioapic_mem_write ioapic mem write addr 0x0 regsel: 0x26 size 0x4 val 0x26
ioapic_mem_read ioapic mem read addr 0x10 regsel: 0x26 size 0x4 retval 0x18021
[    0.525235] ioapic_finish_move IRQ 11 calls irq_move_masked_irq()
[    0.526147] irq_do_set_affinity for IRQ 11, 0
[    0.526732] ioapic_set_affinity for IRQ 11, 0
[    0.527330] ioapic_setup_msg_from_msi for IRQ11 target 0
ioapic_mem_write ioapic mem write addr 0x0 regsel: 0x26 size 0x4 val 0x27
ioapic_mem_write ioapic mem write addr 0x10 regsel: 0x27 size 0x4 val 0x0
ioapic_mem_write ioapic mem write addr 0x0 regsel: 0x27 size 0x4 val 0x26
ioapic_mem_write ioapic mem write addr 0x10 regsel: 0x26 size 0x4 val 0x18021
[    0.527623] ioapic_set_affinity returns 0
[    0.527623] ioapic_finish_move IRQ 11 calls unmask_ioapic_irq()
ioapic_mem_write ioapic mem write addr 0x0 regsel: 0x26 size 0x4 val 0x26
ioapic_mem_write ioapic mem write addr 0x10 regsel: 0x26 size 0x4 val 0x8021
ioapic_set_remote_irr set remote irr for pin 11
ioapic_service: trigger KVM IRQ 11
ioapic_update_kvm_routes: update KVM route for IRQ 11: fee00000 8021
[    0.529571] The affinity mask was 0 and the handler is on 2
[    xenstore_watch path memory/target token FFFFFFFF92847D40

There are no other code paths in ioapic_mem_write() which need the KVM
IRQ routing table to be updated, so just shift the call from the end
of the function to happen right before the call to ioapic_service()
and thus deliver the re-enabled IRQ to the right place.

Alternative fixes might have been just to remove the part in
ioapic_service() which delivers the IRQ via kvm_set_irq() because
surely delivering as MSI ought to work just fine anyway in all cases?
That code lacks a comment justifying its existence.

Or maybe in the specific case shown in the above log, it would have
sufficed for ioapic_update_kvm_routes() to update the route *even*
when the IRQ is masked. It's not like it's actually going to get
triggered unless QEMU deliberately does so, anyway? But that only
works because the target CPU happens to be in the high word of the
RTE; if something in the *low* word (vector, perhaps) was changed
at the same time as the unmask, we'd still trigger with stale data.

Fixes: 15eafc2e602f "kvm: x86: add support for KVM_CAP_SPLIT_IRQCHIP"
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
---
 hw/intc/ioapic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 6364ecab1b..716ffc8bbb 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -405,6 +405,7 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val,
                 s->ioredtbl[index] |= ro_bits;
                 s->irq_eoi[index] = 0;
                 ioapic_fix_edge_remote_irr(&s->ioredtbl[index]);
+                ioapic_update_kvm_routes(s);
                 ioapic_service(s);
             }
         }
@@ -417,8 +418,6 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val,
         ioapic_eoi_broadcast(val);
         break;
     }
-
-    ioapic_update_kvm_routes(s);
 }
 
 static const MemoryRegionOps ioapic_io_ops = {
-- 
2.39.0

[PATCH v2 2/3] tests/avocado: Add Fedora 34 distro, including kernel/initrd checksums

[David Woodhouse]

From: David Woodhouse <dwmw@amazon.co.uk>

The kernel in Fedora 31 doesn't support 'xen_no_vector_callback' on
its command line, so add a slightly newer version as a prelude to
enabling avocado tests for Xen guests.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 tests/avocado/avocado_qemu/__init__.py | 27 ++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
index a313e88c07..49e414e267 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -485,6 +485,23 @@ class LinuxDistro:
                                   ' console=tty0'),
                  },
             },
+            '34': {
+                'x86_64':
+                {'checksum': ('b9b621b26725ba95442d9a56cbaa0547'
+                              '84e0779a9522ec6eafff07c6e6f717ea'),
+                 'pxeboot_url': ('https://archives.fedoraproject.org/'
+                                 'pub/archive/fedora/linux/releases/34/'
+                                 'Everything/x86_64/os/images/pxeboot/'),
+                 'kernel_hash': ('085fc6e47f2e3a271b591f3e56739ca9'
+                                 '4c16718837a5f431ab95468e1e95f9eb'),
+                 'initrd_hash': ('d6cd2e03e8188eed6c896fd65ff05f81'
+                                 '2c4c1c8777d630b5909e9a1a4627e337'),
+                 'kernel_params': ('root=UUID=386769a3-cfa5-47c8-8797-'
+                                   'd5ec58c9cb6c ro no_timer_check '
+                                   'net.ifnames=0 console=tty1 '
+                                   'console=ttyS0,115200n8'),
+                },
+            },
         }
     }
 
@@ -513,6 +530,16 @@ def pxeboot_url(self):
         """Gets the repository url where pxeboot files can be found"""
         return self._info.get('pxeboot_url', None)
 
+    @property
+    def kernel_hash(self):
+        """Gets checksum of the pxeboot kernel image"""
+        return self._info.get('kernel_hash', None)
+
+    @property
+    def initrd_hash(self):
+        """Gets checksum of the pxeboot initrd image"""
+        return self._info.get('initrd_hash', None)
+
     @property
     def default_kernel_params(self):
         """Gets the default kernel parameters"""
-- 
2.39.0

[PATCH v2 3/3] tests/avocado: Test Xen guest support under KVM

[David Woodhouse]

From: David Woodhouse <dwmw@amazon.co.uk>

Exercise guests with a few different modes for interrupt delivery. In
particular we want to cover:

 • Xen event channel delivery via GSI to the I/O APIC
 • Xen event channel delivery via GSI to the i8259 PIC
 • MSIs routed to PIRQ event channels
 • GSIs routed to PIRQ event channels

As well as some variants of normal non-Xen stuff like MSI to vAPIC and
PCI INTx going to the I/O APIC and PIC, which ought to still work even
in Xen mode.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 tests/avocado/xen_guest.py | 147 +++++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)
 create mode 100644 tests/avocado/xen_guest.py

diff --git a/tests/avocado/xen_guest.py b/tests/avocado/xen_guest.py
new file mode 100644
index 0000000000..c50b52958f
--- /dev/null
+++ b/tests/avocado/xen_guest.py
@@ -0,0 +1,147 @@
+# Xen guest functional tests
+#
+# Copyright © 2021 Red Hat, Inc.
+# Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Author:
+#  David Woodhouse <dwmw2@infradead.org>
+#
+# 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 XenGuest(LinuxTest):
+    """
+    :avocado: tags=arch:x86_64
+    :avocado: tags=distro:fedora
+    :avocado: tags=distro_version:34
+    :avocado: tags=machine:q35
+    :avocado: tags=accel:kvm
+    :avocado: tags=xen_guest
+    """
+
+    kernel_path = None
+    initrd_path = None
+    kernel_params = None
+
+    def set_up_boot(self):
+        path = self.download_boot()
+        self.vm.add_args('-drive', 'file=%s,if=none,id=drv0' % path)
+        self.vm.add_args('-device', 'xen-disk,drive=drv0,vdev=xvda')
+
+    def setUp(self):
+        super(XenGuest, self).setUp(None, 'virtio-net-pci')
+
+    def common_vm_setup(self, custom_kernel=None):
+        self.require_accelerator("kvm")
+        self.vm.add_args("-accel", "kvm,xen-version=0x4000a,kernel-irqchip=split")
+        self.vm.add_args("-smp", "4")
+
+        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, algorithm='sha256',
+                                            asset_hash=self.distro.kernel_hash)
+        self.initrd_path = self.fetch_asset(initrd_url, algorithm='sha256',
+                                            asset_hash=self.distro.initrd_hash)
+
+    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 "Grant table initialized"')
+
+    def test_xen_guest(self):
+        """
+        :avocado: tags=xen_guest
+        """
+
+        self.common_vm_setup(True)
+
+        self.kernel_params = (self.distro.default_kernel_params +
+                              ' xen_emul_unplug=ide-disks')
+        self.run_and_check()
+        self.ssh_command('grep xen-pirq.*msi /proc/interrupts')
+
+    def test_xen_guest_nomsi(self):
+        """
+        :avocado: tags=xen_guest_nomsi
+        """
+
+        self.common_vm_setup(True)
+
+        self.kernel_params = (self.distro.default_kernel_params +
+                              ' xen_emul_unplug=ide-disks pci=nomsi')
+        self.run_and_check()
+        self.ssh_command('grep xen-pirq.* /proc/interrupts')
+
+    def test_xen_guest_noapic_nomsi(self):
+        """
+        :avocado: tags=xen_guest_noapic_nomsi
+        """
+
+        self.common_vm_setup(True)
+
+        self.kernel_params = (self.distro.default_kernel_params +
+                              ' xen_emul_unplug=ide-disks noapic pci=nomsi')
+        self.run_and_check()
+        self.ssh_command('grep xen-pirq /proc/interrupts')
+
+    def test_xen_guest_vapic(self):
+        """
+        :avocado: tags=xen_guest_vapic
+        """
+
+        self.common_vm_setup(True)
+        self.vm.add_args('-cpu', 'host,+xen-vapic')
+        self.kernel_params = (self.distro.default_kernel_params +
+                              ' xen_emul_unplug=ide-disks')
+        self.run_and_check()
+        self.ssh_command('grep xen-pirq /proc/interrupts')
+        self.ssh_command('grep PCI-MSI /proc/interrupts')
+
+    def test_xen_guest_novector(self):
+        """
+        :avocado: tags=xen_guest_novector
+        """
+
+        self.common_vm_setup(True)
+        self.kernel_params = (self.distro.default_kernel_params +
+                              ' xen_emul_unplug=ide-disks' +
+                              ' xen_no_vector_callback')
+        self.run_and_check()
+        self.ssh_command('grep xen-platform-pci /proc/interrupts')
+
+    def test_xen_guest_novector_nomsi(self):
+        """
+        :avocado: tags=xen_guest_novector_nomsi
+        """
+
+        self.common_vm_setup(True)
+
+        self.kernel_params = (self.distro.default_kernel_params +
+                              ' xen_emul_unplug=ide-disks pci=nomsi' +
+                              ' xen_no_vector_callback')
+        self.run_and_check()
+        self.ssh_command('grep xen-platform-pci /proc/interrupts')
+
+    def test_xen_guest_novector_noapic(self):
+        """
+        :avocado: tags=xen_guest_novector_noapic
+        """
+
+        self.common_vm_setup(True)
+        self.kernel_params = (self.distro.default_kernel_params +
+                              ' xen_emul_unplug=ide-disks' +
+                              ' xen_no_vector_callback noapic')
+        self.run_and_check()
+        self.ssh_command('grep xen-platform-pci /proc/interrupts')
-- 
2.39.0

Re: [PATCH v2 2/3] tests/avocado: Add Fedora 34 distro, including kernel/initrd checksums

[Alex Bennée]

David Woodhouse <dwmw2@infradead.org> writes:

> From: David Woodhouse <dwmw@amazon.co.uk>
>
> The kernel in Fedora 31 doesn't support 'xen_no_vector_callback' on
> its command line, so add a slightly newer version as a prelude to
> enabling avocado tests for Xen guests.

Why slightly newer rather than current release?

Our existing Fedora guest builds cause all sorts of timeout issues
running under TCG and seem particularly heavyweight considering the
coverage we manage to get.

>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  tests/avocado/avocado_qemu/__init__.py | 27 ++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/tests/avocado/avocado_qemu/__init__.py b/tests/avocado/avocado_qemu/__init__.py
> index a313e88c07..49e414e267 100644
> --- a/tests/avocado/avocado_qemu/__init__.py
> +++ b/tests/avocado/avocado_qemu/__init__.py
> @@ -485,6 +485,23 @@ class LinuxDistro:
>                                    ' console=tty0'),
>                   },
>              },
> +            '34': {
> +                'x86_64':
> +                {'checksum': ('b9b621b26725ba95442d9a56cbaa0547'
> +                              '84e0779a9522ec6eafff07c6e6f717ea'),
> +                 'pxeboot_url': ('https://archives.fedoraproject.org/'
> +                                 'pub/archive/fedora/linux/releases/34/'
> +                                 'Everything/x86_64/os/images/pxeboot/'),
> +                 'kernel_hash': ('085fc6e47f2e3a271b591f3e56739ca9'
> +                                 '4c16718837a5f431ab95468e1e95f9eb'),
> +                 'initrd_hash': ('d6cd2e03e8188eed6c896fd65ff05f81'
> +                                 '2c4c1c8777d630b5909e9a1a4627e337'),
> +                 'kernel_params': ('root=UUID=386769a3-cfa5-47c8-8797-'
> +                                   'd5ec58c9cb6c ro no_timer_check '
> +                                   'net.ifnames=0 console=tty1 '
> +                                   'console=ttyS0,115200n8'),
> +                },
> +            },
>          }
>      }
>  
> @@ -513,6 +530,16 @@ def pxeboot_url(self):
>          """Gets the repository url where pxeboot files can be found"""
>          return self._info.get('pxeboot_url', None)
>  
> +    @property
> +    def kernel_hash(self):
> +        """Gets checksum of the pxeboot kernel image"""
> +        return self._info.get('kernel_hash', None)
> +
> +    @property
> +    def initrd_hash(self):
> +        """Gets checksum of the pxeboot initrd image"""
> +        return self._info.get('initrd_hash', None)
> +
>      @property
>      def default_kernel_params(self):
>          """Gets the default kernel parameters"""


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v2 3/3] tests/avocado: Test Xen guest support under KVM

[Alex Bennée]

David Woodhouse <dwmw2@infradead.org> writes:

> From: David Woodhouse <dwmw@amazon.co.uk>
>
> Exercise guests with a few different modes for interrupt delivery. In
> particular we want to cover:
>
>  • Xen event channel delivery via GSI to the I/O APIC
>  • Xen event channel delivery via GSI to the i8259 PIC
>  • MSIs routed to PIRQ event channels
>  • GSIs routed to PIRQ event channels
>
> As well as some variants of normal non-Xen stuff like MSI to vAPIC and
> PCI INTx going to the I/O APIC and PIC, which ought to still work even
> in Xen mode.
>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  tests/avocado/xen_guest.py | 147 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 147 insertions(+)
>  create mode 100644 tests/avocado/xen_guest.py
>
> diff --git a/tests/avocado/xen_guest.py b/tests/avocado/xen_guest.py
> new file mode 100644
> index 0000000000..c50b52958f
> --- /dev/null
> +++ b/tests/avocado/xen_guest.py
> @@ -0,0 +1,147 @@
> +# Xen guest functional tests
> +#

Perhaps to avoid confusion we should call the file kvm_xen_guests.py and
make it clearer in the comment this is the mode we are talking about.
c.f. the boot_xen.py tests which boot Xen (under TCG emulation) as a
proper hypervisor.

> +# Copyright © 2021 Red Hat, Inc.
> +# Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
> +#
> +# Author:
> +#  David Woodhouse <dwmw2@infradead.org>
> +#
> +# 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')

This just means we will never have CI coverage. In theory when running
on the GitLab shared runners the test should skip anyway when it can't
find /dev/kvm and hopefully can run on our custom runners when it can
see /dev/kvm.

> +class XenGuest(LinuxTest):
> +    """
> +    :avocado: tags=arch:x86_64
> +    :avocado: tags=distro:fedora
> +    :avocado: tags=distro_version:34
> +    :avocado: tags=machine:q35
> +    :avocado: tags=accel:kvm
> +    :avocado: tags=xen_guest
> +    """
> +
> +    kernel_path = None
> +    initrd_path = None
> +    kernel_params = None
> +
> +    def set_up_boot(self):
> +        path = self.download_boot()
> +        self.vm.add_args('-drive', 'file=%s,if=none,id=drv0' % path)
> +        self.vm.add_args('-device', 'xen-disk,drive=drv0,vdev=xvda')
> +
> +    def setUp(self):
> +        super(XenGuest, self).setUp(None, 'virtio-net-pci')
> +
> +    def common_vm_setup(self, custom_kernel=None):
> +        self.require_accelerator("kvm")
> +        self.vm.add_args("-accel", "kvm,xen-version=0x4000a,kernel-irqchip=split")
> +        self.vm.add_args("-smp", "4")
> +
> +        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, algorithm='sha256',
> +                                            asset_hash=self.distro.kernel_hash)
> +        self.initrd_path = self.fetch_asset(initrd_url, algorithm='sha256',
> +                                            asset_hash=self.distro.initrd_hash)
> +
> +    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 "Grant table initialized"')
> +
> +    def test_xen_guest(self):
> +        """
> +        :avocado: tags=xen_guest
> +        """
> +
> +        self.common_vm_setup(True)
> +
> +        self.kernel_params = (self.distro.default_kernel_params +
> +                              ' xen_emul_unplug=ide-disks')
> +        self.run_and_check()
> +        self.ssh_command('grep xen-pirq.*msi /proc/interrupts')
> +
> +    def test_xen_guest_nomsi(self):
> +        """
> +        :avocado: tags=xen_guest_nomsi
> +        """
> +
> +        self.common_vm_setup(True)
> +
> +        self.kernel_params = (self.distro.default_kernel_params +
> +                              ' xen_emul_unplug=ide-disks pci=nomsi')
> +        self.run_and_check()
> +        self.ssh_command('grep xen-pirq.* /proc/interrupts')
> +
> +    def test_xen_guest_noapic_nomsi(self):
> +        """
> +        :avocado: tags=xen_guest_noapic_nomsi
> +        """
> +
> +        self.common_vm_setup(True)
> +
> +        self.kernel_params = (self.distro.default_kernel_params +
> +                              ' xen_emul_unplug=ide-disks noapic pci=nomsi')
> +        self.run_and_check()
> +        self.ssh_command('grep xen-pirq /proc/interrupts')
> +
> +    def test_xen_guest_vapic(self):
> +        """
> +        :avocado: tags=xen_guest_vapic
> +        """
> +
> +        self.common_vm_setup(True)
> +        self.vm.add_args('-cpu', 'host,+xen-vapic')
> +        self.kernel_params = (self.distro.default_kernel_params +
> +                              ' xen_emul_unplug=ide-disks')
> +        self.run_and_check()
> +        self.ssh_command('grep xen-pirq /proc/interrupts')
> +        self.ssh_command('grep PCI-MSI /proc/interrupts')
> +
> +    def test_xen_guest_novector(self):
> +        """
> +        :avocado: tags=xen_guest_novector
> +        """
> +
> +        self.common_vm_setup(True)
> +        self.kernel_params = (self.distro.default_kernel_params +
> +                              ' xen_emul_unplug=ide-disks' +
> +                              ' xen_no_vector_callback')
> +        self.run_and_check()
> +        self.ssh_command('grep xen-platform-pci /proc/interrupts')
> +
> +    def test_xen_guest_novector_nomsi(self):
> +        """
> +        :avocado: tags=xen_guest_novector_nomsi
> +        """
> +
> +        self.common_vm_setup(True)
> +
> +        self.kernel_params = (self.distro.default_kernel_params +
> +                              ' xen_emul_unplug=ide-disks pci=nomsi' +
> +                              ' xen_no_vector_callback')
> +        self.run_and_check()
> +        self.ssh_command('grep xen-platform-pci /proc/interrupts')
> +
> +    def test_xen_guest_novector_noapic(self):
> +        """
> +        :avocado: tags=xen_guest_novector_noapic
> +        """
> +
> +        self.common_vm_setup(True)
> +        self.kernel_params = (self.distro.default_kernel_params +
> +                              ' xen_emul_unplug=ide-disks' +
> +                              ' xen_no_vector_callback noapic')
> +        self.run_and_check()
> +        self.ssh_command('grep xen-platform-pci /proc/interrupts')


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v2 3/3] tests/avocado: Test Xen guest support under KVM

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2331 bytes --]

On Wed, 2023-03-08 at 17:45 +0000, Alex Bennée wrote:
> 
> David Woodhouse <dwmw2@infradead.org> writes:
> 
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > Exercise guests with a few different modes for interrupt delivery. In
> > particular we want to cover:
> > 
> >  • Xen event channel delivery via GSI to the I/O APIC
> >  • Xen event channel delivery via GSI to the i8259 PIC
> >  • MSIs routed to PIRQ event channels
> >  • GSIs routed to PIRQ event channels
> > 
> > As well as some variants of normal non-Xen stuff like MSI to vAPIC and
> > PCI INTx going to the I/O APIC and PIC, which ought to still work even
> > in Xen mode.
> > 
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > ---
> >  tests/avocado/xen_guest.py | 147 +++++++++++++++++++++++++++++++++++++
> >  1 file changed, 147 insertions(+)
> >  create mode 100644 tests/avocado/xen_guest.py
> > 
> > diff --git a/tests/avocado/xen_guest.py b/tests/avocado/xen_guest.py
> > new file mode 100644
> > index 0000000000..c50b52958f
> > --- /dev/null
> > +++ b/tests/avocado/xen_guest.py
> > @@ -0,0 +1,147 @@
> > +# Xen guest functional tests
> > +#
> 
> Perhaps to avoid confusion we should call the file kvm_xen_guests.py and
> make it clearer in the comment this is the mode we are talking about.
> c.f. the boot_xen.py tests which boot Xen (under TCG emulation) as a
> proper hypervisor.

Ack. I'll change it.

> > +# Copyright © 2021 Red Hat, Inc.
> > +# Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
> > +#
> > +# Author:
> > +#  David Woodhouse <dwmw2@infradead.org>
> > +#
> > +# 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')
> 
> This just means we will never have CI coverage. In theory when running
> on the GitLab shared runners the test should skip anyway when it can't
> find /dev/kvm and hopefully can run on our custom runners when it can
> see /dev/kvm.
> 

That one was just cargo-culted from the Intel IOMMU test which I
started from. Happy to drop that if it's redundant.



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH v2 2/3] tests/avocado: Add Fedora 34 distro, including kernel/initrd checksums

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2330 bytes --]

On Wed, 2023-03-08 at 17:42 +0000, Alex Bennée wrote:
> 
> David Woodhouse <dwmw2@infradead.org> writes:
> 
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > The kernel in Fedora 31 doesn't support 'xen_no_vector_callback' on
> > its command line, so add a slightly newer version as a prelude to
> > enabling avocado tests for Xen guests.
> 
> Why slightly newer rather than current release?
> 
> Our existing Fedora guest builds cause all sorts of timeout issues
> running under TCG and seem particularly heavyweight considering the
> coverage we manage to get.

Yeah, I spent a happy afternoon playing with this; I think you may have
observed some of it on IRC. Although I kept some of the naughtier words
to my side of the keyboard. From memory...

I tried Fedora 37 and eventually got that to *boot* (hint:
rootflags=subvol=root) although it concerned me that it wasn't yet at
the URL containing /archive/ so I wondered if adding it at its current
URL might not work in the long term. And then cloudinit didn't seem to
work on it (the finalize step failed in the guest IIRC).

I tried Fedora 36 and cloudinit *did* work there, and it was in
basically the same /pub/archive/fedora/ location so all that made me
happier... but 'dnf install xen-runtime' didn't work, which I was using
in my tests at that point to install the `xenstore-ls` binary. And in
fact dnf didn't seem to find *anything* useful at all in the
repositories which were configured in the Fedora Cloud 36 image.

So I tried Fedora 34, which is the first to have the
'xen_no_vector_callback' option that I'd added in the 5.12 kernel.
And when that worked, I declared all the above to be "Someone Else's
Problem".

The fact that it didn't boot with `-smp 4` was clearly *my* problem,
but the code was already merged upstream at that point, so fixing what
turned out to be the I/OAPIC migration bug ended up being my top
priority. 

Now, I've dropped the dnf invocation because now the second PR has 
been submitted I've switched it to use xen-disk anyway, and that's a
good enough exercise for both xenstore and event channel interrupts.

So it probably *could* move to Fedora 36, and the fact that the intel-
iommu test wouldn't work on Fedora 36 could remain Someone Else's
Problem. :)

Please advise.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH v2 2/3] tests/avocado: Add Fedora 34 distro, including kernel/initrd checksums

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 4854 bytes --]

On Wed, 2023-03-08 at 19:20 +0000, David Woodhouse wrote:
> On Wed, 2023-03-08 at 17:42 +0000, Alex Bennée wrote:
> > Why slightly newer rather than current release?
> >
> > Our existing Fedora guest builds cause all sorts of timeout issues
> > running under TCG and seem particularly heavyweight considering the
> > coverage we manage to get.
>
> Yeah, I spent a happy afternoon playing with this; I think you may have
> observed some of it on IRC. Although I kept some of the naughtier words
> to my side of the keyboard. From memory...

So...

            '34': {
                'x86_64':
                {'checksum': ('b9b621b26725ba95442d9a56cbaa0547'
                              '84e0779a9522ec6eafff07c6e6f717ea'),
                 'pxeboot_url': ('https://archives.fedoraproject.org/'
                                 'pub/archive/fedora/linux/releases/34/'
                                 'Everything/x86_64/os/images/pxeboot/'),
                 'kernel_hash': ('085fc6e47f2e3a271b591f3e56739ca9'
                                 '4c16718837a5f431ab95468e1e95f9eb'),
                 'initrd_hash': ('d6cd2e03e8188eed6c896fd65ff05f81'
                                 '2c4c1c8777d630b5909e9a1a4627e337'),
                 'kernel_params': ('root=UUID=386769a3-cfa5-47c8-8797-'
                                   'd5ec58c9cb6c ro no_timer_check '
                                   'net.ifnames=0 console=tty1 '
                                   'console=ttyS0,115200n8'),
                },
            },
            '36': {
                'x86_64':
                {'checksum': ('ca9e514cc2f4a7a0188e7c68af60eb4e'
                              '573d2e6850cc65b464697223f46b4605'),
                 'pxeboot_url': ('https://archives.fedoraproject.org/'
                                 'pub/archive/fedora/linux/releases/36/'
                                 'Everything/x86_64/os/images/pxeboot/'),
                 'kernel_hash': ('f5d8162b1e086f1a9d09d3d2c6decdbe'
                                 'c84d7020cde4b6f32a98f21eacc8cf3b'),
                 'initrd_hash': ('2c8a9d3f0b607f86422a0df8227106c0'
                                 '847eb4a4f15bade77b1abbc3228533c6'),
                 'kernel_params': ('root=UUID=737459fa-eab9-4d8f-9a14-'
                                   'c8c32d403c08 ro rootflags=subvol=root '
                                   'no_timer_check net.ifnames=0 '
                                   'console=tty1 console=ttyS0,115200n8'),
                },
            },
            '37': {
                'x86_64':
                {'checksum': ('b5b9bec91eee65489a5745f6ee620573'
                              'b23337cbb1eb4501ce200b157a01f3a0'),
                 'pxeboot_url': ('https://archives.fedoraproject.org/'
                                 'pub/fedora/linux/releases/37/'
                                 'Everything/x86_64/os/images/pxeboot/'),
                 'kernel_hash': ('2bd5f834349313d10a9dbeb41d9f71d9'
                                 '85f7b9c4f7e2ba750a1dd6558f082ba9'),
                 'initrd_hash': ('8b7e91d6f14353217ed44a98802b0fef'
                                 'b190ab1ce38a96166da87d629ab39bef'),
                 'kernel_params': ('root=UUID=3975d5e5-e66a-44c3-90f7-'
                                   'dc2d1c7859d1 ro rootflags=subvol=root '
                                   'no_timer_check net.ifnames=0 '
                                   'console=tty1 console=ttyS0,115200n8'),
                },
            },
        }

Fedora 36 does work (since I'm not actually trying to install anything
inside the guest any more, and I'm sure that *would* be a surmountable
problem if we bothered to investigate and fix up the repo config). It's
about 10 seconds slower per test than Fedora 34 was.

Fedora 37 fails thus:

2023-03-09 08:25:17,072 datadrainer      L0193 DEBUG| -----END SSH HOST KEY KEYS-----
2023-03-09 08:25:17,081 datadrainer      L0193 DEBUG| [   49.036621] cloud-init[1550]: 2023-03-09 08:25:16,989 - util.py[WARNING]: Running module phone-home (<module 'cloudinit.config.cc_phone_home' from '/usr/lib/python3.11/site-packages/cloudinit/config/cc_phone_home.py'>) failed
2023-03-09 08:25:17,101 datadrainer      L0193 DEBUG| [   49.055528] cloud-init[1550]: Cloud-init v. 22.2 finished at Thu, 09 Mar 2023 08:25:16 +0000. Datasource DataSourceNoCloud [seed=/dev/sda][dsmode=net].  Up 49.04 seconds
2023-03-09 08:25:17,203 datadrainer      L0193 DEBUG| [FAILED] Failed to start cloud-fina… Execute cloud user/final scripts.

Full log at http://david.woodhou.se/job-2023-03-09T08.20-d57f30d.log

I've made the other changes (mostly xen_guest → kvm_xen_guest) and
pushed to my branch at 
https://git.infradead.org/users/dwmw2/qemu.git/shortlog/refs/heads/xenfv


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]

Re: [PATCH v2 1/3] hw/intc/ioapic: Update KVM routes before redelivering IRQ, on RTE update

[Paolo Bonzini]

On 3/8/23 12:19, David Woodhouse wrote:
> A Linux guest will perform IRQ migration after the IRQ has happened,
> updating the RTE to point to the new destination CPU and then unmasking
> the interrupt.
> 
> However, when the guest updates the RTE, ioapic_mem_write() calls
> ioapic_service(), which redelivers the pending level interrupt via
> kvm_set_irq(), *before* calling ioapic_update_kvm_routes() which sets
> the new target CPU.
> 
> Thus, the IRQ which is supposed to go to the new target CPU is instead
> misdelivered to the previous target. An example where the guest kernel
> is attempting to migrate from CPU#2 to CPU#0 shows:
> 
> xenstore_read tx 0 path control/platform-feature-xs_reset_watches
> ioapic_set_irq vector: 11 level: 1
> ioapic_set_remote_irr set remote irr for pin 11
> ioapic_service: trigger KVM IRQ 11
> [    0.523627] The affinity mask was 0-3 and the handler is on 2
> ioapic_mem_write ioapic mem write addr 0x0 regsel: 0x27 size 0x4 val 0x26
> ioapic_update_kvm_routes: update KVM route for IRQ 11: fee02000 8021
> ioapic_mem_write ioapic mem write addr 0x10 regsel: 0x26 size 0x4 val 0x18021
> xenstore_reset_watches
> ioapic_set_irq vector: 11 level: 1
> ioapic_mem_read ioapic mem read addr 0x10 regsel: 0x26 size 0x4 retval 0x1c021
> [    0.524569] ioapic_ack_level IRQ 11 moveit = 1
> ioapic_eoi_broadcast EOI broadcast for vector 33
> ioapic_clear_remote_irr clear remote irr for pin 11 vector 33
> ioapic_mem_write ioapic mem write addr 0x0 regsel: 0x26 size 0x4 val 0x26
> ioapic_mem_read ioapic mem read addr 0x10 regsel: 0x26 size 0x4 retval 0x18021
> [    0.525235] ioapic_finish_move IRQ 11 calls irq_move_masked_irq()
> [    0.526147] irq_do_set_affinity for IRQ 11, 0
> [    0.526732] ioapic_set_affinity for IRQ 11, 0
> [    0.527330] ioapic_setup_msg_from_msi for IRQ11 target 0
> ioapic_mem_write ioapic mem write addr 0x0 regsel: 0x26 size 0x4 val 0x27
> ioapic_mem_write ioapic mem write addr 0x10 regsel: 0x27 size 0x4 val 0x0
> ioapic_mem_write ioapic mem write addr 0x0 regsel: 0x27 size 0x4 val 0x26
> ioapic_mem_write ioapic mem write addr 0x10 regsel: 0x26 size 0x4 val 0x18021
> [    0.527623] ioapic_set_affinity returns 0
> [    0.527623] ioapic_finish_move IRQ 11 calls unmask_ioapic_irq()
> ioapic_mem_write ioapic mem write addr 0x0 regsel: 0x26 size 0x4 val 0x26
> ioapic_mem_write ioapic mem write addr 0x10 regsel: 0x26 size 0x4 val 0x8021
> ioapic_set_remote_irr set remote irr for pin 11
> ioapic_service: trigger KVM IRQ 11
> ioapic_update_kvm_routes: update KVM route for IRQ 11: fee00000 8021
> [    0.529571] The affinity mask was 0 and the handler is on 2
> [    xenstore_watch path memory/target token FFFFFFFF92847D40
> 
> There are no other code paths in ioapic_mem_write() which need the KVM
> IRQ routing table to be updated, so just shift the call from the end
> of the function to happen right before the call to ioapic_service()
> and thus deliver the re-enabled IRQ to the right place.
> 
> Alternative fixes might have been just to remove the part in
> ioapic_service() which delivers the IRQ via kvm_set_irq() because
> surely delivering as MSI ought to work just fine anyway in all cases?
> That code lacks a comment justifying its existence.
> 
> Or maybe in the specific case shown in the above log, it would have
> sufficed for ioapic_update_kvm_routes() to update the route *even*
> when the IRQ is masked. It's not like it's actually going to get
> triggered unless QEMU deliberately does so, anyway? But that only
> works because the target CPU happens to be in the high word of the
> RTE; if something in the *low* word (vector, perhaps) was changed
> at the same time as the unmask, we'd still trigger with stale data.
> 
> Fixes: 15eafc2e602f "kvm: x86: add support for KVM_CAP_SPLIT_IRQCHIP"
> Signed-off-by: David Woodhouse <dwmw2@infradead.org>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> ---
>   hw/intc/ioapic.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
> index 6364ecab1b..716ffc8bbb 100644
> --- a/hw/intc/ioapic.c
> +++ b/hw/intc/ioapic.c
> @@ -405,6 +405,7 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val,
>                   s->ioredtbl[index] |= ro_bits;
>                   s->irq_eoi[index] = 0;
>                   ioapic_fix_edge_remote_irr(&s->ioredtbl[index]);
> +                ioapic_update_kvm_routes(s);
>                   ioapic_service(s);
>               }
>           }
> @@ -417,8 +418,6 @@ ioapic_mem_write(void *opaque, hwaddr addr, uint64_t val,
>           ioapic_eoi_broadcast(val);
>           break;
>       }
> -
> -    ioapic_update_kvm_routes(s);
>   }
>   
>   static const MemoryRegionOps ioapic_io_ops = {

Queued this one, thanks

Paolo