| Message ID | 20220726190701.1048824-3-burzalodowa@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Create a test job for testing static memory on qemu | expand |
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
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
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
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(-)