[PATCH v2 0/2] Create a test job for testing static memory on qemu

[PATCH v2 0/2] Create a test job for testing static memory on qemu

[Xenia Ragiadakou]

This patch series
- removes all the references to the XEN_CONFIG_EXPERT environmental variable
which is not used anymore
- creates a trivial arm64 test job that boots xen on qemu with dom0 and a
direct mapped dom0less domu with static memory and verifies that domu's memory
ranges are the same as the static memory ranges with which it was configured

The static memory test relies on the existing qemu-smoke-arm64.sh script.
This script uses the kernel-5.9.9 from the test-artifacts container as domu
kernel. This particular kernel does not work with dom0less enhanced enabled.
More specifically, domu crashes when it attempts to dereference the xenstore
interface which is still uninitialized,
So, qemu-smoke-arm64-gcc test, as well as its static memory version, fails.

To be able to test, I had to disable dom0less enhanced by adding the following
commands to the script.

sed -i 's/xen,enhanced "enabled"/xen,enhanced "disabled"/g' binaries/boot.source
mkimage -A arm64 -T script -C none -d binaries/boot.source binaries/boot.scr

These commands are not part of the patch.
Since dom0less enhanced is enabled by default, a newer kernel version would
be more appropriate for testing dom0less.

Xenia Ragiadakou (2):
  automation: Remove XEN_CONFIG_EXPERT leftovers
  automation: arm64: Create a test job for testing static allocation on
    qemu

 automation/build/README.md             |  3 --
 automation/gitlab-ci/test.yaml         | 18 +++++++++++
 automation/scripts/build               | 12 ++++++--
 automation/scripts/containerize        |  1 -
 automation/scripts/qemu-smoke-arm64.sh | 42 ++++++++++++++++++++++++--
 5 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] automation: Remove XEN_CONFIG_EXPERT leftovers

[Xenia Ragiadakou]

The EXPERT config option cannot anymore be selected via the environmental
variable XEN_CONFIG_EXPERT. Remove stale references to XEN_CONFIG_EXPERT
from the automation code.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---

Changes in v2:
- add Stefano's R-b tag

 automation/build/README.md      | 3 ---
 automation/scripts/build        | 4 ++--
 automation/scripts/containerize | 1 -
 3 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/automation/build/README.md b/automation/build/README.md
index 2137957408..00305eed03 100644
--- a/automation/build/README.md
+++ b/automation/build/README.md
@@ -65,9 +65,6 @@ understands.
 - CONTAINER_NO_PULL: If set to 1, the script will not pull from docker hub.
   This is useful when testing container locally.
 
-- XEN_CONFIG_EXPERT: If this is defined in your shell it will be
-  automatically passed through to the container.
-
 If your docker host has Linux kernel > 4.11, and you want to use containers
 that run old glibc (for example, CentOS 6 or SLES11SP4), you may need to add
 
diff --git a/automation/scripts/build b/automation/scripts/build
index 281f8b1fcc..21b3bc57c8 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -91,6 +91,6 @@ for cfg in `ls ${cfg_dir}`; do
     echo "Building $cfg"
     make -j$(nproc) -C xen clean
     rm -f xen/.config
-    make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} XEN_CONFIG_EXPERT=y defconfig
-    make -j$(nproc) -C xen XEN_CONFIG_EXPERT=y
+    make -C xen KBUILD_DEFCONFIG=../../../../${cfg_dir}/${cfg} defconfig
+    make -j$(nproc) -C xen
 done
diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index 8992c67278..9d4beca4fa 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -101,7 +101,6 @@ exec ${docker_cmd} run \
     -v "${CONTAINER_PATH}":/build:rw${selinux} \
     -v "${HOME}/.ssh":/root/.ssh:ro \
     ${SSH_AUTH_DIR:+-v "${SSH_AUTH_DIR}":/tmp/ssh-agent${selinux}} \
-    ${XEN_CONFIG_EXPERT:+-e XEN_CONFIG_EXPERT=${XEN_CONFIG_EXPERT}} \
     ${CONTAINER_ARGS} \
     -${termint}i --rm -- \
     ${CONTAINER} \
-- 
2.34.1

[PATCH v2 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Xenia Ragiadakou]

Enable CONFIG_STATIC_MEMORY in the existing arm64 build.

Create a new test job, called qemu-smoke-arm64-gcc-staticmem.

Adjust qemu-smoke-arm64.sh script to accomodate the static memory test as a
new test variant. The test variant is determined based on the first argument
passed to the script. For testing static memory, the argument is 'static-mem'.

The test configures DOM1 with a static memory region and adds a check in the
init script.
The check consists in comparing the contents of the /proc/device-tree
memory entry with the static memory range with which DOM1 was configured.
If the memory layout is correct, a message gets printed by DOM1.

At the end of the qemu run, the script searches for the specific message
in the logs and fails if not found.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---

Changes in v2:
- enable CONFIG_STATIC_MEMORY for all arm64 builds
- use the existing qemu-smoke-arm64.sh with an argument passed for
  distinguishing between tests, instead of creating a new script
- test does not rely on kernel logs but prints a message itself on success
- remove the part that enables custom configs for arm64 builds
- add comments
- adapt commit message

 automation/gitlab-ci/test.yaml         | 18 +++++++++++
 automation/scripts/build               |  8 +++++
 automation/scripts/qemu-smoke-arm64.sh | 42 ++++++++++++++++++++++++--
 3 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
index 26bdbcc3ea..6f9f64a8da 100644
--- a/automation/gitlab-ci/test.yaml
+++ b/automation/gitlab-ci/test.yaml
@@ -80,6 +80,24 @@ qemu-smoke-arm64-gcc:
   tags:
     - arm64
 
+qemu-smoke-arm64-gcc-staticmem:
+  extends: .test-jobs-common
+  variables:
+    CONTAINER: debian:unstable-arm64v8
+  script:
+    - ./automation/scripts/qemu-smoke-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log
+  needs:
+    - debian-unstable-gcc-arm64
+    - kernel-5.9.9-arm64-export
+    - qemu-system-aarch64-6.0.0-arm64-export
+  artifacts:
+    paths:
+      - smoke.serial
+      - '*.log'
+    when: always
+  tags:
+    - arm64
+
 qemu-smoke-arm32-gcc:
   extends: .test-jobs-common
   variables:
diff --git a/automation/scripts/build b/automation/scripts/build
index 21b3bc57c8..1941763315 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -18,6 +18,14 @@ else
     make -j$(nproc) -C xen defconfig
 fi
 
+if [[ "${XEN_TARGET_ARCH}" = "arm64" ]]; then
+    echo "
+CONFIG_EXPERT=y
+CONFIG_UNSUPPORTED=y
+CONFIG_STATIC_MEMORY=y" > xen/.config
+    make -j$(nproc) -C xen olddefconfig
+fi
+
 # Save the config file before building because build failure causes the script
 # to exit early -- bash is invoked with -e.
 cp xen/.config xen-config
diff --git a/automation/scripts/qemu-smoke-arm64.sh b/automation/scripts/qemu-smoke-arm64.sh
index 53086a5ac7..48513a7832 100755
--- a/automation/scripts/qemu-smoke-arm64.sh
+++ b/automation/scripts/qemu-smoke-arm64.sh
@@ -2,6 +2,15 @@
 
 set -ex
 
+test_variant=$1
+
+if [[ "${test_variant}" == "static-mem" ]]; then
+    # Memory range that is statically allocated to DOM1
+    domu_base="50000000"
+    domu_size="10000000"
+    passed="static-mem test passed"
+fi
+
 # Install QEMU
 export DEBIAN_FRONTENT=noninteractive
 apt-get -qy update
@@ -42,8 +51,22 @@ echo "#!/bin/sh
 
 mount -t proc proc /proc
 mount -t sysfs sysfs /sys
-mount -t devtmpfs devtmpfs /dev
-/bin/sh" > initrd/init
+mount -t devtmpfs devtmpfs /dev" > initrd/init
+
+if [[ "${test_variant}" == "static-mem" ]]; then
+    # Verify that DOM1 booted with the expected memory layout by checking the
+    # contents of the memory entry in /proc/device-tree.
+    echo "
+current=\$(hexdump -e '16/1 \"%02x\"' /proc/device-tree/memory@${domu_base}/reg 2>/dev/null)
+expected=\"$(printf \"%016x%016x\" 0x${domu_base} 0x${domu_size})\"
+if [[ ${expected} == \"\${current}\" ]]; then
+	echo \"${passed}\"
+fi" >> initrd/init
+fi
+
+echo "
+/bin/sh" >> initrd/init
+
 chmod +x initrd/init
 cd initrd
 find . | cpio --create --format='newc' | gzip > ../binaries/initrd
@@ -68,6 +91,12 @@ DOMU_MEM[0]="256"
 LOAD_CMD="tftpb"
 UBOOT_SOURCE="boot.source"
 UBOOT_SCRIPT="boot.scr"' > binaries/config
+
+if [[ "${test_variant}" == "static-mem" ]]; then
+    echo "
+DOMU_STATIC_MEM[0]=\"0x${domu_base} 0x${domu_size}\"" >> binaries/config
+fi
+
 rm -rf imagebuilder
 git clone https://gitlab.com/ViryaOS/imagebuilder
 bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c binaries/config
@@ -89,5 +118,12 @@ timeout -k 1 240 \
     -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin |& tee smoke.serial
 
 set -e
-(grep -q "^BusyBox" smoke.serial && grep -q "DOM1: BusyBox" smoke.serial) || exit 1
+grep -q "^BusyBox" smoke.serial || exit 1
+
+if [[ "${test_variant}" == "static-mem" ]]; then
+    grep -q "DOM1: ${passed}" smoke.serial || exit 1
+else
+    grep -q "DOM1: BusyBox" smoke.serial || exit 1
+fi
+
 exit 0
-- 
2.34.1

Re: [PATCH v2 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Stefano Stabellini]

On Tue, 26 Jul 2022, Xenia Ragiadakou wrote:
> Enable CONFIG_STATIC_MEMORY in the existing arm64 build.
> 
> Create a new test job, called qemu-smoke-arm64-gcc-staticmem.
> 
> Adjust qemu-smoke-arm64.sh script to accomodate the static memory test as a
> new test variant. The test variant is determined based on the first argument
> passed to the script. For testing static memory, the argument is 'static-mem'.
> 
> The test configures DOM1 with a static memory region and adds a check in the
> init script.
> The check consists in comparing the contents of the /proc/device-tree
> memory entry with the static memory range with which DOM1 was configured.
> If the memory layout is correct, a message gets printed by DOM1.
> 
> At the end of the qemu run, the script searches for the specific message
> in the logs and fails if not found.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

This looks good, I only have a couple of minor comments about code style
and a small suggestion for improvements.


> ---
> 
> Changes in v2:
> - enable CONFIG_STATIC_MEMORY for all arm64 builds
> - use the existing qemu-smoke-arm64.sh with an argument passed for
>   distinguishing between tests, instead of creating a new script
> - test does not rely on kernel logs but prints a message itself on success
> - remove the part that enables custom configs for arm64 builds
> - add comments
> - adapt commit message
> 
>  automation/gitlab-ci/test.yaml         | 18 +++++++++++
>  automation/scripts/build               |  8 +++++
>  automation/scripts/qemu-smoke-arm64.sh | 42 ++++++++++++++++++++++++--
>  3 files changed, 65 insertions(+), 3 deletions(-)
> 
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 26bdbcc3ea..6f9f64a8da 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -80,6 +80,24 @@ qemu-smoke-arm64-gcc:
>    tags:
>      - arm64
>  
> +qemu-smoke-arm64-gcc-staticmem:
> +  extends: .test-jobs-common
> +  variables:
> +    CONTAINER: debian:unstable-arm64v8
> +  script:
> +    - ./automation/scripts/qemu-smoke-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log
> +  needs:
> +    - debian-unstable-gcc-arm64
> +    - kernel-5.9.9-arm64-export
> +    - qemu-system-aarch64-6.0.0-arm64-export
> +  artifacts:
> +    paths:
> +      - smoke.serial
> +      - '*.log'
> +    when: always
> +  tags:
> +    - arm64
> +
>  qemu-smoke-arm32-gcc:
>    extends: .test-jobs-common
>    variables:
> diff --git a/automation/scripts/build b/automation/scripts/build
> index 21b3bc57c8..1941763315 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -18,6 +18,14 @@ else
>      make -j$(nproc) -C xen defconfig
>  fi
>  
> +if [[ "${XEN_TARGET_ARCH}" = "arm64" ]]; then
> +    echo "
> +CONFIG_EXPERT=y
> +CONFIG_UNSUPPORTED=y
> +CONFIG_STATIC_MEMORY=y" > xen/.config
> +    make -j$(nproc) -C xen olddefconfig
> +fi

I think we should do this within the not "${RANDCONFIG}" == "y" case
above to avoid calling make defconfig needlessly.


> +
>  # Save the config file before building because build failure causes the script
>  # to exit early -- bash is invoked with -e.
>  cp xen/.config xen-config
> diff --git a/automation/scripts/qemu-smoke-arm64.sh b/automation/scripts/qemu-smoke-arm64.sh
> index 53086a5ac7..48513a7832 100755
> --- a/automation/scripts/qemu-smoke-arm64.sh
> +++ b/automation/scripts/qemu-smoke-arm64.sh
> @@ -2,6 +2,15 @@
>  
>  set -ex
>  
> +test_variant=$1
> +
> +if [[ "${test_variant}" == "static-mem" ]]; then
> +    # Memory range that is statically allocated to DOM1
> +    domu_base="50000000"
> +    domu_size="10000000"
> +    passed="static-mem test passed"
> +fi

I think we should set passed to "BusyBox" for the regular case so that
we don't need an if/else at the bottom to check for passed


>  # Install QEMU
>  export DEBIAN_FRONTENT=noninteractive
>  apt-get -qy update
> @@ -42,8 +51,22 @@ echo "#!/bin/sh
>  
>  mount -t proc proc /proc
>  mount -t sysfs sysfs /sys
> -mount -t devtmpfs devtmpfs /dev
> -/bin/sh" > initrd/init
> +mount -t devtmpfs devtmpfs /dev" > initrd/init
> +
> +if [[ "${test_variant}" == "static-mem" ]]; then
> +    # Verify that DOM1 booted with the expected memory layout by checking the
> +    # contents of the memory entry in /proc/device-tree.
> +    echo "
> +current=\$(hexdump -e '16/1 \"%02x\"' /proc/device-tree/memory@${domu_base}/reg 2>/dev/null)
> +expected=\"$(printf \"%016x%016x\" 0x${domu_base} 0x${domu_size})\"

This works but it is hard to read. I am suggesting the follow to improve
readability:

---
echo "#!/bin/sh

mount -t proc proc /proc
mount -t sysfs sysfs /sys
mount -t devtmpfs devtmpfs /dev

current=\$(hexdump -e '16/1 \"%02x\"' /proc/device-tree/memory@${domu_base}/reg 2>/dev/null)
expected=\"$(printf \"%016x%016x\" 0x${domu_base} 0x${domu_size})\"
if [[ ${expected} == \"\${current}\" ]]; then
    echo \"${passed}\"
fi
" > initrd/init
---

I know that it is going to be a couple of extra instructions on the
non-static case to run inside QEMU but the advantage is that the init
script is identical between the two cases which makes the code easier.




> +if [[ ${expected} == \"\${current}\" ]]; then
> +	echo \"${passed}\"
> +fi" >> initrd/init
> +fi
> +
> +echo "
> +/bin/sh" >> initrd/init

Is this needed?


>  chmod +x initrd/init
>  cd initrd
>  find . | cpio --create --format='newc' | gzip > ../binaries/initrd
> @@ -68,6 +91,12 @@ DOMU_MEM[0]="256"
>  LOAD_CMD="tftpb"
>  UBOOT_SOURCE="boot.source"
>  UBOOT_SCRIPT="boot.scr"' > binaries/config
> +
> +if [[ "${test_variant}" == "static-mem" ]]; then
> +    echo "
> +DOMU_STATIC_MEM[0]=\"0x${domu_base} 0x${domu_size}\"" >> binaries/config

please keep it on a single line for readability (even if it will push it
above 80 chars):

    echo "DOMU_STATIC_MEM[0]=\"0x${domu_base} 0x${domu_size}\"" >> binaries/config


> +fi
> +
>  rm -rf imagebuilder
>  git clone https://gitlab.com/ViryaOS/imagebuilder
>  bash imagebuilder/scripts/uboot-script-gen -t tftp -d binaries/ -c binaries/config
> @@ -89,5 +118,12 @@ timeout -k 1 240 \
>      -bios /usr/lib/u-boot/qemu_arm64/u-boot.bin |& tee smoke.serial
>  
>  set -e
> -(grep -q "^BusyBox" smoke.serial && grep -q "DOM1: BusyBox" smoke.serial) || exit 1
> +grep -q "^BusyBox" smoke.serial || exit 1
> +
> +if [[ "${test_variant}" == "static-mem" ]]; then
> +    grep -q "DOM1: ${passed}" smoke.serial || exit 1
> +else
> +    grep -q "DOM1: BusyBox" smoke.serial || exit 1
> +fi

No need for this if/else/fi if we set $passed in both cases at the top

RE: [PATCH v2 2/2] automation: arm64: Create a test job for testing static allocation on qemu

[Penny Zheng]

Hi Xenia

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> Xenia Ragiadakou
> Sent: Wednesday, July 27, 2022 3:07 AM
> To: xen-devel@lists.xenproject.org
> Cc: Doug Goldstein <cardoe@cardoe.com>; Stefano Stabellini
> <sstabellini@kernel.org>
> Subject: [PATCH v2 2/2] automation: arm64: Create a test job for testing
> static allocation on qemu
> 
> Enable CONFIG_STATIC_MEMORY in the existing arm64 build.
> 
> Create a new test job, called qemu-smoke-arm64-gcc-staticmem.
> 
> Adjust qemu-smoke-arm64.sh script to accomodate the static memory test
> as a new test variant. The test variant is determined based on the first
> argument passed to the script. For testing static memory, the argument is
> 'static-mem'.
> 
> The test configures DOM1 with a static memory region and adds a check in
> the init script.
> The check consists in comparing the contents of the /proc/device-tree
> memory entry with the static memory range with which DOM1 was
> configured.
> If the memory layout is correct, a message gets printed by DOM1.
> 
> At the end of the qemu run, the script searches for the specific message in
> the logs and fails if not found.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>

Reviewed-by: Penny Zheng <penny.zheng@arm.com>

> ---
> 
> Changes in v2:
> - enable CONFIG_STATIC_MEMORY for all arm64 builds
> - use the existing qemu-smoke-arm64.sh with an argument passed for
>   distinguishing between tests, instead of creating a new script
> - test does not rely on kernel logs but prints a message itself on success
> - remove the part that enables custom configs for arm64 builds
> - add comments
> - adapt commit message
> 
>  automation/gitlab-ci/test.yaml         | 18 +++++++++++
>  automation/scripts/build               |  8 +++++
>  automation/scripts/qemu-smoke-arm64.sh | 42
> ++++++++++++++++++++++++--
>  3 files changed, 65 insertions(+), 3 deletions(-)
> 
> 

---
Cheers,
Penny Zheng