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

2022-07-28 Thread Stefano Stabellini
On Thu, 28 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 
> Reviewed-by: Penny Zheng 
> ---
> 
> 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
> 
> Changes in v3:
> - refactor the changes to improve readability, no functionality change 
> intended
> - add Penny's R-b tag
> 
>  automation/gitlab-ci/test.yaml | 18 ++
>  automation/scripts/build   | 10 +-
>  automation/scripts/qemu-smoke-arm64.sh | 25 -
>  3 files changed, 51 insertions(+), 2 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..2b9f2d2b54 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -15,7 +15,15 @@ if [[ "${RANDCONFIG}" == "y" ]]; then
>  make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config 
> randconfig
>  hypervisor_only="y"
>  else
> -make -j$(nproc) -C xen defconfig
> +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
> +else
> +make -j$(nproc) -C xen defconfig
> +fi
>  fi
>  
>  # Save the config file before building because build failure causes the 
> script
> diff --git a/automation/scripts/qemu-smoke-arm64.sh 
> b/automation/scripts/qemu-smoke-arm64.sh
> index 53086a5ac7..69d9eae7a9 100755
> --- a/automation/scripts/qemu-smoke-arm64.sh
> +++ b/automation/scripts/qemu-smoke-arm64.sh
> @@ -2,6 +2,23 @@
>  
>  set -ex
>  
> +test_variant=$1
> +
> +passed="BusyBox"
> +check=""
> +
> +if [[ "${test_variant}" == "static-mem" ]]; then
> +# Memory range that is statically allocated to DOM1
> +domu_base="5000"
> +domu_size="1000"
> +passed="${test_variant} test passed"
> +check="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"
> +fi

I would make a minor code style improvement here:

+if [[ "${test_variant}" == "static-mem" ]]; then
+# Memory range that is statically allocated to DOM1
+domu_base="5000"
+domu_size="1000"
+passed="${test_variant} test passed"
+check="
+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
+"
+fi

I can do that on commit. Everything looks good.

Reviewed-by: Stefano Stabellini 



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

2022-07-28 Thread 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 
Reviewed-by: Penny Zheng 
---

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

Changes in v3:
- refactor the changes to improve readability, no functionality change intended
- add Penny's R-b tag

 automation/gitlab-ci/test.yaml | 18 ++
 automation/scripts/build   | 10 +-
 automation/scripts/qemu-smoke-arm64.sh | 25 -
 3 files changed, 51 insertions(+), 2 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..2b9f2d2b54 100755
--- a/automation/scripts/build
+++ b/automation/scripts/build
@@ -15,7 +15,15 @@ if [[ "${RANDCONFIG}" == "y" ]]; then
 make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config 
randconfig
 hypervisor_only="y"
 else
-make -j$(nproc) -C xen defconfig
+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
+else
+make -j$(nproc) -C xen defconfig
+fi
 fi
 
 # Save the config file before building because build failure causes the script
diff --git a/automation/scripts/qemu-smoke-arm64.sh 
b/automation/scripts/qemu-smoke-arm64.sh
index 53086a5ac7..69d9eae7a9 100755
--- a/automation/scripts/qemu-smoke-arm64.sh
+++ b/automation/scripts/qemu-smoke-arm64.sh
@@ -2,6 +2,23 @@
 
 set -ex
 
+test_variant=$1
+
+passed="BusyBox"
+check=""
+
+if [[ "${test_variant}" == "static-mem" ]]; then
+# Memory range that is statically allocated to DOM1
+domu_base="5000"
+domu_size="1000"
+passed="${test_variant} test passed"
+check="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"
+fi
+
 # Install QEMU
 export DEBIAN_FRONTENT=noninteractive
 apt-get -qy update
@@ -43,6 +60,7 @@ echo "#!/bin/sh
 mount -t proc proc /proc
 mount -t sysfs sysfs /sys
 mount -t devtmpfs devtmpfs /dev
+${check}
 /bin/sh" > initrd/init
 chmod +x initrd/init
 cd initrd
@@ -68,6 +86,11 @@ DOMU_MEM[0]="256"
 LOAD_CMD="tftpb"
 UBOOT_SOURCE="boot.source"
 UBOOT_SCRIPT="boot.scr"' > binaries/config
+
+if [[ "${test_variant}" == "static-mem" ]]; then
+echo -e "\nDOMU_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 +112,5 @@ 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 && grep -q "DOM1: ${passed}" smoke.serial) || 
exit 1
 exit 0
-- 
2.34.1