On Fri, 2 Sep 2022, Michal Orzel wrote:
> Add a new test job qemu-smoke-arm64-gcc-boot-cpupools that will execute
> script qemu-smoke-arm64.sh to test boot time cpupools feature.
> Enable CONFIG_BOOT_TIME_CPUPOOLS for the arm64 build and add a new test
> case in qemu-smoke-arm64.sh that if selected will:
> - create a device tree cpupool node with cpu@1
> - assign created cpupool to domU0
> - add a check in dom0 xen.start to see if domU is assigned a Pool-1
>
> Take the opportunity to refactor the qemu-smoke-arm64.sh script as
> follows:
> - use domU_check to store the test's commands to be run from domU
> - use dom0_check to store the test's commands to be run from dom0
> - use fdtput instead of sed to perform dtb modifications
> - use more meaningful messages for "passed" variable. This way we can
> grep for messages reported either by domU or dom0 and get rid of
> assumption that tests can only be run from domU
>
> Signed-off-by: Michal Orzel <michal.or...@amd.com>
> ---
> automation/gitlab-ci/test.yaml | 19 +++++++++++++++
> automation/scripts/build | 3 ++-
> automation/scripts/qemu-smoke-arm64.sh | 33 +++++++++++++++++++-------
> 3 files changed, 45 insertions(+), 10 deletions(-)
>
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 07209820b474..d899b3bdbf7a 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -100,6 +100,25 @@ qemu-smoke-arm64-gcc-staticmem:
> tags:
> - arm64
>
> +qemu-smoke-arm64-gcc-boot-cpupools:
> + extends: .test-jobs-common
> + variables:
> + CONTAINER: debian:unstable-arm64v8
> + script:
> + - ./automation/scripts/qemu-smoke-arm64.sh boot-cpupools 2>&1 | tee
> qemu-smoke-arm64.log
> + needs:
> + - alpine-3.12-gcc-arm64
> + - alpine-3.12-arm64-rootfs-export
> + - kernel-5.19-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 2b9f2d2b541a..2f15ab3198e6 100755
> --- a/automation/scripts/build
> +++ b/automation/scripts/build
> @@ -19,7 +19,8 @@ else
> echo "
> CONFIG_EXPERT=y
> CONFIG_UNSUPPORTED=y
> -CONFIG_STATIC_MEMORY=y" > xen/.config
> +CONFIG_STATIC_MEMORY=y
> +CONFIG_BOOT_TIME_CPUPOOLS=y" > xen/.config
> make -j$(nproc) -C xen olddefconfig
> else
> make -j$(nproc) -C xen defconfig
> diff --git a/automation/scripts/qemu-smoke-arm64.sh
> b/automation/scripts/qemu-smoke-arm64.sh
> index 7ac96027760d..c2184850293c 100755
> --- a/automation/scripts/qemu-smoke-arm64.sh
> +++ b/automation/scripts/qemu-smoke-arm64.sh
> @@ -4,20 +4,22 @@ set -ex
>
> test_variant=$1
>
> -passed="passed"
> -check="
> +if [ -z "${test_variant}" ]; then
> + passed="ping test passed"
> + domU_check="
> until ifconfig eth0 192.168.0.2 &> /dev/null && ping -c 10 192.168.0.1; do
> sleep 30
> done
> echo \"${passed}\"
> "
> +fi
>
> if [[ "${test_variant}" == "static-mem" ]]; then
> # Memory range that is statically allocated to DOM1
> domu_base="50000000"
> domu_size="10000000"
> passed="${test_variant} test passed"
> - check="
> + domU_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
> @@ -42,11 +44,23 @@ curl -fsSLO
> https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
> -cpu cortex-a57 -machine type=virt \
> -m 1024 -smp 2 -display none \
> -machine dumpdtb=binaries/virt-gicv2.dtb
> -# XXX disable pl061 to avoid Linux crash
> -dtc -I dtb -O dts binaries/virt-gicv2.dtb > binaries/virt-gicv2.dts
> -sed 's/compatible = "arm,pl061.*/status = "disabled";/g'
> binaries/virt-gicv2.dts > binaries/virt-gicv2-edited.dts
> -dtc -I dts -O dtb binaries/virt-gicv2-edited.dts > binaries/virt-gicv2.dtb
>
> +# XXX disable pl061 to avoid Linux crash
> +fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled
Currently this test fails with:
+ fdtput binaries/virt-gicv2.dtb -p -t s /pl061@9030000 status disabled
+ [[ boot-cpupools == \b\o\o\t\-\c\p\u\p\o\o\l\s ]]
++ fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle
Error at 'phandle': FDT_ERR_NOTFOUND
Given my other comment below, I would leave this code as is.
> +if [[ "${test_variant}" == "boot-cpupools" ]]; then
> + # Create cpupool node and assign it to domU0
> + cpu_phandle="$(fdtget binaries/virt-gicv2.dtb -t x /cpus/cpu@1 phandle)"
> + cpupool_phandle="0xffffff"
> + fdtput binaries/virt-gicv2.dtb -p -t s /chosen/cpupool compatible
> xen,cpupool
> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool cpupool-cpus
> $cpu_phandle
> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/cpupool phandle
> $cpupool_phandle
> + fdtput binaries/virt-gicv2.dtb -p -t x /chosen/domU0 domain-cpupool
> $cpupool_phandle
> +
> + # Check if domU0 (id=1) is assigned to Pool-1
> + passed="${test_variant} test passed"
> + dom0_check="if xl list -c 1 | grep -q Pool-1; then echo ${passed}; fi"
> +fi
I would prefer to keep the device tree editing here to a minimum and
instead add boot-cpupool support in ImageBuilder and add CPUPOOL* config
options to the existing config file for ImageBuilder created in this
file below. This way, we keep this test cleaner and we help more the
user by proving a way to set boot-cpupools more easily in general, also
useful outside gitlab-ci.
