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

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

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