All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] automation: Support running FVP Dom0 smoke test for Arm
@ 2023-12-08  5:46 Henry Wang
  2023-12-08  5:46 ` [PATCH v2 1/5] automation: Add a Dockerfile for running FVP_Base jobs Henry Wang
                   ` (4 more replies)
  0 siblings, 5 replies; 26+ messages in thread
From: Henry Wang @ 2023-12-08  5:46 UTC (permalink / raw)
  To: xen-devel
  Cc: Henry Wang, Doug Goldstein, Stefano Stabellini, Michal Orzel,
	Julien Grall, Bertrand Marquis, Wei Chen

This series adds the support for running FVP Dom0 smoke test for Arm on
the Arm64 GitLab CI runner. Detailed changes please refer to the commit
message of each commit.

An example test pipeline with these patches applied (with the docker
registry changed to my own registry and unrelated job removed) can be
found at:
https://gitlab.com/xen-project/people/henryw/xen/-/pipelines/1099757245

The second example of a negative test with breaking the expect script
by a "never met" condition can be found at:
https://gitlab.com/xen-project/people/henryw/xen/-/pipelines/1099757601
The job will fail as expected after the timeout set by the expect
script.

Henry Wang (5):
  automation: Add a Dockerfile for running FVP_Base jobs
  automation: Add the Dockerfile to build TF-A and U-Boot for FVP
  automation: Add the expect script with test case for FVP
  automation: Add the script for the FVP smoke test
  automation: Add the arm64 FVP build and Dom0 smoke test jobs

 .../debian/bookworm-arm64v8-fvp.dockerfile    |  64 ++++++++++
 automation/gitlab-ci/build.yaml               |  17 +++
 automation/gitlab-ci/test.yaml                |  22 ++++
 .../expect/fvp-base-smoke-dom0-arm64.exp      |  73 +++++++++++
 .../scripts/fvp-base-smoke-dom0-arm64.sh      | 120 ++++++++++++++++++
 .../2023.10-2.9.0-arm64v8.dockerfile          |  48 +++++++
 6 files changed, 344 insertions(+)
 create mode 100644 automation/build/debian/bookworm-arm64v8-fvp.dockerfile
 create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
 create mode 100755 automation/scripts/fvp-base-smoke-dom0-arm64.sh
 create mode 100644 automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile

-- 
2.25.1



^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 1/5] automation: Add a Dockerfile for running FVP_Base jobs
  2023-12-08  5:46 [PATCH v2 0/5] automation: Support running FVP Dom0 smoke test for Arm Henry Wang
@ 2023-12-08  5:46 ` Henry Wang
  2023-12-08 12:30   ` Julien Grall
  2023-12-08  5:46 ` [PATCH v2 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP Henry Wang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 26+ messages in thread
From: Henry Wang @ 2023-12-08  5:46 UTC (permalink / raw)
  To: xen-devel
  Cc: Henry Wang, Doug Goldstein, Stefano Stabellini, Michal Orzel,
	Julien Grall, Bertrand Marquis, Wei Chen

Fixed Virtual Platforms (FVPs) are complete simulations of an Arm
system, including processor, memory and peripherals. These are set
out in a "programmer's view", which gives programmers a comprehensive
model on which to build and test software. FVP can be configured to
different setups by its cmdline parameters, and hence having the FVP
in CI will provide us with the flexibility to test Arm features and
setups that we find difficult to use real hardware or emulators.

This commit adds a Dockerfile for the new arm64v8 container with
FVP installed, based on the debian bookworm-arm64v8 image. This
container will be used to run the FVP test jobs. Compared to the
debian bookworm-arm64v8 image, the packages in the newly added FVP
container does not contain the `u-boot-qemu`, and adds the `expect`
to run expect scripts introduced by following commits, `telnet` to
connect to FVP, and `tftpd-hpa` to provide the TFTP service for
the FVP.

Signed-off-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
v2:
- Add Stefano's Reviewed-by tag.
---
 .../debian/bookworm-arm64v8-fvp.dockerfile    | 64 +++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 automation/build/debian/bookworm-arm64v8-fvp.dockerfile

diff --git a/automation/build/debian/bookworm-arm64v8-fvp.dockerfile b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
new file mode 100644
index 0000000000..3b87dc5a5b
--- /dev/null
+++ b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
@@ -0,0 +1,64 @@
+FROM --platform=linux/arm64/v8 debian:bookworm
+LABEL maintainer.name="The Xen Project" \
+      maintainer.email="xen-devel@lists.xenproject.org"
+
+ARG FVP_BASE_VERSION="11.23_9_Linux64_armv8l"
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV USER root
+
+RUN mkdir /build
+WORKDIR /build
+
+# build depends
+RUN apt-get update && \
+    apt-get --quiet --yes install \
+        build-essential \
+        zlib1g-dev \
+        libncurses5-dev \
+        libssl-dev \
+        python3-dev \
+        python3-setuptools \
+        xorg-dev \
+        uuid-dev \
+        libyajl-dev \
+        libaio-dev \
+        libglib2.0-dev \
+        clang \
+        libpixman-1-dev \
+        pkg-config \
+        flex \
+        bison \
+        acpica-tools \
+        libfdt-dev \
+        bin86 \
+        bcc \
+        liblzma-dev \
+        libnl-3-dev \
+        ocaml-nox \
+        libfindlib-ocaml-dev \
+        markdown \
+        transfig \
+        pandoc \
+        checkpolicy \
+        wget \
+        git \
+        nasm \
+        # for test phase, fvp-smoke-* jobs
+        u-boot-tools \
+        expect \
+        device-tree-compiler \
+        curl \
+        cpio \
+        busybox-static \
+        telnet \
+        tftpd-hpa \
+        && \
+        apt-get autoremove -y && \
+        apt-get clean && \
+        rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
+
+RUN wget https://developer.arm.com/-/media/Files/downloads/ecosystem-models/FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz && \
+    mkdir -p /FVP/FVP_Base_RevC-2xAEMvA && \
+    tar -xvzf FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz -C /FVP/FVP_Base_RevC-2xAEMvA && \
+    rm FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP
  2023-12-08  5:46 [PATCH v2 0/5] automation: Support running FVP Dom0 smoke test for Arm Henry Wang
  2023-12-08  5:46 ` [PATCH v2 1/5] automation: Add a Dockerfile for running FVP_Base jobs Henry Wang
@ 2023-12-08  5:46 ` Henry Wang
  2023-12-08  8:39   ` Michal Orzel
  2023-12-08  5:46 ` [PATCH v2 3/5] automation: Add the expect script with test case " Henry Wang
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 26+ messages in thread
From: Henry Wang @ 2023-12-08  5:46 UTC (permalink / raw)
  To: xen-devel
  Cc: Henry Wang, Doug Goldstein, Stefano Stabellini, Michal Orzel,
	Julien Grall, Bertrand Marquis, Wei Chen

Unlike the emulators that currently being used in the CI pipelines,
the FVP must start at EL3. Therefore we need the firmware, i.e. the
TrustedFirmware-A (TF-A), for corresponding functionality.

There is a dedicated board (vexpress_fvp) in U-Boot (serve as the
BL33 of the TF-A) for the FVP platform, so the U-Boot should also be
compiled for the FVP platform instead of reusing the U-Boot for the
existing emulators used in the CI pipelines.

To avoid compiling TF-A and U-Boot everytime in the job, adding a
Dockerfile to the test artifacts to build TF-A v2.9.0 and U-Boot
v2023.10 for FVP. The binaries for the TF-A and U-Boot, as well as
the device tree for the FVP platform, will be saved (and exported by
the CI job introduced by following commits). Note that, a patch for
the TF-A will be applied before building to enable the virtio-net
and the virtio-rng device on the FVP. The virtio-net device will
provide the networking service for FVP, and the virtio-rng device
will improve the speed of the FVP.

Signed-off-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
v2:
- Add Stefano's Reviewed-by tag.
---
 .../2023.10-2.9.0-arm64v8.dockerfile          | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile

diff --git a/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile b/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
new file mode 100644
index 0000000000..6566b60545
--- /dev/null
+++ b/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
@@ -0,0 +1,48 @@
+FROM --platform=linux/arm64/v8 debian:bookworm
+LABEL maintainer.name="The Xen Project" \
+      maintainer.email="xen-devel@lists.xenproject.org"
+
+ENV DEBIAN_FRONTEND=noninteractive
+ENV UBOOT_VERSION="2023.10"
+ENV TFA_VERSION="v2.9.0"
+ENV USER root
+
+RUN mkdir /build
+WORKDIR /build
+
+# build depends
+RUN apt-get update && \
+    apt-get --quiet --yes install \
+        build-essential \
+        libssl-dev \
+        bc \
+        curl \
+        flex \
+        bison \
+        git \
+        device-tree-compiler && \
+    apt-get autoremove -y && \
+    apt-get clean && \
+    rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
+
+# Build U-Boot and TF-A
+RUN curl -fsSLO https://ftp.denx.de/pub/u-boot/u-boot-"$UBOOT_VERSION".tar.bz2 && \
+    tar xvf u-boot-"$UBOOT_VERSION".tar.bz2 && \
+    cd u-boot-"$UBOOT_VERSION" && \
+    make -j$(nproc) V=1 vexpress_fvp_defconfig && \
+    make -j$(nproc) V=1 all && \
+    cd .. && \
+    git clone --branch "$TFA_VERSION" --depth 1 https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git && \
+    cd trusted-firmware-a && \
+    curl -fsSLO https://git.yoctoproject.org/meta-arm/plain/meta-arm-bsp/recipes-bsp/trusted-firmware-a/files/fvp-base/0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch \
+         --output 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
+    git config --global user.email "you@example.com" && \
+    git config --global user.name "Your Name" && \
+    git am 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
+    make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 FVP_DT_PREFIX=fvp-base-gicv3-psci-1t all && \
+    make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 FVP_DT_PREFIX=fvp-base-gicv3-psci-1t fip BL33=../u-boot-"$UBOOT_VERSION"/u-boot.bin && \
+    cp build/fvp/debug/bl1.bin / && \
+    cp build/fvp/debug/fip.bin / && \
+    cp build/fvp/debug/fdts/fvp-base-gicv3-psci-1t.dtb / && \
+    cd /build && \
+    rm -rf u-boot-"$UBOOT_VERSION" trusted-firmware-a
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08  5:46 [PATCH v2 0/5] automation: Support running FVP Dom0 smoke test for Arm Henry Wang
  2023-12-08  5:46 ` [PATCH v2 1/5] automation: Add a Dockerfile for running FVP_Base jobs Henry Wang
  2023-12-08  5:46 ` [PATCH v2 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP Henry Wang
@ 2023-12-08  5:46 ` Henry Wang
  2023-12-08  8:57   ` Michal Orzel
  2023-12-08  5:46 ` [PATCH v2 4/5] automation: Add the script for the FVP smoke test Henry Wang
  2023-12-08  5:46 ` [PATCH v2 5/5] automation: Add the arm64 FVP build and Dom0 smoke test jobs Henry Wang
  4 siblings, 1 reply; 26+ messages in thread
From: Henry Wang @ 2023-12-08  5:46 UTC (permalink / raw)
  To: xen-devel
  Cc: Henry Wang, Doug Goldstein, Stefano Stabellini, Michal Orzel,
	Julien Grall, Bertrand Marquis, Wei Chen

To interact with the FVP (for example entering the U-Boot shell
and transferring the files by TFTP), we need to connect the
corresponding port by the telnet first. Use an expect script to
simplify the automation of the whole "interacting with FVP" stuff.

The expect script will firstly detect the IP of the host, then
connect to the telnet port of the FVP, set the `serverip` and `ipaddr`
for the TFTP service in the U-Boot shell, and finally boot Xen from
U-Boot and wait for the expected results by Xen, Dom0 and DomU.

Signed-off-by: Henry Wang <Henry.Wang@arm.com>
---
v2:
- No change.
---
 .../expect/fvp-base-smoke-dom0-arm64.exp      | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp

diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
new file mode 100755
index 0000000000..25d9a5f81c
--- /dev/null
+++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
@@ -0,0 +1,73 @@
+#!/usr/bin/expect
+
+set timeout 2000
+
+# Command to use to run must be given as first argument
+# if options are required, quotes must be used:
+# xxx.exp "cmd opt1 opt2"
+set runcmd [lindex $argv 0]
+
+# Maximum number of line to be printed, this can be used to prevent runs to
+# go forever on errors when Xen is rebooting
+set maxline 1000
+
+# Configure slow parameters used with send -s
+# This allows us to slow down console writes to prevent issues with slow
+# emulators or targets.
+# Format here is: {NUM TIME} which reads as wait TIME seconds each NUM of
+# characters, here we send 1 char each 100ms
+set send_slow {1 .1}
+
+proc test_boot {{maxline} {host_ip}} {
+    expect_after {
+        -re "(.*)\r" {
+            if {$maxline != 0} {
+                set maxline [expr {$maxline - 1}]
+                if {$maxline <= 0} {
+                    send_user "ERROR-Toomuch!\n"
+                    exit 2
+                }
+            }
+            exp_continue
+        }
+        timeout {send_user "ERROR-Timeout!\n"; exit 3}
+        eof {send_user "ERROR-EOF!\n"; exit 4}
+    }
+
+    # Extract the telnet port numbers from FVP output, because the telnet ports
+    # are not guaranteed to be fixed numbers.
+    expect -re {terminal_0: Listening for serial connection on port [0-9]+}
+    set terminal_0 $expect_out(0,string)
+    if {[regexp {port (\d+)} $terminal_0 match port_0]} {
+        puts "terminal_0 port is $port_0"
+    } else {
+        puts "terminal_0 port not found"
+        exit 5
+    }
+
+    spawn bash -c "telnet localhost $port_0"
+    expect -re "Hit any key to stop autoboot.*"
+    send -s "  \r"
+    send -s "setenv serverip $host_ip; setenv ipaddr $host_ip; tftpb 0x80200000 boot.scr; source 0x80200000\r"
+
+    # Initial Xen boot
+    expect -re "\(XEN\).*Freed .* init memory."
+
+    # Dom0 and DomU
+    expect -re "Domain-0.*"
+    expect -re "BusyBox.*"
+    expect -re "/ #.*"
+}
+
+# Get host IP
+spawn bash -c "hostname -I | awk '{print \$1}'"
+expect -re {(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})}
+set host_ip $expect_out(0,string)
+
+# Start the FVP and run the test
+spawn bash -c "$runcmd"
+
+test_boot 2000 "$host_ip"
+
+send_user "\nExecution with SUCCESS\n"
+exit 0
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 4/5] automation: Add the script for the FVP smoke test
  2023-12-08  5:46 [PATCH v2 0/5] automation: Support running FVP Dom0 smoke test for Arm Henry Wang
                   ` (2 preceding siblings ...)
  2023-12-08  5:46 ` [PATCH v2 3/5] automation: Add the expect script with test case " Henry Wang
@ 2023-12-08  5:46 ` Henry Wang
  2023-12-08 21:46   ` Stefano Stabellini
  2023-12-08  5:46 ` [PATCH v2 5/5] automation: Add the arm64 FVP build and Dom0 smoke test jobs Henry Wang
  4 siblings, 1 reply; 26+ messages in thread
From: Henry Wang @ 2023-12-08  5:46 UTC (permalink / raw)
  To: xen-devel
  Cc: Henry Wang, Doug Goldstein, Stefano Stabellini, Michal Orzel,
	Julien Grall, Bertrand Marquis, Wei Chen

This commit adds the shell script for the FVP smoke test. Similarly
as the QEMU jobs, the shell script will firstly prepare the DomU
BusyBox image, use the ImageBuilder to arrange the binaries in memory
and generate the U-Boot script, then start the test. To provide the
TFTP service for the FVP, the shell script will also start the TFTP
service, and copy the binaries needed by test to the TFTP directory
used by the TFTP server.

Signed-off-by: Henry Wang <Henry.Wang@arm.com>
---
v2:
- Set pipefail before running the expect script, so that the error
  won't be hid by pipe and the tee to the logfile.
---
 .../scripts/fvp-base-smoke-dom0-arm64.sh      | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100755 automation/scripts/fvp-base-smoke-dom0-arm64.sh

diff --git a/automation/scripts/fvp-base-smoke-dom0-arm64.sh b/automation/scripts/fvp-base-smoke-dom0-arm64.sh
new file mode 100755
index 0000000000..99097dad51
--- /dev/null
+++ b/automation/scripts/fvp-base-smoke-dom0-arm64.sh
@@ -0,0 +1,120 @@
+#!/bin/bash
+
+set -ex
+
+# DomU Busybox
+cd binaries
+mkdir -p initrd
+mkdir -p initrd/bin
+mkdir -p initrd/sbin
+mkdir -p initrd/etc
+mkdir -p initrd/dev
+mkdir -p initrd/proc
+mkdir -p initrd/sys
+mkdir -p initrd/lib
+mkdir -p initrd/var
+mkdir -p initrd/mnt
+cp /bin/busybox initrd/bin/busybox
+initrd/bin/busybox --install initrd/bin
+echo "#!/bin/sh
+
+mount -t proc proc /proc
+mount -t sysfs sysfs /sys
+mount -t devtmpfs devtmpfs /dev
+/bin/sh" > initrd/init
+chmod +x initrd/init
+cd initrd
+find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
+cd ..
+
+mkdir -p rootfs
+cd rootfs
+tar xvzf ../initrd.tar.gz
+mkdir proc
+mkdir run
+mkdir srv
+mkdir sys
+rm var/run
+cp -ar ../dist/install/* .
+mv ../initrd.cpio.gz ./root
+cp ../Image ./root
+echo "name=\"test\"
+memory=512
+vcpus=1
+kernel=\"/root/Image\"
+ramdisk=\"/root/initrd.cpio.gz\"
+extra=\"console=hvc0 root=/dev/ram0 rdinit=/bin/sh\"
+" > root/test.cfg
+echo "#!/bin/bash
+
+export LD_LIBRARY_PATH=/usr/local/lib
+bash /etc/init.d/xencommons start
+
+xl list
+
+xl create -c /root/test.cfg
+
+" > etc/local.d/xen.start
+chmod +x etc/local.d/xen.start
+echo "rc_verbose=yes" >> etc/rc.conf
+find . |cpio -H newc -o|gzip > ../xen-rootfs.cpio.gz
+cd ../..
+
+# Start a TFTP server to provide TFTP service to FVP
+service tftpd-hpa start
+
+# ImageBuilder
+echo 'MEMORY_START="0x80000000"
+MEMORY_END="0xFF000000"
+
+DEVICE_TREE="fvp-base-gicv3-psci-1t.dtb"
+XEN="xen"
+DOM0_KERNEL="Image"
+DOM0_RAMDISK="xen-rootfs.cpio.gz"
+XEN_CMD="console=dtuart dom0_mem=1024M console_timestamps=boot"
+
+NUM_DOMUS=0
+
+LOAD_CMD="tftpb"
+UBOOT_SOURCE="boot.source"
+UBOOT_SCRIPT="boot.scr"' > binaries/config
+rm -rf imagebuilder
+git clone https://gitlab.com/ViryaOS/imagebuilder
+bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c binaries/config
+
+# Copy files to the TFTP directory to use
+cp ./binaries/boot.scr /srv/tftp/
+cp ./binaries/Image /srv/tftp/
+cp ./binaries/xen-rootfs.cpio.gz /srv/tftp/
+cp ./binaries/xen /srv/tftp/
+cp ./binaries/fvp-base-gicv3-psci-1t.dtb /srv/tftp/
+
+# Start FVP
+TERM0_CFG="-C bp.terminal_0.mode=telnet -C bp.terminal_0.start_telnet=0"
+TERM1_CFG="-C bp.terminal_1.mode=telnet -C bp.terminal_1.start_telnet=0"
+TERM2_CFG="-C bp.terminal_2.mode=telnet -C bp.terminal_2.start_telnet=0"
+TERM3_CFG="-C bp.terminal_3.mode=telnet -C bp.terminal_3.start_telnet=0"
+
+VIRTIO_USER_NETWORK_CFG="-C bp.virtio_net.enabled=1 \
+-C bp.virtio_net.hostbridge.userNetworking=1 \
+-C bp.virtio_net.hostbridge.userNetPorts=8022=22 \
+-C bp.virtio_net.transport=legacy"
+
+# Set the pipefail so that the error code from the expect script won't
+# be hid by pipe and the tee command.
+set -o pipefail
+./automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp \
+    "/FVP/FVP_Base_RevC-2xAEMvA/Base_RevC_AEMvA_pkg/models/Linux64_armv8l_GCC-9.3/FVP_Base_RevC-2xAEMvA \
+    -C bp.vis.disable_visualisation=1 \
+    -C bp.ve_sysregs.exit_on_shutdown=1 \
+    -C bp.secure_memory=0 \
+    -C cache_state_modelled=0 \
+    -C cluster0.has_arm_v8-4=1 \
+    -C cluster1.has_arm_v8-4=1 \
+    ${TERM0_CFG} ${TERM1_CFG} ${TERM2_CFG} ${TERM3_CFG} \
+    ${VIRTIO_USER_NETWORK_CFG} \
+    -C bp.secureflashloader.fname=$(pwd)/binaries/bl1.bin \
+    -C bp.flashloader0.fname=$(pwd)/binaries/fip.bin" |& \
+        tee smoke.serial
+
+exit 0
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 5/5] automation: Add the arm64 FVP build and Dom0 smoke test jobs
  2023-12-08  5:46 [PATCH v2 0/5] automation: Support running FVP Dom0 smoke test for Arm Henry Wang
                   ` (3 preceding siblings ...)
  2023-12-08  5:46 ` [PATCH v2 4/5] automation: Add the script for the FVP smoke test Henry Wang
@ 2023-12-08  5:46 ` Henry Wang
  2023-12-08  9:19   ` Michal Orzel
  4 siblings, 1 reply; 26+ messages in thread
From: Henry Wang @ 2023-12-08  5:46 UTC (permalink / raw)
  To: xen-devel
  Cc: Henry Wang, Doug Goldstein, Stefano Stabellini, Michal Orzel,
	Julien Grall, Bertrand Marquis, Wei Chen

Add a job in the build stage to export the TF-A, U-Boot and the
device tree for the FVP platform from the test artifact container.

Add a FVP smoke test job in the test stage to do the same test as
the `qemu-smoke-dom0-arm64-gcc` job.

Signed-off-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
v2:
- Add Stefano's Reviewed-by tag.

Although it does not affect the functionality, I am still quite
confused about why the logs displayed by GitLab UI, for
example [1], is much less than the actual output (saved in the
artifacts, see [2]). Had a discussion with Michal on Matrix
and we agree that the log in gitlab UI is usually capped.

[1] https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5690876361
[2] https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5690876361/artifacts/file/smoke.serial
---
 automation/gitlab-ci/build.yaml | 17 +++++++++++++++++
 automation/gitlab-ci/test.yaml  | 22 ++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index 32af30cced..89d2f01302 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -344,6 +344,23 @@ kernel-6.1.19-export:
   tags:
     - x86_64
 
+armfvp-uboot-tfa-2023.10-2.9.0-arm64-export:
+  extends: .test-jobs-artifact-common
+  image: registry.gitlab.com/xen-project/xen/tests-artifacts/armfvp-uboot-tfa:2023.10-2.9.0-arm64v8
+  script:
+    - |
+       mkdir binaries && \
+       cp /bl1.bin binaries/bl1.bin && \
+       cp /fip.bin binaries/fip.bin && \
+       cp /fvp-base-gicv3-psci-1t.dtb binaries/fvp-base-gicv3-psci-1t.dtb
+  artifacts:
+    paths:
+      - binaries/bl1.bin
+      - binaries/fip.bin
+      - binaries/fvp-base-gicv3-psci-1t.dtb
+  tags:
+    - arm64
+
 # Jobs below this line
 
 # Build jobs needed for tests
diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 6aabdb9d15..47e00d0a0b 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -96,6 +96,19 @@
   tags:
     - xilinx
 
+.fvp-arm64:
+  extends: .test-jobs-common
+  variables:
+    CONTAINER: debian:bookworm-arm64v8-fvp
+    LOGFILE: fvp-smoke-arm64.log
+  artifacts:
+    paths:
+      - smoke.serial
+      - '*.log'
+    when: always
+  tags:
+    - arm64
+
 .adl-x86-64:
   extends: .test-jobs-common
   variables:
@@ -459,3 +472,12 @@ qemu-smoke-ppc64le-powernv9-gcc:
   needs:
     - qemu-system-ppc64-8.1.0-ppc64-export
     - debian-bullseye-gcc-ppc64le-debug
+
+fvp-smoke-dom0-arm64-gcc-debug:
+  extends: .fvp-arm64
+  script:
+    - ./automation/scripts/fvp-base-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE}
+  needs:
+    - *arm64-test-needs
+    - armfvp-uboot-tfa-2023.10-2.9.0-arm64-export
+    - alpine-3.18-gcc-debug-arm64
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP
  2023-12-08  5:46 ` [PATCH v2 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP Henry Wang
@ 2023-12-08  8:39   ` Michal Orzel
  2023-12-08  8:56     ` Henry Wang
  0 siblings, 1 reply; 26+ messages in thread
From: Michal Orzel @ 2023-12-08  8:39 UTC (permalink / raw)
  To: Henry Wang, xen-devel
  Cc: Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Wei Chen

Hi Henry,

On 08/12/2023 06:46, Henry Wang wrote:
> 
> 
> Unlike the emulators that currently being used in the CI pipelines,
> the FVP must start at EL3. Therefore we need the firmware, i.e. the
> TrustedFirmware-A (TF-A), for corresponding functionality.
> 
> There is a dedicated board (vexpress_fvp) in U-Boot (serve as the
> BL33 of the TF-A) for the FVP platform, so the U-Boot should also be
> compiled for the FVP platform instead of reusing the U-Boot for the
> existing emulators used in the CI pipelines.
> 
> To avoid compiling TF-A and U-Boot everytime in the job, adding a
> Dockerfile to the test artifacts to build TF-A v2.9.0 and U-Boot
> v2023.10 for FVP. The binaries for the TF-A and U-Boot, as well as
> the device tree for the FVP platform, will be saved (and exported by
> the CI job introduced by following commits). Note that, a patch for
> the TF-A will be applied before building to enable the virtio-net
> and the virtio-rng device on the FVP. The virtio-net device will
> provide the networking service for FVP, and the virtio-rng device
> will improve the speed of the FVP.
> 
> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> v2:
> - Add Stefano's Reviewed-by tag.
> ---
>  .../2023.10-2.9.0-arm64v8.dockerfile          | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
> 
> diff --git a/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile b/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
> new file mode 100644
> index 0000000000..6566b60545
> --- /dev/null
> +++ b/automation/tests-artifacts/armfvp-uboot-tfa/2023.10-2.9.0-arm64v8.dockerfile
> @@ -0,0 +1,48 @@
> +FROM --platform=linux/arm64/v8 debian:bookworm
> +LABEL maintainer.name="The Xen Project" \
> +      maintainer.email="xen-devel@lists.xenproject.org"
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +ENV UBOOT_VERSION="2023.10"
> +ENV TFA_VERSION="v2.9.0"
> +ENV USER root
> +
> +RUN mkdir /build
> +WORKDIR /build
> +
> +# build depends
> +RUN apt-get update && \
> +    apt-get --quiet --yes install \
> +        build-essential \
> +        libssl-dev \
> +        bc \
> +        curl \
> +        flex \
> +        bison \
> +        git \
> +        device-tree-compiler && \
> +    apt-get autoremove -y && \
> +    apt-get clean && \
> +    rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
> +
> +# Build U-Boot and TF-A
> +RUN curl -fsSLO https://ftp.denx.de/pub/u-boot/u-boot-"$UBOOT_VERSION".tar.bz2 && \
> +    tar xvf u-boot-"$UBOOT_VERSION".tar.bz2 && \
> +    cd u-boot-"$UBOOT_VERSION" && \
> +    make -j$(nproc) V=1 vexpress_fvp_defconfig && \
> +    make -j$(nproc) V=1 all && \
Do we need 'all'? Can't we just build target 'u-boot' for u-boot.bin?

> +    cd .. && \
> +    git clone --branch "$TFA_VERSION" --depth 1 https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git && \
> +    cd trusted-firmware-a && \
> +    curl -fsSLO https://git.yoctoproject.org/meta-arm/plain/meta-arm-bsp/recipes-bsp/trusted-firmware-a/files/fvp-base/0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch \
> +         --output 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
> +    git config --global user.email "you@example.com" && \
> +    git config --global user.name "Your Name" && \
If this is needed for git am, you could get away using 'patch -p1'

> +    git am 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
> +    make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 FVP_DT_PREFIX=fvp-base-gicv3-psci-1t all && \
> +    make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 FVP_DT_PREFIX=fvp-base-gicv3-psci-1t fip BL33=../u-boot-"$UBOOT_VERSION"/u-boot.bin && \
> +    cp build/fvp/debug/bl1.bin / && \
> +    cp build/fvp/debug/fip.bin / && \
> +    cp build/fvp/debug/fdts/fvp-base-gicv3-psci-1t.dtb / && \
> +    cd /build && \
> +    rm -rf u-boot-"$UBOOT_VERSION" trusted-firmware-a
You forgot to remove u-boot tar file

Other than that:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP
  2023-12-08  8:39   ` Michal Orzel
@ 2023-12-08  8:56     ` Henry Wang
  2023-12-08 21:20       ` Stefano Stabellini
  0 siblings, 1 reply; 26+ messages in thread
From: Henry Wang @ 2023-12-08  8:56 UTC (permalink / raw)
  To: Michal Orzel
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Wei Chen

Hi Michal,

> On Dec 8, 2023, at 16:39, Michal Orzel <michal.orzel@amd.com> wrote:
> 
> Hi Henry,
> 
> On 08/12/2023 06:46, Henry Wang wrote:
>> 
>> Unlike the emulators that currently being used in the CI pipelines,
>> the FVP must start at EL3. Therefore we need the firmware, i.e. the
>> TrustedFirmware-A (TF-A), for corresponding functionality.
>> 
>> There is a dedicated board (vexpress_fvp) in U-Boot (serve as the
>> BL33 of the TF-A) for the FVP platform, so the U-Boot should also be
>> compiled for the FVP platform instead of reusing the U-Boot for the
>> existing emulators used in the CI pipelines.
>> 
>> To avoid compiling TF-A and U-Boot everytime in the job, adding a
>> Dockerfile to the test artifacts to build TF-A v2.9.0 and U-Boot
>> v2023.10 for FVP. The binaries for the TF-A and U-Boot, as well as
>> the device tree for the FVP platform, will be saved (and exported by
>> the CI job introduced by following commits). Note that, a patch for
>> the TF-A will be applied before building to enable the virtio-net
>> and the virtio-rng device on the FVP. The virtio-net device will
>> provide the networking service for FVP, and the virtio-rng device
>> will improve the speed of the FVP.
>> 
>> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>> ---
>> v2:
>> - Add Stefano's Reviewed-by tag.
>> ---
>> +# Build U-Boot and TF-A
>> +RUN curl -fsSLO https://ftp.denx.de/pub/u-boot/u-boot-"$UBOOT_VERSION".tar.bz2 && \
>> +    tar xvf u-boot-"$UBOOT_VERSION".tar.bz2 && \
>> +    cd u-boot-"$UBOOT_VERSION" && \
>> +    make -j$(nproc) V=1 vexpress_fvp_defconfig && \
>> +    make -j$(nproc) V=1 all && \
> Do we need 'all'? Can't we just build target 'u-boot' for u-boot.bin?

I think your suggestion makes sense, and I can have a try, if changing all to u-boot works,
I will use that in v3.

>> +    cd .. && \
>> +    git clone --branch "$TFA_VERSION" --depth 1 https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git && \
>> +    cd trusted-firmware-a && \
>> +    curl -fsSLO https://git.yoctoproject.org/meta-arm/plain/meta-arm-bsp/recipes-bsp/trusted-firmware-a/files/fvp-base/0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch \
>> +         --output 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
>> +    git config --global user.email "you@example.com" && \
>> +    git config --global user.name "Your Name" && \
> If this is needed for git am, you could get away using 'patch -p1'

Hmmm right, then probably we can even not install git and use the tarball instead of
git clone.


>> +    git am 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
>> +    make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 FVP_DT_PREFIX=fvp-base-gicv3-psci-1t all && \
>> +    make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 FVP_DT_PREFIX=fvp-base-gicv3-psci-1t fip BL33=../u-boot-"$UBOOT_VERSION"/u-boot.bin && \
>> +    cp build/fvp/debug/bl1.bin / && \
>> +    cp build/fvp/debug/fip.bin / && \
>> +    cp build/fvp/debug/fdts/fvp-base-gicv3-psci-1t.dtb / && \
>> +    cd /build && \
>> +    rm -rf u-boot-"$UBOOT_VERSION" trusted-firmware-a
> You forgot to remove u-boot tar file

oops, nice catch, thanks. Will also remove that in v3.

> Other than that:
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Thanks!

Stefano: Can I keep your Reviewed-by tag after addressing Michal’s comments above?

Kind regards,
Henry

> 
> ~Michal


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08  5:46 ` [PATCH v2 3/5] automation: Add the expect script with test case " Henry Wang
@ 2023-12-08  8:57   ` Michal Orzel
  2023-12-08  9:05     ` Henry Wang
  0 siblings, 1 reply; 26+ messages in thread
From: Michal Orzel @ 2023-12-08  8:57 UTC (permalink / raw)
  To: Henry Wang, xen-devel
  Cc: Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Wei Chen

Hi Henry,

On 08/12/2023 06:46, Henry Wang wrote:
> 
> 
> To interact with the FVP (for example entering the U-Boot shell
> and transferring the files by TFTP), we need to connect the
> corresponding port by the telnet first. Use an expect script to
> simplify the automation of the whole "interacting with FVP" stuff.
> 
> The expect script will firstly detect the IP of the host, then
> connect to the telnet port of the FVP, set the `serverip` and `ipaddr`
> for the TFTP service in the U-Boot shell, and finally boot Xen from
> U-Boot and wait for the expected results by Xen, Dom0 and DomU.
> 
> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

with 1 question...

> ---
> v2:
> - No change.
> ---
>  .../expect/fvp-base-smoke-dom0-arm64.exp      | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
> 
> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
> new file mode 100755
> index 0000000000..25d9a5f81c
> --- /dev/null
> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
> @@ -0,0 +1,73 @@
> +#!/usr/bin/expect
> +
> +set timeout 2000
Do we really need such a big timeout (~30 min)?
Looking at your test job, it took 16 mins (quite a lot but I know FVP is slow
+ send_slow slows things down)

~Michal



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08  8:57   ` Michal Orzel
@ 2023-12-08  9:05     ` Henry Wang
  2023-12-08  9:11       ` Michal Orzel
  0 siblings, 1 reply; 26+ messages in thread
From: Henry Wang @ 2023-12-08  9:05 UTC (permalink / raw)
  To: Michal Orzel
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Wei Chen

Hi Michal,

> On Dec 8, 2023, at 16:57, Michal Orzel <michal.orzel@amd.com> wrote:
> 
> Hi Henry,
> 
> On 08/12/2023 06:46, Henry Wang wrote:
>> 
>> 
>> To interact with the FVP (for example entering the U-Boot shell
>> and transferring the files by TFTP), we need to connect the
>> corresponding port by the telnet first. Use an expect script to
>> simplify the automation of the whole "interacting with FVP" stuff.
>> 
>> The expect script will firstly detect the IP of the host, then
>> connect to the telnet port of the FVP, set the `serverip` and `ipaddr`
>> for the TFTP service in the U-Boot shell, and finally boot Xen from
>> U-Boot and wait for the expected results by Xen, Dom0 and DomU.
>> 
>> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Thanks!

> with 1 question...
> 
>> ---
>> v2:
>> - No change.
>> ---
>> .../expect/fvp-base-smoke-dom0-arm64.exp      | 73 +++++++++++++++++++
>> 1 file changed, 73 insertions(+)
>> create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>> 
>> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>> new file mode 100755
>> index 0000000000..25d9a5f81c
>> --- /dev/null
>> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>> @@ -0,0 +1,73 @@
>> +#!/usr/bin/expect
>> +
>> +set timeout 2000
> Do we really need such a big timeout (~30 min)?
> Looking at your test job, it took 16 mins (quite a lot but I know FVP is slow
> + send_slow slows things down)

This is a really good question. I did have the same question while working on
the negative test today. The timeout 2000 indeed will fail the job at about 30min,
and waiting for it is indeed not really pleasant.

But my second thought would be - from my observation, the overall time now
would vary between 15min ~ 20min, and having a 10min margin is not that crazy
given that we probably will do more testing from the job in the future, and if the
GitLab Arm worker is high loaded, FVP will probably become slower. And normally
we don’t even trigger the timeout as the job will normally pass. So I decided
to keep this.

Mind sharing your thoughts about the better value of the timeout? Probably 25min?

Kind regards,
Henry

> 
> ~Michal
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08  9:05     ` Henry Wang
@ 2023-12-08  9:11       ` Michal Orzel
  2023-12-08  9:21         ` Henry Wang
  0 siblings, 1 reply; 26+ messages in thread
From: Michal Orzel @ 2023-12-08  9:11 UTC (permalink / raw)
  To: Henry Wang
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Wei Chen



On 08/12/2023 10:05, Henry Wang wrote:
> 
> 
> Hi Michal,
> 
>> On Dec 8, 2023, at 16:57, Michal Orzel <michal.orzel@amd.com> wrote:
>>
>> Hi Henry,
>>
>> On 08/12/2023 06:46, Henry Wang wrote:
>>>
>>>
>>> To interact with the FVP (for example entering the U-Boot shell
>>> and transferring the files by TFTP), we need to connect the
>>> corresponding port by the telnet first. Use an expect script to
>>> simplify the automation of the whole "interacting with FVP" stuff.
>>>
>>> The expect script will firstly detect the IP of the host, then
>>> connect to the telnet port of the FVP, set the `serverip` and `ipaddr`
>>> for the TFTP service in the U-Boot shell, and finally boot Xen from
>>> U-Boot and wait for the expected results by Xen, Dom0 and DomU.
>>>
>>> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
>> Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> Thanks!
> 
>> with 1 question...
>>
>>> ---
>>> v2:
>>> - No change.
>>> ---
>>> .../expect/fvp-base-smoke-dom0-arm64.exp      | 73 +++++++++++++++++++
>>> 1 file changed, 73 insertions(+)
>>> create mode 100755 automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>
>>> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>> new file mode 100755
>>> index 0000000000..25d9a5f81c
>>> --- /dev/null
>>> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>> @@ -0,0 +1,73 @@
>>> +#!/usr/bin/expect
>>> +
>>> +set timeout 2000
>> Do we really need such a big timeout (~30 min)?
>> Looking at your test job, it took 16 mins (quite a lot but I know FVP is slow
>> + send_slow slows things down)
> 
> This is a really good question. I did have the same question while working on
> the negative test today. The timeout 2000 indeed will fail the job at about 30min,
> and waiting for it is indeed not really pleasant.
> 
> But my second thought would be - from my observation, the overall time now
> would vary between 15min ~ 20min, and having a 10min margin is not that crazy
> given that we probably will do more testing from the job in the future, and if the
> GitLab Arm worker is high loaded, FVP will probably become slower. And normally
> we don’t even trigger the timeout as the job will normally pass. So I decided
> to keep this.
> 
> Mind sharing your thoughts about the better value of the timeout? Probably 25min?
From what you said that the average is 15-20, I think we can leave it set to 30.
But I wonder if we can do something to decrease the average time. ~20 min is a lot
even for FVP :) Have you tried setting send_slow to something lower than 100ms?
That said, we don't send too many chars to FVP, so I doubt it would play a major role
in the overall time.

I use FVP quite rarely these days, so you should know better if this can be perceived as
usual/normal behavior.

~Michal



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 5/5] automation: Add the arm64 FVP build and Dom0 smoke test jobs
  2023-12-08  5:46 ` [PATCH v2 5/5] automation: Add the arm64 FVP build and Dom0 smoke test jobs Henry Wang
@ 2023-12-08  9:19   ` Michal Orzel
  2023-12-08  9:22     ` Henry Wang
  0 siblings, 1 reply; 26+ messages in thread
From: Michal Orzel @ 2023-12-08  9:19 UTC (permalink / raw)
  To: Henry Wang, xen-devel
  Cc: Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Wei Chen

Hi Henry,

On 08/12/2023 06:46, Henry Wang wrote:
> 
> 
> Add a job in the build stage to export the TF-A, U-Boot and the
> device tree for the FVP platform from the test artifact container.
> 
> Add a FVP smoke test job in the test stage to do the same test as
> the `qemu-smoke-dom0-arm64-gcc` job.
> 
> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

with a remark...

> ---
> v2:
> - Add Stefano's Reviewed-by tag.
> 
> Although it does not affect the functionality, I am still quite
> confused about why the logs displayed by GitLab UI, for
> example [1], is much less than the actual output (saved in the
> artifacts, see [2]). Had a discussion with Michal on Matrix
> and we agree that the log in gitlab UI is usually capped.
> 
> [1] https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5690876361
> [2] https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5690876361/artifacts/file/smoke.serial
> ---
>  automation/gitlab-ci/build.yaml | 17 +++++++++++++++++
>  automation/gitlab-ci/test.yaml  | 22 ++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index 32af30cced..89d2f01302 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -344,6 +344,23 @@ kernel-6.1.19-export:
>    tags:
>      - x86_64
> 
> +armfvp-uboot-tfa-2023.10-2.9.0-arm64-export:
> +  extends: .test-jobs-artifact-common
> +  image: registry.gitlab.com/xen-project/xen/tests-artifacts/armfvp-uboot-tfa:2023.10-2.9.0-arm64v8
> +  script:
> +    - |
> +       mkdir binaries && \
> +       cp /bl1.bin binaries/bl1.bin && \
> +       cp /fip.bin binaries/fip.bin && \
> +       cp /fvp-base-gicv3-psci-1t.dtb binaries/fvp-base-gicv3-psci-1t.dtb
> +  artifacts:
> +    paths:
> +      - binaries/bl1.bin
> +      - binaries/fip.bin
> +      - binaries/fvp-base-gicv3-psci-1t.dtb
> +  tags:
> +    - arm64
> +
>  # Jobs below this line
> 
>  # Build jobs needed for tests
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 6aabdb9d15..47e00d0a0b 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -96,6 +96,19 @@
>    tags:
>      - xilinx
> 
> +.fvp-arm64:
> +  extends: .test-jobs-common
> +  variables:
> +    CONTAINER: debian:bookworm-arm64v8-fvp
> +    LOGFILE: fvp-smoke-arm64.log
> +  artifacts:
> +    paths:
> +      - smoke.serial
> +      - '*.log'
> +    when: always
> +  tags:
> +    - arm64
> +
>  .adl-x86-64:
>    extends: .test-jobs-common
>    variables:
> @@ -459,3 +472,12 @@ qemu-smoke-ppc64le-powernv9-gcc:
>    needs:
>      - qemu-system-ppc64-8.1.0-ppc64-export
>      - debian-bullseye-gcc-ppc64le-debug
> +
> +fvp-smoke-dom0-arm64-gcc-debug:
> +  extends: .fvp-arm64
> +  script:
> +    - ./automation/scripts/fvp-base-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE}
> +  needs:
> +    - *arm64-test-needs
This would imply the need for qemu-system-aarch64-6.0.0-arm64-export which you don't need.
I think you could do:

.fvp-arm64-test-needs: &fvp-arm64-test-needs
  - alpine-3.18-arm64-rootfs-export
  - kernel-5.19-arm64-export
  - armfvp-uboot-tfa-2023.10-2.9.0-arm64-export

and then reference this in your job. This will be reused by other FVP tests in the future.

~Michal


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08  9:11       ` Michal Orzel
@ 2023-12-08  9:21         ` Henry Wang
  2023-12-08  9:50           ` Michal Orzel
  0 siblings, 1 reply; 26+ messages in thread
From: Henry Wang @ 2023-12-08  9:21 UTC (permalink / raw)
  To: Michal Orzel
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Wei Chen

Hi Michal,

> On Dec 8, 2023, at 17:11, Michal Orzel <michal.orzel@amd.com> wrote:
> On 08/12/2023 10:05, Henry Wang wrote:
>> 
>> Hi Michal,
>> 
>>> On Dec 8, 2023, at 16:57, Michal Orzel <michal.orzel@amd.com> wrote:
>>> 
>>> Hi Henry,
>>> 
>>> On 08/12/2023 06:46, Henry Wang wrote:
>>>> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>> new file mode 100755
>>>> index 0000000000..25d9a5f81c
>>>> --- /dev/null
>>>> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>> @@ -0,0 +1,73 @@
>>>> +#!/usr/bin/expect
>>>> +
>>>> +set timeout 2000
>>> Do we really need such a big timeout (~30 min)?
>>> Looking at your test job, it took 16 mins (quite a lot but I know FVP is slow
>>> + send_slow slows things down)
>> 
>> This is a really good question. I did have the same question while working on
>> the negative test today. The timeout 2000 indeed will fail the job at about 30min,
>> and waiting for it is indeed not really pleasant.
>> 
>> But my second thought would be - from my observation, the overall time now
>> would vary between 15min ~ 20min, and having a 10min margin is not that crazy
>> given that we probably will do more testing from the job in the future, and if the
>> GitLab Arm worker is high loaded, FVP will probably become slower. And normally
>> we don’t even trigger the timeout as the job will normally pass. So I decided
>> to keep this.
>> 
>> Mind sharing your thoughts about the better value of the timeout? Probably 25min?
> From what you said that the average is 15-20, I think we can leave it set to 30.
> But I wonder if we can do something to decrease the average time. ~20 min is a lot
> even for FVP :) Have you tried setting send_slow to something lower than 100ms?
> That said, we don't send too many chars to FVP, so I doubt it would play a major role
> in the overall time.

I agree with the send_slow part. Actually I do have the same concern, here are my current
understanding and I think you will definitely help with your knowledge:
If you check the full log of Dom0 booting, for example [1], you will find that we wasted so
much time in starting the services of the OS (modloop, udev-settle, etc). All of these services
are retried many times but in the end they are still not up, and from my understanding they
won’t affect the actual test(?) If we can somehow get rid of these services from rootfs, I think
we can save a lot of time.

And honestly, I noticed that qemu-alpine-arm64-gcc suffers from the same problem and it also
takes around 15min to finish. So if we managed to tailor the services from the filesystem, we
can save a lot of time.

But I found it difficult to do the proper tailoring, any suggestions?

[1] https://gitlab.com/xen-project/people/henryw/xen/-/jobs/5708557850/artifacts/file/smoke.serial

Kind regards,
Henry

> I use FVP quite rarely these days, so you should know better if this can be perceived as
> usual/normal behavior.
> 
> ~Michal
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 5/5] automation: Add the arm64 FVP build and Dom0 smoke test jobs
  2023-12-08  9:19   ` Michal Orzel
@ 2023-12-08  9:22     ` Henry Wang
  0 siblings, 0 replies; 26+ messages in thread
From: Henry Wang @ 2023-12-08  9:22 UTC (permalink / raw)
  To: Michal Orzel
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Wei Chen

Hi Michal,

> On Dec 8, 2023, at 17:19, Michal Orzel <michal.orzel@amd.com> wrote:
> 
> Hi Henry,
> 
> On 08/12/2023 06:46, Henry Wang wrote:
>> 
>> 
>> Add a job in the build stage to export the TF-A, U-Boot and the
>> device tree for the FVP platform from the test artifact container.
>> 
>> Add a FVP smoke test job in the test stage to do the same test as
>> the `qemu-smoke-dom0-arm64-gcc` job.
>> 
>> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Thanks.

> 
> with a remark...
> 
>> +fvp-smoke-dom0-arm64-gcc-debug:
>> +  extends: .fvp-arm64
>> +  script:
>> +    - ./automation/scripts/fvp-base-smoke-dom0-arm64.sh 2>&1 | tee ${LOGFILE}
>> +  needs:
>> +    - *arm64-test-needs
> This would imply the need for qemu-system-aarch64-6.0.0-arm64-export which you don't need.
> I think you could do:
> 
> .fvp-arm64-test-needs: &fvp-arm64-test-needs
>  - alpine-3.18-arm64-rootfs-export
>  - kernel-5.19-arm64-export
>  - armfvp-uboot-tfa-2023.10-2.9.0-arm64-export
> 
> and then reference this in your job. This will be reused by other FVP tests in the future.

Sounds good, will do in v3. Thanks!

Kind regards,
Henry

> 
> ~Michal



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08  9:21         ` Henry Wang
@ 2023-12-08  9:50           ` Michal Orzel
  2023-12-08  9:56             ` Henry Wang
  2023-12-08 12:05             ` Julien Grall
  0 siblings, 2 replies; 26+ messages in thread
From: Michal Orzel @ 2023-12-08  9:50 UTC (permalink / raw)
  To: Henry Wang
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Wei Chen



On 08/12/2023 10:21, Henry Wang wrote:
> 
> 
> Hi Michal,
> 
>> On Dec 8, 2023, at 17:11, Michal Orzel <michal.orzel@amd.com> wrote:
>> On 08/12/2023 10:05, Henry Wang wrote:
>>>
>>> Hi Michal,
>>>
>>>> On Dec 8, 2023, at 16:57, Michal Orzel <michal.orzel@amd.com> wrote:
>>>>
>>>> Hi Henry,
>>>>
>>>> On 08/12/2023 06:46, Henry Wang wrote:
>>>>> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>> new file mode 100755
>>>>> index 0000000000..25d9a5f81c
>>>>> --- /dev/null
>>>>> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>> @@ -0,0 +1,73 @@
>>>>> +#!/usr/bin/expect
>>>>> +
>>>>> +set timeout 2000
>>>> Do we really need such a big timeout (~30 min)?
>>>> Looking at your test job, it took 16 mins (quite a lot but I know FVP is slow
>>>> + send_slow slows things down)
>>>
>>> This is a really good question. I did have the same question while working on
>>> the negative test today. The timeout 2000 indeed will fail the job at about 30min,
>>> and waiting for it is indeed not really pleasant.
>>>
>>> But my second thought would be - from my observation, the overall time now
>>> would vary between 15min ~ 20min, and having a 10min margin is not that crazy
>>> given that we probably will do more testing from the job in the future, and if the
>>> GitLab Arm worker is high loaded, FVP will probably become slower. And normally
>>> we don’t even trigger the timeout as the job will normally pass. So I decided
>>> to keep this.
>>>
>>> Mind sharing your thoughts about the better value of the timeout? Probably 25min?
>> From what you said that the average is 15-20, I think we can leave it set to 30.
>> But I wonder if we can do something to decrease the average time. ~20 min is a lot
>> even for FVP :) Have you tried setting send_slow to something lower than 100ms?
>> That said, we don't send too many chars to FVP, so I doubt it would play a major role
>> in the overall time.
> 
> I agree with the send_slow part. Actually I do have the same concern, here are my current
> understanding and I think you will definitely help with your knowledge:
> If you check the full log of Dom0 booting, for example [1], you will find that we wasted so
> much time in starting the services of the OS (modloop, udev-settle, etc). All of these services
> are retried many times but in the end they are still not up, and from my understanding they
> won’t affect the actual test(?) If we can somehow get rid of these services from rootfs, I think
> we can save a lot of time.
> 
> And honestly, I noticed that qemu-alpine-arm64-gcc suffers from the same problem and it also
> takes around 15min to finish. So if we managed to tailor the services from the filesystem, we
> can save a lot of time.
That is not true. Qemu runs the tests relatively fast within few minutes. The reason you see e.g. 12 mins
for some Qemu jobs comes from the timeout we set in Qemu scripts. We don't have yet the solution (we could
do the same as Qubes script) to detect the test success early and exit before timeout. That is why currently
the only way for Qemu tests to finish is by reaching the timeout.

So the problem is not with the rootfs and services (the improvement would not be significant) but with
the simulation being slow. That said, this is something we all know and I expect FVP to only be used in scenarios
which cannot be tested using Qemu or real HW.

~Michal


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08  9:50           ` Michal Orzel
@ 2023-12-08  9:56             ` Henry Wang
  2023-12-08 11:13               ` Wei Chen
  2023-12-08 12:05             ` Julien Grall
  1 sibling, 1 reply; 26+ messages in thread
From: Henry Wang @ 2023-12-08  9:56 UTC (permalink / raw)
  To: Michal Orzel
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Wei Chen

Hi Michal,

> On Dec 8, 2023, at 17:50, Michal Orzel <michal.orzel@amd.com> wrote:
> On 08/12/2023 10:21, Henry Wang wrote:
>> 
>> 
>> Hi Michal,
>> 
>>> On Dec 8, 2023, at 17:11, Michal Orzel <michal.orzel@amd.com> wrote:
>>> On 08/12/2023 10:05, Henry Wang wrote:
>>>> 
>>>> Hi Michal,
>>>> 
>>>>> On Dec 8, 2023, at 16:57, Michal Orzel <michal.orzel@amd.com> wrote:
>>>>> 
>>>>> Hi Henry,
>>>>> 
>>>>> On 08/12/2023 06:46, Henry Wang wrote:
>>>>>> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>>> new file mode 100755
>>>>>> index 0000000000..25d9a5f81c
>>>>>> --- /dev/null
>>>>>> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>>> @@ -0,0 +1,73 @@
>>>>>> +#!/usr/bin/expect
>>>>>> +
>>>>>> +set timeout 2000
>>>>> Do we really need such a big timeout (~30 min)?
>>>>> Looking at your test job, it took 16 mins (quite a lot but I know FVP is slow
>>>>> + send_slow slows things down)
>>>> 
>>>> This is a really good question. I did have the same question while working on
>>>> the negative test today. The timeout 2000 indeed will fail the job at about 30min,
>>>> and waiting for it is indeed not really pleasant.
>>>> 
>>>> But my second thought would be - from my observation, the overall time now
>>>> would vary between 15min ~ 20min, and having a 10min margin is not that crazy
>>>> given that we probably will do more testing from the job in the future, and if the
>>>> GitLab Arm worker is high loaded, FVP will probably become slower. And normally
>>>> we don’t even trigger the timeout as the job will normally pass. So I decided
>>>> to keep this.
>>>> 
>>>> Mind sharing your thoughts about the better value of the timeout? Probably 25min?
>>> From what you said that the average is 15-20, I think we can leave it set to 30.
>>> But I wonder if we can do something to decrease the average time. ~20 min is a lot
>>> even for FVP :) Have you tried setting send_slow to something lower than 100ms?
>>> That said, we don't send too many chars to FVP, so I doubt it would play a major role
>>> in the overall time.
>> 
>> I agree with the send_slow part. Actually I do have the same concern, here are my current
>> understanding and I think you will definitely help with your knowledge:
>> If you check the full log of Dom0 booting, for example [1], you will find that we wasted so
>> much time in starting the services of the OS (modloop, udev-settle, etc). All of these services
>> are retried many times but in the end they are still not up, and from my understanding they
>> won’t affect the actual test(?) If we can somehow get rid of these services from rootfs, I think
>> we can save a lot of time.
>> 
>> And honestly, I noticed that qemu-alpine-arm64-gcc suffers from the same problem and it also
>> takes around 15min to finish. So if we managed to tailor the services from the filesystem, we
>> can save a lot of time.
> That is not true. Qemu runs the tests relatively fast within few minutes. The reason you see e.g. 12 mins
> for some Qemu jobs comes from the timeout we set in Qemu scripts. We don't have yet the solution (we could
> do the same as Qubes script) to detect the test success early and exit before timeout. That is why currently
> the only way for Qemu tests to finish is by reaching the timeout.
> 
> So the problem is not with the rootfs and services (the improvement would not be significant) but with
> the simulation being slow. That said, this is something we all know and I expect FVP to only be used in scenarios
> which cannot be tested using Qemu or real HW.

Ok, you made a point. Let me do some experiments to see if I can improve. Otherwise maybe
we can live with this until a better solution.

Kind regards,
Henry

> 
> ~Michal


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08  9:56             ` Henry Wang
@ 2023-12-08 11:13               ` Wei Chen
  2023-12-08 12:27                 ` Henry Wang
  0 siblings, 1 reply; 26+ messages in thread
From: Wei Chen @ 2023-12-08 11:13 UTC (permalink / raw)
  To: Henry Wang, Michal Orzel
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis

Hi Henry and Michal,

On 2023/12/8 17:56, Henry Wang wrote:
> Hi Michal,
> 
>> On Dec 8, 2023, at 17:50, Michal Orzel <michal.orzel@amd.com> wrote:
>> On 08/12/2023 10:21, Henry Wang wrote:
>>>
>>>
>>> Hi Michal,
>>>
>>>> On Dec 8, 2023, at 17:11, Michal Orzel <michal.orzel@amd.com> wrote:
>>>> On 08/12/2023 10:05, Henry Wang wrote:
>>>>>
>>>>> Hi Michal,
>>>>>
>>>>>> On Dec 8, 2023, at 16:57, Michal Orzel <michal.orzel@amd.com> wrote:
>>>>>>
>>>>>> Hi Henry,
>>>>>>
>>>>>> On 08/12/2023 06:46, Henry Wang wrote:
>>>>>>> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>>>> new file mode 100755
>>>>>>> index 0000000000..25d9a5f81c
>>>>>>> --- /dev/null
>>>>>>> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>>>> @@ -0,0 +1,73 @@
>>>>>>> +#!/usr/bin/expect
>>>>>>> +
>>>>>>> +set timeout 2000
>>>>>> Do we really need such a big timeout (~30 min)?
>>>>>> Looking at your test job, it took 16 mins (quite a lot but I know FVP is slow
>>>>>> + send_slow slows things down)
>>>>>
>>>>> This is a really good question. I did have the same question while working on
>>>>> the negative test today. The timeout 2000 indeed will fail the job at about 30min,
>>>>> and waiting for it is indeed not really pleasant.
>>>>>
>>>>> But my second thought would be - from my observation, the overall time now
>>>>> would vary between 15min ~ 20min, and having a 10min margin is not that crazy
>>>>> given that we probably will do more testing from the job in the future, and if the
>>>>> GitLab Arm worker is high loaded, FVP will probably become slower. And normally
>>>>> we don’t even trigger the timeout as the job will normally pass. So I decided
>>>>> to keep this.
>>>>>
>>>>> Mind sharing your thoughts about the better value of the timeout? Probably 25min?
>>>>  From what you said that the average is 15-20, I think we can leave it set to 30.
>>>> But I wonder if we can do something to decrease the average time. ~20 min is a lot
>>>> even for FVP :) Have you tried setting send_slow to something lower than 100ms?
>>>> That said, we don't send too many chars to FVP, so I doubt it would play a major role
>>>> in the overall time.
>>>
>>> I agree with the send_slow part. Actually I do have the same concern, here are my current
>>> understanding and I think you will definitely help with your knowledge:
>>> If you check the full log of Dom0 booting, for example [1], you will find that we wasted so
>>> much time in starting the services of the OS (modloop, udev-settle, etc). All of these services
>>> are retried many times but in the end they are still not up, and from my understanding they
>>> won’t affect the actual test(?) If we can somehow get rid of these services from rootfs, I think
>>> we can save a lot of time.
>>>
>>> And honestly, I noticed that qemu-alpine-arm64-gcc suffers from the same problem and it also
>>> takes around 15min to finish. So if we managed to tailor the services from the filesystem, we
>>> can save a lot of time.
>> That is not true. Qemu runs the tests relatively fast within few minutes. The reason you see e.g. 12 mins
>> for some Qemu jobs comes from the timeout we set in Qemu scripts. We don't have yet the solution (we could
>> do the same as Qubes script) to detect the test success early and exit before timeout. That is why currently
>> the only way for Qemu tests to finish is by reaching the timeout.
>>
>> So the problem is not with the rootfs and services (the improvement would not be significant) but with
>> the simulation being slow. That said, this is something we all know and I expect FVP to only be used in scenarios
>> which cannot be tested using Qemu or real HW.
> 
> Ok, you made a point. Let me do some experiments to see if I can improve. Otherwise maybe
> we can live with this until a better solution.
> 
> Kind regards,
> Henry
> 

QEMU works like FVP enabled use_real_time flag. How about enable 
use_real_time flag in CI for most test cases, but disable it for
some time sensitive test cases? Normally, enable use_real_time
will give several times improvement of FVP performance.

Cheers,
Wei Chen

>>
>> ~Michal
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08  9:50           ` Michal Orzel
  2023-12-08  9:56             ` Henry Wang
@ 2023-12-08 12:05             ` Julien Grall
  2023-12-08 12:17               ` Henry Wang
  2023-12-08 21:30               ` Stefano Stabellini
  1 sibling, 2 replies; 26+ messages in thread
From: Julien Grall @ 2023-12-08 12:05 UTC (permalink / raw)
  To: Michal Orzel, Henry Wang
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Bertrand Marquis,
	Wei Chen

Hi,

On 08/12/2023 09:50, Michal Orzel wrote:
> On 08/12/2023 10:21, Henry Wang wrote:
>>> On Dec 8, 2023, at 17:11, Michal Orzel <michal.orzel@amd.com> wrote:
>>> On 08/12/2023 10:05, Henry Wang wrote:
>>>>
>>>> Hi Michal,
>>>>
>>>>> On Dec 8, 2023, at 16:57, Michal Orzel <michal.orzel@amd.com> wrote:
>>>>>
>>>>> Hi Henry,
>>>>>
>>>>> On 08/12/2023 06:46, Henry Wang wrote:
>>>>>> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>>> new file mode 100755
>>>>>> index 0000000000..25d9a5f81c
>>>>>> --- /dev/null
>>>>>> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>>> @@ -0,0 +1,73 @@
>>>>>> +#!/usr/bin/expect
>>>>>> +
>>>>>> +set timeout 2000
>>>>> Do we really need such a big timeout (~30 min)?
>>>>> Looking at your test job, it took 16 mins (quite a lot but I know FVP is slow
>>>>> + send_slow slows things down)
>>>>
>>>> This is a really good question. I did have the same question while working on
>>>> the negative test today. The timeout 2000 indeed will fail the job at about 30min,
>>>> and waiting for it is indeed not really pleasant.
>>>>
>>>> But my second thought would be - from my observation, the overall time now
>>>> would vary between 15min ~ 20min, and having a 10min margin is not that crazy
>>>> given that we probably will do more testing from the job in the future, and if the
>>>> GitLab Arm worker is high loaded, FVP will probably become slower. And normally
>>>> we don’t even trigger the timeout as the job will normally pass. So I decided
>>>> to keep this.
>>>>
>>>> Mind sharing your thoughts about the better value of the timeout? Probably 25min?
>>>  From what you said that the average is 15-20, I think we can leave it set to 30.
>>> But I wonder if we can do something to decrease the average time. ~20 min is a lot
>>> even for FVP :) Have you tried setting send_slow to something lower than 100ms?
>>> That said, we don't send too many chars to FVP, so I doubt it would play a major role
>>> in the overall time.
>>
>> I agree with the send_slow part. Actually I do have the same concern, here are my current
>> understanding and I think you will definitely help with your knowledge:
>> If you check the full log of Dom0 booting, for example [1], you will find that we wasted so
>> much time in starting the services of the OS (modloop, udev-settle, etc). All of these services
>> are retried many times but in the end they are still not up, and from my understanding they
>> won’t affect the actual test(?) If we can somehow get rid of these services from rootfs, I think
>> we can save a lot of time.
>>
>> And honestly, I noticed that qemu-alpine-arm64-gcc suffers from the same problem and it also
>> takes around 15min to finish. So if we managed to tailor the services from the filesystem, we
>> can save a lot of time.
> That is not true. Qemu runs the tests relatively fast within few minutes. The reason you see e.g. 12 mins
> for some Qemu jobs comes from the timeout we set in Qemu scripts. We don't have yet the solution (we could
> do the same as Qubes script) to detect the test success early and exit before timeout. That is why currently
> the only way for Qemu tests to finish is by reaching the timeout.
> 
> So the problem is not with the rootfs and services (the improvement would not be significant) but with
> the simulation being slow.

 From my experience with the FVP improvement would be significant. A 
normal boot distribution will start a lot of services. I end up to write 
my own initscript doing the bare minimum for creating a guest. This 
saves me a lot of time everytime I needed to test on FVP.

I think we can do the same for the gitlab. Maybe not to the point of 
writing your initscript but cutting down anything unnecessary.

This will avoid the FVP test to become the bottlneck in the gitlab CI.

Chers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08 12:05             ` Julien Grall
@ 2023-12-08 12:17               ` Henry Wang
  2023-12-08 21:30               ` Stefano Stabellini
  1 sibling, 0 replies; 26+ messages in thread
From: Henry Wang @ 2023-12-08 12:17 UTC (permalink / raw)
  To: Julien Grall
  Cc: Michal Orzel, Xen-devel, Doug Goldstein, Stefano Stabellini,
	Bertrand Marquis, Wei Chen

Hi Julien,

> On Dec 8, 2023, at 20:05, Julien Grall <julien@xen.org> wrote:
> 
> Hi,
> 
> On 08/12/2023 09:50, Michal Orzel wrote:
>> On 08/12/2023 10:21, Henry Wang wrote:
>>>> On Dec 8, 2023, at 17:11, Michal Orzel <michal.orzel@amd.com> wrote:
>>>> On 08/12/2023 10:05, Henry Wang wrote:
>>>>> 
>>>>> Hi Michal,
>>>>> 
>>>>>> On Dec 8, 2023, at 16:57, Michal Orzel <michal.orzel@amd.com> wrote:
>>>>>> 
>>>>>> Hi Henry,
>>>>>> 
>>>>>> On 08/12/2023 06:46, Henry Wang wrote:
>>>>>>> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>>>> new file mode 100755
>>>>>>> index 0000000000..25d9a5f81c
>>>>>>> --- /dev/null
>>>>>>> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>>>> @@ -0,0 +1,73 @@
>>>>>>> +#!/usr/bin/expect
>>>>>>> +
>>>>>>> +set timeout 2000
>>>>>> Do we really need such a big timeout (~30 min)?
>>>>>> Looking at your test job, it took 16 mins (quite a lot but I know FVP is slow
>>>>>> + send_slow slows things down)
>>>>> 
>>>>> This is a really good question. I did have the same question while working on
>>>>> the negative test today. The timeout 2000 indeed will fail the job at about 30min,
>>>>> and waiting for it is indeed not really pleasant.
>>>>> 
>>>>> But my second thought would be - from my observation, the overall time now
>>>>> would vary between 15min ~ 20min, and having a 10min margin is not that crazy
>>>>> given that we probably will do more testing from the job in the future, and if the
>>>>> GitLab Arm worker is high loaded, FVP will probably become slower. And normally
>>>>> we don’t even trigger the timeout as the job will normally pass. So I decided
>>>>> to keep this.
>>>>> 
>>>>> Mind sharing your thoughts about the better value of the timeout? Probably 25min?
>>>> From what you said that the average is 15-20, I think we can leave it set to 30.
>>>> But I wonder if we can do something to decrease the average time. ~20 min is a lot
>>>> even for FVP :) Have you tried setting send_slow to something lower than 100ms?
>>>> That said, we don't send too many chars to FVP, so I doubt it would play a major role
>>>> in the overall time.
>>> 
>>> I agree with the send_slow part. Actually I do have the same concern, here are my current
>>> understanding and I think you will definitely help with your knowledge:
>>> If you check the full log of Dom0 booting, for example [1], you will find that we wasted so
>>> much time in starting the services of the OS (modloop, udev-settle, etc). All of these services
>>> are retried many times but in the end they are still not up, and from my understanding they
>>> won’t affect the actual test(?) If we can somehow get rid of these services from rootfs, I think
>>> we can save a lot of time.
>>> 
>>> And honestly, I noticed that qemu-alpine-arm64-gcc suffers from the same problem and it also
>>> takes around 15min to finish. So if we managed to tailor the services from the filesystem, we
>>> can save a lot of time.
>> That is not true. Qemu runs the tests relatively fast within few minutes. The reason you see e.g. 12 mins
>> for some Qemu jobs comes from the timeout we set in Qemu scripts. We don't have yet the solution (we could
>> do the same as Qubes script) to detect the test success early and exit before timeout. That is why currently
>> the only way for Qemu tests to finish is by reaching the timeout.
>> So the problem is not with the rootfs and services (the improvement would not be significant) but with
>> the simulation being slow.
> 
> From my experience with the FVP improvement would be significant. A normal boot distribution will start a lot of services. I end up to write my own initscript doing the bare minimum for creating a guest. This saves me a lot of time everytime I needed to test on FVP.

+1, I feel the same, but I've never done the time measurement though.

> I think we can do the same for the gitlab. Maybe not to the point of writing your initscript but cutting down anything unnecessary.

Yeah I can try to play with removing some of the unnecessary services when preparing the rootfs
for Dom0 (see patch 4).

Kind regards,
Henry

> This will avoid the FVP test to become the bottlneck in the gitlab CI.
> 
> Chers,
> 
> -- 
> Julien Grall


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08 11:13               ` Wei Chen
@ 2023-12-08 12:27                 ` Henry Wang
  0 siblings, 0 replies; 26+ messages in thread
From: Henry Wang @ 2023-12-08 12:27 UTC (permalink / raw)
  To: Wei Chen, Michal Orzel
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Julien Grall,
	Bertrand Marquis

Hi Wei, Michal,

> On Dec 8, 2023, at 19:13, Wei Chen <Wei.Chen@arm.com> wrote:
> 
> Hi Henry and Michal,
> 
>>>>>>> On 08/12/2023 06:46, Henry Wang wrote:
>>>>>>>> diff --git a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>>>>> new file mode 100755
>>>>>>>> index 0000000000..25d9a5f81c
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
>>>>>>>> @@ -0,0 +1,73 @@
>>>>>>>> +#!/usr/bin/expect
>>>>>>>> +
>>>>>>>> +set timeout 2000
>>>>>>> Do we really need such a big timeout (~30 min)?
>>>>>>> Looking at your test job, it took 16 mins (quite a lot but I know FVP is slow
>>>>>>> + send_slow slows things down)
>>>>>> 
>>>>>> This is a really good question. I did have the same question while working on
>>>>>> the negative test today. The timeout 2000 indeed will fail the job at about 30min,
>>>>>> and waiting for it is indeed not really pleasant.
>>>>>> 
>>>>>> But my second thought would be - from my observation, the overall time now
>>>>>> would vary between 15min ~ 20min, and having a 10min margin is not that crazy
>>>>>> given that we probably will do more testing from the job in the future, and if the
>>>>>> GitLab Arm worker is high loaded, FVP will probably become slower. And normally
>>>>>> we don’t even trigger the timeout as the job will normally pass. So I decided
>>>>>> to keep this.
>>>>>> 
>>>>>> Mind sharing your thoughts about the better value of the timeout? Probably 25min?
>>>>> From what you said that the average is 15-20, I think we can leave it set to 30.
>>>>> But I wonder if we can do something to decrease the average time. ~20 min is a lot
>>>>> even for FVP :) Have you tried setting send_slow to something lower than 100ms?
>>>>> That said, we don't send too many chars to FVP, so I doubt it would play a major role
>>>>> in the overall time.
>>>> 
>>>> I agree with the send_slow part. Actually I do have the same concern, here are my current
>>>> understanding and I think you will definitely help with your knowledge:
>>>> If you check the full log of Dom0 booting, for example [1], you will find that we wasted so
>>>> much time in starting the services of the OS (modloop, udev-settle, etc). All of these services
>>>> are retried many times but in the end they are still not up, and from my understanding they
>>>> won’t affect the actual test(?) If we can somehow get rid of these services from rootfs, I think
>>>> we can save a lot of time.
>>>> 
>>>> And honestly, I noticed that qemu-alpine-arm64-gcc suffers from the same problem and it also
>>>> takes around 15min to finish. So if we managed to tailor the services from the filesystem, we
>>>> can save a lot of time.
>>> That is not true. Qemu runs the tests relatively fast within few minutes. The reason you see e.g. 12 mins
>>> for some Qemu jobs comes from the timeout we set in Qemu scripts. We don't have yet the solution (we could
>>> do the same as Qubes script) to detect the test success early and exit before timeout. That is why currently
>>> the only way for Qemu tests to finish is by reaching the timeout.
>>> 
>>> So the problem is not with the rootfs and services (the improvement would not be significant) but with
>>> the simulation being slow. That said, this is something we all know and I expect FVP to only be used in scenarios
>>> which cannot be tested using Qemu or real HW.
>> Ok, you made a point. Let me do some experiments to see if I can improve. Otherwise maybe
>> we can live with this until a better solution.
>> Kind regards,
>> Henry
> 
> QEMU works like FVP enabled use_real_time flag. How about enable use_real_time flag in CI for most test cases, but disable it for
> some time sensitive test cases? Normally, enable use_real_time
> will give several times improvement of FVP performance.

I am seeing below from the FVP parameter lists of the one we are currently using. The "Deprecated" word worries
me a bit (the old version FVP does not have the “Deprecated" though).
```
bp.refcounter.use_real_time=0                         # (bool  , init-time) default = '0'      : **Deprecated, this parameter will be removed in future versions** Update the Generic Timer counter at a real-time base frequency instead of simulator time
```

Also, from my testing in the GitLab pipeline, I was not able to see significant time improvement. So I guess
instead I will try what Julien suggests to see if things can be better.

Kind regards,
Henry 

> 
> Cheers,
> Wei Chen
> 
>>> 
>>> ~Michal


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 1/5] automation: Add a Dockerfile for running FVP_Base jobs
  2023-12-08  5:46 ` [PATCH v2 1/5] automation: Add a Dockerfile for running FVP_Base jobs Henry Wang
@ 2023-12-08 12:30   ` Julien Grall
  2023-12-08 12:59     ` Henry Wang
  0 siblings, 1 reply; 26+ messages in thread
From: Julien Grall @ 2023-12-08 12:30 UTC (permalink / raw)
  To: Henry Wang, xen-devel
  Cc: Doug Goldstein, Stefano Stabellini, Michal Orzel,
	Bertrand Marquis, Wei Chen

Hi,

On 08/12/2023 05:46, Henry Wang wrote:
> Fixed Virtual Platforms (FVPs) are complete simulations of an Arm
> system, including processor, memory and peripherals. These are set
> out in a "programmer's view", which gives programmers a comprehensive
> model on which to build and test software. FVP can be configured to
> different setups by its cmdline parameters, and hence having the FVP
> in CI will provide us with the flexibility to test Arm features and
> setups that we find difficult to use real hardware or emulators.
> 
> This commit adds a Dockerfile for the new arm64v8 container with
> FVP installed, based on the debian bookworm-arm64v8 image. This
> container will be used to run the FVP test jobs. Compared to the
> debian bookworm-arm64v8 image, the packages in the newly added FVP
> container does not contain the `u-boot-qemu`, and adds the `expect`
> to run expect scripts introduced by following commits, `telnet` to
> connect to FVP, and `tftpd-hpa` to provide the TFTP service for
> the FVP.
> 
> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> v2:
> - Add Stefano's Reviewed-by tag.
> ---
>   .../debian/bookworm-arm64v8-fvp.dockerfile    | 64 +++++++++++++++++++
>   1 file changed, 64 insertions(+)
>   create mode 100644 automation/build/debian/bookworm-arm64v8-fvp.dockerfile
> 
> diff --git a/automation/build/debian/bookworm-arm64v8-fvp.dockerfile b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
> new file mode 100644
> index 0000000000..3b87dc5a5b
> --- /dev/null
> +++ b/automation/build/debian/bookworm-arm64v8-fvp.dockerfile
> @@ -0,0 +1,64 @@
> +FROM --platform=linux/arm64/v8 debian:bookworm
> +LABEL maintainer.name="The Xen Project" \
> +      maintainer.email="xen-devel@lists.xenproject.org"
> +
> +ARG FVP_BASE_VERSION="11.23_9_Linux64_armv8l"
> +
> +ENV DEBIAN_FRONTEND=noninteractive
> +ENV USER root
> +
> +RUN mkdir /build
> +WORKDIR /build
> +
> +# build depends
> +RUN apt-get update && \
> +    apt-get --quiet --yes install \
> +        build-essential \
> +        zlib1g-dev \
> +        libncurses5-dev \
> +        libssl-dev \
> +        python3-dev \
> +        python3-setuptools \
> +        xorg-dev \
> +        uuid-dev \
> +        libyajl-dev \
> +        libaio-dev \
> +        libglib2.0-dev \
> +        clang \
> +        libpixman-1-dev \
> +        pkg-config \
> +        flex \
> +        bison \
> +        acpica-tools \
> +        libfdt-dev \
> +        bin86 \
> +        bcc \
> +        liblzma-dev \
> +        libnl-3-dev \
> +        ocaml-nox \
> +        libfindlib-ocaml-dev \
> +        markdown \
> +        transfig \
> +        pandoc \
> +        checkpolicy \
> +        wget \
> +        git \
> +        nasm \
> +        # for test phase, fvp-smoke-* jobs
> +        u-boot-tools \
> +        expect \
> +        device-tree-compiler \
> +        curl \
> +        cpio \
> +        busybox-static \
> +        telnet \
> +        tftpd-hpa \
> +        && \
> +        apt-get autoremove -y && \
> +        apt-get clean && \
> +        rm -rf /var/lib/apt/lists* /tmp/* /var/tmp/*
> +
> +RUN wget https://developer.arm.com/-/media/Files/downloads/ecosystem-models/FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz && \

I vaguely recall some discussions on whether it was ok for us to publish 
a container with the FVP model due to the license agreement.

I guess this has now been resolved because the download can be done 
without sign-in to the account. Can you confirm?

It would also be good that the commit message indicates whether there is 
any implicit license agreement from Xen Project (or any user that decide 
to use our scripts).

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 1/5] automation: Add a Dockerfile for running FVP_Base jobs
  2023-12-08 12:30   ` Julien Grall
@ 2023-12-08 12:59     ` Henry Wang
  2023-12-08 14:24       ` Julien Grall
  0 siblings, 1 reply; 26+ messages in thread
From: Henry Wang @ 2023-12-08 12:59 UTC (permalink / raw)
  To: Julien Grall
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Michal Orzel,
	Bertrand Marquis, Wei Chen

Hi Julien,

> On Dec 8, 2023, at 20:30, Julien Grall <julien@xen.org> wrote:
> 
> Hi,
> 
> On 08/12/2023 05:46, Henry Wang wrote:
>> Fixed Virtual Platforms (FVPs) are complete simulations of an Arm
>> system, including processor, memory and peripherals. These are set
>> out in a "programmer's view", which gives programmers a comprehensive
>> model on which to build and test software. FVP can be configured to
>> different setups by its cmdline parameters, and hence having the FVP
>> in CI will provide us with the flexibility to test Arm features and
>> setups that we find difficult to use real hardware or emulators.
>> This commit adds a Dockerfile for the new arm64v8 container with
>> FVP installed, based on the debian bookworm-arm64v8 image. This
>> container will be used to run the FVP test jobs. Compared to the
>> debian bookworm-arm64v8 image, the packages in the newly added FVP
>> container does not contain the `u-boot-qemu`, and adds the `expect`
>> to run expect scripts introduced by following commits, `telnet` to
>> connect to FVP, and `tftpd-hpa` to provide the TFTP service for
>> the FVP.
>> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>> ---
>> v2:
>> - Add Stefano's Reviewed-by tag.
>> ---
>> +
>> +RUN wget https://developer.arm.com/-/media/Files/downloads/ecosystem-models/FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz && \
> 
> I vaguely recall some discussions on whether it was ok for us to publish a container with the FVP model due to the license agreement.
> 
> I guess this has now been resolved because the download can be done without sign-in to the account. Can you confirm?

Yes, quoting some words from the people we asked internally:
(the page referred to is https://developer.arm.com/Tools%20and%20Software/Fixed%20Virtual%20Platforms):

"All the FVPs referenced on this page that you are interested in are licensed under
lightweight Eco System EULA that has no restrictions on the redistribution.”

"So, yes, we can ship container images containing the FVP and the license on the FVP will remain as is.”

"No issues with redistributing the model package in a Docker container, as long as the EULA in included."

> It would also be good that the commit message indicates whether there is any implicit license agreement from Xen Project (or any user that decide to use our scripts).

I think it is the “END USER LICENSE AGREEMENT FOR ARM ECOSYSTEM MODELS”?

Kind regards,
Henry

> 
> Cheers,
> 
> -- 
> Julien Grall


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 1/5] automation: Add a Dockerfile for running FVP_Base jobs
  2023-12-08 12:59     ` Henry Wang
@ 2023-12-08 14:24       ` Julien Grall
  0 siblings, 0 replies; 26+ messages in thread
From: Julien Grall @ 2023-12-08 14:24 UTC (permalink / raw)
  To: Henry Wang
  Cc: Xen-devel, Doug Goldstein, Stefano Stabellini, Michal Orzel,
	Bertrand Marquis, Wei Chen



On 08/12/2023 12:59, Henry Wang wrote:
> Hi Julien,

Hi,

>> On Dec 8, 2023, at 20:30, Julien Grall <julien@xen.org> wrote:
>>
>> Hi,
>>
>> On 08/12/2023 05:46, Henry Wang wrote:
>>> Fixed Virtual Platforms (FVPs) are complete simulations of an Arm
>>> system, including processor, memory and peripherals. These are set
>>> out in a "programmer's view", which gives programmers a comprehensive
>>> model on which to build and test software. FVP can be configured to
>>> different setups by its cmdline parameters, and hence having the FVP
>>> in CI will provide us with the flexibility to test Arm features and
>>> setups that we find difficult to use real hardware or emulators.
>>> This commit adds a Dockerfile for the new arm64v8 container with
>>> FVP installed, based on the debian bookworm-arm64v8 image. This
>>> container will be used to run the FVP test jobs. Compared to the
>>> debian bookworm-arm64v8 image, the packages in the newly added FVP
>>> container does not contain the `u-boot-qemu`, and adds the `expect`
>>> to run expect scripts introduced by following commits, `telnet` to
>>> connect to FVP, and `tftpd-hpa` to provide the TFTP service for
>>> the FVP.
>>> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
>>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>>> ---
>>> v2:
>>> - Add Stefano's Reviewed-by tag.
>>> ---
>>> +
>>> +RUN wget https://developer.arm.com/-/media/Files/downloads/ecosystem-models/FVP_Base_RevC-2xAEMvA_${FVP_BASE_VERSION}.tgz && \
>>
>> I vaguely recall some discussions on whether it was ok for us to publish a container with the FVP model due to the license agreement.
>>
>> I guess this has now been resolved because the download can be done without sign-in to the account. Can you confirm?
> 
> Yes, quoting some words from the people we asked internally:
> (the page referred to is https://developer.arm.com/Tools%20and%20Software/Fixed%20Virtual%20Platforms):
> 
> "All the FVPs referenced on this page that you are interested in are licensed under
> lightweight Eco System EULA that has no restrictions on the redistribution.”
> 
> "So, yes, we can ship container images containing the FVP and the license on the FVP will remain as is.”
> 
> "No issues with redistributing the model package in a Docker container, as long as the EULA in included."

Thanks for checking. In the current form, I don't think it is easy to 
know that the FVP has a specific license. I think this should be written 
down at the top of the container file. Something:

"The FVP is license under... Please read the file in ... for more details".

> 
>> It would also be good that the commit message indicates whether there is any implicit license agreement from Xen Project (or any user that decide to use our scripts).
> 
> I think it is the “END USER LICENSE AGREEMENT FOR ARM ECOSYSTEM MODELS”?

It looks like it.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP
  2023-12-08  8:56     ` Henry Wang
@ 2023-12-08 21:20       ` Stefano Stabellini
  0 siblings, 0 replies; 26+ messages in thread
From: Stefano Stabellini @ 2023-12-08 21:20 UTC (permalink / raw)
  To: Henry Wang
  Cc: Michal Orzel, Xen-devel, Doug Goldstein, Stefano Stabellini,
	Julien Grall, Bertrand Marquis, Wei Chen

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

On Fri, 8 Dec 2023, Henry Wang wrote:
> Hi Michal,
> 
> > On Dec 8, 2023, at 16:39, Michal Orzel <michal.orzel@amd.com> wrote:
> > 
> > Hi Henry,
> > 
> > On 08/12/2023 06:46, Henry Wang wrote:
> >> 
> >> Unlike the emulators that currently being used in the CI pipelines,
> >> the FVP must start at EL3. Therefore we need the firmware, i.e. the
> >> TrustedFirmware-A (TF-A), for corresponding functionality.
> >> 
> >> There is a dedicated board (vexpress_fvp) in U-Boot (serve as the
> >> BL33 of the TF-A) for the FVP platform, so the U-Boot should also be
> >> compiled for the FVP platform instead of reusing the U-Boot for the
> >> existing emulators used in the CI pipelines.
> >> 
> >> To avoid compiling TF-A and U-Boot everytime in the job, adding a
> >> Dockerfile to the test artifacts to build TF-A v2.9.0 and U-Boot
> >> v2023.10 for FVP. The binaries for the TF-A and U-Boot, as well as
> >> the device tree for the FVP platform, will be saved (and exported by
> >> the CI job introduced by following commits). Note that, a patch for
> >> the TF-A will be applied before building to enable the virtio-net
> >> and the virtio-rng device on the FVP. The virtio-net device will
> >> provide the networking service for FVP, and the virtio-rng device
> >> will improve the speed of the FVP.
> >> 
> >> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
> >> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> >> ---
> >> v2:
> >> - Add Stefano's Reviewed-by tag.
> >> ---
> >> +# Build U-Boot and TF-A
> >> +RUN curl -fsSLO https://ftp.denx.de/pub/u-boot/u-boot-"$UBOOT_VERSION".tar.bz2 && \
> >> +    tar xvf u-boot-"$UBOOT_VERSION".tar.bz2 && \
> >> +    cd u-boot-"$UBOOT_VERSION" && \
> >> +    make -j$(nproc) V=1 vexpress_fvp_defconfig && \
> >> +    make -j$(nproc) V=1 all && \
> > Do we need 'all'? Can't we just build target 'u-boot' for u-boot.bin?
> 
> I think your suggestion makes sense, and I can have a try, if changing all to u-boot works,
> I will use that in v3.
> 
> >> +    cd .. && \
> >> +    git clone --branch "$TFA_VERSION" --depth 1 https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git && \
> >> +    cd trusted-firmware-a && \
> >> +    curl -fsSLO https://git.yoctoproject.org/meta-arm/plain/meta-arm-bsp/recipes-bsp/trusted-firmware-a/files/fvp-base/0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch \
> >> +         --output 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
> >> +    git config --global user.email "you@example.com" && \
> >> +    git config --global user.name "Your Name" && \
> > If this is needed for git am, you could get away using 'patch -p1'
> 
> Hmmm right, then probably we can even not install git and use the tarball instead of
> git clone.
> 
> 
> >> +    git am 0001-fdts-fvp-base-Add-stdout-path-and-virtio-net-and-rng.patch && \
> >> +    make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 FVP_DT_PREFIX=fvp-base-gicv3-psci-1t all && \
> >> +    make -j$(nproc) DEBUG=1 PLAT=fvp ARCH=aarch64 FVP_DT_PREFIX=fvp-base-gicv3-psci-1t fip BL33=../u-boot-"$UBOOT_VERSION"/u-boot.bin && \
> >> +    cp build/fvp/debug/bl1.bin / && \
> >> +    cp build/fvp/debug/fip.bin / && \
> >> +    cp build/fvp/debug/fdts/fvp-base-gicv3-psci-1t.dtb / && \
> >> +    cd /build && \
> >> +    rm -rf u-boot-"$UBOOT_VERSION" trusted-firmware-a
> > You forgot to remove u-boot tar file
> 
> oops, nice catch, thanks. Will also remove that in v3.
> 
> > Other than that:
> > Reviewed-by: Michal Orzel <michal.orzel@amd.com>
> 
> Thanks!
> 
> Stefano: Can I keep your Reviewed-by tag after addressing Michal’s comments above?

Yes

Cheers,

Stefano

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 3/5] automation: Add the expect script with test case for FVP
  2023-12-08 12:05             ` Julien Grall
  2023-12-08 12:17               ` Henry Wang
@ 2023-12-08 21:30               ` Stefano Stabellini
  1 sibling, 0 replies; 26+ messages in thread
From: Stefano Stabellini @ 2023-12-08 21:30 UTC (permalink / raw)
  To: Julien Grall
  Cc: Michal Orzel, Henry Wang, Xen-devel, Doug Goldstein,
	Stefano Stabellini, Bertrand Marquis, Wei Chen

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

On Fri, 8 Dec 2023, Julien Grall wrote:
> On 08/12/2023 09:50, Michal Orzel wrote:
> > On 08/12/2023 10:21, Henry Wang wrote:
> > > > On Dec 8, 2023, at 17:11, Michal Orzel <michal.orzel@amd.com> wrote:
> > > > On 08/12/2023 10:05, Henry Wang wrote:
> > > > > 
> > > > > Hi Michal,
> > > > > 
> > > > > > On Dec 8, 2023, at 16:57, Michal Orzel <michal.orzel@amd.com> wrote:
> > > > > > 
> > > > > > Hi Henry,
> > > > > > 
> > > > > > On 08/12/2023 06:46, Henry Wang wrote:
> > > > > > > diff --git
> > > > > > > a/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
> > > > > > > b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
> > > > > > > new file mode 100755
> > > > > > > index 0000000000..25d9a5f81c
> > > > > > > --- /dev/null
> > > > > > > +++ b/automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp
> > > > > > > @@ -0,0 +1,73 @@
> > > > > > > +#!/usr/bin/expect
> > > > > > > +
> > > > > > > +set timeout 2000
> > > > > > Do we really need such a big timeout (~30 min)?
> > > > > > Looking at your test job, it took 16 mins (quite a lot but I know
> > > > > > FVP is slow
> > > > > > + send_slow slows things down)
> > > > > 
> > > > > This is a really good question. I did have the same question while
> > > > > working on
> > > > > the negative test today. The timeout 2000 indeed will fail the job at
> > > > > about 30min,
> > > > > and waiting for it is indeed not really pleasant.
> > > > > 
> > > > > But my second thought would be - from my observation, the overall time
> > > > > now
> > > > > would vary between 15min ~ 20min, and having a 10min margin is not
> > > > > that crazy
> > > > > given that we probably will do more testing from the job in the
> > > > > future, and if the
> > > > > GitLab Arm worker is high loaded, FVP will probably become slower. And
> > > > > normally
> > > > > we don’t even trigger the timeout as the job will normally pass. So I
> > > > > decided
> > > > > to keep this.
> > > > > 
> > > > > Mind sharing your thoughts about the better value of the timeout?
> > > > > Probably 25min?
> > > >  From what you said that the average is 15-20, I think we can leave it
> > > > set to 30.
> > > > But I wonder if we can do something to decrease the average time. ~20
> > > > min is a lot
> > > > even for FVP :) Have you tried setting send_slow to something lower than
> > > > 100ms?
> > > > That said, we don't send too many chars to FVP, so I doubt it would play
> > > > a major role
> > > > in the overall time.
> > > 
> > > I agree with the send_slow part. Actually I do have the same concern, here
> > > are my current
> > > understanding and I think you will definitely help with your knowledge:
> > > If you check the full log of Dom0 booting, for example [1], you will find
> > > that we wasted so
> > > much time in starting the services of the OS (modloop, udev-settle, etc).
> > > All of these services
> > > are retried many times but in the end they are still not up, and from my
> > > understanding they
> > > won’t affect the actual test(?) If we can somehow get rid of these
> > > services from rootfs, I think
> > > we can save a lot of time.
> > > 
> > > And honestly, I noticed that qemu-alpine-arm64-gcc suffers from the same
> > > problem and it also
> > > takes around 15min to finish. So if we managed to tailor the services from
> > > the filesystem, we
> > > can save a lot of time.
> > That is not true. Qemu runs the tests relatively fast within few minutes.
> > The reason you see e.g. 12 mins
> > for some Qemu jobs comes from the timeout we set in Qemu scripts. We don't
> > have yet the solution (we could
> > do the same as Qubes script) to detect the test success early and exit
> > before timeout. That is why currently
> > the only way for Qemu tests to finish is by reaching the timeout.
> > 
> > So the problem is not with the rootfs and services (the improvement would
> > not be significant) but with
> > the simulation being slow.
> 
> From my experience with the FVP improvement would be significant. A normal
> boot distribution will start a lot of services. I end up to write my own
> initscript doing the bare minimum for creating a guest. This saves me a lot of
> time everytime I needed to test on FVP.
> 
> I think we can do the same for the gitlab. Maybe not to the point of writing
> your initscript but cutting down anything unnecessary.
> 
> This will avoid the FVP test to become the bottlneck in the gitlab CI.

Along the same lines another idea would be to use busybox alone (no
Alpine Linux) as Dom0 rootfs. That's going to be faster, but you
cannot really use xl to create DomUs due to libraries and other
dependencies but you can for sure create additional guests using
Dom0less, see for instance
automation/scripts/qemu-smoke-dom0less-arm64.sh

So if you have troubles improving the boot times of Dom0 + xl create an
alternative would be to create two Linux dom0less DomUs both of them
with only busybox as ramdisk.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 4/5] automation: Add the script for the FVP smoke test
  2023-12-08  5:46 ` [PATCH v2 4/5] automation: Add the script for the FVP smoke test Henry Wang
@ 2023-12-08 21:46   ` Stefano Stabellini
  0 siblings, 0 replies; 26+ messages in thread
From: Stefano Stabellini @ 2023-12-08 21:46 UTC (permalink / raw)
  To: Henry Wang
  Cc: xen-devel, Doug Goldstein, Stefano Stabellini, Michal Orzel,
	Julien Grall, Bertrand Marquis, Wei Chen

On Fri, 8 Dec 2023, Henry Wang wrote:
> This commit adds the shell script for the FVP smoke test. Similarly
> as the QEMU jobs, the shell script will firstly prepare the DomU
> BusyBox image, use the ImageBuilder to arrange the binaries in memory
> and generate the U-Boot script, then start the test. To provide the
> TFTP service for the FVP, the shell script will also start the TFTP
> service, and copy the binaries needed by test to the TFTP directory
> used by the TFTP server.
> 
> Signed-off-by: Henry Wang <Henry.Wang@arm.com>
> ---
> v2:
> - Set pipefail before running the expect script, so that the error
>   won't be hid by pipe and the tee to the logfile.
> ---
>  .../scripts/fvp-base-smoke-dom0-arm64.sh      | 120 ++++++++++++++++++
>  1 file changed, 120 insertions(+)
>  create mode 100755 automation/scripts/fvp-base-smoke-dom0-arm64.sh
> 
> diff --git a/automation/scripts/fvp-base-smoke-dom0-arm64.sh b/automation/scripts/fvp-base-smoke-dom0-arm64.sh
> new file mode 100755
> index 0000000000..99097dad51
> --- /dev/null
> +++ b/automation/scripts/fvp-base-smoke-dom0-arm64.sh
> @@ -0,0 +1,120 @@
> +#!/bin/bash
> +
> +set -ex
> +
> +# DomU Busybox
> +cd binaries
> +mkdir -p initrd
> +mkdir -p initrd/bin
> +mkdir -p initrd/sbin
> +mkdir -p initrd/etc
> +mkdir -p initrd/dev
> +mkdir -p initrd/proc
> +mkdir -p initrd/sys
> +mkdir -p initrd/lib
> +mkdir -p initrd/var
> +mkdir -p initrd/mnt
> +cp /bin/busybox initrd/bin/busybox
> +initrd/bin/busybox --install initrd/bin
> +echo "#!/bin/sh
> +
> +mount -t proc proc /proc
> +mount -t sysfs sysfs /sys
> +mount -t devtmpfs devtmpfs /dev
> +/bin/sh" > initrd/init
> +chmod +x initrd/init
> +cd initrd
> +find . | cpio --create --format='newc' | gzip > ../initrd.cpio.gz
> +cd ..
> +
> +mkdir -p rootfs
> +cd rootfs
> +tar xvzf ../initrd.tar.gz
> +mkdir proc
> +mkdir run
> +mkdir srv
> +mkdir sys
> +rm var/run
> +cp -ar ../dist/install/* .
> +mv ../initrd.cpio.gz ./root
> +cp ../Image ./root
> +echo "name=\"test\"
> +memory=512
> +vcpus=1
> +kernel=\"/root/Image\"
> +ramdisk=\"/root/initrd.cpio.gz\"
> +extra=\"console=hvc0 root=/dev/ram0 rdinit=/bin/sh\"
> +" > root/test.cfg
> +echo "#!/bin/bash
> +
> +export LD_LIBRARY_PATH=/usr/local/lib
> +bash /etc/init.d/xencommons start
> +
> +xl list
> +
> +xl create -c /root/test.cfg
> +
> +" > etc/local.d/xen.start
> +chmod +x etc/local.d/xen.start
> +echo "rc_verbose=yes" >> etc/rc.conf
> +find . |cpio -H newc -o|gzip > ../xen-rootfs.cpio.gz
> +cd ../..
> +
> +# Start a TFTP server to provide TFTP service to FVP
> +service tftpd-hpa start
> +
> +# ImageBuilder
> +echo 'MEMORY_START="0x80000000"
> +MEMORY_END="0xFF000000"
> +
> +DEVICE_TREE="fvp-base-gicv3-psci-1t.dtb"
> +XEN="xen"
> +DOM0_KERNEL="Image"
> +DOM0_RAMDISK="xen-rootfs.cpio.gz"
> +XEN_CMD="console=dtuart dom0_mem=1024M console_timestamps=boot"
> +
> +NUM_DOMUS=0
> +
> +LOAD_CMD="tftpb"
> +UBOOT_SOURCE="boot.source"
> +UBOOT_SCRIPT="boot.scr"' > binaries/config
> +rm -rf imagebuilder
> +git clone https://gitlab.com/ViryaOS/imagebuilder
> +bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c binaries/config
> +
> +# Copy files to the TFTP directory to use
> +cp ./binaries/boot.scr /srv/tftp/
> +cp ./binaries/Image /srv/tftp/
> +cp ./binaries/xen-rootfs.cpio.gz /srv/tftp/
> +cp ./binaries/xen /srv/tftp/
> +cp ./binaries/fvp-base-gicv3-psci-1t.dtb /srv/tftp/
> +
> +# Start FVP
> +TERM0_CFG="-C bp.terminal_0.mode=telnet -C bp.terminal_0.start_telnet=0"
> +TERM1_CFG="-C bp.terminal_1.mode=telnet -C bp.terminal_1.start_telnet=0"
> +TERM2_CFG="-C bp.terminal_2.mode=telnet -C bp.terminal_2.start_telnet=0"
> +TERM3_CFG="-C bp.terminal_3.mode=telnet -C bp.terminal_3.start_telnet=0"
> +
> +VIRTIO_USER_NETWORK_CFG="-C bp.virtio_net.enabled=1 \
> +-C bp.virtio_net.hostbridge.userNetworking=1 \
> +-C bp.virtio_net.hostbridge.userNetPorts=8022=22 \
> +-C bp.virtio_net.transport=legacy"
> +
> +# Set the pipefail so that the error code from the expect script won't
> +# be hid by pipe and the tee command.
> +set -o pipefail
> +./automation/scripts/expect/fvp-base-smoke-dom0-arm64.exp \
> +    "/FVP/FVP_Base_RevC-2xAEMvA/Base_RevC_AEMvA_pkg/models/Linux64_armv8l_GCC-9.3/FVP_Base_RevC-2xAEMvA \
> +    -C bp.vis.disable_visualisation=1 \
> +    -C bp.ve_sysregs.exit_on_shutdown=1 \
> +    -C bp.secure_memory=0 \
> +    -C cache_state_modelled=0 \
> +    -C cluster0.has_arm_v8-4=1 \
> +    -C cluster1.has_arm_v8-4=1 \
> +    ${TERM0_CFG} ${TERM1_CFG} ${TERM2_CFG} ${TERM3_CFG} \
> +    ${VIRTIO_USER_NETWORK_CFG} \
> +    -C bp.secureflashloader.fname=$(pwd)/binaries/bl1.bin \
> +    -C bp.flashloader0.fname=$(pwd)/binaries/fip.bin" |& \
> +        tee smoke.serial

Now the expectation is that on failure (such as timeout failure)
fvp-base-smoke-dom0-arm64.exp will exit non-zero and therefore this
script will also exit non-zero. pipefail is necessary otherwise tee will
mask the error. I like it.

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


> +exit 0
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2023-12-08 21:47 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-08  5:46 [PATCH v2 0/5] automation: Support running FVP Dom0 smoke test for Arm Henry Wang
2023-12-08  5:46 ` [PATCH v2 1/5] automation: Add a Dockerfile for running FVP_Base jobs Henry Wang
2023-12-08 12:30   ` Julien Grall
2023-12-08 12:59     ` Henry Wang
2023-12-08 14:24       ` Julien Grall
2023-12-08  5:46 ` [PATCH v2 2/5] automation: Add the Dockerfile to build TF-A and U-Boot for FVP Henry Wang
2023-12-08  8:39   ` Michal Orzel
2023-12-08  8:56     ` Henry Wang
2023-12-08 21:20       ` Stefano Stabellini
2023-12-08  5:46 ` [PATCH v2 3/5] automation: Add the expect script with test case " Henry Wang
2023-12-08  8:57   ` Michal Orzel
2023-12-08  9:05     ` Henry Wang
2023-12-08  9:11       ` Michal Orzel
2023-12-08  9:21         ` Henry Wang
2023-12-08  9:50           ` Michal Orzel
2023-12-08  9:56             ` Henry Wang
2023-12-08 11:13               ` Wei Chen
2023-12-08 12:27                 ` Henry Wang
2023-12-08 12:05             ` Julien Grall
2023-12-08 12:17               ` Henry Wang
2023-12-08 21:30               ` Stefano Stabellini
2023-12-08  5:46 ` [PATCH v2 4/5] automation: Add the script for the FVP smoke test Henry Wang
2023-12-08 21:46   ` Stefano Stabellini
2023-12-08  5:46 ` [PATCH v2 5/5] automation: Add the arm64 FVP build and Dom0 smoke test jobs Henry Wang
2023-12-08  9:19   ` Michal Orzel
2023-12-08  9:22     ` Henry Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.