[PATCH 0/6] automation: add PCI passthrough tests on x86

[PATCH 0/6] automation: add PCI passthrough tests on x86

[Marek Marczykowski-Górecki]

This series adds passthrough tests using the ADL x86 hw gitlab runner. Some of
the patches are improves existing hw tests too.

Example passing run:
https://gitlab.com/xen-project/people/marmarek/xen/-/pipelines/846920786

Marek Marczykowski-Górecki (6):
  automation: specify explicit dom0 mem size for ADL tests
  automation: add runtime qemu dependencies to test container
  automation: re-enable building SeaBIOS in Alpine container
  automation: wait for the login prompt as test end marker
  automation: PCI passthrough tests on ADL hw
  automation: include tail of serial log in the gitlab outout

 automation/gitlab-ci/test.yaml                    | 20 ++++-
 automation/scripts/build                          |  2 +-
 automation/scripts/qubes-x86-64.sh                | 85 +++++++++++++---
 automation/tests-artifacts/alpine/3.12.dockerfile |  1 +-
 4 files changed, 93 insertions(+), 15 deletions(-)

base-commit: 18c128ba66e6308744850aca96dbffd18f91c29b
-- 
git-series 0.9.1

[PATCH 1/6] automation: specify explicit dom0 mem size for ADL tests

[Marek Marczykowski-Górecki]

Avoid memory fragmentation that leads to:
(XEN) common/memory.c:277:d0v10 Could not allocate order=9 extent: id=1 memflags=0xc0 (0 of 4)

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 automation/scripts/qubes-x86-64.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
index 2d4cf2e2268c..916dbaae59c3 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -123,7 +123,7 @@ TFTP=/scratch/gitlab-runner/tftp
 CONTROLLER=control@thor.testnet
 
 echo '
-multiboot2 (http)/gitlab-ci/xen console=com1 com1=115200,8n1 loglvl=all guest_loglvl=all
+multiboot2 (http)/gitlab-ci/xen console=com1 com1=115200,8n1 loglvl=all guest_loglvl=all dom0_mem=4G
 module2 (http)/gitlab-ci/vmlinuz console=hvc0 root=/dev/ram0
 module2 (http)/gitlab-ci/initrd-dom0
 ' > $TFTP/grub.cfg
-- 
git-series 0.9.1

[PATCH 2/6] automation: add runtime qemu dependencies to test container

[Marek Marczykowski-Górecki]

This is necessary to start HVM guests in subsequent tests.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 automation/tests-artifacts/alpine/3.12.dockerfile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/automation/tests-artifacts/alpine/3.12.dockerfile b/automation/tests-artifacts/alpine/3.12.dockerfile
index b3909996b47b..073f16a0d70a 100644
--- a/automation/tests-artifacts/alpine/3.12.dockerfile
+++ b/automation/tests-artifacts/alpine/3.12.dockerfile
@@ -13,6 +13,7 @@ RUN \
   \
   # xen runtime deps
   apk add musl && \
+  apk add libgcc && \
   apk add openrc && \
   apk add busybox && \
   apk add sudo && \
-- 
git-series 0.9.1

[PATCH 3/6] automation: re-enable building SeaBIOS in Alpine container

[Marek Marczykowski-Górecki]

It seems to build just fine with Alpine 3.12, and SeaBIOS is necessary
for a HVM test (that use the Alpine build).

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 automation/scripts/build | 2 --
 1 file changed, 2 deletions(-)

diff --git a/automation/scripts/build b/automation/scripts/build
index 7d1b19c4250d..d830cff7b7c7 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -77,8 +77,6 @@ else
     if ldd /bin/ls | grep -q musl; then
         # disable --disable-werror for QEMUU when building with MUSL
         cfgargs+=("--with-extra-qemuu-configure-args=\"--disable-werror\"")
-        # SeaBIOS doesn't build on MUSL systems
-        cfgargs+=("--with-system-seabios=/bin/false")
     fi
 
     # Qemu requires Python 3.5 or later, and ninja
-- 
git-series 0.9.1

[PATCH 4/6] automation: wait for the login prompt as test end marker

[Marek Marczykowski-Górecki]

The login prompt is printed after all the startup (test) scripts, wait
for that instead of "passed" marker. And only then check if test passed.
Before this patch there was a race: "passed" marker could be already
printed, but the final check would fail because login prompt wasn't
there yet.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 automation/scripts/qubes-x86-64.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
index 916dbaae59c3..c0bc71764f73 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -159,7 +159,7 @@ if [ -n "$wait_and_wakeup" ]; then
     ssh $CONTROLLER wake
 fi
 
-until grep "$passed" smoke.serial || [ $timeout -le 0 ]; do
+until grep "^Welcome to Alpine Linux" smoke.serial || [ $timeout -le 0 ]; do
     sleep 1;
     : $((--timeout))
 done
-- 
git-series 0.9.1

[PATCH 5/6] automation: PCI passthrough tests on ADL hw

[Marek Marczykowski-Górecki]

Add simple PCI passthrough test to both PV and HVM domU. It passes
through a network adapter (the only one in the system), gets an IP via
DHCP (first basic test) and then ping the gateway (second basic test).
Finally, if device is supposed to use MSI or MSI-X (as set in the
PCIDEV_INTR test variable), check if it's in use via /proc/interrupts.

On the current runner, the device in question is this:
03:00.0 Ethernet controller [0200]: Intel Corporation Ethernet Controller I225-V [8086:15f3] (rev 03)
	Subsystem: Micro-Star International Co., Ltd. [MSI] Device [1462:7d25]
	Flags: bus master, fast devsel, latency 0, IRQ 18
	Memory at 50400000 (32-bit, non-prefetchable) [size=1M]
	Memory at 50500000 (32-bit, non-prefetchable) [size=16K]
	Capabilities: [40] Power Management version 3
	Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
	Capabilities: [70] MSI-X: Enable+ Count=5 Masked-
	Capabilities: [a0] Express Endpoint, MSI 00
	Capabilities: [100] Advanced Error Reporting
	Capabilities: [140] Device Serial Number ...
	Capabilities: [1c0] Latency Tolerance Reporting
	Capabilities: [1f0] Precision Time Measurement
	Capabilities: [1e0] L1 PM Substates
	Kernel driver in use: igc
	Kernel modules: igc

With the current Xen version, it uses MSI-X under PV and MSI under HVM.

This patch moves domU config to a variable, to make it configurable on
per-test basis. Add also a few comments for visual separation of tests.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 automation/gitlab-ci/test.yaml     | 20 ++++++++-
 automation/scripts/qubes-x86-64.sh | 80 ++++++++++++++++++++++++++-----
 2 files changed, 89 insertions(+), 11 deletions(-)

diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index d68c584269dd..1ce083e6cd88 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -94,6 +94,8 @@
     # the test controller runs on RPi4
     CONTAINER: alpine:3.12-arm64v8
     LOGFILE: smoke-test.log
+    PCIDEV: "03:00.0"
+    PCIDEV_INTR: "MSI-X"
   artifacts:
     paths:
       - smoke.serial
@@ -147,6 +149,24 @@ adl-suspend-x86-64-gcc-debug:
     - *x86-64-test-needs
     - alpine-3.12-gcc-debug
 
+adl-pci-pv-x86-64-gcc-debug:
+  extends: .adl-x86-64
+  script:
+    - ./automation/scripts/qubes-x86-64.sh pci-pv 2>&1 | tee ${LOGFILE}
+  needs:
+    - *x86-64-test-needs
+    - alpine-3.12-gcc-debug
+
+adl-pci-hvm-x86-64-gcc-debug:
+  extends: .adl-x86-64
+  variables:
+    PCIDEV_INTR: "MSI"
+  script:
+    - ./automation/scripts/qubes-x86-64.sh pci-hvm 2>&1 | tee ${LOGFILE}
+  needs:
+    - *x86-64-test-needs
+    - alpine-3.12-gcc-debug
+
 qemu-smoke-dom0-arm64-gcc:
   extends: .qemu-arm64
   script:
diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
index c0bc71764f73..6442f7dda515 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -4,8 +4,21 @@ set -ex
 
 test_variant=$1
 
+### defaults
 wait_and_wakeup=
 timeout=120
+domU_config='
+type = "pvh"
+name = "domU"
+kernel = "/boot/vmlinuz"
+ramdisk = "/boot/initrd-domU"
+extra = "root=/dev/ram0 console=hvc0"
+memory = 512
+vif = [ "bridge=xenbr0", ]
+disk = [ ]
+'
+
+### test: smoke test
 if [ -z "${test_variant}" ]; then
     passed="ping test passed"
     domU_check="
@@ -23,6 +36,8 @@ done
 tail -n 100 /var/log/xen/console/guest-domU.log
 echo \"${passed}\"
 "
+
+### test: S3
 elif [ "${test_variant}" = "s3" ]; then
     passed="suspend test passed"
     wait_and_wakeup="started, suspending"
@@ -48,6 +63,59 @@ xl dmesg | grep 'Finishing wakeup from ACPI S3 state' || exit 1
 ping -c 10 192.168.0.2 || exit 1
 echo \"${passed}\"
 "
+
+### test: pci-pv, pci-hvm
+elif [ "${test_variant}" = "pci-pv" ] || [ "${test_variant}" = "pci-hvm" ]; then
+
+    if [ -z "$PCIDEV" ]; then
+        echo "Please set 'PCIDEV' variable with BDF of test network adapter" >&2
+        echo "Optionally set also 'PCIDEV_INTR' to 'MSI' or 'MSI-X'" >&2
+        exit 1
+    fi
+
+    passed="pci test passed"
+
+    domU_config='
+type = "'${test_variant#pci-}'"
+name = "domU"
+kernel = "/boot/vmlinuz"
+ramdisk = "/boot/initrd-domU"
+extra = "root=/dev/ram0 console=hvc0"
+memory = 512
+vif = [ ]
+disk = [ ]
+pci = [ "'$PCIDEV',seize=1" ]
+on_reboot = "destroy"
+'
+
+    domU_check="
+set -x -e
+ip link set eth0 up
+timeout 30s udhcpc -i eth0
+pingip=\$(ip -o -4 r show default|cut -f 3 -d ' ')
+ping -c 10 \"\$pingip\"
+echo domU started
+cat /proc/interrupts
+"
+    if [ "$PCIDEV_INTR" = "MSI-X" ]; then
+        domU_check="$domU_check
+grep -- '\\(-msi-x\\|PCI-MSI-X\\).*eth0' /proc/interrupts
+"
+    elif [ "$PCIDEV_INTR" = "MSI" ]; then
+        # depending on the kernel version and domain type, the MSI can be
+        # marked as '-msi', 'PCI-MSI', or 'PCI-MSI-<SBDF>'; be careful to not match
+        # -msi-x nor PCI-MSI-X
+        domU_check="$domU_check
+grep -- '\\(-msi\\|PCI-MSI\\( \\|-[^X]\\)\\).*eth0' /proc/interrupts
+"
+    fi
+    domU_check="$domU_check
+echo \"${passed}\"
+"
+
+    dom0_check="
+tail -n 100 -F /var/log/xen/console/guest-domU.log
+"
 fi
 
 # DomU
@@ -97,17 +165,7 @@ xl create /etc/xen/domU.cfg
 ${dom0_check}
 " > etc/local.d/xen.start
 chmod +x etc/local.d/xen.start
-# just PVH for now
-echo '
-type = "pvh"
-name = "domU"
-kernel = "/boot/vmlinuz"
-ramdisk = "/boot/initrd-domU"
-extra = "root=/dev/ram0 console=hvc0"
-memory = 512
-vif = [ "bridge=xenbr0", ]
-disk = [ ]
-' > etc/xen/domU.cfg
+echo "$domU_config" > etc/xen/domU.cfg
 
 echo "rc_verbose=yes" >> etc/rc.conf
 echo "XENCONSOLED_TRACE=all" >> etc/default/xencommons
-- 
git-series 0.9.1

[PATCH 6/6] automation: include tail of serial log in the gitlab outout

[Marek Marczykowski-Górecki]

Make it a bit easier to see what has failed.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 automation/scripts/qubes-x86-64.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
index 6442f7dda515..9b89d90f653c 100755
--- a/automation/scripts/qubes-x86-64.sh
+++ b/automation/scripts/qubes-x86-64.sh
@@ -228,5 +228,6 @@ fi
 
 sleep 1
 
+tail -n 100 smoke.serial
 (grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1
 exit 0
-- 
git-series 0.9.1

Re: [PATCH 1/6] automation: specify explicit dom0 mem size for ADL tests

[Stefano Stabellini]

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

On Mon, 24 Apr 2023, Marek Marczykowski-Górecki wrote:
> Avoid memory fragmentation that leads to:
> (XEN) common/memory.c:277:d0v10 Could not allocate order=9 extent: id=1 memflags=0xc0 (0 of 4)
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  automation/scripts/qubes-x86-64.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> index 2d4cf2e2268c..916dbaae59c3 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -123,7 +123,7 @@ TFTP=/scratch/gitlab-runner/tftp
>  CONTROLLER=control@thor.testnet
>  
>  echo '
> -multiboot2 (http)/gitlab-ci/xen console=com1 com1=115200,8n1 loglvl=all guest_loglvl=all
> +multiboot2 (http)/gitlab-ci/xen console=com1 com1=115200,8n1 loglvl=all guest_loglvl=all dom0_mem=4G
>  module2 (http)/gitlab-ci/vmlinuz console=hvc0 root=/dev/ram0
>  module2 (http)/gitlab-ci/initrd-dom0
>  ' > $TFTP/grub.cfg
> -- 
> git-series 0.9.1
> 

Re: [PATCH 2/6] automation: add runtime qemu dependencies to test container

[Stefano Stabellini]

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

On Mon, 24 Apr 2023, Marek Marczykowski-Górecki wrote:
> This is necessary to start HVM guests in subsequent tests.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  automation/tests-artifacts/alpine/3.12.dockerfile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/automation/tests-artifacts/alpine/3.12.dockerfile b/automation/tests-artifacts/alpine/3.12.dockerfile
> index b3909996b47b..073f16a0d70a 100644
> --- a/automation/tests-artifacts/alpine/3.12.dockerfile
> +++ b/automation/tests-artifacts/alpine/3.12.dockerfile
> @@ -13,6 +13,7 @@ RUN \
>    \
>    # xen runtime deps
>    apk add musl && \
> +  apk add libgcc && \
>    apk add openrc && \
>    apk add busybox && \
>    apk add sudo && \
> -- 
> git-series 0.9.1
> 

Re: [PATCH 4/6] automation: wait for the login prompt as test end marker

[Stefano Stabellini]

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

On Mon, 24 Apr 2023, Marek Marczykowski-Górecki wrote:
> The login prompt is printed after all the startup (test) scripts, wait
> for that instead of "passed" marker. And only then check if test passed.
> Before this patch there was a race: "passed" marker could be already
> printed, but the final check would fail because login prompt wasn't
> there yet.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  automation/scripts/qubes-x86-64.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> index 916dbaae59c3..c0bc71764f73 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -159,7 +159,7 @@ if [ -n "$wait_and_wakeup" ]; then
>      ssh $CONTROLLER wake
>  fi
>  
> -until grep "$passed" smoke.serial || [ $timeout -le 0 ]; do
> +until grep "^Welcome to Alpine Linux" smoke.serial || [ $timeout -le 0 ]; do
>      sleep 1;
>      : $((--timeout))
>  done
> -- 
> git-series 0.9.1
> 

Re: [PATCH 6/6] automation: include tail of serial log in the gitlab outout

[Stefano Stabellini]

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

On Mon, 24 Apr 2023, Marek Marczykowski-Górecki wrote:
> Make it a bit easier to see what has failed.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  automation/scripts/qubes-x86-64.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> index 6442f7dda515..9b89d90f653c 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -228,5 +228,6 @@ fi
>  
>  sleep 1
>  
> +tail -n 100 smoke.serial
>  (grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1
>  exit 0

Isn't smoke.serial already in stdout and also part of the artifacts? The
user can always click on the full output or on the smoke.serial file
among artifacts.  Maybe the issue is that it is called ".serial" instead
of ".txt" so the browser will not try to open it directly in a browser
window. If we rename it to ".txt" the user could just click on
"artifacts" and then on "serial.txt" and it would be all there.

100 lines is not much, but I think in general it is better if we make it
easier to access smoke.serial in its entirety instead.

Re: [PATCH 6/6] automation: include tail of serial log in the gitlab outout

[Marek Marczykowski-Górecki]

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

On Mon, Apr 24, 2023 at 05:39:19PM -0700, Stefano Stabellini wrote:
> On Mon, 24 Apr 2023, Marek Marczykowski-Górecki wrote:
> > Make it a bit easier to see what has failed.
> > 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> >  automation/scripts/qubes-x86-64.sh | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> > index 6442f7dda515..9b89d90f653c 100755
> > --- a/automation/scripts/qubes-x86-64.sh
> > +++ b/automation/scripts/qubes-x86-64.sh
> > @@ -228,5 +228,6 @@ fi
> >  
> >  sleep 1
> >  
> > +tail -n 100 smoke.serial
> >  (grep -q "^Welcome to Alpine Linux" smoke.serial && grep -q "${passed}" smoke.serial) || exit 1
> >  exit 0
> 
> Isn't smoke.serial already in stdout and also part of the artifacts? The
> user can always click on the full output or on the smoke.serial file
> among artifacts.  Maybe the issue is that it is called ".serial" instead
> of ".txt" so the browser will not try to open it directly in a browser
> window. If we rename it to ".txt" the user could just click on
> "artifacts" and then on "serial.txt" and it would be all there.
> 
> 100 lines is not much, but I think in general it is better if we make it
> easier to access smoke.serial in its entirety instead.

Yes, you can click on it to get it full. But that's two extra clicks,
and if the thing that matters is just that panic message at the end, you
can also have it right in the job preview.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/6] automation: re-enable building SeaBIOS in Alpine container

[Stefano Stabellini]

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

On Mon, 24 Apr 2023, Marek Marczykowski-Górecki wrote:
> It seems to build just fine with Alpine 3.12, and SeaBIOS is necessary
> for a HVM test (that use the Alpine build).
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  automation/scripts/build | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/automation/scripts/build b/automation/scripts/build
> index 7d1b19c4250d..d830cff7b7c7 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -77,8 +77,6 @@ else
>      if ldd /bin/ls | grep -q musl; then
>          # disable --disable-werror for QEMUU when building with MUSL
>          cfgargs+=("--with-extra-qemuu-configure-args=\"--disable-werror\"")
> -        # SeaBIOS doesn't build on MUSL systems
> -        cfgargs+=("--with-system-seabios=/bin/false")
>      fi
>  
>      # Qemu requires Python 3.5 or later, and ninja
> -- 
> git-series 0.9.1
> 

Re: [PATCH 5/6] automation: PCI passthrough tests on ADL hw

[Stefano Stabellini]

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

On Mon, 24 Apr 2023, Marek Marczykowski-Górecki wrote:
> Add simple PCI passthrough test to both PV and HVM domU. It passes
> through a network adapter (the only one in the system), gets an IP via
> DHCP (first basic test) and then ping the gateway (second basic test).
> Finally, if device is supposed to use MSI or MSI-X (as set in the
> PCIDEV_INTR test variable), check if it's in use via /proc/interrupts.
> 
> On the current runner, the device in question is this:
> 03:00.0 Ethernet controller [0200]: Intel Corporation Ethernet Controller I225-V [8086:15f3] (rev 03)
> 	Subsystem: Micro-Star International Co., Ltd. [MSI] Device [1462:7d25]
> 	Flags: bus master, fast devsel, latency 0, IRQ 18
> 	Memory at 50400000 (32-bit, non-prefetchable) [size=1M]
> 	Memory at 50500000 (32-bit, non-prefetchable) [size=16K]
> 	Capabilities: [40] Power Management version 3
> 	Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
> 	Capabilities: [70] MSI-X: Enable+ Count=5 Masked-
> 	Capabilities: [a0] Express Endpoint, MSI 00
> 	Capabilities: [100] Advanced Error Reporting
> 	Capabilities: [140] Device Serial Number ...
> 	Capabilities: [1c0] Latency Tolerance Reporting
> 	Capabilities: [1f0] Precision Time Measurement
> 	Capabilities: [1e0] L1 PM Substates
> 	Kernel driver in use: igc
> 	Kernel modules: igc
> 
> With the current Xen version, it uses MSI-X under PV and MSI under HVM.
> 
> This patch moves domU config to a variable, to make it configurable on
> per-test basis. Add also a few comments for visual separation of tests.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
>  automation/gitlab-ci/test.yaml     | 20 ++++++++-
>  automation/scripts/qubes-x86-64.sh | 80 ++++++++++++++++++++++++++-----
>  2 files changed, 89 insertions(+), 11 deletions(-)
> 
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index d68c584269dd..1ce083e6cd88 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -94,6 +94,8 @@
>      # the test controller runs on RPi4
>      CONTAINER: alpine:3.12-arm64v8
>      LOGFILE: smoke-test.log
> +    PCIDEV: "03:00.0"
> +    PCIDEV_INTR: "MSI-X"

This is minor but I would move PCIDEV_INTR to
adl-pci-pv-x86-64-gcc-debug given that adl-pci-hvm-x86-64-gcc-debug
already redefines it.

I would also move PCIDEV to adl-pci-pv-x86-64-gcc-debug and
adl-pci-hvm-x86-64-gcc-debug, but I am fine either way.

However the two new tests failed for me:

https://gitlab.com/xen-project/people/sstabellini/xen/-/pipelines/847157948


+ grep '^Welcome to Alpine Linux' smoke.serial
+ '[' 0 -le 0 ]
+ '[' 0 -le 0 ]
+ echo 'ERROR: test timeout, aborting'
ERROR: test timeout, aborting

The Welcome to Alpine Linux message is missing


>    artifacts:
>      paths:
>        - smoke.serial
> @@ -147,6 +149,24 @@ adl-suspend-x86-64-gcc-debug:
>      - *x86-64-test-needs
>      - alpine-3.12-gcc-debug
>  
> +adl-pci-pv-x86-64-gcc-debug:
> +  extends: .adl-x86-64
> +  script:
> +    - ./automation/scripts/qubes-x86-64.sh pci-pv 2>&1 | tee ${LOGFILE}
> +  needs:
> +    - *x86-64-test-needs
> +    - alpine-3.12-gcc-debug
> +
> +adl-pci-hvm-x86-64-gcc-debug:
> +  extends: .adl-x86-64
> +  variables:
> +    PCIDEV_INTR: "MSI"
> +  script:
> +    - ./automation/scripts/qubes-x86-64.sh pci-hvm 2>&1 | tee ${LOGFILE}
> +  needs:
> +    - *x86-64-test-needs
> +    - alpine-3.12-gcc-debug
> +
>  qemu-smoke-dom0-arm64-gcc:
>    extends: .qemu-arm64
>    script:
> diff --git a/automation/scripts/qubes-x86-64.sh b/automation/scripts/qubes-x86-64.sh
> index c0bc71764f73..6442f7dda515 100755
> --- a/automation/scripts/qubes-x86-64.sh
> +++ b/automation/scripts/qubes-x86-64.sh
> @@ -4,8 +4,21 @@ set -ex
>  
>  test_variant=$1
>  
> +### defaults
>  wait_and_wakeup=
>  timeout=120
> +domU_config='
> +type = "pvh"
> +name = "domU"
> +kernel = "/boot/vmlinuz"
> +ramdisk = "/boot/initrd-domU"
> +extra = "root=/dev/ram0 console=hvc0"
> +memory = 512
> +vif = [ "bridge=xenbr0", ]
> +disk = [ ]
> +'
> +
> +### test: smoke test
>  if [ -z "${test_variant}" ]; then
>      passed="ping test passed"
>      domU_check="
> @@ -23,6 +36,8 @@ done
>  tail -n 100 /var/log/xen/console/guest-domU.log
>  echo \"${passed}\"
>  "
> +
> +### test: S3
>  elif [ "${test_variant}" = "s3" ]; then
>      passed="suspend test passed"
>      wait_and_wakeup="started, suspending"
> @@ -48,6 +63,59 @@ xl dmesg | grep 'Finishing wakeup from ACPI S3 state' || exit 1
>  ping -c 10 192.168.0.2 || exit 1
>  echo \"${passed}\"
>  "
> +
> +### test: pci-pv, pci-hvm
> +elif [ "${test_variant}" = "pci-pv" ] || [ "${test_variant}" = "pci-hvm" ]; then
> +
> +    if [ -z "$PCIDEV" ]; then
> +        echo "Please set 'PCIDEV' variable with BDF of test network adapter" >&2
> +        echo "Optionally set also 'PCIDEV_INTR' to 'MSI' or 'MSI-X'" >&2
> +        exit 1
> +    fi
> +
> +    passed="pci test passed"
> +
> +    domU_config='
> +type = "'${test_variant#pci-}'"
> +name = "domU"
> +kernel = "/boot/vmlinuz"
> +ramdisk = "/boot/initrd-domU"
> +extra = "root=/dev/ram0 console=hvc0"
> +memory = 512
> +vif = [ ]
> +disk = [ ]
> +pci = [ "'$PCIDEV',seize=1" ]
> +on_reboot = "destroy"
> +'
> +
> +    domU_check="
> +set -x -e
> +ip link set eth0 up
> +timeout 30s udhcpc -i eth0
> +pingip=\$(ip -o -4 r show default|cut -f 3 -d ' ')
> +ping -c 10 \"\$pingip\"
> +echo domU started
> +cat /proc/interrupts
> +"
> +    if [ "$PCIDEV_INTR" = "MSI-X" ]; then
> +        domU_check="$domU_check
> +grep -- '\\(-msi-x\\|PCI-MSI-X\\).*eth0' /proc/interrupts
> +"
> +    elif [ "$PCIDEV_INTR" = "MSI" ]; then
> +        # depending on the kernel version and domain type, the MSI can be
> +        # marked as '-msi', 'PCI-MSI', or 'PCI-MSI-<SBDF>'; be careful to not match
> +        # -msi-x nor PCI-MSI-X
> +        domU_check="$domU_check
> +grep -- '\\(-msi\\|PCI-MSI\\( \\|-[^X]\\)\\).*eth0' /proc/interrupts
> +"
> +    fi
> +    domU_check="$domU_check
> +echo \"${passed}\"
> +"
> +
> +    dom0_check="
> +tail -n 100 -F /var/log/xen/console/guest-domU.log
> +"
>  fi
>  
>  # DomU
> @@ -97,17 +165,7 @@ xl create /etc/xen/domU.cfg
>  ${dom0_check}
>  " > etc/local.d/xen.start
>  chmod +x etc/local.d/xen.start
> -# just PVH for now
> -echo '
> -type = "pvh"
> -name = "domU"
> -kernel = "/boot/vmlinuz"
> -ramdisk = "/boot/initrd-domU"
> -extra = "root=/dev/ram0 console=hvc0"
> -memory = 512
> -vif = [ "bridge=xenbr0", ]
> -disk = [ ]
> -' > etc/xen/domU.cfg
> +echo "$domU_config" > etc/xen/domU.cfg
>  
>  echo "rc_verbose=yes" >> etc/rc.conf
>  echo "XENCONSOLED_TRACE=all" >> etc/default/xencommons
> -- 
> git-series 0.9.1
> 

Re: [PATCH 5/6] automation: PCI passthrough tests on ADL hw

[Jason Andryuk]

Hi, Marek,

On Mon, Apr 24, 2023 at 4:57 PM Marek Marczykowski-Górecki
<marmarek@invisiblethingslab.com> wrote:
> +    elif [ "$PCIDEV_INTR" = "MSI" ]; then
> +        # depending on the kernel version and domain type, the MSI can be
> +        # marked as '-msi', 'PCI-MSI', or 'PCI-MSI-<SBDF>'; be careful to not match
> +        # -msi-x nor PCI-MSI-X
> +        domU_check="$domU_check
> +grep -- '\\(-msi\\|PCI-MSI\\( \\|-[^X]\\)\\).*eth0' /proc/interrupts
> +"

This will match -msi-x.  Do you want to make the first part "-msi "?

Regards,
Jason

Re: [PATCH 5/6] automation: PCI passthrough tests on ADL hw

[Marek Marczykowski-Górecki]

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

On Mon, Apr 24, 2023 at 08:01:46PM -0700, Stefano Stabellini wrote:
> On Mon, 24 Apr 2023, Marek Marczykowski-Górecki wrote:
> > Add simple PCI passthrough test to both PV and HVM domU. It passes
> > through a network adapter (the only one in the system), gets an IP via
> > DHCP (first basic test) and then ping the gateway (second basic test).
> > Finally, if device is supposed to use MSI or MSI-X (as set in the
> > PCIDEV_INTR test variable), check if it's in use via /proc/interrupts.
> > 
> > On the current runner, the device in question is this:
> > 03:00.0 Ethernet controller [0200]: Intel Corporation Ethernet Controller I225-V [8086:15f3] (rev 03)
> > 	Subsystem: Micro-Star International Co., Ltd. [MSI] Device [1462:7d25]
> > 	Flags: bus master, fast devsel, latency 0, IRQ 18
> > 	Memory at 50400000 (32-bit, non-prefetchable) [size=1M]
> > 	Memory at 50500000 (32-bit, non-prefetchable) [size=16K]
> > 	Capabilities: [40] Power Management version 3
> > 	Capabilities: [50] MSI: Enable- Count=1/1 Maskable+ 64bit+
> > 	Capabilities: [70] MSI-X: Enable+ Count=5 Masked-
> > 	Capabilities: [a0] Express Endpoint, MSI 00
> > 	Capabilities: [100] Advanced Error Reporting
> > 	Capabilities: [140] Device Serial Number ...
> > 	Capabilities: [1c0] Latency Tolerance Reporting
> > 	Capabilities: [1f0] Precision Time Measurement
> > 	Capabilities: [1e0] L1 PM Substates
> > 	Kernel driver in use: igc
> > 	Kernel modules: igc
> > 
> > With the current Xen version, it uses MSI-X under PV and MSI under HVM.
> > 
> > This patch moves domU config to a variable, to make it configurable on
> > per-test basis. Add also a few comments for visual separation of tests.
> > 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> > ---
> >  automation/gitlab-ci/test.yaml     | 20 ++++++++-
> >  automation/scripts/qubes-x86-64.sh | 80 ++++++++++++++++++++++++++-----
> >  2 files changed, 89 insertions(+), 11 deletions(-)
> > 
> > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> > index d68c584269dd..1ce083e6cd88 100644
> > --- a/automation/gitlab-ci/test.yaml
> > +++ b/automation/gitlab-ci/test.yaml
> > @@ -94,6 +94,8 @@
> >      # the test controller runs on RPi4
> >      CONTAINER: alpine:3.12-arm64v8
> >      LOGFILE: smoke-test.log
> > +    PCIDEV: "03:00.0"
> > +    PCIDEV_INTR: "MSI-X"
> 
> This is minor but I would move PCIDEV_INTR to
> adl-pci-pv-x86-64-gcc-debug given that adl-pci-hvm-x86-64-gcc-debug
> already redefines it.

The device is MSI-X capable and I'd expect Linux should use MSI-X. My
guess is it's using MSI in HVM because MSI-X doesn't work (and I hope my
other series will fix that), but I have _not_ verified this theory.
If I'm right, I hope the PCIDEV_INTR="MSI" will go away from the HVM
test, and it will be testing if MSI-X didn't regressed.

> I would also move PCIDEV to adl-pci-pv-x86-64-gcc-debug and
> adl-pci-hvm-x86-64-gcc-debug, but I am fine either way.

My idea is those both are more of a test target properties, not an
individual test properties.


> However the two new tests failed for me:
> 
> https://gitlab.com/xen-project/people/sstabellini/xen/-/pipelines/847157948
> 
> 
> + grep '^Welcome to Alpine Linux' smoke.serial
> + '[' 0 -le 0 ]
> + '[' 0 -le 0 ]
> + echo 'ERROR: test timeout, aborting'
> ERROR: test timeout, aborting
> 
> The Welcome to Alpine Linux message is missing

Ah, I forgot to remove debug shell...

I'll post v2 once it passes test run (waiting in the queue...).
-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]